fix(core): make "All cohorts" selector show a combined overview#26
Conversation
The cohort selector on the Common Core overview offered an "All cohorts" option, but choosing it navigated to `/core/` which matches the `/core` route. That route is hardcoded to redirect to the latest cohort, so the page bounced straight back to the most recent year (e.g. 2026) instead of showing all cohorts. This was a copy/paste of the `/users/students` selector, which does have a real all-cohorts view behind it; the core overview never did. Fix: - getCommonCoreCohortData now accepts `year: number | null`; a null year omits the per-year `begin_at` filter to include every cohort and caches under `core-all`. - buildCommonCoreCache also pre-warms the combined view. - Add a dedicated `GET /core/all` route, registered before `/core/:year` (otherwise `all` fails the 4-digit year check and the handler returns without responding). - The selector now navigates to `/core/all` for the "All cohorts" option and marks it selected when no specific year is set. The `/core` landing redirect to the latest cohort is unchanged.
FreekBes
left a comment
FreekBes
left a comment
There was a problem hiding this comment.
Nice change! I would have removed the "All cohorts" option entirely (indeed mistakenly having added that option to the dropdown menu after copying it over), but having an option to display everyone is a nice addition.
One minor change requested: let's have /core display all cohorts instead of /core/all, similar to the /users/students route. The redirect to the latest cohort from /core can then be removed.
Address review: instead of redirecting /core to the latest cohort and exposing the combined view at /core/all, /core now renders all cohorts directly, mirroring /users/students. The /core/all route and the latest-cohort redirect are removed; the selector's "All cohorts" option navigates back to /core.
|
Done in 0cb4333 — |
|
🎉 This PR is included in version 2.3.3 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
3 participants
Bug
On the Common Core overview (
/core/:year), the cohort selector has an All cohorts option. Selecting it navigates back to the latest cohort (e.g.2026) instead of showing all cohorts.Repro
/core/2026(Common Core Overview)./core/2026.Cause
The selector's
changehandler sends value0("All cohorts") to/core/(empty year). That matches the/coreroute, which is hardcoded to redirect to the latest cohort — so it bounces straight back.This dropdown was copied from the
/users/studentsselector, which does have a real all-cohorts view behind it. The core overview never had one.Fix
getCommonCoreCohortData(prisma, year: number | null, ...)— anullyear omits the per-yearbegin_atfilter so every cohort is included, cached undercore-all.buildCommonCoreCachealso pre-warms the combined view so the first request isn't slow.GET /core/allroute, registered before/core/:year(otherwiseallfails the 4-digit-year check and the handler returns without responding)./core/allfor "All cohorts", and that option is markedselectedwhen no specific year is set./corelanding redirect to the latest cohort is unchanged.Testing
tsctype-check passes (only the pre-existing unrelatedscripts/rootDir error remains)./core/allreturns exactly the union of every per-cohort query (one user in 2025, two in 2026 -> all three under/core/all).