From f7550d4170cc3abc1ee0fdb442b95c05b0ed9090 Mon Sep 17 00:00:00 2001 From: aamoghS Date: Sat, 8 Aug 2026 18:23:20 -0700 Subject: [PATCH 1/2] feat(admin): staff, memberships, audit log and the club event surfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six procedures that existed with no screen behind them, and one page that lied. **Staff and roles.** `admin.create`/`update`/`list` had no caller, so the only way to grant the volunteer tier was an INSERT against production — while /scan's own rejection screen told people to "ask an organiser to add you as event staff". /admin/staff finds somebody by their exact sign-in email (exact, not a prefix search: this is the lookup that precedes handing out a role, and a partial-match list is how the wrong Alex gets scanner access) and grants, changes or deactivates it. **Memberships.** A cash payer at a table, a comped officer, a refund that has to be honoured — none come through Stripe and none had any path but SQL. /admin/members searches, grants, extends, shortens and ends. Months are added to whatever term is left rather than restarting it, so comping somebody mid-year does not silently shorten them. Every write records a membership_history row with the typed reason and an audit entry at critical — handing out a paid membership for free is exactly the action a record needs to exist for. **The audit log is readable.** `audit.list` existed with no screen while retention prunes routine rows at 90 days, so the evidence expired before anyone could look at it. **The interest list is readable.** The four questions the public form collects were shown to no organiser at all. **The member profile.** The columns, `member.register`/`update` and SkillsInterestsInput all existed with nothing calling any of them. A Membership tab on /settings writes them and shows status and history. **/events told the truth.** It rendered a hardcoded "No upcoming events scheduled" no matter what was in the database, because `events.list` had no caller anywhere — club events existed only for whoever was standing in front of the QR code. It is now read server-side (the tRPC provider is mounted only inside the portal route group, and this page's whole audience is signed-out). Also: the club event form can set capacity, which the schema, the row lock and the "Event is full" gate have always supported and no screen could reach. Verified: typecheck, 420 tests, lint --max-warnings 0, build. --- .../.internal-tests/participant-edge.test.ts | 134 +++++++ .../src/.internal-tests/qr-checkin.test.ts | 68 ++++ packages/api/src/routers/admin.ts | 40 ++- packages/api/src/routers/member.ts | 250 ++++++++++++- .../mainweb/app/(portal)/admin/audit/page.tsx | 147 ++++++++ .../app/(portal)/admin/members/page.tsx | 290 +++++++++++++++ sites/mainweb/app/(portal)/admin/page.tsx | 89 ++++- .../mainweb/app/(portal)/admin/staff/page.tsx | 297 +++++++++++++++ sites/mainweb/app/(portal)/settings/page.tsx | 12 +- sites/mainweb/app/events/page.tsx | 96 ++++- .../admin/hackathons/AnnouncementsTab.tsx | 68 ++++ .../components/portal/EventFormModal.tsx | 66 +++- .../components/portal/MembershipTab.tsx | 338 ++++++++++++++++++ .../components/portal/PortalSidebar.tsx | 27 ++ 14 files changed, 1898 insertions(+), 24 deletions(-) create mode 100644 sites/mainweb/app/(portal)/admin/audit/page.tsx create mode 100644 sites/mainweb/app/(portal)/admin/members/page.tsx create mode 100644 sites/mainweb/app/(portal)/admin/staff/page.tsx create mode 100644 sites/mainweb/components/portal/MembershipTab.tsx diff --git a/packages/api/src/.internal-tests/participant-edge.test.ts b/packages/api/src/.internal-tests/participant-edge.test.ts index 5da3cbaf..a1e5ab90 100644 --- a/packages/api/src/.internal-tests/participant-edge.test.ts +++ b/packages/api/src/.internal-tests/participant-edge.test.ts @@ -1159,4 +1159,138 @@ describe("Participant edge cases", () => { }); }); + // ===================================================================== + describe("12. Admin membership operations", () => { + /** + * A cash payer at a table, a comped officer, a refund that has to be + * honoured: none of these come through Stripe, and none had any path but + * SQL against production before this. + */ + const ADMIN_ROW = { userId: "admin_user", isActive: true, role: "admin" }; + + const wire = (member: Record | undefined) => + mockFindFirst.mockImplementation((table: string) => { + if (table === "admins") return ADMIN_ROW; + if (table === "users") return { id: "user_a", name: "Ada Lovelace" }; + if (table === "members") return member; + return undefined; + }); + + it("creates a membership for somebody who has never paid", async () => { + wire(undefined); + mockInsert.mockReturnValue([{ id: "member_new" }]); + + const res = await callerFor("admin_user").member.adminGrant({ + userId: "user_a", + months: 12, + note: "Paid $15 cash at the kickoff", + }); + + expect(res.isActive).toBe(true); + const memberRow = insertedInto(members)[0]![2][0]; + expect(memberRow).toMatchObject({ + userId: "user_a", + firstName: "Ada", + lastName: "Lovelace", + isActive: true, + }); + // The reason has to outlive the person who typed it. + const historyRow = insertedInto(membershipHistory)[0]![2][0]; + expect(historyRow).toMatchObject({ action: "joined" }); + expect(historyRow.notes).toContain("cash"); + }); + + /** + * Extending measures from the end of the term, not from today — otherwise + * comping somebody mid-year silently shortens them to twelve months from + * the moment an organiser happened to press the button. + */ + it("extends from the end of an unexpired term", async () => { + const existingEnd = new Date(Date.now() + 100 * DAY); + wire({ + id: "member_1", + userId: "user_a", + isActive: true, + membershipStartDate: new Date(Date.now() - 265 * DAY), + membershipEndDate: existingEnd, + }); + + const res = await callerFor("admin_user").member.adminGrant({ + userId: "user_a", + months: 12, + note: "Comped officer", + }); + + const expected = new Date(existingEnd); + expected.setMonth(expected.getMonth() + 12); + expect(res.membershipEndDate.getTime()).toBe(expected.getTime()); + }); + + it("walks a mistake back with negative months", async () => { + wire({ + id: "member_1", + userId: "user_a", + isActive: true, + membershipStartDate: new Date(), + membershipEndDate: new Date(Date.now() + 20 * DAY), + }); + + const res = await callerFor("admin_user").member.adminGrant({ + userId: "user_a", + months: -12, + note: "Refunded — charged twice", + }); + + // The term lands in the past, so the row reads lapsed rather than + // claiming to be active with an expired date. + expect(res.isActive).toBe(false); + expect(updatedTables()).toContain(members); + }); + + it("refuses to shorten a membership that does not exist", async () => { + wire(undefined); + + await expect( + callerFor("admin_user").member.adminGrant({ + userId: "user_a", + months: -12, + note: "Nothing to take away", + }), + ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + }); + + // The history is the only record of which years somebody was a member, so + // revoking ends the term rather than deleting the row. + it("ends a membership without destroying its history", async () => { + wire({ + id: "member_1", + userId: "user_a", + isActive: true, + membershipStartDate: new Date(Date.now() - 30 * DAY), + membershipEndDate: new Date(Date.now() + 300 * DAY), + }); + + await callerFor("admin_user").member.adminRevoke({ + userId: "user_a", + note: "Left the club", + }); + + expect(deletedTables()).not.toContain(members); + expect(insertedInto(membershipHistory)[0]![2][0]).toMatchObject({ + action: "cancelled", + }); + }); + + it("is refused to a caller who is not staff", async () => { + mockFindFirst.mockImplementation(() => undefined); + + await expect( + callerFor("user_a").member.adminGrant({ + userId: "user_a", + months: 12, + note: "Granting myself a year", + }), + ).rejects.toMatchObject({ code: "FORBIDDEN" }); + }); + }); }); diff --git a/packages/api/src/.internal-tests/qr-checkin.test.ts b/packages/api/src/.internal-tests/qr-checkin.test.ts index 3c07a890..ff2d20d8 100644 --- a/packages/api/src/.internal-tests/qr-checkin.test.ts +++ b/packages/api/src/.internal-tests/qr-checkin.test.ts @@ -901,4 +901,72 @@ describe("QR check-in", () => { }); }); + + // ------------------------------------------------------------------- + describe("Editing a club event", () => { + const asAdmin = (event: Record) => + mockFindFirst.mockImplementation((table: string) => { + if (table === "admins") return ADMIN_ROW; + if (table === "events") return event; + return undefined; + }); + + it("corrects a title without touching the QR or the check-ins", async () => { + asAdmin(clubEvent({ currentCheckIns: 12 })); + mockUpdate.mockReturnValue([ + { ...clubEvent(), title: "General Meeting #2" }, + ]); + + const res = await appRouter + .createCaller(createMockCtx("admin_user_id")) + .events.update({ eventId: CLUB_EVENT, title: "General Meeting #2" }); + + expect(res?.title).toBe("General Meeting #2"); + const written = mockUpdate.mock.calls[0]![2][0]; + expect(written).toMatchObject({ title: "General Meeting #2" }); + // Nothing else may ride along: a new qrCode would invalidate every + // printed sign, and the counters are the door's own state. + expect(written).not.toHaveProperty("qrCode"); + expect(written).not.toHaveProperty("currentCheckIns"); + }); + + /** + * A cap below the number already scanned makes the door refuse everyone + * forever, and the counter read as over-full with nothing explaining it. + */ + it("refuses a capacity below the people already checked in", async () => { + asAdmin(clubEvent({ currentCheckIns: 40 })); + + await expect( + appRouter + .createCaller(createMockCtx("admin_user_id")) + .events.update({ eventId: CLUB_EVENT, maxCheckIns: 20 }), + ).rejects.toMatchObject({ code: "CONFLICT" }); + + expect(mockUpdate).not.toHaveBeenCalled(); + }); + + it("allows removing the cap entirely", async () => { + asAdmin(clubEvent({ currentCheckIns: 40, maxCheckIns: 50 })); + mockUpdate.mockReturnValue([{ ...clubEvent(), maxCheckIns: null }]); + + await appRouter + .createCaller(createMockCtx("admin_user_id")) + .events.update({ eventId: CLUB_EVENT, maxCheckIns: null }); + + expect(mockUpdate.mock.calls[0]![2][0]).toMatchObject({ + maxCheckIns: null, + }); + }); + + it("is refused to a caller who is not staff", async () => { + mockFindFirst.mockImplementation(() => undefined); + + await expect( + appRouter + .createCaller(createMockCtx("member_user")) + .events.update({ eventId: CLUB_EVENT, title: "Nope" }), + ).rejects.toMatchObject({ code: "FORBIDDEN" }); + }); + }); }); diff --git a/packages/api/src/routers/admin.ts b/packages/api/src/routers/admin.ts index 12d1f0e2..4f0a2504 100644 --- a/packages/api/src/routers/admin.ts +++ b/packages/api/src/routers/admin.ts @@ -141,11 +141,43 @@ export const adminRouter = createTRPCRouter({ return allAdmins; }), + /** + * Finds the person a staff role is about to be granted to, by their exact + * sign-in address. + * + * Exact rather than a prefix search: this is the lookup that precedes handing + * out a role, and a partial match list is how the wrong Alex ends up with + * scanner access. It also means the endpoint cannot be used to enumerate the + * user table — it confirms an address somebody already knows. + */ + findUserByEmail: isSuperAdmin + .input(z.object({ email: z.string().trim().email().max(255) })) + .query(async ({ ctx, input }) => { + const user = await (ctx.db as DrizzleDB).query.users.findFirst({ + // Stored lowercased by every writer. + where: eq(users.email, input.email.toLowerCase()), + columns: { id: true, name: true, email: true, image: true }, + }); + + if (!user) return null; + + const existing = await (ctx.db as DrizzleDB).query.admins.findFirst({ + where: eq(admins.userId, user.id), + columns: { id: true, role: true, isActive: true }, + }); + + return { ...user, existingRole: existing ?? null }; + }), + create: isSuperAdmin .input( z.object({ userId: z.string().min(1).max(255), - role: z.enum(["super_admin", "admin", "moderator"]), + // "volunteer" is the check-in desk tier: an active admins row that + // isAdmin deliberately rejects, so it grants badge scanning and + // nothing else. Without it here the only way to staff a scan station + // is a hand-written INSERT. + role: z.enum(["super_admin", "admin", "moderator", "volunteer"]), permissions: z.array(z.string().max(100)).max(50).optional(), }), ) @@ -201,7 +233,11 @@ export const adminRouter = createTRPCRouter({ .input( z.object({ adminId: z.string().uuid(), - role: z.enum(["super_admin", "admin", "moderator"]).optional(), + // Same set as create — otherwise an existing admin could be made a + // volunteer but a volunteer could never be promoted back. + role: z + .enum(["super_admin", "admin", "moderator", "volunteer"]) + .optional(), permissions: z.array(z.string().max(100)).max(50).optional(), isActive: z.boolean().optional(), }), diff --git a/packages/api/src/routers/member.ts b/packages/api/src/routers/member.ts index ee0a6149..11e3cbee 100644 --- a/packages/api/src/routers/member.ts +++ b/packages/api/src/routers/member.ts @@ -2,14 +2,17 @@ import { z } from "zod"; import { TRPCError } from "@trpc/server"; import { createTRPCRouter, protectedProcedure, publicProcedure } from "../trpc"; // membershipHistory is written by createOrUpdateMembership on a real payment, -// not here: `register` no longer grants a term, so it has nothing to record. -import { members } from "@query/db"; -import { eq, and } from "drizzle-orm"; +// and by the admin operations below when somebody pays another way. +import { members, membershipHistory, users } from "@query/db"; +import { eq, and, ilike, or, desc } from "drizzle-orm"; import type { DrizzleDB } from "@query/db"; import { clearMembershipCaches, invalidatePortalContext, } from "../middleware/cache"; +import { isAdmin } from "../middleware/procedures"; +import { recordAdminAction } from "../middleware/audit"; +import { splitName } from "@query/db/services/membership"; // Letters from every script, plus the combining marks, spaces, hyphens and // apostrophes (straight and typographic) that real names are written with. @@ -355,4 +358,245 @@ export const memberRouter = createTRPCRouter({ return result; }), + /** + * Staff-facing membership operations. + * + * Somebody paying in cash at a table, an officer being comped, a refund that + * has to be honoured — none of these come through Stripe, and until now none + * of them had any path but direct SQL. Every one is audit-logged, because + * granting a paid membership for free is exactly the action a record needs to + * exist for. + */ + adminSearch: isAdmin + .input( + z.object({ + query: z.string().trim().min(1).max(200), + limit: z.number().int().min(1).max(50).default(20), + }), + ) + .query(async ({ ctx, input }) => { + const pattern = `%${input.query}%`; + + const rows = await (ctx.db as DrizzleDB) + .select({ + userId: users.id, + name: users.name, + email: users.email, + memberId: members.id, + firstName: members.firstName, + lastName: members.lastName, + isActive: members.isActive, + membershipEndDate: members.membershipEndDate, + memberType: members.memberType, + renewalCount: members.renewalCount, + bootcampMember: members.bootcampMember, + }) + .from(users) + .leftJoin(members, eq(members.userId, users.id)) + .where(or(ilike(users.email, pattern), ilike(users.name, pattern))) + .limit(input.limit); + + const now = new Date(); + return rows.map((row) => ({ + ...row, + // Same rule as checkStatus and the portal context: paid and unexpired. + isCurrentMember: Boolean( + row.isActive && + row.membershipEndDate && + row.membershipEndDate > now, + ), + })); + }), + + /** One person's membership history, for staff resolving a dispute. */ + adminHistory: isAdmin + .input(z.object({ userId: z.string().min(1).max(255) })) + .query(async ({ ctx, input }) => { + const member = await (ctx.db as DrizzleDB).query.members.findFirst({ + where: eq(members.userId, input.userId), + columns: { id: true }, + }); + + if (!member) return []; + + return await (ctx.db as DrizzleDB).query.membershipHistory.findMany({ + where: eq(membershipHistory.memberId, member.id), + orderBy: [desc(membershipHistory.createdAt)], + limit: 100, + }); + }), + + /** + * Grants or extends a membership without a payment, or shortens one. + * + * `months` is added to whatever term the person already has left, so comping + * somebody mid-term does not shorten them; a negative value is how a refund + * or a mistake is walked back. + */ + adminGrant: isAdmin + .input( + z.object({ + userId: z.string().min(1).max(255), + months: z.number().int().min(-24).max(24).refine((n) => n !== 0, { + message: "Choose a number of months to add or remove.", + }), + /** Recorded on the history row, so the reason survives the person. */ + note: z.string().trim().min(1).max(500), + }), + ) + .mutation(async ({ ctx, input }) => { + const db = ctx.db as DrizzleDB; + + const user = await db.query.users.findFirst({ + where: eq(users.id, input.userId), + columns: { id: true, name: true }, + }); + + if (!user) { + throw new TRPCError({ code: "NOT_FOUND", message: "User not found" }); + } + + const existing = await db.query.members.findFirst({ + where: eq(members.userId, input.userId), + }); + + const now = new Date(); + // Extending measures from the end of the current term, so a comp added + // mid-year is a year on top rather than a year from today — which would + // silently shorten somebody who had months left. + const base = + existing?.membershipEndDate && existing.membershipEndDate > now + ? existing.membershipEndDate + : now; + const termEnd = new Date(base); + termEnd.setMonth(termEnd.getMonth() + input.months); + + if (existing) { + await db + .update(members) + .set({ + // Removing months can leave the term in the past; the row then + // reads as lapsed rather than pretending to be active. + isActive: termEnd > now, + membershipEndDate: termEnd, + memberType: "continuous", + updatedAt: now, + }) + .where(eq(members.id, existing.id)); + + await db.insert(membershipHistory).values({ + memberId: existing.id, + action: input.months > 0 ? "renewed" : "cancelled", + startDate: base, + endDate: termEnd, + notes: `Admin: ${input.note}`, + }); + } else { + if (input.months < 0) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "That person has no membership to shorten.", + }); + } + + const { firstName, lastName } = splitName(user.name); + + const [created] = await db + .insert(members) + .values({ + userId: input.userId, + firstName, + lastName, + memberType: "new", + isActive: true, + membershipStartDate: now, + membershipEndDate: termEnd, + renewalCount: 0, + }) + .returning({ id: members.id }); + + if (created) { + await db.insert(membershipHistory).values({ + memberId: created.id, + action: "joined", + startDate: now, + endDate: termEnd, + notes: `Admin: ${input.note}`, + }); + } + } + + clearMembershipCaches(input.userId); + + // After the writes, never inside a transaction with them: a failed audit + // insert aborts the Postgres session and turns the COMMIT into a silent + // ROLLBACK. + await recordAdminAction(db, { + userId: ctx.userId, + action: "member.adminGrant", + resourceId: input.userId, + // Handing out a paid membership for nothing is precisely the action + // somebody may later need to account for. + severity: "critical", + metadata: { months: input.months, note: input.note }, + }); + + return { membershipEndDate: termEnd, isActive: termEnd > now }; + }), + + /** + * Ends a membership now. + * + * The row and its history stay: deleting the member would take the record of + * every year they were one with it, and this is usually a correction rather + * than a denial that the person existed. + */ + adminRevoke: isAdmin + .input( + z.object({ + userId: z.string().min(1).max(255), + note: z.string().trim().min(1).max(500), + }), + ) + .mutation(async ({ ctx, input }) => { + const db = ctx.db as DrizzleDB; + + const existing = await db.query.members.findFirst({ + where: eq(members.userId, input.userId), + }); + + if (!existing) { + throw new TRPCError({ + code: "NOT_FOUND", + message: "That person has no membership.", + }); + } + + const now = new Date(); + + await db + .update(members) + .set({ isActive: false, membershipEndDate: now, updatedAt: now }) + .where(eq(members.id, existing.id)); + + await db.insert(membershipHistory).values({ + memberId: existing.id, + action: "cancelled", + startDate: existing.membershipStartDate, + endDate: now, + notes: `Admin: ${input.note}`, + }); + + clearMembershipCaches(input.userId); + + await recordAdminAction(db, { + userId: ctx.userId, + action: "member.adminRevoke", + resourceId: input.userId, + severity: "critical", + metadata: { note: input.note }, + }); + + return { success: true }; + }), }); diff --git a/sites/mainweb/app/(portal)/admin/audit/page.tsx b/sites/mainweb/app/(portal)/admin/audit/page.tsx new file mode 100644 index 00000000..60b7a504 --- /dev/null +++ b/sites/mainweb/app/(portal)/admin/audit/page.tsx @@ -0,0 +1,147 @@ +"use client"; + +import { useState } from "react"; +import { trpc } from "@/lib/trpc"; +import { LiquidGlass } from "@/components/portal/LiquidGlass"; +import { ScrollText } from "lucide-react"; + +/** + * The audit log. + * + * `audit.list` has existed with no screen calling it, while retention prunes + * routine rows at 90 days — so the evidence expired before anyone could look at + * it. This is the reader. + */ + +const PAGE = 50; + +const SEVERITIES = [ + { id: undefined, label: "All" }, + { id: "critical" as const, label: "Critical" }, + { id: "warn" as const, label: "Warnings" }, + { id: "info" as const, label: "Info" }, +]; + +const severityClass = (severity: string) => + severity === "critical" + ? "text-red-400 border-red-500/30 bg-red-500/10" + : severity === "warn" + ? "text-amber-300 border-amber-500/30 bg-amber-500/10" + : "text-[var(--text-muted)] border-[var(--border-subtle)] bg-white/[0.02]"; + +export default function AuditPage() { + const [severity, setSeverity] = useState< + "info" | "warn" | "critical" | undefined + >(undefined); + const [offset, setOffset] = useState(0); + + const { data, isLoading } = trpc.audit.list.useQuery({ + limit: PAGE, + offset, + severity, + }); + + return ( +
+
+

+ + Audit log +

+

+ Destructive and forced admin actions. Routine entries are pruned after + 90 days; critical ones are kept for a year. +

+
+ +
+ {SEVERITIES.map((option) => ( + + ))} +
+ + + {isLoading ? ( +

+ Loading... +

+ ) : (data?.logs.length ?? 0) === 0 ? ( +

+ Nothing recorded in this range. +

+ ) : ( +
+ {data?.logs.map((log) => ( +
+
+ + {log.severity} + + + {log.action} + + + {log.createdAt.toLocaleString()} + +
+

+ by {log.userId ?? "system"} + {log.resourceId ? ` · on ${log.resourceId}` : ""} +

+ {log.metadata != null && + Object.keys(log.metadata as object).length > 0 && ( + 
+                      {JSON.stringify(log.metadata, null, 2)}
+
+ )} +
+ ))} +
+ )} + +
+

+ {data + ? `${offset + 1}–${Math.min(offset + PAGE, data.pagination.total)} of ${data.pagination.total}` + : ""} +

+
+ + +
+
+ +
+ ); +} diff --git a/sites/mainweb/app/(portal)/admin/members/page.tsx b/sites/mainweb/app/(portal)/admin/members/page.tsx new file mode 100644 index 00000000..b359100a --- /dev/null +++ b/sites/mainweb/app/(portal)/admin/members/page.tsx @@ -0,0 +1,290 @@ +"use client"; + +import { useState } from "react"; +import { trpc } from "@/lib/trpc"; +import { LiquidGlass } from "@/components/portal/LiquidGlass"; +import { CreditCard } from "lucide-react"; + +/** + * Membership operations for staff. + * + * Cash at a table, a comped officer, a refund that has to be honoured — none of + * these arrive through Stripe, and until now none had any path but SQL against + * production. Every action here is audit-logged as critical. + */ +export default function AdminMembersPage() { + const utils = trpc.useUtils(); + + const [query, setQuery] = useState(""); + const [searched, setSearched] = useState(""); + const [selected, setSelected] = useState(null); + const [months, setMonths] = useState(12); + const [note, setNote] = useState(""); + const [error, setError] = useState(null); + const [notice, setNotice] = useState(null); + + const results = trpc.member.adminSearch.useQuery( + { query: searched }, + { enabled: searched.length > 0 }, + ); + + const history = trpc.member.adminHistory.useQuery( + { userId: selected ?? "" }, + { enabled: !!selected }, + ); + + const refresh = () => { + utils.member.adminSearch.invalidate(); + utils.member.adminHistory.invalidate(); + }; + + const grant = trpc.member.adminGrant.useMutation({ + onSuccess: (result) => { + setError(null); + setNotice( + `Done. Membership now ${result.isActive ? "runs to" : "ended"} ${result.membershipEndDate.toLocaleDateString()}.`, + ); + setNote(""); + refresh(); + }, + onError: (e) => setError(e.message), + }); + + const revoke = trpc.member.adminRevoke.useMutation({ + onSuccess: () => { + setError(null); + setNotice("Membership ended."); + setNote(""); + refresh(); + }, + onError: (e) => setError(e.message), + }); + + const busy = grant.isPending || revoke.isPending; + const selectedRow = results.data?.find((row) => row.userId === selected); + + return ( +
+
+

+ + Memberships +

+

+ Grant, extend or end a membership. Every change is recorded in the + audit log and in the member's own history. +

+
+ + + +
+ setQuery(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter") setSearched(query.trim()); + }} + placeholder="ada@gatech.edu" + className="flex-1 px-4 py-3 bg-[var(--bg-primary)]/40 border border-[var(--border-subtle)] rounded-none text-[var(--text-primary)] text-sm font-mono placeholder:text-gray-600 focus:border-accent/50 focus:outline-none transition-colors" + /> + +
+ + {searched && results.data?.length === 0 && ( +

+ Nobody matches “{searched}”. They must have signed in at least once. +

+ )} + + {(results.data?.length ?? 0) > 0 && ( +
+ {results.data?.map((row) => ( + + ))} +
+ )} + + + {selectedRow && ( + +
+

+ {selectedRow.email} +

+

+ {selectedRow.isCurrentMember + ? `Active until ${selectedRow.membershipEndDate?.toLocaleDateString()} · renewed ${selectedRow.renewalCount ?? 0}×` + : "No current membership"} +

+
+ +
+
+ + setMonths(Number(e.target.value))} + className="w-full px-4 py-3 bg-[var(--bg-primary)]/40 border border-[var(--border-subtle)] rounded-none text-[var(--text-primary)] text-sm font-mono focus:border-accent/50 focus:outline-none transition-colors" + /> +

+ Added to whatever term is left. Negative takes time away. +

+
+
+ + setNote(e.target.value)} + placeholder="Paid $15 cash at the fall kickoff" + maxLength={500} + className="w-full px-4 py-3 bg-[var(--bg-primary)]/40 border border-[var(--border-subtle)] rounded-none text-[var(--text-primary)] text-sm font-mono placeholder:text-gray-600 focus:border-accent/50 focus:outline-none transition-colors" + /> +
+
+ +
+ + {selectedRow.isCurrentMember && ( + + )} +
+ + {error && ( +

+ {error} +

+ )} + {notice && ( +

+ {notice} +

+ )} + +
+

+ History +

+ {(history.data?.length ?? 0) === 0 ? ( +

+ Nothing recorded yet. +

+ ) : ( +
    + {history.data?.map((row) => ( +
  • + {row.createdAt.toLocaleDateString()} · {row.action} ·{" "} + {row.startDate.toLocaleDateString()} →{" "} + {row.endDate ? row.endDate.toLocaleDateString() : "—"} + {row.notes ? ` · ${row.notes}` : ""} +
    • + ))} +
+ )} +
+ + )} +
+ ); +} diff --git a/sites/mainweb/app/(portal)/admin/page.tsx b/sites/mainweb/app/(portal)/admin/page.tsx index 6ec1722d..39cf263e 100644 --- a/sites/mainweb/app/(portal)/admin/page.tsx +++ b/sites/mainweb/app/(portal)/admin/page.tsx @@ -30,6 +30,8 @@ export default function AdminPage() { const [showQRCode, setShowQRCode] = useState(null); const [qrCodeDataURL, setQrCodeDataURL] = useState(""); const [selectedEvent, setSelectedEvent] = useState(null); + const [editingEvent, setEditingEvent] = useState(null); + const [editError, setEditError] = useState(null); const { data: portalContext } = usePortalContext(); const { data: events, isLoading: eventsLoading } = @@ -63,6 +65,17 @@ export default function AdminPage() { }, }); + // Editing rather than delete-and-recreate: deleting takes every check-in + // already collected with it, and mints a QR nobody's printed sign matches. + const updateEventMutation = trpc.events.update.useMutation({ + onSuccess: () => { + utils.events.listAll.invalidate(); + setEditingEvent(null); + setEditError(null); + }, + onError: (error) => setEditError(error.message), + }); + const regenerateQRMutation = trpc.events.regenerateQR.useMutation({ onSuccess: (updatedEvent) => { utils.events.listAll.invalidate(); @@ -85,21 +98,55 @@ export default function AdminPage() { } }; - const handleCreateEvent = (formData: { + type EventForm = { title: string; description: string; location: string; eventDate: string; - }) => { + maxCheckIns: string; + }; + + // Empty means no cap — the column is nullable and the door gate only runs + // when a number is set. The form used to hardcode undefined, so capacity was + // unreachable from anywhere in the product. + const parseCapacity = (value: string) => { + const parsed = Number(value); + return value.trim() && Number.isFinite(parsed) && parsed > 0 + ? Math.floor(parsed) + : undefined; + }; + + const handleCreateEvent = (formData: EventForm) => { createEventMutation.mutate({ title: formData.title, description: formData.description || undefined, location: formData.location || undefined, eventDate: new Date(formData.eventDate), - maxCheckIns: undefined, + maxCheckIns: parseCapacity(formData.maxCheckIns), + }); + }; + + const handleEditEvent = (formData: EventForm) => { + if (!editingEvent) return; + setEditError(null); + updateEventMutation.mutate({ + eventId: editingEvent.id, + title: formData.title, + description: formData.description || null, + location: formData.location || null, + eventDate: new Date(formData.eventDate), + maxCheckIns: parseCapacity(formData.maxCheckIns) ?? null, }); }; + /** `datetime-local` wants local wall-clock, not the UTC ISO string. */ + const toLocalInput = (date: Date) => { + const local = new Date( + date.getTime() - new Date(date).getTimezoneOffset() * 60 * 1000, + ); + return local.toISOString().slice(0, 16); + }; + const downloadQRCode = () => { const link = document.createElement("a"); link.download = `${selectedEvent?.title || "event"}-qr.png`; @@ -117,6 +164,28 @@ export default function AdminPage() { /> )} + {editingEvent && ( + { + setEditingEvent(null); + setEditError(null); + }} + onSubmit={handleEditEvent} + isSubmitting={updateEventMutation.isPending} + error={editError} + /> + )} + {showQRCode && selectedEvent && ( {event.checkInEnabled ? "Close" : "Open"} + + +

+ They must have signed in at least once — the role attaches to an + existing account. +

+ + + {searchedEmail && !lookup.isPending && !lookup.data && ( +

+ No account for {searchedEmail}. Ask them to sign in once, then look + again. +

+ )} + + {lookup.data && ( +
+
+

+ {lookup.data.name ?? "Unnamed account"} +

+

+ {lookup.data.email} +

+ {lookup.data.existingRole && ( +

+ Already {lookup.data.existingRole.role} + {lookup.data.existingRole.isActive ? "" : " (deactivated)"} +

+ )} +
+ +
+ + Role + +
+ {ROLES.map((option) => ( + + ))} +
+
+ + +
+ )} + + {error && ( +

+ {error} +

+ )} + {notice && ( +

+ {notice} +

+ )} + + + +

+ Current staff +

+ {staffLoading ? ( +

+ Loading... +

+ ) : (staff?.length ?? 0) === 0 ? ( +

+ Nobody yet. +

+ ) : ( +
+ {staff?.map((row) => ( +
+
+

+ {row.user?.name ?? "Unnamed account"} +

+

+ {row.user?.email} · {row.role} + {row.isActive ? "" : " · deactivated"} +

+
+ +
+ ))} +
+ )} + + + ); +} diff --git a/sites/mainweb/app/(portal)/settings/page.tsx b/sites/mainweb/app/(portal)/settings/page.tsx index d52a01cc..960f02b6 100644 --- a/sites/mainweb/app/(portal)/settings/page.tsx +++ b/sites/mainweb/app/(portal)/settings/page.tsx @@ -17,7 +17,9 @@ import { CheckCircle, Mail, Link as LinkIcon, + CreditCard, } from "lucide-react"; +import { MembershipTab } from "@/components/portal/MembershipTab"; import Image from "next/image"; import { trpc } from "@/lib/trpc"; import { trpcErrorMessage } from "@/lib/trpc-error"; @@ -29,7 +31,9 @@ export default function SettingsPage() { const router = useRouter(); const { theme, setTheme } = useTheme(); const [mounted, setMounted] = useState(false); - const [activeTab, setActiveTab] = useState<"profile" | "appearance" | "account">("profile"); + const [activeTab, setActiveTab] = useState< + "profile" | "membership" | "appearance" | "account" + >("profile"); const [isSaving, setIsSaving] = useState(false); const [saved, setSaved] = useState(false); const [isUploadingImage, setIsUploadingImage] = useState(false); @@ -160,6 +164,9 @@ export default function SettingsPage() { const tabs = [ { id: "profile", label: "Profile", icon: UserCircle, desc: "Personal info & bio" }, + // The club member profile: school, major, skills, interests, socials. The + // columns and the procedures existed with no screen behind them. + { id: "membership", label: "Membership", icon: CreditCard, desc: "Club profile & status" }, { id: "appearance", label: "Appearance", icon: Sun, desc: "Theme & display" }, { id: "account", label: "Account", icon: Mail, desc: "Account & sign out" }, ] as const; @@ -358,6 +365,9 @@ export default function SettingsPage() { )} + {/* Membership Tab */} + {activeTab === "membership" && } + {/* Appearance Tab */} {activeTab === "appearance" && (
diff --git a/sites/mainweb/app/events/page.tsx b/sites/mainweb/app/events/page.tsx index 5c597239..d1f2e5ad 100644 --- a/sites/mainweb/app/events/page.tsx +++ b/sites/mainweb/app/events/page.tsx @@ -1,10 +1,62 @@ -"use client"; - import Navbar from "@/components/Navbar"; import Footer from "@/components/Footer"; import Section from "@/components/Section"; +import { db, events } from "@query/db"; +import { gte } from "drizzle-orm"; + +/** + * The club's upcoming events. + * + * This page said "No upcoming events scheduled" no matter what was in the + * database — `events.list` had no caller anywhere, so club events existed only + * for whoever was standing in front of the QR code. + * + * Read on the server rather than through tRPC: the tRPC provider is mounted + * only inside the (portal) route group, and this page's whole audience is + * people who are not signed in. + */ +export const dynamic = "force-dynamic"; + +const formatWhen = (date: Date) => + date.toLocaleString("en-US", { + weekday: "long", + month: "long", + day: "numeric", + hour: "numeric", + minute: "2-digit", + timeZone: "America/New_York", + timeZoneName: "short", + }); + +async function loadUpcoming() { + if (!db) return []; + + // From the start of today, so an event running this afternoon does not + // disappear from the list at lunchtime. + const since = new Date(); + since.setHours(0, 0, 0, 0); + + return await db.query.events.findMany({ + where: gte(events.eventDate, since), + orderBy: (event, { asc }) => [asc(event.eventDate)], + limit: 20, + // qrCode is deliberately absent: publishing it would let anyone check + // themselves in without being in the room. + columns: { + id: true, + title: true, + description: true, + location: true, + eventDate: true, + maxCheckIns: true, + currentCheckIns: true, + }, + }); +} + +export default async function EventsPage() { + const upcoming = await loadUpcoming(); -export default function EventsPage() { return (
@@ -21,9 +73,41 @@ export default function EventsPage() {

Upcoming Events

-

- No upcoming events scheduled. Check back soon! -

+ {upcoming.length === 0 ? ( +

+ No upcoming events scheduled. Check back soon! +

+ ) : ( +
    + {upcoming.map((event) => { + const full = + !!event.maxCheckIns && + event.currentCheckIns >= event.maxCheckIns; + + return ( +
  • +
    +

    {event.title}

    + {full && ( + + Full + + )} +
    +

    + {formatWhen(event.eventDate)} + {event.location ? ` · ${event.location}` : ""} +

    + {event.description && ( +

    + {event.description} +

    + )} +
    • + ); + })} +
+ )}
diff --git a/sites/mainweb/components/admin/hackathons/AnnouncementsTab.tsx b/sites/mainweb/components/admin/hackathons/AnnouncementsTab.tsx index 03a25362..1f98078a 100644 --- a/sites/mainweb/components/admin/hackathons/AnnouncementsTab.tsx +++ b/sites/mainweb/components/admin/hackathons/AnnouncementsTab.tsx @@ -41,6 +41,14 @@ export function AnnouncementsTab({ hackathonId }: { hackathonId: string }) { const [sending, setSending] = useState(false); const [progress, setProgress] = useState(null); const [error, setError] = useState(null); + const [showInterest, setShowInterest] = useState(false); + + // The four questions the public form collects were shown to no organiser at + // all — the data was gathered and then only ever read as a recipient count. + const { data: interestRows } = trpc.hackathon.listInterest.useQuery( + { hackathonId }, + { enabled: showInterest }, + ); const { data: counts } = trpc.hackathon.audienceCounts.useQuery({ hackathonId, @@ -353,6 +361,66 @@ export function AnnouncementsTab({ hackathonId }: { hackathonId: string }) { )} + +
+
+

+ Who is on the interest list +

+

+ School, country, graduation year and experience — the answers the + public form collects. +

+
+ +
+ + {showInterest && ( +
+ {(interestRows?.length ?? 0) === 0 ? ( +

+ Nobody has registered interest yet. +

+ ) : ( + + + + + + + + + + + + + {interestRows?.map((row) => ( + + + + + + + + + ))} + +
NameEmailSchoolCountryGradExperience
+ {row.name ?? "—"} + {row.email}{row.school ?? "—"}{row.country ?? "—"}{row.graduationYear ?? "—"}{row.experience ?? "—"}
+ )} +
+ )} + ); } diff --git a/sites/mainweb/components/portal/EventFormModal.tsx b/sites/mainweb/components/portal/EventFormModal.tsx index 722af8b5..0f550f8c 100644 --- a/sites/mainweb/components/portal/EventFormModal.tsx +++ b/sites/mainweb/components/portal/EventFormModal.tsx @@ -8,12 +8,19 @@ interface EventFormData { description: string; location: string; eventDate: string; + /** Empty means no cap. The column is nullable and the door gate only runs + * when a number is set. */ + maxCheckIns: string; } interface EventFormModalProps { onClose: () => void; onSubmit: (data: EventFormData) => void; isSubmitting?: boolean; + /** Present when editing: prefills the form and relabels the action. */ + initial?: Partial; + mode?: "create" | "edit"; + error?: string | null; } function getCurrentDateTimeLocal(): string { @@ -27,21 +34,27 @@ export function EventFormModal({ onClose, onSubmit, isSubmitting = false, + initial, + mode = "create", + error = null, }: EventFormModalProps) { const [form, setForm] = useState({ - title: "", - description: "", - location: "", - eventDate: "", + title: initial?.title ?? "", + description: initial?.description ?? "", + location: initial?.location ?? "", + eventDate: initial?.eventDate ?? "", + maxCheckIns: initial?.maxCheckIns ?? "", }); - // Auto-fill date/time when modal opens + // Auto-fill date/time when creating. An edit already carries the event's own + // date, and overwriting it with "now" would silently reschedule it. useEffect(() => { + if (initial?.eventDate) return; setForm((prev) => ({ ...prev, eventDate: getCurrentDateTimeLocal(), })); - }, []); + }, [initial?.eventDate]); const handleSubmit = () => { onSubmit(form); @@ -55,7 +68,7 @@ export function EventFormModal({

- Create Event + {mode === "edit" ? "Edit Event" : "Create Event"}

Configure QR Protocols @@ -143,6 +156,39 @@ export function EventFormModal({

+ {/* Capacity. The column and the door gate have always existed; the form + hardcoded undefined, so the row lock, the "Event is full" refusal + and the counter re-test never ran for anybody. */} +
+ + setForm({ ...form, maxCheckIns: e.target.value })} + className="w-full bg-[var(--bg-primary)]/40 border border-[var(--border-subtle)] rounded-none px-4 py-3 text-[var(--text-primary)] text-sm focus:border-accent focus:outline-none transition-ui font-mono" + placeholder="Leave empty for no limit" + /> +

+ Check-in refuses everyone past this number, counted at the door. +

+
+ + {error && ( +

+ {error} +

+ )} + {/* Submit Button */} diff --git a/sites/mainweb/components/portal/MembershipTab.tsx b/sites/mainweb/components/portal/MembershipTab.tsx new file mode 100644 index 00000000..5ff94fbc --- /dev/null +++ b/sites/mainweb/components/portal/MembershipTab.tsx @@ -0,0 +1,338 @@ +"use client"; + +import { useEffect, useState } from "react"; +import { trpc } from "@/lib/trpc"; +import SkillsInterestsInput from "@/components/portal/profile/SkillsInterestsInput"; + +/** + * The club member profile. + * + * The columns (school, major, graduation year, skills, interests, socials), + * `member.register`/`update` and SkillsInterestsInput all existed with nothing + * calling any of them — the data was collectable in principle and reachable + * from nowhere. This is the screen that closes that loop. + * + * Membership itself is not editable here: a term comes from a payment, and the + * only things this writes are the profile fields. + */ + +const inputClass = + "w-full px-4 py-3 bg-[var(--bg-primary)]/40 border border-[var(--border-subtle)] rounded-none text-[var(--text-primary)] text-sm font-mono placeholder:text-gray-600 focus:border-accent/50 focus:outline-none transition-colors"; + +export function MembershipTab() { + const utils = trpc.useUtils(); + + const { data: member, isPending } = trpc.member.me.useQuery(); + const { data: status } = trpc.member.checkStatus.useQuery(); + const { data: history } = trpc.member.history.useQuery(undefined, { + // Throws NOT_FOUND for somebody who has no member row at all, which is a + // normal state here rather than an error worth retrying. + enabled: !!member, + retry: false, + }); + + const [form, setForm] = useState({ + firstName: "", + lastName: "", + school: "", + major: "", + graduationYear: "", + linkedinUrl: "", + githubUrl: "", + portfolioUrl: "", + }); + const [skills, setSkills] = useState([]); + const [interests, setInterests] = useState([]); + const [error, setError] = useState(null); + const [saved, setSaved] = useState(false); + + useEffect(() => { + if (!member) return; + setForm({ + firstName: member.firstName ?? "", + lastName: member.lastName ?? "", + school: member.school ?? "", + major: member.major ?? "", + graduationYear: member.graduationYear ? String(member.graduationYear) : "", + linkedinUrl: member.linkedinUrl ?? "", + githubUrl: member.githubUrl ?? "", + portfolioUrl: member.portfolioUrl ?? "", + }); + setSkills(member.skills ?? []); + setInterests(member.interests ?? []); + }, [member]); + + const done = () => { + setError(null); + setSaved(true); + utils.member.me.invalidate(); + utils.member.history.invalidate(); + setTimeout(() => setSaved(false), 3000); + }; + + const create = trpc.member.register.useMutation({ + onSuccess: done, + onError: (e) => setError(e.message), + }); + const update = trpc.member.update.useMutation({ + onSuccess: done, + onError: (e) => setError(e.message), + }); + + const save = () => { + setError(null); + + const year = form.graduationYear.trim() + ? Number(form.graduationYear) + : undefined; + if (year !== undefined && !Number.isInteger(year)) { + setError("That graduation year does not look right."); + return; + } + + // Empty strings are omitted rather than sent: the schema validates these as + // URLs, and "" fails that instead of meaning "not set". + const payload = { + school: form.school.trim() || undefined, + major: form.major.trim() || undefined, + graduationYear: year, + skills, + interests, + linkedinUrl: form.linkedinUrl.trim() || undefined, + githubUrl: form.githubUrl.trim() || undefined, + portfolioUrl: form.portfolioUrl.trim() || undefined, + }; + + if (member) { + update.mutate(payload); + } else { + if (!form.firstName.trim() || !form.lastName.trim()) { + setError("First and last name are required to create your profile."); + return; + } + create.mutate({ + ...payload, + firstName: form.firstName.trim(), + lastName: form.lastName.trim(), + }); + } + }; + + if (isPending) { + return ( +

Loading…

+ ); + } + + return ( +
+
+

+ Membership +

+

+ {status?.isMember + ? `Active until ${status.expiresAt ? new Date(status.expiresAt).toLocaleDateString() : "—"}${ + status.daysRemaining !== null + ? ` · ${status.daysRemaining} days left` + : "" + }` + : status?.hasLapsed + ? "Your membership has lapsed. Renew from the portal dashboard." + : "You are not a paid member yet. Membership is bought from the portal dashboard."} +

+
+ +
+ {!member && ( +
+
+ + + setForm({ ...form, firstName: e.target.value }) + } + maxLength={100} + /> +
+
+ + setForm({ ...form, lastName: e.target.value })} + maxLength={100} + /> +
+
+ )} + +
+
+ + setForm({ ...form, school: e.target.value })} + placeholder="Georgia Institute of Technology" + maxLength={200} + /> +
+
+ + setForm({ ...form, major: e.target.value })} + placeholder="Computer Science" + maxLength={200} + /> +
+
+ + + setForm({ ...form, graduationYear: e.target.value }) + } + placeholder="2029" + inputMode="numeric" + /> +
+
+ +
+

+ Skills +

+ +
+ +
+

+ Interests +

+ +
+ +
+ {( + [ + ["linkedinUrl", "LinkedIn", "https://linkedin.com/in/…"], + ["githubUrl", "GitHub", "https://github.com/…"], + ["portfolioUrl", "Portfolio", "https://…"], + ] as const + ).map(([field, label, placeholder]) => ( +
+ + setForm({ ...form, [field]: e.target.value })} + placeholder={placeholder} + maxLength={500} + /> +
+ ))} +
+ + {error && ( +

+ {error} +

+ )} + {saved && ( +

+ Saved. +

+ )} + + +
+ + {(history?.length ?? 0) > 0 && ( +
+

+ Membership history +

+
    + {history?.map((row) => ( +
  • + {new Date(row.startDate).toLocaleDateString()} →{" "} + {row.endDate ? new Date(row.endDate).toLocaleDateString() : "—"}{" "} + · {row.action} +
    • + ))} +
+
+ )} +
+ ); +} diff --git a/sites/mainweb/components/portal/PortalSidebar.tsx b/sites/mainweb/components/portal/PortalSidebar.tsx index 8499b2a2..29611472 100644 --- a/sites/mainweb/components/portal/PortalSidebar.tsx +++ b/sites/mainweb/components/portal/PortalSidebar.tsx @@ -24,6 +24,9 @@ import { Rocket, Upload, FolderGit2, + CreditCard, + ShieldCheck, + ScrollText, } from "lucide-react"; import { useTheme } from "next-themes"; import { usePortalContext } from "@/lib/use-portal-context"; @@ -74,6 +77,23 @@ export default function PortalSidebar({ icon: Zap, show: !portalContext?.isAdmin, }, + { + name: "Check-In Desk", + href: "/scan", + icon: QrCode, + // Volunteers hold an admins row but isAdmin rejects them, so the admin + // nav below is invisible to them — this is their only entry point. + show: portalContext?.isScanner && !portalContext?.isAdmin, + }, + { + name: "Submit Project", + href: "/submit", + icon: Upload, + // The only route to team.submitProject, and nothing else in the product + // linked to it — so no project could be submitted, and with submissions + // now feeding judging, nothing could be judged either. + show: !portalContext?.isAdmin, + }, { name: "Club Portal", href: "/club", @@ -119,7 +139,14 @@ export default function PortalSidebar({ { name: "Projects", href: "/admin/projects", icon: FolderGit2 }, { name: "Initiatives", href: "/admin/initiatives", icon: Rocket }, { name: "Attendees", href: "/admin/attendees", icon: Users }, + { name: "Memberships", href: "/admin/members", icon: CreditCard }, + // Granting the volunteer tier is otherwise an INSERT against production, + // and /scan's rejection screen tells people to ask an organiser for it. + { name: "Staff & Roles", href: "/admin/staff", icon: ShieldCheck }, { name: "Analytics", href: "/admin/analytics", icon: BarChart3 }, + // Retention prunes routine entries at 90 days, so an unreadable log is an + // expiring one. + { name: "Audit Log", href: "/admin/audit", icon: ScrollText }, ]; return ( From ab32833a4f84c994494dbe3d864021d31534f13c Mon Sep 17 00:00:00 2001 From: aamoghS Date: Sat, 8 Aug 2026 18:39:40 -0700 Subject: [PATCH 2/2] fix(admin): make membership writes atomic and profile fields clearable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three points from Greptile on #323. **A membership could be changed with no record of why.** The member update and its membership_history row were separate statements, so a failure between them left a moved term and nothing saying who moved it or on what grounds — and the history is now the only record of which years somebody was a member. Both writes are one transaction. **Two staff extending the same person lost one of the grants.** The new term was computed from a row read outside any lock, so both read the same end date, both wrote, and the second silently overwrote the first — twelve months paid for and gone. The row is now read with SELECT … FOR UPDATE inside the transaction that writes it. **An emptied profile field came straight back.** `undefined` was the only "not set" value the form could send, so clearing a field sent nothing, the server skipped the column, and the next read restored the old value — a save that reported success and changed nothing. Optional fields are nullable now, null means clear, and the form sends it. `""` is not usable for this: those fields validate as URLs and as min-length strings, so an empty string is a validation error rather than a clear. The lock is mutation-tested — removing `.for("update")` fails the test — which needed the file's select mock upgraded to the traced chain judge-edge already uses, since a chain of fixed stubs cannot show whether a lock was taken. Verified: typecheck, 422 tests, lint --max-warnings 0, build. --- .../.internal-tests/participant-edge.test.ts | 110 ++++++++-- packages/api/src/routers/member.ts | 203 +++++++++++------- .../components/portal/MembershipTab.tsx | 25 ++- 3 files changed, 236 insertions(+), 102 deletions(-) diff --git a/packages/api/src/.internal-tests/participant-edge.test.ts b/packages/api/src/.internal-tests/participant-edge.test.ts index a1e5ab90..aaae7b0a 100644 --- a/packages/api/src/.internal-tests/participant-edge.test.ts +++ b/packages/api/src/.internal-tests/participant-edge.test.ts @@ -19,6 +19,7 @@ const mockFindMany = vi.fn(); const mockInsert = vi.fn(); const mockUpdate = vi.fn(); const mockDelete = vi.fn(); +const mockSelect = vi.fn((..._args: any[]) => [{ count: 0 }] as unknown[]); vi.mock("@query/db", async () => { const { createTransactionMock } = await import("./_db-tx-mock"); @@ -90,26 +91,34 @@ vi.mock("@query/db", async () => { }); }, }), - select: vi.fn().mockImplementation(() => ({ - from: vi.fn().mockImplementation(() => ({ - where: vi.fn().mockImplementation(() => ({ - orderBy: vi.fn().mockResolvedValue([{ count: 0 }]), - groupBy: vi.fn().mockResolvedValue([]), - limit: vi.fn().mockResolvedValue([]), - offset: vi.fn().mockResolvedValue([]), - // `.for("update")` row-locks before a seat recount. - for: vi.fn().mockResolvedValue([{ count: 0 }]), - })), - orderBy: vi.fn().mockResolvedValue([{ count: 0 }]), - groupBy: vi.fn().mockResolvedValue([]), - innerJoin: vi.fn().mockImplementation(() => ({ - innerJoin: vi.fn().mockImplementation(() => ({ - where: vi.fn().mockResolvedValue([]), - })), - where: vi.fn().mockResolvedValue([]), - })), - })), - })), + // A lazily-resolved builder chain that records the methods it was called + // with, so a test can assert that a read took `.for("update")` — the lock + // is the whole point of some of these paths, and a chain of fixed stubs + // cannot show whether it was taken. + select: (...selectArgs: any[]) => { + const trace: [string, any[]][] = []; + const chain: any = { + then: (onOk: any, onErr: any) => + Promise.resolve(mockSelect(trace, selectArgs)).then(onOk, onErr), + }; + for (const method of [ + "from", + "where", + "innerJoin", + "leftJoin", + "groupBy", + "orderBy", + "limit", + "offset", + "for", + ]) { + chain[method] = (...args: any[]) => { + trace.push([method, args]); + return chain; + }; + } + return chain; + }, }, admins: { userId: "user_id", isActive: "is_active", role: "role" }, users: { id: "id", email: "email", name: "name", image: "image" }, @@ -280,6 +289,7 @@ describe("Participant edge cases", () => { mockInsert.mockReset().mockReturnValue([]); mockUpdate.mockReset().mockReturnValue([]); mockDelete.mockReset().mockReturnValue([]); + mockSelect.mockReset().mockReturnValue([{ count: 0 }]); cache.clear(); }); @@ -1168,13 +1178,17 @@ describe("Participant edge cases", () => { */ const ADMIN_ROW = { userId: "admin_user", isActive: true, role: "admin" }; - const wire = (member: Record | undefined) => + const wire = (member: Record | undefined) => { mockFindFirst.mockImplementation((table: string) => { if (table === "admins") return ADMIN_ROW; if (table === "users") return { id: "user_a", name: "Ada Lovelace" }; if (table === "members") return member; return undefined; }); + // The membership row is now read with SELECT … FOR UPDATE inside the + // transaction, so it arrives through select() rather than findFirst. + mockSelect.mockReturnValue(member ? [member] : []); + }; it("creates a membership for somebody who has never paid", async () => { wire(undefined); @@ -1281,6 +1295,60 @@ describe("Participant edge cases", () => { }); }); + /** + * All three reported by review on #323. + * + * The term was computed from a row read outside any lock, so two staff + * extending the same person at once both measured from the same end date + * and one grant vanished. And the member write and its history row were + * separate statements, so a failure between them left a changed term with + * no record of why. + */ + it("locks the member row and writes the term and its history together", async () => { + wire({ + id: "member_1", + userId: "user_a", + isActive: true, + membershipStartDate: new Date(), + membershipEndDate: new Date(Date.now() + 100 * DAY), + }); + + await callerFor("admin_user").member.adminGrant({ + userId: "user_a", + months: 12, + note: "Comped officer", + }); + + const lockedForUpdate = mockSelect.mock.calls.some((call) => + (call[0] as [string, unknown[]][]).some(([method]) => method === "for"), + ); + expect(lockedForUpdate).toBe(true); + // The history row is written by the same transaction that moves the term. + expect(insertedInto(membershipHistory)).toHaveLength(1); + }); + + /** + * Reported by review on #323: `undefined` was the only "not set" value, so + * a member who emptied a field sent nothing, the server skipped the column, + * and the old value came back on the next read — a save that reported + * success and changed nothing. + */ + it("clears a profile field when the member empties it", async () => { + mockFindFirst.mockImplementation((table: string) => + table === "members" ? { id: "member_1", userId: "user_a" } : undefined, + ); + mockUpdate.mockReturnValue([{ id: "member_1" }]); + + await callerFor("user_a").member.update({ + school: null, + linkedinUrl: null, + }); + + const written = mockUpdate.mock.calls[0]![2][0]; + expect(written.school).toBeNull(); + expect(written.linkedinUrl).toBeNull(); + }); + it("is refused to a caller who is not staff", async () => { mockFindFirst.mockImplementation(() => undefined); diff --git a/packages/api/src/routers/member.ts b/packages/api/src/routers/member.ts index 11e3cbee..067f2f02 100644 --- a/packages/api/src/routers/member.ts +++ b/packages/api/src/routers/member.ts @@ -45,18 +45,26 @@ export const memberRouter = createTRPCRouter({ register: protectedProcedure .input( + // Nullable for the same reason `update` is: the form sends null for a + // field left blank, and creating with null is simply creating without it. z.object({ firstName: nameSchema, lastName: nameSchema, - phoneNumber: phoneSchema, - school: z.string().min(1).max(200).optional(), - major: z.string().min(1).max(200).optional(), - graduationYear: z.number().int().min(2024).max(2035).optional(), + phoneNumber: phoneSchema.nullable().optional(), + school: z.string().min(1).max(200).nullable().optional(), + major: z.string().min(1).max(200).nullable().optional(), + graduationYear: z + .number() + .int() + .min(2024) + .max(2035) + .nullable() + .optional(), skills: z.array(z.string().max(50)).max(20).optional(), interests: z.array(z.string().max(50)).max(20).optional(), - linkedinUrl: urlSchema, - githubUrl: urlSchema, - portfolioUrl: urlSchema, + linkedinUrl: urlSchema.nullable(), + githubUrl: urlSchema.nullable(), + portfolioUrl: urlSchema.nullable(), }), ) .mutation(async ({ ctx, input }) => { @@ -135,20 +143,35 @@ export const memberRouter = createTRPCRouter({ * themselves another year over tRPC without paying. */ + /** + * Every optional field is nullable, and null means CLEAR IT. + * + * Reported by review: with `undefined` as the only "not set" value, a member + * who emptied their LinkedIn field sent nothing, the server skipped the + * column, and the old value came back on the next read — a save that + * reported success and changed nothing. `null` is the difference between + * "leave this alone" and "remove it". + */ update: protectedProcedure .input( z.object({ firstName: nameSchema.optional(), lastName: nameSchema.optional(), - phoneNumber: phoneSchema, - school: z.string().min(1).max(200).optional(), - major: z.string().min(1).max(200).optional(), - graduationYear: z.number().int().min(2024).max(2035).optional(), + phoneNumber: phoneSchema.nullable().optional(), + school: z.string().min(1).max(200).nullable().optional(), + major: z.string().min(1).max(200).nullable().optional(), + graduationYear: z + .number() + .int() + .min(2024) + .max(2035) + .nullable() + .optional(), skills: z.array(z.string().max(50)).max(20).optional(), interests: z.array(z.string().max(50)).max(20).optional(), - linkedinUrl: urlSchema, - githubUrl: urlSchema, - portfolioUrl: urlSchema, + linkedinUrl: urlSchema.nullable(), + githubUrl: urlSchema.nullable(), + portfolioUrl: urlSchema.nullable(), }), ) .mutation(async ({ ctx, input }) => { @@ -456,42 +479,62 @@ export const memberRouter = createTRPCRouter({ throw new TRPCError({ code: "NOT_FOUND", message: "User not found" }); } - const existing = await db.query.members.findFirst({ - where: eq(members.userId, input.userId), - }); - - const now = new Date(); - // Extending measures from the end of the current term, so a comp added - // mid-year is a year on top rather than a year from today — which would - // silently shorten somebody who had months left. - const base = - existing?.membershipEndDate && existing.membershipEndDate > now - ? existing.membershipEndDate - : now; - const termEnd = new Date(base); - termEnd.setMonth(termEnd.getMonth() + input.months); - - if (existing) { - await db - .update(members) - .set({ - // Removing months can leave the term in the past; the row then - // reads as lapsed rather than pretending to be active. - isActive: termEnd > now, - membershipEndDate: termEnd, - memberType: "continuous", - updatedAt: now, + /** + * One transaction, behind a lock on the member row. + * + * Two problems, both reported by review: the member update and its + * history row were separate writes, so a failure between them left a + * changed term with no record of why; and the new term was computed from + * a row read outside any lock, so two staff extending the same person at + * once both measured from the same end date and one of the two grants + * silently vanished. + */ + const outcome = await db.transaction(async (tx) => { + const [locked] = await tx + .select({ + id: members.id, + membershipEndDate: members.membershipEndDate, + membershipStartDate: members.membershipStartDate, }) - .where(eq(members.id, existing.id)); + .from(members) + .where(eq(members.userId, input.userId)) + .for("update"); + + const now = new Date(); + // Extending measures from the end of the current term, so a comp added + // mid-year is a year on top rather than a year from today — which would + // silently shorten somebody who had months left. + const base = + locked?.membershipEndDate && locked.membershipEndDate > now + ? locked.membershipEndDate + : now; + const termEnd = new Date(base); + termEnd.setMonth(termEnd.getMonth() + input.months); + + if (locked) { + await tx + .update(members) + .set({ + // Removing months can leave the term in the past; the row then + // reads as lapsed rather than pretending to be active. + isActive: termEnd > now, + membershipEndDate: termEnd, + memberType: "continuous", + updatedAt: now, + }) + .where(eq(members.id, locked.id)); + + await tx.insert(membershipHistory).values({ + memberId: locked.id, + action: input.months > 0 ? "renewed" : "cancelled", + startDate: base, + endDate: termEnd, + notes: `Admin: ${input.note}`, + }); + + return { termEnd, isActive: termEnd > now }; + } - await db.insert(membershipHistory).values({ - memberId: existing.id, - action: input.months > 0 ? "renewed" : "cancelled", - startDate: base, - endDate: termEnd, - notes: `Admin: ${input.note}`, - }); - } else { if (input.months < 0) { throw new TRPCError({ code: "BAD_REQUEST", @@ -501,7 +544,7 @@ export const memberRouter = createTRPCRouter({ const { firstName, lastName } = splitName(user.name); - const [created] = await db + const [created] = await tx .insert(members) .values({ userId: input.userId, @@ -516,7 +559,7 @@ export const memberRouter = createTRPCRouter({ .returning({ id: members.id }); if (created) { - await db.insert(membershipHistory).values({ + await tx.insert(membershipHistory).values({ memberId: created.id, action: "joined", startDate: now, @@ -524,7 +567,9 @@ export const memberRouter = createTRPCRouter({ notes: `Admin: ${input.note}`, }); } - } + + return { termEnd, isActive: true }; + }); clearMembershipCaches(input.userId); @@ -541,7 +586,10 @@ export const memberRouter = createTRPCRouter({ metadata: { months: input.months, note: input.note }, }); - return { membershipEndDate: termEnd, isActive: termEnd > now }; + return { + membershipEndDate: outcome.termEnd, + isActive: outcome.isActive, + }; }), /** @@ -561,30 +609,39 @@ export const memberRouter = createTRPCRouter({ .mutation(async ({ ctx, input }) => { const db = ctx.db as DrizzleDB; - const existing = await db.query.members.findFirst({ - where: eq(members.userId, input.userId), - }); + // Same reasoning as adminGrant: the end and its history row are one + // transaction, so a membership can never be ended with no record of why. + await db.transaction(async (tx) => { + const [locked] = await tx + .select({ + id: members.id, + membershipStartDate: members.membershipStartDate, + }) + .from(members) + .where(eq(members.userId, input.userId)) + .for("update"); - if (!existing) { - throw new TRPCError({ - code: "NOT_FOUND", - message: "That person has no membership.", - }); - } + if (!locked) { + throw new TRPCError({ + code: "NOT_FOUND", + message: "That person has no membership.", + }); + } - const now = new Date(); + const now = new Date(); - await db - .update(members) - .set({ isActive: false, membershipEndDate: now, updatedAt: now }) - .where(eq(members.id, existing.id)); - - await db.insert(membershipHistory).values({ - memberId: existing.id, - action: "cancelled", - startDate: existing.membershipStartDate, - endDate: now, - notes: `Admin: ${input.note}`, + await tx + .update(members) + .set({ isActive: false, membershipEndDate: now, updatedAt: now }) + .where(eq(members.id, locked.id)); + + await tx.insert(membershipHistory).values({ + memberId: locked.id, + action: "cancelled", + startDate: locked.membershipStartDate, + endDate: now, + notes: `Admin: ${input.note}`, + }); }); clearMembershipCaches(input.userId); diff --git a/sites/mainweb/components/portal/MembershipTab.tsx b/sites/mainweb/components/portal/MembershipTab.tsx index 5ff94fbc..e18f5a1c 100644 --- a/sites/mainweb/components/portal/MembershipTab.tsx +++ b/sites/mainweb/components/portal/MembershipTab.tsx @@ -84,23 +84,32 @@ export function MembershipTab() { const year = form.graduationYear.trim() ? Number(form.graduationYear) - : undefined; + : null; if (year !== undefined && !Number.isInteger(year)) { setError("That graduation year does not look right."); return; } - // Empty strings are omitted rather than sent: the schema validates these as - // URLs, and "" fails that instead of meaning "not set". + /** + * An emptied field sends `null`, not `undefined`. + * + * `undefined` means "leave it alone", so sending it for a field somebody + * just cleared made the save report success and change nothing — the old + * value came straight back on the next read. `""` is not an option either: + * these validate as URLs and as min-length strings, so an empty string is + * a validation error rather than a clear. + */ + const orNull = (value: string) => value.trim() || null; + const payload = { - school: form.school.trim() || undefined, - major: form.major.trim() || undefined, + school: orNull(form.school), + major: orNull(form.major), graduationYear: year, skills, interests, - linkedinUrl: form.linkedinUrl.trim() || undefined, - githubUrl: form.githubUrl.trim() || undefined, - portfolioUrl: form.portfolioUrl.trim() || undefined, + linkedinUrl: orNull(form.linkedinUrl), + githubUrl: orNull(form.githubUrl), + portfolioUrl: orNull(form.portfolioUrl), }; if (member) {