Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a new property to turn on/off GSVA features #2823

Merged
merged 1 commit into from Nov 15, 2019

Conversation

ngocnn1104
Copy link
Contributor

Implemented cBioPortal/cbioportal#6749

Changes:

  • Added skin_show_gsva to frontendConfig with default value = false;
  • When there is a need to show the GSVA feature, go to src/config/serverConfigDefaults.ts and change skin_show_gsva: false, to skin_show_gsva: true,;
  • Updated local e2e test with a function that manually updates skin_show_gsva in frontendConfig to true.

@@ -26,11 +27,15 @@ const styles = styles_any as {
@observer
export default class MolecularProfileSelector extends QueryStoreComponent<{}, {}>
{
private get showGSVA() {
Copy link
Collaborator

@alisman alisman Nov 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should only interface with configuration via AppConfig. So this would be AppConfig.serverConfig.skin_show_gsva.
I've tested it out and manipulating this via frontendConfig window global (as you do above in test) should still work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alisman Thanks. I have updated the PR. So when should we use getBrowserWindow()?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally never! it's a global and should be avoided. it's in the code base as a convenience to avoid having to pass, say, the routing store into the view layers. but really it's not a good practice. if you for some reason you find yourself needing to use it, let me know.

@Sjoerd-van-Hagen
Copy link

@alisman what still needs to be done to get this one merged?

Signed-off-by: Ngoc Nguyen <nguyennhungoc1104@gmail.com>
@alisman alisman merged commit 06eac14 into cBioPortal:master Nov 15, 2019
@alisman alisman deleted the config_show_gsva branch November 15, 2019 21:08
@inodb inodb added the feature label Nov 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants