Skip to content

Conversation

@richiemcilroy
Copy link
Member

@richiemcilroy richiemcilroy commented Sep 16, 2025

Adds a signup page (duplicate of /login pretty much) and updates the navbar to include the signup.

Summary by CodeRabbit

  • New Features
    • Introduced a full signup experience with email, Google, and SSO options, animated transitions, and friendly error handling. Logged-in users are redirected to the dashboard.
    • Added a sign-up prompt on the login screen.
    • Updated header: removed GitHub button; unauthenticated users see a Log In link in navigation, and the primary action toggles between Sign Up (logged out) and Dashboard (logged in).
  • Style
    • Minor spacing and formatting adjustments, including refined divider margins and general layout polish.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 16, 2025

Walkthrough

Introduces a new signup page and client-side SignupForm with email, Google, and SSO flows, plus billing handoff. Updates navbar to surface login/signup appropriately. Adds a login-page sign-up prompt and adjusts spacing. One component receives formatting-only changes. No backend/auth logic changes beyond integrating existing providers.

Changes

Cohort / File(s) Summary
Signup feature (UI + routing)
apps/web/app/(org)/signup/form.tsx, apps/web/app/(org)/signup/page.tsx
Adds SignupForm with email, Google, and SSO (WorkOS) flows, OAuth error toasts, theme handling, post-email billing subscribe, and transitions. New server page forces dynamic rendering, redirects authenticated users to /dashboard, and renders the form for unauthenticated users.
Navbar auth links update
apps/web/app/(site)/Navbar.tsx
Removes GitHub icon/button. Adds AuthLinks for "Log In". Adjusts LoginOrDashboard: unauthenticated shows "Sign Up" (/signup); authenticated shows "Dashboard" (/dashboard).
Login UI tweak
apps/web/app/(org)/login/form.tsx
Replaces a flex container with animated sign-up prompt linking to /signup; refines divider margins from my-4 to mt-4 mb-4. Presentation-only.
Formatting-only
apps/web/components/ReadyToGetStarted.tsx
Import order/formatting changes only; no functional modifications.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant P as /signup page (server)
  participant C as SignupForm (client)
  participant A as Auth (NextAuth)
  participant B as Billing API
  participant N as Next Router

  U->>P: GET /signup
  P->>A: getCurrentUser()
  alt Session exists
    P-->>U: redirect to /dashboard
  else No session
    P-->>U: Render SignupForm
    U->>C: Choose flow
    alt Email signup
      C->>A: signIn('email', {email, redirect:false, callbackUrl})
      A-->>C: success/failure
      opt On success
        C->>N: navigate to /verify-otp?email=...&next=...&lastSent=...
        opt Pending price in localStorage
          C->>B: POST /api/settings/billing/subscribe
          B-->>C: { redirectUrl? }
          alt redirectUrl provided
            C->>N: navigate to redirectUrl
          end
        end
      end
    else Google signup
      C->>A: signIn('google', {callbackUrl?})
    else SSO signup
      U->>C: Enter Organization ID
      C->>A: signIn('workos', {organization, connection})
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • Brendonovich

Poem

I thump my paws—new paths to hop,
From login nudge to signup stop.
Email, SSO, or Google’s light,
The navbar points your way just right.
Billing burrows wait ahead—
Onward, onward!—carrots fed. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "feat: Add signup page and update navbar" accurately and concisely summarizes the main changes in the PR (addition of a signup page/form and navbar updates to surface signup/login flows). It is a single clear sentence using conventional commit style and directly maps to the files changed in the diff. The phrasing is specific enough for a teammate scanning history to understand the primary purpose without extraneous detail.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch signup-page

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

@richiemcilroy richiemcilroy merged commit e237cf7 into main Sep 16, 2025
14 of 15 checks passed
Copy link
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.

Actionable comments posted: 4

🧹 Nitpick comments (7)
apps/web/components/ReadyToGetStarted.tsx (1)

8-16: File naming: switch to kebab-case.
Guidelines call for kebab-case file names. Suggest renaming to apps/web/components/ready-to-get-started.tsx and updating imports.

apps/web/app/(site)/Navbar.tsx (2)

162-179: Invalid Tailwind class “focus:text-8”.
Likely intended “focus:text-blue-8”. Fix to avoid dead class.

-                              "px-2 py-0 text-sm font-medium hover:text-blue-9 focus:text-8",
+                              "px-2 py-0 text-sm font-medium hover:text-blue-9 focus:text-blue-8",

103-103: File naming: kebab-case preferred.
Consider renaming Navbar.tsx → navbar.tsx per guidelines (update imports).

apps/web/app/(org)/signup/form.tsx (4)

247-261: Extract resend wait into a constant.
Minor readability and single-source-of-truth for rate limit window.

-            const waitTime = 30000; // 30 seconds
-            if (timeSinceLastRequest < waitTime) {
+            const RESEND_WAIT_MS = 30_000; // 30 seconds
+            if (timeSinceLastRequest < RESEND_WAIT_MS) {
               const remainingSeconds = Math.ceil(
-                (waitTime - timeSinceLastRequest) / 1000,
+                (RESEND_WAIT_MS - timeSinceLastRequest) / 1000,
               );

321-356: Add rel="noopener noreferrer" to target="_blank" links.
Prevents opener leaks; even for same-origin it’s a safe default.

-            <Link
-              href="/terms"
-              target="_blank"
-              className="text-xs font-semibold text-gray-12 hover:text-blue-300"
-            >
+            <Link
+              href="/terms"
+              target="_blank"
+              rel="noopener noreferrer"
+              className="text-xs font-semibold text-gray-12 hover:text-blue-300"
+            >
...
-            <Link
-              href="/privacy"
-              target="_blank"
-              className="text-xs font-semibold text-gray-12 hover:text-blue-300"
-            >
+            <Link
+              href="/privacy"
+              target="_blank"
+              rel="noopener noreferrer"
+              className="text-xs font-semibold text-gray-12 hover:text-blue-300"
+            >

112-131: Disable SSO submit while looking up org.
Reuse loading state to prevent double submissions.

-  const handleOrganizationLookup = async (e: React.FormEvent) => {
+  const handleOrganizationLookup = async (e: React.FormEvent) => {
     e.preventDefault();
     if (!organizationId) {
       toast.error("Please enter an organization ID");
       return;
     }
 
     try {
+      setLoading(true);
       const data = await getOrganizationSSOData(organizationId);
       setOrganizationName(data.name);
 
       signIn("workos", undefined, {
         organization: data.organizationId,
         connection: data.connectionId,
       });
     } catch (error) {
       console.error("Lookup Error:", error);
       toast.error("Organization not found or SSO not configured");
+    } finally {
+      setLoading(false);
     }
   };

Also pass loading to SignupWithSSO and set Button disabled={loading}.

-    <SignupWithSSO
+    <SignupWithSSO
       handleOrganizationLookup={handleOrganizationLookup}
       organizationId={organizationId}
       setOrganizationId={setOrganizationId}
-      organizationName={organizationName}
+      organizationName={organizationName}
+      loading={loading}
     />

And in SignupWithSSO props and Button:

-}: {
+}: {
   handleOrganizationLookup: (e: React.FormEvent) => void;
   organizationId: string;
   setOrganizationId: (organizationId: string) => void;
-  organizationName: string | null;
+  organizationName: string | null;
+  loading: boolean;
 }) => {
...
-    <Button type="submit" variant="dark" className="w-full max-w-full">
+    <Button type="submit" variant="dark" className="w-full max-w-full" disabled={loading}>

77-104: Shared logic duplicated with LoginForm (billing handoff, error toasts).
Consider a small hook (e.g., useAuthBillingHandoff, useAuthErrorToasts) to DRY up both forms.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3974619 and a758467.

📒 Files selected for processing (5)
  • apps/web/app/(org)/login/form.tsx (1 hunks)
  • apps/web/app/(org)/signup/form.tsx (1 hunks)
  • apps/web/app/(org)/signup/page.tsx (1 hunks)
  • apps/web/app/(site)/Navbar.tsx (3 hunks)
  • apps/web/components/ReadyToGetStarted.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for client-side server state and data fetching in the web app
Mutations should call Server Actions and perform precise cache updates with setQueryData/setQueriesData, avoiding broad invalidations
Prefer Server Components for initial data and pass initialData to client components for React Query hydration

Files:

  • apps/web/app/(org)/login/form.tsx
  • apps/web/app/(org)/signup/page.tsx
  • apps/web/components/ReadyToGetStarted.tsx
  • apps/web/app/(org)/signup/form.tsx
  • apps/web/app/(site)/Navbar.tsx
{apps/web,packages/ui}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

{apps/web,packages/ui}/**/*.{ts,tsx}: Use Tailwind CSS exclusively for styling in the web app and shared React UI components
Component naming: React components in PascalCase; hooks in camelCase starting with 'use'

Files:

  • apps/web/app/(org)/login/form.tsx
  • apps/web/app/(org)/signup/page.tsx
  • apps/web/components/ReadyToGetStarted.tsx
  • apps/web/app/(org)/signup/form.tsx
  • apps/web/app/(site)/Navbar.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use strict TypeScript and avoid any; prefer shared types from packages

**/*.{ts,tsx}: Use Biome to format/lint TypeScript with a 2-space indent
TypeScript file names should be kebab-case (e.g., user-menu.tsx)

Files:

  • apps/web/app/(org)/login/form.tsx
  • apps/web/app/(org)/signup/page.tsx
  • apps/web/components/ReadyToGetStarted.tsx
  • apps/web/app/(org)/signup/form.tsx
  • apps/web/app/(site)/Navbar.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

React/Solid components should be named using PascalCase

Files:

  • apps/web/app/(org)/login/form.tsx
  • apps/web/app/(org)/signup/page.tsx
  • apps/web/components/ReadyToGetStarted.tsx
  • apps/web/app/(org)/signup/form.tsx
  • apps/web/app/(site)/Navbar.tsx
🧬 Code graph analysis (3)
apps/web/app/(org)/signup/page.tsx (1)
apps/web/app/(org)/signup/form.tsx (1)
  • SignupForm (27-361)
apps/web/components/ReadyToGetStarted.tsx (1)
apps/web/data/homepage-copy.ts (1)
  • homepageCopy (110-318)
apps/web/app/(org)/signup/form.tsx (2)
packages/ui/src/components/icons/LogoBadge.tsx (1)
  • LogoBadge (1-28)
apps/web/actions/organization/get-organization-sso-data.ts (1)
  • getOrganizationSSOData (7-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Analyze (rust)
🔇 Additional comments (7)
apps/web/components/ReadyToGetStarted.tsx (1)

5-5: Import reorder is fine.
No functional impact; consistent with style.

apps/web/app/(org)/login/form.tsx (2)

440-451: Nice UX touch: sign-up prompt on login.
Copy, link target, and styling look good.


452-456: Spacing tweak LGTM.
Explicit mt-4/mb-4 maintains previous layout.

apps/web/app/(site)/Navbar.tsx (2)

96-102: Auth link config is clear and scoped.
Good separation; renders only when unauthenticated.


249-256: Auth-aware CTA is correct.
Unauthenticated users see “Sign Up” to /signup; authenticated see Dashboard.

apps/web/app/(org)/signup/page.tsx (2)

8-14: Correct redirect gate.
Authenticated users are redirected server-side to /dashboard; dynamic rendering prevents stale cache.


16-27: Page scaffold looks good.
Clear back-to-home affordance and composes the client SignupForm cleanly.

Comment on lines +30 to +31
const next = searchParams?.get("next");
const [email, setEmail] = useState("");
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Sanitize callbackUrl to prevent open redirects.
Validate next before passing to NextAuth. Use only same-origin relative paths.

-  const next = searchParams?.get("next");
+  const next = searchParams?.get("next");
+  const isSafeRelativePath = (p: string) =>
+    typeof p === "string" && p.startsWith("/") && !p.startsWith("//") && !p.includes("://");
+  const safeNext = next && isSafeRelativePath(next) ? next : undefined;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const next = searchParams?.get("next");
const [email, setEmail] = useState("");
- const next = searchParams?.get("next");
const next = searchParams?.get("next");
const isSafeRelativePath = (p: string | null | undefined): p is string =>
typeof p === "string" &&
p.startsWith("/") &&
!p.startsWith("//") &&
!p.includes("://");
const safeNext = isSafeRelativePath(next) ? next : undefined;
const [email, setEmail] = useState("");
🤖 Prompt for AI Agents
In apps/web/app/(org)/signup/form.tsx around lines 30-31, the raw next from
searchParams is used directly which can enable open redirects; validate and
sanitize it before passing to NextAuth by allowing only same-origin relative
paths: if next is missing or invalid, replace with a safe default (e.g., "/");
reject values that start with "//" or contain an absolute scheme (http:, https:)
or a different origin; implement the check by ensuring the value begins with a
single "/" and does not contain "://" or by constructing a URL and comparing its
origin to location.origin, then use the validated/sanitized path variable
instead of the raw searchParams value.

Comment on lines +105 to +110
const handleGoogleSignIn = () => {
trackEvent("auth_started", { method: "google", is_signup: true });
signIn("google", {
...(next && next.length > 0 ? { callbackUrl: next } : {}),
});
};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Also use the sanitized next for Google sign-up.

-  signIn("google", {
-    ...(next && next.length > 0 ? { callbackUrl: next } : {}),
-  });
+  signIn("google", {
+    ...(safeNext ? { callbackUrl: safeNext } : {}),
+  });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handleGoogleSignIn = () => {
trackEvent("auth_started", { method: "google", is_signup: true });
signIn("google", {
...(next && next.length > 0 ? { callbackUrl: next } : {}),
});
};
const handleGoogleSignIn = () => {
trackEvent("auth_started", { method: "google", is_signup: true });
signIn("google", {
...(safeNext ? { callbackUrl: safeNext } : {}),
});
};
🤖 Prompt for AI Agents
In apps/web/app/(org)/signup/form.tsx around lines 105 to 110, the Google
sign-in handler currently uses the raw next value for the callbackUrl; change it
to use the already-computed sanitizedNext instead. Update the signIn call to
pass callbackUrl: sanitizedNext when sanitizedNext is non-empty (matching the
existing pattern used elsewhere), ensuring you reference the correct sanitized
variable in scope and preserve the trackEvent and other options.

Comment on lines +269 to +274
email,
redirect: false,
...(next && next.length > 0
? { callbackUrl: next }
: {}),
})
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Sanitize callbackUrl for email flow too.
Use safeNext computed above.

-          ...(next && next.length > 0
-            ? { callbackUrl: next }
-            : {}),
+          ...(safeNext ? { callbackUrl: safeNext } : {}),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
email,
redirect: false,
...(next && next.length > 0
? { callbackUrl: next }
: {}),
})
email,
redirect: false,
...(safeNext ? { callbackUrl: safeNext } : {}),
})
🤖 Prompt for AI Agents
In apps/web/app/(org)/signup/form.tsx around lines 269 to 274, the email sign-in
call is passing the raw next value as callbackUrl instead of using the sanitized
safeNext; update the spread to conditionally include callbackUrl only when
safeNext is set (i.e. replace usage of next with safeNext or check
safeNext.length) so the email flow uses the precomputed sanitized URL.

Comment on lines +24 to +39
<Button
variant="gray"
href="/pricing"
size="lg"
className="w-full font-medium sm:w-auto"
>
{homepageCopy.readyToGetStarted.buttons.secondary}
</Button>
<Button
variant="blue"
href="/download"
size="lg"
className="w-full font-medium sm:w-auto"
>
{homepageCopy.readyToGetStarted.buttons.primary}
</Button>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

CTA label/URL pairing looks reversed (Download ↔️ Pricing).
Primary label “Upgrade to Cap Pro” should link to /pricing; secondary “Download for free” should link to /download. Apply swap to avoid user confusion.

-          <Button
-            variant="gray"
-            href="/pricing"
+          <Button
+            variant="gray"
+            href="/download"
             size="lg"
             className="w-full font-medium sm:w-auto"
           >
             {homepageCopy.readyToGetStarted.buttons.secondary}
           </Button>
           <Button
             variant="blue"
-            href="/download"
+            href="/pricing"
             size="lg"
             className="w-full font-medium sm:w-auto"
           >
             {homepageCopy.readyToGetStarted.buttons.primary}
           </Button>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Button
variant="gray"
href="/pricing"
size="lg"
className="w-full font-medium sm:w-auto"
>
{homepageCopy.readyToGetStarted.buttons.secondary}
</Button>
<Button
variant="blue"
href="/download"
size="lg"
className="w-full font-medium sm:w-auto"
>
{homepageCopy.readyToGetStarted.buttons.primary}
</Button>
<Button
variant="gray"
href="/download"
size="lg"
className="w-full font-medium sm:w-auto"
>
{homepageCopy.readyToGetStarted.buttons.secondary}
</Button>
<Button
variant="blue"
href="/pricing"
size="lg"
className="w-full font-medium sm:w-auto"
>
{homepageCopy.readyToGetStarted.buttons.primary}
</Button>
🤖 Prompt for AI Agents
In apps/web/components/ReadyToGetStarted.tsx around lines 24 to 39 the CTA
label/URL pairing is reversed: the primary button currently links to /download
and the secondary to /pricing. Swap the hrefs so the primary (“Upgrade to Cap
Pro” / homepageCopy.readyToGetStarted.buttons.primary) uses /pricing and the
secondary (“Download for free” /
homepageCopy.readyToGetStarted.buttons.secondary) uses /download; you can either
swap the href props between the two Button components or reorder the Button
elements so labels and URLs align accordingly.

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