DT-530: Add conditional rendering around apply for access #2718
Conversation
rushtong
requested review from
pshapiro4broad and
rjohanek
and removed request for
a team
Yes, I should do that! |
src/pages/DatasetStatistics.jsx
Outdated
| setIsLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| const accessButton = () => { | ||
| const accessManagement = extract('Access Management').toLowerCase(); | ||
| if (accessManagement === 'controlled') { |
There was a problem hiding this comment.
Would a switch work here? I think using switch would make it clearer that each if is comparing against the same thing, and that each if clause is terminal.
If you used a functional style switch (like lodash https://lodash.com/docs/4.17.15#cond), you could say return _.cond(...);.
There was a problem hiding this comment.
Yes, this could be cleaner with a switch. I will look into cond - have not used that before.
There was a problem hiding this comment.
I found some unintended behavior with using cond (hrefs were being removed from anchor elements) so I stuck with a switch approach.
src/pages/DatasetStatistics.jsx
Outdated
| } | ||
| </span>; | ||
| } | ||
| return <div/>; |
There was a problem hiding this comment.
I see accessManagement is type string, do we expect other values here?
There was a problem hiding this comment.
Just string values come through. On the consent side, we do have an enum so we can actually enforce this here as well.
src/pages/DatasetStatistics.jsx
Outdated
| setIsLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| const accessButton = () => { |
There was a problem hiding this comment.
Is this name correct? It only returns a button in one case.
There was a problem hiding this comment.
I'll rename it to make it more reflective of what it's doing now that it's not just a button any more.
Addresses
https://broadworkbench.atlassian.net/browse/DT-530
Summary
Note to developers: I suggest hiding whitespace due to the large number of lint changes. I modified the datasets seen in the screenshots so they have URL values. Most dev datasets do not have them.
This PR makes the following modifications to the individual dataset page:
alias(this is a bug fix)In the Open/External cases, we only render a link (and relevant link language) if one exists as a URL property on the dataset.
Controlled Access (no changes)
Open Access
External Access
Have you read Terra's Contributing Guide lately? If not, do that first.