Repository navigation
feat(web): move the datahub behind a subject hub - #1654
Merged
Merged
Conversation
The subject listing needs a record count and a latest collection date per subject, which it previously had no way to ask for: fetching every record in the group to count them client-side does not scale, and the existing `hasRecord` parameter answers only the boolean case. Aggregation groups by group and subject rather than by subject alone so the per-row `groupId` is available to the ability check, and the fold happens in JS so a subject readable in two groups becomes one row with the counts added and the later date kept. The grouped set is bounded by subjects, not records. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The datahub was a single page listing subjects, with no room for a second way into the same records. It becomes a nav group whose only child today is the subject view at /datahub/subjects; /datahub redirects there so existing links and bookmarks keep working, and the per-subject routes move underneath. The listing itself gains what the new summary endpoint supplies: a record count and a last-collected date, both sortable, plus a minimum-count filter and a collection-date window. The minimum count replaces the old "with records only" checkbox, which was the same question with one answer. It narrows on the counts already loaded for the column rather than refetching, because re-querying suspended the table and left the stale set on screen a keystroke behind. Sex becomes a colour-tagged cell. The colour is always redundant to the label beside it, so it stays readable without relying on hue. TabLink moves out of the subject layout into its own component, unchanged, so the next view into the datahub can reuse it rather than copying it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`SourceStep` and `DeleteRemoteAssignments` each carried a verbatim copy of the same sortable column header, differing only in the row type it was written against. Now that the datahub needs a third, the component is generic and lives on its own, so the two copies become imports. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n-instruments changes The rebase onto main needed four reconciliations that could not be resolved mechanically: - route-tree.ts: add the /datahub/subjects/ nesting to main's version, which gained /admin/instruments and a prettier reformat - datahub-table-controls.test.tsx: point the import at the subjects route and mock useSubjectRecordSummaryQuery - subjects/index.tsx: adopt main's flex-wrap / grow phone-layout classes - subjects/index.page.ts: check data-state before clicking the filter trigger, so two filter methods in sequence do not toggle it closed Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Main's test-coverage work landed suites that assert the pre-move `/datahub/...` addresses, and git merged them cleanly because the disagreement is semantic rather than textual: - the walkthrough, dashboard, nav-item, subject-layout and subject-assignments tests now expect `/datahub/subjects/...` - `instrument-records.service.spec.ts` declared `groupsService` twice, once from each side of the merge, which made the file a syntax error rather than a failing test Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
thomasbeaudry
force-pushed
the
feat/datahub-subject-hub
branch
from
October 5, 2026 16:50
96c8118 to
c7c571b
Compare
Main moved every web route spec into a __tests__ folder beside its route, removed the redundant gateway guard spec and the unused subject data table page object, and now gates CI on 100% unit coverage. Reconciling that with the subject hub: - The datahub route specs move again, to sit beside the routes under subjects/. - The sortable header spec moves beside the component it tests, now that it is shared. - The /datahub redirect, the by-subject summary endpoint and the collected-date filter carry their own cases, so the merged tree covers every line again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`eslint --fix` strips the `as HTMLInputElement` the assertions needed, and `lint` type-checks before it fixes — so the cast survived the local run and failed `tsc` on CI. Asking `getByTestId` for the element type instead leaves nothing for the fixer to remove. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
joshunrau
approved these changes
Oct 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/datahubinto a nav group with/datahub/subjectsas the entry point (the old/datahubredirects). Per-subject routes move undersubjects/.GET /v1/instrument-records/summary/by-subjectendpoint that returns per-subject record counts and last-collected dates.SortableHeaderinto a shared component, removing the duplicate copies inSourceStepandDeleteRemoteAssignments.This is PR 1 of 2 for the datahub revamp. PR 2 adds record provenance columns and the instrument hub view.
Test plan
pnpm lint— 34/34 greenpnpm exec env-cmd pnpm exec vitest run— 181 files / 1678 tests passingpnpm test:e2e— 277/278 passed (the one failure isrecord-file-deletion.spec.ts, a pre-existing issue on main)route-tree.tsand confirm it matches the manually reconstructed version🤖 Generated with Claude Code