fix(signup): add a route to login when the email already has an account - #8177
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe signup flow now records the submitted email, filters matching errors, redirects existing accounts to login, and displays account-specific notifications. Form controls support immediate invalid styling. Invite tests cover login access, enabled signup, and duplicate-email errors. HomePage routing and notification icon usage were also updated. Estimated code review effort: 3 (Moderate) | ~20 minutes ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
87b819f to
dfb4c6f
Compare
Docker builds report
|
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 94067142-2c12-4593-81ed-cb922a446112
📒 Files selected for processing (8)
frontend/e2e/tests/invite-test.pw.tsfrontend/web/components/base/forms/Input.tsxfrontend/web/components/base/forms/InputGroup.tsxfrontend/web/components/pages/home-page/HomePage.scssfrontend/web/components/pages/home-page/HomePage.tsxfrontend/web/components/pages/home-page/index.tsfrontend/web/routes.jsfrontend/web/styles/project/_forms.scss
💤 Files with no reviewable changes (1)
- frontend/web/styles/project/_forms.scss
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19002 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-16 — run #19002 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19002 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #19002 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #19001 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19001 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19001 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #19001 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18966 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #18966 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-16 — run #18966 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18965 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
|
Submitting with an address that already has an account produced "Email already exists. Please log in." and nothing on the page that logs you in. /signup had no link to login at all, and on the invite screen the only one sat above the first field, three fields from the error. #5077 moved it into the invite-only branch in February 2025 and plain signup lost it. - A route back to login under Create Account, on every signup form. - Once the address is known to be taken: the row reads "You already have an account. Log in", highlights twice, and Create Account is disabled since submitting again can only fail. - One message per failure. The generic "Please check your details and try again" banner is suppressed when the error belongs to a field. - The email field is coloured, and the message, border and disabled state all clear on the first keystroke, because the error object never clears itself. - The invite notification uses our own Icon, sized to its text, and the block keeps its spacing. The highlight is finite and settles to a static tint under prefers-reduced-motion, since a continuous blink cannot be dismissed. HomePage moves into its own folder, having picked up a stylesheet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…union The E2E slept 500ms and clicked. The button stays disabled until the password requirements pass, so it now asserts enabled instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d-request The comparison was exact while the API looks the address up with iexact, so changing only the casing cleared the message and re-enabled the button on an address the API would still reject. The error also survived into the next attempt. Submitting a corrected address set submittedEmail to it while the old error was still in the store, so for the length of the request the form claimed the new address was taken. Both spotted by Wadii in review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
submittedEmail started as an empty string, which matched an untouched email field. Both /login and /signup render this component, so failing a login with a blank email and then clicking Sign up put that error under the signup field before anyone had submitted it. Null until there has been an attempt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Kyle's point in review: the message under the card was easy to miss and only repeated the error. So a taken address now takes you to the login form with the address filled in and the reason at the top, which is no clicks instead of one. Gone with it: the copy that changed on the row, the highlight that drew the eye to it, the stylesheet holding that animation, and the disabled Create Account, which had nothing left to prevent once we leave the page. The plain "Have an account? Log in" row stays, that is the #5077 regression. Both email inputs were uncontrolled, so the address survived the navigation in state but the box rendered empty. The login one is controlled now. First name, last name and password already survive, so Back returns to a filled form. The redirect runs from a small component rather than an effect in the page, because the error only exists inside the provider's render prop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The store keeps the error and the login form reads it, so after the redirect the login page showed "Email already exists" under the field and a generic banner below the button, on top of the notice already explaining why you were there. Three messages for one thing. The notice covers it, so the field error and the banner are suppressed while it is showing. Submitting the login form clears the flag, so a real login failure reports normally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cut the three in the E2E that restated their assertions, the InputGroup one that duplicated the prop doc on Input, and the state one that restated its variable name. Trimmed four others. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
8e97c06 to
9c9063e
Compare
isInvalid and isValid read as opposites but were not, so the pair was easy to misuse. autoValidate already meant "do not wait for a touch", it was just read once at mount and so could not react to an error arriving later. Deriving it instead covers the API-rejected case, and InputGroup keeps its own isInvalid prop as the caller-facing name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The redirect means an already-registered email never dwells on the signup form, so colouring its border bought nothing here. Splitting it out keeps a shared control used across the app out of a signup fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@talissoncosta working great, let's get this merged! |
|
@kyle-ssg Can I get your approval again? I dropped the changes on input as they are not strictly needed for this fix anymore. |
|
@talissoncosta done! |
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Closes #8176
Sign up with an address that already has an account and you get "Email already exists. Please log in." with nothing on the page that logs you in. #5077 moved the login link into the invite-only branch in February 2025 and plain
/signuplost it.Frontend only. Two commits, the first is two lines in shared form components and reads on its own.
fix(forms)InputGrouppasses the existing flag through rather than turning it on for every field, which would restyle all 107 fields in the app.fix(signup)Icon, sized to its text. Contributes to Remove @ionic/react IonIcon dependency #7022.HomePagemoves into its own folder, having gained a stylesheet.Not in scope: reaching the dead end at all, which is #8164 and not being done. Independent of #8179.
How did you test this code?
tsc: 0 new errors vsmain.eslint: clean.invite-test.pw.ts. Not run.Signed out, on
/signup:Cannot be checked on localhost: the signup experiment gate and the Google, GitHub and SSO buttons need real flag values.
Unresolved, flagged inline at
HomePage.tsx:569: the generic banner still appeared alongside the field message on my last check. The page reads the error from two places that can disagree. Worth a second opinion on whether the condition is wrong or the two sources are.