Feature: stack/4-admin to dev - #322
Closed
github-actions[bot] wants to merge 9 commits into
Closed
Conversation
* adding * feat(club): decouple membership from the hackathon edition A club membership was welded to a hackathon edition: `member` was `unique(userId, hackathonId)` NOT NULL and every read resolved through `resolveCurrentHackathonId`. The day next year's edition flipped to `open`, every paying member silently became a non-member — the portal showed the pay button, club check-in threw at the door, initiatives locked out, and re-paying discarded whatever months were left. Membership is now annual and belongs to a person: `unique(userId)`, and the edition clause is gone from every read site (registration, member, events, initiative, portal-context, verify-email). `membership_history` is finally written on join and renewal, because `hackathonId` had been doing accidental duty as the only record of which year somebody was a member. Also here: - The Stripe payment row commits before any membership work. The two shared a transaction, so a grant that threw rolled back the payment record as well — the customer was charged and nothing anywhere recorded it, and none of the recovery paths could help because they look for a row that was never written. - Mock mode grants a real membership through the production confirm path, so the club half can be developed without a Stripe key. The old test asserted a returned shape, which is how "Access Granted with nothing written" survived. - verify-email calls the shared membership service instead of its own third copy, which restarted the term from today and wrote no history. The database change is deliberately NOT applied by `drizzle-kit push` — push offers to truncate when adding the unique constraint. Apply packages/db/ddl/2026-08-08-membership-decouple.sql by hand; it back-fills membership_history before dropping the column it replaces. Verified: typecheck, 373 tests, lint --max-warnings 0, build. * fix(stripe): recover a payment whose membership grant failed Both points raised by Greptile on #316. **A linked payment could never be retried.** The webhook records the payment first and grants the membership after — deliberately, because sharing a transaction meant a failed grant rolled the payment row back and lost the charge entirely. But that ordering leaves a real state: payment row linked to the user, no membership. Every recovery path skipped already-linked payments (`if (existing.linkedUserId) continue;`), so that state was permanent — the customer was charged, the payment was on file, and nothing ever retried. reconcileMyPayments now treats "linked" as "not proof of a grant" and checks membership_history instead. Every grant writes a history row, so a payment with no history row at or after its own timestamp was never honoured. That also distinguishes a failed grant from a membership that was granted a year ago and has since lapsed, which must not be silently renewed off an old payment. Both directions are tested, and the guard was mutation-tested. **The DDL file overstated what it covers.** It handles only the change drizzle-kit push cannot be trusted with (the unique constraint, where push offers to truncate), but the same release also changes the judging tables. The file now says so and gives the order: apply it, run migrate:push for the rest, then push must report "No changes detected". Verified: typecheck, 375 tests, lint --max-warnings 0, build. ---------
…live funnel (#318) * feat(hackathon): close the notification loop Four things the product collected data for and then never used. **The interest list is finally told.** It exists for exactly one moment — registration opening — and nothing sent it, so the runbook told organisers to hand-compose an announcement instead. Registration Controls now offers "Email N interested" the moment an edition is open, 500 at a time, stamping hackathon_interest.registration_open_email_sent_at as each one goes out. A closed tab resumes rather than re-sending. **Announcements survive a closed tab.** The send loop runs in an organiser's browser and walked the audience by offset across separate requests, re-resolving it each time — so any row that moved between batches shifted the window, and a half-finished send could only be repeated from the start. Composing now freezes the message and its audience into rows (hackathon_announcement and hackathon_announcement_recipient), and each recipient is marked as their message leaves. Unfinished sends are listed and resumable, and "exactly once" is a unique constraint rather than arithmetic in a browser. **/hacklytics survives registration opening.** getUpcoming filtered to `announced` only, so the single public entrance to the hackathon — the 2027 site's only CTA and the navbar both land there — went blank with "Nothing announced yet" at the exact moment it mattered. It now covers announced, open and in_progress, and swaps the interest form for a register CTA. **/submit says whether it is open.** The page never queried the window it was gated on, so an attendee could write a full description and learn it was refused only on submit. Both now share computeSubmissionWindow. Also: one email template layer. Every message is {subject, heading, paragraphs[], cta} with the text always escaped and a plain-text alternative built from the same content; the acceptance email was folded onto it from its own hand-written HTML and its own escaping. Database: packages/db/ddl/2026-08-08-notifications.sql. Additive only — safe to apply before this deploys. Verified: typecheck, 391 tests, lint --max-warnings 0, build. * fix(notifications): claim recipients before sending, and sanitise webhook logs Greptile on #318, plus the CodeQL alerts on main. **Overlapping batches could mail the same person twice.** Both send loops selected rows with no sent marker and wrote the marker afterwards, so two requests — two organisers, or one impatient double-click — read the same rows and both sent to them. Each loop now claims its batch with a single atomic UPDATE and works only from what it won; the loser gets a smaller batch instead of a duplicate delivery. A claim older than fifteen minutes is reclaimable, so a request killed by a deploy or a timeout does not strand its recipients and make the send permanently unfinishable. **The interest send reported itself finished while recipients were pending.** `done` was inferred from the batch size, so a batch where some addresses failed came back done with those people never mailed. Failures are now marked — left pending, a permanently bad address is retried on every batch and the send can never finish — and `done` is counted from the table rather than inferred. **Externally-controlled format string / log injection** (CodeQL, 2 high + 2 medium on main). The Stripe webhook interpolated ids from the request body into `console.error` messages. The mock branch parses that body without verifying a signature, so those values are not always Stripe's: a newline forges log entries and a `%s` is read as a format directive. Ids now go through `safeLogId` and are passed as arguments, never as the message. Verified: typecheck, 391 tests, lint --max-warnings 0, build. The claim guard was mutation-tested.
* feat(judging): make judging work without an organiser improvising **Approving a judge builds their queue.** `setActive` flipped a flag and created nothing; `judge.register` always writes an assignment row and `assignToHackathon` refuses anyone who already has one — so the documented recruitment path produced an active judge whose portal said "All Done" having judged nothing. The only remedy was `assignJudgesToProjects`, which deletes and rebuilds every queue in the event and is unsafe once judging is live. Approval now builds the queue itself, and only when the queue is empty: a judge suspended mid-event and reinstated comes back to the queue they were part-way through. **Late submissions reach a judge.** `promoteSubmissions` returned `queuesNeedRebuild` and nothing consumed it beyond a banner, so a project promoted after assignment sat in nobody's queue, received zero votes, and was then dropped from the standings entirely by the zero-vote rule — the team simply absent. Promotion now appends those projects to the queues that already exist, at the coverage the existing queues give a project, without reordering a single row anyone is part-way through. The banner now means only "these projects reached no judge", which is a track mismatch a human has to resolve. **A judge's track has to exist.** Free text went straight into the routing column, and any string not on the edition classified them as sponsor/special and filtered their pool to zero — invisible to them and to the organiser, and uncorrectable because assignToHackathon refuses a second assignment. Both entry points now validate against the edition's own tracks and challenges, both pickers offer them, and `updateAssignmentTrack` can fix a wrong one (rebuilding the queue unless the judge has already scored). **Recruitment stays open when registration closes.** `closed` — the natural step before recruiting judges — emptied /judge/register with no explanation. Also: judges are told they were approved (the success screen promised it), applicants get a pending state where the apply button used to sit and throw, and results can be announced from the same screen that publishes them. Verified: typecheck, 405 tests, lint --max-warnings 0, build. * fix(judging): serialise queue creation, refuse a track change that has scores Both points from Greptile on #320. **Two approvals could build the queue twice.** "Is the queue empty? then build it" is a read followed by a write, and judge_queue has no unique on (judge, project) to fall back on — so a double-click, or two organisers working the same list, both saw an empty queue and both built one, handing the judge every project twice. The check and the build now happen in one transaction behind a lock on the judge row: the second request sees what the first wrote and does nothing. **Changing a track after scoring left the queue disagreeing with it.** The track was written and the rebuild skipped, so the judge carried on scoring the pool they were routed to before with nothing on screen saying so. It now refuses, names how many projects they have already scored, and offers an explicit override — the pattern assignJudgesToProjects already uses. Forcing keeps every completed slot (skipProject marks one done without writing a vote, so they cannot be rebuilt from the votes table) and rebuilds only the remainder, numbering the new slots after the kept ones. Both guards are mutation-tested: removing the lock fails the lock test, and removing the refusal fails two. Verified: typecheck, 410 tests, lint --max-warnings 0, build.
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.
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.
Contributor
Author
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
Author
|
Visit the preview URL for this PR (updated for commit ab32833): https://hacklytics2027--pr-322-qqoyikjb.web.app (expires Sun, 16 Aug 2026 01:40:36 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: c48ba34db61581e25fe2978355160b5eefe0e83f |
| // directive. Flagged by CodeQL as both. | ||
| console.error( | ||
| "[Stripe webhook] membership grant failed for checkout session", | ||
| safeLogId(session.id), |
| // Same reasoning as the checkout-session branch above. | ||
| console.error( | ||
| "[Stripe webhook] membership grant failed for payment intent", | ||
| safeLogId(pi.id), |
Member
|
Superseded. This branch's work is already on |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated PR tracking changes from
stack/4-adminintodev.