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

Option to disable data download but still allow figure download #4662

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

7xuanlu
Copy link
Contributor

@7xuanlu 7xuanlu commented Jul 7, 2023

Fix cBioPortal/cbioportal#10180

Describe changes proposed in this pull request:

  • disable data download in study view, group comparison view, result view, and patient view.
  • support enum SHOW_ALL, SHOW_DATA, HIDE_ALL in frontend based on the config in backend skin.hide_download_controls
  • pass showCopyDownload or showDownload as props to components to customize its behavior

Related backend PR cBioPortal/cbioportal#10258

@@ -68,9 +69,27 @@ export default class Container extends React.Component<IContainerProps, {}> {
}

get appContext(): IAppContext {
const downloadConfig: string = getServerConfig()
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kalletlak, will this be backward compatible? if they had it as false in portal.properties, it seems it would now default to true?

Choose a reason for hiding this comment

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

Yeah, looks like not backward compatible. I'll discuss and let you know.

src/config/serverConfigDefaults.ts Outdated Show resolved Hide resolved
@alisman
Copy link
Collaborator

alisman commented Jul 14, 2023

The context api is a legitimate way to do this but it departs from the usual way of accessing global settings loaded from server, which is getServerConfig(). I think since that's already established, we might as well keep using it. Let me know if you see a reason otherwise.

@kalletlak
Copy link
Member

The context api is a legitimate way to do this but it departs from the usual way of accessing global settings loaded from server, which is getServerConfig(). I think since that's already established, we might as well keep using it. Let me know if you see a reason otherwise.

You mean to replace context with getServerConfig() for showDownloadControls?

@kalletlak
Copy link
Member

@kalletlak kalletlak force-pushed the download-option-data branch 2 times, most recently from 45d5783 to 2ace835 Compare July 17, 2023 17:27
Support 'data' option and disable data download on Study page and Group Comparison page
@kalletlak kalletlak force-pushed the download-option-data branch from 2ace835 to c306862 Compare July 17, 2023 20:08
@kalletlak kalletlak changed the title Download option data Option to disable data download but still allow figure download Jul 17, 2023
@kalletlak kalletlak merged commit 8e13244 into cBioPortal:master Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

option to disable data download but still allow figure download
6 participants