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-1815][risk=no] New Researcher Profile Page #1672

Merged
merged 11 commits into from
Jul 18, 2022
Merged

Conversation

connorlbark
Copy link
Contributor

@connorlbark connorlbark commented Jul 13, 2022

Addresses

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

Notes:

  • Calls new PUT /api/user/me endpoint on profile page - endpoint created at [DUOS-1815][risk=low] Add update own user properties endpoint consent#1649 , so must use that backend branch for testing.
  • Changes the Institution and Signing Official search bars to allow freetext entry (press enter after searching) separately from selecting a pre-existing institution. If set, will update suggestedInstitution/suggestedSigningOfficial instead of sending the ids.
  • The page is different if you are a research or a signing official. Signing officials cannot change their institution once set. Researchers may change their institution and signing officials anytime.

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

@connorlbark connorlbark marked this pull request as ready for review July 13, 2022 14:07
@connorlbark connorlbark requested a review from a team as a code owner July 13, 2022 14:07
@lu-c
Copy link
Contributor

lu-c commented Jul 14, 2022

Hi!

I'm seeing some differences in the branch between the mock ups - Am I running it wrong?

  1. "An eRA Commons ID will be required to submit a dar." <-- The eRA commons id is a link in the mock up, but seems like just text here
  2. The checklist with "what are you trying to do" isn't showing up - Reading the ticket, I think it might be out of scope?

@connorlbark
Copy link
Contributor Author

Hi!

I'm seeing some differences in the branch between the mock ups - Am I running it wrong?

  1. "An eRA Commons ID will be required to submit a dar." <-- The eRA commons id is a link in the mock up, but seems like just text here
  2. The checklist with "what are you trying to do" isn't showing up - Reading the ticket, I think it might be out of scope?

oops, you're right about the link eRA commons link! and yes, i believe there is another ticket for the 'what are you trying to do' box

src/libs/ajax.js Outdated Show resolved Hide resolved
src/pages/ResearcherProfile.js Show resolved Hide resolved
@connorlbark
Copy link
Contributor Author

connorlbark commented Jul 14, 2022

note: just found a bug in the backend code for getting signing officials which breaks the frontend, investigating and will see if i can fix
edit: has been fixed in the linked backend pull request

Copy link
Contributor

@lu-c lu-c left a comment

Choose a reason for hiding this comment

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

Looks good! :)

@shaemarks
Copy link
Contributor

Not sure about the intended behavior, but I was able to delete my profileName and save it as empty, and then when I tried to submit a DAR, part 1 of the application wouldn't load

@shaemarks
Copy link
Contributor

This is probably better suited for a different ticket, but I could only see profile name changes appear in the topleft corner of the header if I sign out and sign back in

@rushtong
Copy link
Contributor

This is probably better suited for a different ticket, but I could only see profile name changes appear in the topleft corner of the header if I sign out and sign back in

I agree, we could do a better job of refreshing the user saved in storage. There are other scenarios where this happens as well, such as role additions/removals

@rushtong
Copy link
Contributor

rushtong commented Jul 15, 2022

Not sure about the intended behavior, but I was able to delete my profileName and save it as empty, and then when I tried to submit a DAR, part 1 of the application wouldn't load

@connorlbark Please do add a validation check for this - we should not be setting the user name to "".

@connorlbark
Copy link
Contributor Author

added validation to make sure the username is at least four characters (that seems reasonable to me, let me know if there is a preference for another length or just ensuring it's not empty)

Copy link
Contributor

@shaemarks shaemarks left a comment

Choose a reason for hiding this comment

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

I left a couple of small suggestions, but looks and works great!

src/components/SearchSelectOrText.js Show resolved Hide resolved
src/components/SearchSelectOrText.js Outdated Show resolved Hide resolved
src/pages/ResearcherProfile.js Outdated Show resolved Hide resolved
@connorlbark
Copy link
Contributor Author

added:

  • validation to the signing official (makes sure it's a valid email if you're suggesting)
  • updates manual on blur (so if you click off) or if you hit enter

Copy link
Contributor

@shaemarks shaemarks left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for the changes!

@connorlbark connorlbark merged commit 3c90eb2 into develop Jul 18, 2022
@connorlbark connorlbark deleted the DUOS-1815 branch July 18, 2022 18:40
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.

4 participants