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

Detect if first login #6262

Open
wants to merge 7 commits into
base: production
Choose a base branch
from
Open

Detect if first login #6262

wants to merge 7 commits into from

Conversation

CarolineDenis
Copy link
Contributor

@CarolineDenis CarolineDenis commented Feb 20, 2025

Fixes #6211

Checklist

  • Self-review the PR after opening it to make sure the changes look good and
    self-explanatory (or properly documented)
  • Add relevant issue to release milestone

Testing instructions

  • Open a blank db (with no institutions)
  • Verify that you land on an empty screen displaying the message "Welcome! No institutions are available at the moment."

@CarolineDenis CarolineDenis added this to the 7.11 milestone Feb 20, 2025
@CarolineDenis CarolineDenis requested review from sharadsw, acwhite211 and a team and removed request for a team, acwhite211 and sharadsw February 25, 2025 15:18
@CarolineDenis CarolineDenis requested review from acwhite211, sharadsw and a team February 25, 2025 16:04
Copy link
Member

@grantfitzsimmons grantfitzsimmons left a comment

Choose a reason for hiding this comment

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

This code suggests that there may be multiple institution records in a single database, both in what is displayed to the user and in the API endpoint. Currently, I'm not aware of any plans to support more than one institution per database, but perhaps this decision was made with the understanding that this may not always be the case.

Is this intentional, or should it return only one institution at any given time?

@grantfitzsimmons grantfitzsimmons requested a review from a team February 25, 2025 16:59
Copy link
Member

@grantfitzsimmons grantfitzsimmons left a comment

Choose a reason for hiding this comment

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

Testing instructions

  • Open a blank db (with no institutions)
  • Verify that you land on an empty screen displaying the message "Welcome! No institutions are available at the moment."
Screenshot 2025-02-25 at 11 05 56 AM

@grantfitzsimmons grantfitzsimmons requested a review from a team February 25, 2025 17:07
@CarolineDenis
Copy link
Contributor Author

@grantfitzsimmons,
updated to single institution

Copy link
Contributor

@sharadsw sharadsw left a comment

Choose a reason for hiding this comment

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

Code looks fine functionally. However, I think we should reconsider the route we're creating, especially after 0f25eee (sorry!)

@CarolineDenis CarolineDenis requested a review from sharadsw March 4, 2025 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Dev Attention Needed
Development

Successfully merging this pull request may close these issues.

Add a new way to detect if the user connects for the first time
3 participants