Staff, memberships, audit log and the club event surfaces

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();
   });
 
@@ -1159,4 +1169,196 @@ 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<string, unknown> | 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);
+      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",
+      });
+    });
+
+    /**
+     * 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);
+
+      await expect(
+        callerFor("user_a").member.adminGrant({
+          userId: "user_a",
+          months: 12,
+          note: "Granting myself a year",
+        }),
+      ).rejects.toMatchObject({ code: "FORBIDDEN" });
+    });
+  });
 });

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<string, unknown>) =>
+      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" });
+    });
+  });
 });

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(),
       }),