new changes for B3 WALDER project to make changes for discovery porta… #1551
Conversation
|
Created temp branch for testing: |
src/Discovery/DiscoveryActionBar/components/OpenFillRequestFormButton.tsx
Outdated
Show resolved
Hide resolved
| <React.Fragment> | ||
| | ||
| <a target='_blank' rel='noreferrer' href='https://pandemicresponsecommons.org'> | ||
| {'Pandemic Response Commons Website'} |
There was a problem hiding this comment.
I hope we don't have to hardcode these env specific values in here
the url can be inferred from localconf, the text might be also about to be pulled from some value there
if not, maybe put them in configs
There was a problem hiding this comment.
I was planning to put it in config but I wasnt sure that it would be a good idea to just have config for value which will not change ever. I can add it into config if needed
There was a problem hiding this comment.
Put variable in the portal config for URL and Text, add external link icon
There was a problem hiding this comment.
external link icon in portal https://github.com/uc-cdis/data-portal/blob/master/src/components/layout/FooterNIAID.jsx#L162-L164
src/Discovery/DiscoveryActionBar/components/OpenFillRequestFormButton.tsx
Outdated
Show resolved
Hide resolved
| } | ||
| combinedIds += item._medical_sample_id; | ||
| }); | ||
| const url = `${props.config.features.exportToWorkspace.fillRequestFormURL}?query=${encodeURIComponent(combinedIds)}`; |
There was a problem hiding this comment.
| const url = `${props.config.features.exportToWorkspace.fillRequestFormURL}?query=${encodeURIComponent(combinedIds)}`; | |
| const url = `${props.config.features.exportToWorkspace.fillRequestFormURL}query=${encodeURIComponent(combinedIds)}`; |
No need to check here, already done in L28 by the ternary operator
There was a problem hiding this comment.
also combinedIds here might be empty, unless _medical_sample_id is guaranteed to exists in the metadata of all selectable studies, consider if you need some extra error handling for that case here
There was a problem hiding this comment.
also
combinedIdshere might be empty, unless_medical_sample_idis guaranteed to exists in the metadata of all selectable studies, consider if you need some extra error handling for that case here
OCC is pushing data into this mds by a semi-automated system that parses the file, appends a medical ID, and then pushes it to mds. Even though we want to put an error handling, I have to put a UI error handling in this case, can you tell me what reference in gen3 I can take and replicate here?
There was a problem hiding this comment.
No need to check here, already done in L28 by the ternary operator
That is not a ternary operator, that is a URL generated with a query parameter that requires a question mark
There was a problem hiding this comment.
Disable checkbox if _medical_sample_id doesn't exist also add check into code if combinedIds are empty
| const url = `${props.config.features.exportToWorkspace.fillRequestFormURL}?query=${encodeURIComponent(combinedIds)}`; | ||
| window.open(url, '_blank'); | ||
| } : () => { | ||
| window.open(props.config.features.exportToWorkspace.fillRequestFormURL, '_blank'); |
There was a problem hiding this comment.
why it should open this URL in this case, where fillRequestFormURL doesn't exists?
There was a problem hiding this comment.
Technically it should not redirect anywhere else as there is nothing setup
There was a problem hiding this comment.
if fillRequestFormURL doesn't exist, hide the button request Access
src/Discovery/DiscoveryActionBar/components/OpenFillRequestFormButton.tsx
Outdated
Show resolved
Hide resolved
src/Discovery/DiscoveryActionBar/components/OpenFillRequestFormButton.tsx
Outdated
Show resolved
Hide resolved
src/Discovery/DiscoveryListView.tsx
Outdated
| props.config.features.exportToWorkspace | ||
| && props.config.features.exportToWorkspace.enabled | ||
| (props.config.features.exportToWorkspace | ||
| && props.config.features.exportToWorkspace.enabled) || ((props.config.features.exportToWorkspace?.enableFillRequestForm |
There was a problem hiding this comment.
we probably should consider renaming the config section exportToWorkspace to something else since it is more like an action button bar rather than just for exporting to ws
but this is more like a note to ourselves, not a item that in the scope of your PR
There was a problem hiding this comment.
Are you still planning to merge PR when all comments are resolved? We’d recommend that as we have a deadline to be met
…rvi-occ/data-portal into feat/b3_walder_discovery_prc_2
…into feat/b3_walder_discovery_prc_2 resolving conflict
Improvements
changes in the discovery portal for the new search portal in PRC QA. I created this PR to test it in QA. I had previous PRs in #1392 and #1394 but I had to make a new commit to incorporate changes for new DiscoeryActionBar folder and to resolve conflicts.
Currently, This PR should NOT be merged, I only need this PR to be here so I can use this in QA COVID 19
similar to this
OCC is working on getting this PR to be merged by end of Q3 2024