Skip to content

Commit 23ed627

Browse files
committed
fix: account delete N+1, env webhook secret refine, empty reorder broadcast, members dialog double-submit + email validation
- account.delete: collapse the N+1 `COUNT(*) per room` loop into a single GROUP BY query with FILTER. Max plan owns up to 50 rooms; this turns 51 round-trips into 1. - lib/env: add refine that requires STRIPE_WEBHOOK_SECRET when STRIPE_SECRET_KEY is set in production. Without it the webhook returns 503 on every event and subscription state never reaches the DB — currently a silent misconfiguration. - tabs.routes reorder: replace `serialized[0]?.roomId ?? ""` fallback with an explicit `if (firstRoomId)` guard. Passing "" to broadcastTabsReordered opens a stray Hocuspocus document named "room:" via openDirectConnection. - members-dialog AddEmailInput: client-side email regex check + submitting state + disabled input/button. Previously rapid Enter or button mashing sent N parallel POSTs; an obvious typo (no @, no .) round-tripped to the server before the user got feedback. Code review pass 6.
1 parent 06fe1bf commit 23ed627

4 files changed

Lines changed: 54 additions & 23 deletions

File tree

apps/server/src/account/routes.ts

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -49,25 +49,29 @@ export const accountRoutes: FastifyPluginAsync = async (app) => {
4949
// biome-ignore lint/style/noNonNullAssertion: auth plugin guarantees req.user is set for /api/ routes
5050
const userId = req.user!.id;
5151

52-
const ownedRooms = await db.query.rooms.findMany({
53-
where: and(eq(rooms.ownerId, userId), isNull(rooms.deletedAt)),
54-
columns: { id: true, slug: true, name: true },
55-
});
52+
// Single query: identify which owned rooms have other members (blocking)
53+
// vs. solo (safe to soft-delete). Previous version ran one COUNT(*) per
54+
// owned room — N+1 against the user's owned-room list. Max plan owns up
55+
// to 50 rooms, so this collapses 51 round-trips to 1.
56+
const roomsWithOtherCounts = await db
57+
.select({
58+
id: rooms.id,
59+
slug: rooms.slug,
60+
name: rooms.name,
61+
otherCount: sql<number>`count(${roomMembers.userId}) filter (where ${roomMembers.userId} != ${userId})::int`,
62+
})
63+
.from(rooms)
64+
.leftJoin(roomMembers, eq(roomMembers.roomId, rooms.id))
65+
.where(and(eq(rooms.ownerId, userId), isNull(rooms.deletedAt)))
66+
.groupBy(rooms.id, rooms.slug, rooms.name);
5667

57-
// Identify which owned rooms have other members (blocking) vs. solo
58-
// (safe to soft-delete).
5968
const blockingRooms: Array<{ slug: string; name: string | null }> = [];
6069
const soloRooms: Array<{ id: string; slug: string }> = [];
61-
for (const room of ownedRooms) {
62-
const otherMemberCount = await db
63-
.select({ count: sql<number>`count(*)::int` })
64-
.from(roomMembers)
65-
.where(and(eq(roomMembers.roomId, room.id), sql`${roomMembers.userId} != ${userId}`));
66-
const others = otherMemberCount[0]?.count ?? 0;
67-
if (others > 0) {
68-
blockingRooms.push({ slug: room.slug, name: room.name });
70+
for (const r of roomsWithOtherCounts) {
71+
if (r.otherCount > 0) {
72+
blockingRooms.push({ slug: r.slug, name: r.name });
6973
} else {
70-
soloRooms.push({ id: room.id, slug: room.slug });
74+
soloRooms.push({ id: r.id, slug: r.slug });
7175
}
7276
}
7377

apps/server/src/lib/env.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@ const envSchema = z
4040
message:
4141
"SUPABASE_SERVICE_ROLE_KEY is required in production (whitelist invitee notifications, kick auto-blacklist, member email lookups all silently no-op without it)",
4242
path: ["SUPABASE_SERVICE_ROLE_KEY"],
43+
})
44+
.refine((d) => d.NODE_ENV !== "production" || !d.STRIPE_SECRET_KEY || d.STRIPE_WEBHOOK_SECRET, {
45+
message:
46+
"STRIPE_WEBHOOK_SECRET is required in production when STRIPE_SECRET_KEY is set — webhook signature verification fails closed and the billing webhook returns 503 without it, so subscription updates never reach the database",
47+
path: ["STRIPE_WEBHOOK_SECRET"],
4348
});
4449

4550
const parsed = envSchema.safeParse(process.env);

apps/server/src/rooms/tabs.routes.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,13 @@ export const tabsRoutes: FastifyPluginAsync = async (app) => {
6666
req.body.tabIds,
6767
);
6868
const serialized = reordered.map(serializeTab);
69-
if (serialized.length > 0) {
70-
void broadcastTabsReordered(app.hocuspocus, serialized[0]?.roomId ?? "", serialized);
69+
// Guard explicitly: passing `""` to broadcastTabsReordered would open
70+
// a Hocuspocus direct connection named `room:` which fails UUID
71+
// validation in onAuthenticate but bypasses auth via openDirectConnection
72+
// — it'd silently create a stray document.
73+
const firstRoomId = serialized[0]?.roomId;
74+
if (firstRoomId) {
75+
void broadcastTabsReordered(app.hocuspocus, firstRoomId, serialized);
7176
}
7277
return { tabs: serialized };
7378
},

apps/web/src/components/rooms/members-dialog.tsx

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -410,21 +410,36 @@ export function MembersDialog({
410410
);
411411
}
412412

413+
// Permissive client-side email shape check — defense against an obvious typo
414+
// before paying for a round-trip. The server is the source of truth.
415+
const EMAIL_RE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
416+
413417
function AddEmailInput({
414418
placeholder,
415419
onSubmit,
416420
}: {
417421
placeholder: string;
418-
onSubmit: (email: string) => void;
422+
onSubmit: (email: string) => void | Promise<void>;
419423
}) {
420424
const [value, setValue] = useState("");
425+
const [submitting, setSubmitting] = useState(false);
421426

422-
function handleSubmit(e: React.FormEvent) {
427+
async function handleSubmit(e: React.FormEvent) {
423428
e.preventDefault();
429+
if (submitting) return;
424430
const trimmed = value.trim();
425431
if (!trimmed) return;
426-
onSubmit(trimmed.toLowerCase());
427-
setValue("");
432+
if (!EMAIL_RE.test(trimmed)) {
433+
toast.error("Please enter a valid email address");
434+
return;
435+
}
436+
setSubmitting(true);
437+
try {
438+
await onSubmit(trimmed.toLowerCase());
439+
setValue("");
440+
} finally {
441+
setSubmitting(false);
442+
}
428443
}
429444

430445
return (
@@ -435,12 +450,14 @@ function AddEmailInput({
435450
value={value}
436451
onChange={(e) => setValue(e.target.value)}
437452
placeholder={placeholder}
438-
className="flex-1 rounded-md border border-border bg-background px-3 py-1.5 text-sm outline-none focus:ring-2 focus:ring-ring/30"
453+
disabled={submitting}
454+
className="flex-1 rounded-md border border-border bg-background px-3 py-1.5 text-sm outline-none focus:ring-2 focus:ring-ring/30 disabled:opacity-50"
439455
/>
440456
<button
441457
type="submit"
442458
aria-label="Add email"
443-
className="grid h-8 w-8 place-items-center rounded-md bg-foreground text-background hover:bg-foreground/90 transition-colors shrink-0"
459+
disabled={submitting}
460+
className="grid h-8 w-8 place-items-center rounded-md bg-foreground text-background hover:bg-foreground/90 transition-colors shrink-0 disabled:opacity-50"
444461
>
445462
<Plus className="h-4 w-4" />
446463
</button>

0 commit comments

Comments
 (0)