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

[DUOS-2158][risk=no] Swap default pages for researcher tab #1877

Merged
merged 2 commits into from
Nov 8, 2022

Conversation

rushtong
Copy link
Contributor

@rushtong rushtong commented Nov 3, 2022

Addresses

https://broadworkbench.atlassian.net/browse/DUOS-2158

The default page for the researcher console is now set to the dataset catalog instead of the user's DARs.

Old

Screen Shot 2022-11-03 at 5 31 43 AM

New

Screen Shot 2022-11-03 at 5 31 49 AM


Have you read Terra's Contributing Guide lately? If not, do that first.

  • Label PR with a Jira ticket number and include a link to the ticket
  • Label PR with a security risk modifier [no, low, medium, high]
  • PR describes scope of changes
  • Get a minimum of one thumbs worth of review, preferably two if enough team members are available
  • Get PO sign-off for all non-trivial UI or workflow changes
  • Verify all tests go green
  • Test this change deployed correctly and works on dev environment after deployment

@rushtong rushtong requested a review from a team as a code owner November 3, 2022 09:32
{ label: 'DAR Requests', link: '/researcher_console' },
{ label: 'Data Catalog', link: '/dataset_catalog' }
{ label: 'Data Catalog', link: '/dataset_catalog' },
{ label: 'DAR Requests', link: '/researcher_console' }
Copy link
Contributor

Choose a reason for hiding this comment

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

is /researcher_console an inaccurate url now that it isn't the default? I could see this being a big change though, considering we likely have links pointing to this page in many places, so maybe not worth changing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is /researcher_console an inaccurate url now that it isn't the default?

Changing that is outside the scope of what I'm trying to do here. The only goal is to arrange the default tabs for the researcher.

@connorlbark
Copy link
Contributor

The admin console still has the DAR Requests first but when you click on Researcher it acts as if it is still under the Admin header, like this:
image
fixing the navbar might be outside of the scope of this ticket, but just something I noticed which this ticket makes look awkward.

Copy link
Contributor

@connorlbark connorlbark left a comment

Choose a reason for hiding this comment

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

other than the thoughts above (which don't really have any action items for this ticket), this looks good!

Copy link
Contributor

@quazi-h quazi-h left a comment

Choose a reason for hiding this comment

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

Change looks pretty straightforward herebut I'm noticing the same behavior that Connor pointed out.

Clicking on the Researcher Console tab in the header navigates me to https://local.broadinstitute.org:3000/dataset_catalog but the Header shows one of the other consoles being selected. If already in the DAC Chair Console or DAC Member Console, clicking on the "Researcher Console" switches to one of the "Datasets" tab within the Chair or Member consoles. Otherwise it navigates to the Admin Console / Dataset Catalog and I can not access the Researcher Console / DAR Requests tab. Not sure how to address this.

@rushtong
Copy link
Contributor Author

rushtong commented Nov 3, 2022

fixing the navbar might be outside of the scope of this ticket, but just something I noticed which this ticket makes look awkward.

There is some weirdness with the selected tab state - I'll dig into that.

@rushtong
Copy link
Contributor Author

rushtong commented Nov 3, 2022

@quazi-broad @connorlbark - Connor fixed the bug, PTAL

Copy link
Contributor

@quazi-h quazi-h left a comment

Choose a reason for hiding this comment

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

The Researcher Console tabs are working correctly with the new changes. Everything looks good!

Copy link
Contributor

@connorlbark connorlbark left a comment

Choose a reason for hiding this comment

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

👍 updating approval

@rushtong rushtong merged commit 335ca02 into develop Nov 8, 2022
@rushtong rushtong deleted the DUOS-2158 branch November 8, 2022 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants