From 81586d408682f23f15ea76248855cf0b7ed881cd Mon Sep 17 00:00:00 2001 From: Caleb Bae <144546374+balebbae@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:59:13 -0500 Subject: [PATCH 01/11] chore(main): release 0.13.0 (#141) --- CHANGELOG.md | 20 ++++++++++++++++++++ version.txt | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f234418..a7cc77d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,25 @@ # Changelog +## [0.13.0](https://github.com/hackutd/harp/compare/v0.12.0...v0.13.0) (2026-09-02) + + +### Features + +* "Add to Home Screen" wording + "Get Notified" push dialog ([#142](https://github.com/hackutd/harp/issues/142)) ([db20740](https://github.com/hackutd/harp/commit/db20740136e4917f4252c849e1422f098939d6f9)) +* auto-open install walkthrough on mobile browsers instead of toast ([#144](https://github.com/hackutd/harp/issues/144)) ([74598c5](https://github.com/hackutd/harp/commit/74598c58fe3ce771adaa667fe4c47b17f8ee3937)) +* hide hacker information from admins ([#140](https://github.com/hackutd/harp/issues/140)) ([f9ec8d0](https://github.com/hackutd/harp/commit/f9ec8d09e6533372ab8a95ca1f3013047c50d513)) +* install walkthrough slideshow + push notification dialog ([#138](https://github.com/hackutd/harp/issues/138)) ([67c84cc](https://github.com/hackutd/harp/commit/67c84cc7754d25f3cbfcfd44d186e1e11bb24eae)) +* many frontend improvements & delete user ([#151](https://github.com/hackutd/harp/issues/151)) ([7bf35b0](https://github.com/hackutd/harp/commit/7bf35b0078d6f7930efdbcb31b5a7dd252e664b9)) +* performance optimizations ([#149](https://github.com/hackutd/harp/issues/149)) ([ae11579](https://github.com/hackutd/harp/commit/ae115796032c1f1df0a55b8261ce12c4791a07b2)) +* rsvp and travel ([#145](https://github.com/hackutd/harp/issues/145)) ([49ba0e8](https://github.com/hackutd/harp/commit/49ba0e892e90e61ae4dffd9134e18e71ec52dc19)) +* superadmin-configurable Hacker Links shown as cards on hacker home ([#143](https://github.com/hackutd/harp/issues/143)) ([f3d6a1c](https://github.com/hackutd/harp/commit/f3d6a1c7cfd60f37cbb82efa97ed0c75fda94640)) +* surface hacker meal group in portal UI ([#139](https://github.com/hackutd/harp/issues/139)) ([4a20ee8](https://github.com/hackutd/harp/commit/4a20ee82d79960ee24f0f46f7ef0e2dd11252c60)) + + +### Bug Fixes + +* key rate limiter by session user with per-IP fallback ([#152](https://github.com/hackutd/harp/issues/152)) ([cca2761](https://github.com/hackutd/harp/commit/cca2761f4e9f855ffbe9c60c2d0ae038da4378f0)) + ## [0.12.0](https://github.com/hackutd/harp/compare/v0.11.0...v0.12.0) (2026-08-27) diff --git a/version.txt b/version.txt index ac454c6a..54d1a4f2 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.12.0 +0.13.0 From b95bb89ef0ab49dad6cea17845855d2dcf2fd9f6 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 09:38:25 -0500 Subject: [PATCH 02/11] chore(go): upgrade toolchain to Go 1.27 and bump vulnerable dependencies (#154) Co-authored-by: Caleb Bae --- .claude/skills/ci-pipeline/SKILL.md | 6 +- .github/workflows/audit.yaml | 4 +- Dockerfile | 2 +- dev.Dockerfile | 2 +- go.mod | 94 +++++++++++------------- go.sum | 109 ++++++++++++++-------------- 6 files changed, 105 insertions(+), 112 deletions(-) diff --git a/.claude/skills/ci-pipeline/SKILL.md b/.claude/skills/ci-pipeline/SKILL.md index d153cc46..20fd80aa 100644 --- a/.claude/skills/ci-pipeline/SKILL.md +++ b/.claude/skills/ci-pipeline/SKILL.md @@ -49,14 +49,14 @@ fails, the check is red and the PR can't merge cleanly. ### `backend-audit` job (Go) -Go version **1.24.x**. Steps, in order — each is a gate: +Go version **1.27.x**. Steps, in order — each is a gate: 1. **Check gofmt** — `gofmt -l .`; fails if any file is unformatted. Fix with `gofmt -w .`. 2. **Verify Dependencies** — `go mod verify`. 3. **Build** — `go build -v ./...`. 4. **go vet** — `go vet ./...`. -5. **staticcheck** — installs `honnef.co/go/tools/cmd/staticcheck@v0.6.1`, then +5. **staticcheck** — installs `honnef.co/go/tools/cmd/staticcheck@v0.8.1`, then `staticcheck ./...`. 6. **Tests** — `go test -race ./...` (race detector on). @@ -134,7 +134,7 @@ Multi-stage, producing a tiny `scratch` image: 1. **Stage `frontend`** (`node:22-alpine`): `npm ci` then `npm run build` in `client/portal`. Takes a build arg `VITE_GOOGLE_AUTH_ENABLED` (default `true`). -2. **Stage `builder`** (`golang:1.24`): `go mod download`, then a static build +2. **Stage `builder`** (`golang:1.27`): `go mod download`, then a static build `CGO_ENABLED=0 GOOS=linux go build -trimpath -ldflags="-s -w" -o /app/api ./cmd/api`. 3. **Stage final** (`scratch`): copies CA certs, the `api` binary, and the built frontend into `./static`. `EXPOSE 8080`, `CMD ["./api"]`. diff --git a/.github/workflows/audit.yaml b/.github/workflows/audit.yaml index 16411b18..951d1486 100644 --- a/.github/workflows/audit.yaml +++ b/.github/workflows/audit.yaml @@ -18,7 +18,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v6 with: - go-version: "1.24.x" + go-version: "1.27.x" - name: Check gofmt run: | @@ -42,7 +42,7 @@ jobs: run: go vet ./... - name: Install staticcheck - run: go install honnef.co/go/tools/cmd/staticcheck@v0.6.1 + run: go install honnef.co/go/tools/cmd/staticcheck@v0.8.1 - name: Run staticcheck run: staticcheck ./... diff --git a/Dockerfile b/Dockerfile index 8df8f10f..9ec71800 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,7 +13,7 @@ ENV VITE_GOOGLE_AUTH_ENABLED=$VITE_GOOGLE_AUTH_ENABLED RUN npm run build # Stage 2: Build backend -FROM golang:1.24.13 AS builder +FROM golang:1.27.1 AS builder WORKDIR /app COPY go.mod go.sum ./ diff --git a/dev.Dockerfile b/dev.Dockerfile index 3cf6714a..51fe87a8 100644 --- a/dev.Dockerfile +++ b/dev.Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.24.13 +FROM golang:1.27.1 WORKDIR /app # Install dev tools: air (hot reload), swag (swagger), task (taskfile runner) diff --git a/go.mod b/go.mod index f7798c79..e4eb8825 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,6 @@ module github.com/hackutd/harp -go 1.24.0 - -toolchain go1.24.11 +go 1.27.0 require ( cloud.google.com/go/storage v1.60.0 @@ -10,9 +8,11 @@ require ( github.com/go-chi/chi v1.5.5 github.com/go-chi/cors v1.2.2 github.com/go-playground/validator/v10 v10.30.1 - github.com/jackc/pgx/v5 v5.8.0 + github.com/jackc/pgx/v5 v5.9.2 github.com/joho/godotenv v1.5.1 + github.com/sendgrid/sendgrid-go v3.16.1+incompatible github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e + github.com/stretchr/testify v1.11.1 github.com/supertokens/supertokens-golang v0.25.1 github.com/swaggo/http-swagger v1.3.4 github.com/swaggo/swag v1.16.6 @@ -23,54 +23,29 @@ require ( ) require ( - cel.dev/expr v0.24.0 // indirect + cel.dev/expr v0.25.1 // indirect cloud.google.com/go v0.123.0 // indirect cloud.google.com/go/auth v0.18.1 // indirect cloud.google.com/go/auth/oauth2adapt v0.2.8 // indirect cloud.google.com/go/compute/metadata v0.9.0 // indirect cloud.google.com/go/iam v1.5.3 // indirect cloud.google.com/go/monitoring v1.24.3 // indirect - github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.30.0 // indirect + github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.32.0 // indirect github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.55.0 // indirect github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.55.0 // indirect - github.com/cespare/xxhash/v2 v2.3.0 // indirect - github.com/cncf/xds/go v0.0.0-20251022180443-0feb69152e9f // indirect - github.com/envoyproxy/go-control-plane/envoy v1.35.0 // indirect - github.com/envoyproxy/protoc-gen-validate v1.2.1 // indirect - github.com/felixge/httpsnoop v1.0.4 // indirect - github.com/go-jose/go-jose/v4 v4.1.3 // indirect - github.com/go-logr/logr v1.4.3 // indirect - github.com/go-logr/stdr v1.2.2 // indirect - github.com/google/s2a-go v0.1.9 // indirect - github.com/google/uuid v1.6.0 // indirect - github.com/googleapis/enterprise-certificate-proxy v0.3.11 // indirect - github.com/googleapis/gax-go/v2 v2.17.0 // indirect - github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect - github.com/spiffe/go-spiffe/v2 v2.6.0 // indirect - go.opentelemetry.io/auto/sdk v1.2.1 // indirect - go.opentelemetry.io/contrib/detectors/gcp v1.38.0 // indirect - go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.63.0 // indirect - go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.61.0 // indirect - go.opentelemetry.io/otel v1.39.0 // indirect - go.opentelemetry.io/otel/metric v1.39.0 // indirect - go.opentelemetry.io/otel/sdk v1.39.0 // indirect - go.opentelemetry.io/otel/sdk/metric v1.39.0 // indirect - go.opentelemetry.io/otel/trace v1.39.0 // indirect - golang.org/x/oauth2 v0.35.0 // indirect - golang.org/x/time v0.14.0 // indirect - google.golang.org/genproto v0.0.0-20260128011058-8636f8732409 // indirect - google.golang.org/genproto/googleapis/api v0.0.0-20260203192932-546029d2fa20 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20260203192932-546029d2fa20 // indirect - google.golang.org/grpc v1.78.0 // indirect - google.golang.org/protobuf v1.36.11 // indirect -) - -require ( github.com/KyleBanks/depth v1.2.1 // indirect github.com/MicahParks/keyfunc/v2 v2.1.0 // indirect + github.com/cespare/xxhash/v2 v2.3.0 // indirect + github.com/cncf/xds/go v0.0.0-20260202195803-dba9d589def2 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/derekstavis/go-qs v0.0.0-20180720192143-9eef69e6c4e7 // indirect + github.com/envoyproxy/go-control-plane/envoy v1.37.0 // indirect + github.com/envoyproxy/protoc-gen-validate v1.3.3 // indirect + github.com/felixge/httpsnoop v1.0.4 // indirect github.com/gabriel-vasile/mimetype v1.4.12 // indirect + github.com/go-jose/go-jose/v4 v4.1.4 // indirect + github.com/go-logr/logr v1.4.3 // indirect + github.com/go-logr/stdr v1.2.2 // indirect github.com/go-openapi/jsonpointer v0.19.5 // indirect github.com/go-openapi/jsonreference v0.20.0 // indirect github.com/go-openapi/spec v0.20.6 // indirect @@ -79,7 +54,10 @@ require ( github.com/go-playground/universal-translator v0.18.1 // indirect github.com/golang-jwt/jwt/v5 v5.3.0 // indirect github.com/golang/mock v1.6.0 // indirect - github.com/golang/protobuf v1.5.4 // indirect + github.com/google/s2a-go v0.1.9 // indirect + github.com/google/uuid v1.6.0 // indirect + github.com/googleapis/enterprise-certificate-proxy v0.3.11 // indirect + github.com/googleapis/gax-go/v2 v2.17.0 // indirect github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect @@ -87,23 +65,39 @@ require ( github.com/josharian/intern v1.0.0 // indirect github.com/leodido/go-urn v1.4.0 // indirect github.com/mailru/easyjson v0.7.6 // indirect - github.com/nyaruka/phonenumbers v1.0.73 // indirect + github.com/nyaruka/phonenumbers v1.8.1 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/sendgrid/rest v2.6.9+incompatible // indirect - github.com/sendgrid/sendgrid-go v3.16.1+incompatible + github.com/spiffe/go-spiffe/v2 v2.6.0 // indirect github.com/stretchr/objx v0.5.2 // indirect - github.com/stretchr/testify v1.11.1 github.com/swaggo/files v0.0.0-20220610200504-28940afbdbfe // indirect github.com/twilio/twilio-go v0.26.0 // indirect + go.opentelemetry.io/auto/sdk v1.2.1 // indirect + go.opentelemetry.io/contrib/detectors/gcp v1.43.0 // indirect + go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.63.0 // indirect + go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.61.0 // indirect + go.opentelemetry.io/otel v1.43.0 // indirect + go.opentelemetry.io/otel/metric v1.43.0 // indirect + go.opentelemetry.io/otel/sdk v1.43.0 // indirect + go.opentelemetry.io/otel/sdk/metric v1.43.0 // indirect + go.opentelemetry.io/otel/trace v1.43.0 // indirect go.uber.org/multierr v1.11.0 // indirect - golang.org/x/crypto v0.47.0 // indirect - golang.org/x/mod v0.31.0 // indirect - golang.org/x/net v0.49.0 // indirect - golang.org/x/sync v0.19.0 // indirect - golang.org/x/sys v0.40.0 // indirect - golang.org/x/text v0.33.0 // indirect - golang.org/x/tools v0.40.0 // indirect + golang.org/x/crypto v0.53.0 // indirect + golang.org/x/mod v0.37.0 // indirect + golang.org/x/net v0.56.0 // indirect + golang.org/x/oauth2 v0.36.0 // indirect + golang.org/x/sync v0.21.0 // indirect + golang.org/x/sys v0.46.0 // indirect + golang.org/x/text v0.39.0 // indirect + golang.org/x/time v0.14.0 // indirect + golang.org/x/tools v0.47.0 // indirect + google.golang.org/genproto v0.0.0-20260128011058-8636f8732409 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 // indirect + google.golang.org/grpc v1.82.1 // indirect + google.golang.org/protobuf v1.36.11 // indirect gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect gopkg.in/gomail.v2 v2.0.0-20160411212932-81ebce5c23df // indirect gopkg.in/h2non/gock.v1 v1.1.2 // indirect diff --git a/go.sum b/go.sum index 4dd8dff5..e600be3e 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -cel.dev/expr v0.24.0 h1:56OvJKSH3hDGL0ml5uSxZmz3/3Pq4tJ+fb1unVLAFcY= -cel.dev/expr v0.24.0/go.mod h1:hLPLo1W4QUmuYdA72RBX06QTs6MXw941piREPl3Yfiw= +cel.dev/expr v0.25.1 h1:1KrZg61W6TWSxuNZ37Xy49ps13NUovb66QLprthtwi4= +cel.dev/expr v0.25.1/go.mod h1:hrXvqGP6G6gyx8UAHSHJ5RGk//1Oj5nXQ2NI02Nrsg4= cloud.google.com/go v0.123.0 h1:2NAUJwPR47q+E35uaJeYoNhuNEM9kM8SjgRgdeOJUSE= cloud.google.com/go v0.123.0/go.mod h1:xBoMV08QcqUGuPW65Qfm1o9Y4zKZBpGS+7bImXLTAZU= cloud.google.com/go/auth v0.18.1 h1:IwTEx92GFUo2pJ6Qea0EU3zYvKnTAeRCODxfA/G5UWs= @@ -20,8 +20,8 @@ cloud.google.com/go/storage v1.60.0 h1:oBfZrSOCimggVNz9Y/bXY35uUcts7OViubeddTTVz cloud.google.com/go/storage v1.60.0/go.mod h1:q+5196hXfejkctrnx+VYU8RKQr/L3c0cBIlrjmiAKE0= cloud.google.com/go/trace v1.11.7 h1:kDNDX8JkaAG3R2nq1lIdkb7FCSi1rCmsEtKVsty7p+U= cloud.google.com/go/trace v1.11.7/go.mod h1:TNn9d5V3fQVf6s4SCveVMIBS2LJUqo73GACmq/Tky0s= -github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.30.0 h1:sBEjpZlNHzK1voKq9695PJSX2o5NEXl7/OL3coiIY0c= -github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.30.0/go.mod h1:P4WPRUkOhJC13W//jWpyfJNDAIpvRbAUIYLX/4jtlE0= +github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.32.0 h1:rIkQfkCOVKc1OiRCNcSDD8ml5RJlZbH/Xsq7lbpynwc= +github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.32.0/go.mod h1:RD2SsorTmYhF6HkTmDw7KmPYQk8OBYwTkuasChwv7R4= github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.55.0 h1:UnDZ/zFfG1JhH/DqxIZYU/1CUAlTUScoXD/LcM2Ykk8= github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.55.0/go.mod h1:IA1C1U7jO/ENqm/vhi7V9YYpBsp+IMyqNrEN94N7tVc= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0.55.0 h1:7t/qx5Ost0s0wbA/VDrByOooURhp+ikYwv20i9Y07TQ= @@ -36,8 +36,8 @@ github.com/SherClockHolmes/webpush-go v1.4.0 h1:ocnzNKWN23T9nvHi6IfyrQjkIc0oJWv1 github.com/SherClockHolmes/webpush-go v1.4.0/go.mod h1:XSq8pKX11vNV8MJEMwjrlTkxhAj1zKfxmyhdV7Pd6UA= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= -github.com/cncf/xds/go v0.0.0-20251022180443-0feb69152e9f h1:Y8xYupdHxryycyPlc9Y+bSQAYZnetRJ70VMVKm5CKI0= -github.com/cncf/xds/go v0.0.0-20251022180443-0feb69152e9f/go.mod h1:HlzOvOjVBOfTGSRXRyY0OiCS/3J1akRGQQpRO/7zyF4= +github.com/cncf/xds/go v0.0.0-20260202195803-dba9d589def2 h1:aBangftG7EVZoUb69Os8IaYg++6uMOdKK83QtkkvJik= +github.com/cncf/xds/go v0.0.0-20260202195803-dba9d589def2/go.mod h1:qwXFYgsP6T7XnJtbKlf1HP8AjxZZyzxMmc+Lq5GjlU4= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -45,14 +45,14 @@ github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/derekstavis/go-qs v0.0.0-20180720192143-9eef69e6c4e7 h1:zmAiXR9h1TCVN/0yCMRYQNE91dNRORpSzMFiqfTTPOs= github.com/derekstavis/go-qs v0.0.0-20180720192143-9eef69e6c4e7/go.mod h1:Vgz4nKcG6+B7QcALsWZpmhyQTLSl7nwFGKSrbq2LxEo= -github.com/envoyproxy/go-control-plane v0.13.5-0.20251024222203-75eaa193e329 h1:K+fnvUM0VZ7ZFJf0n4L/BRlnsb9pL/GuDG6FqaH+PwM= -github.com/envoyproxy/go-control-plane v0.13.5-0.20251024222203-75eaa193e329/go.mod h1:Alz8LEClvR7xKsrq3qzoc4N0guvVNSS8KmSChGYr9hs= -github.com/envoyproxy/go-control-plane/envoy v1.35.0 h1:ixjkELDE+ru6idPxcHLj8LBVc2bFP7iBytj353BoHUo= -github.com/envoyproxy/go-control-plane/envoy v1.35.0/go.mod h1:09qwbGVuSWWAyN5t/b3iyVfz5+z8QWGrzkoqm/8SbEs= +github.com/envoyproxy/go-control-plane v0.14.0 h1:hbG2kr4RuFj222B6+7T83thSPqLjwBIfQawTkC++2HA= +github.com/envoyproxy/go-control-plane v0.14.0/go.mod h1:NcS5X47pLl/hfqxU70yPwL9ZMkUlwlKxtAohpi2wBEU= +github.com/envoyproxy/go-control-plane/envoy v1.37.0 h1:u3riX6BoYRfF4Dr7dwSOroNfdSbEPe9Yyl09/B6wBrQ= +github.com/envoyproxy/go-control-plane/envoy v1.37.0/go.mod h1:DReE9MMrmecPy+YvQOAOHNYMALuowAnbjjEMkkWOi6A= github.com/envoyproxy/go-control-plane/ratelimit v0.1.0 h1:/G9QYbddjL25KvtKTv3an9lx6VBE2cnb8wp1vEGNYGI= github.com/envoyproxy/go-control-plane/ratelimit v0.1.0/go.mod h1:Wk+tMFAFbCXaJPzVVHnPgRKdUdwW/KdbRt94AzgRee4= -github.com/envoyproxy/protoc-gen-validate v1.2.1 h1:DEo3O99U8j4hBFwbJfrz9VtgcDfUKS7KJ7spH3d86P8= -github.com/envoyproxy/protoc-gen-validate v1.2.1/go.mod h1:d/C80l/jxXLdfEIhX1W2TmLfsJ31lvEjwamM4DxlWXU= +github.com/envoyproxy/protoc-gen-validate v1.3.3 h1:MVQghNeW+LZcmXe7SY1V36Z+WFMDjpqGAGacLe2T0ds= +github.com/envoyproxy/protoc-gen-validate v1.3.3/go.mod h1:TsndJ/ngyIdQRhMcVVGDDHINPLWB7C82oDArY51KfB0= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/gabriel-vasile/mimetype v1.4.12 h1:e9hWvmLYvtp846tLHam2o++qitpguFiYCKbn0w9jyqw= @@ -61,8 +61,8 @@ github.com/go-chi/chi v1.5.5 h1:vOB/HbEMt9QqBqErz07QehcOKHaWFtuj87tTDVz2qXE= github.com/go-chi/chi v1.5.5/go.mod h1:C9JqLr3tIYjDOZpzn+BCuxY8z8vmca43EeMgyZt7irw= github.com/go-chi/cors v1.2.2 h1:Jmey33TE+b+rB7fT8MUy1u0I4L+NARQlK6LhzKPSyQE= github.com/go-chi/cors v1.2.2/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58= -github.com/go-jose/go-jose/v4 v4.1.3 h1:CVLmWDhDVRa6Mi/IgCgaopNosCaHz7zrMeF9MlZRkrs= -github.com/go-jose/go-jose/v4 v4.1.3/go.mod h1:x4oUasVrzR7071A4TnHLGSPpNOm2a21K9Kf04k1rs08= +github.com/go-jose/go-jose/v4 v4.1.4 h1:moDMcTHmvE6Groj34emNPLs/qtYXRVcd6S7NHbHz3kA= +github.com/go-jose/go-jose/v4 v4.1.4/go.mod h1:x4oUasVrzR7071A4TnHLGSPpNOm2a21K9Kf04k1rs08= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= @@ -92,7 +92,6 @@ github.com/golang-jwt/jwt/v5 v5.3.0 h1:pv4AsKCKKZuqlgs5sUmn4x8UlGa0kEVt/puTpKx9v github.com/golang-jwt/jwt/v5 v5.3.0/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= -github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= @@ -114,8 +113,8 @@ github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsI github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg= github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 h1:iCEnooe7UlwOQYpKFhBabPMi4aNAfoODPEFNiAnClxo= github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM= -github.com/jackc/pgx/v5 v5.8.0 h1:TYPDoleBBme0xGSAX3/+NujXXtpZn9HBONkQC7IEZSo= -github.com/jackc/pgx/v5 v5.8.0/go.mod h1:QVeDInX2m9VyzvNeiCJVjCkNFqzsNb43204HshNSZKw= +github.com/jackc/pgx/v5 v5.9.2 h1:3ZhOzMWnR4yJ+RW1XImIPsD1aNSz4T4fyP7zlQb56hw= +github.com/jackc/pgx/v5 v5.9.2/go.mod h1:mal1tBGAFfLHvZzaYh77YS/eC6IX9OWbRV1QIIM0Jn4= github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo= github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4= github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0= @@ -138,8 +137,8 @@ github.com/mailru/easyjson v0.7.6/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJ github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32 h1:W6apQkHrMkS0Muv8G/TipAy/FJl/rCYT0+EuS8+Z0z4= github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32/go.mod h1:9wM+0iRr9ahx58uYLpLIr5fm8diHn0JbqRycJi6w0Ms= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= -github.com/nyaruka/phonenumbers v1.0.73 h1:bP2WN8/NUP8tQebR+WCIejFaibwYMHOaB7MQVayclUo= -github.com/nyaruka/phonenumbers v1.0.73/go.mod h1:3aiS+PS3DuYwkbK3xdcmRwMiPNECZ0oENH8qUT1lY7Q= +github.com/nyaruka/phonenumbers v1.8.1 h1:2K9YMQuv1dCGqjjzB1DwmdCe89khT4KPBQb2CxAMMlU= +github.com/nyaruka/phonenumbers v1.8.1/go.mod h1:fsKPJ70O9JetEA4ggnJadYTFWwtGPvu/lETTXNXq6Cs= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 h1:GFCKgmp0tecUJ0sJuv4pzYCqS9+RGSn52M3FUwPs+uo= @@ -183,24 +182,24 @@ go.mozilla.org/pkcs7 v0.10.0 h1:jmljzDzNYFzaP1dFlgmCiQml9e+iEMmv8/NNs4evQbg= go.mozilla.org/pkcs7 v0.10.0/go.mod h1:SNgMg+EgDFwmvSmLRTNKC5fegJjB7v23qTQ0XLGUNHk= go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= -go.opentelemetry.io/contrib/detectors/gcp v1.38.0 h1:ZoYbqX7OaA/TAikspPl3ozPI6iY6LiIY9I8cUfm+pJs= -go.opentelemetry.io/contrib/detectors/gcp v1.38.0/go.mod h1:SU+iU7nu5ud4oCb3LQOhIZ3nRLj6FNVrKgtflbaf2ts= +go.opentelemetry.io/contrib/detectors/gcp v1.43.0 h1:62yY3dT7/ShwOxzA0RsKRgshBmfElKI4d/Myu2OxDFU= +go.opentelemetry.io/contrib/detectors/gcp v1.43.0/go.mod h1:RyaZMFY7yi1kAs45S6mbFGz8O8rqB0dTY14uzvG4LCs= go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.63.0 h1:YH4g8lQroajqUwWbq/tr2QX1JFmEXaDLgG+ew9bLMWo= go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.63.0/go.mod h1:fvPi2qXDqFs8M4B4fmJhE92TyQs9Ydjlg3RvfUp+NbQ= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.61.0 h1:F7Jx+6hwnZ41NSFTO5q4LYDtJRXBf2PD0rNBkeB/lus= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.61.0/go.mod h1:UHB22Z8QsdRDrnAtX4PntOl36ajSxcdUMt1sF7Y6E7Q= -go.opentelemetry.io/otel v1.39.0 h1:8yPrr/S0ND9QEfTfdP9V+SiwT4E0G7Y5MO7p85nis48= -go.opentelemetry.io/otel v1.39.0/go.mod h1:kLlFTywNWrFyEdH0oj2xK0bFYZtHRYUdv1NklR/tgc8= +go.opentelemetry.io/otel v1.43.0 h1:mYIM03dnh5zfN7HautFE4ieIig9amkNANT+xcVxAj9I= +go.opentelemetry.io/otel v1.43.0/go.mod h1:JuG+u74mvjvcm8vj8pI5XiHy1zDeoCS2LB1spIq7Ay0= go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v1.39.0 h1:5gn2urDL/FBnK8OkCfD1j3/ER79rUuTYmCvlXBKeYL8= go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v1.39.0/go.mod h1:0fBG6ZJxhqByfFZDwSwpZGzJU671HkwpWaNe2t4VUPI= -go.opentelemetry.io/otel/metric v1.39.0 h1:d1UzonvEZriVfpNKEVmHXbdf909uGTOQjA0HF0Ls5Q0= -go.opentelemetry.io/otel/metric v1.39.0/go.mod h1:jrZSWL33sD7bBxg1xjrqyDjnuzTUB0x1nBERXd7Ftcs= -go.opentelemetry.io/otel/sdk v1.39.0 h1:nMLYcjVsvdui1B/4FRkwjzoRVsMK8uL/cj0OyhKzt18= -go.opentelemetry.io/otel/sdk v1.39.0/go.mod h1:vDojkC4/jsTJsE+kh+LXYQlbL8CgrEcwmt1ENZszdJE= -go.opentelemetry.io/otel/sdk/metric v1.39.0 h1:cXMVVFVgsIf2YL6QkRF4Urbr/aMInf+2WKg+sEJTtB8= -go.opentelemetry.io/otel/sdk/metric v1.39.0/go.mod h1:xq9HEVH7qeX69/JnwEfp6fVq5wosJsY1mt4lLfYdVew= -go.opentelemetry.io/otel/trace v1.39.0 h1:2d2vfpEDmCJ5zVYz7ijaJdOF59xLomrvj7bjt6/qCJI= -go.opentelemetry.io/otel/trace v1.39.0/go.mod h1:88w4/PnZSazkGzz/w84VHpQafiU4EtqqlVdxWy+rNOA= +go.opentelemetry.io/otel/metric v1.43.0 h1:d7638QeInOnuwOONPp4JAOGfbCEpYb+K6DVWvdxGzgM= +go.opentelemetry.io/otel/metric v1.43.0/go.mod h1:RDnPtIxvqlgO8GRW18W6Z/4P462ldprJtfxHxyKd2PY= +go.opentelemetry.io/otel/sdk v1.43.0 h1:pi5mE86i5rTeLXqoF/hhiBtUNcrAGHLKQdhg4h4V9Dg= +go.opentelemetry.io/otel/sdk v1.43.0/go.mod h1:P+IkVU3iWukmiit/Yf9AWvpyRDlUeBaRg6Y+C58QHzg= +go.opentelemetry.io/otel/sdk/metric v1.43.0 h1:S88dyqXjJkuBNLeMcVPRFXpRw2fuwdvfCGLEo89fDkw= +go.opentelemetry.io/otel/sdk/metric v1.43.0/go.mod h1:C/RJtwSEJ5hzTiUz5pXF1kILHStzb9zFlIEe85bhj6A= +go.opentelemetry.io/otel/trace v1.43.0 h1:BkNrHpup+4k4w+ZZ86CZoHHEkohws8AY+WTX09nk+3A= +go.opentelemetry.io/otel/trace v1.43.0/go.mod h1:/QJhyVBUUswCphDVxq+8mld+AvhXZLhe+8WVFxiFff0= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= @@ -214,16 +213,16 @@ golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliY golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v8= golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk= -golang.org/x/crypto v0.47.0 h1:V6e3FRj+n4dbpw86FJ8Fv7XVOql7TEwpHapKoMJ/GO8= -golang.org/x/crypto v0.47.0/go.mod h1:ff3Y9VzzKbwSSEzWqJsJVBnWmRwRSHt/6Op5n9bQc4A= +golang.org/x/crypto v0.53.0 h1:QZ4Muo8THX6CizN2vPPd5fBGHyogrdK9fG4wLPFUsto= +golang.org/x/crypto v0.53.0/go.mod h1:DNLU434OwVakk9PzuwV8w62mAJpRJL3vsgcfp4Qnsio= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.15.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/mod v0.17.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= -golang.org/x/mod v0.31.0 h1:HaW9xtz0+kOcWKwli0ZXy79Ix+UW/vOfmWI5QVd2tgI= -golang.org/x/mod v0.31.0/go.mod h1:43JraMp9cGx1Rx3AqioxrbrhNsLl2l/iNAvuBkrezpg= +golang.org/x/mod v0.37.0 h1:vF1DjpVEshcIqoEaauuHebaLk1O1forxjxBaVn884JQ= +golang.org/x/mod v0.37.0/go.mod h1:m8S8VeM9r4dzDwjrKO0a1sZP3YjeMamRRlD+fmR2Q/0= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= @@ -235,10 +234,10 @@ golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk= golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= golang.org/x/net v0.25.0/go.mod h1:JkAGAh7GEvH74S6FOH42FLoXpXbE/aqXSrIQjXgsiwM= -golang.org/x/net v0.49.0 h1:eeHFmOGUTtaaPSGNmjBKpbng9MulQsJURQUAfUwY++o= -golang.org/x/net v0.49.0/go.mod h1:/ysNB2EvaqvesRkuLAyjI1ycPZlQHM3q01F02UY/MV8= -golang.org/x/oauth2 v0.35.0 h1:Mv2mzuHuZuY2+bkyWXIHMfhNdJAdwW3FuWeCPYN5GVQ= -golang.org/x/oauth2 v0.35.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA= +golang.org/x/net v0.56.0 h1:Rw8j/hFzGvJUZwNBXnAtf5sVDVt+65SK2C7IxCxZt5o= +golang.org/x/net v0.56.0/go.mod h1:D3Ku6r+V6JROoZK144D2XfMHFcMq/0zSfLelVTCFKec= +golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= +golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -247,8 +246,8 @@ golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= -golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= -golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= +golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM= +golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -264,8 +263,8 @@ golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.40.0 h1:DBZZqJ2Rkml6QMQsZywtnjnnGvHza6BTfYFWY9kjEWQ= -golang.org/x/sys v0.40.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw= +golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/telemetry v0.0.0-20240228155512-f48c80bd79b2/go.mod h1:TeRTkGYfJXctD9OcfyVLyj2J3IxLnKwHJR8f4D8a3YE= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= @@ -285,8 +284,8 @@ golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.15.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ= -golang.org/x/text v0.33.0 h1:B3njUFyqtHDUI5jMn1YIr5B0IE2U0qck04r6d4KPAxE= -golang.org/x/text v0.33.0/go.mod h1:LuMebE6+rBincTi9+xWTY8TztLzKHc/9C1uBCG27+q8= +golang.org/x/text v0.39.0 h1:UbZz4pLOvn600D6Oh6GGEI6VAmndrEBLv8/6BEXzyus= +golang.org/x/text v0.39.0/go.mod h1:3UwRclnC2g0TU9x8PZiyfOajCd1zaUNHF9cvqcQZ+ZM= golang.org/x/time v0.14.0 h1:MRx4UaLrDotUKUdCIqzPC48t1Y9hANFKIRpNx+Te8PI= golang.org/x/time v0.14.0/go.mod h1:eL/Oa2bBBK0TkX57Fyni+NgnyQQN4LitPmob2Hjnqw4= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= @@ -296,23 +295,23 @@ golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58= golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d/go.mod h1:aiJjzUbINMkxbQROHiO6hDPo2LHcIPhhQsa9DLh0yGk= -golang.org/x/tools v0.40.0 h1:yLkxfA+Qnul4cs9QA3KnlFu0lVmd8JJfoq+E41uSutA= -golang.org/x/tools v0.40.0/go.mod h1:Ik/tzLRlbscWpqqMRjyWYDisX8bG13FrdXp3o4Sr9lc= +golang.org/x/tools v0.47.0 h1:7Kn5x/d1svx/PzryTsqeoZN4TZwqeH5pGWjefhLi/1Q= +golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -gonum.org/v1/gonum v0.16.0 h1:5+ul4Swaf3ESvrOnidPp4GZbzf0mxVQpDCYUQE7OJfk= -gonum.org/v1/gonum v0.16.0/go.mod h1:fef3am4MQ93R2HHpKnLk4/Tbh/s0+wqD5nfa6Pnwy4E= +gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= +gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E= google.golang.org/api v0.265.0 h1:FZvfUdI8nfmuNrE34aOWFPmLC+qRBEiNm3JdivTvAAU= google.golang.org/api v0.265.0/go.mod h1:uAvfEl3SLUj/7n6k+lJutcswVojHPp2Sp08jWCu8hLY= google.golang.org/genproto v0.0.0-20260128011058-8636f8732409 h1:VQZ/yAbAtjkHgH80teYd2em3xtIkkHd7ZhqfH2N9CsM= google.golang.org/genproto v0.0.0-20260128011058-8636f8732409/go.mod h1:rxKD3IEILWEu3P44seeNOAwZN4SaoKaQ/2eTg4mM6EM= -google.golang.org/genproto/googleapis/api v0.0.0-20260203192932-546029d2fa20 h1:7ei4lp52gK1uSejlA8AZl5AJjeLUOHBQscRQZUgAcu0= -google.golang.org/genproto/googleapis/api v0.0.0-20260203192932-546029d2fa20/go.mod h1:ZdbssH/1SOVnjnDlXzxDHK2MCidiqXtbYccJNzNYPEE= -google.golang.org/genproto/googleapis/rpc v0.0.0-20260203192932-546029d2fa20 h1:Jr5R2J6F6qWyzINc+4AM8t5pfUz6beZpHp678GNrMbE= -google.golang.org/genproto/googleapis/rpc v0.0.0-20260203192932-546029d2fa20/go.mod h1:j9x/tPzZkyxcgEFkiKEEGxfvyumM01BEtsW8xzOahRQ= -google.golang.org/grpc v1.78.0 h1:K1XZG/yGDJnzMdd/uZHAkVqJE+xIDOcmdSFZkBUicNc= -google.golang.org/grpc v1.78.0/go.mod h1:I47qjTo4OKbMkjA/aOOwxDIiPSBofUtQUI5EfpWvW7U= +google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478 h1:yQugLulqltosq0B/f8l4w9VryjV+N/5gcW0jQ3N8Qec= +google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478/go.mod h1:C6ADNqOxbgdUUeRTU+LCHDPB9ttAMCTff6auwCVa4uc= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 h1:RmoJA1ujG+/lRGNfUnOMfhCy5EipVMyvUE+KNbPbTlw= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= +google.golang.org/grpc v1.82.1 h1:NnAxzGRA0677vCa4BUkOAnO5+FfQqVl9iUXeD0IqcGE= +google.golang.org/grpc v1.82.1/go.mod h1:yzTZ1TB1Z3SG+LIYaI+WiE8D5+PZ3ArnrSp8zF3+/ZA= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc h1:2gGKlE2+asNV9m7xrywl36YYNnBG5ZQ0r/BOOxqPpmk= From 9ca9dffe513669c2d355689a613faed4c95e7c94 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 09:39:03 -0500 Subject: [PATCH 03/11] fix(notifications): restrict push endpoints to known push services and bound dispatcher requests (#153) Co-authored-by: Caleb Bae --- .env.example | 6 ++ cmd/api/api.go | 5 ++ cmd/api/dispatcher.go | 9 ++- cmd/api/dispatcher_test.go | 125 +++++++++++++++++++++++++++++++--- cmd/api/main.go | 8 ++- cmd/api/notifications.go | 5 ++ cmd/api/notifications_test.go | 54 +++++++++++++++ cmd/api/push_endpoint.go | 82 ++++++++++++++++++++++ cmd/api/test_utils_test.go | 3 + 9 files changed, 282 insertions(+), 15 deletions(-) create mode 100644 cmd/api/push_endpoint.go diff --git a/.env.example b/.env.example index 82fa37d3..314039d7 100644 --- a/.env.example +++ b/.env.example @@ -146,6 +146,12 @@ VAPID_PRIVATE_KEY= # Contact address for push services, used if a provider needs to reach you. VAPID_SUBJECT=noreply@example.com +# Comma-separated push-service hosts (exact or any subdomain) that browser +# subscription endpoints must point at. The server POSTs to these URLs, so +# anything else is rejected. Leave empty for the built-in list covering +# Chrome/Edge (FCM), Firefox, Safari, Windows (WNS) and Samsung Internet. +PUSH_ENDPOINT_ALLOWED_HOSTS= + # ── Rate limiting ──────────────────────────────────────────────────────────── diff --git a/cmd/api/api.go b/cmd/api/api.go index a2fc06a9..443822d7 100644 --- a/cmd/api/api.go +++ b/cmd/api/api.go @@ -38,6 +38,8 @@ type application struct { // requiring a session. Injected so tests can stub it. sessionUserID sessionUserIDResolver dispatcherCancel context.CancelFunc + // pushClient is the HTTP client the dispatcher uses to reach push services. + pushClient *http.Client } type config struct { @@ -60,6 +62,9 @@ type vapidConfig struct { publicKey string privateKey string subject string + // allowedEndpointHosts is the push-service host allowlist (exact or + // subdomain match) that subscription endpoints must fall under. + allowedEndpointHosts []string } type supertokensConfig struct { diff --git a/cmd/api/dispatcher.go b/cmd/api/dispatcher.go index 31f26002..142e4ec9 100644 --- a/cmd/api/dispatcher.go +++ b/cmd/api/dispatcher.go @@ -63,6 +63,7 @@ func (app *application) dispatchDueNotifications(ctx context.Context) { VAPIDPrivateKey: app.config.vapid.privateKey, Subscriber: app.config.vapid.subject, TTL: 60 * 60, // 1 hour + HTTPClient: app.pushClient, } for _, n := range due { @@ -102,6 +103,12 @@ func (app *application) deliverNotification(ctx context.Context, n store.Schedul var toPrune []string for _, sub := range subs { + if err := validatePushEndpoint(sub.Endpoint, app.config.vapid.allowedEndpointHosts); err != nil { + app.logger.Warnw("pruning subscription with disallowed endpoint", "endpoint", sub.Endpoint) + toPrune = append(toPrune, sub.Endpoint) + continue + } + webpushSub := &webpush.Subscription{ Endpoint: sub.Endpoint, Keys: webpush.Keys{ @@ -116,7 +123,7 @@ func (app *application) deliverNotification(ctx context.Context, n store.Schedul continue } - _, _ = io.Copy(io.Discard, resp.Body) + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, pushResponseBodyCap)) resp.Body.Close() switch { diff --git a/cmd/api/dispatcher_test.go b/cmd/api/dispatcher_test.go index 1c620288..8c5eda98 100644 --- a/cmd/api/dispatcher_test.go +++ b/cmd/api/dispatcher_test.go @@ -7,7 +7,9 @@ import ( "encoding/base64" "net/http" "net/http/httptest" + "sync/atomic" "testing" + "time" webpush "github.com/SherClockHolmes/webpush-go" "github.com/hackutd/harp/internal/store" @@ -29,25 +31,42 @@ func newTestPushKeys(t *testing.T) (p256dh, auth string) { base64.RawURLEncoding.EncodeToString(authBytes) } -// newPushServer returns an httptest server that responds with the given status to any push. +// newPushServer returns a TLS httptest server that responds with the given status to any push. +// Endpoints must be https and on an allowed host, so tests allowlist the loopback address. func newPushServer(t *testing.T, status int) *httptest.Server { t.Helper() - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(status) })) t.Cleanup(srv.Close) return srv } -func newTestVAPIDOptions(t *testing.T) *webpush.Options { +// newTestDispatcherApp returns a test app whose push allowlist covers the httptest +// loopback servers. +func newTestDispatcherApp(t *testing.T) *application { + t.Helper() + app := newTestApplication(t) + app.config.vapid.allowedEndpointHosts = []string{"127.0.0.1"} + return app +} + +// newTestVAPIDOptions builds dispatcher options that trust the shared httptest +// certificate. Every httptest TLS server uses the same cert, so one server's +// client works for all of them. +func newTestVAPIDOptions(t *testing.T, srv *httptest.Server) *webpush.Options { t.Helper() priv, pub, err := webpush.GenerateVAPIDKeys() require.NoError(t, err) + client := srv.Client() + client.Timeout = pushRequestTimeout + client.CheckRedirect = newPushHTTPClient().CheckRedirect return &webpush.Options{ VAPIDPublicKey: pub, VAPIDPrivateKey: priv, Subscriber: "mailto:test@example.com", TTL: 60, + HTTPClient: client, } } @@ -61,7 +80,7 @@ func TestDeliverNotification(t *testing.T) { notification := store.ScheduledNotification{ID: "n1", Title: "Hi", Body: "There"} t.Run("prunes auth-failed sub but keeps delivering to live ones", func(t *testing.T) { - app := newTestApplication(t) + app := newTestDispatcherApp(t) mockSubs := app.store.PushSubscriptions.(*store.MockPushSubscriptionsStore) live := newPushServer(t, http.StatusCreated) @@ -74,14 +93,14 @@ func TestDeliverNotification(t *testing.T) { mockSubs.On("ListByRole", mock.Anything).Return(subs, nil).Once() mockSubs.On("DeleteByEndpointAdmin", stale.URL).Return(nil).Once() - delivered := app.deliverNotification(context.Background(), notification, newTestVAPIDOptions(t)) + delivered := app.deliverNotification(context.Background(), notification, newTestVAPIDOptions(t, live)) assert.Equal(t, 1, delivered) mockSubs.AssertExpectations(t) }) t.Run("prunes both on mixed 410/403 with no delivery", func(t *testing.T) { - app := newTestApplication(t) + app := newTestDispatcherApp(t) mockSubs := app.store.PushSubscriptions.(*store.MockPushSubscriptionsStore) gone := newPushServer(t, http.StatusGone) @@ -95,14 +114,14 @@ func TestDeliverNotification(t *testing.T) { mockSubs.On("DeleteByEndpointAdmin", gone.URL).Return(nil).Once() mockSubs.On("DeleteByEndpointAdmin", forbidden.URL).Return(nil).Once() - delivered := app.deliverNotification(context.Background(), notification, newTestVAPIDOptions(t)) + delivered := app.deliverNotification(context.Background(), notification, newTestVAPIDOptions(t, gone)) assert.Equal(t, 0, delivered) mockSubs.AssertExpectations(t) }) t.Run("guard skips prune when every sub fails auth (suspected misconfig)", func(t *testing.T) { - app := newTestApplication(t) + app := newTestDispatcherApp(t) mockSubs := app.store.PushSubscriptions.(*store.MockPushSubscriptionsStore) s1 := newPushServer(t, http.StatusForbidden) @@ -114,7 +133,7 @@ func TestDeliverNotification(t *testing.T) { mockSubs.On("ListByRole", mock.Anything).Return(subs, nil).Once() - delivered := app.deliverNotification(context.Background(), notification, newTestVAPIDOptions(t)) + delivered := app.deliverNotification(context.Background(), notification, newTestVAPIDOptions(t, s1)) assert.Equal(t, 0, delivered) mockSubs.AssertNotCalled(t, "DeleteByEndpointAdmin", mock.Anything) @@ -122,7 +141,7 @@ func TestDeliverNotification(t *testing.T) { }) t.Run("still prunes 410 (regression)", func(t *testing.T) { - app := newTestApplication(t) + app := newTestDispatcherApp(t) mockSubs := app.store.PushSubscriptions.(*store.MockPushSubscriptionsStore) gone := newPushServer(t, http.StatusGone) @@ -131,9 +150,93 @@ func TestDeliverNotification(t *testing.T) { mockSubs.On("ListByRole", mock.Anything).Return(subs, nil).Once() mockSubs.On("DeleteByEndpointAdmin", gone.URL).Return(nil).Once() - delivered := app.deliverNotification(context.Background(), notification, newTestVAPIDOptions(t)) + delivered := app.deliverNotification(context.Background(), notification, newTestVAPIDOptions(t, gone)) assert.Equal(t, 0, delivered) mockSubs.AssertExpectations(t) }) + + t.Run("prunes disallowed endpoints without contacting them", func(t *testing.T) { + app := newTestDispatcherApp(t) + mockSubs := app.store.PushSubscriptions.(*store.MockPushSubscriptionsStore) + + var hits atomic.Int32 + rogue := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + hits.Add(1) + w.WriteHeader(http.StatusCreated) + })) + t.Cleanup(rogue.Close) + + live := newPushServer(t, http.StatusCreated) + subs := []store.PushSubscription{ + newTestPushSub(t, rogue.URL), // plain http, not allowed + newTestPushSub(t, "https://internal.example.com/push"), + newTestPushSub(t, live.URL), + } + + mockSubs.On("ListByRole", mock.Anything).Return(subs, nil).Once() + mockSubs.On("DeleteByEndpointAdmin", rogue.URL).Return(nil).Once() + mockSubs.On("DeleteByEndpointAdmin", "https://internal.example.com/push").Return(nil).Once() + + delivered := app.deliverNotification(context.Background(), notification, newTestVAPIDOptions(t, live)) + + assert.Equal(t, 1, delivered) + assert.Equal(t, int32(0), hits.Load()) + mockSubs.AssertExpectations(t) + }) + + t.Run("does not follow redirects", func(t *testing.T) { + app := newTestDispatcherApp(t) + mockSubs := app.store.PushSubscriptions.(*store.MockPushSubscriptionsStore) + + var hits atomic.Int32 + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + hits.Add(1) + w.WriteHeader(http.StatusCreated) + })) + t.Cleanup(target.Close) + + redirect := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, target.URL, http.StatusTemporaryRedirect) + })) + t.Cleanup(redirect.Close) + + subs := []store.PushSubscription{newTestPushSub(t, redirect.URL)} + mockSubs.On("ListByRole", mock.Anything).Return(subs, nil).Once() + + delivered := app.deliverNotification(context.Background(), notification, newTestVAPIDOptions(t, redirect)) + + assert.Equal(t, 0, delivered) + assert.Equal(t, int32(0), hits.Load()) + mockSubs.AssertNotCalled(t, "DeleteByEndpointAdmin", mock.Anything) + }) + + t.Run("times out on a hanging endpoint", func(t *testing.T) { + app := newTestDispatcherApp(t) + mockSubs := app.store.PushSubscriptions.(*store.MockPushSubscriptionsStore) + + release := make(chan struct{}) + hang := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + select { + case <-release: + case <-r.Context().Done(): + } + })) + t.Cleanup(func() { + close(release) + hang.Close() + }) + + subs := []store.PushSubscription{newTestPushSub(t, hang.URL)} + mockSubs.On("ListByRole", mock.Anything).Return(subs, nil).Once() + + options := newTestVAPIDOptions(t, hang) + options.HTTPClient.(*http.Client).Timeout = 200 * time.Millisecond + + start := time.Now() + delivered := app.deliverNotification(context.Background(), notification, options) + + assert.Equal(t, 0, delivered) + assert.Less(t, time.Since(start), 5*time.Second) + }) } diff --git a/cmd/api/main.go b/cmd/api/main.go index a695a965..b6fe2b99 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -107,9 +107,10 @@ func main() { googleClientSecret: env.GetString("GOOGLE_CLIENT_SECRET", ""), }, vapid: vapidConfig{ - publicKey: env.GetString("VAPID_PUBLIC_KEY", ""), - privateKey: env.GetString("VAPID_PRIVATE_KEY", ""), - subject: env.GetString("VAPID_SUBJECT", "noreply@example.com"), + publicKey: env.GetString("VAPID_PUBLIC_KEY", ""), + privateKey: env.GetString("VAPID_PRIVATE_KEY", ""), + subject: env.GetString("VAPID_SUBJECT", "noreply@example.com"), + allowedEndpointHosts: parsePushEndpointHosts(env.GetString("PUSH_ENDPOINT_ALLOWED_HOSTS", "")), }, appleWallet: appleWalletConfig{ enabled: env.GetBool("APPLE_WALLET_ENABLED", false), @@ -248,6 +249,7 @@ func main() { dispatcherCtx, cancelDispatcher := context.WithCancel(context.Background()) app.dispatcherCancel = cancelDispatcher + app.pushClient = newPushHTTPClient() go app.runNotificationDispatcher(dispatcherCtx) log.Fatal(app.run(mux)) diff --git a/cmd/api/notifications.go b/cmd/api/notifications.go index c8e14019..cf6edbc4 100644 --- a/cmd/api/notifications.go +++ b/cmd/api/notifications.go @@ -116,6 +116,11 @@ func (app *application) subscribePushHandler(w http.ResponseWriter, r *http.Requ return } + if err := validatePushEndpoint(payload.Endpoint, app.config.vapid.allowedEndpointHosts); err != nil { + app.badRequestResponse(w, r, err) + return + } + sub := &store.PushSubscription{ UserID: user.ID, Endpoint: payload.Endpoint, diff --git a/cmd/api/notifications_test.go b/cmd/api/notifications_test.go index 45cb230c..57c033f2 100644 --- a/cmd/api/notifications_test.go +++ b/cmd/api/notifications_test.go @@ -113,6 +113,54 @@ func TestSubscribePush(t *testing.T) { mockSubs.AssertExpectations(t) }) + t.Run("returns 400 for endpoints off the push-service allowlist", func(t *testing.T) { + for _, endpoint := range []string{ + "http://fcm.googleapis.com/fcm/send/abc", + "https://10.0.0.5/admin", + "https://169.254.169.254/computeMetadata/v1/", + "https://evil.example.com/fcm.googleapis.com", + "https://fcm.googleapis.com.evil.example.com/x", + "https://user@fcm.googleapis.com/x", + } { + app := newTestApplication(t) + app.config.vapid.publicKey = "test-public-key" + mockSubs := app.store.PushSubscriptions.(*store.MockPushSubscriptionsStore) + + body := `{"endpoint":"` + endpoint + `","p256dh":"key","auth":"auth-secret"}` + req, err := http.NewRequest(http.MethodPost, "/", strings.NewReader(body)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + req = setUserContext(req, newTestUser()) + + rr := executeRequest(req, http.HandlerFunc(app.subscribePushHandler)) + checkResponseCode(t, http.StatusBadRequest, rr.Code) + mockSubs.AssertNotCalled(t, "Upsert", mock.Anything) + } + }) + + t.Run("accepts subdomains of allowed hosts", func(t *testing.T) { + for _, endpoint := range []string{ + "https://updates.push.services.mozilla.com/wpush/v2/abc", + "https://web.push.apple.com/QAbc", + "https://wns2-par02p.notify.windows.com/w/?token=abc", + } { + app := newTestApplication(t) + app.config.vapid.publicKey = "test-public-key" + mockSubs := app.store.PushSubscriptions.(*store.MockPushSubscriptionsStore) + mockSubs.On("Upsert", mock.AnythingOfType("*store.PushSubscription")).Return(nil).Once() + + body := `{"endpoint":"` + endpoint + `","p256dh":"key","auth":"auth-secret"}` + req, err := http.NewRequest(http.MethodPost, "/", strings.NewReader(body)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + req = setUserContext(req, newTestUser()) + + rr := executeRequest(req, http.HandlerFunc(app.subscribePushHandler)) + checkResponseCode(t, http.StatusNoContent, rr.Code) + mockSubs.AssertExpectations(t) + } + }) + t.Run("returns 400 on missing endpoint", func(t *testing.T) { app := newTestApplication(t) app.config.vapid.publicKey = "test-public-key" @@ -141,6 +189,12 @@ func TestSubscribePush(t *testing.T) { }) } +func TestParsePushEndpointHosts(t *testing.T) { + assert.Equal(t, defaultPushEndpointHosts, parsePushEndpointHosts("")) + assert.Equal(t, defaultPushEndpointHosts, parsePushEndpointHosts(" , ")) + assert.Equal(t, []string{"push.example.com", "127.0.0.1"}, parsePushEndpointHosts(" Push.Example.com, 127.0.0.1 ,")) +} + func TestUnsubscribePush(t *testing.T) { t.Run("deletes the subscription on happy path", func(t *testing.T) { app := newTestApplication(t) diff --git a/cmd/api/push_endpoint.go b/cmd/api/push_endpoint.go new file mode 100644 index 00000000..97f75f12 --- /dev/null +++ b/cmd/api/push_endpoint.go @@ -0,0 +1,82 @@ +package main + +import ( + "errors" + "net/http" + "net/url" + "strings" + "time" +) + +// Push services used by the browsers hackers actually show up with. Each entry +// matches the host exactly or any subdomain of it. +var defaultPushEndpointHosts = []string{ + "fcm.googleapis.com", // Chrome, Edge, Brave, Opera, Vivaldi + "android.googleapis.com", // legacy Chrome + "push.services.mozilla.com", // Firefox + "push.apple.com", // Safari + "notify.windows.com", // Edge on Windows (WNS) + "push.samsungosp.com", // Samsung Internet +} + +const ( + pushRequestTimeout = 10 * time.Second + pushResponseBodyCap = 64 << 10 +) + +var errPushEndpointNotAllowed = errors.New("endpoint must be an https URL on a supported push service") + +// parsePushEndpointHosts turns a comma-separated env value into a host suffix +// list, falling back to the built-in browser push services when empty. +func parsePushEndpointHosts(raw string) []string { + if strings.TrimSpace(raw) == "" { + return defaultPushEndpointHosts + } + var hosts []string + for _, h := range strings.Split(raw, ",") { + h = strings.ToLower(strings.TrimSpace(h)) + if h != "" { + hosts = append(hosts, h) + } + } + if len(hosts) == 0 { + return defaultPushEndpointHosts + } + return hosts +} + +// validatePushEndpoint rejects anything that is not an https URL whose host is +// (a subdomain of) an allowed push service. The dispatcher POSTs to whatever is +// stored here, so this is what keeps a hacker from pointing the server at an +// internal address. +func validatePushEndpoint(raw string, allowedHosts []string) error { + u, err := url.Parse(raw) + if err != nil { + return errPushEndpointNotAllowed + } + if u.Scheme != "https" || u.User != nil { + return errPushEndpointNotAllowed + } + host := strings.ToLower(u.Hostname()) + if host == "" { + return errPushEndpointNotAllowed + } + for _, allowed := range allowedHosts { + if host == allowed || strings.HasSuffix(host, "."+allowed) { + return nil + } + } + return errPushEndpointNotAllowed +} + +// newPushHTTPClient returns the client the dispatcher uses to reach push +// services: hard timeout covering connect through body read, and no redirect +// following so a push endpoint cannot bounce us somewhere else. +func newPushHTTPClient() *http.Client { + return &http.Client{ + Timeout: pushRequestTimeout, + CheckRedirect: func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + }, + } +} diff --git a/cmd/api/test_utils_test.go b/cmd/api/test_utils_test.go index 1a31387b..bc76e142 100644 --- a/cmd/api/test_utils_test.go +++ b/cmd/api/test_utils_test.go @@ -80,6 +80,9 @@ func newTestApplication(t *testing.T) *application { TimeFrame: 5 * time.Second, Enabled: true, }, + vapid: vapidConfig{ + allowedEndpointHosts: defaultPushEndpointHosts, + }, }, store: mockStore, logger: logger, From dd215e69f45b9a34eddddf81b086d12a8b3145ad Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 09:39:42 -0500 Subject: [PATCH 04/11] fix(ratelimiter): atomic fixed-window counting and explicit client-IP trust (#155) Co-authored-by: Caleb Bae --- .env.example | 14 +++ claude.md | 4 +- cmd/api/api.go | 15 ++- cmd/api/applications.go | 2 +- cmd/api/applications_test.go | 2 +- cmd/api/faqs.go | 2 +- cmd/api/faqs_test.go | 2 +- cmd/api/hacker_links.go | 2 +- cmd/api/hacker_links_test.go | 2 +- cmd/api/main.go | 4 + cmd/api/middlewares.go | 34 ++++++- cmd/api/middlewares_test.go | 81 ++++++++++++++++ cmd/api/resume.go | 2 +- cmd/api/resume_test.go | 2 +- cmd/api/reviews.go | 2 +- cmd/api/reviews_test.go | 2 +- cmd/api/rsvp.go | 2 +- cmd/api/rsvp_test.go | 2 +- cmd/api/scans.go | 2 +- cmd/api/scans_test.go | 2 +- cmd/api/schedule.go | 2 +- cmd/api/schedule_test.go | 2 +- cmd/api/scheduled_notifications.go | 2 +- cmd/api/scheduled_notifications_test.go | 2 +- cmd/api/sponsors.go | 2 +- cmd/api/sponsors_test.go | 2 +- cmd/api/superadmin_users.go | 2 +- cmd/api/superadmin_users_test.go | 2 +- cmd/api/travelrsvp.go | 2 +- cmd/api/travelrsvp_test.go | 2 +- go.mod | 2 +- go.sum | 4 +- internal/ratelimiter/fixed-window.go | 64 ++++++++----- internal/ratelimiter/fixed-window_test.go | 110 ++++++++++++++++++++++ 34 files changed, 321 insertions(+), 59 deletions(-) create mode 100644 internal/ratelimiter/fixed-window_test.go diff --git a/.env.example b/.env.example index 314039d7..7492e52f 100644 --- a/.env.example +++ b/.env.example @@ -166,6 +166,20 @@ RATELIMITER_REQUESTS_COUNT=20 # at the venue typically shares one IP. RATELIMITER_IP_REQUESTS_COUNT=200 +# How the per-IP limiter learns the client address. Forwarded headers are +# never trusted by default because any client can send them. +# +# CLIENT_IP_HEADER: a single-IP header your edge proxy OVERWRITES on every +# request (Cloudflare: CF-Connecting-IP, nginx realip: X-Real-IP). Leave +# empty if no such proxy exists. +# CLIENT_IP_TRUSTED_PROXIES: used only when CLIENT_IP_HEADER is empty; the +# number of reverse proxies between the internet and this server; the +# X-Forwarded-For entry that many hops from the right is the client (one load +# balancer: 1). Verify with a request from a known IP. 0 means the TCP peer +# address is used as-is. +CLIENT_IP_HEADER=CF-Connecting-IP +CLIENT_IP_TRUSTED_PROXIES=0 + # ── Apple Wallet passes (optional) ─────────────────────────────────────────── diff --git a/claude.md b/claude.md index fd2e67ed..8419039f 100644 --- a/claude.md +++ b/claude.md @@ -55,8 +55,8 @@ Note: `air` runs `task gen-docs` as a pre-command on every rebuild, so `swag` CL - **Entry point:** `cmd/api/main.go` — loads config, `cmd/api/api.go` — Chi router setup in `mount()` - **Database:** PostgreSQL 16.3, raw SQL (no ORM), repository pattern in `internal/store/` - **Auth:** SuperTokens (Passwordless magic link + Google OAuth), initialized in `internal/auth/` -- **Middleware chain:** RequestID → RealIP → Logger → Recoverer → CORS → SuperTokens → RateLimiter (`/v1` only) → AuthRequired → RequireRole -- **Rate limiting:** keyed by SuperTokens user ID when the request carries a verified session (`RATELIMITER_REQUESTS_COUNT`), falling back to client IP otherwise (`RATELIMITER_IP_REQUESTS_COUNT`, larger because a whole venue shares one NAT). Static assets and `/auth/*` are never limited. +- **Middleware chain:** RequestID → ClientIP → Logger → Recoverer → CORS → SuperTokens → RateLimiter (`/v1` only) → AuthRequired → RequireRole +- **Rate limiting:** keyed by SuperTokens user ID when the request carries a verified session (`RATELIMITER_REQUESTS_COUNT`), falling back to client IP otherwise (`RATELIMITER_IP_REQUESTS_COUNT`, larger because a whole venue shares one NAT). The client IP comes from `CLIENT_IP_HEADER` (default `CF-Connecting-IP`) or `CLIENT_IP_TRUSTED_PROXIES` hops into `X-Forwarded-For`; other forwarded headers are ignored. Static assets and `/auth/*` are never limited. - **Roles (hierarchical):** `hacker` (1) < `admin` (2) < `super_admin` (3) - **JSON envelope:** Success: `{"data": ...}`, Error: `{"error": "..."}` - **Pagination:** Cursor-based with base64-encoded JSON cursors diff --git a/cmd/api/api.go b/cmd/api/api.go index 443822d7..356e7afe 100644 --- a/cmd/api/api.go +++ b/cmd/api/api.go @@ -10,8 +10,8 @@ import ( "syscall" "time" - "github.com/go-chi/chi" - "github.com/go-chi/chi/middleware" + "github.com/go-chi/chi/v5" + "github.com/go-chi/chi/v5/middleware" "github.com/go-chi/cors" "github.com/hackutd/harp/internal/gcs" "github.com/hackutd/harp/internal/mailer" @@ -52,12 +52,21 @@ type config struct { gcs gcsConfig auth authConfig rateLimiter ratelimiter.Config + clientIP clientIPConfig supertokens supertokensConfig publicCORSOrigin string vapid vapidConfig appleWallet appleWalletConfig } +// clientIPConfig selects the trusted source of the client address used for +// per-IP rate limiting. header wins when set; otherwise trustedProxies > 0 +// reads X-Forwarded-For; otherwise the TCP peer address is used. +type clientIPConfig struct { + header string + trustedProxies int +} + type vapidConfig struct { publicKey string privateKey string @@ -124,7 +133,7 @@ func (app *application) mount() http.Handler { r := chi.NewRouter() r.Use(middleware.RequestID) - r.Use(middleware.RealIP) + r.Use(app.clientIPMiddleware()) r.Use(middleware.Logger) r.Use(middleware.Recoverer) diff --git a/cmd/api/applications.go b/cmd/api/applications.go index 15b595c5..fe9d3e27 100644 --- a/cmd/api/applications.go +++ b/cmd/api/applications.go @@ -8,7 +8,7 @@ import ( "strconv" "strings" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" ) diff --git a/cmd/api/applications_test.go b/cmd/api/applications_test.go index 20ef80ad..03783389 100644 --- a/cmd/api/applications_test.go +++ b/cmd/api/applications_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" diff --git a/cmd/api/faqs.go b/cmd/api/faqs.go index 924f7822..ec4cb82e 100644 --- a/cmd/api/faqs.go +++ b/cmd/api/faqs.go @@ -4,7 +4,7 @@ import ( "errors" "net/http" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" ) diff --git a/cmd/api/faqs_test.go b/cmd/api/faqs_test.go index d25aebfb..8138c907 100644 --- a/cmd/api/faqs_test.go +++ b/cmd/api/faqs_test.go @@ -8,7 +8,7 @@ import ( "testing" "time" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" diff --git a/cmd/api/hacker_links.go b/cmd/api/hacker_links.go index 7f4b5596..3d06945a 100644 --- a/cmd/api/hacker_links.go +++ b/cmd/api/hacker_links.go @@ -4,7 +4,7 @@ import ( "errors" "net/http" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" ) diff --git a/cmd/api/hacker_links_test.go b/cmd/api/hacker_links_test.go index 3058877e..05a51a25 100644 --- a/cmd/api/hacker_links_test.go +++ b/cmd/api/hacker_links_test.go @@ -8,7 +8,7 @@ import ( "testing" "time" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" diff --git a/cmd/api/main.go b/cmd/api/main.go index b6fe2b99..c78cdfcd 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -97,6 +97,10 @@ func main() { TimeFrame: time.Second * 5, Enabled: env.GetBool("RATE_LIMITER_ENABLED", true), }, + clientIP: clientIPConfig{ + header: env.GetString("CLIENT_IP_HEADER", "CF-Connecting-IP"), + trustedProxies: env.GetInt("CLIENT_IP_TRUSTED_PROXIES", 0), + }, frontendURL: frontendURL, publicCORSOrigin: env.GetString("PUBLIC_CORS_ORIGIN", ""), supertokens: supertokensConfig{ diff --git a/cmd/api/middlewares.go b/cmd/api/middlewares.go index ec0b3a17..81bd2cd8 100644 --- a/cmd/api/middlewares.go +++ b/cmd/api/middlewares.go @@ -6,10 +6,13 @@ import ( "encoding/base64" "errors" "fmt" + "math" "net" "net/http" + "strconv" "strings" + "github.com/go-chi/chi/v5/middleware" "github.com/hackutd/harp/internal/auth" "github.com/hackutd/harp/internal/ratelimiter" "github.com/hackutd/harp/internal/store" @@ -80,7 +83,11 @@ func (app *application) RateLimiterMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { limiter, key := app.rateLimiterFor(w, r) if allow, retryAfter := limiter.Allow(key); !allow { - app.rateLimiterExceededResponse(w, r, key, retryAfter.String()) + seconds := int(math.Ceil(retryAfter.Seconds())) + if seconds < 1 { + seconds = 1 + } + app.rateLimiterExceededResponse(w, r, key, strconv.Itoa(seconds)) return } next.ServeHTTP(w, r) @@ -98,10 +105,29 @@ func (app *application) rateLimiterFor(w http.ResponseWriter, r *http.Request) ( return app.ipRateLimiter, "ip:" + clientIP(r) } -// middleware.RealIP rewrites RemoteAddr to the bare forwarded IP behind a -// proxy, but without one RemoteAddr keeps its port, which would make every -// connection its own bucket. +// clientIPMiddleware picks how the client address is derived, per config: +// a single-IP header the edge proxy overwrites on every request (Cloudflare's +// CF-Connecting-IP), the X-Forwarded-For entry a known number of proxies deep, +// or the TCP peer when nothing sits in front of the server. Forwarded headers +// are never trusted implicitly, since a client can set them freely. +func (app *application) clientIPMiddleware() func(http.Handler) http.Handler { + switch { + case app.config.clientIP.header != "": + return middleware.ClientIPFromHeader(app.config.clientIP.header) + case app.config.clientIP.trustedProxies > 0: + return middleware.ClientIPFromXFFTrustedProxies(app.config.clientIP.trustedProxies) + default: + return middleware.ClientIPFromRemoteAddr + } +} + +// clientIP prefers the address resolved by the configured ClientIPFrom* +// middleware (see clientIPMiddleware); when that yields nothing the TCP peer +// is used, minus its ephemeral port so one host is one bucket. func clientIP(r *http.Request) string { + if ip := middleware.GetClientIP(r.Context()); ip != "" { + return ip + } if host, _, err := net.SplitHostPort(r.RemoteAddr); err == nil { return host } diff --git a/cmd/api/middlewares_test.go b/cmd/api/middlewares_test.go index a1590653..75f997a5 100644 --- a/cmd/api/middlewares_test.go +++ b/cmd/api/middlewares_test.go @@ -353,6 +353,87 @@ func TestRateLimiterMiddleware(t *testing.T) { assert.NotEqual(t, http.StatusTooManyRequests, get(path).Code, "expected %s to bypass the rate limiter", path) } }) + + t.Run("should ignore forwarded headers the client can forge", func(t *testing.T) { + app := newTestApplication(t) + app.ipRateLimiter = ratelimiter.NewFixedWindowLimiter(1, 5*time.Second) + mux := app.mount() + + get := func(headers map[string]string) *httptest.ResponseRecorder { + req, err := http.NewRequest(http.MethodGet, "/v1/health", nil) + require.NoError(t, err) + req.RemoteAddr = "10.0.0.10:1234" + for k, v := range headers { + req.Header.Set(k, v) + } + return executeRequest(req, mux) + } + + checkResponseCode(t, http.StatusUnauthorized, get(nil).Code) + + // a fresh spoofed address per request must not mint a fresh bucket + for _, h := range []map[string]string{ + {"X-Real-IP": "203.0.113.1"}, + {"X-Forwarded-For": "203.0.113.2"}, + {"True-Client-IP": "203.0.113.3"}, + {"CF-Connecting-IP": "203.0.113.4"}, + } { + checkResponseCode(t, http.StatusTooManyRequests, get(h).Code) + } + }) + + t.Run("should key by the configured edge header when present", func(t *testing.T) { + app := newTestApplication(t) + app.config.clientIP.header = "CF-Connecting-IP" + app.ipRateLimiter = ratelimiter.NewFixedWindowLimiter(1, 5*time.Second) + mux := app.mount() + + get := func(clientIP string) *httptest.ResponseRecorder { + req, err := http.NewRequest(http.MethodGet, "/v1/health", nil) + require.NoError(t, err) + req.RemoteAddr = "10.0.0.11:1234" // the proxy; identical for every client + req.Header.Set("CF-Connecting-IP", clientIP) + req.Header.Set("X-Forwarded-For", "203.0.113.99") // must be ignored + return executeRequest(req, mux) + } + + checkResponseCode(t, http.StatusUnauthorized, get("198.51.100.1").Code) + checkResponseCode(t, http.StatusTooManyRequests, get("198.51.100.1").Code) + checkResponseCode(t, http.StatusUnauthorized, get("198.51.100.2").Code) + }) + + t.Run("should key by X-Forwarded-For only past the trusted proxy count", func(t *testing.T) { + app := newTestApplication(t) + app.config.clientIP.trustedProxies = 1 + app.ipRateLimiter = ratelimiter.NewFixedWindowLimiter(1, 5*time.Second) + mux := app.mount() + + get := func(xff string) *httptest.ResponseRecorder { + req, err := http.NewRequest(http.MethodGet, "/v1/health", nil) + require.NoError(t, err) + req.RemoteAddr = "10.0.0.12:1234" + req.Header.Set("X-Forwarded-For", xff) + return executeRequest(req, mux) + } + + // one proxy appends the real client as the rightmost entry; anything + // the client prepended on the left is ignored + checkResponseCode(t, http.StatusUnauthorized, get("198.51.100.1").Code) + checkResponseCode(t, http.StatusTooManyRequests, get("203.0.113.5, 198.51.100.1").Code) + checkResponseCode(t, http.StatusTooManyRequests, get("203.0.113.6, 198.51.100.1").Code) + checkResponseCode(t, http.StatusUnauthorized, get("198.51.100.2").Code) + }) + + t.Run("should send Retry-After in whole seconds", func(t *testing.T) { + app := newTestApplication(t) + app.ipRateLimiter = ratelimiter.NewFixedWindowLimiter(1, 5*time.Second) + handler := app.RateLimiterMiddleware(ok) + + executeRequest(newRequest(t, "10.0.0.13:1234", ""), handler) + rr := executeRequest(newRequest(t, "10.0.0.13:1234", ""), handler) + checkResponseCode(t, http.StatusTooManyRequests, rr.Code) + assert.Regexp(t, `^[1-5]$`, rr.Header().Get("Retry-After")) + }) } func TestApplicationsEnabledMiddleware(t *testing.T) { diff --git a/cmd/api/resume.go b/cmd/api/resume.go index 72c28dc9..e4ca19dd 100644 --- a/cmd/api/resume.go +++ b/cmd/api/resume.go @@ -8,7 +8,7 @@ import ( "net/http" "strings" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/slug" "github.com/hackutd/harp/internal/store" ) diff --git a/cmd/api/resume_test.go b/cmd/api/resume_test.go index 9def2252..8e6b9121 100644 --- a/cmd/api/resume_test.go +++ b/cmd/api/resume_test.go @@ -8,7 +8,7 @@ import ( "strings" "testing" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/gcs" "github.com/hackutd/harp/internal/store" "github.com/stretchr/testify/assert" diff --git a/cmd/api/reviews.go b/cmd/api/reviews.go index d042b16d..5f8d9430 100644 --- a/cmd/api/reviews.go +++ b/cmd/api/reviews.go @@ -4,7 +4,7 @@ import ( "errors" "net/http" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" ) diff --git a/cmd/api/reviews_test.go b/cmd/api/reviews_test.go index d3a2ac62..0d351f60 100644 --- a/cmd/api/reviews_test.go +++ b/cmd/api/reviews_test.go @@ -8,7 +8,7 @@ import ( "testing" "time" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/cmd/api/rsvp.go b/cmd/api/rsvp.go index ad614e43..4ea24234 100644 --- a/cmd/api/rsvp.go +++ b/cmd/api/rsvp.go @@ -7,7 +7,7 @@ import ( "net/http" "time" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" ) diff --git a/cmd/api/rsvp_test.go b/cmd/api/rsvp_test.go index 4e73b6b1..84002fcc 100644 --- a/cmd/api/rsvp_test.go +++ b/cmd/api/rsvp_test.go @@ -8,7 +8,7 @@ import ( "testing" "time" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/gcs" "github.com/hackutd/harp/internal/store" "github.com/stretchr/testify/assert" diff --git a/cmd/api/scans.go b/cmd/api/scans.go index 93c0ad05..72d4b2fc 100644 --- a/cmd/api/scans.go +++ b/cmd/api/scans.go @@ -7,7 +7,7 @@ import ( "math/rand" "net/http" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" ) diff --git a/cmd/api/scans_test.go b/cmd/api/scans_test.go index 2829ab48..54975fec 100644 --- a/cmd/api/scans_test.go +++ b/cmd/api/scans_test.go @@ -8,7 +8,7 @@ import ( "testing" "time" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/mailer" "github.com/hackutd/harp/internal/store" "github.com/stretchr/testify/assert" diff --git a/cmd/api/schedule.go b/cmd/api/schedule.go index 4fa8511b..79c979de 100644 --- a/cmd/api/schedule.go +++ b/cmd/api/schedule.go @@ -6,7 +6,7 @@ import ( "net/http" "time" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" ) diff --git a/cmd/api/schedule_test.go b/cmd/api/schedule_test.go index 6c286bcd..523f686b 100644 --- a/cmd/api/schedule_test.go +++ b/cmd/api/schedule_test.go @@ -7,7 +7,7 @@ import ( "testing" "time" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" diff --git a/cmd/api/scheduled_notifications.go b/cmd/api/scheduled_notifications.go index f98fb409..82b677c3 100644 --- a/cmd/api/scheduled_notifications.go +++ b/cmd/api/scheduled_notifications.go @@ -7,7 +7,7 @@ import ( "strings" "time" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" ) diff --git a/cmd/api/scheduled_notifications_test.go b/cmd/api/scheduled_notifications_test.go index 85bbbee1..74ccb4bc 100644 --- a/cmd/api/scheduled_notifications_test.go +++ b/cmd/api/scheduled_notifications_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" diff --git a/cmd/api/sponsors.go b/cmd/api/sponsors.go index 8896fc19..2db55733 100644 --- a/cmd/api/sponsors.go +++ b/cmd/api/sponsors.go @@ -6,7 +6,7 @@ import ( "fmt" "net/http" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" ) diff --git a/cmd/api/sponsors_test.go b/cmd/api/sponsors_test.go index 8eae2c26..8793b611 100644 --- a/cmd/api/sponsors_test.go +++ b/cmd/api/sponsors_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" diff --git a/cmd/api/superadmin_users.go b/cmd/api/superadmin_users.go index d2479be0..ccd50cb0 100644 --- a/cmd/api/superadmin_users.go +++ b/cmd/api/superadmin_users.go @@ -5,7 +5,7 @@ import ( "net/http" "strconv" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" ) diff --git a/cmd/api/superadmin_users_test.go b/cmd/api/superadmin_users_test.go index f61e7efd..62772a83 100644 --- a/cmd/api/superadmin_users_test.go +++ b/cmd/api/superadmin_users_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/store" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/cmd/api/travelrsvp.go b/cmd/api/travelrsvp.go index 78dfbfc7..2873bb84 100644 --- a/cmd/api/travelrsvp.go +++ b/cmd/api/travelrsvp.go @@ -10,7 +10,7 @@ import ( "strings" "time" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/slug" "github.com/hackutd/harp/internal/store" ) diff --git a/cmd/api/travelrsvp_test.go b/cmd/api/travelrsvp_test.go index feca1999..e2aa329c 100644 --- a/cmd/api/travelrsvp_test.go +++ b/cmd/api/travelrsvp_test.go @@ -8,7 +8,7 @@ import ( "testing" "time" - "github.com/go-chi/chi" + "github.com/go-chi/chi/v5" "github.com/hackutd/harp/internal/gcs" "github.com/hackutd/harp/internal/store" "github.com/stretchr/testify/assert" diff --git a/go.mod b/go.mod index e4eb8825..50b7e0b4 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.27.0 require ( cloud.google.com/go/storage v1.60.0 github.com/SherClockHolmes/webpush-go v1.4.0 - github.com/go-chi/chi v1.5.5 + github.com/go-chi/chi/v5 v5.3.2 github.com/go-chi/cors v1.2.2 github.com/go-playground/validator/v10 v10.30.1 github.com/jackc/pgx/v5 v5.9.2 diff --git a/go.sum b/go.sum index e600be3e..6a74e848 100644 --- a/go.sum +++ b/go.sum @@ -57,8 +57,8 @@ github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2 github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/gabriel-vasile/mimetype v1.4.12 h1:e9hWvmLYvtp846tLHam2o++qitpguFiYCKbn0w9jyqw= github.com/gabriel-vasile/mimetype v1.4.12/go.mod h1:d+9Oxyo1wTzWdyVUPMmXFvp4F9tea18J8ufA774AB3s= -github.com/go-chi/chi v1.5.5 h1:vOB/HbEMt9QqBqErz07QehcOKHaWFtuj87tTDVz2qXE= -github.com/go-chi/chi v1.5.5/go.mod h1:C9JqLr3tIYjDOZpzn+BCuxY8z8vmca43EeMgyZt7irw= +github.com/go-chi/chi/v5 v5.3.2 h1:5YQkICvTCSZ25hoRsyJazN0scjzKGiu4VAUc7H1o1nY= +github.com/go-chi/chi/v5 v5.3.2/go.mod h1:R+tYY2hNuVUUjxoPtqUdgBqevM9s9njzkTLutVsOCto= github.com/go-chi/cors v1.2.2 h1:Jmey33TE+b+rB7fT8MUy1u0I4L+NARQlK6LhzKPSyQE= github.com/go-chi/cors v1.2.2/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58= github.com/go-jose/go-jose/v4 v4.1.4 h1:moDMcTHmvE6Groj34emNPLs/qtYXRVcd6S7NHbHz3kA= diff --git a/internal/ratelimiter/fixed-window.go b/internal/ratelimiter/fixed-window.go index 5c9cd3d4..cc7bfe48 100644 --- a/internal/ratelimiter/fixed-window.go +++ b/internal/ratelimiter/fixed-window.go @@ -5,44 +5,62 @@ import ( "time" ) +type bucket struct { + count int + resetAt time.Time +} + +// FixedWindowLimiter allows up to limit requests per key in each window. +// Windows are reset lazily on the next request for the key, and expired keys +// are swept from the map at most once per window, so no per-key goroutines +// are spawned. type FixedWindowLimiter struct { - sync.RWMutex - clients map[string]int - limit int - window time.Duration + mu sync.Mutex + clients map[string]*bucket + limit int + window time.Duration + nextSweep time.Time + now func() time.Time } func NewFixedWindowLimiter(limit int, window time.Duration) *FixedWindowLimiter { return &FixedWindowLimiter{ - clients: make(map[string]int), + clients: make(map[string]*bucket), limit: limit, window: window, + now: time.Now, } } func (rl *FixedWindowLimiter) Allow(key string) (bool, time.Duration) { - rl.RLock() - count, exists := rl.clients[key] - rl.RUnlock() - - if !exists || count < rl.limit { - rl.Lock() - if !exists { - go rl.resetCount(key) - } + now := rl.now() + + rl.mu.Lock() + defer rl.mu.Unlock() - rl.clients[key]++ - rl.Unlock() + if !now.Before(rl.nextSweep) { + rl.sweep(now) + } - return true, 0 + w, ok := rl.clients[key] + if !ok || !now.Before(w.resetAt) { + w = &bucket{resetAt: now.Add(rl.window)} + rl.clients[key] = w } - return false, rl.window + if w.count >= rl.limit { + return false, w.resetAt.Sub(now) + } + w.count++ + return true, 0 } -func (rl *FixedWindowLimiter) resetCount(key string) { - time.Sleep(rl.window) - rl.Lock() - delete(rl.clients, key) - rl.Unlock() +// sweep drops every expired bucket. Caller must hold mu. +func (rl *FixedWindowLimiter) sweep(now time.Time) { + for key, w := range rl.clients { + if !now.Before(w.resetAt) { + delete(rl.clients, key) + } + } + rl.nextSweep = now.Add(rl.window) } diff --git a/internal/ratelimiter/fixed-window_test.go b/internal/ratelimiter/fixed-window_test.go new file mode 100644 index 00000000..ee094f9a --- /dev/null +++ b/internal/ratelimiter/fixed-window_test.go @@ -0,0 +1,110 @@ +package ratelimiter + +import ( + "sync" + "sync/atomic" + "testing" + "time" +) + +func newTestLimiter(limit int, window time.Duration) (*FixedWindowLimiter, *time.Time) { + now := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + rl := NewFixedWindowLimiter(limit, window) + rl.now = func() time.Time { return now } + return rl, &now +} + +func TestFixedWindowLimiter_AllowsUpToLimit(t *testing.T) { + rl, _ := newTestLimiter(3, time.Second) + + for i := 0; i < 3; i++ { + if ok, _ := rl.Allow("k"); !ok { + t.Fatalf("request %d should be allowed", i+1) + } + } + if ok, _ := rl.Allow("k"); ok { + t.Fatal("request over the limit should be denied") + } + if ok, _ := rl.Allow("other"); !ok { + t.Fatal("a different key must have its own budget") + } +} + +func TestFixedWindowLimiter_RetryAfterIsTimeLeftInWindow(t *testing.T) { + rl, now := newTestLimiter(1, 5*time.Second) + + rl.Allow("k") + *now = now.Add(2 * time.Second) + + ok, retry := rl.Allow("k") + if ok { + t.Fatal("expected denial") + } + if retry != 3*time.Second { + t.Fatalf("retry-after = %v, want 3s", retry) + } +} + +func TestFixedWindowLimiter_ResetsAfterWindow(t *testing.T) { + rl, now := newTestLimiter(1, time.Second) + + rl.Allow("k") + if ok, _ := rl.Allow("k"); ok { + t.Fatal("expected denial inside the window") + } + + *now = now.Add(time.Second) + if ok, _ := rl.Allow("k"); !ok { + t.Fatal("expected a fresh budget once the window elapsed") + } +} + +func TestFixedWindowLimiter_SweepsExpiredKeys(t *testing.T) { + rl, now := newTestLimiter(1, time.Second) + + for _, k := range []string{"a", "b", "c"} { + rl.Allow(k) + } + if got := len(rl.clients); got != 3 { + t.Fatalf("len(clients) = %d, want 3", got) + } + + *now = now.Add(time.Second) + rl.Allow("d") + + rl.mu.Lock() + defer rl.mu.Unlock() + if got := len(rl.clients); got != 1 { + t.Fatalf("len(clients) = %d after sweep, want 1 (only the live key)", got) + } + if _, ok := rl.clients["d"]; !ok { + t.Fatal("live key must survive the sweep") + } +} + +func TestFixedWindowLimiter_ConcurrentRequestsNeverExceedLimit(t *testing.T) { + const limit, workers, perWorker = 50, 32, 100 + rl := NewFixedWindowLimiter(limit, time.Minute) + + var allowed atomic.Int64 + var wg sync.WaitGroup + start := make(chan struct{}) + for i := 0; i < workers; i++ { + wg.Add(1) + go func() { + defer wg.Done() + <-start + for j := 0; j < perWorker; j++ { + if ok, _ := rl.Allow("shared"); ok { + allowed.Add(1) + } + } + }() + } + close(start) + wg.Wait() + + if got := allowed.Load(); got != limit { + t.Fatalf("allowed %d requests, want exactly %d", got, limit) + } +} From 72533acfa7073884e53c9dfbaf3271e37abff1b9 Mon Sep 17 00:00:00 2001 From: Caleb Bae <144546374+balebbae@users.noreply.github.com> Date: Tue, 8 Sep 2026 07:41:33 -0700 Subject: [PATCH 05/11] fix: error messaging on applications (#157) --- .../components/ApplicationsTable.tsx | 2 +- .../src/pages/admin/all-applicants/utils.ts | 15 +- .../src/pages/admin/reviews/ReviewsPage.tsx | 7 +- .../admin/reviews/components/ReviewsTable.tsx | 6 +- .../admin/reviews/grading/GradingPage.tsx | 6 +- .../apply/components/ApplicationWizard.tsx | 38 ++++- .../pages/hacker/apply/steps/ReviewStep.tsx | 3 +- .../hacker/apply/steps/SchemaStepRenderer.tsx | 69 +++++---- .../forms/components/ResponseDetailSheet.tsx | 2 +- .../forms/components/ResponsesTable.tsx | 2 +- .../reviews/components/ReviewsTable.tsx | 2 +- .../reviews/grading/GradingPage.tsx | 6 +- client/portal/src/shared/lib/api.ts | 10 ++ client/portal/src/shared/lib/schema-utils.ts | 141 ++++++++++++++---- client/portal/src/types.ts | 6 + cmd/api/applications.go | 89 ++++++++--- cmd/api/applications_test.go | 133 +++++++++++++++++ cmd/api/errors.go | 12 ++ cmd/api/json.go | 12 ++ cmd/api/rsvp.go | 3 +- cmd/api/travelrsvp.go | 2 +- docs/docs.go | 8 +- internal/store/integration_test.go | 43 ++++++ internal/store/reviews.go | 20 ++- 24 files changed, 536 insertions(+), 101 deletions(-) diff --git a/client/portal/src/pages/admin/all-applicants/components/ApplicationsTable.tsx b/client/portal/src/pages/admin/all-applicants/components/ApplicationsTable.tsx index 293685f3..d8e2fbf2 100644 --- a/client/portal/src/pages/admin/all-applicants/components/ApplicationsTable.tsx +++ b/client/portal/src/pages/admin/all-applicants/components/ApplicationsTable.tsx @@ -73,7 +73,7 @@ export const ApplicationsTable = memo(function ApplicationsTable({ applications.map((app) => { const name = redact ? formatApplicantLabel(app.id) - : formatName(app.first_name, app.last_name); + : formatName(app.first_name, app.last_name, app.email); const email = redact ? maskEmail(app.email) : app.email; const isSelected = selectedId === app.id; diff --git a/client/portal/src/pages/admin/all-applicants/utils.ts b/client/portal/src/pages/admin/all-applicants/utils.ts index 01ad933e..9b1295eb 100644 --- a/client/portal/src/pages/admin/all-applicants/utils.ts +++ b/client/portal/src/pages/admin/all-applicants/utils.ts @@ -21,10 +21,21 @@ export function getStatusColor(status: string): string { } } +/** + * Display name for an applicant, falling back to their email. + * + * Walk-ins get an application row with empty responses (see WalkInsStore), so + * first_name/last_name are null for them forever and there is no name to + * recover — without the fallback those rows read as "-" in every admin view. + * Pass the email only from non-redacted branches; redacted views use + * formatApplicantLabel/maskEmail instead. + */ export function formatName( firstName: string | null, lastName: string | null, + fallbackEmail?: string | null, ): string { - if (!firstName && !lastName) return "-"; - return `${firstName ?? ""} ${lastName ?? ""}`.trim(); + const name = `${firstName ?? ""} ${lastName ?? ""}`.trim(); + if (name) return name; + return fallbackEmail || "-"; } diff --git a/client/portal/src/pages/admin/reviews/ReviewsPage.tsx b/client/portal/src/pages/admin/reviews/ReviewsPage.tsx index fc39eeed..a5d21d4c 100644 --- a/client/portal/src/pages/admin/reviews/ReviewsPage.tsx +++ b/client/portal/src/pages/admin/reviews/ReviewsPage.tsx @@ -240,7 +240,11 @@ export default function ReviewsPage() { Grade{" "} {redact ? formatApplicantLabel(reviews[0].application_id) - : formatName(reviews[0].first_name, reviews[0].last_name)} + : formatName( + reviews[0].first_name, + reviews[0].last_name, + reviews[0].email, + )} ) : undefined; @@ -288,6 +292,7 @@ export default function ReviewsPage() { : formatName( selectedReview.first_name, selectedReview.last_name, + selectedReview.email, ) : "Review"} diff --git a/client/portal/src/pages/admin/reviews/components/ReviewsTable.tsx b/client/portal/src/pages/admin/reviews/components/ReviewsTable.tsx index a200a140..539f4682 100644 --- a/client/portal/src/pages/admin/reviews/components/ReviewsTable.tsx +++ b/client/portal/src/pages/admin/reviews/components/ReviewsTable.tsx @@ -93,7 +93,11 @@ export const ReviewsTable = memo(function ReviewsTable({ {redact ? formatApplicantLabel(review.application_id) - : formatName(review.first_name, review.last_name)} + : formatName( + review.first_name, + review.last_name, + review.email, + )} {redact ? maskEmail(review.email) : review.email} diff --git a/client/portal/src/pages/admin/reviews/grading/GradingPage.tsx b/client/portal/src/pages/admin/reviews/grading/GradingPage.tsx index a5c9d076..4c4f2e0b 100644 --- a/client/portal/src/pages/admin/reviews/grading/GradingPage.tsx +++ b/client/portal/src/pages/admin/reviews/grading/GradingPage.tsx @@ -106,7 +106,11 @@ export default function GradingPage() {

{redact ? formatApplicantLabel(currentReview.application_id) - : formatName(currentReview.first_name, currentReview.last_name)} + : formatName( + currentReview.first_name, + currentReview.last_name, + currentReview.email, + )}

diff --git a/client/portal/src/pages/hacker/apply/components/ApplicationWizard.tsx b/client/portal/src/pages/hacker/apply/components/ApplicationWizard.tsx index 88d5fadf..01109988 100644 --- a/client/portal/src/pages/hacker/apply/components/ApplicationWizard.tsx +++ b/client/portal/src/pages/hacker/apply/components/ApplicationWizard.tsx @@ -19,6 +19,7 @@ import { deriveSections, groupFieldsBySection, resolveResumeSectionId, + stripLabelLinks, } from "@/shared/lib/schema-utils"; import type { Application, ApplicationSchemaField } from "@/types"; @@ -420,10 +421,41 @@ export function ApplicationWizard({ userEmail }: ApplicationWizardProps) { navigate("/app", { state: { justSubmitted: submitRes.data.id }, }); - } else { - setApiError(submitRes.error || "Failed to submit application"); - errorAlert(submitRes); + setSubmitting(false); + return; + } + + // The server re-validates against the live schema, so it can still reject a + // form the client considered complete — e.g. a question a super admin made + // required after this page loaded. Blame the fields it named so the hacker + // gets the same "which section, which question" summary as a client-side + // failure, instead of a raw message naming a field id. + const fieldsById = new Map(schemaFields.map((f) => [f.id, f])); + const blamed = (submitRes.fields ?? []).filter((id) => fieldsById.has(id)); + if (blamed.length > 0) { + for (const id of blamed) { + // The server reports which questions it rejected, not why in + // hacker-readable terms (its own messages name field ids), so keep the + // wording broad enough to cover missing and invalid alike. + form.setError(id, { + type: "server", + message: `${stripLabelLinks(fieldsById.get(id)!.label)} needs a valid answer`, + }); + } + setShowIncomplete(true); + setSubmitting(false); + window.scrollTo({ top: 0, behavior: "smooth" }); + return; } + + // A 400 here is always schema validation; anything the client can't map to + // a question is still not worth showing verbatim. + const message = + submitRes.status === 400 + ? "Some answers are missing or invalid. Please review your application and try again." + : submitRes.error || "Failed to submit application"; + setApiError(message); + errorAlert(submitRes, message); setSubmitting(false); }; diff --git a/client/portal/src/pages/hacker/apply/steps/ReviewStep.tsx b/client/portal/src/pages/hacker/apply/steps/ReviewStep.tsx index 3d1b235a..26c93b7d 100644 --- a/client/portal/src/pages/hacker/apply/steps/ReviewStep.tsx +++ b/client/portal/src/pages/hacker/apply/steps/ReviewStep.tsx @@ -114,7 +114,8 @@ export function ReviewStep({ Review

- Check your answers before submitting + Check your answers before submitting — once you submit, your + application can no longer be edited.

diff --git a/client/portal/src/pages/hacker/apply/steps/SchemaStepRenderer.tsx b/client/portal/src/pages/hacker/apply/steps/SchemaStepRenderer.tsx index c3b1f3a0..3786584e 100644 --- a/client/portal/src/pages/hacker/apply/steps/SchemaStepRenderer.tsx +++ b/client/portal/src/pages/hacker/apply/steps/SchemaStepRenderer.tsx @@ -37,6 +37,7 @@ import { getFieldPresets } from "@/shared/lib/field-presets"; import { conditionSatisfied, getFieldCondition, + getWholeNumberRule, renderLabel, } from "@/shared/lib/schema-utils"; import { cn } from "@/shared/lib/utils"; @@ -244,7 +245,10 @@ function SchemaFormField({ /> ); - case "number": + case "number": { + // Well-known count fields (age) take whole numbers only, so the decimal + // point never makes it into the value. + const whole = getWholeNumberRule(field.id); return ( e.target.select()} onMouseUp={(e) => e.preventDefault()} onChange={(e) => { - const cleaned = e.target.value.replace(/[^\d.-]/g, ""); + const cleaned = e.target.value.replace( + whole ? /[^\d-]/g : /[^\d.-]/g, + "", + ); + // Empty stays undefined rather than collapsing to 0, so a + // required number the hacker never answered still fails. if (cleaned === "" || cleaned === "-") { - formField.onChange(0); + formField.onChange(undefined); return; } const num = Number(cleaned); - formField.onChange(Number.isNaN(num) ? 0 : num); + formField.onChange(Number.isNaN(num) ? undefined : num); }} /> @@ -282,6 +292,7 @@ function SchemaFormField({ )} /> ); + } case "textarea": return ( @@ -338,11 +349,14 @@ function SchemaFormField({ name={field.id} render={() => ( - {field.label} + + {field.label} + {requiredMark} + Select all that apply -
+
{(field.options ?? []).map((opt) => ( { const value = (formField.value as string[]) || []; return ( - - - { - if (checked) { - formField.onChange([...value, opt]); - } else { - formField.onChange( - value.filter((v) => v !== opt), - ); - } - }} - /> - - + + {/* h-5 matches the label's leading-5 line box, so the + box stays on the first line of an option that + wraps to two. */} +
+ + { + if (checked) { + formField.onChange([...value, opt]); + } else { + formField.onChange( + value.filter((v) => v !== opt), + ); + } + }} + /> + +
+ {opt} diff --git a/client/portal/src/pages/superadmin/forms/components/ResponseDetailSheet.tsx b/client/portal/src/pages/superadmin/forms/components/ResponseDetailSheet.tsx index 5078b8d8..f4941252 100644 --- a/client/portal/src/pages/superadmin/forms/components/ResponseDetailSheet.tsx +++ b/client/portal/src/pages/superadmin/forms/components/ResponseDetailSheet.tsx @@ -317,7 +317,7 @@ export function ResponseDetailSheet({
{item - ? formatName(item.first_name, item.last_name) + ? formatName(item.first_name, item.last_name, item.email) : "Response"} diff --git a/client/portal/src/pages/superadmin/forms/components/ResponsesTable.tsx b/client/portal/src/pages/superadmin/forms/components/ResponsesTable.tsx index 6b612e37..e3e51e7e 100644 --- a/client/portal/src/pages/superadmin/forms/components/ResponsesTable.tsx +++ b/client/portal/src/pages/superadmin/forms/components/ResponsesTable.tsx @@ -120,7 +120,7 @@ function ResponseRow({

- {formatName(item.first_name, item.last_name)} + {formatName(item.first_name, item.last_name, item.email)}

{item.email}

diff --git a/client/portal/src/pages/superadmin/reviews/components/ReviewsTable.tsx b/client/portal/src/pages/superadmin/reviews/components/ReviewsTable.tsx index c09112e9..ff9d5cdf 100644 --- a/client/portal/src/pages/superadmin/reviews/components/ReviewsTable.tsx +++ b/client/portal/src/pages/superadmin/reviews/components/ReviewsTable.tsx @@ -109,7 +109,7 @@ export const ReviewsTable = memo(function ReviewsTable({ - {formatName(app.first_name, app.last_name)} + {formatName(app.first_name, app.last_name, app.email)} {app.email} diff --git a/client/portal/src/pages/superadmin/reviews/grading/GradingPage.tsx b/client/portal/src/pages/superadmin/reviews/grading/GradingPage.tsx index 5cfec6da..8245bc74 100644 --- a/client/portal/src/pages/superadmin/reviews/grading/GradingPage.tsx +++ b/client/portal/src/pages/superadmin/reviews/grading/GradingPage.tsx @@ -129,7 +129,11 @@ export default function GradingPage() { currentApp ? ( <>

- {formatName(currentApp.first_name, currentApp.last_name)} + {formatName( + currentApp.first_name, + currentApp.last_name, + currentApp.email, + )}

{currentApp.status} diff --git a/client/portal/src/shared/lib/api.ts b/client/portal/src/shared/lib/api.ts index 6434073a..7731abbd 100644 --- a/client/portal/src/shared/lib/api.ts +++ b/client/portal/src/shared/lib/api.ts @@ -36,6 +36,8 @@ export async function getRequest( error: !response.ok ? json?.error || `Failed to fetch ${errorContext || endpoint}` : undefined, + fields: + !response.ok && Array.isArray(json?.fields) ? json.fields : undefined, }; } catch (error) { if (error instanceof DOMException && error.name === "AbortError") { @@ -76,6 +78,8 @@ export async function postRequest( error: !response.ok ? json?.error || `Failed to post ${errorContext || endpoint}` : undefined, + fields: + !response.ok && Array.isArray(json?.fields) ? json.fields : undefined, }; } catch (error) { if (error instanceof DOMException && error.name === "AbortError") { @@ -116,6 +120,8 @@ export async function putRequest( error: !response.ok ? json?.error || `Failed to update ${errorContext || endpoint}` : undefined, + fields: + !response.ok && Array.isArray(json?.fields) ? json.fields : undefined, }; } catch (error) { if (error instanceof DOMException && error.name === "AbortError") { @@ -156,6 +162,8 @@ export async function patchRequest( error: !response.ok ? json?.error || `Failed to update ${errorContext || endpoint}` : undefined, + fields: + !response.ok && Array.isArray(json?.fields) ? json.fields : undefined, }; } catch (error) { if (error instanceof DOMException && error.name === "AbortError") { @@ -194,6 +202,8 @@ export async function deleteRequest( error: !response.ok ? json?.error || `Failed to delete ${errorContext || endpoint}` : undefined, + fields: + !response.ok && Array.isArray(json?.fields) ? json.fields : undefined, }; } catch (error) { if (error instanceof DOMException && error.name === "AbortError") { diff --git a/client/portal/src/shared/lib/schema-utils.ts b/client/portal/src/shared/lib/schema-utils.ts index f734b559..c5f92790 100644 --- a/client/portal/src/shared/lib/schema-utils.ts +++ b/client/portal/src/shared/lib/schema-utils.ts @@ -151,6 +151,26 @@ export function isFieldVisible( return !condition || conditionSatisfied(condition, values); } +/** + * Number fields answered as a whole count, with a floor the stored schema can + * raise but not lower. Keyed by field id (not type) so it applies to exactly the + * well-known fields — the same approach as getFieldPresets in field-presets.ts. + * Add an id here to give another number field the same treatment. + * + * Age is here because the seeded schema declares min: 0, which would otherwise + * accept "0" as an age (and, being a plain number field, "20.5" as well). + */ +const WHOLE_NUMBER_FIELDS: Record = { + age: { min: 1 }, +}; + +/** Whole-number rule for a field id, or undefined if it has none. */ +export function getWholeNumberRule( + fieldId: string, +): { min: number } | undefined { + return WHOLE_NUMBER_FIELDS[fieldId]; +} + /** Build a Zod schema for a single field based on its ApplicationSchemaField definition. */ function buildFieldZod(field: ApplicationSchemaField): z.ZodType { const validation = field.validation ?? {}; @@ -158,7 +178,11 @@ function buildFieldZod(field: ApplicationSchemaField): z.ZodType { switch (field.type) { case "text": { if (field.required) { - return z.string().min(1, `${field.label} is required`); + // Trim-aware, so a whitespace-only answer fails here rather than making + // it all the way to the server, which trims before its own check. + return z + .string() + .refine((v) => v.trim() !== "", `${field.label} is required`); } return z.string().optional().default(""); } @@ -180,18 +204,43 @@ function buildFieldZod(field: ApplicationSchemaField): z.ZodType { } case "number": { let n = z.coerce.number({ message: `${field.label} is required` }); - if (typeof validation.min === "number") - n = n.min(validation.min as number); - if (typeof validation.max === "number") - n = n.max(validation.max as number); - if (field.required && typeof validation.min !== "number") n = n.min(0); - return n; + const whole = getWholeNumberRule(field.id); + if (whole) n = n.int(`${field.label} must be a whole number`); + + const schemaMin = + typeof validation.min === "number" + ? (validation.min as number) + : undefined; + // A whole-number field's floor is the higher of the two, so a super admin + // can raise age's minimum but not drop it back below the rule. + const min = whole + ? Math.max(schemaMin ?? whole.min, whole.min) + : schemaMin; + + if (typeof min === "number") + n = n.min(min, `${field.label} must be at least ${min}`); + if (typeof validation.max === "number") { + const max = validation.max as number; + n = n.max(max, `${field.label} must be at most ${max}`); + } + if (field.required && typeof min !== "number") n = n.min(0); + + // Numbers default to undefined rather than 0 (see buildDefaultValues), so + // an untouched required field fails while an optional one passes. + return field.required ? n : n.optional(); } case "textarea": { let s = z.string(); - if (field.required) s = s.min(1, `${field.label} is required`); - if (typeof validation.maxLength === "number") - s = s.max(validation.maxLength as number); + if (typeof validation.maxLength === "number") { + const maxLength = validation.maxLength as number; + s = s.max( + maxLength, + `${field.label} must be ${maxLength} characters or fewer`, + ); + } + if (field.required) { + return s.refine((v) => v.trim() !== "", `${field.label} is required`); + } return s; } case "select": { @@ -200,8 +249,12 @@ function buildFieldZod(field: ApplicationSchemaField): z.ZodType { } return z.string().optional().default(""); } - case "multi_select": + case "multi_select": { + if (field.required) { + return z.array(z.string()).min(1, `${field.label} is required`); + } return z.array(z.string()).optional().default([]); + } case "checkbox": if (field.required) { return z.literal(true, { @@ -216,33 +269,43 @@ function buildFieldZod(field: ApplicationSchemaField): z.ZodType { /** * Build a Zod object schema from an array of ApplicationSchemaField definitions. - * Returns a z.object() with one key per field. Fields with a "required_if" - * validation key become required when their controlling checkbox is checked. + * Returns a z.object() with one key per field. + * + * Requiredness for conditional fields is decided in the refinement rather than + * in the per-field schema, because it depends on another field's answer: + * - a field hidden by an unsatisfied "show_if" is never required (the step + * validator triggers every field in a section, including hidden ones, so + * enforcing it would block the step with nothing on screen to fix); + * - a field with "required_if" becomes required once its controller is set. */ export function buildZodSchema(fields: ApplicationSchemaField[]) { + const conditional = fields.map((f) => ({ + field: f, + showIf: getFieldCondition(f, "show_if"), + requiredIf: getFieldCondition(f, "required_if"), + })); + const shape: Record = {}; - for (const field of fields) { - shape[field.id] = buildFieldZod(field); + for (const { field, showIf } of conditional) { + // A show_if-gated field is shaped as optional; the refinement below applies + // its requiredness only while it is actually visible. + shape[field.id] = buildFieldZod( + showIf ? { ...field, required: false } : field, + ); } - const conditional = fields - .map((f) => ({ field: f, condition: getFieldCondition(f, "required_if") })) - .filter( - (c): c is { field: ApplicationSchemaField; condition: FieldCondition } => - !!c.condition, - ); + const refined = conditional.filter((c) => c.showIf || c.requiredIf); return z.object(shape).superRefine((data, ctx) => { - for (const { field, condition } of conditional) { - if (!conditionSatisfied(condition, data)) continue; - - const value = data[field.id]; - const empty = - value === undefined || - value === null || - (typeof value === "string" && value.trim() === "") || - (Array.isArray(value) && value.length === 0); - if (empty) { + for (const { field, showIf, requiredIf } of refined) { + if (showIf && !conditionSatisfied(showIf, data)) continue; + + const required = + field.required || + (!!requiredIf && conditionSatisfied(requiredIf, data)); + if (!required) continue; + + if (isEmptyValue(data[field.id])) { ctx.addIssue({ code: "custom", path: [field.id], @@ -253,6 +316,17 @@ export function buildZodSchema(fields: ApplicationSchemaField[]) { }); } +/** True when an answer counts as unanswered: blank, unchecked, or nothing picked. */ +function isEmptyValue(value: unknown): boolean { + return ( + value === undefined || + value === null || + value === false || + (typeof value === "string" && value.trim() === "") || + (Array.isArray(value) && value.length === 0) + ); +} + /** Build default form values from schema fields. */ export function buildDefaultValues( fields: ApplicationSchemaField[], @@ -260,8 +334,11 @@ export function buildDefaultValues( const defaults: Record = {}; for (const field of fields) { switch (field.type) { + // Numbers start blank rather than at 0: a pre-filled 0 satisfies a + // required field (and age's seeded min of 0) without the hacker ever + // answering it. case "number": - defaults[field.id] = 0; + defaults[field.id] = undefined; break; case "multi_select": defaults[field.id] = []; diff --git a/client/portal/src/types.ts b/client/portal/src/types.ts index 0e74de6c..bebd70e9 100644 --- a/client/portal/src/types.ts +++ b/client/portal/src/types.ts @@ -169,6 +169,12 @@ export interface ApiResponse { status: number; data?: T; error?: string; + /** + * Field ids an endpoint blamed for a failed request, when it reports them. + * Lets a form map a server-side rejection back onto its own inputs instead of + * showing the raw message. + */ + fields?: string[]; } export interface Scan { diff --git a/cmd/api/applications.go b/cmd/api/applications.go index fe9d3e27..8a19ac0d 100644 --- a/cmd/api/applications.go +++ b/cmd/api/applications.go @@ -159,7 +159,7 @@ func (app *application) updateApplicationHandler(w http.ResponseWriter, r *http. } if validationErrors := validateResponses(schema, responses, false); len(validationErrors) > 0 { - app.badRequestResponse(w, r, fmt.Errorf("validation errors: %v", validationErrors)) + app.validationErrorResponse(w, r, validationErrors) return } @@ -196,7 +196,7 @@ func (app *application) updateApplicationHandler(w http.ResponseWriter, r *http. // @Tags hackers // @Produce json // @Success 200 {object} store.Application -// @Failure 400 {object} object{error=string} "Missing required fields" +// @Failure 400 {object} object{error=string,fields=[]string} "Missing required fields; fields lists the offending schema field ids" // @Failure 401 {object} object{error=string} // @Failure 404 {object} object{error=string} // @Failure 409 {object} object{error=string} "Application not in draft status" @@ -245,7 +245,7 @@ func (app *application) submitApplicationHandler(w http.ResponseWriter, r *http. validationErrors := validateResponses(schema, responses, true) if len(validationErrors) > 0 { - app.badRequestResponse(w, r, fmt.Errorf("validation errors: %v", validationErrors)) + app.validationErrorResponse(w, r, validationErrors) return } @@ -262,16 +262,58 @@ func (app *application) submitApplicationHandler(w http.ResponseWriter, r *http. } } +// fieldValidationError ties a validation failure to the schema field it belongs +// to, so the handler can report both the message and the offending field id. +type fieldValidationError struct { + Field string + Message string +} + +// validationMessages returns the human-readable half of each error. +func validationMessages(errs []fieldValidationError) []string { + messages := make([]string, 0, len(errs)) + for _, e := range errs { + messages = append(messages, e.Message) + } + return messages +} + +// validationFieldIDs returns the offending field ids, deduplicated and in the +// order they were reported. +func validationFieldIDs(errs []fieldValidationError) []string { + seen := make(map[string]struct{}, len(errs)) + fields := make([]string, 0, len(errs)) + for _, e := range errs { + if _, ok := seen[e.Field]; ok { + continue + } + seen[e.Field] = struct{}{} + fields = append(fields, e.Field) + } + return fields +} + // validateResponses checks each response value against its schema field definition. -// Returns a list of human-readable validation error strings. When enforceRequired -// is false, missing/empty required fields are allowed (used for draft saves) while -// type checks on present values still apply. -func validateResponses(schema []store.ApplicationSchemaField, responses map[string]interface{}, enforceRequired bool) []string { - var errs []string +// Returns one entry per failure, carrying the field id and a human-readable +// message. When enforceRequired is false, missing/empty required fields are +// allowed (used for draft saves) while type checks on present values still apply. +func validateResponses(schema []store.ApplicationSchemaField, responses map[string]interface{}, enforceRequired bool) []fieldValidationError { + var errs []fieldValidationError + fail := func(fieldID, message string) { + errs = append(errs, fieldValidationError{Field: fieldID, Message: message}) + } for _, field := range schema { val, exists := responses[field.ID] + // A field hidden by an unsatisfied validation.show_if isn't being asked, + // so it is never required — the client doesn't render it either. Type + // checks on any leftover value still apply. + hidden := false + if showIf, ok := field.Validation["show_if"].(string); ok && showIf != "" { + hidden = !conditionSatisfied(showIf, responses) + } + // A field with validation.required_if is required only when its // controller condition holds (e.g. travel questions are required only // when travel_reimbursement is checked, or flight fields only when @@ -284,10 +326,13 @@ func validateResponses(schema []store.ApplicationSchemaField, responses map[stri } } } + if hidden { + required = false + } // Required check if enforceRequired && required && (!exists || isEmpty(val)) { - errs = append(errs, field.ID+" is required") + fail(field.ID, field.ID+" is required") continue } @@ -301,65 +346,65 @@ func validateResponses(schema []store.ApplicationSchemaField, responses map[stri case "text", "textarea", "phone": s, ok := val.(string) if !ok { - errs = append(errs, field.ID+" must be a string") + fail(field.ID, field.ID+" must be a string") continue } if maxLen, ok := field.Validation["maxLength"]; ok { if ml, ok := maxLen.(float64); ok && float64(len(s)) > ml { - errs = append(errs, fmt.Sprintf("%s exceeds max length of %d", field.ID, int(ml))) + fail(field.ID, fmt.Sprintf("%s exceeds max length of %d", field.ID, int(ml))) } } case "number": n, ok := val.(float64) if !ok { - errs = append(errs, field.ID+" must be a number") + fail(field.ID, field.ID+" must be a number") continue } if minVal, ok := field.Validation["min"]; ok { if mv, ok := minVal.(float64); ok && n < mv { - errs = append(errs, fmt.Sprintf("%s must be at least %v", field.ID, mv)) + fail(field.ID, fmt.Sprintf("%s must be at least %v", field.ID, mv)) } } if maxVal, ok := field.Validation["max"]; ok { if mv, ok := maxVal.(float64); ok && n > mv { - errs = append(errs, fmt.Sprintf("%s must be at most %v", field.ID, mv)) + fail(field.ID, fmt.Sprintf("%s must be at most %v", field.ID, mv)) } } case "select": s, ok := val.(string) if !ok { - errs = append(errs, field.ID+" must be a string") + fail(field.ID, field.ID+" must be a string") continue } if len(field.Options) > 0 && !containsString(field.Options, s) { - errs = append(errs, field.ID+" has invalid option: "+s) + fail(field.ID, field.ID+" has invalid option: "+s) } case "multi_select": arr, ok := val.([]interface{}) if !ok { - errs = append(errs, field.ID+" must be an array") + fail(field.ID, field.ID+" must be an array") continue } for _, item := range arr { s, ok := item.(string) if !ok { - errs = append(errs, field.ID+" array items must be strings") + fail(field.ID, field.ID+" array items must be strings") break } if len(field.Options) > 0 && !containsString(field.Options, s) { - errs = append(errs, field.ID+" has invalid option: "+s) + fail(field.ID, field.ID+" has invalid option: "+s) } } case "checkbox": b, ok := val.(bool) if !ok { - errs = append(errs, field.ID+" must be a boolean") - } else if enforceRequired && field.Required && !b { - errs = append(errs, field.ID+" must be checked") + fail(field.ID, field.ID+" must be a boolean") + } else if enforceRequired && required && !b { + fail(field.ID, field.ID+" must be checked") } } } diff --git a/cmd/api/applications_test.go b/cmd/api/applications_test.go index 03783389..a3f8100e 100644 --- a/cmd/api/applications_test.go +++ b/cmd/api/applications_test.go @@ -413,6 +413,139 @@ func TestSubmitApplication(t *testing.T) { mockSettings.AssertExpectations(t) }) + t.Run("should return 400 with the field id when a required multi_select is empty", func(t *testing.T) { + user := newTestUser() + application := newCompleteApplication(user.ID) + application.Responses = json.RawMessage(`{"first_name":"John","last_name":"Doe","dietary_restrictions":[]}`) + + schema := []store.ApplicationSchemaField{ + {ID: "first_name", Type: "text", Label: "First Name", Required: true}, + {ID: "last_name", Type: "text", Label: "Last Name", Required: true}, + {ID: "dietary_restrictions", Type: "multi_select", Label: "Dietary Restrictions", Required: true, Options: []string{"Vegan", "Halal"}}, + } + + mockApps.On("GetByUserID", user.ID).Return(application, nil).Once() + mockSettings.On("GetApplicationSchema").Return(schema, nil).Once() + + req, err := http.NewRequest(http.MethodPost, "/", nil) + require.NoError(t, err) + req = setUserContext(req, user) + + rr := executeRequest(req, http.HandlerFunc(app.submitApplicationHandler)) + checkResponseCode(t, http.StatusBadRequest, rr.Code) + + var body struct { + Error string `json:"error"` + Fields []string `json:"fields"` + } + err = json.NewDecoder(rr.Body).Decode(&body) + require.NoError(t, err) + assert.Contains(t, body.Error, "dietary_restrictions is required") + // The field ids let the form blame the question instead of showing the + // raw message. + assert.Equal(t, []string{"dietary_restrictions"}, body.Fields) + + mockApps.AssertExpectations(t) + mockSettings.AssertExpectations(t) + }) + + t.Run("should report every offending field id once", func(t *testing.T) { + user := newTestUser() + application := &store.Application{ID: "app-1", UserID: user.ID, Status: store.StatusDraft} + + schema := []store.ApplicationSchemaField{ + {ID: "first_name", Type: "text", Label: "First Name", Required: true}, + {ID: "last_name", Type: "text", Label: "Last Name", Required: true}, + } + + mockApps.On("GetByUserID", user.ID).Return(application, nil).Once() + mockSettings.On("GetApplicationSchema").Return(schema, nil).Once() + + req, err := http.NewRequest(http.MethodPost, "/", nil) + require.NoError(t, err) + req = setUserContext(req, user) + + rr := executeRequest(req, http.HandlerFunc(app.submitApplicationHandler)) + checkResponseCode(t, http.StatusBadRequest, rr.Code) + + var body struct { + Fields []string `json:"fields"` + } + err = json.NewDecoder(rr.Body).Decode(&body) + require.NoError(t, err) + assert.Equal(t, []string{"first_name", "last_name"}, body.Fields) + + mockApps.AssertExpectations(t) + mockSettings.AssertExpectations(t) + }) + + t.Run("should not require a field hidden by an unsatisfied show_if", func(t *testing.T) { + user := newTestUser() + application := newCompleteApplication(user.ID) + + // travel_origin is required, but its controlling checkbox is unchecked, + // so the form never asks for it and submit must not block on it. + schema := []store.ApplicationSchemaField{ + {ID: "first_name", Type: "text", Label: "First Name", Required: true}, + {ID: "last_name", Type: "text", Label: "Last Name", Required: true}, + {ID: travelOptInFieldID, Type: "checkbox", Label: "Travel reimbursement"}, + { + ID: "travel_origin", Type: "text", Label: "Traveling from", Required: true, + Validation: map[string]interface{}{"show_if": travelOptInFieldID}, + }, + } + + mockApps.On("GetByUserID", user.ID).Return(application, nil).Once() + mockSettings.On("GetApplicationSchema").Return(schema, nil).Once() + mockApps.On("Submit", application, travelOptInFieldID).Return(nil).Once() + + req, err := http.NewRequest(http.MethodPost, "/", nil) + require.NoError(t, err) + req = setUserContext(req, user) + + rr := executeRequest(req, http.HandlerFunc(app.submitApplicationHandler)) + checkResponseCode(t, http.StatusOK, rr.Code) + + mockApps.AssertExpectations(t) + mockSettings.AssertExpectations(t) + }) + + t.Run("should require a field once its show_if condition holds", func(t *testing.T) { + user := newTestUser() + application := newCompleteApplication(user.ID) + application.Responses = json.RawMessage(`{"first_name":"John","last_name":"Doe","travel_reimbursement":true}`) + + schema := []store.ApplicationSchemaField{ + {ID: "first_name", Type: "text", Label: "First Name", Required: true}, + {ID: "last_name", Type: "text", Label: "Last Name", Required: true}, + {ID: travelOptInFieldID, Type: "checkbox", Label: "Travel reimbursement"}, + { + ID: "travel_origin", Type: "text", Label: "Traveling from", Required: true, + Validation: map[string]interface{}{"show_if": travelOptInFieldID}, + }, + } + + mockApps.On("GetByUserID", user.ID).Return(application, nil).Once() + mockSettings.On("GetApplicationSchema").Return(schema, nil).Once() + + req, err := http.NewRequest(http.MethodPost, "/", nil) + require.NoError(t, err) + req = setUserContext(req, user) + + rr := executeRequest(req, http.HandlerFunc(app.submitApplicationHandler)) + checkResponseCode(t, http.StatusBadRequest, rr.Code) + + var body struct { + Fields []string `json:"fields"` + } + err = json.NewDecoder(rr.Body).Decode(&body) + require.NoError(t, err) + assert.Equal(t, []string{"travel_origin"}, body.Fields) + + mockApps.AssertExpectations(t) + mockSettings.AssertExpectations(t) + }) + t.Run("should return 409 when application already submitted", func(t *testing.T) { user := newTestUser() application := &store.Application{ID: "app-1", UserID: user.ID, Status: store.StatusSubmitted} diff --git a/cmd/api/errors.go b/cmd/api/errors.go index 0f926b4e..94f89289 100644 --- a/cmd/api/errors.go +++ b/cmd/api/errors.go @@ -2,6 +2,7 @@ package main import ( "context" + "fmt" "net/http" "github.com/hackutd/harp/internal/store" @@ -31,6 +32,17 @@ func (app *application) badRequestResponse(w http.ResponseWriter, r *http.Reques err.Error()) } +// validationErrorResponse reports schema-validation failures. The message keeps +// its historical shape for compatibility; the field ids let the form map each +// failure back onto the question that caused it. +func (app *application) validationErrorResponse(w http.ResponseWriter, r *http.Request, errs []fieldValidationError) { + message := fmt.Sprintf("validation errors: %v", validationMessages(errs)) + + app.logger.Warnw("validation failed", "method", r.Method, "path", r.URL.Path, "error", message) + + writeJSONFieldError(w, http.StatusBadRequest, message, validationFieldIDs(errs)) +} + func (app *application) conflictResponse(w http.ResponseWriter, r *http.Request, err error) { app.logger.Errorw("conflict response", "method", r.Method, "path", r.URL.Path, "error", err.Error()) diff --git a/cmd/api/json.go b/cmd/api/json.go index e64cf300..f7887e81 100644 --- a/cmd/api/json.go +++ b/cmd/api/json.go @@ -37,6 +37,18 @@ func writeJSONError(w http.ResponseWriter, status int, message string) error { return writeJSON(w, status, &envolope{Error: message}) } +// writeJSONFieldError writes the standard error envelope plus the ids of the +// fields the request was rejected for, so a client form can blame its own +// inputs instead of surfacing the raw message. +func writeJSONFieldError(w http.ResponseWriter, status int, message string, fields []string) error { + type envelope struct { + Error string `json:"error"` + Fields []string `json:"fields,omitempty"` + } + + return writeJSON(w, status, &envelope{Error: message, Fields: fields}) +} + func (app *application) jsonResponse(w http.ResponseWriter, status int, data any) error { type envelope struct { Data any `json:"data"` diff --git a/cmd/api/rsvp.go b/cmd/api/rsvp.go index 4ea24234..f8597fe7 100644 --- a/cmd/api/rsvp.go +++ b/cmd/api/rsvp.go @@ -3,7 +3,6 @@ package main import ( "encoding/json" "errors" - "fmt" "net/http" "time" @@ -159,7 +158,7 @@ func (app *application) submitMyRSVPHandler(w http.ResponseWriter, r *http.Reque } if validationErrors := validateResponses(schema, responses, true); len(validationErrors) > 0 { - app.badRequestResponse(w, r, fmt.Errorf("validation errors: %v", validationErrors)) + app.validationErrorResponse(w, r, validationErrors) return } diff --git a/cmd/api/travelrsvp.go b/cmd/api/travelrsvp.go index 2873bb84..e529e503 100644 --- a/cmd/api/travelrsvp.go +++ b/cmd/api/travelrsvp.go @@ -225,7 +225,7 @@ func (app *application) submitMyTravelRSVPHandler(w http.ResponseWriter, r *http } if validationErrors := validateResponses(schema, responses, true); len(validationErrors) > 0 { - app.badRequestResponse(w, r, fmt.Errorf("validation errors: %v", validationErrors)) + app.validationErrorResponse(w, r, validationErrors) return } diff --git a/docs/docs.go b/docs/docs.go index 3d6a554f..584ae2a8 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -3049,12 +3049,18 @@ const docTemplate = `{ } }, "400": { - "description": "Missing required fields", + "description": "Missing required fields; fields lists the offending schema field ids", "schema": { "type": "object", "properties": { "error": { "type": "string" + }, + "fields": { + "type": "array", + "items": { + "type": "string" + } } } } diff --git a/internal/store/integration_test.go b/internal/store/integration_test.go index de1991c5..4213b1c5 100644 --- a/internal/store/integration_test.go +++ b/internal/store/integration_test.go @@ -222,6 +222,49 @@ func TestIntegrationSubmitVote(t *testing.T) { } } +func TestIntegrationReviewQueueToleratesBadNumbers(t *testing.T) { + db := integrationDB(t) + defer db.Close() + seedIntegration(t, db) + s := &ApplicationReviewsStore{db: db} + ctx := context.Background() + admin := "44444444-4444-4444-4444-444444444444" + + // responses is free-text JSONB, so age can hold a decimal or an out-of-range + // value. A bare ::smallint cast would fail the whole query and 500 the + // grading queue for every admin; these must read as NULL instead. + if _, err := db.ExecContext(ctx, ` + UPDATE applications + SET responses = responses || '{"age":"20.5","hackathons_attended":"99999"}' + WHERE id = 'aaaaaaaa-0000-0000-0000-000000000002' + `); err != nil { + t.Fatal(err) + } + + pending, err := s.GetPendingByAdminID(ctx, admin) + if err != nil { + t.Fatalf("GetPendingByAdminID: %v", err) + } + if len(pending) == 0 { + t.Fatal("expected pending reviews") + } + for _, r := range pending { + if r.ApplicationID == "aaaaaaaa-0000-0000-0000-000000000002" && r.Age != nil { + t.Errorf("age = %v, want nil for an unparseable value", *r.Age) + } + } + + if _, err := db.ExecContext(ctx, ` + UPDATE application_reviews SET vote = 'accept', reviewed_at = NOW() + WHERE id = 'bbbbbbbb-0000-0000-0000-000000000001' + `); err != nil { + t.Fatal(err) + } + if _, err := s.GetCompletedByAdminID(ctx, admin); err != nil { + t.Fatalf("GetCompletedByAdminID: %v", err) + } +} + func TestIntegrationSettingsCache(t *testing.T) { db := integrationDB(t) defer db.Close() diff --git a/internal/store/reviews.go b/internal/store/reviews.go index 372930a5..f5dc1a3e 100644 --- a/internal/store/reviews.go +++ b/internal/store/reviews.go @@ -141,10 +141,16 @@ func (s *ApplicationReviewsStore) GetPendingByAdminID(ctx context.Context, admin ar.id, ar.application_id, ar.admin_id, ar.vote, ar.travel_vote, ar.notes, ar.assigned_at, ar.reviewed_at, ar.created_at, ar.updated_at, a.responses->>'first_name', a.responses->>'last_name', u.email, - NULLIF(a.responses->>'age', '')::smallint, + -- responses is free-text JSONB, so these can hold any string. A bare + -- ::smallint cast makes one bad value fail the whole query and 500 the + -- grading queue for every admin, so only values that provably fit are + -- cast; anything else reads as NULL (see ApplicationsStore.List). + CASE WHEN a.responses->>'age' ~ '^[0-9]{1,3}$' + THEN (a.responses->>'age')::smallint END, a.responses->>'university', a.responses->>'major', a.responses->>'country_of_residence', - NULLIF(a.responses->>'hackathons_attended', '')::smallint, + CASE WHEN a.responses->>'hackathons_attended' ~ '^[0-9]{1,4}$' + THEN (a.responses->>'hackathons_attended')::smallint END, a.travel_status FROM application_reviews ar JOIN applications a ON ar.application_id = a.id @@ -194,10 +200,16 @@ func (s *ApplicationReviewsStore) GetCompletedByAdminID(ctx context.Context, adm ar.id, ar.application_id, ar.admin_id, ar.vote, ar.travel_vote, ar.notes, ar.assigned_at, ar.reviewed_at, ar.created_at, ar.updated_at, a.responses->>'first_name', a.responses->>'last_name', u.email, - NULLIF(a.responses->>'age', '')::smallint, + -- responses is free-text JSONB, so these can hold any string. A bare + -- ::smallint cast makes one bad value fail the whole query and 500 the + -- grading queue for every admin, so only values that provably fit are + -- cast; anything else reads as NULL (see ApplicationsStore.List). + CASE WHEN a.responses->>'age' ~ '^[0-9]{1,3}$' + THEN (a.responses->>'age')::smallint END, a.responses->>'university', a.responses->>'major', a.responses->>'country_of_residence', - NULLIF(a.responses->>'hackathons_attended', '')::smallint, + CASE WHEN a.responses->>'hackathons_attended' ~ '^[0-9]{1,4}$' + THEN (a.responses->>'hackathons_attended')::smallint END, a.travel_status FROM application_reviews ar JOIN applications a ON ar.application_id = a.id From 831806d40248085e2401573020ce16ed5f197d20 Mon Sep 17 00:00:00 2001 From: Caleb Bae <144546374+balebbae@users.noreply.github.com> Date: Tue, 8 Sep 2026 07:42:00 -0700 Subject: [PATCH 06/11] fix: a more balanced review process & confirmation dialogs (#156) --- Taskfile.yml | 5 + client/portal/package-lock.json | 1 + client/portal/package.json | 4 +- .../scripts/review-regressions.test.mjs | 310 +++++++++++++ .../components/ApplicationDetailPanel.tsx | 13 + .../pages/admin/all-applicants/createStore.ts | 41 +- .../hooks/useApplicationDetail.ts | 19 +- .../src/pages/admin/reviews/ReviewsPage.tsx | 25 +- .../admin/reviews/grading/GradingPage.tsx | 73 +-- .../src/pages/admin/reviews/grading/store.ts | 54 ++- .../portal/src/pages/admin/reviews/store.ts | 24 +- .../pages/superadmin/reviews/ReviewsPage.tsx | 363 +++++++++++---- .../src/pages/superadmin/reviews/api.ts | 9 + .../reviews/components/ReviewsTable.tsx | 14 +- cmd/api/reviews_test.go | 4 +- cmd/api/schemafields_test.go | 25 ++ cmd/api/settings.go | 4 +- cmd/resetschema/main.go | 232 ++++++++++ docs/docs.go | 14 +- internal/store/default_schemas.go | 89 ++++ internal/store/default_schemas_test.go | 117 +++++ .../store/defaults/application_schema.json | 376 ++++++++++++++++ internal/store/defaults/rsvp_schema.json | 55 +++ .../store/defaults/travel_rsvp_schema.json | 85 ++++ internal/store/integration_test.go | 37 ++ internal/store/mock_store.go | 5 + internal/store/reviews.go | 335 +++++++------- .../reviews_assignment_integration_test.go | 416 ++++++++++++++++++ internal/store/settings.go | 2 +- internal/store/storage.go | 3 + 30 files changed, 2421 insertions(+), 333 deletions(-) create mode 100644 client/portal/scripts/review-regressions.test.mjs create mode 100644 cmd/resetschema/main.go create mode 100644 internal/store/default_schemas.go create mode 100644 internal/store/default_schemas_test.go create mode 100644 internal/store/defaults/application_schema.json create mode 100644 internal/store/defaults/rsvp_schema.json create mode 100644 internal/store/defaults/travel_rsvp_schema.json create mode 100644 internal/store/reviews_assignment_integration_test.go diff --git a/Taskfile.yml b/Taskfile.yml index ae7054df..72555d6f 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -32,6 +32,11 @@ tasks: cmds: - go run cmd/migrate/seed/main.go + reset-schema: + desc: Restore form schemas to the shipped defaults (task reset-schema -- -all -dry-run) + cmds: + - go run ./cmd/resetschema {{.CLI_ARGS}} + gen-docs: desc: Generate Swagger docs cmds: diff --git a/client/portal/package-lock.json b/client/portal/package-lock.json index 9fb01b1c..259716ec 100644 --- a/client/portal/package-lock.json +++ b/client/portal/package-lock.json @@ -70,6 +70,7 @@ "@types/react": "^19.2.5", "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^5.1.1", + "esbuild": "0.27.3", "eslint": "^9.39.1", "eslint-config-prettier": "^10.1.8", "eslint-plugin-boundaries": "^6.0.2", diff --git a/client/portal/package.json b/client/portal/package.json index 14a666cc..40c6ff64 100644 --- a/client/portal/package.json +++ b/client/portal/package.json @@ -9,7 +9,8 @@ "lint": "eslint .", "format": "prettier --write \"{src,branding}/**/*.{ts,tsx,css,json}\"", "format:check": "prettier --check \"{src,branding}/**/*.{ts,tsx,css,json}\"", - "preview": "vite preview" + "preview": "vite preview", + "test:reviews": "node --test scripts/review-regressions.test.mjs" }, "dependencies": { "@hookform/resolvers": "^5.2.2", @@ -74,6 +75,7 @@ "@types/react": "^19.2.5", "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^5.1.1", + "esbuild": "0.27.3", "eslint": "^9.39.1", "eslint-config-prettier": "^10.1.8", "eslint-plugin-boundaries": "^6.0.2", diff --git a/client/portal/scripts/review-regressions.test.mjs b/client/portal/scripts/review-regressions.test.mjs new file mode 100644 index 00000000..cdb2ea7c --- /dev/null +++ b/client/portal/scripts/review-regressions.test.mjs @@ -0,0 +1,310 @@ +import assert from "node:assert/strict"; +import { createRequire } from "node:module"; +import { beforeEach, test } from "node:test"; +import { fileURLToPath } from "node:url"; + +import { build } from "esbuild"; + +const project = fileURLToPath(new URL("../", import.meta.url)); +const require = createRequire(new URL("../package.json", import.meta.url)); +const bundle = await build({ + absWorkingDir: project, + stdin: { + contents: ` + export { useAdminGradingStore as grading } from './src/pages/admin/reviews/grading/store.ts'; + export { useReviewsStore as list } from './src/pages/admin/reviews/store.ts'; + export { useReviewApplicationsStore as applications } from './src/pages/superadmin/reviews/store.ts'; + `, + resolveDir: project, + loader: "ts", + }, + platform: "node", + format: "cjs", + bundle: true, + write: false, + plugins: [ + { + name: "silent-toasts", + setup(b) { + b.onResolve({ filter: /^sonner$/ }, () => ({ + path: "toast", + namespace: "test", + })); + b.onLoad({ filter: /.*/, namespace: "test" }, () => ({ + contents: + "export const toast = { success(){}, warning(){}, error(){} };", + })); + }, + }, + ], +}); +const module = { exports: {} }; +new Function("module", "exports", "require", bundle.outputFiles[0].text)( + module, + module.exports, + require, +); +const { grading, list, applications } = module.exports; +const reviews = [1, 2, 3].map((i) => ({ + id: `r${i}`, + application_id: `a${i}`, + vote: null, + travel_status: "not_requested", +})); +const pendingPath = "/v1/admin/reviews/pending"; +const completedPath = "/v1/admin/reviews/completed"; +const appPath = "/v1/admin/applications"; +let handlers; +let requests; +let failVote; + +function ok(data) { + return Response.json({ data }); +} +function failed() { + return Response.json({ error: "Database unavailable" }, { status: 500 }); +} +function deferred() { + let resolve; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; +} +function appList(ids = ["a1"]) { + return { + applications: ids.map((id) => ({ id })), + next_cursor: null, + prev_cursor: null, + has_more: false, + }; +} + +beforeEach(() => { + grading.getState().reset(); + list.getState().setTab("assigned"); + applications.getState().resetPagination(); + handlers = new Map(); + requests = []; + failVote = false; + // Exercise the real API wrappers as well as the actual bundled stores. + globalThis.fetch = async (url, options) => { + const path = new URL(url, "http://test.local").pathname; + requests.push({ url, ...options }); + if (handlers.has(path)) return handlers.get(path)(url, options); + if (path === pendingPath) return ok({ reviews }); + if (path === completedPath) return ok({ reviews: [] }); + if (path === appPath) return ok(appList()); + if (path === `${appPath}/stats`) return ok({ submitted: 1 }); + if (path.endsWith("/notes")) return ok({ notes: [] }); + if (path.startsWith(`${appPath}/`)) + return ok({ id: path.split("/").at(-1), ai_percent: null }); + if (path.startsWith("/v1/admin/reviews/") && options.method === "PUT") { + return failVote ? failed() : ok({}); + } + throw new Error(`Unexpected request: ${options.method} ${url}`); + }; +}); + +for (const tab of ["assigned", "completed"]) { + test(`${tab} queue distinguishes failure from empty and supports retry`, async () => { + list.getState().setTab(tab); + const path = tab === "assigned" ? pendingPath : completedPath; + handlers.set(path, failed); + await list.getState().fetchReviews(); + assert.equal(list.getState().error, "Database unavailable"); + assert.equal(list.getState().loading, false); + handlers.set(path, () => ok({ reviews })); + await list.getState().fetchReviews(); + assert.equal(list.getState().error, null); + assert.equal(list.getState().reviews.length, 3); + handlers.set(path, () => ok({ reviews: [] })); + await list.getState().fetchReviews(); + assert.equal(list.getState().error, null); + assert.deepEqual(list.getState().reviews, []); + }); +} + +test("network failures remain visible in both review queues", async () => { + handlers.set(pendingPath, () => { + throw new Error("Network offline"); + }); + await list.getState().fetchReviews(); + await grading.getState().fetchReviews(); + for (const store of [list, grading]) { + assert.equal(store.getState().error, "Network offline"); + assert.equal(store.getState().loading, false); + } +}); + +test("grading retry selects the requested review and loads its detail and notes", async () => { + handlers.set(pendingPath, failed); + await grading.getState().fetchReviews("r2"); + assert.equal(grading.getState().error, "Database unavailable"); + handlers.delete(pendingPath); + await grading.getState().fetchReviews("r2"); + assert.equal(grading.getState().error, null); + assert.equal(grading.getState().currentIndex, 1); + assert.equal(grading.getState().detail.id, "a2"); + assert.equal(grading.getState().notesLoading, false); + assert.ok(requests.some((r) => r.url === `${appPath}/a2/notes`)); + await grading.getState().fetchReviews("removed-review"); + assert.equal(grading.getState().currentIndex, 0); + assert.equal(grading.getState().detail.id, "a1"); +}); + +test("a successful empty grading queue has no error or stale detail", async () => { + await grading.getState().fetchReviews(); + handlers.set(pendingPath, () => ok({ reviews: [] })); + await grading.getState().fetchReviews(); + assert.equal(grading.getState().error, null); + assert.equal(grading.getState().detail, null); + assert.deepEqual(grading.getState().reviews, []); +}); + +test("grading advances through every assigned review and retains a failed vote", async () => { + await grading.getState().fetchReviews(); + failVote = true; + await grading.getState().submitVote("r1", "accept"); + assert.equal(grading.getState().reviews.length, 3); + assert.equal(grading.getState().submitting, false); + failVote = false; + for (const id of ["r1", "r2", "r3"]) { + const state = grading.getState(); + assert.equal(state.reviews[state.currentIndex].id, id); + await state.submitVote(id, "accept"); + } + assert.equal(grading.getState().reviews.length, 0); + assert.equal(grading.getState().detail, null); +}); + +for (const [name, store] of [ + ["list", list], + ["grading", grading], +]) { + test(`${name} ignores superseded fetch responses`, async () => { + const older = deferred(); + handlers.set(pendingPath, () => older.promise); + const first = store.getState().fetchReviews(); + handlers.set(pendingPath, () => ok({ reviews: [reviews[2]] })); + await store.getState().fetchReviews(); + older.resolve(failed()); + await first; + assert.equal(store.getState().error, null); + assert.equal(store.getState().reviews[0].id, "r3"); + }); + test(`${name} abort releases loading without presenting an error`, async () => { + const response = deferred(); + handlers.set(pendingPath, () => response.promise); + const controller = new AbortController(); + const request = + name === "list" + ? store.getState().fetchReviews(controller.signal) + : store.getState().fetchReviews(undefined, controller.signal); + controller.abort(); + response.resolve(failed()); + await request; + assert.equal(store.getState().error, null); + assert.equal(store.getState().loading, false); + assert.deepEqual(store.getState().reviews, []); + }); +} + +test("switching queue tabs invalidates the previous tab's request", async () => { + const older = deferred(); + handlers.set(pendingPath, () => older.promise); + const first = list.getState().fetchReviews(); + list.getState().setTab("completed"); + await list.getState().fetchReviews(); + older.resolve(ok({ reviews })); + await first; + assert.equal(list.getState().tab, "completed"); + assert.deepEqual(list.getState().reviews, []); +}); + +test("reset invalidates pending grading requests and detail requests", async () => { + const detail = deferred(); + handlers.set(`${appPath}/a1`, () => detail.promise); + const first = grading.getState().fetchReviews(); + await new Promise((r) => setImmediate(r)); + grading.getState().reset(); + await grading.getState().fetchReviews("r2"); + detail.resolve(ok({ id: "a1" })); + await first; + assert.equal(grading.getState().detail.id, "a2"); +}); + +test("grading cannot submit or navigate while the queue is failed or loading", async () => { + handlers.set(pendingPath, failed); + await grading.getState().fetchReviews(); + await grading.getState().submitVote("r1", "accept"); + grading.getState().navigateNext(); + assert.equal(grading.getState().currentIndex, 0); + assert.equal(requests.filter((r) => r.method === "PUT").length, 0); +}); + +test("application refresh preserves filters and sort, drops cursor, and exposes retry errors", async () => { + await applications.getState().fetchApplications({ + status: "submitted", + search: "alice", + sort_by: "reject_votes", + cursor: "page2", + }); + handlers.set(appPath, failed); + await applications.getState().fetchApplications(); + assert.equal(applications.getState().error, "Database unavailable"); + assert.equal(applications.getState().currentSearch, "alice"); + handlers.delete(appPath); + await applications.getState().fetchApplications(); + assert.equal(applications.getState().error, null); + const query = new URL(requests.at(-1).url, "http://test.local").searchParams; + assert.equal(query.get("status"), "submitted"); + assert.equal(query.get("search"), "alice"); + assert.equal(query.get("sort_by"), "reject_votes"); + assert.equal(query.get("cursor"), null); +}); + +test("application and statistics refreshes ignore stale responses", async () => { + for (const [path, action, errorKey] of [ + [appPath, "fetchApplications", "error"], + [`${appPath}/stats`, "fetchStats", "statsError"], + ]) { + const older = deferred(); + handlers.set(path, () => older.promise); + const first = applications.getState()[action](); + handlers.delete(path); + await applications.getState()[action](); + older.resolve(failed()); + await first; + assert.equal(applications.getState()[errorKey], null); + } +}); + +test("a vote response from before a queue reset cannot change the new queue", async () => { + await grading.getState().fetchReviews(); + const vote = deferred(); + handlers.set("/v1/admin/reviews/r1", () => vote.promise); + const first = grading.getState().submitVote("r1", "accept"); + grading.getState().reset(); + await grading.getState().fetchReviews("r2"); + grading.getState().setLocalNotes("New session notes"); + vote.resolve(ok({})); + await first; + assert.equal(grading.getState().reviews.length, 3); + assert.equal(grading.getState().currentIndex, 1); + assert.equal(grading.getState().localNotes, "New session notes"); +}); + +test("completing the last review invalidates its outstanding detail request", async () => { + handlers.set(pendingPath, () => ok({ reviews: [reviews[0]] })); + const detail = deferred(); + handlers.set(`${appPath}/a1`, () => detail.promise); + const first = grading.getState().fetchReviews(); + await new Promise((r) => setImmediate(r)); + await grading.getState().submitVote("r1", "accept"); + detail.resolve(ok({ id: "a1" })); + await first; + assert.equal(grading.getState().reviews.length, 0); + assert.equal(grading.getState().detail, null); + assert.equal(grading.getState().detailLoading, false); +}); diff --git a/client/portal/src/pages/admin/all-applicants/components/ApplicationDetailPanel.tsx b/client/portal/src/pages/admin/all-applicants/components/ApplicationDetailPanel.tsx index 99de8ae2..571a9084 100644 --- a/client/portal/src/pages/admin/all-applicants/components/ApplicationDetailPanel.tsx +++ b/client/portal/src/pages/admin/all-applicants/components/ApplicationDetailPanel.tsx @@ -35,6 +35,8 @@ import { TimelineSection } from "./detail-sections/TimelineSection"; interface ApplicationDetailPanelProps { application: Application | null; loading: boolean; + error?: string | null; + onRetry?: () => void; open: boolean; onClose: () => void; onGrade?: () => void; @@ -47,6 +49,8 @@ interface ApplicationDetailPanelProps { export const ApplicationDetailPanel = memo(function ApplicationDetailPanel({ application, loading, + error, + onRetry, open, onClose, onGrade, @@ -159,6 +163,15 @@ export const ApplicationDetailPanel = memo(function ApplicationDetailPanel({
))}
+ ) : error ? ( +
+

{error}

+ {onRetry && ( + + )} +
) : application ? (
diff --git a/client/portal/src/pages/admin/all-applicants/createStore.ts b/client/portal/src/pages/admin/all-applicants/createStore.ts index 5a319f79..2ed6c10b 100644 --- a/client/portal/src/pages/admin/all-applicants/createStore.ts +++ b/client/portal/src/pages/admin/all-applicants/createStore.ts @@ -15,6 +15,7 @@ import type { export interface ApplicationsState { applications: ApplicationListItem[]; loading: boolean; + error: string | null; nextCursor: string | null; prevCursor: string | null; hasMore: boolean; @@ -23,6 +24,7 @@ export interface ApplicationsState { currentSortBy?: ApplicationSortBy; stats: ApplicationStats | null; statsLoading: boolean; + statsError: string | null; fetchApplications: ( params?: FetchParams, signal?: AbortSignal, @@ -38,9 +40,12 @@ interface ApplicationsStoreConfig { } export function createApplicationsStore(config: ApplicationsStoreConfig) { + let fetchSequence = 0; + let statsSequence = 0; return create((set, get) => ({ applications: [], loading: false, + error: null, nextCursor: null, prevCursor: null, hasMore: false, @@ -49,9 +54,11 @@ export function createApplicationsStore(config: ApplicationsStoreConfig) { currentSortBy: config.defaultSortBy, stats: null, statsLoading: false, + statsError: null, fetchApplications: async (params?: FetchParams, signal?: AbortSignal) => { - set({ loading: true }); + const requestId = ++fetchSequence; + set({ loading: true, error: null }); let status: ApplicationStatus | null; if (params && "status" in params && params.status !== undefined) { @@ -74,6 +81,13 @@ export function createApplicationsStore(config: ApplicationsStoreConfig) { sortBy = get().currentSortBy; } + // Remember the requested view immediately so retries and assignment + // refreshes keep filters even while another fetch is pending. + set({ + currentStatus: status, + currentSearch: search, + currentSortBy: sortBy, + }); const res = await apiFetchApplications( { ...params, @@ -84,7 +98,11 @@ export function createApplicationsStore(config: ApplicationsStoreConfig) { signal, ); - if (signal?.aborted) return; + if (requestId !== fetchSequence) return; + if (signal?.aborted) { + set({ loading: false }); + return; + } if (res.status === 200 && res.data) { set({ @@ -99,6 +117,7 @@ export function createApplicationsStore(config: ApplicationsStoreConfig) { }); } else { set({ + error: res.error || "Unable to load applications. Please try again.", applications: [], nextCursor: null, prevCursor: null, @@ -109,16 +128,25 @@ export function createApplicationsStore(config: ApplicationsStoreConfig) { }, fetchStats: async (signal?: AbortSignal) => { - set({ statsLoading: true }); + const requestId = ++statsSequence; + set({ statsLoading: true, statsError: null }); const res = await fetchApplicationStats(signal); - if (signal?.aborted) return; + if (requestId !== statsSequence) return; + if (signal?.aborted) { + set({ statsLoading: false }); + return; + } if (res.status === 200 && res.data) { set({ stats: res.data, statsLoading: false }); } else { - set({ stats: null, statsLoading: false }); + set({ + stats: null, + statsLoading: false, + statsError: res.error || "Unable to load application statistics.", + }); } }, @@ -127,7 +155,10 @@ export function createApplicationsStore(config: ApplicationsStoreConfig) { }, resetPagination: () => { + ++fetchSequence; set({ + error: null, + loading: false, applications: [], nextCursor: null, prevCursor: null, diff --git a/client/portal/src/pages/admin/all-applicants/hooks/useApplicationDetail.ts b/client/portal/src/pages/admin/all-applicants/hooks/useApplicationDetail.ts index 92b60e72..12980623 100644 --- a/client/portal/src/pages/admin/all-applicants/hooks/useApplicationDetail.ts +++ b/client/portal/src/pages/admin/all-applicants/hooks/useApplicationDetail.ts @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useCallback, useEffect, useState } from "react"; import { errorAlert, getRequest } from "@/shared/lib/api"; import type { Application } from "@/types"; @@ -7,6 +7,8 @@ interface UseApplicationDetailResult { detail: Application | null; loading: boolean; clear: () => void; + refresh: () => void; + error: string | null; } export function useApplicationDetail( @@ -14,6 +16,9 @@ export function useApplicationDetail( ): UseApplicationDetailResult { const [detail, setDetail] = useState(null); const [loading, setLoading] = useState(false); + const [error, setError] = useState(null); + const [refreshKey, setRefreshKey] = useState(0); + const refresh = useCallback(() => setRefreshKey((key) => key + 1), []); useEffect(() => { if (!applicationId) { @@ -24,6 +29,8 @@ export function useApplicationDetail( (async () => { setLoading(true); + setDetail(null); + setError(null); const res = await getRequest( `/admin/applications/${applicationId}`, "application", @@ -34,6 +41,7 @@ export function useApplicationDetail( if (res.status === 200 && res.data) { setDetail(res.data); } else { + setError(res.error || "Unable to load application details."); errorAlert(res); } setLoading(false); @@ -42,11 +50,12 @@ export function useApplicationDetail( return () => { controller.abort(); }; - }, [applicationId]); + }, [applicationId, refreshKey]); - const clear = () => { + const clear = useCallback(() => { setDetail(null); - }; + setError(null); + }, []); - return { detail, loading, clear }; + return { detail, loading, clear, refresh, error }; } diff --git a/client/portal/src/pages/admin/reviews/ReviewsPage.tsx b/client/portal/src/pages/admin/reviews/ReviewsPage.tsx index a5d21d4c..e4400ea7 100644 --- a/client/portal/src/pages/admin/reviews/ReviewsPage.tsx +++ b/client/portal/src/pages/admin/reviews/ReviewsPage.tsx @@ -45,7 +45,8 @@ import type { ReviewNote } from "./types"; export default function ReviewsPage() { const navigate = useNavigate(); - const { tab, reviews, loading, setTab, fetchReviews } = useReviewsStore(); + const { tab, reviews, loading, error, setTab, fetchReviews } = + useReviewsStore(); const refreshKey = refreshAssignedPage((state) => state.refreshKey); const [selectedId, setSelectedId] = useState(null); @@ -214,12 +215,13 @@ export default function ReviewsPage() { }, [tab, selectedId]); // --- Descriptions --- - const description = - tab === "assigned" ? ( - <>{filteredReviews.length} review(s) assigned to you - ) : ( - <>{filteredReviews.length} completed review(s) - ); + const description = error ? ( + <>Unable to load reviews + ) : tab === "assigned" ? ( + <>{filteredReviews.length} review(s) assigned to you + ) : ( + <>{filteredReviews.length} completed review(s) + ); // --- Header actions --- const headerActions = @@ -250,7 +252,14 @@ export default function ReviewsPage() { ) : undefined; // --- Table --- - const table = ( + const table = error ? ( +
+

{error}

+ +
+ ) : ( s.reviews); const loading = useAdminGradingStore((s) => s.loading); + const error = useAdminGradingStore((s) => s.error); const currentIndex = useAdminGradingStore((s) => s.currentIndex); const detail = useAdminGradingStore((s) => s.detail); const detailLoading = useAdminGradingStore((s) => s.detailLoading); @@ -32,7 +33,6 @@ export default function GradingPage() { const localNotes = useAdminGradingStore((s) => s.localNotes); const localTravelVote = useAdminGradingStore((s) => s.localTravelVote); const fetchReviews = useAdminGradingStore((s) => s.fetchReviews); - const loadDetail = useAdminGradingStore((s) => s.loadDetail); const navigateNext = useAdminGradingStore((s) => s.navigateNext); const navigatePrev = useAdminGradingStore((s) => s.navigatePrev); const submitVote = useAdminGradingStore((s) => s.submitVote); @@ -40,38 +40,39 @@ export default function GradingPage() { const setLocalTravelVote = useAdminGradingStore((s) => s.setLocalTravelVote); const reset = useAdminGradingStore((s) => s.reset); - const [aiPercent, setAiPercent] = useState(null); + const aiPercent = detail?.ai_percent ?? null; + const setAiPercent = (percent: number) => { + useAdminGradingStore.setState((state) => ({ + detail: + state.detail && state.detail.id === detail?.id + ? { ...state.detail, ai_percent: percent } + : state.detail, + })); + }; const redact = useRedactApplicants(); const currentReview = reviews[currentIndex] ?? null; - // Initialize + const targetReviewId = searchParams.get("review") ?? undefined; useEffect(() => { - const targetReviewId = searchParams.get("review"); - + const controller = new AbortController(); reset(); - fetchReviews().then(() => { - const revs = useAdminGradingStore.getState().reviews; - if (revs.length > 0) { - const targetIndex = targetReviewId - ? revs.findIndex((r) => r.id === targetReviewId) - : -1; - const idx = targetIndex >= 0 ? targetIndex : 0; - useAdminGradingStore.setState({ currentIndex: idx }); - loadDetail(revs[idx].application_id); - } - }); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); - - // Sync AI percent from detail - useEffect(() => { - setAiPercent(detail?.ai_percent ?? null); - }, [detail]); + void fetchReviews(targetReviewId, controller.signal); + return () => { + controller.abort(); + reset(); + }; + }, [fetchReviews, reset, targetReviewId]); const handleVote = useCallback( (vote: ReviewVote) => { - if (currentReview && !submitting && !currentReview.vote) { + if ( + currentReview && + !loading && + !error && + !submitting && + !currentReview.vote + ) { // A travel yes/no is required when the applicant requested travel if ( currentReview.travel_status !== "not_requested" && @@ -82,11 +83,11 @@ export default function GradingPage() { submitVote(currentReview.id, vote); } }, - [currentReview, submitting, submitVote, localTravelVote], + [currentReview, loading, error, submitting, submitVote, localTravelVote], ); useGradingKeyboardShortcuts({ - disabled: submitting, + disabled: submitting || loading || !!error, canAct: !!currentReview?.id && !currentReview?.vote, escapeUrl: "/admin/reviews", onNavigateNext: navigateNext, @@ -120,8 +121,10 @@ export default function GradingPage() { totalCount={reviews.length} onNavigateNext={navigateNext} onNavigatePrev={navigatePrev} - canNavigatePrev={!loading && currentIndex > 0} - canNavigateNext={!loading && currentIndex < reviews.length - 1} + canNavigatePrev={!loading && !error && !submitting && currentIndex > 0} + canNavigateNext={ + !loading && !error && !submitting && currentIndex < reviews.length - 1 + } detailsPanel={ {currentReview && ( @@ -162,7 +165,17 @@ export default function GradingPage() { } emptyState={
-

No pending reviews to grade.

+

+ {error || "No pending reviews to grade."} +

+ {error && ( + + )} - {reviewsPerApp} + {reviewsPerApp ?? "—"}

- Reviews needed before a decision + Assignment target per application. Run Assign Reviews after + changing it.

@@ -325,7 +442,13 @@ export default function ReviewsPage() {
@@ -346,7 +469,15 @@ export default function ReviewsPage() {
Assign + + )} + {lastBatch && ( +
+

+ Last assignment run (target {lastBatch.reviews_per_application}):{" "} + {lastBatch.reviews_created} created; {lastBatch.reviews_removed}{" "} + assignments released from unavailable reviewers. +

+ {lastBatch.applications_below_target > 0 && ( +

+ {lastBatch.applications_below_target} submitted application + {lastBatch.applications_below_target === 1 ? "" : "s"} still{" "} + {lastBatch.applications_below_target === 1 ? "needs" : "need"}{" "} + {lastBatch.reviews_unfilled} assignment + {lastBatch.reviews_unfilled === 1 ? "" : "s"} because no + additional distinct eligible reviewers are available. +

+ )} +
+ )} + {(tableError || statsError) && ( +
+

{tableError || statsError}

+ +
+ )} + {/* Applications Table Section */}
@@ -441,14 +624,17 @@ export default function ReviewsPage() {
- + {!tableError && ( + + )}
@@ -456,6 +642,8 @@ export default function ReviewsPage() { 0} @@ -481,13 +669,39 @@ export default function ReviewsPage() { stats={stats} /> + + + + Change Reviews Per Application? + + Are you sure you want to change reviews per application from{" "} + {reviewsPerApp} to {pendingReviewsPerApp}? Run Assign Reviews + after saving to apply the new target. + + + + + Cancel + + + Yes, Change Count + + + + + Confirm Batch Assignment - This will assign admin reviewers to all submitted applications - that still need reviews. Are you sure you want to proceed? + This will fill submitted applications toward {reviewsPerApp}{" "} + distinct reviewer assignments each and reroute pending work from + unavailable reviewers. Existing completed reviews are preserved. @@ -496,6 +710,7 @@ export default function ReviewsPage() { Yes, Assign Reviews diff --git a/client/portal/src/pages/superadmin/reviews/api.ts b/client/portal/src/pages/superadmin/reviews/api.ts index 54f5174b..e301fae5 100644 --- a/client/portal/src/pages/superadmin/reviews/api.ts +++ b/client/portal/src/pages/superadmin/reviews/api.ts @@ -40,3 +40,12 @@ export async function sendDecisionEmails(payload: SendDecisionEmailsPayload) { "send decision emails", ); } + +// Additive response from POST /superadmin/applications/assign. +export interface BatchAssignmentResult { + reviews_created: number; + reviews_removed: number; + reviews_per_application: number; + applications_below_target: number; + reviews_unfilled: number; +} diff --git a/client/portal/src/pages/superadmin/reviews/components/ReviewsTable.tsx b/client/portal/src/pages/superadmin/reviews/components/ReviewsTable.tsx index ff9d5cdf..7a8de9e2 100644 --- a/client/portal/src/pages/superadmin/reviews/components/ReviewsTable.tsx +++ b/client/portal/src/pages/superadmin/reviews/components/ReviewsTable.tsx @@ -18,6 +18,7 @@ import type { import { formatName, getStatusColor } from "@/pages/admin/all-applicants/utils"; interface ReviewsTableProps { + reviewsPerApp: number | null; applications: ApplicationListItem[]; loading: boolean; selectedId: string | null; @@ -36,6 +37,7 @@ const SORTABLE_COLUMNS: { key: SortableColumn; label: string }[] = [ ]; export const ReviewsTable = memo(function ReviewsTable({ + reviewsPerApp, applications, loading, selectedId, @@ -144,7 +146,17 @@ export const ReviewsTable = memo(function ReviewsTable({ )} - {app.reviews_completed}/{app.reviews_assigned} +
+ {app.reviews_completed} completed · {app.reviews_assigned}{" "} + assigned +
+ {app.status === "submitted" && reviewsPerApp !== null && ( +
+ Target {reviewsPerApp} + {app.reviews_assigned < reviewsPerApp && + ` · ${reviewsPerApp - app.reviews_assigned} assignment${reviewsPerApp - app.reviews_assigned === 1 ? "" : "s"} missing`} +
+ )} {app.ai_percent != null ? `${app.ai_percent}%` : "-"} diff --git a/cmd/api/reviews_test.go b/cmd/api/reviews_test.go index 0d351f60..f85f9b2a 100644 --- a/cmd/api/reviews_test.go +++ b/cmd/api/reviews_test.go @@ -349,7 +349,7 @@ func TestBatchAssignReviews(t *testing.T) { mockSettings := app.store.Settings.(*store.MockSettingsStore) t.Run("should batch assign reviews", func(t *testing.T) { - result := &store.BatchAssignmentResult{ReviewsCreated: 15} + result := &store.BatchAssignmentResult{ReviewsCreated: 15, ReviewsRemoved: 2, ReviewsPerApplication: 3, ApplicationsBelowTarget: 1, ReviewsUnfilled: 2} mockSettings.On("GetReviewsPerApplication").Return(3, nil).Once() mockReviews.On("BatchAssign", 3).Return(result, nil).Once() @@ -366,7 +366,7 @@ func TestBatchAssignReviews(t *testing.T) { } err = json.NewDecoder(rr.Body).Decode(&body) require.NoError(t, err) - assert.Equal(t, 15, body.Data.ReviewsCreated) + assert.Equal(t, *result, body.Data) mockReviews.AssertExpectations(t) mockSettings.AssertExpectations(t) diff --git a/cmd/api/schemafields_test.go b/cmd/api/schemafields_test.go index 70d82624..2134078d 100644 --- a/cmd/api/schemafields_test.go +++ b/cmd/api/schemafields_test.go @@ -154,3 +154,28 @@ func TestSchemaContractFieldID(t *testing.T) { assert.Equal(t, travelOptInFieldID, schemaContractFieldID(fields, travelOptInFieldID)) assert.Empty(t, schemaContractFieldID(nil, travelOptInFieldID)) } + +// The reset script writes the shipped defaults straight to the settings table, +// bypassing the editors that normally enforce these bindings — so the defaults +// themselves have to satisfy them. +func TestDefaultSchemasSatisfyContracts(t *testing.T) { + tests := []struct { + key string + contracts []SchemaFieldContract + }{ + {store.SettingsKeyApplicationSchema, applicationSchemaContracts}, + {store.SettingsKeyRSVPSchema, nil}, + {store.SettingsKeyTravelRSVPSchema, travelRSVPSchemaContracts}, + } + + for _, tt := range tests { + t.Run(tt.key, func(t *testing.T) { + fields, err := store.DefaultFormSchemaFields(tt.key) + require.NoError(t, err) + + warnings, err := validateSchemaFields(tt.contracts, fields) + require.NoError(t, err) + assert.Empty(t, warnings) + }) + } +} diff --git a/cmd/api/settings.go b/cmd/api/settings.go index eeee9517..cb041c83 100644 --- a/cmd/api/settings.go +++ b/cmd/api/settings.go @@ -433,10 +433,10 @@ func (app *application) getReviewsPerApp(w http.ResponseWriter, r *http.Request) } } -// setReviewsPerApp sets the number of reviews required per application +// setReviewsPerApp sets the assignment target per application // // @Summary Set reviews per application (Super Admin) -// @Description Sets the number of reviews required per application +// @Description Sets the reviewer assignment target per application; run batch assignment to fill it // @Tags superadmin/settings // @Accept json // @Produce json diff --git a/cmd/resetschema/main.go b/cmd/resetschema/main.go new file mode 100644 index 00000000..a2381fed --- /dev/null +++ b/cmd/resetschema/main.go @@ -0,0 +1,232 @@ +// Command resetschema restores the editable form schemas to the defaults HARP +// ships with, for when a super admin has edited a form into a state that is +// easier to start over from than to repair. +// +// It writes settings rows only. Applications, reviews and uploads are left +// untouched: responses stored against a field the default schema does not +// declare stay in the database, they simply stop being rendered. +// +// Usage: +// +// DB_ADDR=... go run ./cmd/resetschema # application form +// DB_ADDR=... go run ./cmd/resetschema -forms=rsvp,travel-rsvp +// DB_ADDR=... go run ./cmd/resetschema -all -dry-run +package main + +import ( + "bufio" + "context" + "database/sql" + "encoding/json" + "errors" + "flag" + "fmt" + "log" + "os" + "sort" + "strings" + + "github.com/hackutd/harp/internal/db" + "github.com/hackutd/harp/internal/env" + "github.com/hackutd/harp/internal/store" +) + +// formNames maps the -forms values an operator types to settings keys. +var formNames = map[string]string{ + "application": store.SettingsKeyApplicationSchema, + "rsvp": store.SettingsKeyRSVPSchema, + "travel-rsvp": store.SettingsKeyTravelRSVPSchema, +} + +func main() { + forms := flag.String("forms", "application", "comma-separated forms to reset: application, rsvp, travel-rsvp") + all := flag.Bool("all", false, "reset every form schema, ignoring -forms") + dryRun := flag.Bool("dry-run", false, "report what would change without writing") + assumeYes := flag.Bool("y", false, "skip the confirmation prompt") + flag.Parse() + + keys, err := selectKeys(*forms, *all) + if err != nil { + log.Fatal(err) + } + + conn, err := db.New(env.GetRequiredString("DB_ADDR"), 25, 25, "15m") + if err != nil { + log.Fatal(err) + } + defer conn.Close() + + ctx := context.Background() + storage := store.NewStorage(conn) + + plan, err := buildPlan(ctx, conn, storage, keys) + if err != nil { + log.Fatal(err) + } + + fmt.Print(plan) + + if *dryRun { + fmt.Println("dry run: nothing written") + return + } + + if !*assumeYes && !confirm() { + fmt.Println("aborted") + return + } + + for _, key := range keys { + if err := storage.Settings.RestoreDefaultFormSchema(ctx, key); err != nil { + log.Fatalf("failed to restore %s: %v", key, err) + } + fmt.Printf("restored %s\n", key) + } + + // Running API instances cache settings in process, so a reset reaches them + // on their next read rather than immediately. No restart is needed. + fmt.Println("done — running servers pick this up within their settings cache TTL (10s)") +} + +// selectKeys turns the -forms/-all flags into settings keys, preserving the +// order the store declares them in. +func selectKeys(forms string, all bool) ([]string, error) { + if all { + return store.FormSchemaKeys, nil + } + + wanted := make(map[string]bool) + for _, name := range strings.Split(forms, ",") { + name = strings.TrimSpace(name) + if name == "" { + continue + } + + key, ok := formNames[name] + if !ok { + return nil, fmt.Errorf("unknown form %q: expected one of application, rsvp, travel-rsvp", name) + } + wanted[key] = true + } + + if len(wanted) == 0 { + return nil, fmt.Errorf("no forms selected") + } + + keys := make([]string, 0, len(wanted)) + for _, key := range store.FormSchemaKeys { + if wanted[key] { + keys = append(keys, key) + } + } + + return keys, nil +} + +// buildPlan renders what the reset would change, so the operator confirms +// against real numbers rather than the flag they typed. +func buildPlan(ctx context.Context, conn *sql.DB, storage store.Storage, keys []string) (string, error) { + var b strings.Builder + + for _, key := range keys { + current, err := currentFields(ctx, conn, key) + if err != nil { + return "", err + } + + defaults, err := store.DefaultFormSchemaFields(key) + if err != nil { + return "", err + } + + added, removed := diffFieldIDs(current, defaults) + + fmt.Fprintf(&b, "%s: %d field(s) now -> %d default field(s)\n", key, len(current), len(defaults)) + if len(removed) > 0 { + fmt.Fprintf(&b, " dropped: %s\n", strings.Join(removed, ", ")) + } + if len(added) > 0 { + fmt.Fprintf(&b, " added: %s\n", strings.Join(added, ", ")) + } + if len(added) == 0 && len(removed) == 0 { + fmt.Fprintf(&b, " same field ids; labels, options and ordering are still rewritten\n") + } + } + + count, err := applicationCount(ctx, conn) + if err != nil { + return "", err + } + if count > 0 { + fmt.Fprintf(&b, "\n%d application row(s) exist. Answers to dropped fields stay in the\n"+ + "database but disappear from the forms and from review.\n", count) + } + + return b.String(), nil +} + +// currentFields reads a schema straight from the settings table, bypassing the +// store's cache and its empty-on-missing default so a missing row is visible. +func currentFields(ctx context.Context, conn *sql.DB, key string) ([]store.ApplicationSchemaField, error) { + var raw []byte + err := conn.QueryRowContext(ctx, `SELECT value FROM settings WHERE key = $1`, key).Scan(&raw) + if errors.Is(err, sql.ErrNoRows) { + return nil, nil + } + if err != nil { + return nil, err + } + + var fields []store.ApplicationSchemaField + if err := json.Unmarshal(raw, &fields); err != nil { + return nil, fmt.Errorf("stored %s is not a field array: %w", key, err) + } + + return fields, nil +} + +func applicationCount(ctx context.Context, conn *sql.DB) (int, error) { + var count int + err := conn.QueryRowContext(ctx, `SELECT COUNT(*) FROM applications`).Scan(&count) + return count, err +} + +// diffFieldIDs reports the field ids the default adds and the ones it drops. +func diffFieldIDs(current, defaults []store.ApplicationSchemaField) (added, removed []string) { + currentIDs := idSet(current) + defaultIDs := idSet(defaults) + + for id := range defaultIDs { + if !currentIDs[id] { + added = append(added, id) + } + } + for id := range currentIDs { + if !defaultIDs[id] { + removed = append(removed, id) + } + } + + sort.Strings(added) + sort.Strings(removed) + return added, removed +} + +func idSet(fields []store.ApplicationSchemaField) map[string]bool { + ids := make(map[string]bool, len(fields)) + for _, f := range fields { + ids[f.ID] = true + } + return ids +} + +func confirm() bool { + fmt.Print("\nOverwrite the schema(s) above with the shipped defaults? [y/N]: ") + + answer, err := bufio.NewReader(os.Stdin).ReadString('\n') + if err != nil { + return false + } + + return strings.EqualFold(strings.TrimSpace(answer), "y") +} diff --git a/docs/docs.go b/docs/docs.go index 584ae2a8..7bbba52d 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -8200,7 +8200,7 @@ const docTemplate = `{ "CookieAuth": [] } ], - "description": "Sets the number of reviews required per application", + "description": "Sets the reviewer assignment target per application; run batch assignment to fill it", "consumes": [ "application/json" ], @@ -11796,8 +11796,20 @@ const docTemplate = `{ "store.BatchAssignmentResult": { "type": "object", "properties": { + "applications_below_target": { + "type": "integer" + }, "reviews_created": { "type": "integer" + }, + "reviews_per_application": { + "type": "integer" + }, + "reviews_removed": { + "type": "integer" + }, + "reviews_unfilled": { + "type": "integer" } } }, diff --git a/internal/store/default_schemas.go b/internal/store/default_schemas.go new file mode 100644 index 00000000..84818f6c --- /dev/null +++ b/internal/store/default_schemas.go @@ -0,0 +1,89 @@ +package store + +import ( + "context" + "embed" + "encoding/json" + "fmt" +) + +// defaultSchemaFS holds the shipped form schemas as they are seeded by +// migrations 000006 (application, plus the hackathons_attended bound added by +// 000027), 000035 (RSVP) and 000040 (travel RSVP). They are duplicated here +// because a migration only ever runs once: restoring a schema a super admin has +// since edited needs the default available at runtime. TestDefaultSchemasMatch +// Migrations guards the copy against drift. +// +//go:embed defaults/*.json +var defaultSchemaFS embed.FS + +// defaultSchemaFiles maps each editable form schema setting to its default. +var defaultSchemaFiles = map[string]string{ + SettingsKeyApplicationSchema: "defaults/application_schema.json", + SettingsKeyRSVPSchema: "defaults/rsvp_schema.json", + SettingsKeyTravelRSVPSchema: "defaults/travel_rsvp_schema.json", +} + +// FormSchemaKeys lists the settings keys that hold an editable form schema, in +// the order an operator thinks about them. +var FormSchemaKeys = []string{ + SettingsKeyApplicationSchema, + SettingsKeyRSVPSchema, + SettingsKeyTravelRSVPSchema, +} + +// DefaultFormSchema returns the shipped JSON for a form schema setting. +func DefaultFormSchema(key string) (json.RawMessage, error) { + name, ok := defaultSchemaFiles[key] + if !ok { + return nil, fmt.Errorf("no default schema for settings key %q", key) + } + + raw, err := defaultSchemaFS.ReadFile(name) + if err != nil { + return nil, err + } + + return json.RawMessage(raw), nil +} + +// DefaultFormSchemaFields returns the shipped fields for a form schema setting. +func DefaultFormSchemaFields(key string) ([]ApplicationSchemaField, error) { + raw, err := DefaultFormSchema(key) + if err != nil { + return nil, err + } + + var fields []ApplicationSchemaField + if err := json.Unmarshal(raw, &fields); err != nil { + return nil, fmt.Errorf("parsing default %s: %w", key, err) + } + + return fields, nil +} + +// RestoreDefaultFormSchema replaces a form schema setting with the shipped +// default. Responses already stored against removed field ids are left in +// place: they stay in the applications table, simply unreferenced by the form. +func (s *SettingsStore) RestoreDefaultFormSchema(ctx context.Context, key string) error { + raw, err := DefaultFormSchema(key) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) + defer cancel() + + query := ` + INSERT INTO settings (key, value) + VALUES ($1, $2::jsonb) + ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value, updated_at = NOW() + ` + + if _, err := s.db.ExecContext(ctx, query, key, string(raw)); err != nil { + return err + } + + s.invalidate(key) + return nil +} diff --git a/internal/store/default_schemas_test.go b/internal/store/default_schemas_test.go new file mode 100644 index 00000000..c904d71b --- /dev/null +++ b/internal/store/default_schemas_test.go @@ -0,0 +1,117 @@ +package store + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +// The embedded defaults are a second copy of what the seed migrations write, so +// they only stay correct while nobody edits one side alone. These read the +// migrations back and compare. + +func TestDefaultFormSchemasParse(t *testing.T) { + for _, key := range FormSchemaKeys { + fields, err := DefaultFormSchemaFields(key) + if err != nil { + t.Fatalf("%s: %v", key, err) + } + if len(fields) == 0 { + t.Fatalf("%s: default schema is empty", key) + } + + seen := make(map[string]bool, len(fields)) + for _, f := range fields { + if f.ID == "" || f.Type == "" || f.Label == "" { + t.Errorf("%s: field %+v is missing id, type or label", key, f) + } + if seen[f.ID] { + t.Errorf("%s: duplicate field id %q", key, f.ID) + } + seen[f.ID] = true + } + } +} + +func TestDefaultFormSchemasMatchMigrations(t *testing.T) { + tests := []struct { + key string + migration string + // patch applies the later migrations that amended the seeded schema in + // place, so the comparison is against the current default rather than + // the original one. + patch func([]ApplicationSchemaField) + }{ + { + key: SettingsKeyApplicationSchema, + migration: "000006_seed_settings.up.sql", + // 000027_alter_settings_add_hackathons_attended_max + patch: func(fields []ApplicationSchemaField) { + for i := range fields { + if fields[i].ID == "hackathons_attended" { + fields[i].Validation["max"] = float64(100) + } + } + }, + }, + {key: SettingsKeyRSVPSchema, migration: "000035_seed_rsvp_schema.up.sql"}, + {key: SettingsKeyTravelRSVPSchema, migration: "000040_seed_travel_rsvp_schema.up.sql"}, + } + + for _, tt := range tests { + t.Run(tt.key, func(t *testing.T) { + seeded, err := seededSchema(tt.migration, tt.key) + if err != nil { + t.Fatal(err) + } + if tt.patch != nil { + tt.patch(seeded) + } + + embedded, err := DefaultFormSchemaFields(tt.key) + if err != nil { + t.Fatal(err) + } + + if !reflect.DeepEqual(seeded, embedded) { + t.Errorf("internal/store/defaults/%s.json has drifted from %s.\n"+ + "Update the JSON file to match the migration (or add the amending "+ + "migration to this test's patch).", tt.key, tt.migration) + } + }) + } +} + +// seededSchema pulls a schema out of the JSONB literal a seed migration +// inserts, undoing SQL's doubled single quotes. +func seededSchema(migration, key string) ([]ApplicationSchemaField, error) { + path := filepath.Join("..", "..", "cmd", "migrate", "migrations", migration) + sql, err := os.ReadFile(path) + if err != nil { + return nil, err + } + + marker := fmt.Sprintf("VALUES ('%s', '", key) + start := strings.Index(string(sql), marker) + if start < 0 { + return nil, fmt.Errorf("%s does not insert %s", migration, key) + } + start += len(marker) + + rest := string(sql)[start:] + end := strings.Index(rest, "'::jsonb") + if end < 0 { + return nil, fmt.Errorf("%s: unterminated JSONB literal for %s", migration, key) + } + + var fields []ApplicationSchemaField + if err := json.Unmarshal([]byte(strings.ReplaceAll(rest[:end], "''", "'")), &fields); err != nil { + return nil, fmt.Errorf("%s: %w", migration, err) + } + + return fields, nil +} diff --git a/internal/store/defaults/application_schema.json b/internal/store/defaults/application_schema.json new file mode 100644 index 00000000..ad4a37ee --- /dev/null +++ b/internal/store/defaults/application_schema.json @@ -0,0 +1,376 @@ +[ + { + "id": "first_name", + "type": "text", + "label": "First Name", + "required": true, + "section": "personal", + "display_order": 1 + }, + { + "id": "last_name", + "type": "text", + "label": "Last Name", + "required": true, + "section": "personal", + "display_order": 2 + }, + { + "id": "phone", + "type": "phone", + "label": "Phone Number", + "required": false, + "section": "personal", + "display_order": 3 + }, + { + "id": "age", + "type": "number", + "label": "Age", + "required": true, + "section": "personal", + "display_order": 4, + "validation": { + "min": 0, + "max": 120 + } + }, + { + "id": "country_of_residence", + "type": "text", + "label": "Country of Residence", + "required": false, + "section": "personal", + "display_order": 5 + }, + { + "id": "gender", + "type": "text", + "label": "Gender", + "required": false, + "section": "personal", + "display_order": 6 + }, + { + "id": "race", + "type": "text", + "label": "Race", + "required": false, + "section": "personal", + "display_order": 7 + }, + { + "id": "ethnicity", + "type": "text", + "label": "Ethnicity", + "required": false, + "section": "personal", + "display_order": 8 + }, + { + "id": "university", + "type": "text", + "label": "University", + "required": true, + "section": "education", + "display_order": 10 + }, + { + "id": "major", + "type": "text", + "label": "Major", + "required": true, + "section": "education", + "display_order": 11 + }, + { + "id": "level_of_study", + "type": "select", + "label": "Level of Study", + "required": true, + "section": "education", + "display_order": 12, + "options": [ + "Freshman", + "Sophomore", + "Junior", + "Senior", + "Graduate", + "PhD", + "Other" + ] + }, + { + "id": "github", + "type": "text", + "label": "GitHub", + "required": false, + "section": "links", + "display_order": 20 + }, + { + "id": "linkedin", + "type": "text", + "label": "LinkedIn", + "required": false, + "section": "links", + "display_order": 21 + }, + { + "id": "website", + "type": "text", + "label": "Personal Website", + "required": false, + "section": "links", + "display_order": 22 + }, + { + "id": "hackathons_attended", + "type": "number", + "label": "Hackathons Attended", + "required": false, + "section": "experience", + "display_order": 30, + "validation": { + "min": 0, + "max": 100 + } + }, + { + "id": "experience_level", + "type": "select", + "label": "Software Experience", + "required": false, + "section": "experience", + "display_order": 31, + "options": [ + "Beginner", + "Intermediate", + "Advanced", + "Expert" + ] + }, + { + "id": "heard_about", + "type": "text", + "label": "How did you hear about us?", + "required": false, + "section": "experience", + "display_order": 32 + }, + { + "id": "saq_1", + "type": "textarea", + "label": "Why do you want to attend this hackathon?", + "required": true, + "section": "short_answers", + "display_order": 40, + "validation": { + "maxLength": 1000 + } + }, + { + "id": "saq_2", + "type": "textarea", + "label": "How many hackathons have you submitted to and what did you learn from them?", + "required": true, + "section": "short_answers", + "display_order": 41, + "validation": { + "maxLength": 1000 + } + }, + { + "id": "saq_3", + "type": "textarea", + "label": "If you haven't been to a hackathon, what do you hope to learn from this hackathon?", + "required": true, + "section": "short_answers", + "display_order": 42, + "validation": { + "maxLength": 1000 + } + }, + { + "id": "saq_4", + "type": "textarea", + "label": "What are you looking forward to do at this hackathon?", + "required": true, + "section": "short_answers", + "display_order": 43, + "validation": { + "maxLength": 1000 + } + }, + { + "id": "shirt_size", + "type": "select", + "label": "Shirt Size", + "required": false, + "section": "logistics", + "display_order": 50, + "options": [ + "XS", + "S", + "M", + "L", + "XL", + "XXL" + ] + }, + { + "id": "dietary_restrictions", + "type": "multi_select", + "label": "Dietary Restrictions", + "required": false, + "section": "logistics", + "display_order": 51, + "options": [ + "Vegan", + "Vegetarian", + "Halal", + "Nuts", + "Fish", + "Wheat", + "Dairy", + "Eggs", + "No Beef", + "No Pork" + ] + }, + { + "id": "accommodations", + "type": "textarea", + "label": "Accommodations", + "required": false, + "section": "logistics", + "display_order": 52 + }, + { + "id": "travel_reimbursement", + "type": "checkbox", + "label": "I would like to apply for travel reimbursement", + "required": false, + "section": "travel", + "section_label": "Travel Reimbursement", + "display_order": 53 + }, + { + "id": "travel_origin", + "type": "text", + "label": "Where will you be traveling from? (City, State/Country)", + "required": false, + "section": "travel", + "section_label": "Travel Reimbursement", + "display_order": 54, + "validation": { + "show_if": "travel_reimbursement", + "required_if": "travel_reimbursement" + } + }, + { + "id": "travel_mode", + "type": "select", + "label": "How do you plan to travel?", + "required": false, + "section": "travel", + "section_label": "Travel Reimbursement", + "display_order": 55, + "options": [ + "Car", + "Bus", + "Train", + "Flight", + "Other" + ], + "validation": { + "show_if": "travel_reimbursement", + "required_if": "travel_reimbursement" + } + }, + { + "id": "travel_estimated_cost", + "type": "number", + "label": "Estimated round-trip travel cost (USD)", + "required": false, + "section": "travel", + "section_label": "Travel Reimbursement", + "display_order": 56, + "validation": { + "min": 0, + "show_if": "travel_reimbursement", + "required_if": "travel_reimbursement" + } + }, + { + "id": "travel_has_team", + "type": "select", + "label": "Do you already have a team for the event?", + "required": false, + "section": "travel", + "section_label": "Travel Reimbursement", + "display_order": 57, + "options": [ + "Yes", + "No" + ], + "validation": { + "show_if": "travel_reimbursement", + "required_if": "travel_reimbursement" + } + }, + { + "id": "travel_justification", + "type": "textarea", + "label": "Why do you need travel reimbursement to attend?", + "required": false, + "section": "travel", + "section_label": "Travel Reimbursement", + "display_order": 58, + "validation": { + "maxLength": 1000, + "show_if": "travel_reimbursement", + "required_if": "travel_reimbursement" + } + }, + { + "id": "ack_mlh_coc", + "type": "checkbox", + "label": "I have read and agree to the [MLH Code of Conduct](https://mlh.io/code-of-conduct)", + "required": true, + "section": "agreements", + "display_order": 60 + }, + { + "id": "ack_mlh_data_sharing", + "type": "checkbox", + "label": "I authorize sharing my application/registration information with Major League Hacking for event administration, ranking, and MLH administration in-line with the [MLH Privacy Policy](https://mlh.io/privacy).", + "required": true, + "section": "agreements", + "display_order": 61 + }, + { + "id": "ack_mlh_contest_terms", + "type": "checkbox", + "label": "I agree to the [MLH Contest Terms and Conditions](https://github.com/MLH/mlh-policies/blob/main/contest-terms.md).", + "required": true, + "section": "agreements", + "display_order": 62 + }, + { + "id": "ack_mlh_privacy_policy", + "type": "checkbox", + "label": "I agree to the [MLH Privacy Policy](https://mlh.io/privacy).", + "required": true, + "section": "agreements", + "display_order": 63 + }, + { + "id": "opt_in_mlh_emails", + "type": "checkbox", + "label": "I authorize MLH to send me occasional emails about relevant events, career opportunities, and community announcements", + "required": false, + "section": "agreements", + "display_order": 64 + } +] diff --git a/internal/store/defaults/rsvp_schema.json b/internal/store/defaults/rsvp_schema.json new file mode 100644 index 00000000..a3de520a --- /dev/null +++ b/internal/store/defaults/rsvp_schema.json @@ -0,0 +1,55 @@ +[ + { + "id": "discord_username", + "type": "text", + "label": "Discord Username", + "required": true, + "section": "rsvp", + "section_label": "RSVP Details", + "section_order": 1, + "display_order": 1 + }, + { + "id": "emergency_contact_name", + "type": "text", + "label": "Emergency Contact Name", + "required": true, + "section": "rsvp", + "section_label": "RSVP Details", + "section_order": 1, + "display_order": 2 + }, + { + "id": "emergency_contact_phone", + "type": "phone", + "label": "Emergency Contact Phone", + "required": true, + "section": "rsvp", + "section_label": "RSVP Details", + "section_order": 1, + "display_order": 3 + }, + { + "id": "ack_attendance", + "type": "checkbox", + "label": "I confirm that I will attend the event and understand my spot may be released if I do not check in", + "required": true, + "section": "rsvp", + "section_label": "RSVP Details", + "section_order": 1, + "display_order": 4 + }, + { + "id": "additional_notes", + "type": "textarea", + "label": "Anything else we should know?", + "required": false, + "section": "rsvp", + "section_label": "RSVP Details", + "section_order": 1, + "display_order": 5, + "validation": { + "maxLength": 1000 + } + } +] diff --git a/internal/store/defaults/travel_rsvp_schema.json b/internal/store/defaults/travel_rsvp_schema.json new file mode 100644 index 00000000..01f9690b --- /dev/null +++ b/internal/store/defaults/travel_rsvp_schema.json @@ -0,0 +1,85 @@ +[ + { + "id": "travel_rsvp_mode", + "type": "select", + "label": "How will you be traveling?", + "required": true, + "section": "travel_rsvp", + "section_label": "Travel Details", + "section_order": 1, + "display_order": 1, + "options": [ + "Driving", + "Flying", + "Bus", + "Train", + "Other" + ] + }, + { + "id": "flight_airline", + "type": "text", + "label": "Airline", + "required": false, + "section": "travel_rsvp", + "section_label": "Travel Details", + "section_order": 1, + "display_order": 2, + "validation": { + "show_if": "travel_rsvp_mode=Flying", + "required_if": "travel_rsvp_mode=Flying" + } + }, + { + "id": "flight_numbers", + "type": "text", + "label": "Flight number(s)", + "required": false, + "section": "travel_rsvp", + "section_label": "Travel Details", + "section_order": 1, + "display_order": 3, + "validation": { + "show_if": "travel_rsvp_mode=Flying", + "required_if": "travel_rsvp_mode=Flying" + } + }, + { + "id": "payment_method", + "type": "select", + "label": "How would you like to be paid?", + "required": true, + "section": "travel_rsvp", + "section_label": "Travel Details", + "section_order": 1, + "display_order": 4, + "options": [ + "Zelle", + "Venmo", + "PayPal" + ] + }, + { + "id": "payment_details", + "type": "text", + "label": "Payment handle / details (Zelle email, Venmo username, or PayPal email)", + "required": true, + "section": "travel_rsvp", + "section_label": "Travel Details", + "section_order": 1, + "display_order": 5 + }, + { + "id": "travel_notes", + "type": "textarea", + "label": "Anything else we should know?", + "required": false, + "section": "travel_rsvp", + "section_label": "Travel Details", + "section_order": 1, + "display_order": 6, + "validation": { + "maxLength": 1000 + } + } +] diff --git a/internal/store/integration_test.go b/internal/store/integration_test.go index 4213b1c5..d16c9fe3 100644 --- a/internal/store/integration_test.go +++ b/internal/store/integration_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "os" + "reflect" "testing" _ "github.com/jackc/pgx/v5/stdlib" @@ -298,6 +299,42 @@ func TestIntegrationSettingsCache(t *testing.T) { } } +// TestIntegrationRestoreDefaultFormSchema covers the write behind the +// resetschema command: the upsert reaches a key that has no row yet, replaces +// one that does, and drops the cached copy on the way out. +func TestIntegrationRestoreDefaultFormSchema(t *testing.T) { + db := integrationDB(t) + defer db.Close() + s := newSettingsStore(db) + ctx := context.Background() + + edited := []ApplicationSchemaField{{ID: "only_field", Type: "text", Label: "Only Field"}} + if err := s.UpdateApplicationSchema(ctx, edited); err != nil { + t.Fatal(err) + } + + if err := s.RestoreDefaultFormSchema(ctx, SettingsKeyApplicationSchema); err != nil { + t.Fatal(err) + } + + want, err := DefaultFormSchemaFields(SettingsKeyApplicationSchema) + if err != nil { + t.Fatal(err) + } + + got, err := s.GetApplicationSchema(ctx) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("restored schema has %d field(s), want the %d shipped default(s)", len(got), len(want)) + } + + if err := s.RestoreDefaultFormSchema(ctx, "not_a_form_schema"); err == nil { + t.Error("expected an error for a key with no shipped default") + } +} + // seedIntegrationDeletion layers the rows a user deletion has to reason about on // top of seedIntegration: the admin has reviewed two applications, checked Alice // in, and scheduled a notification, and both they and Alice carry a diff --git a/internal/store/mock_store.go b/internal/store/mock_store.go index 75346e41..1397487f 100644 --- a/internal/store/mock_store.go +++ b/internal/store/mock_store.go @@ -267,6 +267,11 @@ func (m *MockSettingsStore) UpdateApplicationSchema(ctx context.Context, fields return args.Error(0) } +func (m *MockSettingsStore) RestoreDefaultFormSchema(ctx context.Context, key string) error { + args := m.Called(key) + return args.Error(0) +} + func (m *MockSettingsStore) GetRSVPSchema(ctx context.Context) ([]ApplicationSchemaField, error) { args := m.Called() if args.Get(0) == nil { diff --git a/internal/store/reviews.go b/internal/store/reviews.go index f5dc1a3e..d23225f6 100644 --- a/internal/store/reviews.go +++ b/internal/store/reviews.go @@ -285,13 +285,18 @@ func (s *ApplicationReviewsStore) GetNotesByApplicationID(ctx context.Context, a return notes, nil } -// BatchAssignmentResult contains stats about a batch assignment operation +// BatchAssignmentResult reports the committed changes and any remaining shortage +// among the submitted applications considered by this run. type BatchAssignmentResult struct { - ReviewsCreated int `json:"reviews_created"` + ReviewsCreated int `json:"reviews_created"` + ReviewsRemoved int `json:"reviews_removed"` + ReviewsPerApplication int `json:"reviews_per_application"` + ApplicationsBelowTarget int `json:"applications_below_target"` + ReviewsUnfilled int `json:"reviews_unfilled"` } -// BatchAssign assigns reviews to admins for submitted applications needing more reviews. -// Uses workload balancing — admins with fewer pending reviews are assigned first. +// BatchAssign recovers inaccessible pending reviews and fills submitted +// applications' assignment targets with distinct, currently eligible reviewers. func (s *ApplicationReviewsStore) BatchAssign(ctx context.Context, reviewsPerApp int) (*BatchAssignmentResult, error) { ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration*2) defer cancel() @@ -302,255 +307,209 @@ func (s *ApplicationReviewsStore) BatchAssign(ctx context.Context, reviewsPerApp } defer tx.Rollback() - // Ensure all super_admins exist in the review assignment setting. - // This acts as a backfill for any super_admins that were created before this setting existed - // or were added to the database manually. - var entries []ReviewAssignmentEntry - - selectSettingQuery := `SELECT value FROM settings WHERE key = $1 FOR UPDATE` + // Serialize batches even when this setting has not been created yet. + if _, err := tx.ExecContext(ctx, ` + INSERT INTO settings (key, value) VALUES ($1, '[]'::jsonb) + ON CONFLICT (key) DO NOTHING + `, SettingsKeyReviewAssignmentToggle); err != nil { + return nil, err + } var value []byte - err = tx.QueryRowContext(ctx, selectSettingQuery, SettingsKeyReviewAssignmentToggle).Scan(&value) - - isNewSetting := false - if err != nil { - if !errors.Is(err, sql.ErrNoRows) { - return nil, err - } - isNewSetting = true + if err := tx.QueryRowContext(ctx, `SELECT value FROM settings WHERE key = $1 FOR UPDATE`, + SettingsKeyReviewAssignmentToggle).Scan(&value); err != nil { + return nil, err + } + entries, err := parseReviewAssignmentEntries(value) + if err != nil || entries == nil { + // Match the toggle setting's existing default-enabled behavior. entries = []ReviewAssignmentEntry{} - } else { - if jerr := json.Unmarshal(value, &entries); jerr != nil { - var ids []string - if jerr2 := json.Unmarshal(value, &ids); jerr2 == nil { - entries = []ReviewAssignmentEntry{} - for _, id := range ids { - entries = append(entries, ReviewAssignmentEntry{ID: id, Enabled: true}) - } - } else { - entries = []ReviewAssignmentEntry{} - } - } } - - // Only run the full backfill query if entries might be out of sync - needsBackfill := isNewSetting - if !isNewSetting { - var adminCount int - countQuery := `SELECT COUNT(*) FROM users WHERE role = 'super_admin'` - if err := tx.QueryRowContext(ctx, countQuery).Scan(&adminCount); err != nil { - return nil, err + disabledIDs := []string{} + listed := make(map[string]bool, len(entries)) + for _, entry := range entries { + listed[entry.ID] = true + if !entry.Enabled { + disabledIDs = append(disabledIDs, entry.ID) } - needsBackfill = adminCount != len(entries) } - if needsBackfill { - backfillAdminsQuery := ` - SELECT u.id - FROM users u - WHERE u.role = 'super_admin' - ` - adminRows, err := tx.QueryContext(ctx, backfillAdminsQuery) - if err != nil { - return nil, err - } - - var allAdminIDs []string - for adminRows.Next() { - var id string - if err := adminRows.Scan(&id); err != nil { - adminRows.Close() - return nil, err - } - allAdminIDs = append(allAdminIDs, id) - } - adminRows.Close() - if err := adminRows.Err(); err != nil { - return nil, err - } - - existingAdminMap := make(map[string]bool) - for _, entry := range entries { - existingAdminMap[entry.ID] = true - } - - changesMade := false - for _, adminID := range allAdminIDs { - if _, exists := existingAdminMap[adminID]; !exists { - entries = append(entries, ReviewAssignmentEntry{ID: adminID, Enabled: true}) - changesMade = true - } - } - - if changesMade || isNewSetting { - jsonValue, err := json.Marshal(entries) - if err != nil { - return nil, err - } - - upsertQuery := ` - INSERT INTO settings (key, value) - VALUES ($1, $2) - ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value, updated_at = NOW() - ` - if _, err := tx.ExecContext(ctx, upsertQuery, SettingsKeyReviewAssignmentToggle, string(jsonValue)); err != nil { - return nil, err - } - } - } - - // Remove pending assignments owned by admins who are not listed in the - // review assignment setting so those applications can be redistributed - // to enabled admins. The setting is stored in `settings` with key - // 'review_assignment_toggle' as a JSONB array of objects {"id","enabled"}. - cleanupQuery := ` + result := &BatchAssignmentResult{ReviewsPerApplication: reviewsPerApp} + removed, err := tx.ExecContext(ctx, ` DELETE FROM application_reviews ar - WHERE ar.vote IS NULL - AND EXISTS ( - SELECT 1 - FROM settings s - CROSS JOIN jsonb_array_elements(s.value) AS elem - WHERE s.key = 'review_assignment_toggle' - AND elem->>'id' = ar.admin_id::text - AND (elem->'enabled')::boolean = false - ); - ` - - if _, err := tx.ExecContext(ctx, cleanupQuery); err != nil { + WHERE ar.vote IS NULL AND ( + ar.admin_id::text = ANY($1::text[]) OR NOT EXISTS ( + SELECT 1 FROM users u + WHERE u.id = ar.admin_id AND u.role IN ('admin', 'super_admin') + ) + ) + `, disabledIDs) + if err != nil { + return nil, err + } + n, err := removed.RowsAffected() + if err != nil { return nil, err } + result.ReviewsRemoved = int(n) - // Get admins sorted by pending workload (fewest pending first) - adminsQuery := ` - SELECT u.id + // Read workloads after cleanup. Creation time and ID provide stable ties. + adminRows, err := tx.QueryContext(ctx, ` + SELECT u.id, u.role, COUNT(ar.id), NOT (u.id::text = ANY($1::text[])) FROM users u - LEFT JOIN application_reviews ar - ON u.id = ar.admin_id AND ar.vote IS NULL - LEFT JOIN settings s - ON s.key = 'review_assignment_toggle' + LEFT JOIN application_reviews ar ON ar.admin_id = u.id AND ar.vote IS NULL WHERE u.role IN ('admin', 'super_admin') - AND NOT EXISTS ( - SELECT 1 - FROM jsonb_array_elements(s.value) AS elem - WHERE elem->>'id' = u.id::text - AND (elem->'enabled')::boolean = false - ) - GROUP BY u.id, u.created_at - ORDER BY COUNT(ar.id) ASC, u.created_at ASC; - ` - - adminRows, err := tx.QueryContext(ctx, adminsQuery) + GROUP BY u.id + ORDER BY u.created_at, u.id + `, disabledIDs) if err != nil { return nil, err } defer adminRows.Close() - - var adminIDs []string + type reviewer struct { + ID string + Pending int + } + var admins []reviewer for adminRows.Next() { - var id string - if err := adminRows.Scan(&id); err != nil { + var admin reviewer + var role UserRole + var enabled bool + if err := adminRows.Scan(&admin.ID, &role, &admin.Pending, &enabled); err != nil { return nil, err } - adminIDs = append(adminIDs, id) + if role == RoleSuperAdmin && !listed[admin.ID] { + entries = append(entries, ReviewAssignmentEntry{ID: admin.ID, Enabled: true}) + } + if enabled { + admins = append(admins, admin) + } } if err := adminRows.Err(); err != nil { return nil, err } + adminRows.Close() - if len(adminIDs) == 0 { - return &BatchAssignmentResult{}, nil + // Normalize legacy settings and retain the super-admin backfill. + encoded, err := json.Marshal(entries) + if err != nil { + return nil, err + } + if _, err := tx.ExecContext(ctx, `UPDATE settings SET value = $2, updated_at = NOW() WHERE key = $1`, + SettingsKeyReviewAssignmentToggle, string(encoded)); err != nil { + return nil, err } - // Get submitted applications needing reviews - appsQuery := ` - SELECT id, user_id, reviews_assigned - FROM applications + appRows, err := tx.QueryContext(ctx, ` + SELECT id, user_id, reviews_assigned FROM applications WHERE status = 'submitted' AND reviews_assigned < $1 - ORDER BY reviews_assigned ASC, submitted_at ASC + ORDER BY reviews_assigned, submitted_at, id FOR UPDATE - ` - - appRows, err := tx.QueryContext(ctx, appsQuery, reviewsPerApp) + `, reviewsPerApp) if err != nil { return nil, err } defer appRows.Close() - - type appInfo struct { - ID string - UserID string - ReviewsAssigned int + type application struct { + ID string + UserID string + Assigned int } - - var apps []appInfo + var apps []application + appIDs := []string{} for appRows.Next() { - var a appInfo - if err := appRows.Scan(&a.ID, &a.UserID, &a.ReviewsAssigned); err != nil { + var app application + if err := appRows.Scan(&app.ID, &app.UserID, &app.Assigned); err != nil { return nil, err } - apps = append(apps, a) + apps = append(apps, app) + appIDs = append(appIDs, app.ID) } if err := appRows.Err(); err != nil { return nil, err } + appRows.Close() - if len(apps) == 0 { - return &BatchAssignmentResult{}, nil + // Both pending and completed reviews reserve their reviewer/application pair. + pairs := make(map[string]map[string]bool, len(apps)) + if len(apps) > 0 { + rows, err := tx.QueryContext(ctx, ` + SELECT application_id, admin_id FROM application_reviews + WHERE application_id = ANY($1::uuid[]) + `, appIDs) + if err != nil { + return nil, err + } + defer rows.Close() + for rows.Next() { + var appID, adminID string + if err := rows.Scan(&appID, &adminID); err != nil { + return nil, err + } + if pairs[appID] == nil { + pairs[appID] = make(map[string]bool) + } + pairs[appID][adminID] = true + } + if err := rows.Err(); err != nil { + return nil, err + } + rows.Close() } - // Round-robin assignment with workload balancing. - // Build the full list of (application_id, admin_id) pairs in Go, then - // issue a single bulk INSERT to avoid N network roundtrips to the DB. - var pairAppIDs []string - var pairAdminIDs []string - adminIndex := 0 - + var pairAppIDs, pairAdminIDs []string for _, app := range apps { - needed := reviewsPerApp - app.ReviewsAssigned - - for range needed { - for range adminIDs { - adminID := adminIDs[adminIndex] - adminIndex = (adminIndex + 1) % len(adminIDs) - - // Skip self-review - if adminID == app.UserID { + if pairs[app.ID] == nil { + pairs[app.ID] = make(map[string]bool) + } + for range reviewsPerApp - app.Assigned { + best := -1 + for i, admin := range admins { + if admin.ID == app.UserID || pairs[app.ID][admin.ID] { continue } - - pairAppIDs = append(pairAppIDs, app.ID) - pairAdminIDs = append(pairAdminIDs, adminID) + if best == -1 || admin.Pending < admins[best].Pending { + best = i + } + } + if best == -1 { break } + admin := &admins[best] + pairs[app.ID][admin.ID] = true + admin.Pending++ + pairAppIDs = append(pairAppIDs, app.ID) + pairAdminIDs = append(pairAdminIDs, admin.ID) } } - - reviewsCreated := 0 if len(pairAppIDs) > 0 { - insertQuery := ` + inserted, err := tx.ExecContext(ctx, ` INSERT INTO application_reviews (application_id, admin_id) SELECT * FROM unnest($1::uuid[], $2::uuid[]) ON CONFLICT (application_id, admin_id) DO NOTHING - ` - - result, err := tx.ExecContext(ctx, insertQuery, pairAppIDs, pairAdminIDs) + `, pairAppIDs, pairAdminIDs) if err != nil { return nil, err } - - rowsAffected, err := result.RowsAffected() + n, err := inserted.RowsAffected() if err != nil { return nil, err } - reviewsCreated = int(rowsAffected) + result.ReviewsCreated = int(n) } + // Use actual counters after insertion, never the number of attempted pairs. + if err := tx.QueryRowContext(ctx, ` + SELECT COUNT(*), COALESCE(SUM($2 - reviews_assigned), 0) + FROM applications + WHERE id = ANY($1::uuid[]) AND reviews_assigned < $2 + `, appIDs, reviewsPerApp).Scan(&result.ApplicationsBelowTarget, &result.ReviewsUnfilled); err != nil { + return nil, err + } + // Cleanup and backfill must also commit when no new assignments are possible. if err := tx.Commit(); err != nil { return nil, err } - - return &BatchAssignmentResult{ - ReviewsCreated: reviewsCreated, - }, nil + return result, nil } // SetAIPercent sets the AI-generated percent on an application, only if the admin is assigned to it and it hasn't been set yet. diff --git a/internal/store/reviews_assignment_integration_test.go b/internal/store/reviews_assignment_integration_test.go new file mode 100644 index 00000000..ae51c939 --- /dev/null +++ b/internal/store/reviews_assignment_integration_test.go @@ -0,0 +1,416 @@ +package store + +import ( + "context" + "database/sql" + "errors" + "fmt" + "testing" +) + +func batchTestExec(t *testing.T, db *sql.DB, query string, args ...any) { + t.Helper() + if _, err := db.Exec(query, args...); err != nil { + t.Fatal(err) + } +} + +func batchTestSeed(t *testing.T, admins, apps int) (*sql.DB, *ApplicationReviewsStore, []string, []string) { + t.Helper() + db := integrationDB(t) + t.Cleanup(func() { db.Close() }) + batchTestExec(t, db, "TRUNCATE application_reviews, applications, users CASCADE") + batchTestExec(t, db, "DELETE FROM settings WHERE key IN ('review_assignment_toggle', 'reviews_per_application')") + t.Cleanup(func() { + batchTestCheckCounters(t, db) + batchTestExec(t, db, "DELETE FROM settings WHERE key IN ('review_assignment_toggle', 'reviews_per_application')") + }) + var adminIDs, appIDs []string + for i := 0; i < admins; i++ { + id := fmt.Sprintf("10000000-0000-0000-0000-%012d", i+1) + batchTestExec(t, db, "INSERT INTO users (id, supertokens_user_id, email, role, created_at) VALUES ($1, $2, $3, 'admin', '2026-01-01'::timestamptz + $4 * interval '1 second')", id, id, fmt.Sprintf("admin%d@example.com", i), i) + adminIDs = append(adminIDs, id) + } + for i := 0; i < apps; i++ { + userID := fmt.Sprintf("20000000-0000-0000-0000-%012d", i+1) + appID := fmt.Sprintf("30000000-0000-0000-0000-%012d", i+1) + batchTestExec(t, db, "INSERT INTO users (id, supertokens_user_id, email, role) VALUES ($1, $2, $3, 'hacker')", userID, userID, fmt.Sprintf("hacker%d@example.com", i)) + batchTestExec(t, db, "INSERT INTO applications (id, user_id, status, submitted_at) VALUES ($1, $2, 'submitted', '2026-01-02'::timestamptz + $3 * interval '1 second')", appID, userID, i) + appIDs = append(appIDs, appID) + } + return db, &ApplicationReviewsStore{db: db}, adminIDs, appIDs +} + +func batchTestBatch(t *testing.T, s *ApplicationReviewsStore, target int) int { + t.Helper() + r, err := s.BatchAssign(context.Background(), target) + if err != nil { + t.Fatal(err) + } + return r.ReviewsCreated +} + +func batchTestCount(t *testing.T, db *sql.DB, query string, args ...any) int { + t.Helper() + var n int + if err := db.QueryRow(query, args...).Scan(&n); err != nil { + t.Fatal(err) + } + return n +} + +func batchTestCheckCounters(t *testing.T, db *sql.DB) { + t.Helper() + n := batchTestCount(t, db, `SELECT count(*) FROM applications a WHERE + a.reviews_assigned <> (SELECT count(*) FROM application_reviews r WHERE r.application_id = a.id) OR + a.reviews_completed <> (SELECT count(*) FROM application_reviews r WHERE r.application_id = a.id AND r.vote IS NOT NULL) OR + a.accept_votes <> (SELECT count(*) FROM application_reviews r WHERE r.application_id = a.id AND r.vote = 'accept') OR + a.reject_votes <> (SELECT count(*) FROM application_reviews r WHERE r.application_id = a.id AND r.vote = 'reject')`) + if n != 0 { + t.Errorf("%d applications have incorrect counters", n) + } +} + +func TestIntegrationBatchAssign(t *testing.T) { + ctx := context.Background() + t.Run("fresh_assignment_and_repeat", func(t *testing.T) { + db, s, _, _ := batchTestSeed(t, 4, 12) + if n := batchTestBatch(t, s, 3); n != 36 { + t.Errorf("created=%d, want 36", n) + } + if n := batchTestCount(t, db, "SELECT count(*) FROM applications WHERE reviews_assigned <> 3"); n != 0 { + t.Errorf("%d applications lack three reviews", n) + } + if n := batchTestBatch(t, s, 3); n != 0 { + t.Errorf("repeat created=%d, want 0", n) + } + batchTestCheckCounters(t, db) + }) + t.Run("raise_count_after_completed_reviews", func(t *testing.T) { + db, s, admins, _ := batchTestSeed(t, 3, 1) + batchTestBatch(t, s, 2) + for _, admin := range admins { + pending, err := s.GetPendingByAdminID(ctx, admin) + if err != nil { + t.Fatal(err) + } + for _, r := range pending { + if _, err := s.SubmitVote(ctx, r.ID, admin, ReviewVoteAccept, nil, nil); err != nil { + t.Fatal(err) + } + } + } + if n := batchTestBatch(t, s, 3); n != 1 { + t.Errorf("target increase created=%d, want 1", n) + } + if n := batchTestBatch(t, s, 3); n != 0 { + t.Errorf("repeat created=%d, want 0", n) + } + if n := batchTestCount(t, db, "SELECT reviews_assigned FROM applications"); n != 3 { + t.Errorf("assigned=%d, want 3; third reviewer is eligible", n) + } + batchTestCheckCounters(t, db) + }) + t.Run("raise_count_with_pending_reviews", func(t *testing.T) { + db, s, _, _ := batchTestSeed(t, 3, 6) + batchTestBatch(t, s, 2) + if n := batchTestBatch(t, s, 3); n != 6 { + t.Errorf("target increase created=%d, want 6", n) + } + if n := batchTestCount(t, db, "SELECT count(*) FROM applications WHERE reviews_assigned < 3"); n != 0 { + t.Errorf("%d applications remain under-assigned despite enough reviewers", n) + } + }) + t.Run("decrease_target_retains_existing_assignments", func(t *testing.T) { + db, s, _, _ := batchTestSeed(t, 3, 1) + batchTestBatch(t, s, 3) + settings := &SettingsStore{db: db} + if err := settings.SetReviewsPerApplication(ctx, 1); err != nil { + t.Fatal(err) + } + target, err := settings.GetReviewsPerApplication(ctx) + if err != nil { + t.Fatal(err) + } + batchTestBatch(t, s, target) + n := batchTestCount(t, db, "SELECT count(*) FROM application_reviews WHERE vote IS NULL") + t.Logf("saved target=%d; pending assignments retained=%d", target, n) + if n != 3 { + t.Errorf("unexpected pending count=%d", n) + } + }) + t.Run("disabled_reviewer_cleanup_without_eligible_admins", func(t *testing.T) { + db, s, admins, _ := batchTestSeed(t, 1, 1) + batchTestExec(t, db, "UPDATE users SET role='super_admin' WHERE id=$1", admins[0]) + batchTestBatch(t, s, 1) + settings := &SettingsStore{db: db} + if err := settings.SetReviewAssignmentToggle(ctx, admins[0], false); err != nil { + t.Fatal(err) + } + batchTestBatch(t, s, 1) + if n := batchTestCount(t, db, "SELECT count(*) FROM application_reviews WHERE vote IS NULL"); n != 0 { + t.Errorf("%d pending reviews retained for disabled reviewer; cleanup rolled back", n) + } + }) + t.Run("disabled_reviewer_cleanup_when_application_decided", func(t *testing.T) { + db, s, admins, _ := batchTestSeed(t, 2, 1) + batchTestExec(t, db, "UPDATE users SET role='super_admin' WHERE id=$1", admins[0]) + batchTestBatch(t, s, 1) + batchTestExec(t, db, "UPDATE applications SET status='accepted'") + settings := &SettingsStore{db: db} + if err := settings.SetReviewAssignmentToggle(ctx, admins[0], false); err != nil { + t.Fatal(err) + } + batchTestBatch(t, s, 1) + if n := batchTestCount(t, db, "SELECT count(*) FROM application_reviews WHERE vote IS NULL"); n != 0 { + t.Errorf("%d pending reviews retained after cleanup because there were no submitted candidates", n) + } + }) + t.Run("disabled_reviewer_redistribution", func(t *testing.T) { + db, s, admins, _ := batchTestSeed(t, 3, 1) + batchTestExec(t, db, "UPDATE users SET role='super_admin' WHERE id=$1", admins[0]) + batchTestBatch(t, s, 2) + settings := &SettingsStore{db: db} + if err := settings.SetReviewAssignmentToggle(ctx, admins[0], false); err != nil { + t.Fatal(err) + } + batchTestBatch(t, s, 2) + if n := batchTestCount(t, db, "SELECT count(*) FROM application_reviews WHERE admin_id=$1", admins[0]); n != 0 { + t.Errorf("disabled reviewer still owns %d assignments", n) + } + if n := batchTestCount(t, db, "SELECT reviews_assigned FROM applications"); n != 2 { + t.Errorf("assigned=%d, want 2", n) + } + batchTestCheckCounters(t, db) + }) + t.Run("demoted_reviewer_assignments_recovered", func(t *testing.T) { + db, s, admins, _ := batchTestSeed(t, 3, 1) + batchTestBatch(t, s, 2) + if _, err := (&UsersStore{db: db}).UpdateRole(ctx, admins[0], RoleHacker); err != nil { + t.Fatal(err) + } + batchTestBatch(t, s, 2) + if n := batchTestCount(t, db, "SELECT count(*) FROM application_reviews r JOIN users u ON u.id=r.admin_id WHERE r.vote IS NULL AND u.role='hacker'"); n != 0 { + t.Errorf("%d assignments stuck under a user who can no longer access review endpoints", n) + } + }) + t.Run("workload_balancing", func(t *testing.T) { + db, s, admins, apps := batchTestSeed(t, 2, 14) + for _, app := range apps[:10] { + batchTestExec(t, db, "INSERT INTO application_reviews(application_id,admin_id) VALUES ($1,$2)", app, admins[0]) + } + batchTestBatch(t, s, 1) + a := batchTestCount(t, db, "SELECT count(*) FROM application_reviews WHERE admin_id=$1", admins[0]) + b := batchTestCount(t, db, "SELECT count(*) FROM application_reviews WHERE admin_id=$1", admins[1]) + t.Logf("initial workload 10:0; after four new assignments %d:%d", a, b) + if a != 10 || b != 4 { + t.Errorf("expected new assignments to go to reviewer with lower workload") + } + }) + t.Run("self_review_and_insufficient_capacity", func(t *testing.T) { + db, s, admins, apps := batchTestSeed(t, 2, 1) + batchTestExec(t, db, "UPDATE applications SET user_id=$1 WHERE id=$2", admins[0], apps[0]) + t.Logf("target=3; eligible distinct non-self reviewers=1; created=%d", batchTestBatch(t, s, 3)) + if n := batchTestCount(t, db, "SELECT count(*) FROM application_reviews r JOIN applications a ON a.id=r.application_id WHERE r.admin_id=a.user_id"); n != 0 { + t.Errorf("self reviews=%d", n) + } + if n := batchTestCount(t, db, "SELECT count(*) FROM application_reviews"); n != 1 { + t.Errorf("assignments=%d, want 1", n) + } + }) + t.Run("vote_update_counters_and_queue_completion", func(t *testing.T) { + db, s, admins, _ := batchTestSeed(t, 3, 3) + batchTestBatch(t, s, 3) + for _, admin := range admins { + pending, err := s.GetPendingByAdminID(ctx, admin) + if err != nil { + t.Fatal(err) + } + for _, r := range pending { + if _, err := s.SubmitVote(ctx, r.ID, admin, ReviewVoteAccept, nil, nil); err != nil { + t.Fatal(err) + } + if _, err := s.SubmitVote(ctx, r.ID, admin, ReviewVoteReject, nil, nil); err != nil { + t.Fatal(err) + } + } + pending, err = s.GetPendingByAdminID(ctx, admin) + if err != nil || len(pending) != 0 { + t.Fatalf("pending=%d err=%v", len(pending), err) + } + completed, err := s.GetCompletedByAdminID(ctx, admin) + if err != nil || len(completed) != 3 { + t.Fatalf("completed=%d err=%v", len(completed), err) + } + } + batchTestCheckCounters(t, db) + if n := batchTestCount(t, db, "SELECT count(*) FROM applications WHERE reviews_completed=3 AND reject_votes=3"); n != 3 { + t.Errorf("fully completed applications=%d, want 3", n) + } + }) + + t.Run("committed_cleanup_statistics", func(t *testing.T) { + for _, decided := range []bool{false, true} { + t.Run(fmt.Sprint(decided), func(t *testing.T) { + db, s, admins, _ := batchTestSeed(t, 1, 1) + batchTestBatch(t, s, 1) + batchTestExec(t, db, "UPDATE users SET role='hacker' WHERE id=$1", admins[0]) + if decided { + batchTestExec(t, db, "UPDATE applications SET status='accepted'") + } + r, err := s.BatchAssign(ctx, 2) + if err != nil { + t.Fatal(err) + } + want := BatchAssignmentResult{ReviewsRemoved: 1, ReviewsPerApplication: 2} + if !decided { + want.ApplicationsBelowTarget = 1 + want.ReviewsUnfilled = 2 + } + if *r != want { + t.Errorf("result=%+v, want %+v", *r, want) + } + if n := batchTestCount(t, db, "SELECT count(*) FROM application_reviews"); n != 0 { + t.Errorf("cleanup retained %d rows", n) + } + }) + } + }) + t.Run("completed_ineligible_reviews_preserved", func(t *testing.T) { + for _, demoted := range []bool{false, true} { + t.Run(fmt.Sprint(demoted), func(t *testing.T) { + db, s, admins, _ := batchTestSeed(t, 3, 2) + batchTestBatch(t, s, 2) + pending, err := s.GetPendingByAdminID(ctx, admins[0]) + if err != nil || len(pending) == 0 { + t.Fatalf("pending: %v", err) + } + if _, err := s.SubmitVote(ctx, pending[0].ID, admins[0], ReviewVoteAccept, nil, nil); err != nil { + t.Fatal(err) + } + if demoted { + if _, err := (&UsersStore{db: db}).UpdateRole(ctx, admins[0], RoleHacker); err != nil { + t.Fatal(err) + } + } else if err := (&SettingsStore{db: db}).SetReviewAssignmentToggle(ctx, admins[0], false); err != nil { + t.Fatal(err) + } + r, err := s.BatchAssign(ctx, 2) + if err != nil { + t.Fatal(err) + } + if r.ApplicationsBelowTarget != 0 { + t.Errorf("shortage: %+v", r) + } + if n := batchTestCount(t, db, "SELECT count(*) FROM application_reviews WHERE admin_id=$1 AND vote='accept'", admins[0]); n != 1 { + t.Errorf("completed reviews=%d", n) + } + if n := batchTestCount(t, db, "SELECT count(*) FROM application_reviews WHERE admin_id=$1 AND vote IS NULL", admins[0]); n != 0 { + t.Errorf("ineligible pending reviews=%d", n) + } + }) + } + }) + t.Run("shortage_and_self_review_reporting", func(t *testing.T) { + db, s, admins, apps := batchTestSeed(t, 2, 1) + batchTestExec(t, db, "UPDATE applications SET user_id=$1 WHERE id=$2", admins[0], apps[0]) + r, err := s.BatchAssign(ctx, 3) + if err != nil { + t.Fatal(err) + } + want := BatchAssignmentResult{ReviewsCreated: 1, ReviewsPerApplication: 3, ApplicationsBelowTarget: 1, ReviewsUnfilled: 2} + if *r != want { + t.Errorf("result=%+v, want %+v", *r, want) + } + r, err = s.BatchAssign(ctx, 3) + if err != nil { + t.Fatal(err) + } + want.ReviewsCreated = 0 + if *r != want { + t.Errorf("repeat=%+v, want %+v", *r, want) + } + }) + t.Run("legacy_toggle_and_backfill", func(t *testing.T) { + db, s, admins, _ := batchTestSeed(t, 2, 0) + batchTestExec(t, db, "UPDATE users SET role='super_admin'") + batchTestExec(t, db, "INSERT INTO settings(key,value) VALUES ('review_assignment_toggle',jsonb_build_array($1::text))", admins[0]) + r, err := s.BatchAssign(ctx, 3) + if err != nil { + t.Fatal(err) + } + if r.ReviewsCreated != 0 { + t.Fatalf("unexpected assignments: %+v", r) + } + entries, err := (&SettingsStore{db: db}).GetAllReviewAssignmentToggles(ctx) + if err != nil || len(entries) != 2 { + t.Fatalf("backfill=%+v, err=%v", entries, err) + } + for _, entry := range entries { + if !entry.Enabled { + t.Errorf("legacy/backfilled reviewer disabled: %+v", entry) + } + } + }) + t.Run("simultaneous_batches", func(t *testing.T) { + for _, absent := range []bool{false, true} { + t.Run(fmt.Sprint(absent), func(t *testing.T) { + db, s, _, _ := batchTestSeed(t, 4, 12) + if !absent { + batchTestExec(t, db, "INSERT INTO settings(key,value) VALUES ('review_assignment_toggle','[]')") + } + type outcome struct { + result *BatchAssignmentResult + err error + } + start := make(chan struct{}) + results := make(chan outcome, 2) + for range 2 { + go func() { <-start; r, err := s.BatchAssign(ctx, 3); results <- outcome{r, err} }() + } + close(start) + created := 0 + for range 2 { + r := <-results + if r.err != nil { + t.Error(r.err) + continue + } + created += r.result.ReviewsCreated + if r.result.ReviewsUnfilled != 0 { + t.Errorf("shortage: %+v", r.result) + } + } + if created != 36 { + t.Errorf("created=%d, want 36", created) + } + if n := batchTestCount(t, db, "SELECT count(*) FROM applications WHERE reviews_assigned <> 3"); n != 0 { + t.Errorf("%d applications have wrong totals", n) + } + }) + } + }) + t.Run("individual_and_batch_assignment", func(t *testing.T) { + db, s, admins, _ := batchTestSeed(t, 3, 8) + start := make(chan struct{}) + errs := make(chan error, 2) + go func() { <-start; _, err := s.BatchAssign(ctx, 2); errs <- err }() + go func() { + <-start + _, err := s.AssignNextForAdmin(ctx, admins[0], 2) + if errors.Is(err, ErrNotFound) { + err = nil + } + errs <- err + }() + close(start) + for range 2 { + if err := <-errs; err != nil { + t.Error(err) + } + } + if n := batchTestCount(t, db, "SELECT count(*) FROM applications WHERE reviews_assigned <> 2"); n != 0 { + t.Errorf("%d applications have wrong totals", n) + } + }) +} diff --git a/internal/store/settings.go b/internal/store/settings.go index cacab7de..e3af6ffd 100644 --- a/internal/store/settings.go +++ b/internal/store/settings.go @@ -356,7 +356,7 @@ func (s *SettingsStore) GetReviewsPerApplication(ctx context.Context) (int, erro return count, nil } -// SetReviewsPerApplication updates the number of reviews required per application +// SetReviewsPerApplication updates the reviewer assignment target per application func (s *SettingsStore) SetReviewsPerApplication(ctx context.Context, value int) error { ctx, cancel := context.WithTimeout(ctx, QueryTimeoutDuration) defer cancel() diff --git a/internal/store/storage.go b/internal/store/storage.go index f3a11faa..af7bd5ae 100644 --- a/internal/store/storage.go +++ b/internal/store/storage.go @@ -81,6 +81,9 @@ type Storage struct { SetRSVPEnabled(ctx context.Context, enabled bool) error GetTravelRSVPSchema(ctx context.Context) ([]ApplicationSchemaField, error) UpdateTravelRSVPSchema(ctx context.Context, fields []ApplicationSchemaField) error + // RestoreDefaultFormSchema overwrites one of the editable form + // schemas with the default HARP ships with. + RestoreDefaultFormSchema(ctx context.Context, key string) error GetTravelRSVPEnabled(ctx context.Context) (bool, error) SetTravelRSVPEnabled(ctx context.Context, enabled bool) error GetReviewsPerApplication(ctx context.Context) (int, error) From 95030e07918e629bce96e23d8ab823f91001ed55 Mon Sep 17 00:00:00 2001 From: Caleb Bae Date: Tue, 8 Sep 2026 09:58:00 -0500 Subject: [PATCH 07/11] fix: required conditional check against application form --- .../apply/components/ApplicationWizard.tsx | 13 +- .../src/pages/hacker/apply/validations.ts | 10 +- .../portal/src/pages/hacker/rsvp/RSVPPage.tsx | 7 +- .../hacker/travel-rsvp/TravelRSVPPage.tsx | 7 +- client/portal/src/shared/lib/schema-utils.ts | 141 +++++++++++------- 5 files changed, 103 insertions(+), 75 deletions(-) diff --git a/client/portal/src/pages/hacker/apply/components/ApplicationWizard.tsx b/client/portal/src/pages/hacker/apply/components/ApplicationWizard.tsx index 01109988..0b00a360 100644 --- a/client/portal/src/pages/hacker/apply/components/ApplicationWizard.tsx +++ b/client/portal/src/pages/hacker/apply/components/ApplicationWizard.tsx @@ -1,4 +1,3 @@ -import { zodResolver } from "@hookform/resolvers/zod"; import { AlertCircle } from "lucide-react"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { FormProvider, useForm } from "react-hook-form"; @@ -34,7 +33,7 @@ import { import { ReviewStep } from "../steps/ReviewStep"; import { SchemaStepRenderer } from "../steps/SchemaStepRenderer"; import { SponsorInfoStep } from "../steps/SponsorInfoStep"; -import { buildApplicationSchema } from "../validations"; +import { buildApplicationResolver } from "../validations"; import { StepIndicator } from "./StepIndicator"; import { StepNavigation } from "./StepNavigation"; @@ -170,14 +169,16 @@ export function ApplicationWizard({ userEmail }: ApplicationWizardProps) { [schemaFields], ); - // Build Zod schema dynamically from application_schema - const formSchema = useMemo( - () => buildApplicationSchema(schemaFields), + // Validate against a schema rebuilt from the current answers, so a question + // that only applies once another is answered (e.g. the travel questions + // behind the reimbursement opt-in) is enforced as soon as it appears. + const resolver = useMemo( + () => buildApplicationResolver(schemaFields), [schemaFields], ); const form = useForm({ - resolver: zodResolver(formSchema), + resolver, defaultValues: buildDefaultValues(schemaFields), mode: "onTouched", }); diff --git a/client/portal/src/pages/hacker/apply/validations.ts b/client/portal/src/pages/hacker/apply/validations.ts index 59e391de..aab11d95 100644 --- a/client/portal/src/pages/hacker/apply/validations.ts +++ b/client/portal/src/pages/hacker/apply/validations.ts @@ -1,11 +1,13 @@ -import { buildZodSchema } from "@/shared/lib/schema-utils"; +import { buildSchemaResolver } from "@/shared/lib/schema-utils"; import type { ApplicationSchemaField } from "@/types"; /** - * Build the full application form schema from the dynamic application_schema. + * Build the form resolver for the dynamic application_schema. The schema is + * rebuilt per validation pass so conditional questions (show_if / required_if) + * are judged against the answers on screen. */ -export function buildApplicationSchema(fields: ApplicationSchemaField[]) { - return buildZodSchema(fields); +export function buildApplicationResolver(fields: ApplicationSchemaField[]) { + return buildSchemaResolver(fields); } // Select options — provide human-readable labels for field values diff --git a/client/portal/src/pages/hacker/rsvp/RSVPPage.tsx b/client/portal/src/pages/hacker/rsvp/RSVPPage.tsx index c34adaa2..908efbcd 100644 --- a/client/portal/src/pages/hacker/rsvp/RSVPPage.tsx +++ b/client/portal/src/pages/hacker/rsvp/RSVPPage.tsx @@ -1,4 +1,3 @@ -import { zodResolver } from "@hookform/resolvers/zod"; import { ChevronLeft } from "lucide-react"; import { useEffect, useMemo, useState } from "react"; import { FormProvider, useForm } from "react-hook-form"; @@ -27,7 +26,7 @@ import { } from "@/shared/lib/form-errors"; import { buildDefaultValues, - buildZodSchema, + buildSchemaResolver, deriveSections, groupFieldsBySection, } from "@/shared/lib/schema-utils"; @@ -71,10 +70,10 @@ export default function RSVPPage() { const schema = useMemo(() => rsvp?.rsvp_schema ?? [], [rsvp]); const sections = useMemo(() => deriveSections(schema), [schema]); const grouped = useMemo(() => groupFieldsBySection(schema), [schema]); - const formSchema = useMemo(() => buildZodSchema(schema), [schema]); + const resolver = useMemo(() => buildSchemaResolver(schema), [schema]); const form = useForm({ - resolver: zodResolver(formSchema), + resolver, defaultValues: buildDefaultValues(schema), mode: "onTouched", }); diff --git a/client/portal/src/pages/hacker/travel-rsvp/TravelRSVPPage.tsx b/client/portal/src/pages/hacker/travel-rsvp/TravelRSVPPage.tsx index dc1faf48..b13be28c 100644 --- a/client/portal/src/pages/hacker/travel-rsvp/TravelRSVPPage.tsx +++ b/client/portal/src/pages/hacker/travel-rsvp/TravelRSVPPage.tsx @@ -1,4 +1,3 @@ -import { zodResolver } from "@hookform/resolvers/zod"; import { ChevronLeft, Eye } from "lucide-react"; import { useEffect, useMemo, useState } from "react"; import { FormProvider, useForm } from "react-hook-form"; @@ -28,7 +27,7 @@ import { } from "@/shared/lib/form-errors"; import { buildDefaultValues, - buildZodSchema, + buildSchemaResolver, deriveSections, groupFieldsBySection, } from "@/shared/lib/schema-utils"; @@ -100,10 +99,10 @@ export default function TravelRSVPPage() { ); const sections = useMemo(() => deriveSections(schema), [schema]); const grouped = useMemo(() => groupFieldsBySection(schema), [schema]); - const formSchema = useMemo(() => buildZodSchema(schema), [schema]); + const resolver = useMemo(() => buildSchemaResolver(schema), [schema]); const form = useForm({ - resolver: zodResolver(formSchema), + resolver, defaultValues: buildDefaultValues(schema), mode: "onTouched", }); diff --git a/client/portal/src/shared/lib/schema-utils.ts b/client/portal/src/shared/lib/schema-utils.ts index c5f92790..9cc8dbc8 100644 --- a/client/portal/src/shared/lib/schema-utils.ts +++ b/client/portal/src/shared/lib/schema-utils.ts @@ -1,4 +1,6 @@ +import { zodResolver } from "@hookform/resolvers/zod"; import { createElement, type ReactNode } from "react"; +import type { Resolver } from "react-hook-form"; import { z } from "zod"; import type { ApplicationSchemaField } from "@/types"; @@ -174,6 +176,8 @@ export function getWholeNumberRule( /** Build a Zod schema for a single field based on its ApplicationSchemaField definition. */ function buildFieldZod(field: ApplicationSchemaField): z.ZodType { const validation = field.validation ?? {}; + // Agreement labels carry markdown links; error messages name the question only. + const label = stripLabelLinks(field.label); switch (field.type) { case "text": { @@ -182,7 +186,7 @@ function buildFieldZod(field: ApplicationSchemaField): z.ZodType { // it all the way to the server, which trims before its own check. return z .string() - .refine((v) => v.trim() !== "", `${field.label} is required`); + .refine((v) => v.trim() !== "", `${label} is required`); } return z.string().optional().default(""); } @@ -191,10 +195,7 @@ function buildFieldZod(field: ApplicationSchemaField): z.ZodType { const usPhone = /^\+1\d{10}$/; const msg = "Enter a 10-digit US phone number"; if (field.required) { - return z - .string() - .min(1, `${field.label} is required`) - .regex(usPhone, msg); + return z.string().min(1, `${label} is required`).regex(usPhone, msg); } return z .string() @@ -203,9 +204,9 @@ function buildFieldZod(field: ApplicationSchemaField): z.ZodType { .refine((v) => !v || usPhone.test(v), msg); } case "number": { - let n = z.coerce.number({ message: `${field.label} is required` }); + let n = z.coerce.number({ message: `${label} is required` }); const whole = getWholeNumberRule(field.id); - if (whole) n = n.int(`${field.label} must be a whole number`); + if (whole) n = n.int(`${label} must be a whole number`); const schemaMin = typeof validation.min === "number" @@ -218,10 +219,10 @@ function buildFieldZod(field: ApplicationSchemaField): z.ZodType { : schemaMin; if (typeof min === "number") - n = n.min(min, `${field.label} must be at least ${min}`); + n = n.min(min, `${label} must be at least ${min}`); if (typeof validation.max === "number") { const max = validation.max as number; - n = n.max(max, `${field.label} must be at most ${max}`); + n = n.max(max, `${label} must be at most ${max}`); } if (field.required && typeof min !== "number") n = n.min(0); @@ -235,30 +236,30 @@ function buildFieldZod(field: ApplicationSchemaField): z.ZodType { const maxLength = validation.maxLength as number; s = s.max( maxLength, - `${field.label} must be ${maxLength} characters or fewer`, + `${label} must be ${maxLength} characters or fewer`, ); } if (field.required) { - return s.refine((v) => v.trim() !== "", `${field.label} is required`); + return s.refine((v) => v.trim() !== "", `${label} is required`); } return s; } case "select": { if (field.required) { - return z.string().min(1, `${field.label} is required`); + return z.string().min(1, `${label} is required`); } return z.string().optional().default(""); } case "multi_select": { if (field.required) { - return z.array(z.string()).min(1, `${field.label} is required`); + return z.array(z.string()).min(1, `${label} is required`); } return z.array(z.string()).optional().default([]); } case "checkbox": if (field.required) { return z.literal(true, { - message: `${stripLabelLinks(field.label)} is required`, + message: `${label} is required`, }); } return z.boolean().optional().default(false); @@ -271,60 +272,86 @@ function buildFieldZod(field: ApplicationSchemaField): z.ZodType { * Build a Zod object schema from an array of ApplicationSchemaField definitions. * Returns a z.object() with one key per field. * - * Requiredness for conditional fields is decided in the refinement rather than - * in the per-field schema, because it depends on another field's answer: + * Conditional requiredness (validation.show_if / required_if) depends on + * another field's answer, so it is resolved against `values` — the answers + * being validated — and baked into the per-field schema: * - a field hidden by an unsatisfied "show_if" is never required (the step * validator triggers every field in a section, including hidden ones, so * enforcing it would block the step with nothing on screen to fix); - * - a field with "required_if" becomes required once its controller is set. + * - a field with "required_if" is required once its controller is set. + * + * The rules live in the per-field schemas rather than in a top-level + * superRefine because Zod drops an object's refinements as soon as one of its + * properties raises a fatal issue — an untouched required number, or an + * unchecked required checkbox (z.literal(true)), both of which are the norm + * while the form is still being filled in. That silently disabled every + * conditional rule until the rest of the form was already valid, letting the + * wizard advance past an opted-in-but-empty travel section and only catching + * it at submit. + * + * Callers that validate live answers should use buildSchemaResolver, which + * feeds the current values back in on every validation pass. Omitting values + * treats every condition as unsatisfied. */ -export function buildZodSchema(fields: ApplicationSchemaField[]) { - const conditional = fields.map((f) => ({ - field: f, - showIf: getFieldCondition(f, "show_if"), - requiredIf: getFieldCondition(f, "required_if"), - })); - +export function buildZodSchema( + fields: ApplicationSchemaField[], + values?: Record | null, +) { const shape: Record = {}; - for (const { field, showIf } of conditional) { - // A show_if-gated field is shaped as optional; the refinement below applies - // its requiredness only while it is actually visible. - shape[field.id] = buildFieldZod( - showIf ? { ...field, required: false } : field, - ); - } - const refined = conditional.filter((c) => c.showIf || c.requiredIf); + for (const field of fields) { + const showIf = getFieldCondition(field, "show_if"); + const requiredIf = getFieldCondition(field, "required_if"); + + const visible = !showIf || conditionSatisfied(showIf, values); + const required = + visible && + (field.required || + (!!requiredIf && conditionSatisfied(requiredIf, values))); - return z.object(shape).superRefine((data, ctx) => { - for (const { field, showIf, requiredIf } of refined) { - if (showIf && !conditionSatisfied(showIf, data)) continue; + shape[field.id] = buildFieldZod({ ...field, required }); + } - const required = - field.required || - (!!requiredIf && conditionSatisfied(requiredIf, data)); - if (!required) continue; + return z.object(shape); +} - if (isEmptyValue(data[field.id])) { - ctx.addIssue({ - code: "custom", - path: [field.id], - message: `${stripLabelLinks(field.label)} is required`, - }); - } - } - }); +/** Field ids that gate another field's visibility or requiredness. */ +function conditionControllerIds(fields: ApplicationSchemaField[]): string[] { + return [ + ...new Set( + fields.flatMap((f) => + [ + getFieldCondition(f, "show_if")?.field, + getFieldCondition(f, "required_if")?.field, + ].filter((id): id is string => !!id), + ), + ), + ]; } -/** True when an answer counts as unanswered: blank, unchecked, or nothing picked. */ -function isEmptyValue(value: unknown): boolean { - return ( - value === undefined || - value === null || - value === false || - (typeof value === "string" && value.trim() === "") || - (Array.isArray(value) && value.length === 0) - ); +/** + * React Hook Form resolver for a dynamic application schema. The schema is + * rebuilt from the answers under validation so show_if / required_if rules see + * the current controller values; the build is reused until one of those + * controllers changes. + */ +export function buildSchemaResolver( + fields: ApplicationSchemaField[], +): Resolver> { + const controllerIds = conditionControllerIds(fields); + let cachedKey: string | undefined; + let cachedResolver: Resolver> | undefined; + + return (values, context, options) => { + const key = JSON.stringify(controllerIds.map((id) => values[id] ?? null)); + if (!cachedResolver || key !== cachedKey) { + cachedResolver = zodResolver(buildZodSchema(fields, values)) as Resolver< + Record + >; + cachedKey = key; + } + return cachedResolver(values, context, options); + }; } /** Build default form values from schema fields. */ From e216a57ee85175925565ed87c17960b1d5ee4519 Mon Sep 17 00:00:00 2001 From: Caleb Bae Date: Tue, 8 Sep 2026 10:02:25 -0500 Subject: [PATCH 08/11] fix:(sa): form unwanted scroll --- .../src/pages/superadmin/forms/components/FormDetail.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/client/portal/src/pages/superadmin/forms/components/FormDetail.tsx b/client/portal/src/pages/superadmin/forms/components/FormDetail.tsx index af27db29..a23de716 100644 --- a/client/portal/src/pages/superadmin/forms/components/FormDetail.tsx +++ b/client/portal/src/pages/superadmin/forms/components/FormDetail.tsx @@ -243,7 +243,13 @@ export function FormDetail({ form, data, onRefresh }: FormDetailProps) { - + {/* pb-2/-mb-2: overflow-x-auto makes this a scroll container on + both axes (CSS computes the visible axis to auto), and the + active tab's underline sits 8px below the trigger. The padding + keeps that underline inside the scroll box — otherwise it is + clipped out of sight and the strip scrolls vertically — while + the negative margin keeps the row's height unchanged. */} + Overview From 6f33aeb569675079dfbe2c46d1b4b373c7a3ceac Mon Sep 17 00:00:00 2001 From: Caleb Bae Date: Tue, 8 Sep 2026 10:16:56 -0500 Subject: [PATCH 09/11] chore: update submit confirm dialog text --- .../pages/hacker/apply/components/StepNavigation.tsx | 10 +++++++--- .../portal/src/pages/hacker/apply/steps/ReviewStep.tsx | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/client/portal/src/pages/hacker/apply/components/StepNavigation.tsx b/client/portal/src/pages/hacker/apply/components/StepNavigation.tsx index b6379c3a..848f47cb 100644 --- a/client/portal/src/pages/hacker/apply/components/StepNavigation.tsx +++ b/client/portal/src/pages/hacker/apply/components/StepNavigation.tsx @@ -71,9 +71,13 @@ export function StepNavigation({ Submit your application? - Once submitted, you won't be able to make any further - edits to your application. Please double check your answers - before continuing. + Submitting is{" "} + final + . You{" "} + + cannot edit + {" "} + your application afterwards, so double check your answers. diff --git a/client/portal/src/pages/hacker/apply/steps/ReviewStep.tsx b/client/portal/src/pages/hacker/apply/steps/ReviewStep.tsx index 26c93b7d..69e735a0 100644 --- a/client/portal/src/pages/hacker/apply/steps/ReviewStep.tsx +++ b/client/portal/src/pages/hacker/apply/steps/ReviewStep.tsx @@ -114,7 +114,7 @@ export function ReviewStep({ Review

- Check your answers before submitting — once you submit, your + Check your answers before submitting. Once you submit, your application can no longer be edited.

From 4228aefeb92f130c42b114c5612e28c18c548c65 Mon Sep 17 00:00:00 2001 From: Caleb Bae <144546374+balebbae@users.noreply.github.com> Date: Tue, 8 Sep 2026 08:51:51 -0700 Subject: [PATCH 10/11] feat: public endpoint for tracks (#159) --- .../src/pages/admin/_shared/AppSidebar.tsx | 6 + client/portal/src/pages/admin/index.ts | 1 + .../src/pages/admin/tracks/TracksPage.tsx | 58 ++ client/portal/src/pages/admin/tracks/api.ts | 61 ++ .../tracks/components/TrackFormDialog.tsx | 312 +++++++ .../admin/tracks/components/TracksTable.tsx | 420 +++++++++ .../src/pages/admin/tracks/constants.ts | 10 + client/portal/src/pages/admin/tracks/store.ts | 132 +++ client/portal/src/pages/admin/tracks/types.ts | 29 + .../settings/components/OnboardingDialog.tsx | 14 +- .../settings/tabs/PermissionsTab.tsx | 44 + .../settings/tabs/ResetHackathonCard.tsx | 7 + .../src/pages/superadmin/settings/types.ts | 1 + client/portal/src/routes.tsx | 9 + cmd/api/api.go | 18 + cmd/api/middlewares.go | 28 + cmd/api/public.go | 15 + cmd/api/reset_hackathon.go | 6 +- cmd/api/reset_hackathon_test.go | 3 +- cmd/api/settings.go | 70 ++ cmd/api/tracks.go | 322 +++++++ cmd/api/tracks_test.go | 575 ++++++++++++ .../migrations/000049_add_tracks.down.sql | 2 + .../migrations/000049_add_tracks.up.sql | 16 + ...050_seed_admin_track_edit_enabled.down.sql | 1 + ...00050_seed_admin_track_edit_enabled.up.sql | 2 + docs/docs.go | 817 +++++++++++++++++- internal/store/hackathon.go | 10 +- internal/store/mock_store.go | 52 ++ internal/store/settings.go | 47 + internal/store/storage.go | 11 + internal/store/tracks.go | 228 +++++ internal/store/tracks_test.go | 116 +++ 33 files changed, 3422 insertions(+), 21 deletions(-) create mode 100644 client/portal/src/pages/admin/tracks/TracksPage.tsx create mode 100644 client/portal/src/pages/admin/tracks/api.ts create mode 100644 client/portal/src/pages/admin/tracks/components/TrackFormDialog.tsx create mode 100644 client/portal/src/pages/admin/tracks/components/TracksTable.tsx create mode 100644 client/portal/src/pages/admin/tracks/constants.ts create mode 100644 client/portal/src/pages/admin/tracks/store.ts create mode 100644 client/portal/src/pages/admin/tracks/types.ts create mode 100644 cmd/api/tracks.go create mode 100644 cmd/api/tracks_test.go create mode 100644 cmd/migrate/migrations/000049_add_tracks.down.sql create mode 100644 cmd/migrate/migrations/000049_add_tracks.up.sql create mode 100644 cmd/migrate/migrations/000050_seed_admin_track_edit_enabled.down.sql create mode 100644 cmd/migrate/migrations/000050_seed_admin_track_edit_enabled.up.sql create mode 100644 internal/store/tracks.go create mode 100644 internal/store/tracks_test.go diff --git a/client/portal/src/pages/admin/_shared/AppSidebar.tsx b/client/portal/src/pages/admin/_shared/AppSidebar.tsx index 15834b0c..f1d57621 100644 --- a/client/portal/src/pages/admin/_shared/AppSidebar.tsx +++ b/client/portal/src/pages/admin/_shared/AppSidebar.tsx @@ -11,6 +11,7 @@ import { ScanLine, Settings, Star, + Trophy, UserCheck, Users, } from "lucide-react"; @@ -66,6 +67,11 @@ const eventNav = [ url: "/admin/faq", icon: MessageSquare, }, + { + name: "Tracks", + url: "/admin/tracks", + icon: Trophy, + }, ]; const superAdminNav = [ diff --git a/client/portal/src/pages/admin/index.ts b/client/portal/src/pages/admin/index.ts index 06c8cf92..e23c41a3 100644 --- a/client/portal/src/pages/admin/index.ts +++ b/client/portal/src/pages/admin/index.ts @@ -4,3 +4,4 @@ export { default as ReviewsPage } from "./reviews/ReviewsPage"; export { default as ScansPage } from "./scans/ScansPage"; export { default as SchedulePage } from "./schedule/SchedulePage"; export { default as SponsorsPage } from "./sponsors/SponsorsPage"; +export { default as TracksPage } from "./tracks/TracksPage"; diff --git a/client/portal/src/pages/admin/tracks/TracksPage.tsx b/client/portal/src/pages/admin/tracks/TracksPage.tsx new file mode 100644 index 00000000..b805f40c --- /dev/null +++ b/client/portal/src/pages/admin/tracks/TracksPage.tsx @@ -0,0 +1,58 @@ +import { useEffect } from "react"; + +import { Card, CardContent, CardHeader } from "@/components/ui/card"; +import { Skeleton } from "@/components/ui/skeleton"; + +import { TracksTable } from "./components/TracksTable"; +import { useTracksStore } from "./store"; + +export default function TracksPage() { + const { + tracks, + canEdit, + loading, + saving, + fetch: loadTracks, + createTrack, + updateTrack, + deleteTrack, + uploadLogo, + } = useTracksStore(); + + useEffect(() => { + const controller = new AbortController(); + loadTracks(controller.signal); + return () => controller.abort(); + }, [loadTracks]); + + if (loading && tracks.length === 0) { + return ( +
+ + + + + + {[...Array(3)].map((_, i) => ( + + ))} + + +
+ ); + } + + return ( +
+ +
+ ); +} diff --git a/client/portal/src/pages/admin/tracks/api.ts b/client/portal/src/pages/admin/tracks/api.ts new file mode 100644 index 00000000..083b0e78 --- /dev/null +++ b/client/portal/src/pages/admin/tracks/api.ts @@ -0,0 +1,61 @@ +import { + deleteRequest, + getRequest, + postRequest, + putRequest, +} from "@/shared/lib/api"; +import type { ApiResponse } from "@/types"; + +import type { Track, TrackListResponse, TrackPayload } from "./types"; + +export async function fetchTracks( + signal?: AbortSignal, +): Promise> { + return getRequest("/admin/tracks", "tracks", signal); +} + +export async function fetchTrackEditPermission( + signal?: AbortSignal, +): Promise> { + return getRequest<{ enabled: boolean }>( + "/admin/tracks/edit-permission", + "track edit permission", + signal, + ); +} + +export async function createTrack( + payload: TrackPayload, + signal?: AbortSignal, +): Promise> { + return postRequest("/admin/tracks", payload, "track", signal); +} + +export async function updateTrack( + id: string, + payload: TrackPayload, + signal?: AbortSignal, +): Promise> { + return putRequest(`/admin/tracks/${id}`, payload, "track", signal); +} + +export async function deleteTrack( + id: string, + signal?: AbortSignal, +): Promise> { + return deleteRequest(`/admin/tracks/${id}`, "track", signal); +} + +export async function uploadTrackLogo( + trackId: string, + logoData: string, + contentType: string, + signal?: AbortSignal, +): Promise> { + return putRequest( + `/admin/tracks/${trackId}/logo`, + { logo_data: logoData, content_type: contentType }, + "track logo", + signal, + ); +} diff --git a/client/portal/src/pages/admin/tracks/components/TrackFormDialog.tsx b/client/portal/src/pages/admin/tracks/components/TrackFormDialog.tsx new file mode 100644 index 00000000..f3c935cb --- /dev/null +++ b/client/portal/src/pages/admin/tracks/components/TrackFormDialog.tsx @@ -0,0 +1,312 @@ +import { ImagePlus, Plus, X } from "lucide-react"; +import { useRef, useState } from "react"; +import { toast } from "sonner"; + +import { Button } from "@/components/ui/button"; +import { + Dialog, + DialogContent, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@/components/ui/dialog"; +import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; +import { Textarea } from "@/components/ui/textarea"; + +import { ALLOWED_LOGO_TYPES, MAX_LOGO_BYTES } from "../constants"; +import type { Track, TrackPayload, TrackPrize } from "../types"; + +interface TrackFormDialogProps { + open: boolean; + onOpenChange: (open: boolean) => void; + track: Track | null; + saving: boolean; + onSubmit: (payload: TrackPayload, logoFile?: File) => void; +} + +function logoPreviewFor(track: Track | null): string { + return track?.logo_data + ? `data:${track.logo_content_type};base64,${track.logo_data}` + : ""; +} + +function TrackForm({ + track, + saving, + onSubmit, + onCancel, +}: { + track: Track | null; + saving: boolean; + onSubmit: (payload: TrackPayload, logoFile?: File) => void; + onCancel: () => void; +}) { + const [title, setTitle] = useState(track?.title ?? ""); + const [sponsorName, setSponsorName] = useState(track?.sponsor_name ?? ""); + const [description, setDescription] = useState(track?.description ?? ""); + const [prizes, setPrizes] = useState( + track?.prizes?.length ? track.prizes : [{ place: "1st", prize: "" }], + ); + const [displayOrder, setDisplayOrder] = useState(track?.display_order ?? 0); + const [logoFile, setLogoFile] = useState(null); + const [logoPreview, setLogoPreview] = useState(logoPreviewFor(track)); + const logoInputRef = useRef(null); + + const handleLogoChange = (e: React.ChangeEvent) => { + const file = e.target.files?.[0]; + if (!file) return; + + if (!ALLOWED_LOGO_TYPES.includes(file.type)) { + toast.error("Unsupported file type. Use PNG, JPEG, WebP, or GIF."); + return; + } + + if (file.size > MAX_LOGO_BYTES) { + toast.error("File too large. Maximum size is 750KB."); + return; + } + + setLogoFile(file); + const reader = new FileReader(); + reader.onload = () => setLogoPreview(reader.result as string); + reader.readAsDataURL(file); + }; + + const clearLogo = () => { + setLogoFile(null); + setLogoPreview(logoPreviewFor(track)); + if (logoInputRef.current) logoInputRef.current.value = ""; + }; + + const updatePrize = (index: number, patch: Partial) => { + setPrizes((current) => + current.map((prize, i) => (i === index ? { ...prize, ...patch } : prize)), + ); + }; + + const addPrize = () => { + setPrizes((current) => [...current, { place: "", prize: "" }]); + }; + + const removePrize = (index: number) => { + setPrizes((current) => current.filter((_, i) => i !== index)); + }; + + const handleSubmit = (e: React.FormEvent) => { + e.preventDefault(); + if (!title.trim()) return; + + // Blank rows are the natural state of a freshly added prize, so drop them + // rather than failing validation on the server. + const filledPrizes = prizes + .map((p) => ({ place: p.place.trim(), prize: p.prize.trim() })) + .filter((p) => p.place !== "" || p.prize !== ""); + + if (filledPrizes.some((p) => p.place === "" || p.prize === "")) { + toast.error("Every prize needs both a place and a prize."); + return; + } + + onSubmit( + { + title: title.trim(), + sponsor_name: sponsorName.trim(), + description: description.trim(), + prizes: filledPrizes, + display_order: displayOrder, + }, + logoFile ?? undefined, + ); + }; + + return ( +
+
+ +
+ {logoPreview ? ( + Logo preview + ) : ( +
+ +
+ )} +
+ + + {logoFile && ( + + )} +
+
+

+ PNG, JPEG, WebP, or GIF (max 750KB) +

+
+ +
+ + setTitle(e.target.value)} + placeholder="Best Financial Hack" + required + /> +
+ +
+ + setSponsorName(e.target.value)} + placeholder="Capital One" + /> +
+ +
+ +