fix(#794): correct list type provenances and fix non-strategic sensitivity access control - #843
Conversation
- Remove hardcoded id from ListTypeData interface and all 59 entries; DB uses autoincrement and seed upserts by name, so these were never read - Fix provenance values to match Java pip-data-models enum: - SJP lists: COMMON_PLATFORM → PI_AAD - Non-strategic MANUAL_UPLOAD lists → CFT_IDAM - MAGISTRATES_PUBLIC_LIST and MAGISTRATES_STANDARD_LIST → CRIME_IDAM,PI_AAD - KINGS_BENCH_MASTERS_DAILY_CAUSE_LIST: MANUAL_UPLOAD → CFT_IDAM - Update canAccessPublication to split comma-separated allowed_provenance so multi-provenance lists (e.g. CRIME_IDAM,PI_AAD) work correctly - Add SQL scripts to correct existing DB rows in STG/prod environments - Add unit tests for comma-separated provenance access control Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ON CONFLICT DO NOTHING was silently skipping rows that already existed with wrong data. Changed to DO UPDATE SET so all fields are corrected regardless of whether the row pre-existed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…veListType helper Extracted resolveListType from @hmcts/publication so list-type-handler.ts and flat-file-service.ts share the same DB lookup instead of each inlining prisma.listType.findUnique + ListType construction. Also deduplicated createUtiacDailyRender/createUtiacJrRegionalDailyRender by delegating to createWeeklyHearingListRender, merged SimpleRenderCallback into RenderCallback, and extracted a renderError helper to collapse repetitive handler error paths. Duplication reduced from 11.99% to 1.98%. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR adds role- and provenance-based authorisation enforcement to previously exposed non-strategic list rendering and flat-file display/download paths, introducing a database-backed ChangesNon-strategic list sensitivity enforcement
Sequence Diagram(s)sequenceDiagram
participant Route
participant flatFileService
participant canAccessPublicationData
participant resolveListType
Route->>flatFileService: getFlatFileForDisplay/getFileForDownload(artefactId, user)
flatFileService->>resolveListType: resolveListType(listTypeId)
resolveListType-->>flatFileService: ListType
flatFileService->>canAccessPublicationData: check(user, artefact, ListType)
canAccessPublicationData-->>flatFileService: allow/deny
alt denied
flatFileService-->>Route: ACCESS_DENIED
Route-->>Route: 403 + Cache-Control headers
else allowed
flatFileService-->>Route: file data
end
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎭 Playwright E2E Test Results84 tests 52 ✅ 6m 49s ⏱️ Results for commit 92c38ff. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
libs/publication/src/authorisation/service.ts (1)
14-17: 🚀 Performance & Scalability | 🔵 TrivialConsider caching
resolveListTyperesults for high-traffic paths.
resolveListTypeissues a database query on every access check. BothgetFlatFileForDisplayandgetFileForDownloadcall it per request. A short-lived cache (or request-scoped memoisation) would reduce database load under traffic spikes without changing semantics, since list-type provenance changes infrequently.Also applies to: 40-50
libs/public-pages/src/flat-file/flat-file-service.ts (1)
26-28: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winEliminate duplicate database query for the same list type record.
resolveListType(line 26) andfindListTypeById(line 37) both issueprisma.listType.findUniquefor the sameartefact.listTypeId. SincefindListTypeByIdalready returnsallowedProvenanceandisNonStrategic, its result can be used to construct theListTypeobject for authorization, removing the extra round-trip introduced byresolveListType.♻️ Proposed refactor: reuse findListTypeById for both authorization and display
- if (!canAccessPublicationData(user, artefact, await resolveListType(artefact.listTypeId))) { - return { error: "ACCESS_DENIED" as const }; - } - - const fileBuffer = await getFileBuffer(artefact.artefactId); - - if (!fileBuffer) { - return { error: "FILE_NOT_FOUND" as const }; - } - - const location = await getLocationById(Number.parseInt(artefact.locationId, 10)); - const listTypeInfo = await findListTypeById(artefact.listTypeId); + const location = await getLocationById(Number.parseInt(artefact.locationId, 10)); + const listTypeInfo = await findListTypeById(artefact.listTypeId); + const listType = listTypeInfo + ? { id: listTypeInfo.id, provenance: listTypeInfo.allowedProvenance, isNonStrategic: listTypeInfo.isNonStrategic } + : undefined; + + if (!canAccessPublicationData(user, artefact, listType)) { + return { error: "ACCESS_DENIED" as const }; + } + + const fileBuffer = await getFileBuffer(artefact.artefactId); + + if (!fileBuffer) { + return { error: "FILE_NOT_FOUND" as const }; + }Also applies to: 37-37
apps/web/src/pages/(list-types)/list-type-handler.test.ts (1)
71-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
Cache-Controlheader assertion to 403 testsThe 403 tests verify
res.status(403)andres.render("errors/403", ...)but don't assert thatres.setHeaderwas called with the expectedCache-Controlvalue. This header is a security-relevant detail that should be verified to prevent regressions.✅ Suggested addition for each 403 test case
expect(res.status).toHaveBeenCalledWith(403); expect(res.render).toHaveBeenCalledWith("errors/403", expect.any(Object)); + expect(res.setHeader).toHaveBeenCalledWith("Cache-Control", "private, max-age=0, no-cache, no-store, must-revalidate");apps/web/src/pages/(list-types)/ast-daily-hearing-list/index.test.ts (1)
14-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
@hmcts/postgres-prismamock is unused.
resolveListTypeis mocked at the@hmcts/publicationlevel (line 26), so the real implementation (which callsprisma.listType.findUnique) is never invoked. Theprisma.listType.findUniquemock on line 17 is dead code. Either remove it, or remove theresolveListTypemock and let the real implementation use the prisma mock for integration-style coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 219f9c12-da2a-4edd-81b4-00096f2d8cc1
📒 Files selected for processing (40)
apps/postgres/prisma/scripts/001_insert_missing_list_types.sqlapps/postgres/prisma/scripts/002_update_list_type_provenances.sqlapps/web/src/pages/(list-types)/administrative-court-daily-cause-list/index.test.tsapps/web/src/pages/(list-types)/ast-daily-hearing-list/index.test.tsapps/web/src/pages/(list-types)/care-standards-tribunal-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/cic-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/court-of-appeal-civil-daily-cause-list/index.test.tsapps/web/src/pages/(list-types)/ftt-lands-registration-tribunal-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/ftt-rpt-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/ftt-tax-chamber-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/grc-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/list-type-handler.test.tsapps/web/src/pages/(list-types)/list-type-handler.tsapps/web/src/pages/(list-types)/london-administrative-court-daily-cause-list/index.test.tsapps/web/src/pages/(list-types)/rcj-standard-daily-cause-list/index.test.tsapps/web/src/pages/(list-types)/send-daily-hearing-list/index.test.tsapps/web/src/pages/(list-types)/siac-poac-paac-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/sscs-daily-hearing-list/index.test.tsapps/web/src/pages/(list-types)/upper-tribunal-administrative-appeals-chamber-daily-hearing-list/index.test.tsapps/web/src/pages/(list-types)/upper-tribunal-lands-chamber-daily-hearing-list/index.test.tsapps/web/src/pages/(list-types)/upper-tribunal-tax-and-chancery-chamber-daily-hearing-list/index.test.tsapps/web/src/pages/(list-types)/utiac-jr-daily-hearing-list/index.test.tsapps/web/src/pages/(list-types)/utiac-statutory-appeal-daily-hearing-list/index.test.tsapps/web/src/pages/(list-types)/wpafcc-weekly-hearing-list/index.test.tsapps/web/src/pages/(public)/hearing-lists/[locationId]/[artefactId]/index.test.tsapps/web/src/pages/(public)/hearing-lists/[locationId]/[artefactId]/index.tsapps/web/src/pages/(public)/hearing-lists/cy.tsapps/web/src/pages/(public)/hearing-lists/en.tsdocs/tickets/794/plan.mddocs/tickets/794/review.mddocs/tickets/794/tasks.mddocs/tickets/794/ticket.mdlibs/location/src/list-type-data.tslibs/public-pages/src/flat-file/flat-file-service.test.tslibs/public-pages/src/flat-file/flat-file-service.tslibs/public-pages/src/routes/api/flat-file/[artefactId]/download.test.tslibs/public-pages/src/routes/api/flat-file/[artefactId]/download.tslibs/publication/src/authorisation/service.test.tslibs/publication/src/authorisation/service.tslibs/publication/src/index.ts
| ('UTIAC_JR_LONDON_DAILY_HEARING_LIST', 'Upper Tribunal (Immigration and Asylum) Chamber - Judicial Review: London Daily Hearing List', 'Upper Tribunal (Immigration and Asylum) Chamber - Judicial Review: London Daily Hearing List', 'UTIAC JR London Daily Hearing List', 'utiac-jr-daily-hearing-list', 'Public', 'CFT_IDAM', true), | ||
| ('UTIAC_JR_LEEDS_DAILY_HEARING_LIST', 'Upper Tribunal (Immigration and Asylum) Chamber - Judicial Review: Leeds Daily Hearing List', 'Upper Tribunal (Immigration and Asylum) Chamber - Judicial Review: Leeds Daily Hearing List', 'UTIAC JR Leeds Daily Hearing List', 'utiac-jr-daily-hearing-list', 'Public', 'CFT_IDAM', true), | ||
| ('UTIAC_JR_MANCHESTER_DAILY_HEARING_LIST', 'Upper Tribunal (Immigration and Asylum) Chamber - Judicial Review: Manchester Daily Hearing List', 'Upper Tribunal (Immigration and Asylum) Chamber - Judicial Review: Manchester Daily Hearing List', 'UTIAC JR Manchester Daily Hearing List', 'utiac-jr-daily-hearing-list', 'Public', 'CFT_IDAM', true), | ||
| ('UTIAC_JR_BIRMINGHAM_DAILY_HEARING_LIST', 'Upper Tribunal (Immigration and Asylum) Chamber - Judicial Review: Birmingham Daily Hearing List', 'Upper Tribunal (Immigration and Asylum) Chamber - Judicial Review: Birmingham Daily Hearing List', 'UTIAC JR Birmingham Daily Hearing List', 'utiac-jr-daily-hearing-list', 'Public', 'CFT_IDAM', true), | ||
| ('UTIAC_JR_CARDIFF_DAILY_HEARING_LIST', 'Upper Tribunal (Immigration and Asylum) Chamber - Judicial Review: Cardiff Daily Hearing List', 'Upper Tribunal (Immigration and Asylum) Chamber - Judicial Review: Cardiff Daily Hearing List', 'UTIAC JR Cardiff Daily Hearing List', 'utiac-jr-daily-hearing-list', 'Public', 'CFT_IDAM', true), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Welsh names for UTIAC JR entries differ from TypeScript seed data.
The SQL sets welsh_friendly_name to English text (e.g., 'Upper Tribunal (Immigration and Asylum) Chamber - Judicial Review: London Daily Hearing List'), while list-type-data.ts uses "[WELSH TRANSLATION REQUIRED: ...]" markers for the same entries. This inconsistency means the database and seed data disagree on Welsh display names.
| vi.mock("@hmcts/postgres-prisma", () => ({ | ||
| prisma: { | ||
| listType: { | ||
| findUnique: vi.fn().mockResolvedValue({ id: 30, allowedProvenance: "MANUAL_UPLOAD", isNonStrategic: true }) | ||
| } | ||
| } | ||
| })); | ||
|
|
||
| vi.mock("@hmcts/publication", () => ({ | ||
| getArtefactById: vi.fn(), | ||
| getPublicationJson: vi.fn(), | ||
| canAccessPublicationData: vi.fn().mockReturnValue(true), | ||
| resolveListType: vi.fn().mockResolvedValue({ id: 1, provenance: "CFT_IDAM", isNonStrategic: false }), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
No test exercises the 403 access-denied path.
canAccessPublicationData is unconditionally mocked to return true, so the access-denied branch added to createSimpleListTypeHandler is never tested. Given this PR's core objective is to enforce role/provenance-based authorisation, at least one test should mock canAccessPublicationData to return false and assert a 403 response with the errors/403 template. Additionally, req lacks a user property in all tests, so even the happy path doesn't verify that req.user is correctly passed through.
| if (checkAccess && !canAccessPublicationData(req.user, artefact, await resolveListType(artefact.listTypeId))) { | ||
| return renderError(res, 403, "errors/403", { | ||
| en: { title: en.error403Title, message: en.error403Message }, | ||
| cy: { title: cy.error403Title, message: cy.error403Message } | ||
| }); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Missing Cache-Control header and fallback strings in createListTypeHandler 403 response
The 403 path in createListTypeHandler neither sets the Cache-Control header nor provides ?? fallbacks for error403Title/error403Message, both of which the equivalent path in createSimpleListTypeHandler (lines 125–137) does. Since these fields are optional in LocaleContent, the errors/403 template could receive undefined title/message values, producing broken error pages. Without Cache-Control, intermediaries may cache 403 responses, potentially serving stale access-denied pages to users who later gain access.
🔒️ Proposed fix aligning with `createSimpleListTypeHandler`
if (checkAccess && !canAccessPublicationData(req.user, artefact, await resolveListType(artefact.listTypeId))) {
+ res.setHeader("Cache-Control", "private, max-age=0, no-cache, no-store, must-revalidate");
return renderError(res, 403, "errors/403", {
- en: { title: en.error403Title, message: en.error403Message },
- cy: { title: cy.error403Title, message: cy.error403Message }
+ en: {
+ title: en.error403Title ?? "Access denied",
+ message: en.error403Message ?? "You do not have permission to view this publication."
+ },
+ cy: {
+ title: cy.error403Title ?? "Mynediad wedi'i wrthod",
+ message: cy.error403Message ?? "Nid oes gennych ganiatâd i weld y cyhoeddiad hwn."
+ }
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (checkAccess && !canAccessPublicationData(req.user, artefact, await resolveListType(artefact.listTypeId))) { | |
| return renderError(res, 403, "errors/403", { | |
| en: { title: en.error403Title, message: en.error403Message }, | |
| cy: { title: cy.error403Title, message: cy.error403Message } | |
| }); | |
| } | |
| if (checkAccess && !canAccessPublicationData(req.user, artefact, await resolveListType(artefact.listTypeId))) { | |
| res.setHeader("Cache-Control", "private, max-age=0, no-cache, no-store, must-revalidate"); | |
| return renderError(res, 403, "errors/403", { | |
| en: { | |
| title: en.error403Title ?? "Access denied", | |
| message: en.error403Message ?? "You do not have permission to view this publication." | |
| }, | |
| cy: { | |
| title: cy.error403Title ?? "Mynediad wedi'i wrthod", | |
| message: cy.error403Message ?? "Nid oes gennych ganiatâd i weld y cyhoeddiad hwn." | |
| } | |
| }); | |
| } |
- Fix TS2345: user.provenance is string | undefined, guard with && before passing to Array.includes - Extract checkArtefactAccess helper in flat-file-service.ts to eliminate duplicated guard logic between getFlatFileForDisplay and getFileForDownload - Add NOSONAR suppression on list-type-data.ts (intentional data repetition) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Data file with 59 intentionally repetitive object entries; exclude from copy-paste detection the same way page files are already excluded. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
…_DAILY_LIST - Move ListTypeData and listTypeData from @hmcts/location to @hmcts/list-types-common where it semantically belongs alongside other list type concerns - Remove vestigial @hmcts/location dependency from list-types-common (unused) - Add @hmcts/list-types-common dependency to @hmcts/location and @hmcts/postgres - Update all consumers (seed.ts, seed-list-types.ts) to import from new location - Update test mocks to match new import paths - Remove CRIME_DAILY_LIST from seed-list-types.ts, manage-list-types test fixture, and 001_insert_missing_list_types.sql (invalid list type) - Set MAGISTRATES_PUBLIC_LIST defaultSensitivity to Public in SQL seed script - Update sonar.cpd.exclusions to reflect new file path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Script 003 is idempotent and: - Upserts all 30 sub-jurisdictions (creates missing, updates names) - Links all 58 list types to their sub-jurisdiction(s) via name-based JOIN rather than hardcoded IDs, safe to re-run via ON CONFLICT DO NOTHING Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/postgres/prisma/seed.ts (1)
102-107: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftStale sub-jurisdiction links are not cleaned up during list type seeding.
The upsert loop only adds/updates links for
listType.subJurisdictionIds— it never removes links that are no longer in the array. This contrasts with the location seeding pattern (which usesdeleteManybeforecreateMany, as verified in the test at lines 386–395 ofseed.test.ts). If a list type'ssubJurisdictionIdschanges, orphaned links will grant incorrect sub-jurisdiction access.Consider deleting stale links before upserting:
const upserted = await prisma.listType.upsert({ // ... }); + // Remove stale sub-jurisdiction links before re-linking + await prisma.listTypeSubJurisdiction.deleteMany({ + where: { listTypeId: upserted.id } + }); + for (const sj of relevantSubJurisdictions) { await prisma.listTypeSubJurisdiction.upsert({
🧹 Nitpick comments (1)
apps/postgres/prisma/seed.ts (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated list-type seeding logic between
seed.tsandseed-list-types.ts.The list type seeding logic here (lines 64–108) is nearly identical to
libs/location/src/seed-list-types.ts(lines 4–82): both find sub-jurisdictions, looplistTypeData, filter relevant ones, upsert the list type, and link sub-jurisdictions. This DRY violation risks divergence when one copy is updated without the other.Consider extracting a shared
seedListTypes(prisma)function into@hmcts/list-types-commonand calling it from both locations.Also applies to: 64-108
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d47c6ccb-1971-4ad9-8548-b7d7ab22ef02
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
apps/postgres/package.jsonapps/postgres/prisma/scripts/001_insert_missing_list_types.sqlapps/postgres/prisma/scripts/002_update_list_type_provenances.sqlapps/postgres/prisma/scripts/003_upsert_sub_jurisdictions_and_list_type_links.sqlapps/postgres/prisma/seed.test.tsapps/postgres/prisma/seed.tsapps/web/src/pages/(system-admin)/manage-list-types/index.test.tse2e-tests/utils/seed-list-types.tslibs/list-types/common/package.jsonlibs/list-types/common/src/index.tslibs/list-types/common/src/list-type-data.tslibs/location/package.jsonlibs/location/src/index.tslibs/location/src/seed-list-types.tslibs/public-pages/src/flat-file/flat-file-service.tslibs/publication/src/authorisation/service.tssonar-project.properties
💤 Files with no reviewable changes (4)
- libs/location/src/index.ts
- libs/list-types/common/package.json
- e2e-tests/utils/seed-list-types.ts
- apps/web/src/pages/(system-admin)/manage-list-types/index.test.ts
✅ Files skipped from review due to trivial changes (2)
- sonar-project.properties
- libs/list-types/common/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/postgres/prisma/scripts/002_update_list_type_provenances.sql
- apps/postgres/prisma/scripts/001_insert_missing_list_types.sql
- libs/public-pages/src/flat-file/flat-file-service.ts
- libs/publication/src/authorisation/service.ts
| INSERT INTO list_types_sub_jurisdictions (list_type_id, sub_jurisdiction_id) | ||
| SELECT lt.id, sj.sub_jurisdiction_id | ||
| FROM (VALUES | ||
| -- CIVIL_DAILY_CAUSE_LIST → Civil Court (1) | ||
| ('CIVIL_DAILY_CAUSE_LIST', 1), | ||
| -- FAMILY_DAILY_CAUSE_LIST → Family Court (2) | ||
| ('FAMILY_DAILY_CAUSE_LIST', 2), | ||
| -- MAGISTRATES_PUBLIC_LIST → Magistrates Court (7) | ||
| ('MAGISTRATES_PUBLIC_LIST', 7), | ||
| -- CROWN_WARNED_LIST → Crown Court (4) | ||
| ('CROWN_WARNED_LIST', 4), | ||
| -- CROWN_DAILY_LIST → Crown Court (4) | ||
| ('CROWN_DAILY_LIST', 4), | ||
| -- CROWN_FIRM_LIST → Crown Court (4) | ||
| ('CROWN_FIRM_LIST', 4), | ||
| -- CIVIL_AND_FAMILY_DAILY_CAUSE_LIST → Civil Court (1), Family Court (2) | ||
| ('CIVIL_AND_FAMILY_DAILY_CAUSE_LIST', 1), | ||
| ('CIVIL_AND_FAMILY_DAILY_CAUSE_LIST', 2), | ||
| -- CARE_STANDARDS_TRIBUNAL_WEEKLY_HEARING_LIST → Care Standards Tribunal (9) | ||
| ('CARE_STANDARDS_TRIBUNAL_WEEKLY_HEARING_LIST', 9), | ||
| -- CIVIL_COURTS_RCJ_DAILY_CAUSE_LIST → Civil Court (1) | ||
| ('CIVIL_COURTS_RCJ_DAILY_CAUSE_LIST', 1), | ||
| -- COUNTY_COURT_LONDON_CIVIL_DAILY_CAUSE_LIST → Civil Court (1) | ||
| ('COUNTY_COURT_LONDON_CIVIL_DAILY_CAUSE_LIST', 1), | ||
| -- COURT_OF_APPEAL_CRIMINAL_DAILY_CAUSE_LIST → Court of Appeal (Criminal Division) (12) | ||
| ('COURT_OF_APPEAL_CRIMINAL_DAILY_CAUSE_LIST', 12), | ||
| -- FAMILY_DIVISION_HIGH_COURT_DAILY_CAUSE_LIST → Family Court (2) | ||
| ('FAMILY_DIVISION_HIGH_COURT_DAILY_CAUSE_LIST', 2), | ||
| -- KINGS_BENCH_DIVISION_DAILY_CAUSE_LIST → Civil Court (1) | ||
| ('KINGS_BENCH_DIVISION_DAILY_CAUSE_LIST', 1), | ||
| -- KINGS_BENCH_MASTERS_DAILY_CAUSE_LIST → Civil Court (1) | ||
| ('KINGS_BENCH_MASTERS_DAILY_CAUSE_LIST', 1), | ||
| -- MAYOR_CITY_CIVIL_DAILY_CAUSE_LIST → Civil Court (1) | ||
| ('MAYOR_CITY_CIVIL_DAILY_CAUSE_LIST', 1), | ||
| -- SENIOR_COURTS_COSTS_OFFICE_DAILY_CAUSE_LIST → Civil Court (1) | ||
| ('SENIOR_COURTS_COSTS_OFFICE_DAILY_CAUSE_LIST', 1), | ||
| -- LONDON_ADMINISTRATIVE_COURT_DAILY_CAUSE_LIST → Civil Court (1) | ||
| ('LONDON_ADMINISTRATIVE_COURT_DAILY_CAUSE_LIST', 1), | ||
| -- COURT_OF_APPEAL_CIVIL_DAILY_CAUSE_LIST → Court of Appeal (Civil Division) (5) | ||
| ('COURT_OF_APPEAL_CIVIL_DAILY_CAUSE_LIST', 5), | ||
| -- BIRMINGHAM_ADMINISTRATIVE_COURT_DAILY_CAUSE_LIST → Civil Court (1) | ||
| ('BIRMINGHAM_ADMINISTRATIVE_COURT_DAILY_CAUSE_LIST', 1), | ||
| -- LEEDS_ADMINISTRATIVE_COURT_DAILY_CAUSE_LIST → Civil Court (1) | ||
| ('LEEDS_ADMINISTRATIVE_COURT_DAILY_CAUSE_LIST', 1), | ||
| -- BRISTOL_CARDIFF_ADMINISTRATIVE_COURT_DAILY_CAUSE_LIST → Civil Court (1) | ||
| ('BRISTOL_CARDIFF_ADMINISTRATIVE_COURT_DAILY_CAUSE_LIST', 1), | ||
| -- MANCHESTER_ADMINISTRATIVE_COURT_DAILY_CAUSE_LIST → Civil Court (1) | ||
| ('MANCHESTER_ADMINISTRATIVE_COURT_DAILY_CAUSE_LIST', 1), | ||
| -- SJP_PRESS_LIST → Magistrates Court (7) | ||
| ('SJP_PRESS_LIST', 7), | ||
| -- SJP_PUBLIC_LIST → Magistrates Court (7) | ||
| ('SJP_PUBLIC_LIST', 7), | ||
| -- SJP_DELTA_PRESS_LIST → Magistrates Court (7) | ||
| ('SJP_DELTA_PRESS_LIST', 7), | ||
| -- SJP_DELTA_PUBLIC_LIST → Magistrates Court (7) | ||
| ('SJP_DELTA_PUBLIC_LIST', 7), | ||
| -- SIAC_WEEKLY_HEARING_LIST → Special Immigration Appeals Commission (25) | ||
| ('SIAC_WEEKLY_HEARING_LIST', 25), | ||
| -- POAC_WEEKLY_HEARING_LIST → Proscribed Organisations Appeal Commission (23) | ||
| ('POAC_WEEKLY_HEARING_LIST', 23), | ||
| -- PAAC_WEEKLY_HEARING_LIST → Pathogens Access Appeal Commission (21) | ||
| ('PAAC_WEEKLY_HEARING_LIST', 21), | ||
| -- FTT_TAX_CHAMBER_WEEKLY_HEARING_LIST → First-Tier Tribunal (Tax Chamber) (16) | ||
| ('FTT_TAX_CHAMBER_WEEKLY_HEARING_LIST', 16), | ||
| -- FTT_LANDS_REGISTRATION_TRIBUNAL_WEEKLY_HEARING_LIST → First-Tier Tribunal (Land Registration) (15) | ||
| ('FTT_LANDS_REGISTRATION_TRIBUNAL_WEEKLY_HEARING_LIST', 15), | ||
| -- FTT_RPT_EASTERN_WEEKLY_HEARING_LIST → Residential Property Tribunal (24) | ||
| ('FTT_RPT_EASTERN_WEEKLY_HEARING_LIST', 24), | ||
| -- FTT_RPT_LONDON_WEEKLY_HEARING_LIST → Residential Property Tribunal (24) | ||
| ('FTT_RPT_LONDON_WEEKLY_HEARING_LIST', 24), | ||
| -- FTT_RPT_MIDLANDS_WEEKLY_HEARING_LIST → Residential Property Tribunal (24) | ||
| ('FTT_RPT_MIDLANDS_WEEKLY_HEARING_LIST', 24), | ||
| -- FTT_RPT_NORTHERN_WEEKLY_HEARING_LIST → Residential Property Tribunal (24) | ||
| ('FTT_RPT_NORTHERN_WEEKLY_HEARING_LIST', 24), | ||
| -- FTT_RPT_SOUTHERN_WEEKLY_HEARING_LIST → Residential Property Tribunal (24) | ||
| ('FTT_RPT_SOUTHERN_WEEKLY_HEARING_LIST', 24), | ||
| -- SEND_DAILY_HEARING_LIST → First-tier Tribunal (SEND) (18) | ||
| ('SEND_DAILY_HEARING_LIST', 18), | ||
| -- CIC_WEEKLY_HEARING_LIST → Criminal Injuries Compensation Tribunal (14) | ||
| ('CIC_WEEKLY_HEARING_LIST', 14), | ||
| -- AST_DAILY_HEARING_LIST → Asylum Support Tribunal (13) | ||
| ('AST_DAILY_HEARING_LIST', 13), | ||
| -- GRC_WEEKLY_HEARING_LIST → General Regulatory Chamber (19) | ||
| ('GRC_WEEKLY_HEARING_LIST', 19), | ||
| -- WPAFCC_WEEKLY_HEARING_LIST → First-Tier Tribunal (War Pensions and Armed Forces Compensation) (17) | ||
| ('WPAFCC_WEEKLY_HEARING_LIST', 17), | ||
| -- UTIAC_STATUTORY_APPEAL_DAILY_HEARING_LIST → Upper Tribunal (IA) - Statutory Appeal (28) | ||
| ('UTIAC_STATUTORY_APPEAL_DAILY_HEARING_LIST', 28), | ||
| -- UTIAC_JR_LONDON_DAILY_HEARING_LIST → Upper Tribunal (IA) - Judicial Review (27) | ||
| ('UTIAC_JR_LONDON_DAILY_HEARING_LIST', 27), | ||
| -- UTIAC_JR_LEEDS_DAILY_HEARING_LIST → Upper Tribunal (IA) - Judicial Review (27) | ||
| ('UTIAC_JR_LEEDS_DAILY_HEARING_LIST', 27), | ||
| -- UTIAC_JR_MANCHESTER_DAILY_HEARING_LIST → Upper Tribunal (IA) - Judicial Review (27) | ||
| ('UTIAC_JR_MANCHESTER_DAILY_HEARING_LIST', 27), | ||
| -- UTIAC_JR_BIRMINGHAM_DAILY_HEARING_LIST → Upper Tribunal (IA) - Judicial Review (27) | ||
| ('UTIAC_JR_BIRMINGHAM_DAILY_HEARING_LIST', 27), | ||
| -- UTIAC_JR_CARDIFF_DAILY_HEARING_LIST → Upper Tribunal (IA) - Judicial Review (27) | ||
| ('UTIAC_JR_CARDIFF_DAILY_HEARING_LIST', 27), | ||
| -- SSCS_MIDLANDS_DAILY_HEARING_LIST → Social Security and Child Support (8) | ||
| ('SSCS_MIDLANDS_DAILY_HEARING_LIST', 8), | ||
| -- SSCS_SOUTH_EAST_DAILY_HEARING_LIST → Social Security and Child Support (8) | ||
| ('SSCS_SOUTH_EAST_DAILY_HEARING_LIST', 8), | ||
| -- SSCS_WALES_AND_SOUTH_WEST_DAILY_HEARING_LIST → Social Security and Child Support (8) | ||
| ('SSCS_WALES_AND_SOUTH_WEST_DAILY_HEARING_LIST', 8), | ||
| -- SSCS_SCOTLAND_DAILY_HEARING_LIST → Social Security and Child Support (8) | ||
| ('SSCS_SCOTLAND_DAILY_HEARING_LIST', 8), | ||
| -- SSCS_NORTH_EAST_DAILY_HEARING_LIST → Social Security and Child Support (8) | ||
| ('SSCS_NORTH_EAST_DAILY_HEARING_LIST', 8), | ||
| -- SSCS_NORTH_WEST_DAILY_HEARING_LIST → Social Security and Child Support (8) | ||
| ('SSCS_NORTH_WEST_DAILY_HEARING_LIST', 8), | ||
| -- SSCS_LONDON_DAILY_HEARING_LIST → Social Security and Child Support (8) | ||
| ('SSCS_LONDON_DAILY_HEARING_LIST', 8), | ||
| -- MAGISTRATES_STANDARD_LIST → Magistrates Court (7) | ||
| ('MAGISTRATES_STANDARD_LIST', 7), | ||
| -- UT_TAX_AND_CHANCERY_CHAMBER_DAILY_HEARING_LIST → Upper Tribunal (Tax and Chancery Chamber) (30) | ||
| ('UT_TAX_AND_CHANCERY_CHAMBER_DAILY_HEARING_LIST', 30), | ||
| -- UT_LANDS_CHAMBER_DAILY_HEARING_LIST → Upper Tribunal (Lands Chamber) (29) | ||
| ('UT_LANDS_CHAMBER_DAILY_HEARING_LIST', 29), | ||
| -- UT_ADMINISTRATIVE_APPEALS_CHAMBER_DAILY_HEARING_LIST → Upper Tribunal (Administrative Appeals Chamber) (26) | ||
| ('UT_ADMINISTRATIVE_APPEALS_CHAMBER_DAILY_HEARING_LIST', 26) | ||
| ) AS mapping(list_type_name, sub_jurisdiction_id) | ||
| JOIN list_types lt ON lt.name = mapping.list_type_name | ||
| JOIN sub_jurisdiction sj ON sj.sub_jurisdiction_id = mapping.sub_jurisdiction_id | ||
| ON CONFLICT (list_type_id, sub_jurisdiction_id) DO NOTHING; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Stale sub-jurisdiction links are not cleaned up.
The ON CONFLICT DO NOTHING clause only adds new links — it never removes links that no longer exist in listTypeData.subJurisdictionIds. If a list type's sub-jurisdiction assignment changes, the old link persists, potentially granting incorrect access. Consider adding a cleanup step that deletes links not in the current mapping:
-- After inserting, remove stale links
DELETE FROM list_types_sub_jurisdictions
WHERE list_type_id IN (SELECT id FROM list_types WHERE name IN (...))
AND (list_type_id, sub_jurisdiction_id) NOT IN (
SELECT lt.id, mapping.sub_jurisdiction_id
FROM (VALUES ...) AS mapping(list_type_name, sub_jurisdiction_id)
JOIN list_types lt ON lt.name = mapping.list_type_name
);
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Wire 001–004 SQL scripts into start.sh so list types, provenances, sub-jurisdictions, and the CRIME_DAILY_LIST soft-delete are applied on every deployment without manual intervention. Also fix 001 to include updated_at (NOT NULL, no DB default) and fix 003 to upsert parent jurisdictions before sub-jurisdictions to satisfy the FK constraint on environments where seed.ts has not been run. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
|
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
1 participant
Summary
allowed_provenancevalues (MANUAL_UPLOAD/COMMON_PLATFORM) — values that don't exist in the application's provenance system — so the CLASSIFIED sensitivity check never matched any real user provenance, allowing public access to protected lists.canAccessPublicationto support comma-separated provenances (e.g.CRIME_IDAM,PI_AADfor Magistrates lists) via a.split(",").includes()check instead of===.list-type-data.ts) to match the authoritative Javapip-data-modelsenum. Added two idempotent SQL scripts to patch existing environments.resolveListTypehelper from@hmcts/publicationto eliminate 4 copies of theprisma.listType.findUnique + ListTypeconstruction pattern. Reduced code duplication from 11.99% to 1.98%.Changes
libs/location/src/list-type-data.ts— removed deadidfield; fixed provenances: SJP lists →PI_AAD, Magistrates lists →CRIME_IDAM,PI_AAD, 31 non-strategic lists →CFT_IDAM, Kings Bench Masters →CFT_IDAMlibs/publication/src/authorisation/service.ts— comma-separated provenance support; extractedresolveListTypeasync helperlibs/public-pages/src/flat-file/flat-file-service.ts— usesresolveListTypeinstead of inline Prisma lookupapps/web/src/pages/(list-types)/list-type-handler.ts— usesresolveListType; merged duplicate render callbacks; extractedrenderErrorhelper;createUtiacDailyRenderdelegates tocreateWeeklyHearingListRenderapps/postgres/prisma/scripts/001_insert_missing_list_types.sql— idempotent upsert of all 59 list types with correct provenancesapps/postgres/prisma/scripts/002_update_list_type_provenances.sql— targeted UPDATE statements to fix existing DB rowsTest plan
yarn test— 184/184 test files pass, 1876 tests passyarn lint:fix— no errors🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores