Add user onboarding flow with username selection (#430)

* Add username onboarding prompt for new signups

New users (especially via OAuth/Google) are redirected to /onboarding
to choose a display username before accessing the app. Adds unique
constraint on users.username and case-insensitive uniqueness validation.

https://claude.ai/code/session_01UinBMAy9d6dQzzbcUAdq8J

* Address all code review issues on username onboarding

- Replace case-sensitive unique constraint with functional unique index on
  lower(username) so DB-level uniqueness is case-insensitive (fix #1)
- Share USERNAME_RE from models/user.ts; add same format + uniqueness
  validation to the settings update-profile action (fix #2)
- Wrap updateUser calls in try/catch to handle race-condition DB errors
  gracefully in both onboarding and settings (fix #3)
- Add unit tests for suggestUsername, safeRedirectTo, USERNAME_RE, and
  isOnboardingExempt (fix #4)
- Guard suggestUsername against returning strings shorter than 3 chars (fix #5)
- Preserve the user's intended destination via redirectTo query param
  through the onboarding flow (fix #6)
- Treat empty username in settings as a validation error instead of a
  silent no-op (fix #7)
- Use exact/sub-path matching in isOnboardingExempt to prevent
  false-positive prefix matches like /loginpage (fix #8)

https://claude.ai/code/session_01UinBMAy9d6dQzzbcUAdq8J

---------

Co-authored-by: Claude <noreply@anthropic.com>

app/models/user.ts

@@ -6,6 +6,8 @@ import { generateFlagConfig } from "~/lib/flag-generator";
 export type User = typeof schema.users.$inferSelect;
 export type NewUser = typeof schema.users.$inferInsert;
 
+export const USERNAME_RE = /^[a-zA-Z0-9_-]{3,30}$/;
+
 export function getUserDisplayName(
   user: Pick<User, "username" | "displayName">
 ): string | null {
@@ -38,6 +40,13 @@ export async function findUsersByIds(ids: string[]): Promise<User[]> {
     .where(inArray(schema.users.id, ids));
 }
 
+export async function findUserByUsername(username: string): Promise<User | undefined> {
+  const db = database();
+  return await db.query.users.findFirst({
+    where: (users, { sql }) => sql`lower(${users.username}) = lower(${username})`,
+  });
+}
+
 export async function updateUser(
   id: string,
   data: Partial<NewUser>

app/root.tsx

@@ -6,6 +6,7 @@ import {
   Scripts,
   ScrollRestoration,
   useLocation,
+  redirect,
 } from "react-router";
 
 import type { Route } from "./+types/root";
@@ -20,10 +21,31 @@ import { findUserById } from "~/models/user";
 import { AlertCircle, FileQuestion, Lock, ShieldOff, ServerCrash } from "lucide-react";
 import logoUrl from "../public/logo.svg?url";
 
+// Paths that should never trigger the onboarding redirect.
+// Trailing-slash entries are matched as prefixes; others require exact match or sub-path.
+const ONBOARDING_EXEMPT = ["/onboarding", "/api/", "/login", "/register", "/check-email", "/forgot-password", "/reset-password"];
+
+export function isOnboardingExempt(pathname: string): boolean {
+  return ONBOARDING_EXEMPT.some((p) =>
+    p.endsWith("/")
+      ? pathname.startsWith(p)
+      : pathname === p || pathname.startsWith(p + "/")
+  );
+}
+
 export async function loader({ request }: Route.LoaderArgs) {
   const session = await auth.api.getSession({ headers: request.headers });
   const currentUser = session ? await findUserById(session.user.id) : null;
   const isAdmin = currentUser?.isAdmin ?? false;
+
+  if (currentUser && !currentUser.username) {
+    const { pathname } = new URL(request.url);
+    if (!isOnboardingExempt(pathname)) {
+      const redirectTo = pathname === "/" ? "" : `?redirectTo=${encodeURIComponent(pathname)}`;
+      return redirect(`/onboarding${redirectTo}`);
+    }
+  }
+
   return { isAdmin, currentUser };
 }
 

app/routes.ts

@@ -60,6 +60,7 @@ export default [
   route("check-email", "routes/check-email.tsx"),
   route("forgot-password", "routes/forgot-password.tsx"),
   route("reset-password", "routes/reset-password.tsx"),
+  route("onboarding", "routes/onboarding.tsx"),
   route("settings", "routes/settings.tsx"),
   route("user-profile", "routes/user-profile-redirect.tsx"),
   route("how-to-play", "routes/how-to-play.tsx"),

app/routes/__tests__/onboarding.test.ts

@@ -0,0 +1,98 @@
+import { describe, it, expect } from "vitest";
+import { suggestUsername, safeRedirectTo } from "../onboarding";
+import { USERNAME_RE } from "~/models/user";
+
+describe("suggestUsername", () => {
+  it("returns empty string for null", () => {
+    expect(suggestUsername(null)).toBe("");
+  });
+
+  it("returns empty string for empty string", () => {
+    expect(suggestUsername("")).toBe("");
+  });
+
+  it("lowercases and strips spaces", () => {
+    expect(suggestUsername("John Smith")).toBe("johnsmith");
+  });
+
+  it("strips special characters", () => {
+    expect(suggestUsername("Jane O'Brien!")).toBe("janeobrien");
+  });
+
+  it("preserves underscores and hyphens", () => {
+    expect(suggestUsername("cool_user-name")).toBe("cool_user-name");
+  });
+
+  it("slices to 20 characters", () => {
+    expect(suggestUsername("averylongdisplaynamethatexceedstwenty")).toBe("averylongdisplayname");
+    expect(suggestUsername("averylongdisplaynamethatexceedstwenty").length).toBeLessThanOrEqual(20);
+  });
+
+  it("returns empty string when cleaned result is shorter than 3 characters", () => {
+    expect(suggestUsername("Jo")).toBe("");
+    expect(suggestUsername("AB")).toBe("");
+  });
+
+  it("returns the suggestion when cleaned result is exactly 3 characters", () => {
+    expect(suggestUsername("abc")).toBe("abc");
+  });
+
+  it("returns empty string for non-ASCII display names that strip to nothing", () => {
+    expect(suggestUsername("小林")).toBe("");
+  });
+});
+
+describe("safeRedirectTo", () => {
+  it("returns / for null", () => {
+    expect(safeRedirectTo(null)).toBe("/");
+  });
+
+  it("returns / for empty string", () => {
+    expect(safeRedirectTo("")).toBe("/");
+  });
+
+  it("returns / for bare slash", () => {
+    expect(safeRedirectTo("/")).toBe("/");
+  });
+
+  it("allows a root-relative path", () => {
+    expect(safeRedirectTo("/leagues/123/draft/456")).toBe("/leagues/123/draft/456");
+  });
+
+  it("rejects an absolute URL", () => {
+    expect(safeRedirectTo("https://evil.com")).toBe("/");
+  });
+
+  it("rejects a protocol-relative URL", () => {
+    expect(safeRedirectTo("//evil.com")).toBe("/");
+  });
+});
+
+describe("USERNAME_RE", () => {
+  it("allows 3-character usernames", () => {
+    expect(USERNAME_RE.test("abc")).toBe(true);
+  });
+
+  it("allows 30-character usernames", () => {
+    expect(USERNAME_RE.test("a".repeat(30))).toBe(true);
+  });
+
+  it("allows letters, numbers, underscores, and hyphens", () => {
+    expect(USERNAME_RE.test("Cool_User-123")).toBe(true);
+  });
+
+  it("rejects usernames shorter than 3 characters", () => {
+    expect(USERNAME_RE.test("ab")).toBe(false);
+    expect(USERNAME_RE.test("")).toBe(false);
+  });
+
+  it("rejects usernames longer than 30 characters", () => {
+    expect(USERNAME_RE.test("a".repeat(31))).toBe(false);
+  });
+
+  it("rejects special characters", () => {
+    expect(USERNAME_RE.test("user!")).toBe(false);
+    expect(USERNAME_RE.test("user name")).toBe(false);
+    expect(USERNAME_RE.test("user@host")).toBe(false);
+  });
+});

app/routes/__tests__/root.onboarding-exempt.test.ts

@@ -0,0 +1,41 @@
+import { describe, it, expect } from "vitest";
+import { isOnboardingExempt } from "../../root";
+
+describe("isOnboardingExempt", () => {
+  it("exempts /onboarding exactly", () => {
+    expect(isOnboardingExempt("/onboarding")).toBe(true);
+  });
+
+  it("exempts /onboarding sub-paths", () => {
+    expect(isOnboardingExempt("/onboarding/step2")).toBe(true);
+  });
+
+  it("exempts /api/ prefix paths", () => {
+    expect(isOnboardingExempt("/api/auth/callback")).toBe(true);
+    expect(isOnboardingExempt("/api/draft/start")).toBe(true);
+  });
+
+  it("does not exempt paths that merely start with /api without trailing slash", () => {
+    // /api/ has a trailing slash so startsWith("/api/") is used — /apiary would not match
+    expect(isOnboardingExempt("/apiary")).toBe(false);
+  });
+
+  it("exempts auth pages exactly", () => {
+    expect(isOnboardingExempt("/login")).toBe(true);
+    expect(isOnboardingExempt("/register")).toBe(true);
+    expect(isOnboardingExempt("/check-email")).toBe(true);
+    expect(isOnboardingExempt("/forgot-password")).toBe(true);
+    expect(isOnboardingExempt("/reset-password")).toBe(true);
+  });
+
+  it("does not exempt a path that merely starts with an exempt path without slash", () => {
+    expect(isOnboardingExempt("/registered-user")).toBe(false);
+    expect(isOnboardingExempt("/loginpage")).toBe(false);
+  });
+
+  it("does not exempt normal app routes", () => {
+    expect(isOnboardingExempt("/")).toBe(false);
+    expect(isOnboardingExempt("/leagues/abc")).toBe(false);
+    expect(isOnboardingExempt("/settings")).toBe(false);
+  });
+});

app/routes/onboarding.tsx

@@ -0,0 +1,110 @@
+import { redirect, Form } from "react-router";
+import { auth } from "~/lib/auth.server";
+import { findUserById, findUserByUsername, updateUser, USERNAME_RE } from "~/models/user";
+import type { Route } from "./+types/onboarding";
+import { Button } from "~/components/ui/button";
+import { Input } from "~/components/ui/input";
+import { Label } from "~/components/ui/label";
+import { Card, CardContent, CardHeader, CardTitle, CardDescription } from "~/components/ui/card";
+
+export function meta(): Route.MetaDescriptors {
+  return [{ title: "Choose a Username - Brackt" }];
+}
+
+export function suggestUsername(displayName: string | null): string {
+  if (!displayName) return "";
+  const cleaned = displayName
+    .toLowerCase()
+    .replace(/[^a-z0-9_-]/g, "")
+    .slice(0, 20);
+  return cleaned.length >= 3 ? cleaned : "";
+}
+
+export function safeRedirectTo(value: string | null): string {
+  if (!value) return "/";
+  return value.startsWith("/") && !value.startsWith("//") ? value : "/";
+}
+
+export async function loader({ request }: Route.LoaderArgs) {
+  const session = await auth.api.getSession({ headers: request.headers });
+  if (!session) return redirect("/login");
+  const user = await findUserById(session.user.id);
+  if (!user) return redirect("/login");
+  if (user.username) return redirect("/");
+  const redirectTo = safeRedirectTo(new URL(request.url).searchParams.get("redirectTo"));
+  return { suggestion: suggestUsername(user.displayName), redirectTo };
+}
+
+export async function action({ request }: Route.ActionArgs) {
+  const session = await auth.api.getSession({ headers: request.headers });
+  if (!session) return redirect("/login");
+
+  const formData = await request.formData();
+  const username = (formData.get("username") as string | null)?.trim() ?? "";
+  const redirectTo = safeRedirectTo(formData.get("redirectTo") as string | null);
+
+  if (!USERNAME_RE.test(username)) {
+    return {
+      error: "Username must be 3–30 characters and contain only letters, numbers, underscores, or hyphens.",
+      redirectTo,
+    };
+  }
+
+  const existing = await findUserByUsername(username);
+  if (existing && existing.id !== session.user.id) {
+    return { error: "That username is already taken.", redirectTo };
+  }
+
+  try {
+    await updateUser(session.user.id, { username });
+  } catch {
+    return { error: "That username is already taken.", redirectTo };
+  }
+
+  return redirect(redirectTo);
+}
+
+export default function OnboardingPage({ loaderData, actionData }: Route.ComponentProps) {
+  const redirectTo = actionData && "redirectTo" in actionData ? actionData.redirectTo : loaderData.redirectTo;
+
+  return (
+    <div className="min-h-screen flex items-center justify-center p-4">
+      <Card className="w-full max-w-md">
+        <CardHeader className="space-y-1">
+          <CardTitle className="text-2xl">Choose a username</CardTitle>
+          <CardDescription>
+            Pick a username that will be shown to other players. You can change it later in Settings.
+          </CardDescription>
+        </CardHeader>
+        <CardContent>
+          <Form method="post" className="space-y-4">
+            <input type="hidden" name="redirectTo" value={redirectTo} />
+            <div className="space-y-2">
+              <Label htmlFor="username">Username</Label>
+              <Input
+                id="username"
+                name="username"
+                type="text"
+                defaultValue={loaderData.suggestion}
+                placeholder="e.g. fantasy_king"
+                minLength={3}
+                maxLength={30}
+                autoFocus
+                autoComplete="off"
+              />
+              <p className="text-xs text-muted-foreground">
+                3–30 characters. Letters, numbers, underscores, and hyphens only.
+              </p>
+            </div>
+            {actionData && "error" in actionData && (
+              <p className="text-sm text-destructive">{actionData.error}</p>
+            )}
+            <Button type="submit" className="w-full">
+              Continue
+            </Button>
+          </Form>
+        </CardContent>
+      </Card>
+    </div>
+  );
+}

app/routes/settings.tsx

@@ -2,7 +2,7 @@ import { redirect } from "react-router";
 import { useState } from "react";
 import { Bell, Key, Lock, Shield, User } from "lucide-react";
 import { auth } from "~/lib/auth.server";
-import { findUserById, isUserInActiveDraft, updateUser, anonymizeUserAccount } from "~/models/user";
+import { findUserById, findUserByUsername, isUserInActiveDraft, updateUser, anonymizeUserAccount, USERNAME_RE } from "~/models/user";
 import { findLinkedAccountsByUserId } from "~/models/account";
 import { findTeamsByOwnerId, removeTeamOwner } from "~/models/team";
 import { removeAllCommissionersByUserId } from "~/models/commissioner";
@@ -119,13 +119,24 @@ export async function action(args: Route.ActionArgs): Promise<ActionData | Respo
   }
 
   if (intent === "update-profile") {
-    const username = formData.get("username") as string | null;
+    const username = (formData.get("username") as string | null)?.trim() ?? "";
     const timezone = formData.get("timezone") as string | null;
     const timezoneValue = timezone && VALID_TIMEZONES.has(timezone) ? timezone : undefined;
-    await updateUser(session.user.id, {
-      username: username || undefined,
-      timezone: timezoneValue,
-    });
+
+    if (!USERNAME_RE.test(username)) {
+      return { intent: "update-profile", error: "Username must be 3–30 characters and contain only letters, numbers, underscores, or hyphens." };
+    }
+
+    const existing = await findUserByUsername(username);
+    if (existing && existing.id !== session.user.id) {
+      return { intent: "update-profile", error: "That username is already taken." };
+    }
+
+    try {
+      await updateUser(session.user.id, { username, timezone: timezoneValue });
+    } catch {
+      return { intent: "update-profile", error: "That username is already taken." };
+    }
     return { intent: "update-profile", success: true };
   }
 

database/schema.ts

@@ -19,7 +19,9 @@ export const users = pgTable("users", {
   deletedAt: timestamp("deleted_at"),
   createdAt: timestamp("created_at").defaultNow().notNull(),
   updatedAt: timestamp("updated_at").defaultNow().notNull(),
-});
+}, (t) => ({
+  usernameUnique: uniqueIndex("users_username_lower_unique").on(sql`lower(${t.username})`),
+}));
 
 // BetterAuth session/account/verification tables
 export const sessions = pgTable("sessions", {

drizzle/0106_chilly_chimera.sql

@@ -0,0 +1 @@
+ALTER TABLE "users" ADD CONSTRAINT "users_username_unique" UNIQUE("username");

drizzle/0107_outgoing_mikhail_rasputin.sql

@@ -0,0 +1,2 @@
+ALTER TABLE "users" DROP CONSTRAINT "users_username_unique";--> statement-breakpoint
+CREATE UNIQUE INDEX IF NOT EXISTS "users_username_lower_unique" ON "users" USING btree (lower("username"));

drizzle/meta/_journal.json

@@ -743,6 +743,20 @@
       "when": 1778862483243,
       "tag": "0105_mighty_talisman",
       "breakpoints": true
+    },
+    {
+      "idx": 106,
+      "version": "7",
+      "when": 1778867381171,
+      "tag": "0106_chilly_chimera",
+      "breakpoints": true
+    },
+    {
+      "idx": 107,
+      "version": "7",
+      "when": 1778868190284,
+      "tag": "0107_outgoing_mikhail_rasputin",
+      "breakpoints": true
     }
   ]
 }