Skip to content

Commit 7cfac85

Browse files
committed
Address review feedback and fix auth E2E fixtures
Set active org after invitation accept, clear delayed redirects on unmount, and surface sign-out errors. Reject duplicate sign-up in the same org with the expected Better Auth code. Align E2E helpers with the servicestart tenant and stabilize signup tests.
1 parent 857f2f8 commit 7cfac85

8 files changed

Lines changed: 101 additions & 41 deletions

File tree

‎components/auth/AcceptInvitationActions.tsx‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,17 @@ export function AcceptInvitationButton({
5555
setMessage({ kind: "error", text: AuthMessage.AcceptFailed });
5656
return;
5757
}
58+
const { organizationId } = (await res.json()) as {
59+
organizationId: string;
60+
};
61+
const { error: activeOrgError } =
62+
await authClient.organization.setActive({
63+
organizationId,
64+
});
65+
if (activeOrgError) {
66+
setMessage({ kind: "error", text: AuthMessage.AcceptFailed });
67+
return;
68+
}
5869
} catch {
5970
setMessage({ kind: "error", text: AuthMessage.AcceptFailed });
6071
return;
@@ -75,21 +86,30 @@ export function AcceptInvitationButton({
7586
/** Signs out so the invitee can open the link again with their account. */
7687
export function SignOutButton() {
7788
const router = useRouter();
78-
const [pending, setPending] = useState(false);
89+
const [message, setMessage] = useState<FormMessage>();
90+
const pending = message?.kind === "loading";
7991

8092
const signOut = async () => {
81-
setPending(true);
93+
setMessage({ kind: "loading", text: AuthMessage.Loading });
8294
try {
83-
await authClient.signOut();
95+
const { error } = await authClient.signOut();
96+
if (error) {
97+
setMessage({ kind: "error", text: AuthMessage.SignOutFailed });
98+
return;
99+
}
84100
router.refresh();
85-
} finally {
86-
setPending(false);
101+
setMessage(undefined);
102+
} catch {
103+
setMessage({ kind: "error", text: AuthMessage.SignOutFailed });
87104
}
88105
};
89106

90107
return (
91-
<AuthSubmitButton type="button" pending={pending} onClick={signOut}>
92-
Sign out
93-
</AuthSubmitButton>
108+
<div className="flex flex-col gap-6">
109+
<AuthFormMessage message={message} />
110+
<AuthSubmitButton type="button" pending={pending} onClick={signOut}>
111+
Sign out
112+
</AuthSubmitButton>
113+
</div>
94114
);
95115
}

‎components/auth/ResetPasswordForm.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
"use client";
22

33
import { useState, type FormEvent } from "react";
4-
import { useRouter } from "next/navigation";
54
import AuthLayout from "@/components/auth/AuthLayout";
65
import AuthPasswordField from "@/components/auth/AuthPasswordField";
76
import { focusFirstInvalidField } from "@/components/auth/AuthTextField";
@@ -15,6 +14,7 @@ import {
1514
SUCCESS_REDIRECT_DELAY_MS,
1615
} from "@/components/auth/authConstants";
1716
import authClient from "@/lib/authClient";
17+
import { useDelayedRedirect } from "@/lib/hooks/useDelayedRedirect";
1818
import {
1919
PASSWORD_POLICY_HINT,
2020
hasErrors,
@@ -27,7 +27,7 @@ const INVALID_TOKEN_CODE = "INVALID_TOKEN";
2727

2828
/** The reset form for the `token` from the emailed link; null when missing. */
2929
export default function ResetPasswordForm({ token }: { token: string | null }) {
30-
const router = useRouter();
30+
const scheduleRedirect = useDelayedRedirect();
3131
const [expired, setExpired] = useState(false);
3232
const [password, setPassword] = useState("");
3333
const [confirmPassword, setConfirmPassword] = useState("");
@@ -69,7 +69,7 @@ export default function ResetPasswordForm({ token }: { token: string | null }) {
6969
}
7070

7171
setMessage({ kind: "success", text: AuthMessage.PasswordReset });
72-
setTimeout(() => router.push("/login"), SUCCESS_REDIRECT_DELAY_MS);
72+
scheduleRedirect("/login", SUCCESS_REDIRECT_DELAY_MS);
7373
};
7474

7575
if (token === null || expired) {

‎components/auth/SignupForm.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import Link from "next/link";
44
import { useState, type FormEvent } from "react";
5-
import { useRouter } from "next/navigation";
65
import AuthTextField, {
76
focusFirstInvalidField,
87
} from "@/components/auth/AuthTextField";
@@ -25,6 +24,7 @@ import {
2524
} from "@/lib/authValidation";
2625
import { getSlugFromHost, INVITATION_ID_HEADER } from "@/lib/clientAuthUtils";
2726
import { useActiveOrganization } from "@/lib/hooks/useActiveOrganization";
27+
import { useDelayedRedirect } from "@/lib/hooks/useDelayedRedirect";
2828

2929
/** BetterAuth's code for an email that already has an account in this org. */
3030
const ACCOUNT_EXISTS_CODE = "USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL";
@@ -48,7 +48,7 @@ export default function SignupForm({
4848
invitation,
4949
loginHref = "/login",
5050
}: SignupFormProps) {
51-
const router = useRouter();
51+
const scheduleRedirect = useDelayedRedirect();
5252
const org = useActiveOrganization();
5353
const [firstName, setFirstName] = useState("");
5454
const [lastName, setLastName] = useState("");
@@ -116,7 +116,7 @@ export default function SignupForm({
116116
}
117117

118118
setMessage({ kind: "success", text: AuthMessage.AccountCreated });
119-
setTimeout(() => router.push("/"), SUCCESS_REDIRECT_DELAY_MS);
119+
scheduleRedirect("/", SUCCESS_REDIRECT_DELAY_MS);
120120
};
121121

122122
return (

‎components/auth/authConstants.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export const AuthMessage = {
1111
PasswordReset: "Password reset. Taking you to log in…",
1212
ResetFailed: "Couldn't reset your password. Try again.",
1313
AcceptFailed: "Couldn't accept the invitation. Try again.",
14+
SignOutFailed: "Couldn't sign out. Try again.",
1415
} as const;
1516

1617
/** Long enough to read a success message before the page moves on. */

‎lib/authUtils.ts‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
} from "@/lib/services/InvitationService";
1818
import { UserService } from "@/lib/services/UserService";
1919
import { User } from "better-auth";
20+
import { APIError } from "better-auth/api";
2021
import db from "@/lib/db";
2122
import { users } from "@/lib/schema";
2223
import { getSlugFromHost, INVITATION_ID_HEADER } from "./clientAuthUtils";
@@ -314,20 +315,20 @@ export function createUserOverride(ctx: CreateUserCtx) {
314315
email,
315316
organizationId,
316317
);
317-
let id: string;
318-
if (!existingUser) {
319-
// Create new user
320-
id = crypto.randomUUID();
321-
await db.insert(users).values({
322-
name: name || "",
323-
email,
324-
organizationId,
325-
...rest,
326-
id,
318+
if (existingUser) {
319+
throw new APIError("BAD_REQUEST", {
320+
message: "User already exists. Use another email.",
321+
code: "USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL",
327322
});
328-
} else {
329-
id = existingUser.id;
330323
}
324+
const id = crypto.randomUUID();
325+
await db.insert(users).values({
326+
name: name || "",
327+
email,
328+
organizationId,
329+
...rest,
330+
id,
331+
});
331332
return await UserService.findById(id);
332333
};
333334
}

‎lib/hooks/useDelayedRedirect.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
"use client";
2+
3+
import { useCallback, useEffect, useRef } from "react";
4+
import { useRouter } from "next/navigation";
5+
6+
/** Schedules `router.push` after a delay; clears pending navigation on unmount. */
7+
export function useDelayedRedirect() {
8+
const router = useRouter();
9+
const timeoutRef = useRef<ReturnType<typeof setTimeout> | undefined>(
10+
undefined,
11+
);
12+
13+
useEffect(() => {
14+
return () => {
15+
if (timeoutRef.current !== undefined) {
16+
clearTimeout(timeoutRef.current);
17+
}
18+
};
19+
}, []);
20+
21+
return useCallback((path: string, delayMs: number) => {
22+
if (timeoutRef.current !== undefined) {
23+
clearTimeout(timeoutRef.current);
24+
}
25+
timeoutRef.current = setTimeout(() => router.push(path), delayMs);
26+
}, [router]);
27+
}

‎tests/e2e/authPages.spec.ts‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { expect, test } from "@playwright/test";
1+
import { expect, test, type Page } from "@playwright/test";
22
import { and, eq, like } from "drizzle-orm";
33
import db from "@/lib/db";
44
import { verifications } from "@/lib/schema";
@@ -13,7 +13,22 @@ test.describe("Auth pages", () => {
1313
await ensureServicestartOrganization();
1414
});
1515

16-
test.describe("signup", () => {
16+
test.describe.serial("signup", () => {
17+
test.beforeEach(async ({ page }) => {
18+
await page.context().clearCookies();
19+
});
20+
21+
async function fillPasswordFields(page: Page, password: string) {
22+
const passwordField = page.getByLabel("Password", { exact: true });
23+
const confirmField = page.getByLabel("Confirm Password", {
24+
exact: true,
25+
});
26+
await passwordField.fill(password);
27+
await confirmField.fill(password);
28+
await expect(passwordField).toHaveValue(password);
29+
await expect(confirmField).toHaveValue(password);
30+
}
31+
1732
test("shows inline errors instead of submitting an invalid form", async ({
1833
page,
1934
}) => {
@@ -54,16 +69,13 @@ test.describe("Auth pages", () => {
5469
page,
5570
}) => {
5671
const user = buildTestUser();
57-
await signUpAndGetSession(user);
72+
await signUpAndGetSession(user, "servicestart");
5873
await page.goto("/signup");
5974

6075
await page.getByLabel("First Name").fill("Existing");
6176
await page.getByLabel("Last Name").fill("User");
6277
await page.getByLabel("Email").fill(user.email);
63-
await page.getByLabel("Password", { exact: true }).fill(user.password);
64-
await page
65-
.getByLabel("Confirm Password", { exact: true })
66-
.fill(user.password);
78+
await fillPasswordFields(page, user.password);
6779
await page.getByRole("button", { name: "Create Account" }).click();
6880

6981
const message = page.getByTestId("form-message");
@@ -79,10 +91,7 @@ test.describe("Auth pages", () => {
7991
await page.getByLabel("First Name").fill("New");
8092
await page.getByLabel("Last Name").fill("Volunteer");
8193
await page.getByLabel("Email").fill(user.email);
82-
await page.getByLabel("Password", { exact: true }).fill(user.password);
83-
await page
84-
.getByLabel("Confirm Password", { exact: true })
85-
.fill(user.password);
94+
await fillPasswordFields(page, user.password);
8695
await page.getByRole("button", { name: "Create Account" }).click();
8796

8897
await expect(page.getByText(AuthMessage.AccountCreated)).toBeVisible();
@@ -96,7 +105,7 @@ test.describe("Auth pages", () => {
96105
test.describe("login", () => {
97106
test("says when the password is wrong", async ({ page }) => {
98107
const user = buildTestUser();
99-
await signUpAndGetSession(user);
108+
await signUpAndGetSession(user, "servicestart");
100109
await page.goto("/login");
101110

102111
await page.getByLabel("Email").fill(user.email);
@@ -135,7 +144,7 @@ test.describe("Auth pages", () => {
135144

136145
test("forgot password sends a reset email", async ({ page }) => {
137146
const user = buildTestUser();
138-
const { user: created } = await signUpAndGetSession(user);
147+
const { user: created } = await signUpAndGetSession(user, "servicestart");
139148
await page.goto("/forgotpassword");
140149

141150
await page.getByLabel("Email").fill(user.email);

‎tests/unit/testUtils.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,9 @@ export async function createOrganization(slug: string) {
104104
*/
105105
export async function signUpAndGetHeaders(
106106
user: ReturnType<typeof buildTestUser>,
107+
organizationSlug?: string,
107108
) {
108-
const slug = await getOrgSlug();
109+
const slug = organizationSlug ?? (await getOrgSlug());
109110
const res = await auth.api.signUpEmail({
110111
body: {
111112
...user,
@@ -126,8 +127,9 @@ export async function signUpAndGetHeaders(
126127
*/
127128
export async function signUpAndGetSession(
128129
user: ReturnType<typeof buildTestUser>,
130+
organizationSlug?: string,
129131
) {
130-
const slug = await getOrgSlug();
132+
const slug = organizationSlug ?? (await getOrgSlug());
131133
const res = await auth.api.signUpEmail({
132134
body: {
133135
...user,

0 commit comments

Comments
 (0)