diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..0cb8202 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,43 @@ +# apportal + +Go + huma backend (`backend/`), Next.js + TanStack Query frontend (`frontend/`). +The frontend API client is generated from the backend's OpenAPI spec — after +changing a handler's inputs or outputs, run `make openapi` in `backend/` and +`npm run generate:api` in `frontend/`. + +## Never fetch per item — batch it + +**If a page renders N items and needs data for each one, fetch it in one +request, not N.** Build a bulk endpoint that takes the ids and returns them +together. This applies to any per-row detail: answers, reviews, assignments, +counts. + +The N-request version looks fine locally with three rows and falls apart at +scale — browsers cap concurrent connections per host (~6), so the requests +queue in waves and the page fills in raggedly, each one paying its own round +trip, auth middleware, and query planning. + +What this looks like here: + +- **Backend:** a collection route (`GET /answers?application_ids=a,b,c`) + alongside the single-item one, backed by `WHERE id = ANY($1::uuid[])`. + Bound the list (`maxBulkApplications`) so the query string and the fan-out + stay sane, and apply the same visibility rules the single-item route does. +- **Frontend:** one query per *batch*, keyed on the id list. Where rows arrive + in pages (infinite scroll), batch per page so loading more doesn't refetch + what's already in hand. Write each response back into the per-item cache + entries with `queryClient.setQueryData` so single-item views stay warm — the + point is fewer requests, not a worse cache. + +`useAnswersByApplicationIdBatches` in `frontend/src/lib/queries/answers.ts` is +the worked example. + +## Query params the generated client can actually send + +huma binds only primitives from a query string, and axios serializes arrays as +`key[]=…` and objects as `key[0][field]=…` — neither of which huma reads. A +`[]struct` query field silently binds nothing, or panics at request time. + +So for anything that isn't a scalar, take a `string` and parse it in the +handler: JSON for structured filters (`answer_filters`), comma-separated for +id lists (`application_ids`). Return 422 on malformed input. diff --git a/backend/api/openapi.yaml b/backend/api/openapi.yaml index d49c165..710c077 100644 --- a/backend/api/openapi.yaml +++ b/backend/api/openapi.yaml @@ -205,6 +205,35 @@ components: - updated_at - review_status type: object + ApplicationsOutputBody: + additionalProperties: false + properties: + $schema: + description: A URL to the JSON Schema for this object. + examples: + - https://example.com/schemas/ApplicationsOutputBody.json + format: uri + readOnly: true + type: string + applications: + items: + $ref: "#/components/schemas/ApplicationSummary" + type: + - array + - "null" + stage_counts: + additionalProperties: + format: int64 + type: integer + type: object + total: + format: int64 + type: integer + required: + - applications + - total + - stage_counts + type: object AssignRecordingReviewerInputBody: additionalProperties: false properties: @@ -2219,6 +2248,50 @@ info: version: 0.1.0 openapi: 3.1.0 paths: + /answers: + get: + description: One request for a page of applications, instead of one per application. Reviewer-only; draft answers are never included. + operationId: list-answers-bulk + parameters: + - description: Comma-separated application IDs + explode: false + in: query + name: application_ids + schema: + description: Comma-separated application IDs + type: string + responses: + "200": + content: + application/json: + schema: + items: + $ref: "#/components/schemas/WrittenAnswer" + type: + - array + - "null" + description: OK + "401": + content: + application/problem+json: + schema: + $ref: "#/components/schemas/ErrorModel" + description: Unauthorized + "422": + content: + application/problem+json: + schema: + $ref: "#/components/schemas/ErrorModel" + description: Unprocessable Entity + "500": + content: + application/problem+json: + schema: + $ref: "#/components/schemas/ErrorModel" + description: Internal Server Error + summary: List written answers for several applications + tags: + - Answers /applicants: post: description: Applicant-facing; upserts by NUID. @@ -2357,16 +2430,38 @@ paths: schema: description: JSON array of answer filters, e.g. [{"question_id":"…","question_type":"checkbox","values":["Yes"]}]. Values may be a string or an array of strings; a filter matches any of them, and separate filters are AND'd. type: string + - description: Case-insensitive substring match on the applicant's name, NUID, or email + explode: false + in: query + name: search + schema: + description: Case-insensitive substring match on the applicant's name, NUID, or email + type: string + - description: Max results per page; omit (or 0) to return every match + explode: false + in: query + name: limit + schema: + description: Max results per page; omit (or 0) to return every match + format: int64 + maximum: 200 + minimum: 0 + type: integer + - description: Number of results to skip + explode: false + in: query + name: offset + schema: + description: Number of results to skip + format: int64 + minimum: 0 + type: integer responses: "200": content: application/json: schema: - items: - $ref: "#/components/schemas/ApplicationSummary" - type: - - array - - "null" + $ref: "#/components/schemas/ApplicationsOutputBody" description: OK "401": content: diff --git a/backend/internal/handlers/answers.go b/backend/internal/handlers/answers.go index 930a1a5..d77f134 100644 --- a/backend/internal/handlers/answers.go +++ b/backend/internal/handlers/answers.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "net/http" + "strings" "github.com/danielgtaylor/huma/v2" @@ -35,6 +36,50 @@ func (h *answerHandler) register(api huma.API) { Tags: []string{"Answers"}, Errors: []int{http.StatusNotFound}, }, h.list) + + huma.Register(api, huma.Operation{ + OperationID: "list-answers-bulk", + Method: http.MethodGet, + Path: "/answers", + Summary: "List written answers for several applications", + Description: "One request for a page of applications, instead of one per application. Reviewer-only; draft answers are never included.", + Tags: []string{"Answers"}, + Errors: []int{http.StatusUnauthorized, http.StatusUnprocessableEntity}, + }, h.listBulk) +} + +// maxBulkApplications bounds both the query string and the fan-out of the +// underlying `= ANY(...)`. Comfortably above any page size the UI uses. +const maxBulkApplications = 200 + +type ListAnswersBulkInput struct { + // Comma-separated rather than a repeated/array param because huma splits + // this form itself, while the browser client serializes arrays as + // `application_ids[]=…`, which binds to nothing server-side. + ApplicationIDs string `query:"application_ids" doc:"Comma-separated application IDs"` +} + +func (h *answerHandler) listBulk(ctx context.Context, in *ListAnswersBulkInput) (*AnswersOutput, error) { + if err := requireReviewer(ctx); err != nil { + return nil, err + } + ids := make([]string, 0, 8) + for _, id := range strings.Split(in.ApplicationIDs, ",") { + if id = strings.TrimSpace(id); id != "" { + ids = append(ids, id) + } + } + if len(ids) == 0 { + return &AnswersOutput{Body: []models.WrittenAnswer{}}, nil + } + if len(ids) > maxBulkApplications { + return nil, huma.Error422UnprocessableEntity("too many application_ids") + } + answers, err := h.store.ListAnswersForApplications(ctx, ids) + if err != nil { + return nil, storeErr(err) + } + return &AnswersOutput{Body: answers}, nil } type AnswersOutput struct { diff --git a/backend/internal/handlers/answers_test.go b/backend/internal/handlers/answers_test.go index 7fa909b..36f596c 100644 --- a/backend/internal/handlers/answers_test.go +++ b/backend/internal/handlers/answers_test.go @@ -5,9 +5,13 @@ import ( "encoding/json" "errors" "net/http" + "strings" "testing" "github.com/danielgtaylor/huma/v2" + + "github.com/GenerateNU/apportal/backend/internal/middleware" + "github.com/GenerateNU/apportal/backend/internal/models" ) // Ownership and stage checks need a real database (they fetch the @@ -33,3 +37,43 @@ func TestAnswersUpsertRequiresActor(t *testing.T) { t.Fatalf("got %v, want 401", err) } } + +func TestAnswersBulkRequiresReviewer(t *testing.T) { + h := &answerHandler{} + in := &ListAnswersBulkInput{ApplicationIDs: "app-1,app-2"} + + _, err := h.listBulk(context.Background(), in) + var se huma.StatusError + if !errors.As(err, &se) || se.GetStatus() != http.StatusUnauthorized { + t.Fatalf("got %v, want 401", err) + } +} + +// Parsing runs before the store is touched, so the empty and over-limit cases +// are reachable without a database. +func TestAnswersBulkParsesIDs(t *testing.T) { + h := &answerHandler{} + lead := middleware.Actor{NUID: "l1", Roles: []models.UserRole{models.UserRoleLead}} + + // Blank entries are dropped, and an empty list short-circuits to no rows + // rather than querying for none. + out, err := h.listBulk(withActor(lead), &ListAnswersBulkInput{ApplicationIDs: " , ,"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(out.Body) != 0 { + t.Fatalf("got %d answers, want 0", len(out.Body)) + } + + ids := make([]string, maxBulkApplications+1) + for i := range ids { + ids[i] = "app" + } + _, err = h.listBulk(withActor(lead), &ListAnswersBulkInput{ + ApplicationIDs: strings.Join(ids, ","), + }) + var se huma.StatusError + if !errors.As(err, &se) || se.GetStatus() != http.StatusUnprocessableEntity { + t.Fatalf("got %v, want 422", err) + } +} diff --git a/backend/internal/handlers/applications.go b/backend/internal/handlers/applications.go index f0034a6..b18fa73 100644 --- a/backend/internal/handlers/applications.go +++ b/backend/internal/handlers/applications.go @@ -75,8 +75,19 @@ type ApplicationOutput struct { Body models.Application } +// ApplicationsOutput is an envelope rather than a bare array because paging +// happens in SQL: a page of rows says nothing about how many matched, and the +// stage tabs need their own counts. Unpaged callers read `applications` and +// ignore the rest. type ApplicationsOutput struct { - Body []models.ApplicationSummary + Body struct { + Applications []models.ApplicationSummary `json:"applications"` + // Total is every row matching the filter, not just this page. + Total int `json:"total"` + // StageCounts is the same match broken down by stage, ignoring any + // stage filter, so each tab can show a live count. + StageCounts map[string]int `json:"stage_counts"` + } } type CreateApplicationInput struct { @@ -150,6 +161,9 @@ type ListApplicationsInput struct { // string — a []AnswerFilterInput field silently binds nothing (or panics, // depending on how the client serializes it). AnswerFilters string `query:"answer_filters" doc:"JSON array of answer filters, e.g. [{\"question_id\":\"…\",\"question_type\":\"checkbox\",\"values\":[\"Yes\"]}]. Values may be a string or an array of strings; a filter matches any of them, and separate filters are AND'd."` + Search string `query:"search" doc:"Case-insensitive substring match on the applicant's name, NUID, or email"` + Limit int `query:"limit" doc:"Max results per page; omit (or 0) to return every match" minimum:"0" maximum:"200"` + Offset int `query:"offset" doc:"Number of results to skip" minimum:"0"` } func (h *applicationHandler) list(ctx context.Context, in *ListApplicationsInput) (*ApplicationsOutput, error) { @@ -176,6 +190,8 @@ func (h *applicationHandler) list(ctx context.Context, in *ListApplicationsInput UserNUID: in.UserNUID, AssignedTo: in.AssignedTo, AnswerFilters: answerFilters, + Search: in.Search, + Offset: in.Offset, // Only a user listing their own applications by their own identity // ever sees their own draft — the reviewer queue and lookups of // someone else's user_nuid never do. @@ -196,11 +212,24 @@ func (h *applicationHandler) list(ctx context.Context, in *ListApplicationsInput filter.Stage = &parsed } - apps, err := h.store.ListApplications(ctx, filter) + if in.Limit > 0 { + filter.Limit = &in.Limit + } + + // The totals cost a full scan each and are invariant for a given filter, so + // only the first page pays for them; later pages reuse what it returned. + page, err := h.store.ListApplicationsPage(ctx, filter, in.Offset == 0) if err != nil { return nil, storeErr(err) } - return &ApplicationsOutput{Body: apps}, nil + out := &ApplicationsOutput{} + out.Body.Applications = page.Items + out.Body.Total = page.Total + out.Body.StageCounts = make(map[string]int, len(page.StageCounts)) + for stage, n := range page.StageCounts { + out.Body.StageCounts[string(stage)] = n + } + return out, nil } type UpdateApplicationInput struct { diff --git a/backend/internal/store/answers.go b/backend/internal/store/answers.go index 5b6318f..7b7f563 100644 --- a/backend/internal/store/answers.go +++ b/backend/internal/store/answers.go @@ -45,6 +45,10 @@ func (in AnswerInput) isEmpty() bool { const answerColumns = `id, application_id, question_id, answer_text, answer_options, answer_file_path, answer_file_name, submitted_at` +// prefixedAnswerColumns is answerColumns qualified for queries that join +// applications, where the bare names would be ambiguous. +const prefixedAnswerColumns = `wa.id, wa.application_id, wa.question_id, wa.answer_text, wa.answer_options, wa.answer_file_path, wa.answer_file_name, wa.submitted_at` + // UpsertAnswers writes all answers for an application in a single transaction, // keyed on the (application_id, question_id) unique constraint, and returns the // full current answer set. @@ -87,6 +91,29 @@ func (s *Store) UpsertAnswers(ctx context.Context, applicationID string, inputs return s.ListAnswers(ctx, applicationID) } +// ListAnswersForApplications fetches answers for many applications in one +// round trip, for callers rendering a page of applications at once — the +// per-application ListAnswers below turns into a request per row there. +// +// Draft answers are private autosave content, so they are excluded outright +// rather than filtered per caller: this exists for reviewer-facing lists, +// which never show drafts anyway. +func (s *Store) ListAnswersForApplications(ctx context.Context, applicationIDs []string) ([]models.WrittenAnswer, error) { + if len(applicationIDs) == 0 { + return nil, nil + } + const q = `SELECT ` + prefixedAnswerColumns + ` + FROM written_answers wa + JOIN applications a ON a.id = wa.application_id + WHERE wa.application_id = ANY($1::uuid[]) AND a.stage != 'draft' + ORDER BY wa.application_id, wa.submitted_at` + rows, err := s.db.Query(ctx, q, applicationIDs) + if err != nil { + return nil, err + } + return pgx.CollectRows(rows, pgx.RowToStructByPos[models.WrittenAnswer]) +} + func (s *Store) ListAnswers(ctx context.Context, applicationID string) ([]models.WrittenAnswer, error) { const q = `SELECT ` + answerColumns + ` FROM written_answers WHERE application_id = $1 ORDER BY submitted_at` rows, err := s.db.Query(ctx, q, applicationID) diff --git a/backend/internal/store/applications.go b/backend/internal/store/applications.go index 21925da..bf05e51 100644 --- a/backend/internal/store/applications.go +++ b/backend/internal/store/applications.go @@ -75,6 +75,16 @@ type ApplicationFilter struct { // identity — drafts are otherwise invisible (reviewer queues, admin // counts, etc.). IncludeDraft bool + // Search matches the applicant's name, NUID, or email, case-insensitively + // and by substring. + Search string + // Limit caps the page size. Nil returns every matching row — most callers + // (assignment planning, review queues) need the whole set, so paging is + // opt-in rather than the default. + Limit *int + // Offset is the 0-based index of the first row of the page. Ignored unless + // Limit is set. + Offset int } const applicationColumns = `id, cycle_id, user_nuid, application_role, stage, availability, resume_url, submitted_at, updated_at` @@ -114,7 +124,25 @@ func (s *Store) GetApplication(ctx context.Context, id string) (models.Applicati // joined applicant's full_name and email. const applicationSummaryColumns = `a.id, a.cycle_id, a.user_nuid, a.application_role, a.stage, a.availability, a.resume_url, a.submitted_at, a.updated_at, u.full_name, u.email` +// ApplicationPage is one page of the list plus, when asked for, the totals a +// caller needs to size the scroll and label the stage tabs without a second +// request. Both counts are zero-valued when ListApplicationsPage runs without +// them, so read them from the first page rather than the latest. +type ApplicationPage struct { + Items []models.ApplicationSummary + // Total counts every row matching the filter, ignoring Limit/Offset. + Total int + // StageCounts breaks the same match down by stage, ignoring the Stage and + // Stages filters — the tabs need to show what each stage *would* hold, so + // counting with the active stage applied would zero out the others. + StageCounts map[models.ApplicationStage]int +} + +// ListApplications returns every row matching the filter. Paging callers want +// ListApplicationsPage instead; this stays unpaged for the queues and planners +// that need the whole set. func (s *Store) ListApplications(ctx context.Context, f ApplicationFilter) ([]models.ApplicationSummary, error) { + f.Limit, f.Offset = nil, 0 query, args := listApplicationsQuery(f) rows, err := s.db.Query(ctx, query, args...) if err != nil { @@ -123,11 +151,98 @@ func (s *Store) ListApplications(ctx context.Context, f ApplicationFilter) ([]mo return pgx.CollectRows(rows, pgx.RowToStructByPos[models.ApplicationSummary]) } +// ListApplicationsPage runs the list and, when withCounts is set, its total +// and per-stage counts off the same predicate — so the page, the result count, +// and the stage tabs can never disagree about what the filter matched. +// +// The two count queries scan the whole match rather than a page of it, so they +// dominate the request. They also can't change while the filter doesn't, which +// is why a scroll-to-load caller asks for them once on the first page and +// leaves withCounts off for the rest. +func (s *Store) ListApplicationsPage(ctx context.Context, f ApplicationFilter, withCounts bool) (ApplicationPage, error) { + var page ApplicationPage + + query, args := listApplicationsQuery(f) + rows, err := s.db.Query(ctx, query, args...) + if err != nil { + return page, err + } + page.Items, err = pgx.CollectRows(rows, pgx.RowToStructByPos[models.ApplicationSummary]) + if err != nil { + return page, err + } + if !withCounts { + return page, nil + } + + countQuery, countArgs := countApplicationsQuery(f) + if err := s.db.QueryRow(ctx, countQuery, countArgs...).Scan(&page.Total); err != nil { + return page, err + } + + stageQuery, stageArgs := stageCountsQuery(f) + stageRows, err := s.db.Query(ctx, stageQuery, stageArgs...) + if err != nil { + return page, err + } + defer stageRows.Close() + page.StageCounts = map[models.ApplicationStage]int{} + for stageRows.Next() { + var stage models.ApplicationStage + var n int + if err := stageRows.Scan(&stage, &n); err != nil { + return page, err + } + page.StageCounts[stage] = n + } + return page, stageRows.Err() +} + // listApplicationsQuery builds the list statement and its arguments. It is // separate from the call above so the generated SQL — the placeholder // numbering in particular, which shifts with every optional filter — can be // asserted without a database. func listApplicationsQuery(f ApplicationFilter) (string, []any) { + query, args := applicationsFrom(f, applicationFilterAll) + query = `SELECT DISTINCT ` + applicationSummaryColumns + query + + ` ORDER BY a.submitted_at DESC NULLS LAST` + if f.Limit != nil { + args = append(args, *f.Limit) + query += ` LIMIT $` + strconv.Itoa(len(args)) + args = append(args, f.Offset) + query += ` OFFSET $` + strconv.Itoa(len(args)) + } + return query, args +} + +// countApplicationsQuery counts what the list would return unpaged. It counts +// distinct application ids rather than rows so an answer filter's join can +// never inflate the total. +func countApplicationsQuery(f ApplicationFilter) (string, []any) { + query, args := applicationsFrom(f, applicationFilterAll) + return `SELECT COUNT(DISTINCT a.id)` + query, args +} + +// stageCountsQuery counts the same match per stage, with the stage predicate +// itself dropped so every tab shows a live count. +func stageCountsQuery(f ApplicationFilter) (string, []any) { + query, args := applicationsFrom(f, applicationFilterExceptStage) + return `SELECT a.stage, COUNT(DISTINCT a.id)` + query + ` GROUP BY a.stage`, args +} + +// applicationFilterScope selects which predicates applicationsFrom emits. +type applicationFilterScope int + +const ( + applicationFilterAll applicationFilterScope = iota + // applicationFilterExceptStage omits Stage/Stages, for the per-stage counts. + applicationFilterExceptStage +) + +// applicationsFrom builds everything from FROM through WHERE — the part the +// list, the total, and the stage counts must share exactly, since a predicate +// applied to one and not the others would make them contradict each other. +func applicationsFrom(f ApplicationFilter, scope applicationFilterScope) (string, []any) { // A valueless filter can't narrow anything and would emit an empty OR list // below, so drop those before they reach the query. answerFilters := make([]AnswerFilter, 0, len(f.AnswerFilters)) @@ -138,7 +253,7 @@ func listApplicationsQuery(f ApplicationFilter) (string, []any) { answerFilters = append(answerFilters, af) } - query := `SELECT DISTINCT ` + applicationSummaryColumns + ` FROM applications a JOIN users u ON u.nuid = a.user_nuid` + query := ` FROM applications a JOIN users u ON u.nuid = a.user_nuid` args := []any{} // One join per answer filter, each pinned to that filter's question. The @@ -165,17 +280,26 @@ func listApplicationsQuery(f ApplicationFilter) (string, []any) { args = append(args, *f.Role) query += ` AND a.application_role = $` + strconv.Itoa(len(args)) } - if f.Stage != nil { - args = append(args, *f.Stage) - query += ` AND a.stage = $` + strconv.Itoa(len(args)) - } - if len(f.Stages) > 0 { - stages := make([]string, len(f.Stages)) - for i, s := range f.Stages { - stages[i] = string(s) + if scope != applicationFilterExceptStage { + if f.Stage != nil { + args = append(args, *f.Stage) + query += ` AND a.stage = $` + strconv.Itoa(len(args)) + } + if len(f.Stages) > 0 { + stages := make([]string, len(f.Stages)) + for i, s := range f.Stages { + stages[i] = string(s) + } + args = append(args, stages) + query += ` AND a.stage = ANY($` + strconv.Itoa(len(args)) + `::application_stage[])` } - args = append(args, stages) - query += ` AND a.stage = ANY($` + strconv.Itoa(len(args)) + `::application_stage[])` + } + if f.Search != "" { + args = append(args, "%"+escapeLike(f.Search)+"%") + n := strconv.Itoa(len(args)) + query += ` AND (u.full_name ILIKE $` + n + ` ESCAPE '\'` + + ` OR a.user_nuid ILIKE $` + n + ` ESCAPE '\'` + + ` OR u.email ILIKE $` + n + ` ESCAPE '\')` } if f.AssignedTo != "" { args = append(args, f.AssignedTo) @@ -214,8 +338,6 @@ func listApplicationsQuery(f ApplicationFilter) (string, []any) { } } - query += ` ORDER BY a.submitted_at DESC NULLS LAST` - return query, args } diff --git a/backend/internal/store/applications_test.go b/backend/internal/store/applications_test.go index 7cdf1fc..3428d96 100644 --- a/backend/internal/store/applications_test.go +++ b/backend/internal/store/applications_test.go @@ -2,6 +2,8 @@ package store import ( "reflect" + "regexp" + "strconv" "strings" "testing" @@ -115,9 +117,7 @@ func TestListApplicationsQueryAnswerFilters(t *testing.T) { if !reflect.DeepEqual(args, tc.wantArgs) { t.Errorf("args = %#v, want %#v", args, tc.wantArgs) } - if n := strings.Count(query, "$"); n != len(args) { - t.Errorf("query has %d placeholders but %d args\ngot: %s", n, len(args), query) - } + assertPlaceholdersMatchArgs(t, query, args) }) } } @@ -145,3 +145,119 @@ func TestListApplicationsQueryStages(t *testing.T) { t.Fatalf("args = %#v, want %#v", args, want) } } + +func TestListApplicationsQueryPaging(t *testing.T) { + limit := 50 + query, args := listApplicationsQuery(ApplicationFilter{ + CycleID: "c1", + Limit: &limit, + Offset: 100, + }) + // LIMIT/OFFSET come last, so their placeholders follow every filter's. + if !strings.Contains(query, `ORDER BY a.submitted_at DESC NULLS LAST LIMIT $2 OFFSET $3`) { + t.Fatalf("paging clause wrong: %s", query) + } + if want := []any{"c1", 50, 100}; !reflect.DeepEqual(args, want) { + t.Fatalf("args = %#v, want %#v", args, want) + } +} + +func TestListApplicationsQueryUnpagedByDefault(t *testing.T) { + query, _ := listApplicationsQuery(ApplicationFilter{CycleID: "c1"}) + if strings.Contains(query, "LIMIT") || strings.Contains(query, "OFFSET") { + t.Fatalf("no limit set should not page: %s", query) + } +} + +// The page, the total, and the stage tabs must agree about what matched, which +// only holds if they share a predicate. These assert the shared part is +// identical and that only the intended pieces differ. +func TestCountAndStageCountsShareThePredicate(t *testing.T) { + limit := 25 + f := ApplicationFilter{ + CycleID: "c1", + Search: "ho", + Stage: stagePtr(models.StageSubmitted), + AnswerFilters: []AnswerFilter{ + {QuestionID: "q1", Match: MatchContains, Values: []string{"boston"}}, + }, + Limit: &limit, + Offset: 50, + } + + countQuery, countArgs := countApplicationsQuery(f) + if !strings.HasPrefix(countQuery, `SELECT COUNT(DISTINCT a.id) FROM applications a`) { + t.Fatalf("count query shape: %s", countQuery) + } + // The total is of every match, so paging must not leak into it. + if strings.Contains(countQuery, "LIMIT") || strings.Contains(countQuery, "OFFSET") { + t.Fatalf("count must ignore paging: %s", countQuery) + } + if !strings.Contains(countQuery, `a.stage = $`) { + t.Fatalf("count must honour the stage filter: %s", countQuery) + } + + stageQuery, stageArgs := stageCountsQuery(f) + // Dropping the stage predicate is the whole point: with it applied every + // other tab would read zero. + if strings.Contains(stageQuery, `a.stage = $`) { + t.Fatalf("stage counts must ignore the stage filter: %s", stageQuery) + } + if !strings.HasSuffix(stageQuery, ` GROUP BY a.stage`) { + t.Fatalf("stage counts must group by stage: %s", stageQuery) + } + // Every other predicate still applies, so the tabs track the live filters. + for _, want := range []string{`a.cycle_id = $`, `u.full_name ILIKE $`, `wa0.answer_text ILIKE $`} { + if !strings.Contains(stageQuery, want) { + t.Errorf("stage counts missing %q\ngot: %s", want, stageQuery) + } + } + // One fewer arg than the count query: the dropped stage predicate. + if len(stageArgs) != len(countArgs)-1 { + t.Errorf("stage args = %#v, count args = %#v", stageArgs, countArgs) + } + assertPlaceholdersMatchArgs(t, countQuery, countArgs) + assertPlaceholdersMatchArgs(t, stageQuery, stageArgs) +} + +func TestSearchMatchesNameNuidAndEmail(t *testing.T) { + query, args := listApplicationsQuery(ApplicationFilter{Search: "50%"}) + for _, want := range []string{ + `u.full_name ILIKE $1 ESCAPE '\'`, + `a.user_nuid ILIKE $1 ESCAPE '\'`, + `u.email ILIKE $1 ESCAPE '\'`, + } { + if !strings.Contains(query, want) { + t.Errorf("query missing %q\ngot: %s", want, query) + } + } + // One argument reused across all three columns, with LIKE's own + // metacharacters escaped. + if want := []any{`%50\%%`}; !reflect.DeepEqual(args, want) { + t.Fatalf("args = %#v, want %#v", args, want) + } +} + +func stagePtr(s models.ApplicationStage) *models.ApplicationStage { return &s } + +var placeholderPattern = regexp.MustCompile(`\$(\d+)`) + +// assertPlaceholdersMatchArgs checks the highest $N in the query equals the +// number of args. Counting occurrences instead would miscount the search +// predicate, which deliberately reuses one placeholder across three columns. +func assertPlaceholdersMatchArgs(t *testing.T, query string, args []any) { + t.Helper() + highest := 0 + for _, m := range placeholderPattern.FindAllStringSubmatch(query, -1) { + n, err := strconv.Atoi(m[1]) + if err != nil { + t.Fatalf("unparsable placeholder %q", m[0]) + } + if n > highest { + highest = n + } + } + if highest != len(args) { + t.Errorf("highest placeholder is $%d but there are %d args\ngot: %s", highest, len(args), query) + } +} diff --git a/frontend/src/app/(portal)/reviewer/applications/components/AnswerCell.tsx b/frontend/src/app/(portal)/reviewer/applications/components/AnswerCell.tsx index 83443fb..a740938 100644 --- a/frontend/src/app/(portal)/reviewer/applications/components/AnswerCell.tsx +++ b/frontend/src/app/(portal)/reviewer/applications/components/AnswerCell.tsx @@ -16,16 +16,26 @@ export function AnswerCell({ applicable, questionType, truncate = true, + loading = false, }: { answer: WrittenAnswer | undefined applicable: boolean questionType?: QuestionType truncate?: boolean + // The answers for this application are still in flight. Distinct from + // having none, which is what an absent `answer` means once they land. + loading?: boolean }) { if (!applicable) { return — } + // Blank rather than "No response": until the answers arrive we don't know + // whether there is one, and claiming there isn't reads as a real answer. + if (loading && !answer) { + return null + } + const text = formatAnswer(answer) const isFile = questionType === 'url' && answer?.answer_file_path const isUrl = questionType === 'url' && answer && answer.answer_text?.trim() diff --git a/frontend/src/app/(portal)/reviewer/applications/components/ApplicantRow.tsx b/frontend/src/app/(portal)/reviewer/applications/components/ApplicantRow.tsx index 950628b..ab41cf4 100644 --- a/frontend/src/app/(portal)/reviewer/applications/components/ApplicantRow.tsx +++ b/frontend/src/app/(portal)/reviewer/applications/components/ApplicantRow.tsx @@ -9,6 +9,7 @@ export function ApplicantRow({ columns, rowQuestions, answers, + answersLoading, availabilityDays, selectable, selected, @@ -20,6 +21,9 @@ export function ApplicantRow({ columns: Question[] rowQuestions: Question[] answers: WrittenAnswer[] + // This row's answers haven't arrived yet, so its cells stay blank instead of + // asserting "No response". + answersLoading: boolean availabilityDays: string[] selectable: boolean selected: boolean @@ -28,9 +32,12 @@ export function ApplicantRow({ onSelect: () => void }) { return ( + // A row's content lands in three waves — the application, then its + // cycle's questions, then its answers — so its height is pinned up front. + // Without that, every wave reflows the row and the whole list jumps. @@ -67,6 +74,7 @@ export function ApplicantRow({ } applicable={!!rowQuestion} questionType={q.question_type} + loading={answersLoading} /> ) @@ -79,7 +87,11 @@ export function ApplicantRow({ {availabilityDays.length > 0 ? ( -
+ // Four day chips are wider than the "Availability" header, so + // wrapping would put them on two lines and make this the tallest + // cell in the row — and they arrive last, so the row would grow + // after everything else had settled. +
{availabilityDays.map((d) => ( void }) { const router = useRouter() @@ -27,14 +31,16 @@ export function ApplicationDetail({ return ( <> - {/* Backdrop */} + {/* Backdrop. Both layers need an explicit z-index: being `fixed` alone + leaves them at z-auto, which the table's sticky header (z-20) paints + straight over. */}
{/* Drawer */}
e.stopPropagation()} > {/* Header */} @@ -89,6 +95,7 @@ export function ApplicationDetail({ question={q} answer={answer} applicable={!!rowQuestion} + loading={answersLoading} /> ) })} diff --git a/frontend/src/app/(portal)/reviewer/applications/components/ApplicationsClient.tsx b/frontend/src/app/(portal)/reviewer/applications/components/ApplicationsClient.tsx index 2f4508f..c11d5b4 100644 --- a/frontend/src/app/(portal)/reviewer/applications/components/ApplicationsClient.tsx +++ b/frontend/src/app/(portal)/reviewer/applications/components/ApplicationsClient.tsx @@ -1,7 +1,7 @@ 'use client' import { PageContainer } from '@/components/PageContainer' -import { useMemo, useState } from 'react' +import { useEffect, useMemo, useState } from 'react' import { Search, List, Columns } from 'lucide-react' import { Select, @@ -16,15 +16,16 @@ import type { Role, WrittenAnswer, } from '@/lib/api/types' -import { useAnswersByApplicationIds } from '@/lib/queries/answers' +import { useAnswersByApplicationIdBatches } from '@/lib/queries/answers' import { - useApplications, + useInfiniteApplications, useUpdateApplication, } from '@/lib/queries/applications' import { pickDefaultCycleId, useCycles } from '@/lib/queries/cycles' import { useQuestionsByCycleRoles } from '@/lib/queries/questions' import { useCurrentUser } from '@/lib/queries/users' import { ROLE_COLUMNS, ROLE_LABEL } from '@/lib/roles' +import { PAGE_SIZE } from './constants' import { BulkActionBar } from './BulkActionBar' import { AVAILABILITY_DAY_OPTIONS, @@ -40,6 +41,10 @@ import { ApplicationDetail } from './ApplicationDetail' type View = 'table' | 'kanban' +// How long typing settles before the search hits the server. Long enough that +// a typed word is one request, short enough to still feel live. +const SEARCH_DEBOUNCE_MS = 250 + export function ApplicationsClient() { const { data: currentUser } = useCurrentUser() const isChief = !!currentUser?.roles.some( @@ -67,6 +72,17 @@ export function ApplicationsClient() { const [bulkFailed, setBulkFailed] = useState(0) const updateApplication = useUpdateApplication() + // The search box stays instant while the request it drives waits for a + // pause in typing. + const [debouncedSearch, setDebouncedSearch] = useState('') + useEffect(() => { + const timer = setTimeout( + () => setDebouncedSearch(search), + SEARCH_DEBOUNCE_MS + ) + return () => clearTimeout(timer) + }, [search]) + const { data: cycles = [] } = useCycles({}) // Default the cycle filter so reviewers land on a specific cycle instead @@ -82,28 +98,6 @@ export function ApplicationsClient() { setCycleDefaulted(true) } - // Scoped server-side to the selected cycle+role (both required, so this is - // always exactly what's on screen) — keeps the question/answer batch below - // limited to applications actually in view instead of every application. - const { data: applications = [] } = useApplications( - activeCycle - ? { - cycle_id: activeCycle, - role: activeRole, - // Omitted entirely when unfiltered, so the key matches the server - // prefetch in ../page.tsx — an extra `answer_filters: []` would make - // the first paint a cache miss. - ...(filters.length > 0 && { - answer_filters: filters.map((f) => ({ - question_id: f.question_id, - question_type: f.question_type, - values: f.values, - })), - }), - } - : undefined - ) - // Taken from the selected cycle+role rather than from the results, since a // filter that matches nothing would otherwise empty the question list the // filter UI itself is built from. Every application in view is this pair. @@ -122,19 +116,119 @@ export function ApplicationsClient() { return map }, [uniquePairs, questionQueries]) - const applicationIds = useMemo( - () => applications.map((a) => a.id), - [applications] + // "Meeting Availability for the Fall Semester" is a regular checkbox + // question authored per cycle/role in the admin builder, not a dedicated + // field — every application on screen shares one cycle+role, so there's at + // most one such question in view at a time. + const availabilityQuestionId = useMemo( + () => + findAvailabilityQuestionId( + questionsByCycleRole[`${activeCycle}:${activeRole}`] + ), + [questionsByCycleRole, activeCycle, activeRole] ) - const answerQueries = useAnswersByApplicationIds(applicationIds) + + // The availability dropdown filters by day, but the stored answer holds the + // full option label ("Monday 6:00-7:30 PM") and the wording drifts between + // cycles. Expanding the day to the matching labels here — where the options + // are already loaded — keeps the server filter an exact any-of match and + // keeps it consistent with the day tags in the table, which come from the + // same list. + const availabilityFilter = useMemo(() => { + if (activeAvailability === 'all' || !availabilityQuestionId) return null + const day = AVAILABILITY_DAY_OPTIONS.find( + (d) => d.code === activeAvailability + ) + const options = + questionsByCycleRole[`${activeCycle}:${activeRole}`]?.find( + (q) => q.id === availabilityQuestionId + )?.options ?? [] + const values = options.filter((o) => + o.toLowerCase().includes(day?.day ?? '') + ) + if (!day || values.length === 0) return null + return { + question_id: availabilityQuestionId, + question_type: 'checkbox' as const, + values, + } + }, [ + activeAvailability, + availabilityQuestionId, + questionsByCycleRole, + activeCycle, + activeRole, + ]) + + // Every filter is applied in SQL, so the page the table renders is already + // the answer — nothing below narrows it further. That is what makes the + // totals and the stage counts trustworthy: they describe the same match, + // counted server-side over every row rather than the page in hand. + const listParams = useMemo(() => { + if (!activeCycle) return undefined + const answerFilters = [ + ...filters.map((f) => ({ + question_id: f.question_id, + question_type: f.question_type, + values: f.values, + })), + ...(availabilityFilter ? [availabilityFilter] : []), + ] + return { + cycle_id: activeCycle, + role: activeRole, + ...(debouncedSearch && { search: debouncedSearch }), + // Each of these is omitted when inactive so an unfiltered first page + // keys identically to the server prefetch in ../page.tsx. + ...(answerFilters.length > 0 && { answer_filters: answerFilters }), + // Kanban lays every stage out side by side, so it can neither filter by + // one stage nor take a page — it asks for the whole set instead. + ...(view === 'table' && { + ...(activeStage !== 'all' && { stage: activeStage }), + limit: PAGE_SIZE, + }), + } + }, [ + activeCycle, + activeRole, + activeStage, + debouncedSearch, + filters, + availabilityFilter, + view, + ]) + + const { + applications, + applicationIdPages, + stageCounts, + isFetching: fetchingApplications, + fetchNextPage, + hasNextPage, + isFetchingNextPage, + } = useInfiniteApplications(listParams) + + // One request per loaded page rather than one per row. + const answerQueries = useAnswersByApplicationIdBatches(applicationIdPages) const answersByApplicationId = useMemo(() => { const map: Record = {} - applicationIds.forEach((id, i) => { - const data = answerQueries[i]?.data - if (data) map[id] = data + for (const query of answerQueries) { + Object.assign(map, query.data ?? {}) + } + return map + }, [answerQueries]) + + // A whole page's answers land at once, so every row in it shares its + // batch's state. Cells read this to stay blank until then rather than + // reporting "No response" for an answer nobody has looked up yet. + const answersLoadingByApplicationId = useMemo(() => { + const map: Record = {} + applicationIdPages.forEach((ids, i) => { + const pending = answerQueries[i]?.isPending ?? true + for (const id of ids) map[id] = pending }) return map - }, [applicationIds, answerQueries]) + }, [applicationIdPages, answerQueries]) const rows: ApplicantApplication[] = useMemo( () => @@ -151,17 +245,6 @@ export function ApplicationsClient() { [applications] ) - // "Meeting Availability for the Fall Semester" is a regular checkbox - // question authored per cycle/role in the admin builder, not a dedicated - // field — every application on screen shares one cycle+role, so there's at - // most one such question in view at a time. - const availabilityQuestionId = useMemo( - () => - findAvailabilityQuestionId( - questionsByCycleRole[`${activeCycle}:${activeRole}`] - ), - [questionsByCycleRole, activeCycle, activeRole] - ) const availabilityByApplicationId = useMemo(() => { const map: Record = {} for (const app of applications) { @@ -175,30 +258,11 @@ export function ApplicationsClient() { return map }, [applications, answersByApplicationId, availabilityQuestionId]) - // Everything but the stage filter — used both as the base for `filtered` - // and as the denominator for the stage tab counts, so those counts track - // search instead of always reflecting every application in the cycle+role. - const filteredExceptStage = rows.filter((a) => { - const query = search.toLowerCase() - if ( - query && - !a.fullName.toLowerCase().includes(query) && - !a.nuid.toLowerCase().includes(query) - ) { - return false - } - if ( - activeAvailability !== 'all' && - !availabilityByApplicationId[a.id]?.includes(activeAvailability) - ) { - return false - } - return true - }) - - const filtered = filteredExceptStage.filter( - (a) => view === 'kanban' || activeStage === 'all' || a.stage === activeStage - ) + // No client-side narrowing left: search, availability, and stage are all in + // the query above, so these rows are the page as the database returned it. + // Kanban is the exception — it groups by stage itself, so it asks for the + // unpaged set separately below. + const filtered = rows function toggleSelect(id: string) { setSelectedIds((prev) => { @@ -267,8 +331,12 @@ export function ApplicationsClient() { ) }, [questionsByCycleRole]) + // min-h-0 is what makes the table's own pane the scroll container: without + // it this flex item can't shrink below its content, so it grows to the + // table's full height and the ancestor in (portal)/layout.tsx scrolls + // instead — which leaves the sticky header with no scrollport to pin against. return ( - +

Applications @@ -363,12 +431,13 @@ export function ApplicationsClient() { {view === 'table' ? ( ) : undefined } + // Only dim for a fresh query, not for the append — the rows + // already on screen stay put while the next chunk loads. + loading={fetchingApplications && !isFetchingNextPage} + hasMore={hasNextPage} + loadingMore={isFetchingNextPage} + onLoadMore={fetchNextPage} /> ) : ( setSelectedApplicationId(null)} /> ) : null diff --git a/frontend/src/app/(portal)/reviewer/applications/components/ResponseField.tsx b/frontend/src/app/(portal)/reviewer/applications/components/ResponseField.tsx index af68c1b..68e6716 100644 --- a/frontend/src/app/(portal)/reviewer/applications/components/ResponseField.tsx +++ b/frontend/src/app/(portal)/reviewer/applications/components/ResponseField.tsx @@ -6,10 +6,12 @@ export function ResponseField({ question, answer, applicable, + loading, }: { question: Question answer: WrittenAnswer | undefined applicable: boolean + loading?: boolean }) { return (
@@ -21,6 +23,7 @@ export function ResponseField({ applicable={applicable} questionType={question.question_type} truncate={false} + loading={loading} />

diff --git a/frontend/src/app/(portal)/reviewer/applications/components/TableView.tsx b/frontend/src/app/(portal)/reviewer/applications/components/TableView.tsx index e1eafd0..fdf72ed 100644 --- a/frontend/src/app/(portal)/reviewer/applications/components/TableView.tsx +++ b/frontend/src/app/(portal)/reviewer/applications/components/TableView.tsx @@ -1,4 +1,4 @@ -import { useMemo } from 'react' +import { Fragment, useEffect, useMemo, useRef } from 'react' import type { Question, WrittenAnswer } from '@/lib/api/types' import type { ApplicantApplication, ApplicationStage } from './types' import type { AnswerFilter } from './FilterButton' @@ -9,14 +9,20 @@ import { FilterChips } from './FilterButton' const TRAILING_COLUMNS = ['Stage', 'Submitted', 'Availability'] +// How many rows from the end the next fetch starts. Counted in rows rather +// than pixels so it doesn't encode an assumption about row height, zoom, or +// font size — the trigger sits at a fixed position in the list regardless. +const LOAD_AHEAD_ROWS = 15 + export function TableView({ applicants, - allApplicants, + stageCounts, activeStage, onStageChange, columns, questionsByCycleRole, answersByApplicationId, + answersLoadingByApplicationId, availabilityByApplicationId, selectable, selectedIds, @@ -27,14 +33,22 @@ export function TableView({ filters, onFilterChange, bulkBar, + loading, + hasMore, + loadingMore, + onLoadMore, }: { applicants: ApplicantApplication[] - allApplicants: ApplicantApplication[] + // Counted server-side over the whole match, not the page — a page's worth of + // rows can't tell you how many are in each stage. + stageCounts: Record activeStage: ApplicationStage | 'all' onStageChange: (s: ApplicationStage | 'all') => void columns: Question[] questionsByCycleRole: Record answersByApplicationId: Record + // Per application, whether its answers request is still in flight. + answersLoadingByApplicationId: Record availabilityByApplicationId: Record // Row/select-all checkboxes only make sense alongside the bulk-move // toolbar, which is chief/admin-only — other reviewers never see them. @@ -52,11 +66,17 @@ export function TableView({ // Rendered in the filter row's place while a selection is active. Owned by // the parent, which holds the selection and the bulk mutation. bulkBar?: React.ReactNode + // A fetch is in flight while the previous rows are still on screen. + loading?: boolean + hasMore?: boolean + loadingMore?: boolean + onLoadMore?: () => void }) { + // "All" is the sum rather than a separate count, since stageCounts already + // excludes drafts and reflects every other active filter. + const totalCount = Object.values(stageCounts).reduce((sum, n) => sum + n, 0) const countByStage = (stage: ApplicationStage | 'all') => - stage === 'all' - ? allApplicants.length - : allApplicants.filter((a) => a.stage === stage).length + stage === 'all' ? totalCount : (stageCounts[stage] ?? 0) const allSelected = applicants.length > 0 && applicants.every((a) => selectedIds.has(a.id)) @@ -73,6 +93,30 @@ export function TableView({ const columnCount = tableColumns.length + TRAILING_COLUMNS.length + (selectable ? 1 : 0) + // Load the next page when the sentinel enters the scroll pane. Rooted at the + // pane rather than the viewport, since the pane is what actually scrolls. + const scrollRef = useRef(null) + const sentinelRef = useRef(null) + // Sits LOAD_AHEAD_ROWS from the end, so it comes into view — and starts the + // fetch — while there are still that many rows left to scroll through. + // Clamped to 0 so a short list triggers immediately and fills the pane. + const triggerIndex = Math.max(0, applicants.length - LOAD_AHEAD_ROWS) + useEffect(() => { + const sentinel = sentinelRef.current + const root = scrollRef.current + if (!sentinel || !root || !hasMore || !onLoadMore) return + const observer = new IntersectionObserver( + ([entry]) => { + if (entry.isIntersecting) onLoadMore() + }, + { root } + ) + observer.observe(sentinel) + return () => observer.disconnect() + // triggerIndex re-runs this after every append, so the observer follows + // the sentinel to its new position instead of watching a detached node. + }, [hasMore, onLoadMore, triggerIndex]) + return (
@@ -103,12 +147,20 @@ export function TableView({
-
+ {/* This pane is the only thing that scrolls, which is what the sticky + header below pins against — the toolbar above sits outside it and + stays put. */} +
- - + + {selectable && ( - @@ -133,7 +185,7 @@ export function TableView({ {TRAILING_COLUMNS.map((label) => ( @@ -142,22 +194,32 @@ export function TableView({ {applicants.length > 0 ? ( - applicants.map((a) => ( - onToggleSelect(a.id)} - isSelected={selectedApplicationId === a.id} - onSelect={() => onSelectApplication(a.id)} - /> + applicants.map((a, i) => ( + + onToggleSelect(a.id)} + isSelected={selectedApplicationId === a.id} + onSelect={() => onSelectApplication(a.id)} + /> + {i === triggerIndex && ( + // 1px rather than 0: a zero-area target is an edge case + // IntersectionObserver implementations disagree on, and a + // tripwire that never fires is the whole bug. + + + )} + )) ) : ( @@ -182,6 +244,18 @@ export function TableView({ /> ))} + {/* Breathing room past the last row once the list is long enough + to scroll, and the only place the append announces itself now + that there's no footer. Fixed height either way, so nothing + shifts when the message appears. */} + + +
+ {q.question_text} {label}
+
+ {loadingMore ? 'Loading more…' : ''} +
diff --git a/frontend/src/app/(portal)/reviewer/applications/components/constants.ts b/frontend/src/app/(portal)/reviewer/applications/components/constants.ts index 9aaa3e7..44fd4be 100644 --- a/frontend/src/app/(portal)/reviewer/applications/components/constants.ts +++ b/frontend/src/app/(portal)/reviewer/applications/components/constants.ts @@ -1,5 +1,11 @@ import type { ApplicationStage } from '@/lib/api/types' +// Rows fetched per scroll step in the applications table. Paging happens in +// SQL, so this is the size of each request as well as of each append — the +// server prefetch in ../page.tsx has to use it too, or its cache entry keys +// differently from the one the client mounts with. +export const PAGE_SIZE = 25 + export const ORDERED_STAGES: ApplicationStage[] = [ 'submitted', 'lead_review', diff --git a/frontend/src/app/(portal)/reviewer/applications/page.tsx b/frontend/src/app/(portal)/reviewer/applications/page.tsx index 622eb9d..3a04a62 100644 --- a/frontend/src/app/(portal)/reviewer/applications/page.tsx +++ b/frontend/src/app/(portal)/reviewer/applications/page.tsx @@ -3,13 +3,14 @@ import { HydrationBoundary, QueryClient, } from '@tanstack/react-query' -import { listApplications } from '@/generated/applications/applications' +import { fetchApplicationPage } from '@/lib/queries/applications' import { listCycles } from '@/generated/cycles/cycles' import { getServerRequestOptions } from '@/lib/api/server-request-options' import type { Cycle } from '@/lib/api/types' import { defaultApplicationsCycleId } from '@/lib/cycles' import { queryKeys } from '@/lib/queries/keys' import { ROLE_COLUMNS } from '@/lib/roles' +import { PAGE_SIZE } from './components/constants' import { ApplicationsClient } from './components/ApplicationsClient' // Auth-gated, live data fetched per request from the backend — never prerender @@ -32,11 +33,20 @@ export default async function ApplicationsPage() { const cycleId = defaultApplicationsCycleId(cycles) if (cycleId) { - const params = { cycle_id: cycleId, role: ROLE_COLUMNS[0] } - await queryClient.prefetchQuery({ - queryKey: queryKeys.applications.list(params), - queryFn: async () => - (await listApplications(params, requestOptions)) ?? [], + // The table scrolls to load, so this has to be an infinite prefetch: a + // plain one caches a bare response where the client expects + // `{ pages, pageParams }`, and the mismatch costs the round trip it was + // meant to save. Offset is the page param, so it stays out of the key. + const params = { + cycle_id: cycleId, + role: ROLE_COLUMNS[0], + limit: PAGE_SIZE, + } + await queryClient.prefetchInfiniteQuery({ + queryKey: queryKeys.applications.infiniteList(params), + queryFn: ({ pageParam }) => + fetchApplicationPage({ ...params, offset: pageParam }, requestOptions), + initialPageParam: 0, }) } diff --git a/frontend/src/app/(portal)/reviewer/assignments/page.tsx b/frontend/src/app/(portal)/reviewer/assignments/page.tsx index 0afbc76..676b46e 100644 --- a/frontend/src/app/(portal)/reviewer/assignments/page.tsx +++ b/frontend/src/app/(portal)/reviewer/assignments/page.tsx @@ -3,7 +3,7 @@ import { HydrationBoundary, QueryClient, } from '@tanstack/react-query' -import { listApplications } from '@/generated/applications/applications' +import { fetchApplicationPage } from '@/lib/queries/applications' import { listUsers } from '@/generated/users/users' import { getServerRequestOptions } from '@/lib/api/server-request-options' import { queryKeys } from '@/lib/queries/keys' @@ -22,7 +22,7 @@ export default async function AssignmentsPage() { await Promise.all([ queryClient.prefetchQuery({ queryKey: queryKeys.applications.list({}), - queryFn: async () => (await listApplications({}, requestOptions)) ?? [], + queryFn: () => fetchApplicationPage({}, requestOptions), }), queryClient.prefetchQuery({ queryKey: [...queryKeys.users.lists(), 'lead'], diff --git a/frontend/src/app/(portal)/reviewer/chief-review/page.tsx b/frontend/src/app/(portal)/reviewer/chief-review/page.tsx index 0f67255..2808677 100644 --- a/frontend/src/app/(portal)/reviewer/chief-review/page.tsx +++ b/frontend/src/app/(portal)/reviewer/chief-review/page.tsx @@ -3,7 +3,7 @@ import { HydrationBoundary, QueryClient, } from '@tanstack/react-query' -import { listApplications } from '@/generated/applications/applications' +import { fetchApplicationPage } from '@/lib/queries/applications' import { listCycles } from '@/generated/cycles/cycles' import { getServerRequestOptions } from '@/lib/api/server-request-options' import type { Cycle } from '@/lib/api/types' @@ -33,8 +33,7 @@ export default async function ChiefReviewQueuePage() { const params = { cycle_id: cycleId } await queryClient.prefetchQuery({ queryKey: queryKeys.applications.list(params), - queryFn: async () => - (await listApplications(params, requestOptions)) ?? [], + queryFn: () => fetchApplicationPage(params, requestOptions), }) } diff --git a/frontend/src/app/(portal)/reviewer/interview-assignments/page.tsx b/frontend/src/app/(portal)/reviewer/interview-assignments/page.tsx index 7a904ce..5409042 100644 --- a/frontend/src/app/(portal)/reviewer/interview-assignments/page.tsx +++ b/frontend/src/app/(portal)/reviewer/interview-assignments/page.tsx @@ -3,7 +3,7 @@ import { HydrationBoundary, QueryClient, } from '@tanstack/react-query' -import { listApplications } from '@/generated/applications/applications' +import { fetchApplicationPage } from '@/lib/queries/applications' import { getServerRequestOptions } from '@/lib/api/server-request-options' import { queryKeys } from '@/lib/queries/keys' import { InterviewAssignmentsClient } from './components/InterviewAssignmentsClient' @@ -20,7 +20,7 @@ export default async function InterviewAssignmentsPage() { // users), so there's nothing to fetch per applicant here. await queryClient.prefetchQuery({ queryKey: queryKeys.applications.list({}), - queryFn: async () => (await listApplications({}, requestOptions)) ?? [], + queryFn: () => fetchApplicationPage({}, requestOptions), }) return ( diff --git a/frontend/src/app/(portal)/reviewer/my-reviews/page.tsx b/frontend/src/app/(portal)/reviewer/my-reviews/page.tsx index 77f7ac1..d3806e0 100644 --- a/frontend/src/app/(portal)/reviewer/my-reviews/page.tsx +++ b/frontend/src/app/(portal)/reviewer/my-reviews/page.tsx @@ -3,7 +3,7 @@ import { HydrationBoundary, QueryClient, } from '@tanstack/react-query' -import { listApplications } from '@/generated/applications/applications' +import { fetchApplicationPage } from '@/lib/queries/applications' import { listCycles } from '@/generated/cycles/cycles' import { getCurrentUser } from '@/generated/users/users' import { getServerRequestOptions } from '@/lib/api/server-request-options' @@ -34,7 +34,7 @@ export default async function ReviewQueuePage() { const params = { assigned_to: me.nuid } await queryClient.prefetchQuery({ queryKey: queryKeys.applications.list(params), - queryFn: async () => (await listApplications(params, requestOptions)) ?? [], + queryFn: () => fetchApplicationPage(params, requestOptions), }) return ( diff --git a/frontend/src/generated/answers/answers.ts b/frontend/src/generated/answers/answers.ts index 85fb23c..dd2df68 100644 --- a/frontend/src/generated/answers/answers.ts +++ b/frontend/src/generated/answers/answers.ts @@ -30,6 +30,8 @@ import type { CreateUploadURLOutputBody, ErrorModel, ListAnswers200, + ListAnswersBulk200, + ListAnswersBulkParams, UpsertAnswers200, UpsertAnswersInputBody } from '.././model'; @@ -68,6 +70,99 @@ type SecondParameter unknown> = Parameters[1]; +/** + * One request for a page of applications, instead of one per application. Reviewer-only; draft answers are never included. + * @summary List written answers for several applications + */ +export const listAnswersBulk = ( + params?: ListAnswersBulkParams, + options?: SecondParameter,signal?: AbortSignal +) => { + + + return customInstance( + {url: `/answers`, method: 'GET', + params, signal + }, + options); + } + + + + +export const getListAnswersBulkQueryKey = (params?: ListAnswersBulkParams,) => { + return [ + `/answers`, ...(params ? [params]: []) + ] as const; + } + + +export const getListAnswersBulkQueryOptions = >, TError = ErrorModel | ErrorModel | ErrorModel>(params?: ListAnswersBulkParams, options?: { query?:Partial>, TError, TData>>, request?: SecondParameter} +) => { + +const {query: queryOptions, request: requestOptions} = options ?? {}; + + const queryKey = queryOptions?.queryKey ?? getListAnswersBulkQueryKey(params); + + + + const queryFn: QueryFunction>> = ({ signal }) => listAnswersBulk(params, requestOptions, signal); + + + + + + return { queryKey, queryFn, ...queryOptions} as UseQueryOptions>, TError, TData> & { queryKey: DataTag } +} + +export type ListAnswersBulkQueryResult = NonNullable>> +export type ListAnswersBulkQueryError = ErrorModel | ErrorModel | ErrorModel + + +export function useListAnswersBulk>, TError = ErrorModel | ErrorModel | ErrorModel>( + params: undefined | ListAnswersBulkParams, options: { query:Partial>, TError, TData>> & Pick< + DefinedInitialDataOptions< + Awaited>, + TError, + Awaited> + > , 'initialData' + >, request?: SecondParameter} + , queryClient?: QueryClient + ): DefinedUseQueryResult & { queryKey: DataTag } +export function useListAnswersBulk>, TError = ErrorModel | ErrorModel | ErrorModel>( + params?: ListAnswersBulkParams, options?: { query?:Partial>, TError, TData>> & Pick< + UndefinedInitialDataOptions< + Awaited>, + TError, + Awaited> + > , 'initialData' + >, request?: SecondParameter} + , queryClient?: QueryClient + ): UseQueryResult & { queryKey: DataTag } +export function useListAnswersBulk>, TError = ErrorModel | ErrorModel | ErrorModel>( + params?: ListAnswersBulkParams, options?: { query?:Partial>, TError, TData>>, request?: SecondParameter} + , queryClient?: QueryClient + ): UseQueryResult & { queryKey: DataTag } +/** + * @summary List written answers for several applications + */ + +export function useListAnswersBulk>, TError = ErrorModel | ErrorModel | ErrorModel>( + params?: ListAnswersBulkParams, options?: { query?:Partial>, TError, TData>>, request?: SecondParameter} + , queryClient?: QueryClient + ): UseQueryResult & { queryKey: DataTag } { + + const queryOptions = getListAnswersBulkQueryOptions(params,options) + + const query = useQuery(queryOptions, queryClient) as UseQueryResult & { queryKey: DataTag }; + + query.queryKey = queryOptions.queryKey ; + + return query; +} + + + /** * @summary List an application's written answers */ diff --git a/frontend/src/generated/applications/applications.ts b/frontend/src/generated/applications/applications.ts index 7bac988..f717a3b 100644 --- a/frontend/src/generated/applications/applications.ts +++ b/frontend/src/generated/applications/applications.ts @@ -26,9 +26,9 @@ import type { import type { Application, + ApplicationsOutputBody, CreateApplicationInputBody, ErrorModel, - ListApplications200, ListApplicationsParams, UpdateApplicationInputBody } from '.././model'; @@ -77,7 +77,7 @@ export const listApplications = ( ) => { - return customInstance( + return customInstance( {url: `/applications`, method: 'GET', params, signal }, diff --git a/frontend/src/generated/model/applicationsOutputBody.ts b/frontend/src/generated/model/applicationsOutputBody.ts new file mode 100644 index 0000000..b332b84 --- /dev/null +++ b/frontend/src/generated/model/applicationsOutputBody.ts @@ -0,0 +1,17 @@ +/** + * Generated by orval v7.13.2 🍺 + * Do not edit manually. + * apportal API + * Generate application portal — applications, reviews, and the hiring pipeline. + * OpenAPI spec version: 0.1.0 + */ +import type { ApplicationsOutputBodyApplications } from './applicationsOutputBodyApplications'; +import type { ApplicationsOutputBodyStageCounts } from './applicationsOutputBodyStageCounts'; + +export interface ApplicationsOutputBody { + /** A URL to the JSON Schema for this object. */ + readonly $schema?: string; + applications: ApplicationsOutputBodyApplications; + stage_counts: ApplicationsOutputBodyStageCounts; + total: number; +} diff --git a/frontend/src/generated/model/listApplications200.ts b/frontend/src/generated/model/applicationsOutputBodyApplications.ts similarity index 77% rename from frontend/src/generated/model/listApplications200.ts rename to frontend/src/generated/model/applicationsOutputBodyApplications.ts index b922d4e..9906c83 100644 --- a/frontend/src/generated/model/listApplications200.ts +++ b/frontend/src/generated/model/applicationsOutputBodyApplications.ts @@ -7,4 +7,4 @@ */ import type { ApplicationSummary } from './applicationSummary'; -export type ListApplications200 = ApplicationSummary[] | null; +export type ApplicationsOutputBodyApplications = ApplicationSummary[] | null; diff --git a/frontend/src/generated/model/applicationsOutputBodyStageCounts.ts b/frontend/src/generated/model/applicationsOutputBodyStageCounts.ts new file mode 100644 index 0000000..314e3e2 --- /dev/null +++ b/frontend/src/generated/model/applicationsOutputBodyStageCounts.ts @@ -0,0 +1,9 @@ +/** + * Generated by orval v7.13.2 🍺 + * Do not edit manually. + * apportal API + * Generate application portal — applications, reviews, and the hiring pipeline. + * OpenAPI spec version: 0.1.0 + */ + +export type ApplicationsOutputBodyStageCounts = {[key: string]: number}; diff --git a/frontend/src/generated/model/index.ts b/frontend/src/generated/model/index.ts index ab071f7..a031eb2 100644 --- a/frontend/src/generated/model/index.ts +++ b/frontend/src/generated/model/index.ts @@ -17,6 +17,9 @@ export * from './applicationTemplate'; export * from './applicationTemplateApplicationRole'; export * from './applicationTemplateReviewStatus'; export * from './applicationTemplateStatus'; +export * from './applicationsOutputBody'; +export * from './applicationsOutputBodyApplications'; +export * from './applicationsOutputBodyStageCounts'; export * from './assignRecordingReviewerInputBody'; export * from './assignmentPlanPreview'; export * from './assignmentPlanPreviewCoverageCounts'; @@ -82,7 +85,8 @@ export * from './item'; export * from './leadAssignment'; export * from './leadSelection'; export * from './listAnswers200'; -export * from './listApplications200'; +export * from './listAnswersBulk200'; +export * from './listAnswersBulkParams'; export * from './listApplicationsParams'; export * from './listChiefReviews200'; export * from './listCodeSubmissions200'; diff --git a/frontend/src/generated/model/listAnswersBulk200.ts b/frontend/src/generated/model/listAnswersBulk200.ts new file mode 100644 index 0000000..4c3ee7f --- /dev/null +++ b/frontend/src/generated/model/listAnswersBulk200.ts @@ -0,0 +1,10 @@ +/** + * Generated by orval v7.13.2 🍺 + * Do not edit manually. + * apportal API + * Generate application portal — applications, reviews, and the hiring pipeline. + * OpenAPI spec version: 0.1.0 + */ +import type { WrittenAnswer } from './writtenAnswer'; + +export type ListAnswersBulk200 = WrittenAnswer[] | null; diff --git a/frontend/src/generated/model/listAnswersBulkParams.ts b/frontend/src/generated/model/listAnswersBulkParams.ts new file mode 100644 index 0000000..ee68b4a --- /dev/null +++ b/frontend/src/generated/model/listAnswersBulkParams.ts @@ -0,0 +1,14 @@ +/** + * Generated by orval v7.13.2 🍺 + * Do not edit manually. + * apportal API + * Generate application portal — applications, reviews, and the hiring pipeline. + * OpenAPI spec version: 0.1.0 + */ + +export type ListAnswersBulkParams = { +/** + * Comma-separated application IDs + */ +application_ids?: string; +}; diff --git a/frontend/src/generated/model/listApplicationsParams.ts b/frontend/src/generated/model/listApplicationsParams.ts index c9ea7a5..a241c0c 100644 --- a/frontend/src/generated/model/listApplicationsParams.ts +++ b/frontend/src/generated/model/listApplicationsParams.ts @@ -19,4 +19,19 @@ stage?: string; * JSON array of answer filters, e.g. [{"question_id":"…","question_type":"checkbox","values":["Yes"]}]. Values may be a string or an array of strings; a filter matches any of them, and separate filters are AND'd. */ answer_filters?: string; +/** + * Case-insensitive substring match on the applicant's name, NUID, or email + */ +search?: string; +/** + * Max results per page; omit (or 0) to return every match + * @minimum 0 + * @maximum 200 + */ +limit?: number; +/** + * Number of results to skip + * @minimum 0 + */ +offset?: number; }; diff --git a/frontend/src/lib/queries/answers.ts b/frontend/src/lib/queries/answers.ts index 0477c27..96bc516 100644 --- a/frontend/src/lib/queries/answers.ts +++ b/frontend/src/lib/queries/answers.ts @@ -4,7 +4,11 @@ import { useQuery, useQueryClient, } from '@tanstack/react-query' -import { listAnswers, upsertAnswers } from '@/generated/answers/answers' +import { + listAnswers, + listAnswersBulk, + upsertAnswers, +} from '@/generated/answers/answers' import type { RequestOptions } from '@/lib/api/orval-mutator' import type { WrittenAnswer } from '@/lib/api/types' import { queryKeys } from './keys' @@ -18,18 +22,42 @@ export function useAnswers(applicationId: string, opts?: RequestOptions) { }) } -// Fetches a batch of applications' answers, e.g. to preview responses inline -// on an applications list. Each application id gets its own cache entry, -// shared with useAnswers. -export function useAnswersByApplicationIds( - applicationIds: string[], +// Fetches answers for several batches of applications — one request per +// batch, not one per application. Callers pass their applications grouped the +// way they loaded them (a page at a time), so each batch keeps its own cache +// entry and loading more never refetches the ones already in hand. +// +// Each response is also written back to the per-application entries that +// useAnswers reads, so opening a single application afterwards is a cache hit +// rather than a fresh request. +export function useAnswersByApplicationIdBatches( + applicationIdBatches: string[][], opts?: RequestOptions ) { + const queryClient = useQueryClient() return useQueries({ - queries: applicationIds.map((applicationId) => ({ - queryKey: queryKeys.answers.list(applicationId), - queryFn: async () => - ((await listAnswers(applicationId, opts)) ?? []) as WrittenAnswer[], + queries: applicationIdBatches.map((applicationIds) => ({ + queryKey: queryKeys.answers.bulk(applicationIds), + queryFn: async () => { + const answers = ((await listAnswersBulk( + { application_ids: applicationIds.join(',') }, + opts + )) ?? []) as WrittenAnswer[] + + // Start every requested id at an empty list: an application with no + // answers still needs an entry, or its row can't tell "none" from + // "not loaded". + const byApplicationId: Record = {} + for (const id of applicationIds) byApplicationId[id] = [] + for (const answer of answers) { + byApplicationId[answer.application_id]?.push(answer) + } + for (const [id, list] of Object.entries(byApplicationId)) { + queryClient.setQueryData(queryKeys.answers.list(id), list) + } + return byApplicationId + }, + enabled: applicationIds.length > 0, })), }) } diff --git a/frontend/src/lib/queries/applications.ts b/frontend/src/lib/queries/applications.ts index b03b32b..6dc4276 100644 --- a/frontend/src/lib/queries/applications.ts +++ b/frontend/src/lib/queries/applications.ts @@ -1,4 +1,9 @@ -import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' +import { + useInfiniteQuery, + useMutation, + useQuery, + useQueryClient, +} from '@tanstack/react-query' import { createApplication, deleteApplication, @@ -7,7 +12,10 @@ import { updateApplication, } from '@/generated/applications/applications' import type { RequestOptions } from '@/lib/api/orval-mutator' -import type { ListApplicationsParams } from '@/generated/model' +import type { + ApplicationsOutputBody, + ListApplicationsParams, +} from '@/generated/model' import type { AnswerFilterParam, Application, @@ -17,15 +25,38 @@ import type { } from '@/lib/api/types' import { queryKeys } from './keys' +export interface ApplicationListParams { + cycle_id?: string + user_nuid?: string + assigned_to?: string + stage?: ApplicationStage + role?: Role + answer_filters?: AnswerFilterParam[] + // Substring match on the applicant's name, NUID, or email. Server-side + // because it has to narrow the whole match, not just the fetched page. + search?: string + // Omit to fetch every match. Paging is opt-in: the review queues and the + // assignment planner all need the full set. + limit?: number + offset?: number +} + +// fetchApplicationPage is the shared queryFn. The cache always holds the whole +// envelope so a paging caller can read `total` and `stage_counts`; hooks that +// only want the rows unwrap it on the way out, which leaves the cached shape +// (and so the server prefetch in each page.tsx) identical for both. +export async function fetchApplicationPage( + params?: ApplicationListParams, + opts?: RequestOptions +): Promise { + return (await listApplications( + toListParams(params), + opts + )) as ApplicationsOutputBody +} + export function useApplications( - params?: { - cycle_id?: string - user_nuid?: string - assigned_to?: string - stage?: ApplicationStage - role?: Role - answer_filters?: AnswerFilterParam[] - }, + params?: ApplicationListParams, opts?: RequestOptions, // `enabled` lets a caller hold the request until its filters are actually // known. Omitting a filter isn't the same as not having it yet: the backend @@ -35,19 +66,63 @@ export function useApplications( ) { return useQuery({ queryKey: queryKeys.applications.list(params), - queryFn: async () => - ((await listApplications(toListParams(params), opts)) ?? - []) as ApplicationSummary[], + queryFn: () => fetchApplicationPage(params, opts), + select: (data) => (data.applications ?? []) as ApplicationSummary[], enabled, }) } +// useInfiniteApplications walks the same paged endpoint one offset at a time, +// accumulating rows for a scroll-to-load table. `params` carries the page size +// but never the offset — that comes from the page param, so every filter +// change starts a fresh scroll from the top on its own. +export function useInfiniteApplications( + params?: ApplicationListParams, + opts?: RequestOptions, + { enabled = true }: { enabled?: boolean } = {} +) { + const query = useInfiniteQuery({ + queryKey: queryKeys.applications.infiniteList(params), + queryFn: ({ pageParam }) => + fetchApplicationPage({ ...params, offset: pageParam }, opts), + initialPageParam: 0, + // The next offset is however many rows are already in hand; undefined once + // that reaches the total. Only the first page carries the totals — the + // server skips those scans on later pages, since they cost more than the + // page itself and can't change while the filter doesn't. + getNextPageParam: (_lastPage, allPages) => { + const loaded = allPages.reduce( + (sum, p) => sum + (p.applications?.length ?? 0), + 0 + ) + return loaded < (allPages[0]?.total ?? 0) ? loaded : undefined + }, + enabled, + }) + + const pages = query.data?.pages ?? [] + return { + ...query, + applications: pages.flatMap( + (p) => (p.applications ?? []) as ApplicationSummary[] + ), + // Ids grouped the way they were fetched, so anything loaded per-row + // alongside them (answers) can be batched one request per page instead of + // one per row — and appending a page leaves earlier batches cached. + applicationIdPages: pages.map((p) => + (p.applications ?? []).map((a) => a.id) + ), + total: pages[0]?.total ?? 0, + stageCounts: pages[0]?.stage_counts ?? {}, + } +} + // answer_filters crosses the wire as a JSON string: it is the one list-of- // objects param on this endpoint, and neither axios's bracket encoding nor a // repeated key binds to a struct slice server-side. Everything else passes // through untouched. function toListParams( - params?: Parameters[0] + params?: ApplicationListParams ): ListApplicationsParams | undefined { if (!params) return undefined const { answer_filters, ...rest } = params diff --git a/frontend/src/lib/queries/keys.ts b/frontend/src/lib/queries/keys.ts index cbbc30d..9f50dab 100644 --- a/frontend/src/lib/queries/keys.ts +++ b/frontend/src/lib/queries/keys.ts @@ -40,6 +40,20 @@ export const queryKeys = { role?: Role answer_filters?: AnswerFilterParam[] }) => [...queryKeys.applications.lists(), params ?? {}] as const, + // Namespaced away from `list` because an infinite query caches + // `{ pages, pageParams }` rather than a single response — sharing a key + // with a plain list would put two different shapes in one entry. + infiniteList: (params?: { + cycle_id?: string + user_nuid?: string + assigned_to?: string + stage?: ApplicationStage + role?: Role + answer_filters?: AnswerFilterParam[] + search?: string + limit?: number + }) => + [...queryKeys.applications.lists(), 'infinite', params ?? {}] as const, details: () => [...queryKeys.applications.all, 'detail'] as const, detail: (id: string) => [...queryKeys.applications.details(), id] as const, }, @@ -90,6 +104,10 @@ export const queryKeys = { lists: () => [...queryKeys.answers.all, 'list'] as const, list: (applicationId: string) => [...queryKeys.answers.lists(), applicationId] as const, + // One entry per batch of applications fetched together. Keyed by the exact + // id list so a batch stays cached as more are loaded alongside it. + bulk: (applicationIds: string[]) => + [...queryKeys.answers.all, 'bulk', applicationIds] as const, }, submissions: {