fix(#591): align COP daily cause list with reference implementation - #875
Conversation
Add the Court of Protection Daily Cause List (COP_DAILY_CAUSE_LIST) as a new list-type module at libs/list-types/cop-daily-cause-list, mirroring the civil-and-family-daily-cause-list pattern: schema, validator, thin renderer, EN/CY locales, email summary, and PDF generator. Add the web controller at apps/web/src/pages/(list-types)/cop-daily-cause-list using createListTypeHandler + createCauseListRender with access checks, an 8-column hearings table, accordion, Open Justice section, search and data source. Register the module (tsconfig alias, app.ts modulePaths, web/publication deps, PDF_GENERATOR_REGISTRY, list-type-data) and add unit, template and E2E tests. Routing/guards use the stable listTypeName, never a numeric id. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rework the Court of Protection daily cause list to match the reference staging layout for both HTML and PDF: - Add COP-specific header (region, regional lead judge, sitting-at court) - Render the 7-column hearings table with sequence indicator and a full-width reporting restriction row - Use Title Case table headers and drop the colon after "Before" - Add the missing back-to-top link on the style-guide HTML page - Name the court (not the region venue) in the open justice contact line - Wire up the previously-orphaned COP email summary builder Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds a Court of Protection daily cause list with JSON validation, Welsh and English rendering, web and PDF views, email summaries, publication and notification registration, application wiring, metadata, and supporting tests and documentation. ChangesCOP daily cause list
Sequence Diagram(s)sequenceDiagram
participant Publisher
participant WebRoute
participant COPRenderer
participant HTMLTemplate
participant PDFService
participant EmailService
Publisher->>WebRoute: request COP cause list
WebRoute->>COPRenderer: validate and transform list data
COPRenderer->>HTMLTemplate: render localised HTML
HTMLTemplate-->>WebRoute: return cause list page
Publisher->>PDFService: generate COP PDF
PDFService->>COPRenderer: render list data
PDFService-->>Publisher: store generated PDF
Publisher->>EmailService: build COP notification
EmailService-->>Publisher: return case summary content
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 |
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🎭 Playwright E2E Test Results82 tests 52 ✅ 6m 17s ⏱️ Results for commit a77c447. ♻️ 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: 8
🧹 Nitpick comments (6)
libs/list-types/cop-daily-cause-list/src/schemas/cop-daily-cause-list.json (2)
232-239: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
isPresidingdefault/pattern look copy-pasted from a string field.
isPresidingistype: "boolean"but carries"default": ""and a regex"pattern", both of which only make sense for string properties. Looks like leftover boilerplate from another field.🧹 Suggested cleanup
"isPresiding": { "description": "A flag to indicate whether a judiciary is presiding", "title": "Is Presiding", "type": "boolean", - "examples": [true], - "default": "", - "pattern": "^(?!.*<[^>]+>).*$" + "examples": [true], + "default": false }
323-329: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
examplesshould be an array, matching the pattern used elsewhere.Every other field in this schema wraps its example value in an array (e.g.
"examples": ["45684548"]), butreportingRestrictionsuses a bare string. This is inconsistent with the JSON Schemaexampleskeyword contract (which expects an array).🧹 Suggested fix
"reportingRestrictions": { "title": "Reporting Restrictions Detail", "description": "The detail / wording of any reporting restrictions attached to the case", "type": "string", - "examples": "This is a reporting restrictions detail", + "examples": ["This is a reporting restrictions detail"], "pattern": "^(?!(.|\\r|\\n)*<[^>]+>)(.|\\r|\\n)*$" }libs/list-types/cop-daily-cause-list/src/config.ts (1)
7-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the top-level config exports to SCREAMING_SNAKE_CASE.
moduleRoot,assets, andschemaPathare top-level constants exported fromconfig.ts; as a local package rename, update the same names in importing files such asapps/web/src/app.tsandlibs/list-types/cop-daily-cause-list/src/config.test.ts;__dirname/__filenamecan remain as Node’s conventional idiom names.Source: Coding guidelines
libs/list-types/cop-daily-cause-list/src/config.test.ts (1)
19-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse camelCase for the locally constructed path variables.
Rename
__filenameand__dirnameto camelCase names in both tests; these are local ESM variables, not Node globals. As per coding guidelines, “TypeScript variables must use camelCase.”Also applies to: 26-28
Source: Coding guidelines
libs/list-types/cop-daily-cause-list/src/rendering/renderer.ts (1)
15-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid mutating the caller’s publication JSON.
Line 24 modifies
jsonDatain place. Return an updated copy before passing it to the shared renderer so rendering remains side-effect free.As per coding guidelines, “prefer immutable data with
const, minimise side effects”.Source: Coding guidelines
apps/web/package.json (1)
35-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the COP workspace dependency version.
workspace:*permits an unbounded internal version range. Use the module’s explicit workspace version consistently.
apps/web/package.json#L35-L35: replaceworkspace:*with the pinned COP package version.libs/publication/package.json#L30-L30: use the same pinned COP package version.As per coding guidelines, packages must “use pinned dependency versions except for peer dependencies”.
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b9c1e20b-5a3c-42dd-8294-3f073957d0d4
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (35)
apps/web/package.jsonapps/web/src/app.tsapps/web/src/pages/(list-types)/cop-daily-cause-list/cop-daily-cause-list.njkapps/web/src/pages/(list-types)/cop-daily-cause-list/cop-daily-cause-list.njk.test.tsapps/web/src/pages/(list-types)/cop-daily-cause-list/index.test.tsapps/web/src/pages/(list-types)/cop-daily-cause-list/index.tsdocs/tickets/591/plan.mddocs/tickets/591/tasks.mddocs/tickets/591/ticket.mde2e-tests/tests/cop-daily-cause-list.spec.tslibs/list-types/common/src/list-type-data.tslibs/list-types/cop-daily-cause-list/package.jsonlibs/list-types/cop-daily-cause-list/src/config.test.tslibs/list-types/cop-daily-cause-list/src/config.tslibs/list-types/cop-daily-cause-list/src/email-summary/summary-builder.test.tslibs/list-types/cop-daily-cause-list/src/email-summary/summary-builder.tslibs/list-types/cop-daily-cause-list/src/index.test.tslibs/list-types/cop-daily-cause-list/src/index.tslibs/list-types/cop-daily-cause-list/src/locales/cy.tslibs/list-types/cop-daily-cause-list/src/locales/en.tslibs/list-types/cop-daily-cause-list/src/models/types.tslibs/list-types/cop-daily-cause-list/src/pdf/pdf-generator.test.tslibs/list-types/cop-daily-cause-list/src/pdf/pdf-generator.tslibs/list-types/cop-daily-cause-list/src/pdf/pdf-template.njklibs/list-types/cop-daily-cause-list/src/rendering/renderer.test.tslibs/list-types/cop-daily-cause-list/src/rendering/renderer.tslibs/list-types/cop-daily-cause-list/src/schemas/cop-daily-cause-list.jsonlibs/list-types/cop-daily-cause-list/src/validation/json-validator.test.tslibs/list-types/cop-daily-cause-list/src/validation/json-validator.tslibs/list-types/cop-daily-cause-list/tsconfig.jsonlibs/notifications/package.jsonlibs/notifications/src/notification/notification-service.tslibs/publication/package.jsonlibs/publication/src/processing/service.tstsconfig.json
|
|
||
| ### Architecture decisions | ||
|
|
||
| 1. **Reuse `renderCauseListData`** from `@hmcts/daily-cause-list-common` via a one-line re-export in `rendering/renderer.ts`. The COP schema matches the common `CauseListData` shape (judiciary at session level with `johKnownAs`/`isPresiding`, cases with `caseNumber`/`caseName`/`caseType`/`reportingRestrictions`), so no bespoke transform is required. **Verify at implementation time** that no COP-specific fields are dropped; if divergence exists, add a thin wrapper. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Make locationDetails part of the renderer contract.
The plan says the common renderer can be reused unchanged, but the stated objective requires the COP header to render region, Regional Lead Judge, and sitting court from locationDetails. Require a COP-specific transform/wrapper and explicit tests for these fields rather than leaving this as an implementation-time check.
Also applies to: 174-174
|
|
||
| ### Template `cop-daily-cause-list.njk` | ||
|
|
||
| Copy civil-and-family's `.njk` as the base and adapt: page heading (`{{ t.title }} {{ header.locationName }}`), FaCT link, venue address, `List for` / `Last updated`, Important information `<details>` accordion carrying the **full Open Justice statement** (section 7 of the spec), conditional Special Category Data warning, search input (`#case-search-input`), `govuk-accordion` per court room showing `Court Room, Before: <judiciary>`, then the **8-column hearings table** (Start time, Case ref, Case name, Case type, Hearing type, Time estimate, Mode of hearing, Reporting restriction), and `Data source` footer. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Define one authoritative hearings-table contract.
The documentation disagrees on seven versus eight columns and omits or contradicts the final sequence-indicator, restriction-row, Title Case, and “Before” requirements.
docs/tickets/591/plan.md#L124-L124: update the technical approach and linked acceptance tests.docs/tickets/591/tasks.md#L37-L37: update the implementation checklist.docs/tickets/591/ticket.md#L43-L43: reconcile the acceptance criteria with the final table structure.
🧰 Tools
🪛 LanguageTool
[grammar] ~124-~124: The noun “courtroom” is spelled as one word.
Context: ...e-search-input), govuk-accordionper court room showingCourt Room, Before: <judiciary...
(ROOM_COMPOUNDS)
📍 Affects 3 files
docs/tickets/591/plan.md#L124-L124(this comment)docs/tickets/591/tasks.md#L37-L37docs/tickets/591/ticket.md#L43-L43
| ## CLARIFICATIONS NEEDED | ||
|
|
||
| 1. **Column set / labels.** The ticket lists 7 data fields in the AC ("Start Time, Case Ref, Case Details, Case Type, Hearing Type, Time Estimate and Hearing Channel") but 8 columns in the Hearings-table section (adds "Reporting restriction", uses "Case name"). `hearingChannel` label is "Mode of hearing" (issue content) vs "Hearing Channel" (pip-frontend). **Plan assumes the 8-column set and "Mode of hearing".** Confirm definitive set + label. | ||
| 2. **Open Justice contact details.** "Belfast Laganside Court", `a@b.com`, `+44 1234 1234 1234` look like placeholders — and Belfast (NI) contradicts the England & Wales jurisdiction in the metadata. Are these static, or dynamically sourced from `venue`/`venueContact` (as civil-and-family does via `openJusticeContact(venueName, email, phone)`)? **Plan assumes dynamic population from the artefact venue, falling back to the static statement.** |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Specify approved Open Justice contact behaviour.
Both documents preserve placeholder contact details while the objective requires the actual court name. Define the authoritative source and missing-data handling, and prohibit placeholder values in rendered HTML, PDF, and email output.
docs/tickets/591/plan.md#L205-L205: remove the placeholder fallback assumption.docs/tickets/591/ticket.md#L48-L48: replace placeholder acceptance text with approved contact requirements.
📍 Affects 2 files
docs/tickets/591/plan.md#L205-L205(this comment)docs/tickets/591/ticket.md#L48-L48
| - `src/pages/index.ts` — page controller (GET handler) | ||
| - `src/pages/index.test.ts` — unit tests for controller | ||
| - `src/pages/en.ts` — English translations | ||
| - `src/pages/cy.ts` — Welsh translations | ||
| - `src/pages/cop-daily-cause-list.njk` — Nunjucks HTML template | ||
| - `src/pdf/pdf-generator.ts` — PDF generation | ||
| - `src/pdf/pdf-template.njk` — PDF Nunjucks template | ||
| - `src/pdf/pdf-generator.test.ts` — unit tests for PDF generator | ||
| - `src/email-summary/summary-builder.ts` — email summary builder | ||
| - `src/email-summary/summary-builder.test.ts` — unit tests for email summary | ||
| - `src/index.ts` — exports renderer, PDF generator, validator and email summary builder | ||
| - `src/config.ts` — exports `pageRoutes` and `moduleRoot` | ||
| - `package.json` — with build scripts including `build:nunjucks` and `build:pdf-templates` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Bring the ticket in line with the implemented architecture.
The plan and current stack place the controller/template under the auto-discovered apps/web/src/pages/ path and require no pageRoutes or Vite route-prefix change. This ticket still specifies src/pages inside the library, pageRoutes, and a Vite route prefix; remove the stale instructions to prevent future work following the wrong integration path.
Also applies to: 219-220
| summaries.push([ | ||
| { label: "Case reference", value: caseItem.caseNumber || "" }, | ||
| { label: "Case details", value: caseItem.caseName || "" }, | ||
| { label: "Case type", value: caseItem.caseType || "" }, | ||
| { label: "Hearing type", value: hearing.hearingType || "" } | ||
| ]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Localise the email-summary field labels.
These user-facing labels are hard-coded in English, so Welsh COP summaries cannot use Welsh content. Source the labels from the COP English/Welsh locale content and pass the selected locale through the summary-building path. As per coding guidelines, “List-type-specific display strings must come from both English and Welsh locale files, not hardcoded controller strings.”
Source: Coding guidelines
| @@ -0,0 +1 @@ | |||
| export type { CauseListCase, CauseListData, CourtHouse, CourtRoom, Hearing, Party, RenderOptions, Session, Sitting } from "@hmcts/daily-cause-list-common"; | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Replace the generic types.ts filename.
Rename this module to a purpose-specific name (for example, cause-list-types.ts) and update its imports. As per coding guidelines, “Do not create generic types.ts or utils.ts files; colocate types and use specific-purpose filenames.”
Source: Coding guidelines
| import { type ValidationResult, validateJson } from "@hmcts/publication"; | ||
| import schema from "../schemas/cop-daily-cause-list.json" with { type: "json" }; | ||
|
|
||
| /** | ||
| * Validates Court of Protection Daily Cause List JSON data | ||
| * @param jsonData - The JSON data to validate | ||
| * @returns ValidationResult | ||
| */ | ||
| export function validateCopDailyCauseList(jsonData: unknown): ValidationResult { | ||
| return validateJson(jsonData, schema, "1.0"); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether `@hmcts/publication` exports createJsonValidator and whether
# other list-type packages already use it.
rg -n "createJsonValidator" libs/publication/src -A3
rg -n "createJsonValidator" libs/list-types -g '*.ts'Repository: hmcts/cath-service
Length of output: 7852
🏁 Script executed:
#!/bin/bash
# Inspect the target wrapper and package declaration/dependency declarations to determine
# whether `@hmcts/publication.createJsonValidator` is available and whether the existing common helper can be used.
sed -n '1,40p' libs/list-types/cop-daily-cause-list/src/validation/json-validator.ts
printf '\n--- package references ---\n'
python3 - <<'PY'
import json, pathlib
for p in pathlib.Path('.').rglob('package.json'):
if any(s in p.parts for s in ('node_modules','.git')):
continue
try:
data=json.loads(p.read_text())
except Exception:
continue
name=data.get('name')
if name in ('`@hmcts/list-types-common`','`@hmcts/publication`') or 'cop-daily-cause-list' in str(p) or '`@hmcts/list-types-common`' in str(p):
print(p)
for k in ('name','version','dependencies','devDependencies','exports','main'):
if k in data:
print(f' {k}: {data[k]}')
PYRepository: hmcts/cath-service
Length of output: 4661
Use createJsonValidator for the list-type validator wrapper.
This wrapper is required to use the common JSON validator factory, not validateJson from @hmcts/publication directly.
Source: Path instructions
| "@hmcts/cic-weekly-hearing-list": "workspace:*", | ||
| "@hmcts/civil-and-family-daily-cause-list": "workspace:*", | ||
| "@hmcts/civil-daily-cause-list": "workspace:*", | ||
| "@hmcts/cop-daily-cause-list": "workspace:*", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Pin the COP package dependency version.
Replace workspace:* with the explicit compatible release version required by this package’s dependency policy. As per coding guidelines, “Packages must use the @hmcts/* scope, set "type": "module", and use pinned dependency versions except for peer dependencies.”
Source: Coding guidelines
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Resolved two registry conflicts, both "keep both sides" — master and this branch each appended entries at the same insertion point with no semantic overlap: - libs/list-types/common/src/list-type-data.ts: kept COP_DAILY_CAUSE_LIST (this branch) alongside PCOL_DAILY_CAUSE_LIST, ET_DAILY_LIST and ET_FORTNIGHTLY_PRESS_LIST (master). The conflict straddled a shared object brace, so the entries were reconstructed rather than concatenated. - tsconfig.json: kept the @hmcts/cop-daily-cause-list path alias alongside master's @hmcts/et-daily-list and @hmcts/et-fortnightly-list aliases. Verified: no duplicate list type names (71 unique, name is @unique); tsconfig parses and all four aliases resolve; both sides' registrations in apps/web/src/app.ts and libs/publication/src/processing/service.ts survived the auto-merge; generate-seed-sql.ts emits SQL for all four list types; 2287 tests pass across list-types, publication and the list-type pages. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f39f8c48-43ef-4463-b602-d4fd6a345464
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
apps/web/package.jsonapps/web/src/app.tslibs/list-types/common/src/list-type-data.tslibs/notifications/package.jsonlibs/notifications/src/notification/notification-service.tslibs/publication/package.jsonlibs/publication/src/processing/service.tstsconfig.json
| "@hmcts/et-daily-list": "workspace:*", | ||
| "@hmcts/et-fortnightly-list": "workspace:*", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Pin the new workspace dependencies consistently.
Both manifests use workspace:*, leaving the newly added dependency versions unpinned under the repository rule.
apps/web/package.json#L38-L39: pin the ET daily and fortnightly list dependencies.libs/publication/package.json#L35-L36: pin the corresponding publication dependencies.
As per coding guidelines, **/package.json: pin dependency versions.
📍 Affects 2 files
apps/web/package.json#L38-L39(this comment)libs/publication/package.json#L35-L36
Source: Coding guidelines
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. |
2 participants
What this does
Reworks the Court of Protection (COP) daily cause list so the local rendered output matches the reference staging implementation, across both the style-guide HTML page and the generated PDF.
Changes
locationDetails.header.locationName) instead of the region venue.Testing
@hmcts/cop-daily-cause-list: 58 tests pass@hmcts/web(COP page/template): 33 tests pass@hmcts/notifications: 74 tests pass🤖 Generated with Claude Code
Summary by CodeRabbit