Feature/434 Add CIC, SEND and AST tribunal hearing lists - #772
Conversation
📝 WalkthroughWalkthroughThree new hearing list types are added end-to-end: SEND daily, CIC weekly, and AST daily. The change set adds their packages, schemas, renderers, PDF/email support, web pages, shared registries, and the related location, dependency, and documentation wiring. ChangesSEND, CIC, and AST Hearing List Rollout
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 14s ⏱️ Results for commit 4dc55b5. ♻️ 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: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/worktrees/vibe-438 (1)
1-155: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove this checked-in worktree artefact.
This file is unrelated to the hearing-list change and appears to be a local
.claude/worktreessnapshot containing an infra diff, so it should not ship in this PR.
🟡 Minor comments (10)
docs/tickets/434/tasks.md-106-106 (1)
106-106: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix CIC template field access guidance.
Line 106 currently instructs
hearing.venuePlatform, which conflicts with the defined data key"venue/platform"and can lead to broken rendering if followed literally. Please update the checklist text to bracket notation guidance.apps/web/src/pages/(public)/summary-of-publications/index.ts-61-62 (1)
61-62: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRename constant to SCREAMING_SNAKE_CASE.
Line 61 declares a constant as
weeklyListTypes; please rename it (for example,WEEKLY_LIST_TYPES) to match the project’s constant naming rule.As per coding guidelines, "Use SCREAMING_SNAKE_CASE for constant declarations (e.g.,
MAX_FILE_SIZE,DEFAULT_TIMEOUT)."Source: Coding guidelines
libs/list-types/send-daily-hearing-list/src/rendering/renderer.test.ts-20-21 (1)
20-21: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid timezone-flaky date construction in tests.
Using
new Date("2025-06-20")can shift calendar day by runtime timezone. Build the date with numeric components (for examplenew Date(2025, 5, 20)) so expected outputs stay stable.Also applies to: 44-45, 59-60, 73-74
apps/web/src/pages/(list-types)/send-daily-hearing-list/index.ts-23-25 (1)
23-25: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNormalise locale to supported values before rendering.
Line 23 can pass arbitrary locale into
renderSendDailyHearingListData, whiletfalls back to EN unless locale is exactlycy. That can yield mixed-language output. Clamp locale to"en" | "cy"first.Also applies to: 77-79
apps/web/src/pages/(list-types)/cic-weekly-hearing-list/index.test.ts-108-109 (1)
108-109: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace
as anyartefact casts with a concrete typed fixture.The
as anycasts here bypass type safety and can hide controller contract changes in tests.As per coding guidelines, “Enable TypeScript strict mode and avoid
anytype without justification”.Also applies to: 177-177, 197-197, 239-239
Source: Coding guidelines
libs/list-types/cic-weekly-hearing-list/src/schemas/cic-weekly-hearing-list.json-19-19 (1)
19-19: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTighten
hearingTimeschema to match runtime validation.Line 19 allows values such as
00amand19pm, while the converter validation only allows 1–12 hours. Aligning both avoids inconsistent acceptance paths.Suggested fix
- "pattern": "^\\d{1,2}([:.]\\d{2})?[ap]m\\s*$", + "pattern": "^(0?[1-9]|1[0-2])([:.][0-5][0-9])?[ap]m\\s*$",libs/list-types/cic-weekly-hearing-list/src/email-summary/summary-builder.ts-6-12 (1)
6-12: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFormat the hearing date before adding it to the email summary.
renderCicWeeklyHearingListDataformatshearing.datefor display, which implies this field is still in source format here. Returning it verbatim will leak the raw date value into emails; the current test only passes because it feeds an already formatted string. Use the same display-date formatting here, or pass a preformatted date into this builder.libs/list-types/cic-weekly-hearing-list/src/pdf/pdf-template.njk-2-2 (1)
2-2: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winWelsh PDFs still contain hard-coded English.
lang="en"andNo hearings scheduled.bypass the locale data, so a Welsh render still announces English metadata and fallback text.Also applies to: 58-60
apps/web/src/pages/(list-types)/cic-weekly-hearing-list/cic-weekly-hearing-list.njk-53-82 (1)
53-82: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle the empty-list case in the page template.
renderCicWeeklyHearingListDataaccepts an empty array, but this template always renders an empty table shell. That leaves users with a blank results area instead of a clear “no hearings” message.libs/list-types/ast-daily-hearing-list/src/pdf/pdf-template.njk-55-56 (1)
55-56: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the empty-state text.
No hearings scheduled.bypasses the locale bundle, so Welsh PDFs fall back to English whenever the list is empty. Render this throughtlike the rest of the template.Proposed change
- <p>No hearings scheduled.</p> + <p>{{ t.noHearingsScheduled }}</p>
🧹 Nitpick comments (7)
libs/list-types/send-daily-hearing-list/src/config.ts (1)
7-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign exported constants with SCREAMING_SNAKE_CASE.
Please rename
moduleRoot,assets, andschemaPathto SCREAMING_SNAKE_CASE for guideline compliance.As per coding guidelines, "Use SCREAMING_SNAKE_CASE for constant declarations (e.g.,
MAX_FILE_SIZE,DEFAULT_TIMEOUT)".Source: Coding guidelines
apps/web/src/pages/(list-types)/send-daily-hearing-list/index.test.ts (1)
103-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
as anycasts with typed test fixtures.Use a concrete artefact test type (or
satisfies) so these mocks fail fast when the contract changes.As per coding guidelines, "Enable TypeScript strict mode and avoid
anytype without justification".Also applies to: 166-166, 186-186, 223-223
Source: Coding guidelines
libs/list-types/send-daily-hearing-list/src/rendering/renderer.ts (1)
4-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReorder exports to match the TypeScript module ordering rule.
RenderOptionsandRenderedDataare declared before the exported function; per the project rule, exported functions should come before interfaces/types, with interfaces/types at the bottom.As per coding guidelines: "
**/*.ts: Order module exports: top-level constants first, then exported functions, then other functions ordered by usage, with interfaces and types at the bottom".Also applies to: 21-34
Source: Coding guidelines
libs/list-types/cic-weekly-hearing-list/src/index.ts (1)
3-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove type exports to the bottom to match module export ordering.
Line 3 places a type export before value exports; reorder so types are last in the module.
As per coding guidelines, “Order module exports: top-level constants first, then exported functions, then other functions ordered by usage, with interfaces and types at the bottom”.
Source: Coding guidelines
libs/list-types/cic-weekly-hearing-list/src/rendering/renderer.ts (1)
4-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the exported interfaces below the renderer.
This module puts exported types ahead of the exported function, which breaks the repo ordering rule for
.tsfiles.As per coding guidelines,
**/*.ts:Order module exports: top-level constants first, then exported functions, then other functions ordered by usage, with interfaces and types at the bottom.Source: Coding guidelines
libs/list-types/ast-daily-hearing-list/src/config.ts (1)
4-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse SCREAMING_SNAKE_CASE for constants.
Line 4–9 constant declarations are in lower/camel case; please align them with the project constant naming rule for TS files.
As per coding guidelines, "Use SCREAMING_SNAKE_CASE for constant declarations (e.g.,
MAX_FILE_SIZE,DEFAULT_TIMEOUT)."Source: Coding guidelines
apps/web/src/pages/(list-types)/ast-daily-hearing-list/index.test.ts (1)
5-233: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign constant names with SCREAMING_SNAKE_CASE.
Line 5 onwards contains many
constdeclarations in camelCase (for example test fixtures/mocks); please rename these constants to match the repository naming standard.As per coding guidelines, "Use SCREAMING_SNAKE_CASE for constant declarations (e.g.,
MAX_FILE_SIZE,DEFAULT_TIMEOUT)."Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c4c9bb5-01af-4725-bc20-190ded3902d3
⛔ Files ignored due to path filters (2)
requirements/requirements.dbis excluded by!**/*.dbyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (73)
.claude/worktrees/vibe-438apps/web/package.jsonapps/web/src/pages/(admin)/manual-upload-summary/index.tsapps/web/src/pages/(admin)/non-strategic-upload-summary/index.tsapps/web/src/pages/(admin)/non-strategic-upload/index.tsapps/web/src/pages/(list-types)/ast-daily-hearing-list/ast-daily-hearing-list.njkapps/web/src/pages/(list-types)/ast-daily-hearing-list/index.test.tsapps/web/src/pages/(list-types)/ast-daily-hearing-list/index.tsapps/web/src/pages/(list-types)/cic-weekly-hearing-list/cic-weekly-hearing-list.njkapps/web/src/pages/(list-types)/cic-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/cic-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/send-daily-hearing-list/index.test.tsapps/web/src/pages/(list-types)/send-daily-hearing-list/index.tsapps/web/src/pages/(list-types)/send-daily-hearing-list/send-daily-hearing-list.njkapps/web/src/pages/(public)/summary-of-publications/index.tsdocs/tickets/434/plan.mddocs/tickets/434/tasks.mddocs/tickets/434/ticket.mdlibs/list-types/ast-daily-hearing-list/package.jsonlibs/list-types/ast-daily-hearing-list/src/config.test.tslibs/list-types/ast-daily-hearing-list/src/config.tslibs/list-types/ast-daily-hearing-list/src/conversion/ast-config.tslibs/list-types/ast-daily-hearing-list/src/email-summary/summary-builder.test.tslibs/list-types/ast-daily-hearing-list/src/email-summary/summary-builder.tslibs/list-types/ast-daily-hearing-list/src/index.tslibs/list-types/ast-daily-hearing-list/src/locales/cy.tslibs/list-types/ast-daily-hearing-list/src/locales/en.tslibs/list-types/ast-daily-hearing-list/src/models/types.tslibs/list-types/ast-daily-hearing-list/src/pdf/pdf-generator.tslibs/list-types/ast-daily-hearing-list/src/pdf/pdf-template.njklibs/list-types/ast-daily-hearing-list/src/rendering/renderer.test.tslibs/list-types/ast-daily-hearing-list/src/rendering/renderer.tslibs/list-types/ast-daily-hearing-list/src/schemas/ast-daily-hearing-list.jsonlibs/list-types/ast-daily-hearing-list/tsconfig.jsonlibs/list-types/cic-weekly-hearing-list/package.jsonlibs/list-types/cic-weekly-hearing-list/src/config.test.tslibs/list-types/cic-weekly-hearing-list/src/config.tslibs/list-types/cic-weekly-hearing-list/src/conversion/cic-config.tslibs/list-types/cic-weekly-hearing-list/src/email-summary/summary-builder.test.tslibs/list-types/cic-weekly-hearing-list/src/email-summary/summary-builder.tslibs/list-types/cic-weekly-hearing-list/src/index.tslibs/list-types/cic-weekly-hearing-list/src/locales/cy.tslibs/list-types/cic-weekly-hearing-list/src/locales/en.tslibs/list-types/cic-weekly-hearing-list/src/models/types.tslibs/list-types/cic-weekly-hearing-list/src/pdf/pdf-generator.tslibs/list-types/cic-weekly-hearing-list/src/pdf/pdf-template.njklibs/list-types/cic-weekly-hearing-list/src/rendering/renderer.test.tslibs/list-types/cic-weekly-hearing-list/src/rendering/renderer.tslibs/list-types/cic-weekly-hearing-list/src/schemas/cic-weekly-hearing-list.jsonlibs/list-types/cic-weekly-hearing-list/tsconfig.jsonlibs/list-types/send-daily-hearing-list/package.jsonlibs/list-types/send-daily-hearing-list/src/config.test.tslibs/list-types/send-daily-hearing-list/src/config.tslibs/list-types/send-daily-hearing-list/src/conversion/send-config.tslibs/list-types/send-daily-hearing-list/src/email-summary/summary-builder.test.tslibs/list-types/send-daily-hearing-list/src/email-summary/summary-builder.tslibs/list-types/send-daily-hearing-list/src/index.tslibs/list-types/send-daily-hearing-list/src/locales/cy.tslibs/list-types/send-daily-hearing-list/src/locales/en.tslibs/list-types/send-daily-hearing-list/src/models/types.tslibs/list-types/send-daily-hearing-list/src/pdf/pdf-generator.tslibs/list-types/send-daily-hearing-list/src/pdf/pdf-template.njklibs/list-types/send-daily-hearing-list/src/rendering/renderer.test.tslibs/list-types/send-daily-hearing-list/src/rendering/renderer.tslibs/list-types/send-daily-hearing-list/src/schemas/send-daily-hearing-list.jsonlibs/list-types/send-daily-hearing-list/tsconfig.jsonlibs/location/src/list-type-data.tslibs/location/src/location-data.tslibs/notifications/package.jsonlibs/notifications/src/notification/notification-service.tslibs/publication/package.jsonlibs/publication/src/processing/service.tstsconfig.json
| return res.status(400).render("errors/common", { | ||
| en, | ||
| cy, | ||
| errorTitle: "Bad Request", | ||
| errorMessage: "Missing artefactId parameter" | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Localise the error branches as well.
These responses send English errorTitle and errorMessage strings directly, so ?lng=cy still returns English on every failure path. Feed these from language-specific content the same way the success path uses t. As per coding guidelines, "Provide both en and cy content objects in page controllers and render both language versions in templates" and "Implement Welsh language support on all user-facing text with separate en.ts and cy.ts content files".
Also applies to: 41-46, 69-74, 97-101
Source: Coding guidelines
| if (!artefactId) { | ||
| return res.status(400).render("errors/common", { | ||
| en, | ||
| cy, | ||
| errorTitle: "Bad Request", | ||
| errorMessage: "Missing artefactId parameter" | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Localise the error copy as well.
These failure paths always render English errorTitle/errorMessage, so Welsh requests lose language support exactly when something goes wrong.
As per coding guidelines, apps/web/src/pages/**/index.ts: Provide both en and cy content objects in page controllers and render both language versions in templates and Implement Welsh language support on all user-facing text with separate en.ts and cy.ts content files.
Also applies to: 41-46, 69-74, 97-102
Source: Coding guidelines
| importantInformationParagraphs: [ | ||
| "Mae cyfiawnder agored yn egwyddor sylaenol ein system gyfiawnder.", | ||
| "Wrth ystyried defnyddio technoleg ffôn a fideo, bydd y farnwriaeth yn rhoi sylw i egwyddorion cyfiawnder agored. Gall barnwyr benderfynu cynnal gwrandawiad yn breifat os oes angen hynny er mwyn sicrhau'r broses o weinyddu cyfiawnder yn briodol.", | ||
| "Bydd partïon a chynrychiolwyr y Tribiwnlys Digolledu am Anafiadau Troseddol yn cael gwybod yn uniongyrchol am y trefniadau ar gyfer gwrando achosion o bell. Dylai unrhyw un arall sydd â diddordeb mewn ymuno â'r gwrandawiad o bell gysylltu â Swyddfa'r Tribiwnlys Digolledu am Anafiadau Troseddol yn uniongyrchol, cyn dyddiad y gwrandawiad, trwy e-bostio (insert relevant office mailbox email address) fel y gellir gwneud trefniadau. Dylai'r manylion canlynol gael eu cynnwys yn llinell pwnc yr e-bost [OBSERVER/MEDIA] REQUEST – [AN Other v CICA] – [hearing date]. Os yw'r achos yn cael ei wrando yn breifat neu os yw'n destun cyfyngiad adrodd, bydd hyn yn cael ei nodi." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Replace the mailbox placeholder in the Welsh copy.
Line 15 still contains (insert relevant office mailbox email address), so Welsh users will see a placeholder instead of the actual CIC contact route that the English copy already provides. That breaks the remote-observer instructions on the Welsh page.
Suggested fix
- "Bydd partïon a chynrychiolwyr y Tribiwnlys Digolledu am Anafiadau Troseddol yn cael gwybod yn uniongyrchol am y trefniadau ar gyfer gwrando achosion o bell. Dylai unrhyw un arall sydd â diddordeb mewn ymuno â'r gwrandawiad o bell gysylltu â Swyddfa'r Tribiwnlys Digolledu am Anafiadau Troseddol yn uniongyrchol, cyn dyddiad y gwrandawiad, trwy e-bostio (insert relevant office mailbox email address) fel y gellir gwneud trefniadau. Dylai'r manylion canlynol gael eu cynnwys yn llinell pwnc yr e-bost [OBSERVER/MEDIA] REQUEST – [AN Other v CICA] – [hearing date]. Os yw'r achos yn cael ei wrando yn breifat neu os yw'n destun cyfyngiad adrodd, bydd hyn yn cael ei nodi."
+ "Bydd partïon a chynrychiolwyr y Tribiwnlys Digolledu am Anafiadau Troseddol yn cael gwybod yn uniongyrchol am y trefniadau ar gyfer gwrando achosion o bell. Dylai unrhyw un arall sydd â diddordeb mewn ymuno â'r gwrandawiad o bell gysylltu â Swyddfa'r Tribiwnlys Digolledu am Anafiadau Troseddol yn uniongyrchol, cyn dyddiad y gwrandawiad, trwy e-bostio CIC.enquiries@Justice.gov.uk fel y gellir gwneud trefniadau. Dylai'r manylion canlynol gael eu cynnwys yn llinell pwnc yr e-bost [OBSERVER/MEDIA] REQUEST – [AN Other v CICA] – [hearing date]. Os yw'r achos yn cael ei wrando yn breifat neu os yw'n destun cyfyngiad adrodd, bydd hyn yn cael ei nodi."📝 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.
| "Bydd partïon a chynrychiolwyr y Tribiwnlys Digolledu am Anafiadau Troseddol yn cael gwybod yn uniongyrchol am y trefniadau ar gyfer gwrando achosion o bell. Dylai unrhyw un arall sydd â diddordeb mewn ymuno â'r gwrandawiad o bell gysylltu â Swyddfa'r Tribiwnlys Digolledu am Anafiadau Troseddol yn uniongyrchol, cyn dyddiad y gwrandawiad, trwy e-bostio (insert relevant office mailbox email address) fel y gellir gwneud trefniadau. Dylai'r manylion canlynol gael eu cynnwys yn llinell pwnc yr e-bost [OBSERVER/MEDIA] REQUEST – [AN Other v CICA] – [hearing date]. Os yw'r achos yn cael ei wrando yn breifat neu os yw'n destun cyfyngiad adrodd, bydd hyn yn cael ei nodi." | |
| "Bydd partïon a chynrychiolwyr y Tribiwnlys Digolledu am Anafiadau Troseddol yn cael gwybod yn uniongyrchol am y trefniadau ar gyfer gwrando achosion o bell. Dylai unrhyw un arall sydd â diddordeb mewn ymuno â'r gwrandawiad o bell gysylltu â Swyddfa'r Tribiwnlys Digolledu am Anafiadau Troseddol yn uniongyrchol, cyn dyddiad y gwrandawiad, trwy e-bostio CIC.enquiries@Justice.gov.uk fel y gellir gwneud trefniadau. Dylai'r manylion canlynol gael eu cynnwys yn llinell pwnc yr e-bost [OBSERVER/MEDIA] REQUEST – [AN Other v CICA] – [hearing date]. Os yw'r achos yn cael ei wrando yn breifat neu os yw'n destun cyfyngiad adrodd, bydd hyn yn cael ei nodi." |
| </tbody> | ||
| </table> | ||
| {% else %} | ||
| <p>No hearings scheduled.</p> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Localise the empty-state text.
The hard-coded No hearings scheduled. bypasses t translations, so Welsh output stays in English for this state. Use a locale key from t here as well.
| "required": ["time", "caseReferenceNumber", "respondent", "hearingType", "venue", "timeEstimate"], | ||
| "properties": { | ||
| "time": { | ||
| "title": "Time of hearing", | ||
| "type": "string", | ||
| "pattern": "^\\d{1,2}([:.]\\d{2})?[ap]m\\s*$", | ||
| "examples": ["10am", "2:30pm"] | ||
| }, | ||
| "caseReferenceNumber": { | ||
| "title": "Case reference number", | ||
| "type": "string", | ||
| "pattern": "^(?!(.|\\r|\\n)*<[^>]+>)(.|\\r|\\n)*$" | ||
| }, | ||
| "respondent": { | ||
| "title": "Respondent", | ||
| "type": "string", | ||
| "pattern": "^(?!(.|\\r|\\n)*<[^>]+>)(.|\\r|\\n)*$" | ||
| }, | ||
| "hearingType": { | ||
| "title": "Type of hearing", | ||
| "type": "string", | ||
| "pattern": "^(?!(.|\\r|\\n)*<[^>]+>)(.|\\r|\\n)*$" | ||
| }, | ||
| "venue": { | ||
| "title": "Venue", | ||
| "type": "string", | ||
| "pattern": "^(?!(.|\\r|\\n)*<[^>]+>)(.|\\r|\\n)*$" | ||
| }, | ||
| "timeEstimate": { | ||
| "title": "Time estimate", | ||
| "type": "string", | ||
| "pattern": "^(?!(.|\\r|\\n)*<[^>]+>)(.|\\r|\\n)*$" | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Required fields can still be empty strings.
required only enforces key presence. Add minLength: 1 (and ideally trim-aware validation) to required string fields to prevent blank records passing validation.
| "pattern": "^\\d{1,2}([:.]\\d{2})?[ap]m\\s*$", | ||
| "examples": ["10am", "2:30pm"] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Tighten time validation to reject invalid hours/minutes.
Current regex accepts invalid values (for example 19pm). Please constrain hour/minute ranges in the schema pattern.
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. |
…orage Merge master (fa9b633) bringing in: - Azure Blob Storage for all publication uploads/retrieval (#765) - Container-aware blob routing (#773) Conflict resolutions: - pdf-utilities.ts: use blob imports from master (filesystem approach removed) - notifications/package.json: keep both @hmcts/ast-daily-hearing-list and @hmcts/azure-blob - notification-service.ts: keep both AST import and azure-blob import Refactor AST, CIC, SEND page controllers to match existing list type pattern: - Replace filesystem readFile + manual handler with createSimpleListTypeHandler - Use getPublicationJson (blob) instead of reading from temp upload directory - Use resolveDataSource for provenance label resolution - Update tests to mock getPublicationJson instead of readFile 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/src/pages/(admin)/manual-upload-summary/index.ts (1)
18-20: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore the English fallback for Welsh list names.
Line 19 now falls back from
welshFriendlyNamestraight toname, so partially translated list types will show the internal name instead of the friendly label. Other flows still usewelshFriendlyName→friendlyName→name, so this regresses the summary text.Suggested fix
- const listTypeName = listType - ? (locale === "cy" ? listType.welshFriendlyName : listType.shortenedFriendlyName || listType.friendlyName) || listType.name || uploadData.listType - : uploadData.listType; + const listTypeName = listType + ? ( + locale === "cy" + ? listType.welshFriendlyName || listType.friendlyName + : listType.shortenedFriendlyName || listType.friendlyName + ) || listType.name || uploadData.listType + : uploadData.listType;libs/location/src/seed-data.ts (1)
157-163: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftBackfill the new locations outside the empty-DB seed path.
This write only runs during the initial seed. Existing databases return early at Line 40 and only call
seedListTypes(), so the new SEND/CIC/AST virtual locations in this PR will never be inserted unless the environment starts from an empty DB. Please make location seeding idempotent as well, or add a dedicated backfill for the newlocationandlocationReferencerows.libs/list-types/send-daily-hearing-list/src/pdf/pdf-generator.ts (1)
19-23: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftUse the artefact timestamp here, not
new Date().Line 22 makes the PDF header depend on render time rather than the publication’s actual
lastReceivedDate. The web controller already passesartefact.lastReceivedDateinto the same renderer, so the same list can show different “last updated” values between HTML and PDF, and regenerated PDFs will drift over time. Please thread the artefact timestamp through the shared PDF options/registry and use that here instead.libs/list-types/ast-daily-hearing-list/src/pdf/pdf-generator.ts (1)
19-24: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the locale bundle for the PDF title.
renderAstDailyHearingListDatais given an EnglishlistTitlebeforeloadTranslations()runs, so Welsh PDFs still render an English heading even though the rest of the template is locale-aware.Possible fix
- const renderedData = renderAstDailyHearingListData(options.jsonData, { - locale: options.locale, - contentDate: options.contentDate, - lastReceivedDate: new Date().toISOString(), - listTitle: "Asylum Support Tribunal Daily Hearing List" - }); - const translations = await loadTranslations( options.locale, () => import("../locales/en.js"), () => import("../locales/cy.js") ); + + const renderedData = renderAstDailyHearingListData(options.jsonData, { + locale: options.locale, + contentDate: options.contentDate, + lastReceivedDate: new Date().toISOString(), + listTitle: + typeof translations.pageTitle === "string" + ? translations.pageTitle + : "Asylum Support Tribunal Daily Hearing List" + });Also applies to: 26-30
🧹 Nitpick comments (2)
libs/list-types/common/src/index.ts (1)
32-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMatch the barrel export order to the repo convention.
This block still places
type BasePdfGenerationOptionsahead of exported functions. Please move the type exports to the end of the block so the barrel follows the repository ordering rule.Suggested reorder
export { - type BasePdfGenerationOptions, buildPdfFromRenderedList, configureNunjucks, createPdfErrorResult, loadTranslations, MAX_PDF_SIZE_BYTES, - type PdfGenerationResult, - savePdfToStorage + savePdfToStorage, + type BasePdfGenerationOptions, + type PdfGenerationResult } from "./pdf/pdf-utilities.js";As per coding guidelines, "Order module exports: top-level constants first, then exported functions, then other functions ordered by usage, with interfaces and types at the bottom".
Source: Coding guidelines
apps/web/src/pages/(list-types)/send-daily-hearing-list/index.test.ts (1)
96-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
as anyin these artefact fixtures.These casts switch off the type checks that would otherwise catch contract drift in the controller tests. Please type the fixture once with the publication artefact shape, or use
satisfies, and reuse it across the cases. As per coding guidelines, "Enable TypeScript strict mode and avoidanytype without justification".Also applies to: 165-165, 185-185, 222-222
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 71c9a63d-1562-42e7-a6e6-e1dbf46d82ad
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (30)
apps/web/package.jsonapps/web/src/pages/(admin)/manual-upload-summary/index.tsapps/web/src/pages/(admin)/non-strategic-upload-summary/index.tsapps/web/src/pages/(list-types)/ast-daily-hearing-list/index.test.tsapps/web/src/pages/(list-types)/ast-daily-hearing-list/index.tsapps/web/src/pages/(list-types)/cic-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/cic-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/send-daily-hearing-list/index.test.tsapps/web/src/pages/(list-types)/send-daily-hearing-list/index.tsapps/web/src/pages/(public)/summary-of-publications/index.tslibs/list-types/ast-daily-hearing-list/src/email-summary/summary-builder.test.tslibs/list-types/ast-daily-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/ast-daily-hearing-list/src/pdf/pdf-generator.tslibs/list-types/ast-daily-hearing-list/src/pdf/pdf-template.njklibs/list-types/cic-weekly-hearing-list/src/email-summary/summary-builder.test.tslibs/list-types/cic-weekly-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/cic-weekly-hearing-list/src/pdf/pdf-generator.tslibs/list-types/common/package.jsonlibs/list-types/common/src/index.tslibs/list-types/common/src/pdf/pdf-utilities.tslibs/list-types/send-daily-hearing-list/src/email-summary/summary-builder.test.tslibs/list-types/send-daily-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/send-daily-hearing-list/src/pdf/pdf-generator.tslibs/location/src/location-data.tslibs/location/src/repository/model.tslibs/location/src/seed-data.tslibs/notifications/package.jsonlibs/notifications/src/notification/notification-service.tslibs/publication/package.jsontsconfig.json
✅ Files skipped from review due to trivial changes (2)
- libs/location/src/repository/model.ts
- libs/location/src/location-data.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- libs/list-types/send-daily-hearing-list/src/email-summary/summary-builder.test.ts
- libs/list-types/ast-daily-hearing-list/src/pdf/pdf-template.njk
- apps/web/src/pages/(admin)/non-strategic-upload-summary/index.ts
- apps/web/package.json
- tsconfig.json
- libs/notifications/package.json
- libs/publication/package.json
- libs/notifications/src/notification/notification-service.ts
| cy, | ||
| validate, | ||
| logPrefix: "send-daily-hearing-list", | ||
| serverError: { errorTitle: "Server Error", errorMessage: "An error occurred while loading the list" }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localise the 500 error copy.
Line 18 hardcodes English-only text, so Welsh users will still see English on this path even though the controller already carries en/cy content. Please move this message into the locale content objects and select it per locale before passing serverError. As per coding guidelines, "Provide both en and cy content objects in page controllers and render both language versions in templates" and "Implement Welsh language support on all user-facing text with separate en.ts and cy.ts content files".
Source: Coding guidelines
…solution The import was dropped when resolving the merge conflict in pdf-utilities.ts — master's conflict marker only showed azure-blob imports, so the pdf-generation import (needed by our branch's buildPdfFromRenderedList function) was lost. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
… types - Format single-digit days as zero-padded (e.g. "01 July 2026") in formatDisplayDate, formatLastUpdatedDateTime, and formatDdMmYyyyDate - Extract shared PDF generation logic into generateListPdf<T>() in list-types-common, eliminating ~90 lines of duplication across SEND, CIC, and AST pdf-generator files - Update all affected renderer and generator tests to reflect new date format and new mock boundary (generateListPdf instead of buildPdfFromRenderedList/loadTranslations) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (3)
libs/list-types/cic-weekly-hearing-list/src/pdf/pdf-generator.test.ts (1)
83-91: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a Welsh-locale assertion once the title-translation bug is fixed.
This test only checks the static English
listTitle. OncegenerateListPdfis fixed to usetranslations.pageTitleforlocale: "cy", add a case asserting the Welsh title is forwarded/rendered — otherwise this regression path stays uncovered.libs/list-types/common/src/pdf/pdf-utilities.ts (1)
66-79: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInterfaces placed mid-file, not at the bottom of exports.
RenderedListDataandListPdfOptionsare declared betweenloadTranslationsandgenerateListPdf/buildPdfFromRenderedList. As per coding guidelines,**/*.tsexports should be ordered with "interfaces and types at the bottom". Consider moving these two interfaces to the end of the file.Source: Coding guidelines
libs/list-types/send-daily-hearing-list/src/pdf/pdf-generator.ts (1)
12-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared
provenanceLabelresolution into@hmcts/list-types-common.The
options.provenance ? PROVENANCE_LABELS[...] || options.provenance : ""block is duplicated verbatim inast-daily-hearing-list/src/pdf/pdf-generator.ts(and likely other list-type packages per the PR's file list). SincegenerateListPdfalready lives in@hmcts/list-types-common, a smallresolveProvenanceLabel(provenance)helper there would remove the copy-paste and centralise fallback behaviour.♻️ Suggested helper
// libs/list-types/common/src/pdf/pdf-utilities.ts export function resolveProvenanceLabel(provenance?: string): string { if (!provenance) return ""; return PROVENANCE_LABELS[provenance as keyof typeof PROVENANCE_LABELS] || provenance; }- provenanceLabel: options.provenance ? PROVENANCE_LABELS[options.provenance as keyof typeof PROVENANCE_LABELS] || options.provenance : "", + provenanceLabel: resolveProvenanceLabel(options.provenance),
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 90b243d7-9033-436a-86b1-dab37cae6085
📒 Files selected for processing (18)
apps/web/src/pages/(list-types)/ast-daily-hearing-list/index.tsapps/web/src/pages/(list-types)/cic-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/send-daily-hearing-list/index.test.tsapps/web/src/pages/(list-types)/send-daily-hearing-list/index.tslibs/list-types/ast-daily-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/ast-daily-hearing-list/src/pdf/pdf-generator.tslibs/list-types/ast-daily-hearing-list/src/rendering/renderer.test.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/rendering/renderer.test.tslibs/list-types/cic-weekly-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/cic-weekly-hearing-list/src/pdf/pdf-generator.tslibs/list-types/cic-weekly-hearing-list/src/rendering/renderer.test.tslibs/list-types/common/src/index.tslibs/list-types/common/src/pdf/pdf-utilities.tslibs/list-types/common/src/rendering/date-formatting.test.tslibs/list-types/common/src/rendering/date-formatting.tslibs/list-types/send-daily-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/send-daily-hearing-list/src/pdf/pdf-generator.tslibs/list-types/send-daily-hearing-list/src/rendering/renderer.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- libs/list-types/send-daily-hearing-list/src/rendering/renderer.test.ts
- libs/list-types/cic-weekly-hearing-list/src/rendering/renderer.test.ts
- libs/list-types/ast-daily-hearing-list/src/rendering/renderer.test.ts
- apps/web/src/pages/(list-types)/ast-daily-hearing-list/index.ts
- apps/web/src/pages/(list-types)/cic-weekly-hearing-list/index.ts
- apps/web/src/pages/(list-types)/send-daily-hearing-list/index.ts
- apps/web/src/pages/(list-types)/send-daily-hearing-list/index.test.ts
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
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. |
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1e8b1c5b-5959-4fc1-9692-4f5db386e722
📒 Files selected for processing (9)
apps/web/src/pages/(admin)/manual-upload-summary/index.test.tsapps/web/src/pages/(admin)/non-strategic-upload-summary/index.test.tsapps/web/src/pages/(public)/summary-of-publications/index.test.tsapps/web/src/pages/(public)/summary-of-publications/index.tslibs/list-types/ast-daily-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/cic-weekly-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/common/src/pdf/pdf-utilities.test.tslibs/list-types/send-daily-hearing-list/src/pdf/pdf-generator.test.tslibs/publication/src/processing/service.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/list-types/send-daily-hearing-list/src/pdf/pdf-generator.test.ts
- libs/list-types/ast-daily-hearing-list/src/pdf/pdf-generator.test.ts
- libs/list-types/cic-weekly-hearing-list/src/pdf/pdf-generator.test.ts
| it("should call extractAndStoreArtefactSearch when a JSON file is uploaded", async () => { | ||
| const jsonContent = JSON.stringify({ data: "test" }); | ||
| const jsonUploadData = { | ||
| ...mockUploadData, | ||
| fileName: "test-hearing-list.json", | ||
| file: Buffer.from(jsonContent) | ||
| }; | ||
|
|
||
| vi.mocked(getManualUpload).mockResolvedValue(jsonUploadData); | ||
| vi.mocked(saveUploadedFile).mockResolvedValue(".json"); | ||
| vi.mocked(processPublication).mockResolvedValue({}); | ||
| vi.mocked(createArtefact).mockResolvedValue({ artefactId: "test-artefact-id-123", isUpdate: false }); | ||
| vi.mocked(extractAndStoreArtefactSearch).mockResolvedValue(undefined); | ||
|
|
||
| const session = { save: vi.fn((callback) => callback()) }; | ||
| const req = { query: { uploadId: "test-upload-id" }, session } as unknown as Request; | ||
| const res = { redirect: vi.fn(), render: vi.fn() } as unknown as Response; | ||
|
|
||
| await callHandler(POST, req, res); | ||
|
|
||
| expect(extractAndStoreArtefactSearch).toHaveBeenCalledWith("test-artefact-id-123", 6, { data: "test" }); | ||
| expect(res.redirect).toHaveBeenCalledWith("/manual-upload-success"); | ||
| }); | ||
|
|
||
| it("should continue upload when extractAndStoreArtefactSearch throws", async () => { | ||
| const jsonContent = JSON.stringify({ data: "test" }); | ||
| const jsonUploadData = { | ||
| ...mockUploadData, | ||
| fileName: "test-hearing-list.json", | ||
| file: Buffer.from(jsonContent) | ||
| }; | ||
|
|
||
| vi.mocked(getManualUpload).mockResolvedValue(jsonUploadData); | ||
| vi.mocked(saveUploadedFile).mockResolvedValue(".json"); | ||
| vi.mocked(processPublication).mockResolvedValue({}); | ||
| vi.mocked(createArtefact).mockResolvedValue({ artefactId: "test-artefact-id-123", isUpdate: false }); | ||
| vi.mocked(extractAndStoreArtefactSearch).mockRejectedValue(new Error("Search extraction failed")); | ||
|
|
||
| const session = { save: vi.fn((callback) => callback()) }; | ||
| const req = { query: { uploadId: "test-upload-id" }, session } as unknown as Request; | ||
| const res = { redirect: vi.fn(), render: vi.fn() } as unknown as Response; | ||
|
|
||
| await callHandler(POST, req, res); | ||
|
|
||
| expect(res.redirect).toHaveBeenCalledWith("/manual-upload-success"); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- manual-upload-summary controller ---"
rg -n -C6 'extractAndStoreArtefactSearch|processPublication\(' 'apps/web/src/pages/(admin)/manual-upload-summary/index.ts'Repository: hmcts/cath-service
Length of output: 1776
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- locate processPublication implementation ---"
fd -a 'publication' . | head -n 20
echo
rg -n -C8 'function processPublication|const processPublication|processPublication\s*=' . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**'
echo
echo "--- locate extractAndStoreArtefactSearch implementation ---"
rg -n -C8 'extractAndStoreArtefactSearch' . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**'Repository: hmcts/cath-service
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- publication package files ---"
git ls-files | rg '^packages/.*/.*publication.*\.(ts|tsx|js)$|^apps/.*/.*publication.*\.(ts|tsx|js)$|^libs/.*/.*publication.*\.(ts|tsx|js)$'
echo
echo "--- search for processPublication and extractAndStoreArtefactSearch usages ---"
rg -n -C6 'processPublication|extractAndStoreArtefactSearch' packages apps libs --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**'Repository: hmcts/cath-service
Length of output: 50375
Avoid extracting JSON search data twice. The JSON upload path calls extractAndStoreArtefactSearch(...) directly and then passes the same jsonData into processPublication(...), which already calls it internally. Remove one of those calls to avoid duplicate DB work and the extra race window.
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
10 STATUS + IMPL changes (closed issue + merged closing PR → verified): REQ-0078 (#301): implemented → verified (PR #458) REQ-0105 (#428): in_progress → verified (PR #749) REQ-0106 (#429): approved → verified (PR #761) REQ-0107 (#431): implemented → verified (PR #701) REQ-0108 (#434): approved → verified (PR #772) REQ-0109 (#436): implemented → verified (PR #727) REQ-0112 (#467): implemented → verified (PR #670) REQ-0124 (#563): approved → verified (PR #782) REQ-0135 (#569): in_progress → verified (PR #748) REQ-0137 (#729): approved → verified (PR #766) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The database is derived entirely from schema.sql + seed.sql + migrations/*.sql, which init_db.sh applies in order. The migrations describe what is in it, so the binary adds nothing a clone cannot rebuild — and committing it means every sync PR carries a ~2MB binary diff that no reviewer can read. .gitignore has listed requirements/*.db since #695, but the file was already tracked by then: it was swept into #772 (an unrelated list-types PR) the same day, and gitignore does not apply to tracked files. Remove it from the index; it stays on disk and is now correctly ignored. Rebuild with: yarn requirements:build Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
Jira link
#434
Change description
Add CIC, SEND and AST tribunal hearing lists
Checklist
Summary by CodeRabbit