-
Notifications
You must be signed in to change notification settings - Fork 4
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
DT-903: Change export button to link directly to terra-ui import page #2711
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
love the simplification 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At a minimum we probably want a bit of spacing between the links. |
} | ||
|
||
return null; | ||
const link = `${terraUrl}/#import-data?snapshotId=${snapshot.id}&format=tdrexport&tdrSyncPermissions=false`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the code in TDR which also generates this link for import https://github.com/DataBiosphere/jade-data-repo/blob/develop/src/main/java/bio/terra/service/snapshotbuilder/SnapshotBuilderService.java#L475
The link's behavior is implemented in terra-ui: https://github.com/DataBiosphere/terra-ui/blob/dev/src/import-data/useImportRequest.ts#L95
Addresses
https://broadworkbench.atlassian.net/browse/DT-903
Instead of performing a two-step operation in the duos UI, use the new TDR import link to have terra-ui perform the TDR export step. The same APIs are called in both cases, so other than the UX differences there is no difference in behavior.
Summary
Video clip showing behavior, note that this is trying to export an azure dataset so the final step is blocked, but everything else is working correctly.
export.mov