Skip to content

fix: keep invite signup interactive after existing account warning#28855

Open
Monti-27 wants to merge 1 commit intocalcom:mainfrom
Monti-27:fix-invite-signup-loading
Open

fix: keep invite signup interactive after existing account warning#28855
Monti-27 wants to merge 1 commit intocalcom:mainfrom
Monti-27:fix-invite-signup-loading

Conversation

@Monti-27
Copy link
Copy Markdown

@Monti-27 Monti-27 commented Apr 12, 2026

Fixes #28854

Summary

This keeps the invite signup form usable after the existing account warning.

The warning path was still leaving the form in a successful state, so the submit button stayed disabled until the redirect. This change only keeps the button loading while the request is actually in flight, and adds a regression test for that flow.

Testing

  • TZ=UTC yarn test apps/web/modules/signup-view.submit.test.tsx
  • Web type check passed locally
  • I also tried the repo-wide checks, but this checkout already has unrelated failures in packages/trpc and packages/app-store

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 12, 2026

CLA assistant check
All committers have signed the CLA.

@Monti-27 Monti-27 marked this pull request as ready for review April 12, 2026 20:30
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 12, 2026

📝 Walkthrough

Walkthrough

Adds a test file for the signup submit error flow when a user already exists, with extensive mocks for signup, routing, authentication, and notifications. Modifies the signup view component to remove the isSubmitSuccessful flag from username field conditions and the submit button loading state, changing the button loading prop from combining isSubmitSuccessful || isSubmitting to only isSubmitting. This allows username availability checks to continue updating when the form reaches a successful submission state.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: keep invite signup interactive after existing account warning' clearly and specifically describes the main change: enabling form interaction after the existing account warning.
Description check ✅ Passed The description is directly related to the changeset, explaining the fix, the testing approach, and linking to issue #28854 which details the problem being solved.
Linked Issues check ✅ Passed The PR successfully addresses issue #28854 by removing formState.isSubmitSuccessful from the submit button loading state, allowing form interaction after the existing account warning appears.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the invite signup loading state issue: signup-view component fix and a new regression test for the specific flow.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
apps/web/modules/signup-view.submit.test.tsx (1)

157-157: Prefer fake timers over real 3.1s delay in tests.

Line 157’s real sleep can slow and occasionally destabilize CI. Consider vi.useFakeTimers() and advancing time to assert the redirect deterministically.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/modules/signup-view.submit.test.tsx` at line 157, Replace the real
3.1s sleep in the test (the await new Promise(resolve => setTimeout(resolve,
3100)) call in apps/web/modules/signup-view.submit.test.tsx) with fake timers:
call vi.useFakeTimers() at the start of the test, trigger whatever action causes
the setTimeout-based redirect, advance timers deterministically using
vi.advanceTimersByTime(3100) or vi.runAllTimers(), then await any pending
microtasks/promises (e.g., await Promise.resolve() or use
vi.runAllTimersAsync()) to let the redirect promise settle, and finally call
vi.useRealTimers() to restore; this removes the real delay while preserving the
same assertion behavior for the redirect.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/web/modules/signup-view.submit.test.tsx`:
- Line 157: Replace the real 3.1s sleep in the test (the await new
Promise(resolve => setTimeout(resolve, 3100)) call in
apps/web/modules/signup-view.submit.test.tsx) with fake timers: call
vi.useFakeTimers() at the start of the test, trigger whatever action causes the
setTimeout-based redirect, advance timers deterministically using
vi.advanceTimersByTime(3100) or vi.runAllTimers(), then await any pending
microtasks/promises (e.g., await Promise.resolve() or use
vi.runAllTimersAsync()) to let the redirect promise settle, and finally call
vi.useRealTimers() to restore; this removes the real delay while preserving the
same assertion behavior for the redirect.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 61732cb6-f1e2-4dba-bb39-e67194f02def

📥 Commits

Reviewing files that changed from the base of the PR and between 2911168 and 60245de.

📒 Files selected for processing (2)
  • apps/web/modules/signup-view.submit.test.tsx
  • apps/web/modules/signup-view.tsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invite signup stays in a loading state after the existing account warning

2 participants