Register New User form shows error after successfully creating account

[shaunanoordin]

UI Error

Package: probably lib-react-components's AuthModal (unconfirmed)

When a user attempts to register a new Zooniverse user account via the Register New User form, the account will be successfully created - HOWEVER, the form will display an error and refuse to close.

Process basically looks like this:

• User opens Register New User form and submits details for UsernameX.
• Register New User form doesn't close, and displays error: n is not a function
  • but actually, the process was successful, and the page shows the user logged in as UsernameX
• User attempts to re-register UsernameX, only to be informed that UsernameX already exists.
• User gets frustrated. This is bad.

❗ This appears to affect all users currently, meaning our Register New User pipeline is borked.

Screenshot: after clicking "Register" (submit), the Register New User form displays the "n is not a function" error.

Notes:

• Confirmed with Chrome and Firefox, at minimum.
• Reports seem to have gone as far back as 22 Jan (Freshdesk) and 7 Feb (Slack), but the scale/importance of the issue wasn't registered.
• PFE's "register new user" form seems to work OK.
• Backend's "register new user" functionality seems to work OK.
• Update: issue only occurs on the FEM home page, not any FEM project pages.

See Slack: thread 1, thread 2

Replication

• Go to any FEM page the FEM home page, e.g. i.e. the Zooniverse home page https://www.zooniverse.org/
  • Make sure you're not logged in. Log out, or use an incognito window.
• Click on the "Register" button on the top menu, to open the Register New User form (modal).
• Fill in the details for a test user. Click on the "Register" (submit) button at the bottom of the form.
  • Expected: user account created. User is logged in on FEM. Form should close.
  • Actual: user account created. User is logged in on FEM. Form does not close, and an error message appears.

Confirmed with macOS + Chrome 133 (Incognito), Firefox 135 (Privacy), on Zooniverse homepage, on production.

Update: you won't see this issue if you attempt to register a new user from an FEM project page.

Status

High priority issue, as this affects the new user registration process.

Aiming to get a fix done by this week.

Update 1:

• Cause identified:
  1. lib-react-component's RegisterFromContainer.js will crash on a successful submit action, if onSignIn argument isn't defined.
  2. A few instances of <AuthModal> (which uses RegisterFormContainer) is missing onSignIn argument.
• Issue ONLY affects certain FEM pages. e.g. you can register successfully on a project page, but get an error on the home page. (i.e. app-project is fine, app-root has errors)
• Discussing plans for solution now.

[shaunanoordin]

Testing Notes

Here's an ongoing note of all my tests. All tests are done with macOS + Chrome 113 incognito, unless otherwise noted.

Initial tests:

• Target: live Zooniverse server (production)
• Page where I opened the Register New User form: home page (www.zooniverse.org)
• Results: "n is not a function" error
• Browsers: Chrome 133 incognito, Firefox 135 private

Staging tests:

• Target: Zooniverse server (staging)
• Page where I opened the Register New User form: home page (www.zooniverse.org?env=staging)
• Results: "n is not a function" error

These tests indicate that the issue is consistent across browsers, and across production/staging.

Localhost tests 1:

• Target: app-project (staging)
• Page: home page (https://local.zooniverse.org:3000/) ... which is technically a 404 on app-staging
• Results: success (?!?)

Localhost tests 2:

• Target: app-project (production)
• Page: home page (https://local.zooniverse.org:3000/?env=production) ... also a 404
• Results: success (?!?)

Localhost tests 1 & 2 indicate that problem can't be replicated on app-project alone, and once again the results are consistent across staging & production.

Follow up:

• What about app-root?
• Shaun, can you please examine what the missing n is?
  • OK, I will.

[shaunanoordin]

Investigation Notes

Examining the n is not a function error more closely, I see this on the Chrome source panel:

This looks pretty familiar:

File: RegisterFormContainer.js

export default function RegisterFormContainer({
  closeModal = DEFAULT_HANDLER,
  project = null,
  onSignIn  // Wait, why ain't there no DEFAULT_HANDLER here?
}) {
  ...

  async function onSubmit(values, { setFieldError, setSubmitting }) {
    ...
    try {
      const user = await auth.register(valuesToSubmit)
      onSignIn(user)  // Hey are you "n" ??
      setSubmitting(false)
      closeModal()
    } catch (error) {
      ...
    }
  }
}

Hang on, I see something here... RegisterFormContainer always assumes the onSignIn argument/function exists, and doesn't provide any fallback. So in theory, if a parent component forgets to provide onSignIn, it'll cause "this ain't a function" error. 🤔

Hypohesis: app-project's RegisterFormContainer is providing the onSignIn argument. However, app-root's(???) RegisterFromContainer forgot to set onSignIn.

Follow up:

• Examine every instance where (or more likely, the parent <AuthModal>) is used, and see if onSignIn is provided.
• Test Register New User on app-root (localhost)
• Test Register New User on an FEM page that's NOT the home page, e.g. https://frontend.preview.zooniverse.org/projects/darkeshard/test-project-2022 (a Project page)

[shaunanoordin]

Testing Notes

Production tests 2:

Target: frontend.preview (production, I think)
Page: Project page (https://frontend.preview.zooniverse.org/projects/darkeshard/test-project-2022)
Results: success

Localhost tests 3:

Target: app-root (staging)
Page: home page (https://local.zooniverse.org:3000/)
Results: "onSignIn is not a function" error

Gotcha!

[eatyourgreens]

Might date back to this?

• app-project: Use shared AuthModal component #5749.

[shaunanoordin]

Investigation Notes

Searching for every instance of <AuthModal>, I see the following:

app-project's `PageHeader.js` DOES provide onSignIn to `AuthModal` ✔️

<AuthModal
  activeIndex={authModalActiveIndex}
  closeModal={closeAuthModal}
  onActive={setAuthModalActiveIndex}
  onSignIn={onSignIn}
/>

app-root's `PageHeader.js` DOESN'T provide onSignIn to `AuthModal` ❌

<AuthModal
  activeIndex={activeIndex}
  closeModal={closeAuthModal}
  onActive={setActiveIndex}
/>

lib-content's `Introduction.js` DOESN'T provide onSignIn to `AuthModal` ❌

<AuthModal
  activeIndex={activeIndex}
  closeModal={closeAuthModal}
  onActive={setActiveIndex}
/>

lib-react-component's `AuthModal.stories.js` DOESN'T provide onSignIn to `AuthModal` ❌

<AuthModal activeIndex={activeIndex} closeModal={closeModal} onActive={setActiveIndex} />

<AuthModal activeIndex={activeIndex} closeModal={closeModal} onActive={setActiveIndex} />

lib-user's `dev/App.js` doesn't provide onSignIn to AuthModal, but it's a dev thing, so no worries 🤷

<AuthModal
  activeIndex={activeIndex}
  closeModal={closeAuthModal}
  onActive={setActiveIndex}
/>

Key Takeaways

• The "Register New User form shows error after successfully creating account" issue ONLY occurs on specific pages.
• If you try to register a new user account from an FEM project page, you won't see any errors.
• If you try to do the same from the FEM home page, you will receive the error.
  • Note that there are TWO Register buttons on the FEM home page - one on the top menu (app-root/PageHeader) and one on the page content under Introductions (lib-content/Introduction, I think)

Status

Cause of the issue has been identified. Now to determine the solution:

• Add onSignIn() where it's missing:
  • on app-root's PageHeader.js
  • on lib-content's Introduction.js
  • on lib-react-component's AuthModal.stories.js
• Make lib-react-component's RegisterFormContainer more robust
  • e.g. add onSignIn = DEFAULT_HANDLER
  • I'm in favour of this, though I can see the counterargument which says "hey, if OnSignIn isn't provided, then this whole endeavour should fail, because otherwise it'll lead to problems where the user account is created but the app isn't signed in. Or something."
  • Alternatively, just throw an error on rendering if onSignIn isn't defined, so the error is super obvious

[shaunanoordin]

Might date back to this?

• app-project: Use shared AuthModal component #5749.

Looks like you're right! That PR introduces onSignIn to AuthModal but doesn't provide a DEFAULT_HANDLER. (Now that I think of it, I think that's the only requirement to patch this problem - add the default handler.)

Thanks Jim, this helps confirm that the issue goes back to... 15 Dec 2023, wait what, 2023?? EDIT: Wait, no, it was introduced on 15 Dec 2023, but wouldn't affect actual users until the new home page officially switched over for them.

[eatyourgreens]

The new code doesn’t store the signed-in user anywhere, so I think onSignIn is an optional callback. Adding a default should fix it.

Adding a test to catch this bug seems like it would be straightforward too. Maybe a test in the root app that registers a new user?

[eatyourgreens]

NB. the reason that onSignIn is optional is that PJC/auth is stateful, and stores the signed-in user for you.

The original design of the project app assumed a stateless client, with the assumption that auth state is the responsibility of the app, not the API client.

[eatyourgreens]

You could also change the code to:

onSignIn?.()

Just in case someone overrides the default with null? Seems unlikely though.

[eatyourgreens]

The nullish coalescing operator may be used after optional chaining in order to build a default value when none was found:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining#combining_with_the_nullish_coalescing_operator

[shaunanoordin]

lol I actually did have onSignIn?.() in my patch initially, but then my brain immediately sabotaged me by suggesting "but what if someone wrote <AuthModal onSignIn={'hahasuckitthisisastring'}>"... at which point I decided that if someone's that determined to muck up the code, then they deserve whatever error they get.

[eatyourgreens]

You see, that’s where TypeScript can really be useful.

Come to think of it, TypeScript would have caught this and stopped it from building.