Skip to content

Conversation

amy-corson-ibigroup
Copy link
Contributor

Description:
Fixes a bug where if a user is on the verify email pane and they try to change the locale of the page, the page would reload and reset to the previous language. The issue was coming from a failed user update because the isNewUser check was returning true (resulting in a PUT request instead of PATCH) even though the user already exists in the database at this point.

PR Checklist:

  • Does the code follow accessibility standards (WCAG 2.1 AA Compliant)?
  • Are all languages supported (Internationalization/Localization)?
  • Are appropriate Typescript types implemented?

Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

Rename the method, it was confusing to read at first, and also please fix the related bug that you uncovered too.

return !loggedInUser.hasConsentedToTerms
}

export function userExistsInDatabase(loggedInUser) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you mean userDoesNotExistInDatabase or userNotInDatabase?


// Determine URL and method to use.
const isCreatingUser = isNewUser(loggedInUser)
const isCreatingUser = userExistsInDatabase(loggedInUser)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought the user entry in Mongo was meant to be created when they click "Next" after accepting the terms of use. I think you uncovered a bug that results in the user being created when they set the language and before they agree to the terms. Congrats!

To fix the bug:
In lib/actions/ui.js near line 471: replace

      if (loggedInUser) {

with

      if (!userNotInDatabase(loggedInUser)) {

(with correct import and corrected name/meaning.

}

export function userExistsInDatabase(loggedInUser) {
return !loggedInUser.id
Copy link
Collaborator

Choose a reason for hiding this comment

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

loggedInUser might be null (after reloading the 'verify' page), so use the ?. optional chaining syntax.

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.

2 participants