Skip to content

Staff, memberships, audit log and the club event surfaces - #323

Merged
aamoghS merged 2 commits into
mainfrom
stack/4-admin
Aug 9, 2026
Merged

Staff, memberships, audit log and the club event surfaces#323
aamoghS merged 2 commits into
mainfrom
stack/4-admin

Conversation

@aamoghS

@aamoghS aamoghS commented Aug 9, 2026

Copy link
Copy Markdown
Member

Fourth in the stack. Six procedures that existed with no screen behind them, and one page that lied.

Staff and roles — /admin/staff

admin.create / update / list had no caller anywhere, 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".

Lookup is by exact sign-in email, not a prefix search: this is the step immediately before handing somebody a role, and a partial-match list is how the wrong Alex ends up with scanner access. It also means the endpoint cannot enumerate the user table — it confirms an address you already know.

Memberships — /admin/members

Cash 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.

  • Months are added to whatever term is left, not measured from today — comping somebody mid-year would otherwise silently shorten them.
  • Negative months walk a mistake back; the row reads lapsed rather than claiming to be active with an expired date.
  • Revoking ends the term and keeps the row and its history — that history is now the only record of which years somebody was a member.
  • 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.

Two readers for data nobody could see

  • /admin/auditaudit.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 from the Email tab. The four questions the public form collects were shown to no organiser at all.

The member profile — Membership tab on /settings

The columns, member.register/update and SkillsInterestsInput.tsx all existed with nothing calling any of them. Membership itself is not editable there: a term comes from a payment.

/events told the truth

It rendered a hardcoded "No upcoming events scheduled" regardless of the database, because events.list had no caller anywhere — club events existed only for whoever was standing in front of the QR code. Now read server-side, because the tRPC provider is mounted only inside the (portal) route group and this page's entire audience is signed-out. qrCode is deliberately excluded from the query: publishing it would let anyone check themselves in without being in the room.

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.

Verification

typecheck · 420 tests · lint --max-warnings 0 · build — green on this commit.


Note

High Risk
Adds staff mutations that grant or revoke paid memberships and expand admin roles (including volunteer), with broad portal and API surface area; mitigated by isAdmin/isSuperAdmin gates, row locking, and critical audit logging.

Overview
This PR wires staff-facing flows that previously only existed as unused tRPC procedures or required production SQL.

Memberships (member router + /admin/members) adds adminSearch, adminHistory, adminGrant, and adminRevoke. Grants extend from the end of the current term (not “today”), support negative months for refunds, and run in a transaction with SELECT … FOR UPDATE so concurrent extensions and member/history writes stay consistent. Each grant/revoke is critical audit-logged outside the transaction. Member register/update inputs now treat null as clear this field, fixing saves that silently kept old LinkedIn/school values.

Staff (admin router + /admin/staff) adds findUserByEmail (exact email, not enumeration) and the volunteer role on create/update so scan-desk access can be granted without hand-written INSERTs. Volunteers use isScanner, not isAdmin.

Audit (/admin/audit) surfaces audit.list with severity filters and pagination so critical actions can be reviewed before retention prunes them.

Club events get admin edit (title/details/capacity without rotating QR or counters), capacity on create/edit in EventFormModal, and a public /events page that loads upcoming events server-side (no portal tRPC) and omits qrCode. Sidebar adds check-in desk, submit project, memberships, staff, and audit links where portal context allows.

Member UX: Membership tab on settings (MembershipTab) for club profile fields and history; hackathon Announcements tab can show the interest-list table.

Reviewed by Cursor Bugbot for commit ab32833. Bugbot is set up for automated code reviews on this repo. Configure here.

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.
@cursor

cursor Bot commented Aug 9, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_e5500c97-1b1f-4f21-8c06-827d0404ff65)

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Visit the preview URL for this PR (updated for commit ab32833):

https://hacklytics2027--pr-323-s07sykb5.web.app

(expires Sun, 16 Aug 2026 01:40:39 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: c48ba34db61581e25fe2978355160b5eefe0e83f

@greptile-apps

greptile-apps Bot commented Aug 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds administrative interfaces and API operations for staff roles, memberships, audit records, event management, interest responses, and member profiles, while replacing the public events placeholder with database-backed results.

  • Adds staff lookup and volunteer-role administration.
  • Adds transactional membership grants, revocations, history, and profile editing.
  • Adds admin audit, membership, event-capacity, and interest-list surfaces.
  • Adds server-rendered public event listings without QR-code disclosure.

Confidence Score: 3/5

The PR is not yet safe to merge because membership extensions can still be lost during concurrent payment renewal, and members cannot clear an existing graduation year.

Payment renewal computes and writes membership terms without taking the lock used by adminGrant, allowing a stale renewal write to discard one extension; separately, the Membership form rejects the null value generated when graduation year is cleared.

Files Needing Attention: packages/api/src/routers/member.ts, packages/db/src/services/membership.ts, sites/mainweb/components/portal/MembershipTab.tsx

Important Files Changed

Filename Overview
packages/api/src/routers/member.ts Adds privileged membership operations and nullable profile updates; admin operations now lock rows, but payment renewal remains outside that locking discipline.
packages/db/src/services/membership.ts The existing unlocked renewal read/write path remains capable of overwriting a concurrent admin extension.
sites/mainweb/components/portal/MembershipTab.tsx Adds member-profile editing and nullable clearing, but rejects the null generated when graduation year is cleared.
packages/api/src/routers/admin.ts Adds exact-email staff lookup and volunteer-role support with existing super-admin authorization.
sites/mainweb/app/events/page.tsx Replaces the static empty state with server-loaded public event data while excluding QR codes.
sites/mainweb/components/portal/EventFormModal.tsx Adds club-event editing and capacity input backed by server-side capacity checks.

Sequence Diagram

sequenceDiagram
    participant Admin
    participant Grant as adminGrant
    participant DB
    participant Payment as Payment renewal
    Admin->>Grant: Extend membership
    Grant->>DB: SELECT member FOR UPDATE
    Payment->>DB: Read member without lock
    Grant->>DB: Write extended term and commit
    Payment->>DB: Write term computed from stale end date
    Note over DB: One extension can be lost
Loading

Reviews (2): Last reviewed commit: "fix(admin): make membership writes atomi..." | Re-trigger Greptile

Comment thread packages/api/src/routers/member.ts Outdated
Comment thread packages/api/src/routers/member.ts Outdated
Comment thread sites/mainweb/components/portal/MembershipTab.tsx Outdated
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.
@cursor

cursor Bot commented Aug 9, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_7a137ade-3050-4626-9ecf-8a00daffecc8)

@aamoghS

aamoghS commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

All three were real. Fixed in ab32833.

Non-atomic membership writes — the member update and its membership_history row were separate statements, so a failure between them left a moved term with nothing recording who moved it or why. That history is now the only record of which years somebody was a member, so losing it is worse than it looks. Both writes are one transaction; the audit entry stays outside it, deliberately — recordAdminAction swallows its own errors, which is right outside a transaction and wrong inside one, where a failed insert aborts the session and turns the COMMIT into a silent ROLLBACK.

Lost concurrent extensions — also correct. The new term was computed from a row read outside any lock, so two staff extending the same person both measured from the same end date and the second write silently discarded the first: twelve months paid for, gone. Now SELECT … FOR UPDATE inside the transaction that writes it.

Profile fields could not be cleared — correct, and the same class of bug the /settings profile form already carries a comment about. undefined was the only "not set" value available, so emptying a field sent nothing and the old value came back on the next read. Optional fields are nullable, null means clear, and the form sends it. "" was not an option: those fields validate as URLs and 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. That needed this 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.

Gate: typecheck · 422 tests · lint --max-warnings 0 · build.

@aamoghS
aamoghS merged commit 37f94f7 into main Aug 9, 2026
19 checks passed
Comment on lines +88 to +91
if (year !== undefined && !Number.isInteger(year)) {
setError("That graduation year does not look right.");
return;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Cleared graduation year is rejected

When a member clears their graduation year, the form converts the empty value to null but then treats that value as a non-integer and returns before invoking the update mutation, causing the old graduation year to remain stored.

@aamoghS
aamoghS deleted the stack/4-admin branch August 9, 2026 03:17
aamoghS added a commit that referenced this pull request Aug 9, 2026
…ile (#335)

The Membership tab refused to save anything — school, major, skills, interests,
socials — unless a graduation year had been typed in, and the error it showed
("That graduation year does not look right") pointed at a field the member had
deliberately left empty.

The guard compared against `undefined` while the value is `null` for a blank
field: `null !== undefined` is true and `Number.isInteger(null)` is false, so
every save with an empty year fell into the error branch. `null` is the value
that CLEARS the column, so it has to pass.

Introduced by the review fix on #323, which changed the payload from
`undefined` to `null` to make fields clearable and did not move the guard with
it. Found while auditing the product against MVP.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant