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.

Sign up for GitHub

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

(PXP-6544) Limited PFB Export from the Files Tab #729

Merged
merged 23 commits into from
Sep 8, 2020
Merged

(PXP-6544) Limited PFB Export from the Files Tab #729

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
0f919d6
Export file PFB if all selected files on same node
em-ingram Aug 19, 2020
24d1b62
Clean up code
em-ingram Aug 31, 2020
588baac
Add portal configuration
em-ingram Aug 31, 2020
2bbcf9d
Add tooltip if file pfb export button is disabled
em-ingram Aug 31, 2020
e1b9517
Show error message on failed PFB export
em-ingram Sep 1, 2020
23f1f66
Revert disabling refreshManifestEntryCount
em-ingram Sep 2, 2020
b7099ad
Use different button types for File tab PFB export
em-ingram Sep 2, 2020
38ebc57
Clean up code
em-ingram Sep 3, 2020
376aa8e
Use job status for error messages instead of FETCH_ERROR
em-ingram Sep 3, 2020
c2b7a70
Revert mistaken debugging change
em-ingram Sep 3, 2020
1200fdc
Fix bug: disable other pfb buttons when file pfb request in-flight
em-ingram Sep 3, 2020
425e92a
Make export File PFB tooltip text less confusing.
em-ingram Sep 3, 2020
7b8c61c
Revert mistakenly uploaded files
em-ingram Sep 3, 2020
fb8dd05
Merge branch 'master' into feat/limited-pfb-file-export
em-ingram Sep 4, 2020
c55a4e9
Refactor: consolidate duplicated code in isButtonEnabled()
em-ingram Sep 4, 2020
49f5dd7
Refactor: move misconfiguration checks to constructor so they only ru…
em-ingram Sep 4, 2020
5beb15e
Merge branch 'master' into feat/limited-pfb-file-export
em-ingram Sep 4, 2020
540c8f3
Fix syntax error in misconfiguration check
em-ingram Sep 4, 2020
0f6fc7b
Fix inconsistent state update
em-ingram Sep 8, 2020
5ba5a17
Fix another inconsistent state update
em-ingram Sep 8, 2020
1b7254d
Link to downloadable PFB files
em-ingram Sep 8, 2020
571ad86
Add documentation
em-ingram Sep 8, 2020
d104636
Refactor
em-ingram Sep 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
198 changes: 191 additions & 7 deletions src/GuppyDataExplorer/ExplorerButtonGroup/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class ExplorerButtonGroup extends React.Component {
pfbStartText: 'Your export is in progress.',
pfbWarning: 'Please do not navigate away from this page until your export is finished.',
pfbSuccessText: 'Your cohort has been exported to PFB! The URL is displayed below.',
// for export to PFB in Files tab
sourceNodesInCohort: [],
// for export to workspace
exportingToWorkspace: false,
exportWorkspaceFileName: null,
Expand All @@ -42,6 +44,12 @@ class ExplorerButtonGroup extends React.Component {
}

componentWillReceiveProps(nextProps) {
if (nextProps.job && nextProps.job.status === 'Failed' && this.props.job.status !== 'Failed') {
this.setState({
toasterOpen: true,
toasterHeadline: this.state.toasterErrorText,
});
}
if (nextProps.job && nextProps.job.status === 'Completed' && this.props.job.status !== 'Completed') {
this.fetchJobResult()
.then((res) => {
Expand Down Expand Up @@ -74,6 +82,14 @@ class ExplorerButtonGroup extends React.Component {
&& nextProps.totalCount) {
this.refreshManifestEntryCount();
}
if (this.props.buttonConfig.enableLimitedFilePFBExport
&& nextProps.filter !== this.props.filter) {
const sourceNodeField = this.props.buttonConfig.enableLimitedFilePFBExport.sourceNodeField;
if (!sourceNodeField) {
throw new Error('Limited File PFB Export is enabled, but \'sourceNodeField\' has not been specified. Check the portal config.');
}
this.refreshSourceNodes(nextProps.filter, sourceNodeField);
}
}

componentWillUnmount() {
Expand Down Expand Up @@ -101,12 +117,28 @@ class ExplorerButtonGroup extends React.Component {
clickFunc = this.exportToTerra;
}
}
if (buttonConfig.type === 'export-files') {
// REMOVE THIS CODE WHEN TERRA EXPORT WORKS
// =======================================
if (terraExportWarning) {
clickFunc = this.exportFilesToTerraWithWarning;
} else {
// =======================================
clickFunc = this.exportFilesToTerra;
}
}
if (buttonConfig.type === 'export-to-seven-bridges') {
clickFunc = this.exportToSevenBridges;
}
if (buttonConfig.type === 'export-files-to-seven-bridges') {
clickFunc = this.exportFilesToSevenBridges;
}
if (buttonConfig.type === 'export-to-pfb') {
clickFunc = this.exportToPFB;
}
if (buttonConfig.type === 'export-files-to-pfb') {
clickFunc = this.exportFilesToPFB;
}
if (buttonConfig.type === 'export-to-workspace') {
clickFunc = this.exportToWorkspace;
}
Expand All @@ -115,7 +147,6 @@ class ExplorerButtonGroup extends React.Component {
}
return clickFunc;
};

getManifest = async (indexType) => {
if (!this.props.guppyConfig.manifestMapping
|| !this.props.guppyConfig.manifestMapping.referenceIdFieldInDataIndex) {
Expand Down Expand Up @@ -320,6 +351,16 @@ class ExplorerButtonGroup extends React.Component {
this.exportToTerra();
}
}
exportFilesToTerraWithWarning = () => {
// If the number of subjects is over the threshold, warn the user that their
// export to Terra job might fail.
if (this.props.totalCount >= terraExportWarning.subjectThreshold) {
this.setState({ enableTerraWarningPopup: true });
} else {
// If the number is below the threshold, proceed as normal
this.exportFilesToTerra();
}
}
// ==========================================

exportToTerra = () => {
Expand All @@ -328,12 +369,24 @@ class ExplorerButtonGroup extends React.Component {
});
};

exportFilesToTerra = () => {
this.setState({ exportingToTerra: true }, () => {
this.exportFilesToPFB();
});
};

exportToSevenBridges = () => {
this.setState({ exportingToSevenBridges: true }, () => {
this.exportToPFB();
});
}

exportFilesToSevenBridges = () => {
this.setState({ exportingToSevenBridges: true }, () => {
this.exportFilesToPFB();
});
}

sendPFBToTerra = () => {
const url = encodeURIComponent(this.state.exportPFBURL);
let templateParam = '';
Expand All @@ -360,6 +413,31 @@ class ExplorerButtonGroup extends React.Component {
});
};

exportFilesToPFB = () => {
if (this.props.buttonConfig.enableLimitedFilePFBExport) {
if (!this.state.sourceNodesInCohort || this.state.sourceNodesInCohort.length !== 1) {
return;
}
const rootNode = this.state.sourceNodesInCohort[0];
this.props.submitJob({
action: 'export-files',
input: {
filter: getGQLFilter(this.props.filter),
root_node: rootNode,
},
});
this.props.checkJobStatus();
this.setState({
toasterOpen: true,
toasterHeadline: this.state.pfbStartText,
});
} else {
/* eslint-disable no-console */
console.error(`Error: Missing \`enableLimitedFilePFBExport\` in the portal config.
Currently, in order to export a File PFB, \`enableLimitedFilePFBExport\` must be set in the portal config.`);
}
};

exportToWorkspace = async (indexType) => {
this.setState({ exportingToWorkspace: true });
const resultManifest = await this.getManifest(indexType);
Expand Down Expand Up @@ -462,6 +540,43 @@ class ExplorerButtonGroup extends React.Component {
}
};

refreshSourceNodes = async (filter, sourceNodeField) => {
try {
const indexType = this.props.guppyConfig.dataType;
const query = `query ($filter: JSON) {
_aggregation {
${indexType} (filter: $filter) {
${sourceNodeField} {
histogram {
key
count
}
}
}
}
}`;
const body = { query, variables: { filter: getGQLFilter(filter) } };
const res = await fetchWithCreds({
path: guppyGraphQLUrl,
method: 'POST',
body: JSON.stringify(body),
});
// eslint-disable-next-line no-underscore-dangle
const sourceNodesHistogram = res.data.data._aggregation[indexType][sourceNodeField].histogram;
const sourceNodes = [];
sourceNodesHistogram.forEach(({ key, count }) => {
if (count > 0) {
sourceNodes.push(key);
}
});
this.setState({
sourceNodesInCohort: sourceNodes,
});
} catch (err) {
throw Error(`Error when getting data types: ${err}`);
}
};

// check if the user has access to this resource
isButtonDisplayed = (buttonConfig) => {
if (buttonConfig.type === 'export-to-workspace' || buttonConfig.type === 'export-files-to-workspace') {
Expand All @@ -483,6 +598,19 @@ class ExplorerButtonGroup extends React.Component {
// disable the pfb export button if any other pfb export jobs are running
return !(this.state.exportingToTerra || this.state.exportingToSevenBridges);
}
if (buttonConfig.type === 'export-files-to-pfb') {
// disable the pfb export button if any other pfb export jobs are running
const otherPFBJobsRunning = this.state.exportingToTerra
|| this.state.exportingToSevenBridges;
Copy link
Collaborator

Choose a reason for hiding this comment

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

why we don't check for isPFBRunning() in here (also in export-to-pfb)?

and there are so many trios of this.state.exportingToTerra || this.state.exportingToSevenBridges || this.isPFBRunning(), maybe we should consolidate them 🤔

Copy link
Contributor Author

@em-ingram em-ingram Sep 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

why we don't check for isPFBRunning() in here (also in export-to-pfb)?

I wasn't sure why and I was scared to change it 😆. But I can't imagine how adding that extra check for isPFBRunning() would break anything, so I'll consolidate the check for this.state.exportingToTerra || this.state.exportingToSevenBridges || this.isPFBRunning() into a single function.

if (otherPFBJobsRunning) {
return false;
}
// If limited file PFB export is enabled, disable the button if the selected
// data files are on more than one source node. (See https://github.com/uc-cdis/data-portal/pull/729)
if (this.props.buttonConfig.enableLimitedFilePFBExport) {
return this.state.sourceNodesInCohort.length === 1;
}
}
if (buttonConfig.type === 'export') {
if (!this.props.buttonConfig.terraExportURL) {
console.error('Export to Terra button is present, but there is no `terraExportURL` specified in the portal config. Disabling the export to Terra button.'); // eslint-disable-line no-console
Expand All @@ -494,9 +622,28 @@ class ExplorerButtonGroup extends React.Component {
|| this.state.exportingToSevenBridges
|| this.isPFBRunning());
}
if (buttonConfig.type === 'export-files') {
if (!this.props.buttonConfig.terraExportURL) {
console.error('Export to Terra button is present, but there is no `terraExportURL` specified in the portal config. Disabling the export to Terra button.'); // eslint-disable-line no-console
Copy link
Contributor

Choose a reason for hiding this comment

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

something worth noting: each re-render (like clicking a checkbox) produces this error print 12 times :O

Copy link
Contributor

@ZakirG ZakirG Sep 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

if it's too much work, no worries, but -- the constructor might be a better place to do these error prints

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ya good idea, I just stuck these checks in the constructor!

return false;
}
// disable the terra export button if any of the
// pfb export operations are running.
const otherPFBJobsRunning = this.state.exportingToTerra
|| this.state.exportingToSevenBridges
|| this.isPFBRunning();
if (otherPFBJobsRunning) {
return false;
}
// If limited file PFB export is enabled, disable the button if the selected
// data files are on more than one source node. (See https://github.com/uc-cdis/data-portal/pull/729)
if (this.props.buttonConfig.enableLimitedFilePFBExport) {
return this.state.sourceNodesInCohort.length === 1;
}
}
if (buttonConfig.type === 'export-to-seven-bridges') {
if (!this.props.buttonConfig.sevenBridgesExportURL) {
console.error('Export to Terra button is present, but there is no `terraExportURL` specified in the portal config. Disabling the export to Terra button.'); // eslint-disable-line no-console
console.error('Export to Seven Bridges button is present, but there is no `sevenBridgesExportURL` specified in the portal config. Disabling the export to Seven Bridges button.'); // eslint-disable-line no-console
return false;
}
// disable the seven bridges export buttons if any of the
Expand All @@ -505,6 +652,25 @@ class ExplorerButtonGroup extends React.Component {
|| this.state.exportingToSevenBridges
|| this.isPFBRunning());
}
if (buttonConfig.type === 'export-files-to-seven-bridges') {
if (!this.props.buttonConfig.sevenBridgesExportURL) {
console.error('Export to Seven Bridges button is present, but there is no `sevenBridgesExportURL` specified in the portal config. Disabling the export to Seven Bridges button.'); // eslint-disable-line no-console
return false;
}
// disable the seven bridges export buttons if any of the
// pfb export operations are running.
const otherPFBJobsRunning = this.state.exportingToTerra
|| this.state.exportingToSevenBridges
|| this.isPFBRunning();
if (otherPFBJobsRunning) {
return false;
}
// If limited file PFB export is enabled, disable the button if the selected
// data files are on more than one source node. (See https://github.com/uc-cdis/data-portal/pull/729)
if (this.props.buttonConfig.enableLimitedFilePFBExport) {
return this.state.sourceNodesInCohort.length === 1;
}
}
if (buttonConfig.type === 'export-to-workspace') {
return this.state.manifestEntryCount > 0;
}
Expand All @@ -519,19 +685,19 @@ class ExplorerButtonGroup extends React.Component {
if (buttonConfig.type === 'export-to-workspace' || buttonConfig.type === 'export-files-to-workspace') {
return this.state.exportingToWorkspace;
}
if (buttonConfig.type === 'export-to-pfb') {
if (buttonConfig.type === 'export-to-pfb' || buttonConfig.type === 'export-files-to-pfb') {
// export to pfb button is pending if a pfb export job is running and it's
// neither an export to terra job or an export to seven bridges job.
return this.isPFBRunning()
&& !(this.state.exportingToTerra || this.state.exportingToSevenBridges);
}
if (buttonConfig.type === 'export') {
if (buttonConfig.type === 'export' || buttonConfig.type === 'export-files') {
// export to terra button is pending if a pfb export job is running and
// it's an exporting to terra job.
return this.isPFBRunning()
&& this.state.exportingToTerra;
}
if (buttonConfig.type === 'export-to-seven-bridges') {
if (buttonConfig.type === 'export-to-seven-bridges' || buttonConfig.type === 'export-files-to-seven-bridges') {
// export to seven bridges button is pending if a pfb export job is running
// and it's an export to seven bridges job.
return this.isPFBRunning()
Expand All @@ -553,7 +719,25 @@ class ExplorerButtonGroup extends React.Component {
} else if (buttonConfig.type === 'manifest' && this.state.manifestEntryCount > 0) {
buttonTitle = `${buttonConfig.title} (${humanizeNumber(this.state.manifestEntryCount)})`;
}
const btnTooltipText = (this.props.isLocked) ? 'You only have access to summary data' : buttonConfig.tooltipText;

let tooltipEnabled = false;
let btnTooltipText = '';
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpicking: seems you can set the default value for these 2 the same from L728-729 and get rid of the else {}


// If limited file PFB export is enabled, PFB export buttons will be disabled
// if the user selects multiple files that are on different nodes in the graph.
// (See https://github.com/uc-cdis/data-portal/pull/729).
// If the user has selected multiple files on different nodes, display a
// tooltip explaining that the user can only export files of the same type.
const isFilePFBButton = buttonConfig.type === 'export-files' || buttonConfig.type === 'export-files-to-pfb' || buttonConfig.type === 'export-files-to-seven-bridges';
if (this.props.buttonConfig.enableLimitedFilePFBExport
&& isFilePFBButton
&& this.state.sourceNodesInCohort.length > 1) {
tooltipEnabled = true;
btnTooltipText = 'Currently you cannot export files with different Data Types. Please choose a single Data Type from the Data Type filter on the left.';
} else {
tooltipEnabled = buttonConfig.tooltipText ? !this.isButtonEnabled(buttonConfig) : false;
btnTooltipText = (this.props.isLocked) ? 'You only have access to summary data' : buttonConfig.tooltipText;
}

return (
<Button
Expand All @@ -565,7 +749,7 @@ class ExplorerButtonGroup extends React.Component {
className='explorer-button-group__download-button'
buttonType='primary'
enabled={this.isButtonEnabled(buttonConfig)}
tooltipEnabled={buttonConfig.tooltipText ? !this.isButtonEnabled(buttonConfig) : false}
tooltipEnabled={tooltipEnabled}
tooltipText={btnTooltipText}
isPending={this.isButtonPending(buttonConfig)}
/>
Expand Down
1 change: 1 addition & 0 deletions src/GuppyDataExplorer/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ class Explorer extends React.Component {
terraExportURL: explorerConfig[this.state.tab].terraExportURL,
terraTemplate: explorerConfig[this.state.tab].terraTemplate,
sevenBridgesExportURL: explorerConfig[this.state.tab].sevenBridgesExportURL,
enableLimitedFilePFBExport: explorerConfig[this.state.tab].enableLimitedFilePFBExport,
}}
history={this.props.history}
tierAccessLevel={tierAccessLevel}
Expand Down