feat: add magistrates-standard-list and magistrates-public-list modules - #767
feat: add magistrates-standard-list and magistrates-public-list modules#767junaidiqbalmoj wants to merge 1 commit into
Conversation
Implements two new court list modules for magistrates courts with full rendering, PDF generation, email summary extraction, JSON schema validation, and bilingual (EN/CY) support. Both modules follow the civil-and-family-daily-cause-list pattern and are registered in the web app. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughTwo new monorepo packages — ChangesMagistrates Public & Standard List types
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 51 ✅ 6m 26s ⏱️ For more details on these failures, see this check. Results for commit 86a298f. ♻️ This comment has been updated with latest results. |
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
libs/list-types/magistrates-standard-list/src/index.ts (1)
1-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove type exports to the bottom of the barrel.
Line 1 places a type export before value exports; reorder so interfaces/types are last to match repo conventions.
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/magistrates-standard-list/src/schemas/magistrates-standard-list.json (1)
1-294: 🗄️ Data Integrity & Integration | 🔵 TrivialExtract the shared magistrates schema Both list schemas are identical; if that is intentional, use a common schema to avoid drift between the public and standard variants.
apps/web/src/pages/(list-types)/magistrates-public-list/index.test.ts (1)
96-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing the 403 access-denied test case.
The lib-level suite covers
canAccessPublicationDatareturningfalse, but this app-level suite jumps from "artefact not found" straight to the JSON-read path. Adding a 403 case keeps coverage aligned with the controller's access check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 92305571-69b0-4959-92ff-8ec4663402a1
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (49)
apps/web/package.jsonapps/web/src/app.tsapps/web/src/pages/(list-types)/magistrates-public-list/index.test.tsapps/web/src/pages/(list-types)/magistrates-public-list/index.tsapps/web/src/pages/(list-types)/magistrates-public-list/magistrates-public-list.njkapps/web/src/pages/(list-types)/magistrates-standard-list/index.test.tsapps/web/src/pages/(list-types)/magistrates-standard-list/index.tsapps/web/src/pages/(list-types)/magistrates-standard-list/magistrates-standard-list.njklibs/list-types/magistrates-public-list/package.jsonlibs/list-types/magistrates-public-list/src/config.tslibs/list-types/magistrates-public-list/src/email-summary/summary-builder.test.tslibs/list-types/magistrates-public-list/src/email-summary/summary-builder.tslibs/list-types/magistrates-public-list/src/index.tslibs/list-types/magistrates-public-list/src/models/types.tslibs/list-types/magistrates-public-list/src/pages/cy.tslibs/list-types/magistrates-public-list/src/pages/en.tslibs/list-types/magistrates-public-list/src/pages/index.test.tslibs/list-types/magistrates-public-list/src/pages/index.tslibs/list-types/magistrates-public-list/src/pages/magistrates-public-list.njklibs/list-types/magistrates-public-list/src/pdf/pdf-generator.test.tslibs/list-types/magistrates-public-list/src/pdf/pdf-generator.tslibs/list-types/magistrates-public-list/src/pdf/pdf-template.njklibs/list-types/magistrates-public-list/src/rendering/renderer.test.tslibs/list-types/magistrates-public-list/src/rendering/renderer.tslibs/list-types/magistrates-public-list/src/schemas/magistrates-public-list.jsonlibs/list-types/magistrates-public-list/src/validation/json-validator.test.tslibs/list-types/magistrates-public-list/src/validation/json-validator.tslibs/list-types/magistrates-public-list/tsconfig.jsonlibs/list-types/magistrates-standard-list/package.jsonlibs/list-types/magistrates-standard-list/src/config.tslibs/list-types/magistrates-standard-list/src/email-summary/summary-builder.test.tslibs/list-types/magistrates-standard-list/src/email-summary/summary-builder.tslibs/list-types/magistrates-standard-list/src/index.tslibs/list-types/magistrates-standard-list/src/models/types.tslibs/list-types/magistrates-standard-list/src/pages/cy.tslibs/list-types/magistrates-standard-list/src/pages/en.tslibs/list-types/magistrates-standard-list/src/pages/index.test.tslibs/list-types/magistrates-standard-list/src/pages/index.tslibs/list-types/magistrates-standard-list/src/pages/magistrates-standard-list.njklibs/list-types/magistrates-standard-list/src/pdf/pdf-generator.test.tslibs/list-types/magistrates-standard-list/src/pdf/pdf-generator.tslibs/list-types/magistrates-standard-list/src/pdf/pdf-template.njklibs/list-types/magistrates-standard-list/src/rendering/renderer.test.tslibs/list-types/magistrates-standard-list/src/rendering/renderer.tslibs/list-types/magistrates-standard-list/src/schemas/magistrates-standard-list.jsonlibs/list-types/magistrates-standard-list/src/validation/json-validator.test.tslibs/list-types/magistrates-standard-list/src/validation/json-validator.tslibs/list-types/magistrates-standard-list/tsconfig.jsontsconfig.json
| const jsonData = JSON.parse(jsonContent); | ||
|
|
||
| const validationResult = validateMagistratesStandardList(jsonData); | ||
| if (!validationResult.isValid) { | ||
| return res.status(400).render("errors/common", { | ||
| en, | ||
| cy, | ||
| errorTitle: t.errorTitle, | ||
| errorMessage: t.errorMessage | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Malformed JSON returns 500 instead of 400.
readFile failures are mapped to 404, and schema validation failures to 400, but JSON.parse on a corrupt temp file throws into the outer catch and produces a 500. A non-parseable payload is a bad/invalid publication, so it should be treated consistently with the validation-failure path (400).
🛠️ Proposed fix
- const jsonData = JSON.parse(jsonContent);
-
- const validationResult = validateMagistratesStandardList(jsonData);
- if (!validationResult.isValid) {
+ let jsonData: unknown;
+ try {
+ jsonData = JSON.parse(jsonContent);
+ } catch {
+ return res.status(400).render("errors/common", {
+ en,
+ cy,
+ errorTitle: t.errorTitle,
+ errorMessage: t.errorMessage
+ });
+ }
+
+ const validationResult = validateMagistratesStandardList(jsonData);
+ if (!validationResult.isValid) {📝 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.
| const jsonData = JSON.parse(jsonContent); | |
| const validationResult = validateMagistratesStandardList(jsonData); | |
| if (!validationResult.isValid) { | |
| return res.status(400).render("errors/common", { | |
| en, | |
| cy, | |
| errorTitle: t.errorTitle, | |
| errorMessage: t.errorMessage | |
| }); | |
| } | |
| let jsonData: unknown; | |
| try { | |
| jsonData = JSON.parse(jsonContent); | |
| } catch { | |
| return res.status(400).render("errors/common", { | |
| en, | |
| cy, | |
| errorTitle: t.errorTitle, | |
| errorMessage: t.errorMessage | |
| }); | |
| } | |
| const validationResult = validateMagistratesStandardList(jsonData); | |
| if (!validationResult.isValid) { | |
| return res.status(400).render("errors/common", { | |
| en, | |
| cy, | |
| errorTitle: t.errorTitle, | |
| errorMessage: t.errorMessage | |
| }); | |
| } |
| const jsonData = JSON.parse(jsonContent); | ||
|
|
||
| const validationResult = validateMagistratesPublicList(jsonData); | ||
| if (!validationResult.isValid) { | ||
| return res.status(400).render("errors/common", { | ||
| en, | ||
| cy, | ||
| errorTitle: t.errorTitle, | ||
| errorMessage: t.errorMessage | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Malformed JSON falls through to a 500.
JSON.parse sits in the outer try, so an unparsable temp file surfaces as a generic 500 rather than the 400 used for invalid payloads. Consider wrapping the parse to return a 400 for consistency with the validation path.
🛠️ Proposed fix
- const jsonData = JSON.parse(jsonContent);
+ let jsonData: unknown;
+ try {
+ jsonData = JSON.parse(jsonContent);
+ } catch {
+ return res.status(400).render("errors/common", {
+ en,
+ cy,
+ errorTitle: t.errorTitle,
+ errorMessage: t.errorMessage
+ });
+ }📝 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.
| const jsonData = JSON.parse(jsonContent); | |
| const validationResult = validateMagistratesPublicList(jsonData); | |
| if (!validationResult.isValid) { | |
| return res.status(400).render("errors/common", { | |
| en, | |
| cy, | |
| errorTitle: t.errorTitle, | |
| errorMessage: t.errorMessage | |
| }); | |
| } | |
| let jsonData: unknown; | |
| try { | |
| jsonData = JSON.parse(jsonContent); | |
| } catch { | |
| return res.status(400).render("errors/common", { | |
| en, | |
| cy, | |
| errorTitle: t.errorTitle, | |
| errorMessage: t.errorMessage | |
| }); | |
| } | |
| const validationResult = validateMagistratesPublicList(jsonData); | |
| if (!validationResult.isValid) { | |
| return res.status(400).render("errors/common", { | |
| en, | |
| cy, | |
| errorTitle: t.errorTitle, | |
| errorMessage: t.errorMessage | |
| }); | |
| } |
| @@ -0,0 +1,112 @@ | |||
| <!DOCTYPE html> | |||
| <html lang="en"> | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
lang attribute is hardcoded to en for Welsh PDFs.
The same template renders both EN and CY output, but <html lang="en"> is fixed. For Welsh PDFs this mislabels the document language, affecting assistive technology and language metadata. Pass the locale through and set it dynamically.
🌐 Suggested fix
-<html lang="en">
+<html lang="{{ locale or 'en' }}">You will also need to pass locale into the template model from pdf-generator.ts (env.render("pdf-template.njk", { ..., locale: options.locale })).
📝 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.
| <html lang="en"> | |
| <html lang="{{ locale or 'en' }}"> |
| function formatContentDate(date: Date, locale: string): string { | ||
| const localeCode = locale === "cy" ? "cy-GB" : "en-GB"; | ||
| return date.toLocaleDateString(localeCode, { | ||
| day: "2-digit", | ||
| month: "long", | ||
| year: "numeric" | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
for tz in UTC America/New_York Europe/London; do
TZ="$tz" node -e 'const date=new Date("2025-01-01"); console.log(process.env.TZ, date.toLocaleDateString("en-GB",{day:"2-digit",month:"long",year:"numeric"}));'
doneRepository: hmcts/cath-service
Length of output: 240
Pin formatContentDate to Europe/London. toLocaleDateString() uses the host timezone, so artefact.contentDate can render the previous or next day on non-London nodes. Use Intl.DateTimeFormat(..., { timeZone: "Europe/London" }) instead.
| function formatPublicationDateTime(isoDateTime: string, locale: string): string { | ||
| const dt = DateTime.fromISO(isoDateTime).setZone("Europe/London").setLocale(locale); | ||
|
|
||
| const dateStr = dt.toFormat("d MMMM yyyy"); | ||
|
|
||
| const hours = dt.hour; | ||
| const minutes = dt.minute; | ||
| const period = hours >= 12 ? "pm" : "am"; | ||
| const hour12 = hours % 12 || 12; | ||
|
|
||
| const minuteStr = minutes > 0 ? `:${minutes.toString().padStart(2, "0")}` : ""; | ||
| const timeStr = `${hour12}${minuteStr}${period}`; | ||
|
|
||
| return `${dateStr} at ${timeStr}`; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the hard-coded English connector from Welsh output.
Line 37 always emits " at ", so header.lastUpdated stays partially English for locale === "cy".
Suggested fix
function formatPublicationDateTime(isoDateTime: string, locale: string): string {
const dt = DateTime.fromISO(isoDateTime).setZone("Europe/London").setLocale(locale);
@@
- return `${dateStr} at ${timeStr}`;
+ const connector = locale === "cy" ? "am" : "at";
+ return `${dateStr} ${connector} ${timeStr}`;
}As per coding guidelines, "Implement Welsh language support on all user-facing text with separate en.ts and cy.ts content files".
📝 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.
| function formatPublicationDateTime(isoDateTime: string, locale: string): string { | |
| const dt = DateTime.fromISO(isoDateTime).setZone("Europe/London").setLocale(locale); | |
| const dateStr = dt.toFormat("d MMMM yyyy"); | |
| const hours = dt.hour; | |
| const minutes = dt.minute; | |
| const period = hours >= 12 ? "pm" : "am"; | |
| const hour12 = hours % 12 || 12; | |
| const minuteStr = minutes > 0 ? `:${minutes.toString().padStart(2, "0")}` : ""; | |
| const timeStr = `${hour12}${minuteStr}${period}`; | |
| return `${dateStr} at ${timeStr}`; | |
| function formatPublicationDateTime(isoDateTime: string, locale: string): string { | |
| const dt = DateTime.fromISO(isoDateTime).setZone("Europe/London").setLocale(locale); | |
| const dateStr = dt.toFormat("d MMMM yyyy"); | |
| const hours = dt.hour; | |
| const minutes = dt.minute; | |
| const period = hours >= 12 ? "pm" : "am"; | |
| const hour12 = hours % 12 || 12; | |
| const minuteStr = minutes > 0 ? `:${minutes.toString().padStart(2, "0")}` : ""; | |
| const timeStr = `${hour12}${minuteStr}${period}`; | |
| const connector = locale === "cy" ? "am" : "at"; | |
| return `${dateStr} ${connector} ${timeStr}`; | |
| } |
Source: Coding guidelines
| function formatContentDate(date: Date, locale: string): string { | ||
| const localeCode = locale === "cy" ? "cy-GB" : "en-GB"; | ||
| return date.toLocaleDateString(localeCode, { | ||
| day: "2-digit", | ||
| month: "long", | ||
| year: "numeric" | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
for tz in UTC America/New_York Europe/London; do
TZ="$tz" node -e 'const date=new Date("2025-01-01"); console.log(process.env.TZ, date.toLocaleDateString("en-GB",{day:"2-digit",month:"long",year:"numeric"}));'
doneRepository: hmcts/cath-service
Length of output: 240
Pin content-date formatting to Europe/London. libs/list-types/magistrates-standard-list/src/rendering/renderer.ts:15-21
formatContentDate() formats a Date in the host process timezone, so the displayed day can shift on non-UTC/London nodes.
Suggested fix
function formatContentDate(date: Date, locale: string): string {
const localeCode = locale === "cy" ? "cy-GB" : "en-GB";
- return date.toLocaleDateString(localeCode, {
+ return new Intl.DateTimeFormat(localeCode, {
day: "2-digit",
month: "long",
- year: "numeric"
- });
+ year: "numeric",
+ timeZone: "Europe/London"
+ }).format(date);
}📝 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.
| function formatContentDate(date: Date, locale: string): string { | |
| const localeCode = locale === "cy" ? "cy-GB" : "en-GB"; | |
| return date.toLocaleDateString(localeCode, { | |
| day: "2-digit", | |
| month: "long", | |
| year: "numeric" | |
| }); | |
| function formatContentDate(date: Date, locale: string): string { | |
| const localeCode = locale === "cy" ? "cy-GB" : "en-GB"; | |
| return new Intl.DateTimeFormat(localeCode, { | |
| day: "2-digit", | |
| month: "long", | |
| year: "numeric", | |
| timeZone: "Europe/London" | |
| }).format(date); | |
| } |
| function formatPublicationDateTime(isoDateTime: string, locale: string): string { | ||
| const dt = DateTime.fromISO(isoDateTime).setZone("Europe/London").setLocale(locale); | ||
|
|
||
| const dateStr = dt.toFormat("d MMMM yyyy"); | ||
|
|
||
| const hours = dt.hour; | ||
| const minutes = dt.minute; | ||
| const period = hours >= 12 ? "pm" : "am"; | ||
| const hour12 = hours % 12 || 12; | ||
|
|
||
| const minuteStr = minutes > 0 ? `:${minutes.toString().padStart(2, "0")}` : ""; | ||
| const timeStr = `${hour12}${minuteStr}${period}`; | ||
|
|
||
| return `${dateStr} at ${timeStr}`; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the hard-coded English connector from Welsh output.
Line 37 always emits " at ", so header.lastUpdated stays partially English for locale === "cy".
Suggested fix
function formatPublicationDateTime(isoDateTime: string, locale: string): string {
const dt = DateTime.fromISO(isoDateTime).setZone("Europe/London").setLocale(locale);
@@
- return `${dateStr} at ${timeStr}`;
+ const connector = locale === "cy" ? "am" : "at";
+ return `${dateStr} ${connector} ${timeStr}`;
}As per coding guidelines, "Implement Welsh language support on all user-facing text with separate en.ts and cy.ts content files".
📝 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.
| function formatPublicationDateTime(isoDateTime: string, locale: string): string { | |
| const dt = DateTime.fromISO(isoDateTime).setZone("Europe/London").setLocale(locale); | |
| const dateStr = dt.toFormat("d MMMM yyyy"); | |
| const hours = dt.hour; | |
| const minutes = dt.minute; | |
| const period = hours >= 12 ? "pm" : "am"; | |
| const hour12 = hours % 12 || 12; | |
| const minuteStr = minutes > 0 ? `:${minutes.toString().padStart(2, "0")}` : ""; | |
| const timeStr = `${hour12}${minuteStr}${period}`; | |
| return `${dateStr} at ${timeStr}`; | |
| function formatPublicationDateTime(isoDateTime: string, locale: string): string { | |
| const dt = DateTime.fromISO(isoDateTime).setZone("Europe/London").setLocale(locale); | |
| const dateStr = dt.toFormat("d MMMM yyyy"); | |
| const hours = dt.hour; | |
| const minutes = dt.minute; | |
| const period = hours >= 12 ? "pm" : "am"; | |
| const hour12 = hours % 12 || 12; | |
| const minuteStr = minutes > 0 ? `:${minutes.toString().padStart(2, "0")}` : ""; | |
| const timeStr = `${hour12}${minuteStr}${period}`; | |
| const connector = locale === "cy" ? "am" : "at"; | |
| return `${dateStr} ${connector} ${timeStr}`; | |
| } |
Source: Coding guidelines
|
1 participant
Summary
@hmcts/magistrates-standard-listlib module with rendering, PDF generation, JSON schema validation, email summary extraction, and EN/CY translations@hmcts/magistrates-public-listlib module with the same complete feature setapps/web(app.ts, package.json, tsconfig.json)Test plan
yarn build— 37/37 packages passyarn lint— all packages cleanyarn test— 37/37 packages pass, 1709 tests pass/magistrates-standard-list?artefactId=<id>to verify HTML rendering/magistrates-public-list?artefactId=<id>to verify HTML rendering?lng=cyto verify Welsh translations render correctly🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests