VIBE-317 Add RCJ list types - #204
Conversation
Add comprehensive technical planning documents for implementing 13 Royal Courts of Justice and Administrative Court hearing list types. Includes: - Specification document with all requirements and Welsh translations - Technical implementation plan with modular architecture - Module breakdown for 5 distinct components - List type registration strategy - Excel schema and converter configurations - PDF generation approach - Testing strategy Covers: - 8 standard format RCJ lists (7 columns) - 1 Family list with accordion layout (9 columns) - 1 Court of Appeal Civil with two-tab support - 4 Administrative Court lists - RCJ landing page Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughRegisters four new daily-hearing list modules (RCJ standard, London Administrative Court, Court of Appeal — Civil, Administrative Court); adds converters, multi-sheet conversion, JSON schemas, renderers, templates, route handlers and app wiring; introduces shared utilities (date-formatting, validators, table-search), adds lastReceivedDate to artefacts, and expands unit and E2E tests and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as System Admin
participant Upload as Manual Upload Service
participant Converter as Excel Converter
participant Validator as JSON Schema Validator
participant Publication as Publication Repository
participant App as Web App (route/pages)
participant Renderer as Renderer
participant User as End User
Admin->>Upload: POST (Excel/JSON + metadata)
Upload->>Converter: convert uploaded file -> JSON (multi-sheet if needed)
Converter->>Validator: validate JSON against schema
alt valid
Validator->>Publication: store artefact + lastReceivedDate
User->>App: GET page?artefactId
App->>Publication: getArtefactById(artefactId)
Publication-->>App: artefact (incl. lastReceivedDate)
App->>Renderer: render(data, locale, lastReceivedDate)
Renderer-->>App: rendered data
App->>User: HTML page (EN/CY) with table-search
else invalid
Validator->>Upload: return validation errors
Upload->>Admin: display errors
end
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 Results232 tests 232 ✅ 21m 2s ⏱️ Results for commit d027041. ♻️ 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: 7
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)
libs/list-types/civil-and-family-daily-cause-list/src/pages/_handler.ts (1)
23-23: Potential path traversal risk with unsanitisedartefactId.The
artefactIdis user-supplied and used directly inpath.jointo construct a file path. Although the database lookup provides some protection, consider validating the format (e.g., UUID pattern) or usingpath.basenameto prevent directory traversal.🔒 Suggested mitigation
const artefactId = req.query.artefactId as string; if (!artefactId) { return res.status(400).render("errors/common", { en, cy, errorTitle: t.errorTitle, errorMessage: t.errorMessage }); } + // Validate artefactId format to prevent path traversal + const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + if (!UUID_REGEX.test(artefactId)) { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: t.errorTitle, + errorMessage: t.errorMessage + }); + }Also applies to: 48-48
🟡 Minor comments (14)
scripts/convert-manual-upload.ts-5-6 (1)
5-6: ValidatelistTypeIdfor non-numeric input.If a non-numeric string is passed as the third argument,
Number.parseIntreturnsNaN. The script will still fail gracefully viahasConverterForListType, but the error message would be misleading.🔧 Proposed fix
const artefactId = process.argv[2] || "a915a685-7b8f-4c8c-bf7e-87ef98743014"; const listTypeId = Number.parseInt(process.argv[3] || "10", 10); + +if (Number.isNaN(listTypeId)) { + console.error("Invalid list type ID: must be a number"); + process.exit(1); +}libs/list-types/london-administrative-court-daily-cause-list/README.md-20-20 (1)
20-20: Minor typo: "Court room" should be "Courtroom".📝 Suggested fix
-- **Venue** (required): Court room or location +- **Venue** (required): Courtroom or locationlibs/list-types/london-administrative-court-daily-cause-list/src/schemas/london-admin-court.json-22-25 (1)
22-25: Time pattern is case-sensitive.The schema pattern
^\\d{1,2}([:.]\\d{2})?[ap]m\\s*$only matches lowercaseam/pm. The converter config uses/iflag for case-insensitivity. Consider updating to^\\d{1,2}([:.]\\d{2})?[aApP][mM]\\s*$for consistency.Suggested fix
"time": { "type": "string", - "pattern": "^\\d{1,2}([:.]\\d{2})?[ap]m\\s*$" + "pattern": "^\\d{1,2}([:.]\\d{2})?[aApP][mM]\\s*$" },Also applies to: 59-62
libs/list-types/rcj-standard-daily-cause-list/src/models/types.ts-1-9 (1)
1-9: MarkadditionalInformationas optional in the interface.The JSON schema does not include
additionalInformationin its required fields, yet the TypeScript interface marks it as required. Update the field to:- additionalInformation: string; + additionalInformation?: string;docs/tickets/VIBE-317/implementation-progress.md-51-67 (1)
51-67: File count mismatch.Header states "13 files" but the list contains 15 numbered items (1-15). Update the count to match.
Proposed fix
-**Files Created (13 files)**: +**Files Created (15 files)**:libs/list-types/london-administrative-court-daily-cause-list/src/pages/en.ts-19-27 (1)
19-27: Inconsistent capitalisation in tableHeaders.The
tableHeadersvalues use Title Case (e.g.,"Case Number","Case Details"), whereas the equivalent inlibs/list-types/rcj-standard-daily-cause-list/src/pages/en.tsuses sentence case (e.g.,"Case number","Case details"). Consider aligning for consistency across list types.Suggested fix for consistency
tableHeaders: { venue: "Venue", judge: "Judge", time: "Time", - caseNumber: "Case Number", - caseDetails: "Case Details", - hearingType: "Hearing Type", - additionalInformation: "Additional Information" + caseNumber: "Case number", + caseDetails: "Case details", + hearingType: "Hearing type", + additionalInformation: "Additional information" },libs/list-types/rcj-standard-daily-cause-list/README.md-20-20 (1)
20-20: Minor typo: "Court room" should be "Courtroom".The noun is conventionally spelled as one word.
📝 Suggested fix
-- **Venue** (required): Court room or location +- **Venue** (required): Courtroom or locationlibs/list-types/rcj-standard-daily-cause-list/src/schemas/standard-daily-cause-list.json-22-27 (1)
22-27: Time pattern accepts invalid 12-hour format values.The current pattern
^\d{1,2}([:.]\\d{2})?[ap]m\s*$matches invalid times such as13:00am,00:00am, and99:00pm. Additionally, the README states "Hearing time in HH:MM format" whilst the schema accepts 12-hour format with am/pm suffixes (e.g.10:00am,2:30pm), creating documentation inconsistency.Restrict the pattern to valid 12-hour hours (1–12):
Suggested pattern
- "pattern": "^\\d{1,2}([:.]\\d{2})?[ap]m\\s*$", + "pattern": "^(1[0-2]|0?[1-9])([:.]\\d{2})?\\s*[ap]m$",libs/list-types/rcj-court-of-appeal-civil/src/pages/_handler.ts-23-23 (1)
23-23: Type assertion may be unsafe for repeated query parameters.If the query string contains
?artefactId=a&artefactId=b, Express parses this as an array. Theas stringassertion would produce unexpected behaviour.Suggested fix
- const artefactId = req.query.artefactId as string; + const artefactId = Array.isArray(req.query.artefactId) + ? req.query.artefactId[0] + : (req.query.artefactId as string);libs/list-types/rcj-court-of-appeal-civil/src/pages/_handler.ts-73-73 (1)
73-73: Malformed JSON returns 500 instead of 400.
JSON.parsethrowing on invalid JSON is caught by the outer catch block, returning a 500 error. This should return 400 since it's a data format issue, not a server error.Suggested fix
- const jsonData: CourtOfAppealCivilData = JSON.parse(jsonContent); + let jsonData: CourtOfAppealCivilData; + try { + jsonData = JSON.parse(jsonContent); + } catch { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Invalid Data", + errorMessage: "The list data format is invalid" + }); + }libs/list-types/rcj-court-of-appeal-civil/src/schemas/civil-appeal.json-22-25 (1)
22-25: Time pattern inconsistency with README documentation.The README states "Time must be HH:MM format" but the schema pattern
^\\d{1,2}([:.]\\d{2})?[ap]m\\s*$expects 12-hour format with am/pm suffix (e.g., "10:30am"). The minutes portion is also optional. Ensure the documentation aligns with the actual validation rules.Also applies to: 63-66
libs/list-types/london-administrative-court-daily-cause-list/src/pages/_handler.ts-73-74 (1)
73-74: UnhandledJSON.parseexception.Same issue as the administrative court handler - malformed JSON will result in a generic 500 error rather than an appropriate 400 response.
Suggested fix
- const jsonData: LondonAdminCourtData = JSON.parse(jsonContent); + let jsonData: LondonAdminCourtData; + try { + jsonData = JSON.parse(jsonContent); + } catch { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Invalid Data", + errorMessage: "The list data could not be parsed" + }); + }libs/list-types/administrative-court-daily-cause-list/src/pages/_handler.ts-83-84 (1)
83-84: UnhandledJSON.parseexception.If
jsonContentcontains malformed JSON,JSON.parsewill throw and the error will be caught by the outer catch block, returning a generic 500 error. For better user feedback, wrap this in a try-catch to return a 400 with a meaningful message.Suggested fix
- const jsonData: StandardHearingList = JSON.parse(jsonContent); + let jsonData: StandardHearingList; + try { + jsonData = JSON.parse(jsonContent); + } catch { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Invalid Data", + errorMessage: "The list data could not be parsed" + }); + }libs/list-types/rcj-standard-daily-cause-list/src/pages/_handler.ts-78-78 (1)
78-78: Avoid logging full file paths in production.Logging the full file path could expose internal directory structure. Log just the artefactId instead.
🔒 Suggested fix
- console.error(`Error reading JSON file at ${jsonFilePath}:`, error); + console.error(`Error reading JSON file for artefact ${artefactId}:`, error);
🧹 Nitpick comments (46)
libs/list-types/civil-and-family-daily-cause-list/src/pages/_handler.ts (2)
54-54: Avoid logging file paths that include user-controlled input.Per coding guidelines, sensitive data should not appear in logs. The
jsonFilePathcontains theartefactIdwhich could leak information. Consider logging a generic message or just the error type.♻️ Suggested change
- console.error(`Error reading JSON file at ${jsonFilePath}:`, error); + console.error("Error reading JSON file for artefact");
67-67: Validation errors may contain sensitive schema details.Logging the full
validationResult.errorscould expose internal structure. Consider logging only the count or a sanitised summary.♻️ Suggested change
- console.error("Validation errors:", validationResult.errors); + console.error(`Validation failed with ${validationResult.errors?.length ?? 0} error(s)`);scripts/convert-manual-upload.ts (1)
2-2: Document the purpose of the side-effect import.This import registers the converter for the list type but the intent isn't immediately clear. A brief comment would aid future maintainers.
📝 Suggested comment
import { readFile, writeFile } from "node:fs/promises"; +// Side-effect import: registers the RCJ standard converter with the converter registry import "@hmcts/rcj-standard-daily-cause-list";libs/admin-pages/src/pages/non-strategic-upload-summary/index.ts (1)
148-148: Consider structured error logging.Logging the raw error object may inadvertently expose sensitive details (file paths, partial content). Consider logging only the error message or using a structured logger that sanitises output.
libs/list-types/administrative-court-daily-cause-list/src/models/types.ts (1)
1-11: Consider extracting shared type to a common package.The
StandardHearinginterface is duplicated identically across four list-type modules (administrative-court, london-administrative-court, rcj-court-of-appeal-civil, rcj-standard). Extracting to a shared types package (e.g.,@hmcts/list-types-common) would reduce duplication and ensure consistency.Current implementation works correctly; this is a maintainability consideration for future changes.
libs/list-types/london-administrative-court-daily-cause-list/src/models/types.ts (1)
1-9: ExtractStandardHearingto shared module.
StandardHearingis defined identically across four separate modules:administrative-court-daily-cause-list,london-administrative-court-daily-cause-list,rcj-court-of-appeal-civil, andrcj-standard-daily-cause-list. Extract it to@hmcts/list-types-commonand re-export from each module to reduce duplication.libs/list-types/rcj-standard-daily-cause-list/src/rendering/renderer.test.ts (1)
1-92: Good test coverage for core rendering functionality.Tests cover locale handling, data preservation, and empty field handling. Structure follows conventions with
.jsextensions in imports and co-located test file naming.Consider adding edge case tests for undefined
additionalInformation(not just empty string) to verify the fallback logic in the renderer.libs/list-types/administrative-court-daily-cause-list/src/validation/json-validator.ts (2)
13-14: Consider using proper Ajv typing instead ofanycast.Per coding guidelines,
anyshould be avoided without justification. Ajv's default export typing can be handled with:Suggested improvement
-const ajv = new (Ajv as any)({ allErrors: true }); +const ajv = new Ajv({ allErrors: true });If there are type compatibility issues with the Ajv import, add a comment explaining why the cast is necessary.
31-34: Use Ajv'sErrorObjecttype instead ofany.Suggested improvement
+import Ajv, { ErrorObject } from "ajv"; ... - const errors = validate.errors?.map((error: any) => { + const errors = validate.errors?.map((error: ErrorObject) => { const field = error.instancePath.replace(/\//g, ".").substring(1) || "root"; return `${field}: ${error.message}`; }) || ["Unknown validation error"];libs/list-types/london-administrative-court-daily-cause-list/src/validation/json-validator.ts (1)
16-19: DuplicateValidationResultinterface across modules.This interface is identical in
administrative-court-daily-cause-list,rcj-court-of-appeal-civil, andrcj-standard-daily-cause-listvalidators. Consider extracting to@hmcts/list-types-commonto reduce duplication.libs/list-types/london-administrative-court-daily-cause-list/src/schemas/london-admin-court.json (1)
10-43: Duplicated item schema definition.The item schema for
mainHearingsandplanningCourtis identical. Consider using$defsand$refto reduce duplication.Example refactor using $defs
{ "$schema": "http://json-schema.org/draft-07/schema#", "title": "London Administrative Court Daily Cause List", "$defs": { "hearingItem": { "type": "object", "required": ["venue", "judge", "time", "caseNumber", "caseDetails", "hearingType"], "properties": { ... } } }, "properties": { "mainHearings": { "type": "array", "items": { "$ref": "#/$defs/hearingItem" } }, "planningCourt": { "type": "array", "items": { "$ref": "#/$defs/hearingItem" } } } }Also applies to: 47-80
libs/list-types/london-administrative-court-daily-cause-list/src/validation/json-validator.test.ts (1)
88-88: Invalid time test uses wrong format.The time
25:00is invalid for multiple reasons (hour > 12, missing am/pm). Consider using a more targeted invalid value like13:00amto specifically test hour validation, or keep as-is since it still validates the rejection behaviour.libs/list-types/london-administrative-court-daily-cause-list/src/conversion/london-admin-config.ts (2)
87-103: Excessive use ofanytypes.The function signature uses
anyforworksheetparameter and return type without justification. Per coding guidelines,anyshould be avoided or justified.Suggested typing improvement
-async function convertSheetToJson(worksheet: any, config: ExcelConverterConfig): Promise<any[]> { +async function convertSheetToJson(worksheet: ExcelJSPkg.Worksheet, config: ExcelConverterConfig): Promise<unknown[]> {If ExcelJS types are problematic, add a brief comment justifying the
anyusage.
106-109: Type assertion bypasses type safety.The
as anycast on the converter function removes type checking. Consider defining a proper type for multi-sheet converters or adding a comment explaining why the cast is necessary.libs/list-types/rcj-court-of-appeal-civil/src/conversion/civil-appeal-config.ts (3)
5-13: Code duplication with london-admin-config.ts.
TIME_PATTERNandvalidateTimeFormatare duplicated verbatim fromlondon-admin-config.ts. Consider extracting these to@hmcts/list-types-commonalongside the existingvalidateDateFormatutility.
143-159: DuplicatedconvertSheetToJsonhelper.This function is identical to the one in
london-admin-config.ts. Extract to a shared utility in@hmcts/list-types-commonto reduce maintenance burden.Suggested approach
In
@hmcts/list-types-common:export async function convertSheetToJson( worksheet: ExcelJSPkg.Worksheet, config: ExcelConverterConfig ): Promise<unknown[]> { // shared implementation }Then import in both config files.
161-165: Same type assertion issue as london-admin-config.ts.The
as anycast on line 164 has the same type safety concern noted in the other converter config.docs/tickets/VIBE-317/implementation-progress.md (1)
235-253: Add language identifier to code block.Markdownlint flags this fenced code block as missing a language specifier. Use
textorplaintextfor directory structures.Proposed fix
-``` +```text module-name/ ├── package.jsonlibs/list-types/rcj-standard-daily-cause-list/README.md (1)
47-50: Add language specifier to fenced code block.Markdown best practice requires a language identifier for syntax highlighting.
📝 Suggested fix
-``` +```text /civil-courts-rcj-daily-cause-list?artefactId=<id> /court-of-appeal-criminal-division-daily-cause-list?artefactId=<id></details> </blockquote></details> <details> <summary>libs/list-types/rcj-court-of-appeal-civil/src/models/types.ts (1)</summary><blockquote> `1-9`: **Consider extracting `StandardHearing` to a shared module.** This interface is duplicated verbatim in `administrative-court-daily-cause-list`, `london-administrative-court-daily-cause-list`, and `rcj-standard-daily-cause-list`. Centralising to `@hmcts/list-types-common` would reduce maintenance burden. </blockquote></details> <details> <summary>docs/tickets/VIBE-317/implementation-summary.md (2)</summary><blockquote> `53-53`: **Minor spelling nit: "sub-sections" → "subsections".** Per static analysis, "subsections" is typically written as one word. <details> <summary>Suggested fix</summary> ```diff -7. **src/pages/london-admin-court.njk** - Template with two sub-sections +7. **src/pages/london-admin-court.njk** - Template with two subsections
120-120: Wrap bare URL in angle brackets or use proper Markdown link syntax.Suggested fix
-- FaCT link (https://www.find-court-tribunal.service.gov.uk/) +- FaCT link (<https://www.find-court-tribunal.service.gov.uk/>)libs/list-types/administrative-court-daily-cause-list/src/views/administrative-court-daily-cause-list/admin-court.njk (2)
35-39: Fragile HTML string manipulation in template.The inline HTML construction using
.split('\n\n').join(...)is brittle and difficult to maintain. Consider pre-rendering this HTML in the handler/renderer and passing it as a safe HTML variable.
44-47: Redundantaria-labelon input.The
<label for="case-search-input">already provides the accessible name for the input. The additionalaria-labelattribute is redundant and could cause confusion for assistive technologies.Suggested fix
<label class="govuk-label govuk-visually-hidden" for="case-search-input"> {{ common.searchCasesLabel }} </label> - <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text" aria-label="{{ common.searchCasesLabel }}"> + <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text">libs/list-types/rcj-court-of-appeal-civil/README.md (1)
75-77: Add language specifier to fenced code block.The code block lacks a language identifier, which triggers a markdownlint warning (MD040). Use
textorplaintextfor URL examples.📝 Suggested fix
-``` +```text /court-of-appeal-civil-division-daily-cause-list?artefactId=<id></details> </blockquote></details> <details> <summary>libs/list-types/rcj-standard-daily-cause-list/src/validation/json-validator.ts (2)</summary><blockquote> `13-14`: **Consider typing Ajv properly instead of using `as any`.** The `as any` cast on the Ajv constructor bypasses type checking. This is a known Ajv/TypeScript interop issue, but consider using proper import typing. <details> <summary>💡 Alternative approach</summary> ```diff -import Ajv from "ajv"; +import Ajv, { type ErrorObject } from "ajv"; ... -const ajv = new (Ajv as any)({ allErrors: true }); +const ajv = new Ajv({ allErrors: true });If type issues persist with the default export, the cast may be necessary due to Ajv's ESM/CJS dual packaging.
16-19: Consider extractingValidationResultto a shared module.This interface is duplicated across at least four validators (
administrative-court,london-administrative-court,rcj-court-of-appeal-civil, and now this one). A shared type definition would reduce duplication.docs/tickets/VIBE-317/specification.md (1)
99-102: Use markdown link formatting for the URL.Line 102 contains a bare URL which triggers markdownlint MD034. Wrap it in markdown link syntax for consistency with other links in the document.
📝 Suggested fix
### FaCT Link - **EN:** "Find contact details and other information about courts and tribunals in England and Wales, and some non-devolved tribunals in Scotland." - **CY:** "Dewch o hyd i fanylion cyswllt a gwybodaeth arall am lysoedd a thribiwnlysoedd yng Nghymru a Lloegr, a rhai tribiwnlysoedd heb eu datganoli yn yr Alban." -- **URL:** https://www.find-court-tribunal.service.gov.uk/ +- **URL:** <https://www.find-court-tribunal.service.gov.uk/>libs/list-types/rcj-standard-daily-cause-list/src/views/rcj-standard-daily-cause-list/standard-daily-cause-list.njk (1)
166-172: Redundantaria-labelattribute.The input at line 171 has both a
<label>element (visually hidden) and anaria-labelattribute with the same value. The<label>withforattribute is sufficient for accessibility; thearia-labelis redundant.Suggested fix
- <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text" aria-label="{{ common.searchCasesLabel }}"> + <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text">libs/list-types/administrative-court-daily-cause-list/src/pages/_handler.ts (3)
16-17: Fragile path calculation for monorepo root.Relying on five levels of
..is brittle and will break if the file moves. Consider using a shared constant from a configuration module or environment variable.
110-110: Unsafe type assertion withany.
(t as any)[listTypeId]bypasses TypeScript's type checking. Consider defining a proper type for the locale objects that includes numeric index signatures.
74-74: File path logged in error message.Logging the full file path (
jsonFilePath) may expose internal directory structure. Consider logging only theartefactIdor a sanitised identifier.libs/list-types/london-administrative-court-daily-cause-list/src/pages/_handler.ts (3)
16-17: Same fragile path calculation issue.As noted in the other handler, this hardcoded path traversal is brittle.
64-64: File path logged in error message.Consider logging only the
artefactIdto avoid exposing internal paths.
19-113: Significant code duplication with other handlers.This handler shares substantial logic with
administrative-court-daily-cause-list/_handler.ts(artefact fetching, file reading, validation flow, error handling). Consider extracting shared utilities to reduce duplication.libs/list-types/london-administrative-court-daily-cause-list/src/views/london-administrative-court-daily-cause-list/london-admin-court.njk (1)
45-51: Redundantaria-labelattribute.The input has both a
<label for="case-search-input">and anaria-label. The label element is sufficient.Suggested fix
- <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text" aria-label="{{ t.searchCasesLabel }}"> + <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text">libs/list-types/rcj-court-of-appeal-civil/src/views/rcj-court-of-appeal-civil/civil-appeal.njk (3)
39-43: Consider extracting complex HTML to a partial or using structured markup.The concatenated HTML string in
govukDetails.htmlis difficult to read and maintain. Consider moving this content to a separate partial template or using Nunjucks'callblocks for better readability.
142-159: Search provides no feedback when no results match.When all rows are filtered out, users see empty tables with no indication that filtering is active. Consider adding a "no results found" message when
queryis non-empty but no rows are visible.💡 Suggested improvement
searchInput.addEventListener('input', function(e) { const query = e.target.value.toLowerCase().trim(); + let visibleCount = 0; tableRows.forEach(function(row) { const text = row.textContent.toLowerCase(); const matches = !query || text.includes(query); row.style.display = matches ? '' : 'none'; + if (matches) visibleCount++; }); + + // Show/hide no results message + const noResultsEl = document.getElementById('no-search-results'); + if (noResultsEl) { + noResultsEl.style.display = (query && visibleCount === 0) ? '' : 'none'; + } });
47-50: Redundant labelling on search input.The input has both a visually-hidden
<label>and anaria-labelattribute. Thearia-labelis unnecessary when an associated<label>element exists.🧹 Remove redundant aria-label
- <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text" aria-label="{{ t.searchCasesLabel }}"> + <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text">libs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.ts (2)
31-58: Consider extracting shared date/time formatting utilities.
formatDisplayDate,formatLastUpdated, andnormalizeTimeare duplicated across multiple renderers in this PR. Extract these to a shared module in@hmcts/list-types-common.
78-78: Hardcoded title strings are inconsistent with locale-driven approach.The title is hardcoded with inline Welsh/English logic, while other text relies on the
localeparameter. Consider passing the title viaRenderOptions(as done in rcj-standard renderer) for consistency.♻️ Use options.listTitle instead
export interface RenderOptions { locale: string; + listTitle: string; displayFrom: Date; displayTo: Date; lastReceivedDate: string; } export function renderLondonAdminCourt(data: LondonAdminCourtData, options: RenderOptions): RenderedData { // ... return { header: { - listTitle: options.locale === "cy" ? "Rhestr Achosion Dyddiol y Llys Gweinyddol Llundain" : "London Administrative Court Daily Cause List", + listTitle: options.listTitle, listDate, lastUpdated },libs/list-types/rcj-standard-daily-cause-list/src/pages/_handler.ts (3)
87-98: JSON.parse error gets swallowed into generic 500 response.Malformed JSON will throw a
SyntaxErrorwhich the outer catch converts to a generic "An error occurred" message. Consider wrappingJSON.parseto provide a more specific error response.🔧 Add explicit JSON parse error handling
- const jsonData: StandardHearingList = JSON.parse(jsonContent); + let jsonData: StandardHearingList; + try { + jsonData = JSON.parse(jsonContent); + } catch { + console.error(`Invalid JSON in artefact ${artefactId}`); + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Invalid Data", + errorMessage: "The list data could not be parsed" + }); + }
114-114: Unsafeanycast without justification.Per coding guidelines, avoid
anywithout justification. The locale objects have known structure; define a proper type.🛠️ Type-safe alternative
+type LocaleContent = typeof en; + // Get list-specific content -const listContent = (t as any)[listTypeId] || {}; +const listContent = (t as LocaleContent)[listTypeId as keyof LocaleContent] || {};
16-17: Fragile path construction with multiple parent traversals.Five levels of
..is brittle and breaks if directory structure changes. Consider using an environment variable or configuration for storage paths.libs/list-types/rcj-court-of-appeal-civil/src/rendering/renderer.ts (2)
44-58: Consider validating the ISO datetime input.If
isoDateTimeis malformed,DateTime.fromISOreturns an invalid DateTime, and subsequent formatting will produce unexpected output (e.g., "Invalid DateTime"). Consider checkingdt.isValidbefore proceeding.♻️ Suggested validation
function formatLastUpdated(isoDateTime: string, locale: string): string { const dt = DateTime.fromISO(isoDateTime).setZone("Europe/London").setLocale(locale); + if (!dt.isValid) { + return "Unknown"; + } const dateStr = dt.toFormat("d MMMM yyyy");
64-74: Add input validation for date string parsing.If
ddMMyyyyDateis malformed (e.g., missing delimiters or non-numeric parts),split("/")may not yield three elements, andNumber.parseIntwill returnNaN. This results in an "Invalid Date" being formatted silently.♻️ Suggested validation
function formatJudgmentDate(ddMMyyyyDate: string, locale: string): string { const [day, month, year] = ddMMyyyyDate.split("/"); + if (!day || !month || !year) { + return ddMMyyyyDate; // Return original if parsing fails + } const date = new Date(Number.parseInt(year, 10), Number.parseInt(month, 10) - 1, Number.parseInt(day, 10)); + if (Number.isNaN(date.getTime())) { + return ddMMyyyyDate; + } const localeCode = locale === "cy" ? "cy-GB" : "en-GB";
| for (const listTypeId of [20, 21, 22, 23]) { | ||
| registerConverter(listTypeId, createConverter(ADMIN_COURT_EXCEL_CONFIG)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the module is imported in the app entry point
rg -n "administrative-court-daily-cause-list" apps/web/src/app.ts 2>/dev/null || echo "app.ts not found or no match"
# Check if index.ts exists and what it exports
find . -path "./libs/list-types/administrative-court-daily-cause-list/src/index.ts" -type f 2>/dev/nullRepository: hmcts/cath-service
Length of output: 420
🏁 Script executed:
# Search more broadly for imports of this module
rg -n "administrative-court-daily-cause-list" --type ts --type tsx 2>/dev/null | head -20Repository: hmcts/cath-service
Length of output: 44
🏁 Script executed:
# Check the actual module structure
cat -n libs/list-types/administrative-court-daily-cause-list/src/index.ts 2>/dev/null || echo "index.ts not found"Repository: hmcts/cath-service
Length of output: 337
🏁 Script executed:
# Look at the admin-court-config.ts file to understand the registration
head -80 libs/list-types/administrative-court-daily-cause-list/src/conversion/admin-court-config.tsRepository: hmcts/cath-service
Length of output: 2368
🏁 Script executed:
# Find the config export file/path
find libs/list-types/administrative-court-daily-cause-list -name "config.ts" -o -name "config.js" -o -name "config/index.ts" 2>/dev/nullRepository: hmcts/cath-service
Length of output: 129
🏁 Script executed:
# Check if there's a config subdirectory or file
ls -la libs/list-types/administrative-court-daily-cause-list/src/ 2>/dev/nullRepository: hmcts/cath-service
Length of output: 680
🏁 Script executed:
# Look for the actual config export (check package.json exports)
cat libs/list-types/administrative-court-daily-cause-list/package.json 2>/dev/null | grep -A 20 "exports"Repository: hmcts/cath-service
Length of output: 945
🏁 Script executed:
# Check the config.ts file content
cat -n libs/list-types/administrative-court-daily-cause-list/src/config.tsRepository: hmcts/cath-service
Length of output: 468
Import the main entry point to trigger converter registration.
App imports from @hmcts/administrative-court-daily-cause-list/config, which doesn't trigger the side-effect registration in index.ts. The converter registration for list types 20–23 will not occur. Import the main module at application startup (e.g., import "@hmcts/administrative-court-daily-cause-list") to ensure converters are registered before use.
| { | ||
| venue: "Court 1", | ||
| judge: "Judge Smith", | ||
| time: "10:00", |
There was a problem hiding this comment.
Time values missing required am/pm suffix - causes pipeline failures.
The schema pattern requires times in format h:mma (e.g., 10:00am) but test data uses 24-hour format (10:00, 14:30). This is why the tests at lines 34 and 68 are failing.
Fix test data to match schema pattern
venue: "Court 1",
judge: "Judge Smith",
- time: "10:00",
+ time: "10:00am",
caseNumber: "T20257890", venue: "Court 2",
judge: "Judge Brown",
- time: "14:30",
+ time: "2:30pm",
caseNumber: "T20257891", venue: "Court 1",
judge: "Judge Smith",
- time: "10:00",
+ time: "10:00am",
caseNumber: "T20257890",Also applies to: 23-23, 55-55
| "build": "tsc && yarn build:nunjucks && yarn build:schemas", | ||
| "build:nunjucks": "mkdir -p dist/pages && cd src/pages && find . -name '*.njk' -exec sh -c 'mkdir -p ../../dist/pages/$(dirname {}) && cp {} ../../dist/pages/{}' \\;", | ||
| "build:schemas": "mkdir -p dist/schemas && cp src/schemas/*.json dist/schemas/", |
There was a problem hiding this comment.
Missing views copy in build script.
The build:nunjucks script only copies templates from src/pages/ but according to the PR context, this module has views at src/views/. These won't be copied to dist/, causing runtime failures in production.
Compare with administrative-court-daily-cause-list which copies both, or rcj-standard-daily-cause-list which has a separate build:views script.
Proposed fix
"scripts": {
"build": "tsc && yarn build:nunjucks && yarn build:schemas",
- "build:nunjucks": "mkdir -p dist/pages && cd src/pages && find . -name '*.njk' -exec sh -c 'mkdir -p ../../dist/pages/$(dirname {}) && cp {} ../../dist/pages/{}' \\;",
+ "build:nunjucks": "mkdir -p dist/pages dist/views && cd src/pages && find . -name '*.njk' -exec sh -c 'mkdir -p ../../dist/pages/$(dirname {}) && cp {} ../../dist/pages/{}' \\; && cd ../views && find . -name '*.njk' -exec sh -c 'mkdir -p ../../dist/views/$(dirname {}) && cp {} ../../dist/views/{}' \\;",| it("should validate data with both tabs populated", () => { | ||
| const validData = { | ||
| dailyHearings: [ | ||
| { | ||
| venue: "Court 1", | ||
| judge: "Judge Smith", | ||
| time: "10:00", | ||
| caseNumber: "T20257890", | ||
| caseDetails: "Case A vs B", | ||
| hearingType: "Appeal", | ||
| additionalInformation: "" | ||
| } | ||
| ], | ||
| futureJudgments: [ | ||
| { | ||
| date: "15/01/2025", | ||
| venue: "Court 2", | ||
| judge: "Judge Brown", | ||
| time: "14:30", | ||
| caseNumber: "T20257891", | ||
| caseDetails: "Case C vs D", | ||
| hearingType: "Judgment", | ||
| additionalInformation: "" | ||
| } | ||
| ] | ||
| }; | ||
|
|
||
| const result = validateCourtOfAppealCivil(validData); | ||
|
|
||
| expect(result.isValid).toBe(true); | ||
| expect(result.errors).toEqual([]); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the schema file
fd "civil-appeal.json" --type fRepository: hmcts/cath-service
Length of output: 133
🏁 Script executed:
# Find and read the test file
fd "json-validator.test.ts" --type f -p "rcj-court-of-appeal-civil"Repository: hmcts/cath-service
Length of output: 174
🏁 Script executed:
# Also look for the validator implementation
fd "json-validator.ts" --type f -p "rcj-court-of-appeal-civil"Repository: hmcts/cath-service
Length of output: 174
🏁 Script executed:
# Find the test file without path restriction
fd "json-validator.test.ts" --type fRepository: hmcts/cath-service
Length of output: 565
🏁 Script executed:
# Read the schema file
cat "libs/list-types/rcj-court-of-appeal-civil/src/schemas/civil-appeal.json"Repository: hmcts/cath-service
Length of output: 2788
🏁 Script executed:
# Read the full test file
cat "libs/list-types/rcj-court-of-appeal-civil/src/validation/json-validator.test.ts"Repository: hmcts/cath-service
Length of output: 3251
Test times do not match schema validation pattern.
The schema requires times in 12-hour format with am/pm suffix (^\d{1,2}([:.]?\d{2})?[ap]m\s*$), but test data provides 24-hour format without suffix (e.g., "10:00", "14:30"). Update times to "10:00am", "2:30pm", etc.
🧰 Tools
🪛 GitHub Actions: Test
[error] 34-34: Test failed: should validate data with both tabs populated. AssertionError: expected true but got false.
🪛 GitHub Check: Test Changed Packages
[failure] 34-34: src/validation/json-validator.test.ts > validateCourtOfAppealCivil > should validate data with both tabs populated
AssertionError: expected false to be true // Object.is equality
- Expected
- Received
- true
- false
❯ src/validation/json-validator.test.ts:34:28
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
libs/list-types/court-of-appeal-civil-daily-cause-list/package.json (1)
16-18: Missing views directory in build script.The
build:nunjucksscript only copies templates fromsrc/pages/but the module contains views atsrc/views/which won't be included in the production build.libs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.ts (1)
5-36: Time format mismatch causing test failures.The schema requires 12-hour format with
am/pmsuffix (pattern:^\d{1,2}([:.]?\d{2})?[ap]m\s*$), but test data uses 24-hour format without suffix ("10:00","14:30").🔧 Proposed fix
time: "10:00", + time: "10:00am",time: "14:30", + time: "2:30pm",Apply similar changes to all time values in lines 56 and 91.
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/_handler.ts (1)
58-58: Path traversal vulnerability remains unaddressed.As flagged previously,
artefactIdis used directly in path construction without sanitisation. Validate that the resolved path stays withinTEMP_UPLOAD_DIR.
🧹 Nitpick comments (9)
libs/list-types/court-of-appeal-civil-daily-cause-list/src/config.ts (1)
7-11: Consider adding missing standardised exports.Per coding guidelines,
config.tsshould exportpageRoutes,apiRoutes,prismaSchemas, andassets. If this module doesn't require API routes or Prisma schemas, consider exporting empty objects ornullfor consistency.♻️ Suggested addition
export const moduleRoot = __dirname; export const pageRoutes = { path: path.join(__dirname, "pages") }; export const assets = path.join(__dirname, "assets/"); +export const apiRoutes = null; +export const prismaSchemas = null;Based on learnings, config.ts must export standardised interfaces.
libs/list-types/court-of-appeal-civil-daily-cause-list/README.md (1)
75-77: Add language specifier to fenced code block.Markdownlint flags this block as missing a language identifier.
♻️ Suggested fix
-``` +```text /court-of-appeal-civil-division-daily-cause-list?artefactId=<id> ```libs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/civil-appeal-config.ts (1)
162-165: Type assertionas anymasks potential type mismatches.The
as anyon line 164 bypasses type checking for the converter registration. This is likely due to the custom multi-sheet converter returning a different shape than expected. Consider updating the type definitions in@hmcts/list-types-commonif this pattern will be reused.libs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.ts (1)
105-124: Invalid time test case is correct but comment could be clearer.The test correctly expects rejection, but
"25:00"fails the pattern for multiple reasons (no am/pm suffix and invalid hour). Consider using a time that only fails the am/pm requirement for clearer test intent, e.g.,"10:00"(missing suffix) vs"25:00am"(invalid hour with suffix).libs/list-types/court-of-appeal-civil-daily-cause-list/src/views/court-of-appeal-civil-daily-cause-list/civil-appeal.njk (1)
53-89: Missing heading for Daily Hearings section.The Future Judgments section has an
<h2>heading (line 94), but Daily Hearings lacks an equivalent. For consistency and accessibility (screen reader navigation), consider adding a heading.🔧 Proposed fix
{# Daily Hearings Section #} {% if dailyHearings.length > 0 %} <div class="hearings-section" id="daily-hearings-section"> + <h2 class="govuk-heading-m">{{ t.dailyHearingsTitle }}</h2> <div id="daily-hearings-table-container">libs/list-types/court-of-appeal-civil-daily-cause-list/src/schemas/civil-appeal.json (1)
51-54: Date pattern validates format but not semantic correctness.The pattern
^\d{2}/\d{2}/\d{4}$accepts invalid dates like99/99/9999. If stricter validation is needed, consider adding runtime validation in the validator function.libs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.ts (3)
44-58: Check for invalid DateTime parsing.
DateTime.fromISOreturns an invalid DateTime object if parsing fails. Consider checkingdt.isValidbefore using it.Suggested fix
function formatLastUpdated(isoDateTime: string, locale: string): string { const dt = DateTime.fromISO(isoDateTime).setZone("Europe/London").setLocale(locale); + + if (!dt.isValid) { + return "Unknown"; + } const dateStr = dt.toFormat("d MMMM yyyy");
64-74: Date parsing lacks validation.If
ddMMyyyyDatedoesn't match the expecteddd/MM/yyyyformat,split("/")may produce fewer than three elements, andparseIntmay returnNaN, resulting in an invalid date. Consider defensive validation.Suggested fix
function formatJudgmentDate(ddMMyyyyDate: string, locale: string): string { const [day, month, year] = ddMMyyyyDate.split("/"); + + if (!day || !month || !year) { + return ddMMyyyyDate; // Return as-is if format is unexpected + } + const date = new Date(Number.parseInt(year, 10), Number.parseInt(month, 10) - 1, Number.parseInt(day, 10)); + + if (Number.isNaN(date.getTime())) { + return ddMMyyyyDate; + } const localeCode = locale === "cy" ? "cy-GB" : "en-GB";
4-33: Consider moving interfaces to the bottom of the file.Per coding guidelines, interfaces and types should be placed at the bottom of the module. This is a minor ordering preference.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (20)
apps/web/src/app.tslibs/admin-pages/src/pages/non-strategic-upload-summary/index.tslibs/admin-pages/src/pages/non-strategic-upload/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/README.mdlibs/list-types/court-of-appeal-civil-daily-cause-list/package.jsonlibs/list-types/court-of-appeal-civil-daily-cause-list/src/config.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/civil-appeal-config.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/models/types.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/_handler.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/court-of-appeal-civil-division-daily-cause-list.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/cy.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/schemas/civil-appeal.jsonlibs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/views/court-of-appeal-civil-daily-cause-list/civil-appeal.njklibs/list-types/court-of-appeal-civil-daily-cause-list/tsconfig.jsontsconfig.json
✅ Files skipped from review due to trivial changes (1)
- libs/list-types/court-of-appeal-civil-daily-cause-list/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (2)
- tsconfig.json
- libs/admin-pages/src/pages/non-strategic-upload-summary/index.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use camelCase for TypeScript variables:userId,caseDetails,documentId
Use PascalCase for classes and interfaces:UserService,CaseRepository. NOIprefix.
Use kebab-case for file and directory names:user-service.ts,case-management/
Use boolean variable names withis/has/canprefix:isActive,hasAccess,canEdit
Module ordering: constants at the top, exported functions next, other functions in order of use, interfaces and types at the bottom
Always add.jsextension to relative imports in ES modules (e.g.,import { foo } from "./bar.js")
Use workspace aliases for imports (@hmcts/*) instead of relative paths across packages
Use strict TypeScript mode enabled; noanywithout justification
Use parameterized database queries with Prisma (no raw SQL string concatenation)
Do not include sensitive data in logs
Files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/court-of-appeal-civil-division-daily-cause-list.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/cy.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/config.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/models/types.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/_handler.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/civil-appeal-config.tslibs/admin-pages/src/pages/non-strategic-upload/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.tsapps/web/src/app.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use SCREAMING_SNAKE_CASE for constants:
MAX_FILE_SIZE,DEFAULT_TIMEOUT
Files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/court-of-appeal-civil-division-daily-cause-list.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/cy.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/config.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/models/types.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/_handler.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/civil-appeal-config.tslibs/admin-pages/src/pages/non-strategic-upload/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.tsapps/web/src/app.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Input validation must be performed on all endpoints
Files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/court-of-appeal-civil-division-daily-cause-list.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/cy.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/config.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/models/types.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/_handler.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/civil-appeal-config.tslibs/admin-pages/src/pages/non-strategic-upload/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.tsapps/web/src/app.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not use CommonJS - ES modules only with import/export syntax
Files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/court-of-appeal-civil-division-daily-cause-list.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/cy.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/config.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/models/types.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/_handler.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/civil-appeal-config.tslibs/admin-pages/src/pages/non-strategic-upload/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.tsapps/web/src/app.ts
**/config.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Module config.ts must export standardized interfaces: pageRoutes, apiRoutes, prismaSchemas, assets
Files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/config.ts
libs/*/src/pages/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/*/src/pages/**/*.ts: Create page controller files with GET and POST exports following the pattern:export const GET = async (req, res) => { ... }
Provide bothenandcylanguage objects in page controllers for English and Welsh support
Files:
libs/admin-pages/src/pages/non-strategic-upload/index.ts
**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
**/package.json: Use@hmctsscope for package names:@hmcts/auth,@hmcts/case-management
All package.json files must use"type": "module"for ES modules support
Express version must be 5.x only ("express": "5.2.0")
All packages must use"test": "vitest run"script in package.json
Dependencies must use specific versions only (e.g.,"express": "5.2.0"), except for peer dependencies
Module build script must include"build:nunjucks"script if module contains Nunjucks templates in pages/ directory
Files:
libs/list-types/court-of-appeal-civil-daily-cause-list/package.json
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Test files must be co-located with source code using
*.test.tsor*.spec.tsnaming pattern
Files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.ts
🧠 Learnings (18)
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.ts : Create page controller files with GET and POST exports following the pattern: `export const GET = async (req, res) => { ... }`
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/court-of-appeal-civil-division-daily-cause-list.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/_handler.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.ts : Provide both `en` and `cy` language objects in page controllers for English and Welsh support
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/cy.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.ts
📚 Learning: 2025-12-19T15:19:47.640Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 192
File: libs/list-types/common/src/mock-list-types.ts:93-110
Timestamp: 2025-12-19T15:19:47.640Z
Learning: For Single Justice Procedure (SJP) list types in libs/list-types/common/src/mock-list-types.ts, the Welsh translation "Gweithdrefn Ynad Sengl" is the preferred terminology for "Single Justice Procedure" rather than "Gweithdrefn Cyfiawnder Sengl".
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/cy.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Welsh translations are required for all user-facing text
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/cy.ts
📚 Learning: 2025-11-20T09:59:16.776Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 106
File: libs/system-admin-pages/src/pages/reference-data-upload/index.test.ts:84-160
Timestamp: 2025-11-20T09:59:16.776Z
Learning: In the cath-service repository, Welsh localization (lng=cy) is not required for admin screens (system-admin-pages), so locale preservation in admin screen redirects is not necessary.
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/cy.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/config.ts : Module config.ts must export standardized interfaces: pageRoutes, apiRoutes, prismaSchemas, assets
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/config.tsapps/web/src/app.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/tsconfig.json : Module tsconfig.json must extend root tsconfig and configure outDir, rootDir, declaration, and declarationMap
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/config.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/*.{ts,tsx} : Module ordering: constants at the top, exported functions next, other functions in order of use, interfaces and types at the bottom
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/config.tslibs/list-types/court-of-appeal-civil-daily-cause-list/package.jsonlibs/list-types/court-of-appeal-civil-daily-cause-list/src/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/*.{ts,tsx} : Use workspace aliases for imports (`hmcts/*`) instead of relative paths across packages
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/config.tsapps/web/src/app.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.njk : Nunjucks templates must extend 'layouts/base-templates.njk' and use GOV.UK Frontend component macros
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/views/court-of-appeal-civil-daily-cause-list/civil-appeal.njklibs/list-types/court-of-appeal-civil-daily-cause-list/package.json
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/package.json : Use hmcts scope for package names: `hmcts/auth`, `hmcts/case-management`
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/package.jsonapps/web/src/app.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/package.json : All package.json files must use `"type": "module"` for ES modules support
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/package.json
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/package.json : Module build script must include `"build:nunjucks"` script if module contains Nunjucks templates in pages/ directory
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/package.json
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to e2e-tests/**/*.spec.ts : Include validation checks, Welsh translation checks, accessibility checks, and keyboard navigation within a single E2E test journey
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/package.json : All packages must use `"test": "vitest run"` script in package.json
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to e2e-tests/**/*.spec.ts : E2E tests must be located in `e2e-tests/` directory with `*.spec.ts` naming pattern
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.ts
📚 Learning: 2025-11-27T09:48:13.010Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 136
File: libs/api/src/blob-ingestion/validation.ts:156-163
Timestamp: 2025-11-27T09:48:13.010Z
Learning: In libs/api/src/blob-ingestion/validation.ts, the permissive date validation in isValidISODate and isValidISODateTime functions is expected behavior and should not be flagged for stricter validation.
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.ts
📚 Learning: 2025-11-20T10:19:35.873Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 106
File: libs/system-admin-pages/src/reference-data-upload/services/download-service.ts:25-27
Timestamp: 2025-11-20T10:19:35.873Z
Learning: In the HMCTS cath-service project, region and sub-jurisdiction names are managed manually and must not contain semicolons, as semicolons are used as delimiters when joining multiple values in CSV export/import operations (specifically in libs/system-admin-pages/src/reference-data-upload/services/download-service.ts).
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.ts
🧬 Code graph analysis (5)
libs/list-types/court-of-appeal-civil-daily-cause-list/src/config.ts (1)
e2e-tests/run-with-credentials.js (2)
__filename(10-10)__dirname(11-11)
libs/admin-pages/src/pages/non-strategic-upload/index.ts (2)
libs/list-types/common/src/mock-list-types.ts (1)
mockListTypes(11-223)libs/list-types/common/src/index.ts (1)
mockListTypes(18-18)
libs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.ts (1)
e2e-tests/run-with-credentials.js (2)
__filename(10-10)__dirname(11-11)
libs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.ts (1)
libs/list-types/court-of-appeal-civil-daily-cause-list/src/models/types.ts (3)
StandardHearing(1-9)FutureJudgment(11-13)CourtOfAppealCivilData(15-18)
libs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.ts (1)
libs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.ts (1)
validateCourtOfAppealCivil(21-40)
🪛 GitHub Actions: Test
libs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.ts
[error] 20-20: AssertionError: expected false to be true in validateStandardDailyCauseList -> should validate a valid hearing list
[error] 39-39: AssertionError: expected false to be true in validateStandardDailyCauseList -> should allow empty additional information
🪛 GitHub Check: Test Changed Packages
libs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.test.ts
[failure] 68-68: src/validation/json-validator.test.ts > validateCourtOfAppealCivil > should validate data with only daily hearings
AssertionError: expected false to be true // Object.is equality
- Expected
- Received
- true
- false
❯ src/validation/json-validator.test.ts:68:28
[failure] 34-34: src/validation/json-validator.test.ts > validateCourtOfAppealCivil > should validate data with both tabs populated
AssertionError: expected false to be true // Object.is equality
- Expected
- Received
- true
- false
❯ src/validation/json-validator.test.ts:34:28
🪛 LanguageTool
libs/list-types/court-of-appeal-civil-daily-cause-list/README.md
[inconsistency] ~50-~50: A new year has begun. Did you mean “15/01/2026”?
Context: ...: Date must be dd/MM/yyyy format (e.g., 15/01/2025), Time must be HH:MM format - All field...
(DATE_NEW_YEAR)
[inconsistency] ~60-~60: A new year has begun. Did you mean “15 January 2026”?
Context: ...ormatted according to locale (English: "15 January 2025", Welsh: "15 Ionawr 2025") ## Special ...
(DATE_NEW_YEAR)
🪛 markdownlint-cli2 (0.18.1)
libs/list-types/court-of-appeal-civil-daily-cause-list/README.md
75-75: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests
🔇 Additional comments (14)
libs/admin-pages/src/pages/non-strategic-upload/index.ts (1)
14-20: LGTM! Sorting list types alphabetically improves UX.Clean implementation using
localeComparefor proper string sorting at module load time.libs/list-types/court-of-appeal-civil-daily-cause-list/src/models/types.ts (1)
1-18: LGTM! Clean type definitions.Well-structured interfaces following PascalCase convention. The extension pattern for
FutureJudgmentkeeps things DRY.libs/list-types/court-of-appeal-civil-daily-cause-list/package.json (1)
1-45: Package configuration looks good.ES modules support, workspace aliases, and version pinning are correctly configured. The test script and dependencies follow the established patterns.
apps/web/src/app.ts (3)
5-5: New list type imports correctly configured.Imports use workspace aliases and follow the established pattern for config subpath exports.
Also applies to: 14-14, 17-17, 23-23
82-85: Module paths correctly extended.The four new list type module roots are added in a consistent position within the array.
126-129: Route registration follows established pattern.New routes are registered before the generic page routes, ensuring proper route matching precedence.
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/court-of-appeal-civil-division-daily-cause-list.ts (1)
1-1: LGTM!Follows the established page controller pattern with correct
.jsextension for ES module imports. Based on coding guidelines.libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.ts (1)
1-37: LGTM!Comprehensive English localisation with well-structured keys and logical groupings. The nested
tableHeadersobject follows established patterns.libs/list-types/court-of-appeal-civil-daily-cause-list/src/index.ts (1)
1-6: LGTM!Clean module entry point following the established pattern. Side-effect import for converter registration is appropriately documented, and re-exports are well-organised.
libs/list-types/court-of-appeal-civil-daily-cause-list/src/views/court-of-appeal-civil-daily-cause-list/civil-appeal.njk (1)
142-160: Search script is functional and well-scoped.The IIFE pattern prevents global namespace pollution. The early return on missing elements is a good defensive pattern.
libs/list-types/court-of-appeal-civil-daily-cause-list/src/schemas/civil-appeal.json (1)
1-87: Schema structure is sound.Required fields, HTML-injection patterns, and format validations are well-defined. The schema correctly distinguishes between dailyHearings (no date) and futureJudgments (with date).
libs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.ts (1)
21-40: Validation function is well-structured.Clean separation of valid/invalid paths. Error mapping to human-readable format is helpful. The fallback
["Unknown validation error"]handles edge cases gracefully.libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/_handler.ts (1)
19-114: Handler structure follows the expected pattern.The GET export, locale handling, artefact retrieval, validation, and rendering flow are well-organised and consistent with project conventions. Based on learnings, this follows the required page controller pattern.
libs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.ts (1)
101-114: Rendering logic is clean and well-structured.The function correctly composes header, daily hearings, and future judgments with locale-aware formatting. Good use of helper functions for separation of concerns.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| async function convertSheetToJson(worksheet: any, config: ExcelConverterConfig): Promise<any[]> { | ||
| // Create a temporary buffer from the sheet | ||
| const workbook = new Workbook(); | ||
| const tempSheet = workbook.addWorksheet("temp"); | ||
|
|
||
| // Copy all rows from source to temp worksheet | ||
| worksheet.eachRow((row: any, rowNumber: number) => { | ||
| const newRow = tempSheet.getRow(rowNumber); | ||
| row.eachCell((cell: any, colNumber: number) => { | ||
| newRow.getCell(colNumber).value = cell.value; | ||
| }); | ||
| newRow.commit(); | ||
| }); | ||
|
|
||
| const buffer = await workbook.xlsx.writeBuffer(); | ||
| return convertExcelToJson(Buffer.from(buffer), config); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n libs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/civil-appeal-config.ts | head -50Repository: hmcts/cath-service
Length of output: 2141
🏁 Script executed:
# Find package.json to understand ExcelJS version
find . -name "package.json" -type f | head -5Repository: hmcts/cath-service
Length of output: 189
🏁 Script executed:
# Search for ExcelJS imports in the codebase
rg "from.*exceljs|from.*ExcelJS" --type ts --type tsxRepository: hmcts/cath-service
Length of output: 89
🏁 Script executed:
# Search for ExcelJS in package.json files
rg "exceljs" -i apps/*/package.json ./package.json 2>/dev/null || rg "exceljs" -iRepository: hmcts/cath-service
Length of output: 2901
🏁 Script executed:
# Get the full function to see how worksheet is used
sed -n '143,159p' libs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/civil-appeal-config.ts | cat -nRepository: hmcts/cath-service
Length of output: 816
🏁 Script executed:
# Check convertExcelToJson signature and return type
rg "convertExcelToJson" -A 5 --type ts | head -30Repository: hmcts/cath-service
Length of output: 3761
🏁 Script executed:
# Check ExcelJS types available
web_search
ExcelJS Worksheet type TypeScript definitionsRepository: hmcts/cath-service
Length of output: 158
🏁 Script executed:
# Find and check the convertExcelToJson function signature
rg "export.*convertExcelToJson|function convertExcelToJson" -A 3 libs/list-types/common/src/conversion/Repository: hmcts/cath-service
Length of output: 579
🏁 Script executed:
# Check how ExcelJS types are used in tests to see if Worksheet type is available
cat -n libs/list-types/common/src/conversion/excel-to-json.test.ts | head -40Repository: hmcts/cath-service
Length of output: 1671
🏁 Script executed:
# Check if ExcelJS.Worksheet type is used anywhere
rg "Worksheet" libs/list-types/ --type tsRepository: hmcts/cath-service
Length of output: 1376
🏁 Script executed:
# Check how Worksheet type is imported in test files that use proper typing
cat -n libs/list-types/common/src/conversion/excel-to-json.test.ts | grep -E "ExcelJS\.|Worksheet" | head -10Repository: hmcts/cath-service
Length of output: 171
🏁 Script executed:
# Check if the file uses any `@ts-expect-error` comments and context
rg "@ts-expect-error" -B 2 -A 2 libs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/civil-appeal-config.tsRepository: hmcts/cath-service
Length of output: 286
🏁 Script executed:
# Verify the exact structure of worksheet parameter usage in the function
sed -n '143,159p' libs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/civil-appeal-config.tsRepository: hmcts/cath-service
Length of output: 697
🏁 Script executed:
# Check how other files in the codebase handle worksheet parameter typing
rg "worksheet:" -A 2 libs/list-types/ --type tsRepository: hmcts/cath-service
Length of output: 998
🏁 Script executed:
# Verify the exact import pattern needed for Worksheet type
cat -n libs/list-types/common/src/conversion/excel-to-json.test.ts | head -5Repository: hmcts/cath-service
Length of output: 375
Improve type safety for the worksheet parameter.
Using any reduces compile-time safety. The worksheet parameter supports eachRow() and eachCell() methods; use ExcelJS.Worksheet instead. Note: this requires importing ExcelJS types as import * as ExcelJS from "exceljs" rather than the current CommonJS pattern to access the Worksheet type directly.
The return type can be refined to Record<string, string>[], which aligns with the default return type of convertExcelToJson.
♻️ Suggested improvement
-async function convertSheetToJson(worksheet: any, config: ExcelConverterConfig): Promise<any[]> {
+async function convertSheetToJson(worksheet: ExcelJS.Worksheet, config: ExcelConverterConfig): Promise<Record<string, string>[]> {This requires adding import * as ExcelJS from "exceljs"; at the top of the file.
| try { | ||
| jsonContent = await readFile(jsonFilePath, "utf-8"); | ||
| } catch (error) { | ||
| console.error(`Error reading JSON file at ${jsonFilePath}:`, error); |
There was a problem hiding this comment.
Avoid logging full file paths.
Logging jsonFilePath exposes internal directory structure. Log only the artefactId or a sanitised reference. As per coding guidelines, sensitive data should not appear in logs.
Suggested fix
- console.error(`Error reading JSON file at ${jsonFilePath}:`, error);
+ console.error(`Error reading JSON file for artefact ${artefactId}:`, error);📝 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.
| console.error(`Error reading JSON file at ${jsonFilePath}:`, error); | |
| console.error(`Error reading JSON file for artefact ${artefactId}:`, error); |
| const jsonData: CourtOfAppealCivilData = JSON.parse(jsonContent); | ||
|
|
||
| const validationResult = validateCourtOfAppealCivil(jsonData); | ||
| if (!validationResult.isValid) { | ||
| console.error("Validation errors:", validationResult.errors); | ||
| return res.status(400).render("errors/common", { | ||
| en, | ||
| cy, | ||
| errorTitle: "Invalid Data", | ||
| errorMessage: "The list data is invalid" | ||
| }); | ||
| } |
There was a problem hiding this comment.
Malformed JSON returns 500 instead of 400.
JSON.parse can throw on invalid JSON, which falls through to the outer catch block returning a 500 error. This should be a 400 Bad Request.
Suggested fix
- const jsonData: CourtOfAppealCivilData = JSON.parse(jsonContent);
+ let jsonData: CourtOfAppealCivilData;
+ try {
+ jsonData = JSON.parse(jsonContent);
+ } catch {
+ return res.status(400).render("errors/common", {
+ en,
+ cy,
+ errorTitle: "Invalid Data",
+ errorMessage: "The list data could not be parsed"
+ });
+ }📝 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: CourtOfAppealCivilData = JSON.parse(jsonContent); | |
| const validationResult = validateCourtOfAppealCivil(jsonData); | |
| if (!validationResult.isValid) { | |
| console.error("Validation errors:", validationResult.errors); | |
| return res.status(400).render("errors/common", { | |
| en, | |
| cy, | |
| errorTitle: "Invalid Data", | |
| errorMessage: "The list data is invalid" | |
| }); | |
| } | |
| let jsonData: CourtOfAppealCivilData; | |
| try { | |
| jsonData = JSON.parse(jsonContent); | |
| } catch { | |
| return res.status(400).render("errors/common", { | |
| en, | |
| cy, | |
| errorTitle: "Invalid Data", | |
| errorMessage: "The list data could not be parsed" | |
| }); | |
| } | |
| const validationResult = validateCourtOfAppealCivil(jsonData); | |
| if (!validationResult.isValid) { | |
| console.error("Validation errors:", validationResult.errors); | |
| return res.status(400).render("errors/common", { | |
| en, | |
| cy, | |
| errorTitle: "Invalid Data", | |
| errorMessage: "The list data is invalid" | |
| }); | |
| } |
| const ajv = new (Ajv as any)({ allErrors: true }); | ||
| const validate = ajv.compile(schema); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the actual file to see the import and context
cat -n libs/list-types/court-of-appeal-civil-daily-cause-list/src/validation/json-validator.ts | head -30Repository: hmcts/cath-service
Length of output: 1038
🏁 Script executed:
# Check how Ajv is imported in other validators
rg -n "import.*Ajv" --type ts -B2 -A2Repository: hmcts/cath-service
Length of output: 3282
🏁 Script executed:
# Also check for uses of Ajv in the codebase
rg -n "new.*Ajv" --type ts -B1 -A1Repository: hmcts/cath-service
Length of output: 2291
Remove the as any cast—it violates strict TypeScript mode without justification.
The import import Ajv from "ajv" is correct and should work without casting. This pattern appears across multiple validators in the codebase; if there is a genuine type compatibility issue with your Ajv version, add a comment explaining why the cast is necessary. Otherwise, use:
const ajv = new Ajv({ allErrors: true });
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts (3)
23-32: Path traversal vulnerability remains unaddressed.The
artefactIdis used directly in constructingjsonFilePathwithout sanitisation. An attacker could supply../../../etc/passwdto read arbitrary files. Validate that the resolved path remains withinTEMP_UPLOAD_DIR.Suggested fix
const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`); + + // Prevent path traversal attacks + const resolvedPath = path.resolve(jsonFilePath); + if (!resolvedPath.startsWith(path.resolve(TEMP_UPLOAD_DIR))) { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Bad Request", + errorMessage: "Invalid artefactId parameter" + }); + }Also applies to: 58-58
64-64: Avoid logging full file paths.Logging
jsonFilePathexposes internal directory structure. Log only theartefactId. As per coding guidelines, sensitive data should not appear in logs.Suggested fix
- console.error(`Error reading JSON file at ${jsonFilePath}:`, error); + console.error(`Error reading JSON file for artefact ${artefactId}:`, error);
73-84: Malformed JSON returns 500 instead of 400.
JSON.parsecan throw on invalid JSON, which falls through to the outer catch block returning a 500 error. This should be a 400 Bad Request since it's client-supplied data.Suggested fix
- const jsonData: CourtOfAppealCivilData = JSON.parse(jsonContent); + let jsonData: CourtOfAppealCivilData; + try { + jsonData = JSON.parse(jsonContent); + } catch { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Invalid Data", + errorMessage: "The list data could not be parsed" + }); + }
🧹 Nitpick comments (10)
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.ts (1)
23-23: Consider validatingartefactIdbefore use.The
artefactIdis cast directly from the query string and subsequently used in file path construction (line 48). Whilst Prisma handles parameterised queries safely, input validation (e.g., UUID format check) would harden against path traversal or unexpected input.🛡️ Suggested validation
+ const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + const artefactId = req.query.artefactId as string; - if (!artefactId) { + if (!artefactId || !UUID_REGEX.test(artefactId)) { return res.status(400).render("errors/common", { en, cy, errorTitle: "Bad Request", - errorMessage: "Missing artefactId parameter" + errorMessage: "Invalid or missing artefactId parameter" }); }As per coding guidelines, input validation must be performed on all endpoints.
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts (2)
16-17: Consider using environment configuration for paths.The relative path navigation (
"..", "..", "..", "..", "..") is fragile if the directory structure changes. Consider using an environment variable or centralised configuration forTEMP_UPLOAD_DIR.
49-49: Extract magic number to a named constant.The list type ID
19should be defined as a named constant for clarity and maintainability.Suggested fix
+const COURT_OF_APPEAL_CIVIL_LIST_TYPE_ID = 19; + // ... - if (artefact.listTypeId !== 19) { + if (artefact.listTypeId !== COURT_OF_APPEAL_CIVIL_LIST_TYPE_ID) {libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.ts (2)
16-17: Consider centralising the upload directory path.The relative path traversal (
"..", "..", "..", "..", "..") is fragile and duplicated across multiple list-type modules. Consider extractingTEMP_UPLOAD_DIRinto a shared configuration module to improve maintainability.
64-64: Avoid logging full filesystem paths.Logging the full
jsonFilePathexposes server directory structure. Consider logging just theartefactIdinstead.Proposed fix
- console.error(`Error reading JSON file at ${jsonFilePath}:`, error); + console.error(`Error reading JSON file for artefact ${artefactId}:`, error);libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (2)
110-110: Avoidas anycast.Per coding guidelines, avoid
anywithout justification. Consider typing the locale objects to include numeric keys or use a type-safe accessor.Example approach
// In locale type definition: interface LocaleData { common: CommonContent; [listTypeId: number]: ListContent; } // Then usage becomes type-safe: const listContent = t[listTypeId] || {};
74-74: Avoid logging full filesystem paths.Same issue as other modules—log the
artefactIdinstead of the full path.libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (3)
78-78: Avoid logging full filesystem paths.Log the
artefactIdrather than the fulljsonFilePath.
114-114: Avoidas anycast.Same issue as the Administrative Court module—type the locale objects properly to avoid the
anycast.
31-137: Consider extracting shared handler logic.All three page controllers (
london-administrative-court,administrative-court,rcj-standard) share nearly identical structure: artefact lookup, list type validation, JSON file reading, validation, rendering, and error handling. Consider extracting a shared factory function or base handler to reduce duplication.Example approach
// shared/list-handler-factory.ts export function createListHandler(config: { supportedListTypeIds: number[]; validator: (data: unknown) => ValidationResult; renderer: (data: any, options: RenderOptions) => RenderedData; viewPath: string; locales: { en: LocaleData; cy: LocaleData }; }) { return async (req: Request, res: Response) => { // Shared implementation here }; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
libs/admin-pages/src/pages/non-strategic-upload/index.test.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/list-types/common/src/mock-list-types.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use camelCase for TypeScript variables:userId,caseDetails,documentId
Use PascalCase for classes and interfaces:UserService,CaseRepository. NOIprefix.
Use kebab-case for file and directory names:user-service.ts,case-management/
Use boolean variable names withis/has/canprefix:isActive,hasAccess,canEdit
Module ordering: constants at the top, exported functions next, other functions in order of use, interfaces and types at the bottom
Always add.jsextension to relative imports in ES modules (e.g.,import { foo } from "./bar.js")
Use workspace aliases for imports (@hmcts/*) instead of relative paths across packages
Use strict TypeScript mode enabled; noanywithout justification
Use parameterized database queries with Prisma (no raw SQL string concatenation)
Do not include sensitive data in logs
Files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/common/src/mock-list-types.tslibs/admin-pages/src/pages/non-strategic-upload/index.test.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use SCREAMING_SNAKE_CASE for constants:
MAX_FILE_SIZE,DEFAULT_TIMEOUT
Files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/common/src/mock-list-types.tslibs/admin-pages/src/pages/non-strategic-upload/index.test.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Input validation must be performed on all endpoints
Files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/common/src/mock-list-types.tslibs/admin-pages/src/pages/non-strategic-upload/index.test.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not use CommonJS - ES modules only with import/export syntax
Files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/common/src/mock-list-types.tslibs/admin-pages/src/pages/non-strategic-upload/index.test.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Test files must be co-located with source code using
*.test.tsor*.spec.tsnaming pattern
Files:
libs/admin-pages/src/pages/non-strategic-upload/index.test.ts
libs/*/src/pages/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/*/src/pages/**/*.ts: Create page controller files with GET and POST exports following the pattern:export const GET = async (req, res) => { ... }
Provide bothenandcylanguage objects in page controllers for English and Welsh support
Files:
libs/admin-pages/src/pages/non-strategic-upload/index.test.ts
🧠 Learnings (4)
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.ts : Provide both `en` and `cy` language objects in page controllers for English and Welsh support
Applied to files:
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.ts : Create page controller files with GET and POST exports following the pattern: `export const GET = async (req, res) => { ... }`
Applied to files:
libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/routes/**/*.ts : API endpoints should use plural for collections (/api/cases), singular for specific (/api/case/:id), and singular for creation (POST /api/case)
Applied to files:
libs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.ts
📚 Learning: 2025-11-20T10:19:35.873Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 106
File: libs/system-admin-pages/src/reference-data-upload/services/download-service.ts:25-27
Timestamp: 2025-11-20T10:19:35.873Z
Learning: In the HMCTS cath-service project, region and sub-jurisdiction names are managed manually and must not contain semicolons, as semicolons are used as delimiters when joining multiple values in CSV export/import operations (specifically in libs/system-admin-pages/src/reference-data-upload/services/download-service.ts).
Applied to files:
libs/list-types/common/src/mock-list-types.ts
🧬 Code graph analysis (1)
libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.ts (3)
libs/list-types/london-administrative-court-daily-cause-list/src/models/types.ts (1)
LondonAdminCourtData(11-14)libs/list-types/london-administrative-court-daily-cause-list/src/validation/json-validator.ts (1)
validateLondonAdminCourt(21-40)libs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.ts (1)
renderLondonAdminCourt(72-85)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests
🔇 Additional comments (13)
libs/admin-pages/src/pages/non-strategic-upload/index.test.ts (1)
172-193: LGTM – Test logic is sound and follows existing patterns.The test correctly verifies alphabetical ordering by comparing the original array against a sorted copy. The approach of filtering out the placeholder option before comparison is appropriate.
Minor observation: the type assertion on line 185 assumes
renderCall[1]exists and haslistTypes. This is acceptable given the preceding assertion on line 183, but you could consider adding an explicit check for additional robustness:expect(renderCall[1]).toBeDefined();Not essential given the test context.
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.ts (1)
88-96: LGTM – Title addition is clean.The addition of
title: header.listTitlecorrectly passes the list title to the template, aligning with the rendering pattern.libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts (3)
1-11: LGTM!Imports follow ES module syntax with
.jsextensions for relative imports and@hmcts/*workspace aliases for cross-package imports.
86-104: LGTM!Rendering logic correctly transforms data and passes localisation context to the view.
105-113: LGTM!Catch-all error handler appropriately returns 500 for unexpected errors.
libs/list-types/common/src/mock-list-types.ts (1)
92-222: New RCJ list types look well-structured.All 14 new entries consistently follow the established pattern with appropriate Welsh translations, correct provenance (
MANUAL_UPLOAD), andisNonStrategic: true. The grouping comments help readability.libs/list-types/civil-and-family-daily-cause-list/src/pages/index.ts (2)
84-93: LGTM — title addition is consistent with PR objectives.The
title: header.listTitleaddition aligns with the broader list-type rendering approach introduced across this PR. Bothenandcylocale objects are correctly provided for Welsh language support. Based on learnings.
48-48: The database schema safeguard makes additional format validation unnecessary.The
artefactIdis defined in the Prisma schema with@db.Uuidtype constraint, which enforces UUID format at the database level. When the code executesfindUnique({ where: { artefactId } }), PostgreSQL will only match valid UUIDs. Any non-UUID value (including path traversal sequences) will fail the lookup and trigger the 404 response before reachingpath.join, eliminating the path traversal risk. No additional client-side validation is required.libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.ts (2)
1-11: Imports look correct.All relative imports include the
.jsextension and workspace aliases are used appropriately. This follows ES module conventions. Based on learnings, the controller correctly exports a GET handler and provides bothenandcylocale objects.
86-114: Rendering and error handling look sound.The renderer is invoked with appropriate options, and the catch block correctly returns a 500 for unexpected errors. Template data is complete.
libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (2)
1-11: Imports are correctly structured.Uses
.jsextensions for relative imports and workspace aliases for cross-package imports. Follows the established page controller pattern.
19-25: List type mapping is clear.The
LIST_TYPE_NAMESconstant provides a clean mapping between list type IDs and their localised titles.libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (1)
1-29: Structure and imports are correct.The handler follows the established page controller pattern with proper ES module imports. The
LIST_TYPE_NAMESmapping for IDs 10–17 is clear.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const jsonData: StandardHearingList = JSON.parse(jsonContent); | ||
|
|
||
| const validationResult = validateAdminCourt(jsonData); | ||
| if (!validationResult.isValid) { | ||
| console.error("Validation errors:", validationResult.errors); | ||
| return res.status(400).render("errors/common", { | ||
| en, | ||
| cy, | ||
| errorTitle: "Invalid Data", | ||
| errorMessage: "The list data is invalid" | ||
| }); | ||
| } |
There was a problem hiding this comment.
Malformed JSON returns 500 instead of 400.
Same issue as the London Administrative Court module: JSON.parse is outside the inner try-catch. Move it inside to return a proper 400 for invalid JSON.
Proposed fix
let jsonContent: string;
+ let jsonData: StandardHearingList;
try {
jsonContent = await readFile(jsonFilePath, "utf-8");
+ jsonData = JSON.parse(jsonContent);
} catch (error) {
- console.error(`Error reading JSON file at ${jsonFilePath}:`, error);
+ console.error(`Error reading or parsing JSON file for artefact ${artefactId}:`, error);
return res.status(404).render("errors/common", {
en,
cy,
errorTitle: "Not Found",
errorMessage: "The requested list could not be found"
});
}
-
- const jsonData: StandardHearingList = JSON.parse(jsonContent);| const jsonData: LondonAdminCourtData = JSON.parse(jsonContent); | ||
|
|
||
| const validationResult = validateLondonAdminCourt(jsonData); | ||
| if (!validationResult.isValid) { | ||
| console.error("Validation errors:", validationResult.errors); | ||
| return res.status(400).render("errors/common", { | ||
| en, | ||
| cy, | ||
| errorTitle: "Invalid Data", | ||
| errorMessage: "The list data is invalid" | ||
| }); | ||
| } |
There was a problem hiding this comment.
Malformed JSON returns 500 instead of 400.
JSON.parse on line 73 is outside the inner try-catch. If the file contains invalid JSON, the error propagates to the outer catch block and returns a 500 status code. It should return 400 (bad request).
Proposed fix
const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`);
let jsonContent: string;
+ let jsonData: LondonAdminCourtData;
try {
jsonContent = await readFile(jsonFilePath, "utf-8");
+ jsonData = JSON.parse(jsonContent);
} catch (error) {
- console.error(`Error reading JSON file at ${jsonFilePath}:`, error);
+ console.error(`Error reading or parsing JSON file for artefact ${artefactId}:`, error);
return res.status(404).render("errors/common", {
en,
cy,
errorTitle: "Not Found",
errorMessage: "The requested list could not be found"
});
}
- const jsonData: LondonAdminCourtData = JSON.parse(jsonContent);
-
const validationResult = validateLondonAdminCourt(jsonData);📝 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: LondonAdminCourtData = JSON.parse(jsonContent); | |
| const validationResult = validateLondonAdminCourt(jsonData); | |
| if (!validationResult.isValid) { | |
| console.error("Validation errors:", validationResult.errors); | |
| return res.status(400).render("errors/common", { | |
| en, | |
| cy, | |
| errorTitle: "Invalid Data", | |
| errorMessage: "The list data is invalid" | |
| }); | |
| } | |
| const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`); | |
| let jsonContent: string; | |
| let jsonData: LondonAdminCourtData; | |
| try { | |
| jsonContent = await readFile(jsonFilePath, "utf-8"); | |
| jsonData = JSON.parse(jsonContent); | |
| } catch (error) { | |
| console.error(`Error reading or parsing JSON file for artefact ${artefactId}:`, error); | |
| return res.status(404).render("errors/common", { | |
| en, | |
| cy, | |
| errorTitle: "Not Found", | |
| errorMessage: "The requested list could not be found" | |
| }); | |
| } | |
| const validationResult = validateLondonAdminCourt(jsonData); | |
| if (!validationResult.isValid) { | |
| console.error("Validation errors:", validationResult.errors); | |
| return res.status(400).render("errors/common", { | |
| en, | |
| cy, | |
| errorTitle: "Invalid Data", | |
| errorMessage: "The list data is invalid" | |
| }); | |
| } |
| const jsonData: StandardHearingList = JSON.parse(jsonContent); | ||
|
|
||
| const validationResult = validateStandardDailyCauseList(jsonData); | ||
| if (!validationResult.isValid) { | ||
| console.error("Validation errors:", validationResult.errors); | ||
| return res.status(400).render("errors/common", { | ||
| en, | ||
| cy, | ||
| errorTitle: "Invalid Data", | ||
| errorMessage: "The list data is invalid" | ||
| }); | ||
| } |
There was a problem hiding this comment.
Same JSON.parse issue—malformed JSON returns 500.
Consistent with the other modules, JSON.parse should be moved inside the try-catch to return 400 for invalid JSON rather than 500.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.ts (1)
75-86: Malformed JSON returns 500 instead of 400.
JSON.parseon line 75 is outside the inner try-catch. If the file contains invalid JSON, the error propagates to the outer catch and returns 500. Move it inside to return 400 for parse errors.libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (1)
90-101: Malformed JSON returns 500 instead of 400.Same issue as the London Administrative Court module:
JSON.parseis outside the inner try-catch. Move it inside to return a proper 400 for invalid JSON.libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (1)
98-109: Same JSON.parse issue – malformed JSON returns 500.Consistent with the other modules,
JSON.parseshould be moved inside the try-catch to return 400 for invalid JSON rather than 500.libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts (3)
60-60: Path traversal vulnerability remains unaddressed.If
artefactIdcontains sequences like../../../etc/passwd, arbitrary files could be read. Validate that the resolved path stays withinTEMP_UPLOAD_DIR.
66-66: Avoid logging full file paths.Per coding guidelines, sensitive data (including internal directory structure) should not appear in logs. Log only the
artefactId.
75-75: JSON.parse can throw on malformed input.Invalid JSON will propagate to the outer catch and return 500 instead of the more appropriate 400 Bad Request. Wrap in a dedicated try/catch.
🧹 Nitpick comments (11)
libs/simple-router/src/simple-router.ts (3)
92-102: Double cast bypasses type safety.The
as any as Handler[]pattern silences TypeScript entirely. Consider a more targeted approach using a dedicated error-handler type or union.♻️ Suggested improvement
- // Cast to any[] first to bypass TypeScript's strict checking - // Express internally handles both 3-param and 4-param handlers - handlers: [module.onError] as any as Handler[], + handlers: [module.onError as unknown as Handler],Alternatively, extend the
RouteEntryinterface to acceptErrorRequestHandlerinhandlerswhenmethod === "use".
108-113: AddROUTEStoRouteModuleinterface for explicit typing.The function accesses
module.ROUTESbut the interface only has an index signature. Adding an explicit property improves discoverability and IDE support.♻️ Proposed change to RouteModule interface
export interface RouteModule { [key: string]: unknown; onError?: ErrorRequestHandler; + ROUTES?: string[]; }
185-188: Interface missingROUTESproperty.As noted above,
getRoutePathsrelies onmodule.ROUTES. Adding it here documents the contract and enables type inference.♻️ Proposed fix
export interface RouteModule { [key: string]: unknown; onError?: ErrorRequestHandler; + ROUTES?: string[]; }libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.ts (1)
25-25: Consider validatingartefactIdformat before use.The
artefactIdis cast directly tostringwithout format validation. If this value is used in file path construction (line 60) and database queries, consider validating it matches an expected pattern (e.g., UUID) to prevent path traversal or injection attempts.Proposed validation
const artefactId = req.query.artefactId as string; if (!artefactId) { return res.status(400).render("errors/common", { en, cy, errorTitle: "Bad Request", errorMessage: "Missing artefactId parameter" }); } + + // Validate artefactId format (assuming UUID) + const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + if (!uuidRegex.test(artefactId)) { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Bad Request", + errorMessage: "Invalid artefactId format" + }); + }libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (2)
38-38: SameartefactIdvalidation concern applies here.As noted for the London Administrative Court module, consider validating the format of
artefactIdbefore using it in file paths and database queries.
117-117: Avoidas anycast – use proper typing.The
as anycast bypasses TypeScript's type checking. Consider defining a proper type for the translation structure or using a type guard.Proposed improvement
- const listContent = (t as any)[listTypeId] || {}; + const listContent = t[listTypeId as keyof typeof t] || {};Alternatively, define an interface for the translation object structure that includes numeric keys.
libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (3)
13-22: Eight routes from one handler – consider documenting the mapping.This handler serves many list types. A brief comment mapping route paths to list type IDs would aid maintainability.
46-46: SameartefactIdvalidation concern applies here.Consider validating the format before use in file paths and queries, consistent with the other modules.
125-125: Sameas anycast – use proper typing.Same concern as the administrative court module. Replace
as anywith proper type narrowing.libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts (2)
15-19: Fragile path resolution.The 5-level
..traversal toMONOREPO_ROOTis brittle. Consider using an environment variable or centralised configuration forTEMP_UPLOAD_DIRto avoid silent breakage if the directory structure changes.
27-34: Error messages are hardcoded in English.The
tobject is set up for localisation but error titles and messages use hardcoded English strings. Consider usingt.errorTitleandt.errorMessagepatterns for consistency with the locale.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
libs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/simple-router/src/simple-router.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/list-types/civil-and-family-daily-cause-list/src/pages/index.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use camelCase for TypeScript variables:userId,caseDetails,documentId
Use PascalCase for classes and interfaces:UserService,CaseRepository. NOIprefix.
Use kebab-case for file and directory names:user-service.ts,case-management/
Use boolean variable names withis/has/canprefix:isActive,hasAccess,canEdit
Module ordering: constants at the top, exported functions next, other functions in order of use, interfaces and types at the bottom
Always add.jsextension to relative imports in ES modules (e.g.,import { foo } from "./bar.js")
Use workspace aliases for imports (@hmcts/*) instead of relative paths across packages
Use strict TypeScript mode enabled; noanywithout justification
Use parameterized database queries with Prisma (no raw SQL string concatenation)
Do not include sensitive data in logs
Files:
libs/simple-router/src/simple-router.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use SCREAMING_SNAKE_CASE for constants:
MAX_FILE_SIZE,DEFAULT_TIMEOUT
Files:
libs/simple-router/src/simple-router.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Input validation must be performed on all endpoints
Files:
libs/simple-router/src/simple-router.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not use CommonJS - ES modules only with import/export syntax
Files:
libs/simple-router/src/simple-router.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts
🧠 Learnings (3)
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.ts : Create page controller files with GET and POST exports following the pattern: `export const GET = async (req, res) => { ... }`
Applied to files:
libs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/routes/**/*.ts : API endpoints should use plural for collections (/api/cases), singular for specific (/api/case/:id), and singular for creation (POST /api/case)
Applied to files:
libs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.ts : Provide both `en` and `cy` language objects in page controllers for English and Welsh support
Applied to files:
libs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.ts
🧬 Code graph analysis (2)
libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (2)
libs/list-types/rcj-standard-daily-cause-list/src/validation/json-validator.ts (1)
validateStandardDailyCauseList(21-40)libs/list-types/rcj-standard-daily-cause-list/src/rendering/renderer.ts (1)
renderStandardDailyCauseList(61-83)
libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.ts (3)
libs/list-types/london-administrative-court-daily-cause-list/src/models/types.ts (1)
LondonAdminCourtData(11-14)libs/list-types/london-administrative-court-daily-cause-list/src/validation/json-validator.ts (1)
validateLondonAdminCourt(21-40)libs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.ts (1)
renderLondonAdminCourt(72-85)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests
🔇 Additional comments (10)
libs/simple-router/src/simple-router.ts (2)
73-76: LGTM – Clean multi-path resolution.The approach of resolving paths via
getRoutePathsand mapping to full paths is clear and maintainable.
77-87: LGTM – Handler registration per path.Nested iteration correctly generates route entries for each path/method combination. The structure is sound.
libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.ts (2)
1-11: LGTM – Imports follow project conventions.Correct use of
.jsextensions for relative imports and@hmcts/*workspace aliases. Bothenandcylanguage objects are imported as per learnings.
88-106: Rendering and response logic is sound.The render options correctly pass locale, display dates, and last received date. Data source resolution via
PROVENANCE_LABELSwith fallback is appropriate.libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (2)
13-18: Multi-route export pattern is appropriate.Exporting multiple routes from a single handler for related list types reduces duplication whilst maintaining a consistent rendering flow.
119-130: View rendering looks correct.All required template variables are passed including both language objects, header, hearings, and list-specific content.
libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (1)
127-138: View rendering and error handling are solid.Template receives all necessary context including translations, header, hearings, and list content. Error handling follows established patterns.
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts (3)
1-11: Imports follow project conventions.ES modules with
.jsextensions on relative imports and workspace aliases for cross-package dependencies.
13-13: LGTM.Route constant follows the expected pattern per learnings.
88-106: Rendering logic is clean.The render call correctly passes locale, artefact metadata, and structured output to the template.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (2)
libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (1)
77-101: Malformed JSON returns 500 instead of 400.
JSON.parseon line 90 is outside the inner try-catch. Invalid JSON will throw an unhandled exception caught only by the outer block, returning a generic 500. Move parsing inside the try-catch to return a proper 400 for malformed data.libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (1)
85-109: Same JSON.parse issue—malformed JSON returns 500.Consistent with the other modules,
JSON.parseshould be moved inside the try-catch to return 400 for invalid JSON rather than 500.
🧹 Nitpick comments (17)
libs/list-types/rcj-standard-daily-cause-list/src/pages/court-of-appeal-criminal-division-daily-cause-list.njk (1)
29-32: Redundantaria-labelon input element.The input already has an associated
<label>element (line 29-31). The additionalaria-labelattribute on line 32 is redundant and can cause confusion for screen readers.♻️ Proposed fix
- <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text" aria-label="{{ common.searchCasesLabel }}"> + <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text">libs/list-types/common/package.json (1)
14-21: Consider cleaner export paths.The export paths
"./src/assets/js/*"require consumers to includesrc/in their imports, which is verbose. A cleaner pattern would be"./assets/js/*".♻️ Proposed cleaner exports
- "./src/assets/js/*": { + "./assets/js/*": { "production": "./dist/assets/js/*", - "default": "./src/assets/js/*" + "default": "./src/assets/js/*" }, - "./src/assets/css/*": { + "./assets/css/*": { "production": "./dist/assets/css/*", - "default": "./src/assets/css/*" + "default": "./src/assets/css/*" }This would allow imports like:
import { initTableSearch } from "@hmcts/list-types-common/assets/js/table-search";libs/list-types/common/src/assets/css/table-search.scss (1)
1-13: LGTM!Simple and clean layout classes. Consider using GOV.UK spacing scale variables (e.g.,
govuk-spacing(6)for 30px) for consistency with the design system, but this is optional.apps/web/src/assets/js/index.ts (1)
2-2: Import path pattern.Using
/src/assets/js/in the import path. This is consistent with other@hmcts/web-coreimports in this file, but consider exposing via a cleaner package export if refactoring later.libs/list-types/rcj-standard-daily-cause-list/src/pages/county-court-central-london-civil-daily-cause-list.njk (2)
30-33: Redundant accessibility attributes.The
<label>withgovuk-visually-hiddenand thearia-labelattribute on the input serve the same purpose. One is sufficient—typically the visible/hidden label is preferred.Suggested fix
<label class="govuk-label govuk-visually-hidden" for="case-search-input"> {{ common.searchCasesLabel }} </label> - <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text" aria-label="{{ common.searchCasesLabel }}"> + <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text">
37-37: Redundantrole="table".The
role="table"attribute is implicit on<table>elements and can be removed.libs/list-types/rcj-standard-daily-cause-list/src/pages/family-division-high-court-daily-cause-list.njk (1)
29-32: Minor: Redundantaria-labelwhen<label>is present.The visually-hidden
<label>already provides an accessible name for the input. Thearia-labelattribute duplicates this. Either approach works, but using both is redundant.Suggested fix
<label class="govuk-label govuk-visually-hidden" for="case-search-input"> {{ common.searchCasesLabel }} </label> - <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text" aria-label="{{ common.searchCasesLabel }}"> + <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text"> </div>libs/list-types/rcj-standard-daily-cause-list/src/pages/kings-bench-masters-daily-cause-list.njk (2)
1-72: Consider extracting shared template structure to reduce duplication.This template is nearly identical to
family-division-high-court-daily-cause-list.njkand other RCJ templates. The only differences are in thegovukDetailscontent block. A single parameterised template or shared partials for the table, search, and navigation sections would improve maintainability.
29-32: Minor: Redundantaria-labelwhen<label>is present.Same pattern as other templates - the visually-hidden label already provides accessibility. The
aria-labelis redundant.libs/list-types/administrative-court-daily-cause-list/src/pages/administrative-court-daily-cause-list.njk (2)
2-2: Remove unusedgovukTableimport.The
govukTablemacro is imported but not used in this template. The table is constructed manually instead.Suggested fix
{% extends "layouts/base-template.njk" %} -{% from "govuk/components/table/macro.njk" import govukTable %} {% from "govuk/components/details/macro.njk" import govukDetails %}
31-34: Minor: Redundantaria-labelwhen<label>is present.Same redundancy as other templates.
libs/list-types/rcj-standard-daily-cause-list/src/pages/civil-courts-rcj-daily-cause-list.njk (2)
21-25: HTML string concatenation in govukDetails may be fragile.The inline HTML construction using string concatenation could be error-prone and harder to maintain. Consider using a separate partial template or Nunjucks
setblocks for complex HTML content.
27-33: Accessible search input implementation.The search input correctly includes both a visually-hidden label for screen readers and an
aria-labelattribute. The duplicatearia-labelis redundant when the<label>is already associated viafor.Consider removing redundant aria-label
- <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text" aria-label="{{ common.searchCasesLabel }}"> + <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text">libs/list-types/rcj-standard-daily-cause-list/src/pages/kings-bench-division-daily-cause-list.njk (1)
21-25: Complex inline HTML construction reduces maintainability.The
govukDetailshtml parameter constructs a lengthy HTML string with multiple headings and paragraphs. This pattern is repeated across templates.Consider extracting to a reusable macro or partial:
{% macro renderInfoSection(title, text) %} <h3 class="govuk-heading-s govuk-!-margin-bottom-2">{{ title }}</h3> <p class="govuk-body">{{ text | replace('\n\n', '</p><p class="govuk-body">') | safe }}</p> {% endmacro %}libs/list-types/rcj-standard-daily-cause-list/src/pages/en.ts (1)
117-125: Minor casing inconsistency in tableHeaders.The table headers use sentence case (
Case number,Case details) whereas other list types (e.g.,administrative-court-daily-cause-list) use title case (Case Number,Case Details). Consider aligning for consistency across list types.Align with title case convention
tableHeaders: { venue: "Venue", judge: "Judge", time: "Time", - caseNumber: "Case number", - caseDetails: "Case details", - hearingType: "Hearing type", - additionalInformation: "Additional information" + caseNumber: "Case Number", + caseDetails: "Case Details", + hearingType: "Hearing Type", + additionalInformation: "Additional Information" },libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (1)
118-118: Avoidas anycast.Casting to
anybypasses type safety. Define a proper index signature or union type for the locale objects to avoid this.Proposed fix
- const listContent = (t as any)[listTypeId] || {}; + const listContent = t[listTypeId as keyof typeof t] || {};libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (1)
126-126: Avoidas anycast.Same issue as administrative court module. Use a proper type-safe index access instead.
Proposed fix
- const listContent = (t as any)[listTypeId] || {}; + const listContent = t[listTypeId as keyof typeof t] || {};
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ 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/assets/js/index.tsapps/web/vite.build.tslibs/list-types/administrative-court-daily-cause-list/package.jsonlibs/list-types/administrative-court-daily-cause-list/src/pages/administrative-court-daily-cause-list.njklibs/list-types/administrative-court-daily-cause-list/src/pages/cy.tslibs/list-types/administrative-court-daily-cause-list/src/pages/en.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/cy.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/en.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/list-types/common/package.jsonlibs/list-types/common/src/assets/css/table-search.scsslibs/list-types/common/src/assets/js/table-search.tslibs/list-types/common/src/config.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/views/court-of-appeal-civil-daily-cause-list/civil-appeal.njklibs/list-types/london-administrative-court-daily-cause-list/src/views/london-administrative-court-daily-cause-list/london-admin-court.njklibs/list-types/rcj-standard-daily-cause-list/package.jsonlibs/list-types/rcj-standard-daily-cause-list/src/pages/civil-courts-rcj-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/county-court-central-london-civil-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/court-of-appeal-criminal-division-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/cy.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/en.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/family-division-high-court-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/kings-bench-division-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/kings-bench-masters-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/mayor-city-civil-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/senior-courts-costs-office-daily-cause-list.njklibs/web-core/src/assets/js/search-highlight.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- libs/list-types/administrative-court-daily-cause-list/src/pages/en.ts
- libs/list-types/rcj-standard-daily-cause-list/package.json
- libs/list-types/administrative-court-daily-cause-list/package.json
- libs/list-types/court-of-appeal-civil-daily-cause-list/src/views/court-of-appeal-civil-daily-cause-list/civil-appeal.njk
- libs/list-types/london-administrative-court-daily-cause-list/src/views/london-administrative-court-daily-cause-list/london-admin-court.njk
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use camelCase for TypeScript variables:userId,caseDetails,documentId
Use PascalCase for classes and interfaces:UserService,CaseRepository. NOIprefix.
Use kebab-case for file and directory names:user-service.ts,case-management/
Use boolean variable names withis/has/canprefix:isActive,hasAccess,canEdit
Module ordering: constants at the top, exported functions next, other functions in order of use, interfaces and types at the bottom
Always add.jsextension to relative imports in ES modules (e.g.,import { foo } from "./bar.js")
Use workspace aliases for imports (@hmcts/*) instead of relative paths across packages
Use strict TypeScript mode enabled; noanywithout justification
Use parameterized database queries with Prisma (no raw SQL string concatenation)
Do not include sensitive data in logs
Files:
libs/list-types/common/src/config.tsapps/web/src/assets/js/index.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/en.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/cy.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/web-core/src/assets/js/search-highlight.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/en.tsapps/web/vite.build.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/list-types/common/src/assets/js/table-search.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/cy.tslibs/list-types/administrative-court-daily-cause-list/src/pages/cy.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use SCREAMING_SNAKE_CASE for constants:
MAX_FILE_SIZE,DEFAULT_TIMEOUT
Files:
libs/list-types/common/src/config.tsapps/web/src/assets/js/index.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/en.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/cy.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/web-core/src/assets/js/search-highlight.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/en.tsapps/web/vite.build.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/list-types/common/src/assets/js/table-search.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/cy.tslibs/list-types/administrative-court-daily-cause-list/src/pages/cy.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts
**/config.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Module config.ts must export standardized interfaces: pageRoutes, apiRoutes, prismaSchemas, assets
Files:
libs/list-types/common/src/config.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Input validation must be performed on all endpoints
Files:
libs/list-types/common/src/config.tsapps/web/src/assets/js/index.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/en.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/cy.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/web-core/src/assets/js/search-highlight.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/en.tsapps/web/vite.build.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/list-types/common/src/assets/js/table-search.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/cy.tslibs/list-types/administrative-court-daily-cause-list/src/pages/cy.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not use CommonJS - ES modules only with import/export syntax
Files:
libs/list-types/common/src/config.tsapps/web/src/assets/js/index.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/en.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/cy.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/web-core/src/assets/js/search-highlight.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/en.tsapps/web/vite.build.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/list-types/common/src/assets/js/table-search.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/cy.tslibs/list-types/administrative-court-daily-cause-list/src/pages/cy.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts
**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
**/package.json: Use@hmctsscope for package names:@hmcts/auth,@hmcts/case-management
All package.json files must use"type": "module"for ES modules support
Express version must be 5.x only ("express": "5.2.0")
All packages must use"test": "vitest run"script in package.json
Dependencies must use specific versions only (e.g.,"express": "5.2.0"), except for peer dependencies
Module build script must include"build:nunjucks"script if module contains Nunjucks templates in pages/ directory
Files:
apps/web/package.jsonlibs/list-types/common/package.json
🧠 Learnings (12)
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/config.ts : Module config.ts must export standardized interfaces: pageRoutes, apiRoutes, prismaSchemas, assets
Applied to files:
libs/list-types/common/src/config.tsapps/web/vite.build.tslibs/list-types/common/package.json
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/package.json : Use hmcts scope for package names: `hmcts/auth`, `hmcts/case-management`
Applied to files:
apps/web/package.json
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/*.{ts,tsx} : Use workspace aliases for imports (`hmcts/*`) instead of relative paths across packages
Applied to files:
apps/web/package.jsonlibs/list-types/common/package.json
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.ts : Provide both `en` and `cy` language objects in page controllers for English and Welsh support
Applied to files:
libs/list-types/civil-and-family-daily-cause-list/src/pages/en.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/cy.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/en.tslibs/list-types/civil-and-family-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/cy.tslibs/list-types/administrative-court-daily-cause-list/src/pages/cy.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.njk : Nunjucks templates must extend 'layouts/base-templates.njk' and use GOV.UK Frontend component macros
Applied to files:
libs/list-types/rcj-standard-daily-cause-list/src/pages/court-of-appeal-criminal-division-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/mayor-city-civil-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/civil-courts-rcj-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/kings-bench-division-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/county-court-central-london-civil-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/senior-courts-costs-office-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/family-division-high-court-daily-cause-list.njklibs/list-types/administrative-court-daily-cause-list/src/pages/administrative-court-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/kings-bench-masters-daily-cause-list.njk
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.ts : Create page controller files with GET and POST exports following the pattern: `export const GET = async (req, res) => { ... }`
Applied to files:
libs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/routes/**/*.ts : API endpoints should use plural for collections (/api/cases), singular for specific (/api/case/:id), and singular for creation (POST /api/case)
Applied to files:
libs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/*.{ts,tsx} : Always add `.js` extension to relative imports in ES modules (e.g., `import { foo } from "./bar.js"`)
Applied to files:
libs/list-types/common/package.json
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/*.{ts,tsx,js,mjs} : Do not use CommonJS - ES modules only with import/export syntax
Applied to files:
libs/list-types/common/package.json
📚 Learning: 2025-12-19T15:19:47.640Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 192
File: libs/list-types/common/src/mock-list-types.ts:93-110
Timestamp: 2025-12-19T15:19:47.640Z
Learning: For Single Justice Procedure (SJP) list types in libs/list-types/common/src/mock-list-types.ts, the Welsh translation "Gweithdrefn Ynad Sengl" is the preferred terminology for "Single Justice Procedure" rather than "Gweithdrefn Cyfiawnder Sengl".
Applied to files:
libs/list-types/rcj-standard-daily-cause-list/src/pages/cy.tslibs/list-types/administrative-court-daily-cause-list/src/pages/cy.ts
📚 Learning: 2025-11-20T09:59:16.776Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 106
File: libs/system-admin-pages/src/pages/reference-data-upload/index.test.ts:84-160
Timestamp: 2025-11-20T09:59:16.776Z
Learning: In the cath-service repository, Welsh localization (lng=cy) is not required for admin screens (system-admin-pages), so locale preservation in admin screen redirects is not necessary.
Applied to files:
libs/list-types/rcj-standard-daily-cause-list/src/pages/cy.tslibs/list-types/administrative-court-daily-cause-list/src/pages/cy.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Welsh translations are required for all user-facing text
Applied to files:
libs/list-types/rcj-standard-daily-cause-list/src/pages/cy.ts
🧬 Code graph analysis (6)
libs/list-types/common/src/config.ts (2)
libs/list-types/civil-and-family-daily-cause-list/src/config.ts (1)
assets(12-12)e2e-tests/run-with-credentials.js (1)
__dirname(11-11)
apps/web/src/assets/js/index.ts (1)
libs/list-types/common/src/assets/js/table-search.ts (1)
initTableSearch(5-26)
libs/list-types/rcj-standard-daily-cause-list/src/pages/en.ts (4)
libs/list-types/administrative-court-daily-cause-list/src/pages/en.ts (1)
en(1-53)libs/list-types/civil-and-family-daily-cause-list/src/pages/en.ts (1)
en(1-39)libs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/en.ts (1)
en(1-22)libs/list-types/london-administrative-court-daily-cause-list/src/pages/en.ts (1)
en(1-31)
apps/web/vite.build.ts (2)
libs/web-core/src/index.ts (1)
createBaseViteConfig(4-4)libs/web-core/src/assets/vite-config.ts (1)
createBaseViteConfig(11-70)
libs/list-types/rcj-standard-daily-cause-list/src/pages/cy.ts (4)
libs/list-types/administrative-court-daily-cause-list/src/pages/cy.ts (1)
cy(1-32)libs/list-types/civil-and-family-daily-cause-list/src/pages/cy.ts (1)
cy(1-40)libs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/cy.ts (1)
cy(1-22)libs/list-types/london-administrative-court-daily-cause-list/src/pages/cy.ts (1)
cy(1-22)
libs/list-types/administrative-court-daily-cause-list/src/pages/cy.ts (3)
libs/list-types/civil-and-family-daily-cause-list/src/pages/cy.ts (1)
cy(1-40)libs/list-types/rcj-standard-daily-cause-list/src/pages/cy.ts (1)
cy(1-115)libs/list-types/london-administrative-court-daily-cause-list/src/pages/cy.ts (1)
cy(1-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests
🔇 Additional comments (28)
libs/list-types/civil-and-family-daily-cause-list/src/pages/cy.ts (1)
1-3: LGTM!Welsh title addition is consistent with the existing
pageTitlepattern and aligns with the English translation structure. Based on learnings, bothenandcyobjects are correctly maintained.libs/list-types/civil-and-family-daily-cause-list/src/pages/en.ts (1)
1-3: LGTM!English title addition follows the established pattern and maintains symmetry with the Welsh translation file.
libs/list-types/civil-and-family-daily-cause-list/src/pages/index.ts (1)
84-93: LGTM!The
titleproperty is correctly extracted from the locale-specific translations and passed to the render context. This provides clean access for the template whilst maintaining the fulltobject for other translations.apps/web/package.json (1)
26-26: LGTM!The workspace dependency follows the
@hmctsscope convention and integrates correctly with the existing dependency structure.libs/web-core/src/assets/js/search-highlight.ts (1)
9-14: Fallback strategy is well-implemented.The two-step resolution with ID-based containers taking precedence over class-based tables is a sensible approach. Comments clearly document the intent.
libs/list-types/rcj-standard-daily-cause-list/src/pages/court-of-appeal-criminal-division-daily-cause-list.njk (2)
35-62: Table structure follows GOV.UK patterns.Proper semantic markup with
scope="col"headers and accessible table attributes.
21-25: The XSS concern aboutquickGuideLinkUrlis not applicable here.
quickGuideLinkUrlis a static hardcoded configuration value inen.tsandcy.ts(an HTTPS URL to the judiciary.uk domain), not user-controlled input. There is no injection vector. Whilst thehtmlparameter does bypass auto-escaping, this is only a concern when the parameter contains dynamic or user-supplied content, which is not the case.Likely an incorrect or invalid review comment.
libs/list-types/common/package.json (1)
1-41: Package configuration follows conventions.ES module type,
@hmctsscope, vitest test script, and pinned dependency versions all align with guidelines.libs/list-types/common/src/assets/js/table-search.ts (1)
10-15: Rows captured at init time.
tableRowsis queried once during initialisation. If rows are dynamically added/removed after init, they won't be included in filtering. If dynamic content is expected, consider querying rows inside the event handler.apps/web/vite.build.ts (1)
4-4: LGTM!Asset integration follows the established pattern. Import and registration are correctly aligned with existing modules.
Also applies to: 15-22
apps/web/src/assets/js/index.ts (1)
20-20: LGTM!Correctly initialised in both DOM-ready and immediate execution paths.
Also applies to: 29-29
libs/list-types/rcj-standard-daily-cause-list/src/pages/county-court-central-london-civil-daily-cause-list.njk (1)
1-73: LGTM overall.Template correctly extends the base layout, uses GOV.UK macros, and follows the established RCJ standard pattern. Structure and data bindings look correct. Based on learnings, the template appropriately extends
layouts/base-template.njk.libs/list-types/administrative-court-daily-cause-list/src/pages/cy.ts (1)
1-32: LGTM!Welsh localisation structure is well-organised with appropriate translations for the four Administrative Court locations (Birmingham, Leeds, Bristol/Cardiff, Manchester). The common block provides consistent UI strings for table headers and navigation elements.
Based on learnings, this correctly provides the
cycounterpart for Welsh language support.libs/list-types/administrative-court-daily-cause-list/src/pages/administrative-court-daily-cause-list.njk (1)
20-27: Good conditional rendering for list-type-specific content.The conditional logic correctly targets the Administrative Court locations (20-23) for displaying important information. The guard for
common.importantInfoTitleensures graceful handling when content is absent.libs/list-types/rcj-standard-daily-cause-list/src/pages/mayor-city-civil-daily-cause-list.njk (1)
1-73: LGTM with existing duplication concerns.Template follows established patterns. Same template consolidation opportunity applies here as noted for other RCJ templates.
libs/list-types/rcj-standard-daily-cause-list/src/pages/family-division-high-court-daily-cause-list.njk (1)
21-25: The XSS concern is not applicable here. ThelistContentvalues are hardcoded static strings from locale files (en.ts/cy.ts), not user-controlled data, so there is no injection vulnerability.However, the maintainability point stands. The long HTML concatenation on line 23 remains difficult to read. Consider extracting this to a Nunjucks partial or macro for clarity, though this is optional.
Likely an incorrect or invalid review comment.
libs/list-types/rcj-standard-daily-cause-list/src/pages/civil-courts-rcj-daily-cause-list.njk (2)
35-62: Table structure is semantically correct.The hearings table uses proper GOV.UK table classes,
scope="col"on headers, and appropriate ARIA attributes. The iteration overhearingsis straightforward.
1-2: Template extends correct base layout and uses GOV.UK macros appropriately.The template correctly extends
layouts/base-template.njkand imports thegovukDetailsmacro. The template structure follows GOV.UK Frontend patterns correctly with proper accessibility attributes.Likely an incorrect or invalid review comment.
libs/list-types/rcj-standard-daily-cause-list/src/pages/cy.ts (1)
1-115: Welsh localisation structure matches English counterpart.The
cyexport provides comprehensive Welsh translations for all RCJ Standard Daily Cause List pages (keys 10-17) with a sharedcommonblock. The structure aligns with theen.tsfile and follows established patterns from other list types.libs/list-types/rcj-standard-daily-cause-list/src/pages/kings-bench-division-daily-cause-list.njk (1)
35-62: LGTM.Table structure and hearing iteration are correctly implemented with proper GOV.UK styling and accessibility attributes.
libs/list-types/rcj-standard-daily-cause-list/src/pages/senior-courts-costs-office-daily-cause-list.njk (1)
1-72: Template follows established pattern.This template is structurally consistent with the other RCJ daily cause list templates, with template-specific content in the
govukDetailsblock. The repetition across templates suggests an opportunity for a shared base template with content slots, though this can be deferred.libs/list-types/rcj-standard-daily-cause-list/src/pages/en.ts (1)
1-129: Comprehensive English localisation for RCJ list types.The localisation structure is well-organised with court-specific content (keys 10-17) and shared common strings. The content covers locations, contact information, and guidance for multiple RCJ court types.
libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (3)
1-11: Imports look good.ES modules with
.jsextensions for relative imports and workspace aliases (@hmcts/*) for cross-package imports follow the coding guidelines correctly.
13-32: Constants and configuration well-structured.
ROUTESandLIST_TYPE_CONFIGfollow SCREAMING_SNAKE_CASE convention. The locale mapping for page titles is clean.
120-131: Rendering and response logic is correct.The template receives all necessary data including localised content and follows the established pattern.
libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (3)
1-11: Imports follow guidelines.ES modules,
.jsextensions for relative imports, and workspace aliases used correctly.
13-40: ROUTES and LIST_TYPE_CONFIG well-defined.Constants use SCREAMING_SNAKE_CASE. The mapping of list type IDs to locale strings and templates is clear.
128-139: Template rendering follows established pattern.Data passed to the template is complete and consistent with other modules.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| }); | ||
| } | ||
|
|
||
| const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`); |
There was a problem hiding this comment.
Path traversal risk: sanitise artefactId before file access.
User-supplied artefactId is interpolated directly into the file path. A malicious value such as ../../etc/passwd could read arbitrary files. Validate or sanitise the identifier to contain only expected characters (e.g., alphanumeric/UUID).
Proposed fix
+ // Validate artefactId format (UUID expected)
+ const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
+ if (!UUID_REGEX.test(artefactId)) {
+ return res.status(400).render("errors/common", {
+ en,
+ cy,
+ errorTitle: "Bad Request",
+ errorMessage: "Invalid artefactId format"
+ });
+ }
+
const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`);📝 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 jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`); | |
| // Validate artefactId format (UUID expected) | |
| const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; | |
| if (!UUID_REGEX.test(artefactId)) { | |
| return res.status(400).render("errors/common", { | |
| en, | |
| cy, | |
| errorTitle: "Bad Request", | |
| errorMessage: "Invalid artefactId format" | |
| }); | |
| } | |
| const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`); |
| const __dirname = path.dirname(__filename); | ||
|
|
||
| export const moduleRoot = __dirname; | ||
| export const assets = path.join(__dirname, "assets/"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check assets folder locations in list-types modules
echo "=== Checking for assets directories in list-types ==="
find libs/list-types -type d -name "assets" 2>/dev/null | sort
echo ""
echo "=== Checking config.ts files and their assets exports ==="
find libs/list-types -name "config.ts" -type f | while read file; do
echo "--- $file ---"
grep -n "export const assets" "$file" 2>/dev/null || echo " (no assets export found)"
done
echo ""
echo "=== Checking if libs/list-types/common/src/assets exists ==="
ls -la libs/list-types/common/src/assets/ 2>&1 || echo "Directory does not exist"
echo ""
echo "=== Full content of libs/list-types/common/src/config.ts ==="
cat -n libs/list-types/common/src/config.ts 2>/dev/null || echo "File not found"Repository: hmcts/cath-service
Length of output: 1775
Address inconsistent assets path patterns across list-types modules.
The assets folder correctly exists at libs/list-types/common/src/assets/ and the path resolves properly. However, there is an inconsistency in how assets are referenced across modules: care-standards-tribunal-weekly-hearing-list and civil-and-family-daily-cause-list use ../assets/, whilst most other modules (including this one) use assets/. This divergence suggests either different folder structures or inconsistent conventions. Standardise the path pattern across all config.ts files.
| const locale = res.locals.locale || "en"; | ||
| const t = locale === "cy" ? cy : en; | ||
|
|
||
| const artefactId = req.query.artefactId as string; |
There was a problem hiding this comment.
Validate artefactId type before use.
Same issue as the administrative court module: req.query.artefactId can be string | string[] | undefined. The cast silently allows arrays through.
Proposed fix
- const artefactId = req.query.artefactId as string;
+ const artefactId = typeof req.query.artefactId === "string" ? req.query.artefactId : undefined;📝 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 artefactId = req.query.artefactId as string; | |
| const artefactId = typeof req.query.artefactId === "string" ? req.query.artefactId : undefined; |
| }); | ||
| } | ||
|
|
||
| const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`); |
There was a problem hiding this comment.
Path traversal risk: sanitise artefactId before file access.
User-supplied artefactId is interpolated directly into the file path without validation. This allows potential directory traversal attacks. Validate the format (e.g., UUID regex) before constructing the path.
Proposed fix
+ // Validate artefactId format (UUID expected)
+ const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
+ if (!UUID_REGEX.test(artefactId)) {
+ return res.status(400).render("errors/common", {
+ en,
+ cy,
+ errorTitle: "Bad Request",
+ errorMessage: "Invalid artefactId format"
+ });
+ }
+
const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`);📝 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 jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`); | |
| // Validate artefactId format (UUID expected) | |
| const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; | |
| if (!UUID_REGEX.test(artefactId)) { | |
| return res.status(400).render("errors/common", { | |
| en, | |
| cy, | |
| errorTitle: "Bad Request", | |
| errorMessage: "Invalid artefactId format" | |
| }); | |
| } | |
| const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`); |
| const containers = [document.getElementById("hearings-table-container"), document.getElementById("court-lists-container")].filter( | ||
|
|
||
| // Look for containers by ID first, then fall back to class-based tables | ||
| const containersById = [document.getElementById("hearings-table-container"), document.getElementById("court-lists-container")].filter( |
There was a problem hiding this comment.
Typo: "heatings" should be "hearings".
The ID "heatings-table-container" contains a typo and won't match the actual element "hearings-table-container" in the templates.
🐛 Proposed fix
- const containersById = [document.getElementById("heatings-table-container"), document.getElementById("court-lists-container")].filter(
+ const containersById = [document.getElementById("hearings-table-container"), document.getElementById("court-lists-container")].filter(📝 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 containersById = [document.getElementById("hearings-table-container"), document.getElementById("court-lists-container")].filter( | |
| const containersById = [document.getElementById("hearings-table-container"), document.getElementById("court-lists-container")].filter( |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
libs/list-types/london-administrative-court-daily-cause-list/src/pages/london-administrative-court-daily-cause-list.njk (2)
30-33: Redundant aria-label on search input.The
<label>element (even when visually hidden) already provides an accessible name for the input. Thearia-labelattribute is redundant and can be removed.Suggested fix
<label class="govuk-label govuk-visually-hidden" for="case-search-input"> {{ t.searchCasesLabel }} </label> - <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text" aria-label="{{ t.searchCasesLabel }}"> + <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text"> </div>
40-65: Consider extracting duplicated table markup into a Nunjucks macro.The table structure for Main Hearings and Planning Court is identical. Extracting this into a reusable macro would reduce duplication and ease future maintenance.
Example macro extraction
{% macro hearingsTable(hearings, ariaLabel) %} <table class="govuk-table hearings-table" role="table" aria-label="{{ ariaLabel }}"> <thead class="govuk-table__head"> <tr class="govuk-table__row"> <th scope="col" class="govuk-table__header">{{ t.tableHeaders.venue }}</th> <th scope="col" class="govuk-table__header">{{ t.tableHeaders.judge }}</th> <th scope="col" class="govuk-table__header">{{ t.tableHeaders.time }}</th> <th scope="col" class="govuk-table__header">{{ t.tableHeaders.caseNumber }}</th> <th scope="col" class="govuk-table__header">{{ t.tableHeaders.caseDetails }}</th> <th scope="col" class="govuk-table__header">{{ t.tableHeaders.hearingType }}</th> <th scope="col" class="govuk-table__header">{{ t.tableHeaders.additionalInformation }}</th> </tr> </thead> <tbody class="govuk-table__body"> {% for hearing in hearings %} <tr class="govuk-table__row"> <td class="govuk-table__cell">{{ hearing.venue }}</td> <td class="govuk-table__cell">{{ hearing.judge }}</td> <td class="govuk-table__cell">{{ hearing.time }}</td> <td class="govuk-table__cell">{{ hearing.caseNumber }}</td> <td class="govuk-table__cell">{{ hearing.caseDetails }}</td> <td class="govuk-table__cell">{{ hearing.hearingType }}</td> <td class="govuk-table__cell">{{ hearing.additionalInformation }}</td> </tr> {% endfor %} </tbody> </table> {% endmacro %}Usage:
{{ hearingsTable(mainHearings, t.mainHearingsTitle) }} {{ hearingsTable(planningCourt, t.planningCourtTitle) }}Also applies to: 79-104
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/court-of-appeal-civil-daily-cause-list.njk (3)
22-26: Consider extracting complex HTML to a partial or variable.The inline HTML concatenation in
govukDetailsis lengthy and difficult to maintain. For improved readability, consider constructing this HTML in a{% set %}block or extracting it to a partial template.♻️ Suggested improvement
+{% set importantInfoHtml %} + <h3 class="govuk-heading-s govuk-!-margin-bottom-2">{{ t.liveStreamingTitle }}</h3> + <p class="govuk-body">{{ t.liveStreamingText1 }} <a href="{{ t.liveStreamingLinkUrl }}" class="govuk-link">{{ t.liveStreamingLinkText }}</a>.</p> + <p class="govuk-body">{{ t.liveStreamingText2a }} <a href="{{ t.liveStreamingLinkUrl }}" class="govuk-link">{{ t.liveStreamingLinkText }}</a>{{ t.liveStreamingText2b }}</p> + <h3 class="govuk-heading-s govuk-!-margin-top-6 govuk-!-margin-bottom-2">{{ t.judgmentsTitle }}</h3> + <p class="govuk-body">{{ t.judgmentsText }}</p> +{% endset %} + {{ govukDetails({ summaryText: t.importantInfoTitle, - html: '<h3 class="govuk-heading-s govuk-!-margin-bottom-2">' + t.liveStreamingTitle + '</h3><p class="govuk-body">' + t.liveStreamingText1 + ' <a href="' + t.liveStreamingLinkUrl + '" class="govuk-link">' + t.liveStreamingLinkText + '</a>.</p><p class="govuk-body">' + t.liveStreamingText2a + ' <a href="' + t.liveStreamingLinkUrl + '" class="govuk-link">' + t.liveStreamingLinkText + '</a>' + t.liveStreamingText2b + '</p><h3 class="govuk-heading-s govuk-!-margin-top-6 govuk-!-margin-bottom-2">' + t.judgmentsTitle + '</h3><p class="govuk-body">' + t.judgmentsText + '</p>', + html: importantInfoHtml, open: true }) }}
28-34: Search input has redundantaria-labeland no associated action.The
aria-labelis unnecessary when there's already a properly associated<label>element. Additionally, the search input lacks a form wrapper or button—verify that client-side JavaScript handles the search functionality.♻️ Remove redundant aria-label
- <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text" aria-label="{{ t.searchCasesLabel }}"> + <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text">
40-40: Redundantrole="table"attribute.The
role="table"attribute is implicit for<table>elements and can be removed unless there's a specific accessibility requirement.♻️ Remove redundant role attribute
- <table class="govuk-table hearings-table" role="table" aria-label="{{ t.dailyHearingsTitle }}"> + <table class="govuk-table hearings-table" aria-label="{{ t.dailyHearingsTitle }}">- <table class="govuk-table hearings-table" role="table" aria-label="{{ t.futureJudgmentsTitle }}"> + <table class="govuk-table hearings-table" aria-label="{{ t.futureJudgmentsTitle }}">Also applies to: 79-79
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.tslibs/list-types/civil-and-family-daily-cause-list/src/config.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/court-of-appeal-civil-daily-cause-list.njklibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/package.jsonlibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/london-administrative-court-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/config.ts
💤 Files with no reviewable changes (2)
- libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.ts
- libs/list-types/civil-and-family-daily-cause-list/src/config.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.ts
- libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts
- libs/list-types/london-administrative-court-daily-cause-list/package.json
- libs/list-types/rcj-standard-daily-cause-list/src/config.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.njk : Nunjucks templates must extend 'layouts/base-templates.njk' and use GOV.UK Frontend component macros
Applied to files:
libs/list-types/london-administrative-court-daily-cause-list/src/pages/london-administrative-court-daily-cause-list.njklibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/court-of-appeal-civil-daily-cause-list.njk
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests
🔇 Additional comments (4)
libs/list-types/london-administrative-court-daily-cause-list/src/pages/london-administrative-court-daily-cause-list.njk (2)
36-72: Template structure and GOV.UK compliance look good.The conditional rendering for empty states, proper use of GOV.UK Design System classes, semantic table markup with appropriate
scopeattributes, and section structure are all well implemented. Based on learnings, this correctly uses GOV.UK Frontend component macros for the details component.Also applies to: 74-112
1-3: Remove unusedgovukTableimport.The
govukTablemacro is imported but never used in the template — tables are built with raw HTML instead. Remove this import to keep dependencies clean.The base template filename is correct and follows the actual template structure in the codebase.
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/court-of-appeal-civil-daily-cause-list.njk (2)
74-114: Future Judgments section looks good.The structure correctly includes the date column, section heading in both populated and empty states, and follows GOV.UK table patterns.
116-120: Data source and navigation elements are correctly implemented.The back-to-top link correctly references the
#topanchor defined on the heading.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| {# Daily Hearings Section #} | ||
| {% if dailyHearings.length > 0 %} | ||
| <div class="hearings-section" id="daily-hearings-section"> | ||
| <div id="daily-hearings-table-container"> | ||
| <table class="govuk-table hearings-table" role="table" aria-label="{{ t.dailyHearingsTitle }}"> | ||
| <thead class="govuk-table__head"> | ||
| <tr class="govuk-table__row"> | ||
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.venue }}</th> | ||
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.judge }}</th> | ||
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.time }}</th> | ||
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.caseNumber }}</th> | ||
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.caseDetails }}</th> | ||
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.hearingType }}</th> | ||
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.additionalInformation }}</th> | ||
| </tr> | ||
| </thead> | ||
| <tbody class="govuk-table__body"> | ||
| {% for hearing in dailyHearings %} | ||
| <tr class="govuk-table__row"> | ||
| <td class="govuk-table__cell">{{ hearing.venue }}</td> | ||
| <td class="govuk-table__cell">{{ hearing.judge }}</td> | ||
| <td class="govuk-table__cell">{{ hearing.time }}</td> | ||
| <td class="govuk-table__cell">{{ hearing.caseNumber }}</td> | ||
| <td class="govuk-table__cell">{{ hearing.caseDetails }}</td> | ||
| <td class="govuk-table__cell">{{ hearing.hearingType }}</td> | ||
| <td class="govuk-table__cell">{{ hearing.additionalInformation }}</td> | ||
| </tr> | ||
| {% endfor %} | ||
| </tbody> | ||
| </table> | ||
| </div> | ||
| </div> | ||
| {% else %} | ||
| <div class="hearings-section"> | ||
| <p class="govuk-body">{{ t.noHearingsMessage }}</p> | ||
| </div> | ||
| {% endif %} |
There was a problem hiding this comment.
Inconsistent section heading when no hearings are present.
When dailyHearings is empty, no section heading is displayed (line 69-71). However, the empty state for futureJudgments (line 110-113) includes the section heading. Consider adding a heading for consistency, particularly for screen reader users who benefit from consistent page structure.
🔧 Suggested fix
{% else %}
<div class="hearings-section">
+ <h2 class="govuk-heading-m">{{ t.dailyHearingsTitle }}</h2>
<p class="govuk-body">{{ t.noHearingsMessage }}</p>
</div>
{% endif %}📝 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.
| {# Daily Hearings Section #} | |
| {% if dailyHearings.length > 0 %} | |
| <div class="hearings-section" id="daily-hearings-section"> | |
| <div id="daily-hearings-table-container"> | |
| <table class="govuk-table hearings-table" role="table" aria-label="{{ t.dailyHearingsTitle }}"> | |
| <thead class="govuk-table__head"> | |
| <tr class="govuk-table__row"> | |
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.venue }}</th> | |
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.judge }}</th> | |
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.time }}</th> | |
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.caseNumber }}</th> | |
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.caseDetails }}</th> | |
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.hearingType }}</th> | |
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.additionalInformation }}</th> | |
| </tr> | |
| </thead> | |
| <tbody class="govuk-table__body"> | |
| {% for hearing in dailyHearings %} | |
| <tr class="govuk-table__row"> | |
| <td class="govuk-table__cell">{{ hearing.venue }}</td> | |
| <td class="govuk-table__cell">{{ hearing.judge }}</td> | |
| <td class="govuk-table__cell">{{ hearing.time }}</td> | |
| <td class="govuk-table__cell">{{ hearing.caseNumber }}</td> | |
| <td class="govuk-table__cell">{{ hearing.caseDetails }}</td> | |
| <td class="govuk-table__cell">{{ hearing.hearingType }}</td> | |
| <td class="govuk-table__cell">{{ hearing.additionalInformation }}</td> | |
| </tr> | |
| {% endfor %} | |
| </tbody> | |
| </table> | |
| </div> | |
| </div> | |
| {% else %} | |
| <div class="hearings-section"> | |
| <p class="govuk-body">{{ t.noHearingsMessage }}</p> | |
| </div> | |
| {% endif %} | |
| {# Daily Hearings Section #} | |
| {% if dailyHearings.length > 0 %} | |
| <div class="hearings-section" id="daily-hearings-section"> | |
| <div id="daily-hearings-table-container"> | |
| <table class="govuk-table hearings-table" role="table" aria-label="{{ t.dailyHearingsTitle }}"> | |
| <thead class="govuk-table__head"> | |
| <tr class="govuk-table__row"> | |
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.venue }}</th> | |
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.judge }}</th> | |
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.time }}</th> | |
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.caseNumber }}</th> | |
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.caseDetails }}</th> | |
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.hearingType }}</th> | |
| <th scope="col" class="govuk-table__header">{{ t.tableHeaders.additionalInformation }}</th> | |
| </tr> | |
| </thead> | |
| <tbody class="govuk-table__body"> | |
| {% for hearing in dailyHearings %} | |
| <tr class="govuk-table__row"> | |
| <td class="govuk-table__cell">{{ hearing.venue }}</td> | |
| <td class="govuk-table__cell">{{ hearing.judge }}</td> | |
| <td class="govuk-table__cell">{{ hearing.time }}</td> | |
| <td class="govuk-table__cell">{{ hearing.caseNumber }}</td> | |
| <td class="govuk-table__cell">{{ hearing.caseDetails }}</td> | |
| <td class="govuk-table__cell">{{ hearing.hearingType }}</td> | |
| <td class="govuk-table__cell">{{ hearing.additionalInformation }}</td> | |
| </tr> | |
| {% endfor %} | |
| </tbody> | |
| </table> | |
| </div> | |
| </div> | |
| {% else %} | |
| <div class="hearings-section"> | |
| <h2 class="govuk-heading-m">{{ t.dailyHearingsTitle }}</h2> | |
| <p class="govuk-body">{{ t.noHearingsMessage }}</p> | |
| </div> | |
| {% endif %} |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (7)
libs/list-types/court-of-appeal-civil-daily-cause-list/package.json (1)
16-18: Missing views copy in build script.This was flagged previously. The
build:nunjucksscript only copies templates fromsrc/pages/but may be missingsrc/views/if this module uses views like similar packages in the repository.libs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/court-of-appeal-civil-daily-cause-list-config.ts (2)
134-134: Redundant null check after throwing error.Line 129-131 already throws if
dailyHearingsSheetis falsy, making the ternary on line 134 unnecessary.🔧 Suggested fix
- const dailyHearings = dailyHearingsSheet ? await convertSheetToJson(dailyHearingsSheet, DAILY_HEARINGS_CONFIG) : []; + const dailyHearings = await convertSheetToJson(dailyHearingsSheet, DAILY_HEARINGS_CONFIG);
143-143: Improve type safety for worksheet parameter.Using
anyreduces compile-time safety. Consider usingExcelJS.Worksheettype for better type checking.libs/list-types/administrative-court-daily-cause-list/src/conversion/administrative-court-daily-cause-list-config.ts (1)
64-65: Ensure converters are registered at startup.This side-effect registration only executes when the module is imported. As previously flagged, verify the app entry point imports this module (or the package's main entry) so converters for list types 20–23 are available.
libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (3)
40-40: ValidateartefactIdtype before use.
req.query.artefactIdcan bestring | string[] | undefined. The cast silently allows arrays through.Proposed fix
- const artefactId = req.query.artefactId as string; + const artefactId = typeof req.query.artefactId === "string" ? req.query.artefactId : undefined;
77-77: Path traversal risk: sanitiseartefactIdbefore file access.User-supplied
artefactIdis interpolated directly into the file path. Validate it contains only expected characters (e.g., UUID format).Proposed fix
+ const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + if (!UUID_REGEX.test(artefactId)) { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Bad Request", + errorMessage: "Invalid artefactId format" + }); + } + const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`);
92-103: Malformed JSON returns 500 instead of 400.
JSON.parseis outside the inner try-catch. Move it inside to return a proper 400 for invalid JSON.Proposed fix
let jsonContent: string; + let jsonData: StandardHearingList; try { jsonContent = await readFile(jsonFilePath, "utf-8"); + jsonData = JSON.parse(jsonContent); } catch (error) { - console.error(`Error reading JSON file at ${jsonFilePath}:`, error); + console.error(`Error reading or parsing JSON file for artefact ${artefactId}:`, error); return res.status(404).render("errors/common", { en, cy, errorTitle: "Not Found", errorMessage: "The requested list could not be found" }); } - - const jsonData: StandardHearingList = JSON.parse(jsonContent);
🧹 Nitpick comments (10)
libs/list-types/common/package.json (1)
14-21: Consider cleaner export path keys.The export keys expose the internal
srcdirectory structure (e.g.,./src/assets/js/*). A cleaner pattern would use./assets/js/*as the key whilst still mapping to the appropriate paths:"./assets/js/*": { "production": "./dist/assets/js/*", "default": "./src/assets/js/*" }This keeps the public API tidy without leaking internal structure. However, if this pattern is deliberate for consistency with how assets are referenced elsewhere, feel free to keep as-is.
libs/list-types/london-administrative-court-daily-cause-list/src/schemas/london-administrative-court-daily-cause-list.json (1)
8-44: Consider extracting the shared item schema using$defs.The
mainHearingsandplanningCourtarrays have identical item definitions. Draft-07 supportsdefinitions(or$defsin later drafts) to reduce duplication and simplify future maintenance.Example refactor using definitions
{ "$schema": "http://json-schema.org/draft-07/schema#", "title": "London Administrative Court Daily Cause List", "description": "Schema for London Administrative Court with Main hearings and Planning Court tabs", "type": "object", + "definitions": { + "hearingItem": { + "type": "object", + "required": ["venue", "judge", "time", "caseNumber", "caseDetails", "hearingType"], + "properties": { + "venue": { "type": "string", "pattern": "^(?!(.|\\r|\\n)*<[^>]+>)(.|\\r|\\n)*$" }, + "judge": { "type": "string", "pattern": "^(?!(.|\\r|\\n)*<[^>]+>)(.|\\r|\\n)*$" }, + "time": { "type": "string", "pattern": "^(1[0-2]|0?[1-9])([:.][0-5][0-9])?[ap]m\\s*$" }, + "caseNumber": { "type": "string", "pattern": "^(?!(.|\\r|\\n)*<[^>]+>)(.|\\r|\\n)*$" }, + "caseDetails": { "type": "string", "pattern": "^(?!(.|\\r|\\n)*<[^>]+>)(.|\\r|\\n)*$" }, + "hearingType": { "type": "string", "pattern": "^(?!(.|\\r|\\n)*<[^>]+>)(.|\\r|\\n)*$" }, + "additionalInformation": { "type": "string", "pattern": "^(?!(.|\\r|\\n)*<[^>]+>)(.|\\r|\\n)*$" } + } + } + }, "required": ["mainHearings", "planningCourt"], "properties": { "mainHearings": { "type": "array", - "items": { ... } + "items": { "$ref": "#/definitions/hearingItem" } }, "planningCourt": { "type": "array", - "items": { ... } + "items": { "$ref": "#/definitions/hearingItem" } } } }Also applies to: 45-81
libs/list-types/court-of-appeal-civil-daily-cause-list/src/schemas/court-of-appeal-civil-daily-cause-list.json (1)
14-17: Consider using$defsto reduce schema duplication.The HTML-prevention pattern is repeated across all text fields. Using JSON Schema
$defswould improve maintainability.♻️ Example using $defs
{ "$schema": "http://json-schema.org/draft-07/schema#", "title": "Court of Appeal (Civil Division) Daily Cause List", + "$defs": { + "noHtmlString": { + "type": "string", + "pattern": "^(?!(.|\\r|\\n)*<[^>]+>)(.|\\r|\\n)*$" + } + }, ... "venue": { - "type": "string", - "pattern": "^(?!(.|\\r|\\n)*<[^>]+>)(.|\\r|\\n)*$" + "$ref": "#/$defs/noHtmlString" },libs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.ts (1)
56-56: Consider sanitising file paths in error logs.Logging the full
jsonFilePathcould expose internal server directory structure. Consider logging only the artefact ID or a sanitised path.♻️ Suggested fix
- console.error(`Error reading JSON file at ${jsonFilePath}:`, error); + console.error(`Error reading JSON file for artefact ${artefactId}:`, error);libs/list-types/administrative-court-daily-cause-list/src/schemas/administrative-court-daily-cause-list.json (1)
25-26: Consider case-insensitivity for the time pattern.The regex
[ap]mis case-sensitive, but the TypeScript converter uses/iflag. Inputs like10:00AMwould fail schema validation but pass converter validation.🔧 Optional: Make pattern case-insensitive
JSON Schema doesn't support regex flags directly. If case-insensitive matching is needed, consider:
- "pattern": "^\\d{1,2}([:.]\\d{2})?[ap]m\\s*$", + "pattern": "^\\d{1,2}([:.]\\d{2})?[aApP][mM]\\s*$",libs/list-types/common/src/validation/json-validator.ts (2)
4-5: Multipleanytypes lack justification.Per coding guidelines,
anyshould not be used without justification. TheAjv as anycast andMap<string, any>could be better typed.Proposed fix
-const ajv = new (Ajv as any)({ allErrors: true }); -const compiledValidators = new Map<string, any>(); +import type { ValidateFunction } from "ajv"; + +// Cast required due to Ajv ESM/CJS interop - default export typing mismatch +const ajv = new (Ajv as unknown as typeof Ajv)({ allErrors: true }); +const compiledValidators = new Map<string, ValidateFunction>();
29-32: Consider typing the error parameter.The
anytype on the error mapping could use Ajv'sErrorObjecttype for better type safety.Proposed fix
+import type { ErrorObject } from "ajv"; + - const errors = validate.errors?.map((error: any) => { + const errors = validate.errors?.map((error: ErrorObject) => { const field = error.instancePath.replace(/\//g, ".").substring(1) || "root"; return `${field}: ${error.message}`; }) || ["Unknown validation error"];libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (1)
120-120: Avoidanycast for locale object access.Per coding guidelines,
anyshould not be used without justification. Consider typing the locale objects with an index signature.Proposed fix
- const listContent = (t as any)[listTypeId] || {}; + const listContent = t[listTypeId as keyof typeof t] || {};libs/list-types/london-administrative-court-daily-cause-list/src/conversion/london-administrative-court-daily-cause-list-config.ts (2)
29-34: Consider adding HTML tag validation to the Time field.Other fields apply
validateNoHtmlTags, but Time only validates format. While the regex is strict, adding HTML validation maintains consistency and defence-in-depth.Suggested fix
{ header: "Time", fieldName: "time", required: true, - validators: [validateTimeFormat] + validators: [validateTimeFormat, (value: string, rowNumber: number) => validateNoHtmlTags(value, "Time", rowNumber)] },
87-103: Avoid untypedanyparameters.Per coding guidelines,
anyshould not be used without justification. ExcelJS provides proper types.Suggested fix
+import type { Worksheet, Row, Cell } from "exceljs"; -async function convertSheetToJson(worksheet: any, config: ExcelConverterConfig): Promise<any[]> { +async function convertSheetToJson(worksheet: Worksheet, config: ExcelConverterConfig): Promise<Record<string, unknown>[]> { // Create a temporary buffer from the sheet const workbook = new Workbook(); const tempSheet = workbook.addWorksheet("temp"); // Copy all rows from source to temp worksheet - worksheet.eachRow((row: any, rowNumber: number) => { + worksheet.eachRow((row: Row, rowNumber: number) => { const newRow = tempSheet.getRow(rowNumber); - row.eachCell((cell: any, colNumber: number) => { + row.eachCell((cell: Cell, colNumber: number) => { newRow.getCell(colNumber).value = cell.value; }); newRow.commit(); });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
libs/list-types/administrative-court-daily-cause-list/package.jsonlibs/list-types/administrative-court-daily-cause-list/src/conversion/administrative-court-daily-cause-list-config.tslibs/list-types/administrative-court-daily-cause-list/src/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/schemas/administrative-court-daily-cause-list.jsonlibs/list-types/care-standards-tribunal-weekly-hearing-list/package.jsonlibs/list-types/care-standards-tribunal-weekly-hearing-list/src/index.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.test.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/validation/json-validator.test.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/validation/json-validator.tslibs/list-types/common/package.jsonlibs/list-types/common/src/index.tslibs/list-types/common/src/validation/json-validator.tslibs/list-types/common/src/validation/list-type-validator.test.tslibs/list-types/common/src/validation/list-type-validator.tslibs/list-types/court-of-appeal-civil-daily-cause-list/package.jsonlibs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/court-of-appeal-civil-daily-cause-list-config.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/schemas/court-of-appeal-civil-daily-cause-list.jsonlibs/list-types/london-administrative-court-daily-cause-list/package.jsonlibs/list-types/london-administrative-court-daily-cause-list/src/conversion/london-administrative-court-daily-cause-list-config.tslibs/list-types/london-administrative-court-daily-cause-list/src/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/schemas/london-administrative-court-daily-cause-list.jsonlibs/list-types/rcj-standard-daily-cause-list/package.jsonlibs/list-types/rcj-standard-daily-cause-list/src/conversion/rcj-standard-daily-cause-list-config.tslibs/list-types/rcj-standard-daily-cause-list/src/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/schemas/rcj-standard-daily-cause-list.json
💤 Files with no reviewable changes (3)
- libs/list-types/care-standards-tribunal-weekly-hearing-list/src/index.ts
- libs/list-types/care-standards-tribunal-weekly-hearing-list/src/validation/json-validator.test.ts
- libs/list-types/care-standards-tribunal-weekly-hearing-list/src/validation/json-validator.ts
✅ Files skipped from review due to trivial changes (1)
- libs/list-types/common/src/validation/list-type-validator.test.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- libs/list-types/rcj-standard-daily-cause-list/package.json
- libs/list-types/rcj-standard-daily-cause-list/src/index.ts
- libs/list-types/administrative-court-daily-cause-list/src/index.ts
- libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts
- libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts
- libs/list-types/london-administrative-court-daily-cause-list/src/index.ts
- libs/list-types/administrative-court-daily-cause-list/package.json
- libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use camelCase for TypeScript variables:userId,caseDetails,documentId
Use PascalCase for classes and interfaces:UserService,CaseRepository. NOIprefix.
Use kebab-case for file and directory names:user-service.ts,case-management/
Use boolean variable names withis/has/canprefix:isActive,hasAccess,canEdit
Module ordering: constants at the top, exported functions next, other functions in order of use, interfaces and types at the bottom
Always add.jsextension to relative imports in ES modules (e.g.,import { foo } from "./bar.js")
Use workspace aliases for imports (@hmcts/*) instead of relative paths across packages
Use strict TypeScript mode enabled; noanywithout justification
Use parameterized database queries with Prisma (no raw SQL string concatenation)
Do not include sensitive data in logs
Files:
libs/list-types/rcj-standard-daily-cause-list/src/conversion/rcj-standard-daily-cause-list-config.tslibs/list-types/administrative-court-daily-cause-list/src/conversion/administrative-court-daily-cause-list-config.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/court-of-appeal-civil-daily-cause-list-config.tslibs/list-types/common/src/validation/list-type-validator.tslibs/list-types/common/src/validation/json-validator.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.test.tslibs/list-types/london-administrative-court-daily-cause-list/src/conversion/london-administrative-court-daily-cause-list-config.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/index.tslibs/list-types/common/src/index.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use SCREAMING_SNAKE_CASE for constants:
MAX_FILE_SIZE,DEFAULT_TIMEOUT
Files:
libs/list-types/rcj-standard-daily-cause-list/src/conversion/rcj-standard-daily-cause-list-config.tslibs/list-types/administrative-court-daily-cause-list/src/conversion/administrative-court-daily-cause-list-config.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/court-of-appeal-civil-daily-cause-list-config.tslibs/list-types/common/src/validation/list-type-validator.tslibs/list-types/common/src/validation/json-validator.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.test.tslibs/list-types/london-administrative-court-daily-cause-list/src/conversion/london-administrative-court-daily-cause-list-config.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/index.tslibs/list-types/common/src/index.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Input validation must be performed on all endpoints
Files:
libs/list-types/rcj-standard-daily-cause-list/src/conversion/rcj-standard-daily-cause-list-config.tslibs/list-types/administrative-court-daily-cause-list/src/conversion/administrative-court-daily-cause-list-config.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/court-of-appeal-civil-daily-cause-list-config.tslibs/list-types/common/src/validation/list-type-validator.tslibs/list-types/common/src/validation/json-validator.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.test.tslibs/list-types/london-administrative-court-daily-cause-list/src/conversion/london-administrative-court-daily-cause-list-config.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/index.tslibs/list-types/common/src/index.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not use CommonJS - ES modules only with import/export syntax
Files:
libs/list-types/rcj-standard-daily-cause-list/src/conversion/rcj-standard-daily-cause-list-config.tslibs/list-types/administrative-court-daily-cause-list/src/conversion/administrative-court-daily-cause-list-config.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/court-of-appeal-civil-daily-cause-list-config.tslibs/list-types/common/src/validation/list-type-validator.tslibs/list-types/common/src/validation/json-validator.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.test.tslibs/list-types/london-administrative-court-daily-cause-list/src/conversion/london-administrative-court-daily-cause-list-config.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/index.tslibs/list-types/common/src/index.ts
**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
**/package.json: Use@hmctsscope for package names:@hmcts/auth,@hmcts/case-management
All package.json files must use"type": "module"for ES modules support
Express version must be 5.x only ("express": "5.2.0")
All packages must use"test": "vitest run"script in package.json
Dependencies must use specific versions only (e.g.,"express": "5.2.0"), except for peer dependencies
Module build script must include"build:nunjucks"script if module contains Nunjucks templates in pages/ directory
Files:
libs/list-types/court-of-appeal-civil-daily-cause-list/package.jsonlibs/list-types/care-standards-tribunal-weekly-hearing-list/package.jsonlibs/list-types/london-administrative-court-daily-cause-list/package.jsonlibs/list-types/common/package.json
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Test files must be co-located with source code using
*.test.tsor*.spec.tsnaming pattern
Files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.test.ts
🧠 Learnings (14)
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/*.{ts,tsx} : Module ordering: constants at the top, exported functions next, other functions in order of use, interfaces and types at the bottom
Applied to files:
libs/list-types/administrative-court-daily-cause-list/src/conversion/administrative-court-daily-cause-list-config.tslibs/list-types/court-of-appeal-civil-daily-cause-list/package.jsonlibs/list-types/london-administrative-court-daily-cause-list/package.jsonlibs/list-types/court-of-appeal-civil-daily-cause-list/src/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/*.{ts,tsx,js,mjs} : Do not use CommonJS - ES modules only with import/export syntax
Applied to files:
libs/list-types/administrative-court-daily-cause-list/src/conversion/administrative-court-daily-cause-list-config.tslibs/list-types/common/package.jsonlibs/list-types/court-of-appeal-civil-daily-cause-list/src/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/config.ts : Module config.ts must export standardized interfaces: pageRoutes, apiRoutes, prismaSchemas, assets
Applied to files:
libs/list-types/administrative-court-daily-cause-list/src/conversion/administrative-court-daily-cause-list-config.tslibs/list-types/common/package.json
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/*.{ts,tsx} : Use workspace aliases for imports (`hmcts/*`) instead of relative paths across packages
Applied to files:
libs/list-types/administrative-court-daily-cause-list/src/conversion/administrative-court-daily-cause-list-config.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/package.jsonlibs/list-types/london-administrative-court-daily-cause-list/package.jsonlibs/list-types/common/src/validation/list-type-validator.tslibs/list-types/common/package.json
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/package.json : Use hmcts scope for package names: `hmcts/auth`, `hmcts/case-management`
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/package.jsonlibs/list-types/care-standards-tribunal-weekly-hearing-list/package.jsonlibs/list-types/london-administrative-court-daily-cause-list/package.json
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/package.json : All package.json files must use `"type": "module"` for ES modules support
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/package.jsonlibs/list-types/london-administrative-court-daily-cause-list/package.json
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/package.json : Module build script must include `"build:nunjucks"` script if module contains Nunjucks templates in pages/ directory
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/package.json
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.njk : Nunjucks templates must extend 'layouts/base-templates.njk' and use GOV.UK Frontend component macros
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/package.json
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.ts : Create page controller files with GET and POST exports following the pattern: `export const GET = async (req, res) => { ... }`
Applied to files:
libs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/routes/**/*.ts : API endpoints should use plural for collections (/api/cases), singular for specific (/api/case/:id), and singular for creation (POST /api/case)
Applied to files:
libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.ts : Provide both `en` and `cy` language objects in page controllers for English and Welsh support
Applied to files:
libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to e2e-tests/**/*.spec.ts : Include validation checks, Welsh translation checks, accessibility checks, and keyboard navigation within a single E2E test journey
Applied to files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.test.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/package.json : All packages must use `"test": "vitest run"` script in package.json
Applied to files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.test.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/*.{ts,tsx} : Always add `.js` extension to relative imports in ES modules (e.g., `import { foo } from "./bar.js"`)
Applied to files:
libs/list-types/common/package.json
🧬 Code graph analysis (1)
libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (4)
libs/list-types/common/src/validation/json-validator.ts (1)
createJsonValidator(12-36)libs/list-types/administrative-court-daily-cause-list/src/pages/en.ts (1)
en(1-53)libs/list-types/administrative-court-daily-cause-list/src/pages/cy.ts (1)
cy(1-32)libs/list-types/administrative-court-daily-cause-list/src/rendering/renderer.ts (1)
renderAdminCourt(61-83)
🔇 Additional comments (25)
libs/list-types/london-administrative-court-daily-cause-list/package.json (1)
1-44: LGTM!Package configuration adheres to all coding guidelines:
- Uses
@hmctsscope- ES modules enabled via
"type": "module"- Test script correctly set to
vitest run- Dependencies pinned to specific versions
build:nunjucksscript included for template copying- Express peer dependency appropriately uses semver range (exempt from strict pinning per guidelines)
libs/list-types/common/package.json (1)
34-34: LGTM!The
ajvdependency at version8.17.1is correctly pinned to a specific version per coding guidelines.libs/list-types/london-administrative-court-daily-cause-list/src/schemas/london-administrative-court-daily-cause-list.json (1)
14-17: Verify HTML-prevention regex for ReDoS and bypass resilience.The pattern
^(?!(.|\\r|\\n)*<[^>]+>)(.|\\r|\\n)*$uses nested quantifiers within a lookahead, which may be susceptible to catastrophic backtracking on adversarial input. Additionally, blocklist-based sanitisation can be bypassed (e.g., malformed tags, encoding tricks).If the downstream renderer already sanitises output, this validation may be redundant. Otherwise, consider whether an allowlist approach or server-side sanitisation is more robust.
libs/list-types/court-of-appeal-civil-daily-cause-list/package.json (3)
1-14: Package metadata and exports look good.Package name correctly uses
@hmctsscope, includes"type": "module"for ES modules support, and exports are properly configured for both production and development environments.
26-34: Dependencies use specific versions as required.All runtime dependencies correctly use pinned versions for external packages and
workspace:*for internal monorepo packages.
41-43: No change required to Express peer dependency version.The current specification of
^5.1.0is appropriate. Peer dependencies are exempt from strict version pinning per coding guidelines, and the semver range correctly specifies Express 5.x. As of March 2025, v5.1.0 is the latest available in the Express 5.x series.Likely an incorrect or invalid review comment.
libs/list-types/common/src/validation/list-type-validator.ts (1)
1-1: LGTM!Import path correctly updated to reflect the module's new location. The
.jsextension is properly included as per ES module conventions.libs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/court-of-appeal-civil-daily-cause-list-config.ts (1)
5-13: LGTM!Time validation pattern and helper function are well-defined. Constants correctly use
SCREAMING_SNAKE_CASEas per coding guidelines.libs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.ts (2)
18-19: LGTM!Good refactor to use the shared
createJsonValidatorfrom@hmcts/list-types-common. This centralises validation logic and reduces duplication across list-type modules.
94-94: LGTM!Good addition of
titleproperty to render props for improved template flexibility.libs/list-types/care-standards-tribunal-weekly-hearing-list/package.json (1)
27-27: LGTM!Good consolidation of JSON validation into the shared
@hmcts/list-types-commonpackage, removing the directajvdependency. This improves consistency across list-type modules. Based on learnings, workspace aliases are the preferred approach.libs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.test.ts (2)
6-14: LGTM – Clean mock setup for the new validation pattern.The mock hoisting is correctly handled with
mockValidatedeclared beforevi.mock, and the factory pattern returning the mock aligns withcreateJsonValidatorusage.
95-104: Validation assertions correctly updated.The test properly verifies the validator is called with the parsed JSON data.
libs/list-types/administrative-court-daily-cause-list/src/schemas/administrative-court-daily-cause-list.json (1)
1-54: Schema structure is sound.Required and optional fields are correctly defined. The anti-HTML pattern and time format validation are consistent with the RCJ schema.
libs/list-types/rcj-standard-daily-cause-list/src/schemas/rcj-standard-daily-cause-list.json (1)
1-54: Schema is well-defined and consistent with other list type schemas.Structure mirrors the Administrative Court schema appropriately. The same case-sensitivity consideration for the time pattern applies here.
libs/list-types/rcj-standard-daily-cause-list/src/conversion/rcj-standard-daily-cause-list-config.ts (2)
1-10: Clean implementation with good error messaging.The time validation provides clear, actionable error messages with row context. The
/iflag correctly handles case-insensitive input.
61-64: Efficient batch registration.Using a loop for IDs 10-17 is cleaner than individual calls. The comment clarifying ID 18's exclusion is helpful.
libs/list-types/court-of-appeal-civil-daily-cause-list/src/index.ts (1)
1-5: Well-structured module entry point.Side-effect import for converter registration and focused public API exports follow the established pattern. Proper
.jsextensions on relative imports.libs/list-types/common/src/index.ts (1)
18-19: LGTM!New validation exports are correctly structured with
.jsextensions for ES module compatibility.libs/list-types/administrative-court-daily-cause-list/src/conversion/administrative-court-daily-cause-list-config.ts (2)
3-10: LGTM!Time validation pattern and function are well-implemented with clear error messaging including row context.
14-60: LGTM!Excel config follows the established pattern. Field definitions are consistent with other list types.
libs/list-types/common/src/validation/json-validator.ts (1)
12-20: LGTM!Schema caching pattern is sound—compiles once per schema path and reuses the validator.
libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (2)
1-11: LGTM!Imports correctly use workspace aliases (
@hmcts/*) and.jsextensions for relative imports.
108-115: LGTM!Rendering integration with
renderAdminCourtis clean, passing all required options correctly.libs/list-types/london-administrative-court-daily-cause-list/src/conversion/london-administrative-court-daily-cause-list-config.ts (1)
1-12: LGTM!Imports, constant naming, and time validation function follow conventions. The regex pattern is well-documented.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| "date": { | ||
| "type": "string", | ||
| "pattern": "^\\d{2}/\\d{2}/\\d{4}$" | ||
| }, |
There was a problem hiding this comment.
Date format inconsistency between schema and converter config.
The schema description in the PR summary mentions "MM/DD/YYYY", but the converter config (FUTURE_JUDGMENTS_CONFIG) specifies "dd/MM/yyyy (e.g., 15/01/2025)". Verify the intended format and ensure consistency across schema documentation and validation messages.
| "time": { | ||
| "type": "string", | ||
| "pattern": "^\\d{1,2}([:.]\\d{2})?[ap]m\\s*$" | ||
| }, |
There was a problem hiding this comment.
Time validation allows invalid values.
The pattern ^\d{1,2}([:.]\d{2})?[ap]m\s*$ permits invalid times such as 99:99am or 0am. Consider restricting hours to 1-12 and minutes to 00-59.
Proposed stricter time pattern
- "pattern": "^\\d{1,2}([:.]\\d{2})?[ap]m\\s*$"
+ "pattern": "^(1[0-2]|0?[1-9])([:.][0-5][0-9])?[ap]m\\s*$"Also applies to: 59-62
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts (3)
62-62: Potential path traversal vulnerability.If
artefactIdcontains path traversal sequences (e.g.,../../../etc/passwd), an attacker could read arbitrary files. Sanitise the input or validate that the resolved path remains withinTEMP_UPLOAD_DIR.Suggested fix
const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`); + + // Prevent path traversal attacks + const resolvedPath = path.resolve(jsonFilePath); + if (!resolvedPath.startsWith(path.resolve(TEMP_UPLOAD_DIR))) { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Bad Request", + errorMessage: "Invalid artefactId parameter" + }); + }
68-68: Avoid logging full file paths.Logging
jsonFilePathexposes internal directory structure. Log only theartefactIdor a sanitised reference. As per coding guidelines, sensitive data should not appear in logs.Suggested fix
- console.error(`Error reading JSON file at ${jsonFilePath}:`, error); + console.error(`Error reading JSON file for artefact ${artefactId}:`, error);
77-88: Malformed JSON returns 500 instead of 400.
JSON.parseon line 77 can throw on invalid JSON, which falls through to the outer catch block returning a 500 error. This should be a 400 Bad Request since it's a client data issue.Suggested fix
- const jsonData: CourtOfAppealCivilData = JSON.parse(jsonContent); + let jsonData: CourtOfAppealCivilData; + try { + jsonData = JSON.parse(jsonContent); + } catch { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Invalid Data", + errorMessage: "The list data could not be parsed" + }); + }
🧹 Nitpick comments (12)
libs/list-types/london-administrative-court-daily-cause-list/src/conversion/london-administrative-court-daily-cause-list-config.test.ts (1)
111-138: Consider adding HTML validation tests for remaining fields.The HTML tag validation tests cover
venue,judge, andcaseDetails, butcaseNumber,hearingType, andadditionalInformationalso havevalidateNoHtmlTagsvalidators configured. Consider adding tests for these fields for completeness.libs/list-types/court-of-appeal-civil-daily-cause-list/src/conversion/court-of-appeal-civil-daily-cause-list-config.test.ts (1)
6-15: Consider extracting shared test helper to reduce duplication.The
createExcelBufferhelper function is identical across this file andlondon-administrative-court-daily-cause-list-config.test.ts. Consider extracting it to a shared test utilities module within@hmcts/list-types-commonor a dedicated test helpers location.libs/list-types/administrative-court-daily-cause-list/src/conversion/administrative-court-daily-cause-list-config.test.ts (1)
46-49: Test description is slightly misleading.Line 47 tests a trailing space after "am" (
"9:30am "), not before it. The description "should accept times with spaces before am/pm" only accurately describes line 48.Suggested fix
- it("should accept times with spaces before am/pm", () => { - expect(() => timeField?.validators?.[0]("9:30am ", 1)).not.toThrow(); - expect(() => timeField?.validators?.[0]("10:15 pm", 1)).not.toThrow(); + it("should accept times with whitespace around am/pm", () => { + expect(() => timeField?.validators?.[0]("9:30am ", 1)).not.toThrow(); + expect(() => timeField?.validators?.[0]("10:15 pm", 1)).not.toThrow(); });libs/list-types/common/src/validation/json-validator.test.ts (1)
118-128: Caching test doesn't verify actual caching behaviour.This test only confirms both validators work; it doesn't prove they're the same cached instance. Consider asserting referential equality to verify caching.
♻️ Suggested improvement
it("should cache compiled validators for the same schema path", () => { const schemaPath = path.join(TEST_SCHEMA_DIR, "valid-schema.json"); const validator1 = createJsonValidator(schemaPath); const validator2 = createJsonValidator(schemaPath); - const result1 = validator1({ name: "John", age: 30 }); - const result2 = validator2({ name: "Jane", age: 25 }); - - expect(result1.isValid).toBe(true); - expect(result2.isValid).toBe(true); + expect(validator1).toBe(validator2); });libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts (1)
52-60: Consider extracting magic number to a named constant.The list type ID
19is used inline. A named constant would improve readability and maintainability.Suggested improvement
+const COURT_OF_APPEAL_CIVIL_LIST_TYPE_ID = 19; + // ... - if (artefact.listTypeId !== 19) { + if (artefact.listTypeId !== COURT_OF_APPEAL_CIVIL_LIST_TYPE_ID) {libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.test.ts (1)
270-285: Consider adding a test for malformed JSON.There's a test for validation failure, but no test for when
JSON.parsethrows on malformed JSON content. This would exercise the outer catch block (currently returning 500, though arguably should be 400).Suggested test case
it("should return 500 when JSON content is malformed", async () => { const mockArtefact = { artefactId: "test-artefact-123", listTypeId: 19, displayFrom: new Date("2026-01-15"), displayTo: new Date("2026-01-15"), lastReceivedDate: new Date("2026-01-14T12:00:00Z"), provenance: "MANUAL_UPLOAD" }; req.query = { artefactId: "test-artefact-123" }; vi.mocked(prisma.artefact.findUnique).mockResolvedValue(mockArtefact as any); vi.mocked(readFile).mockResolvedValue("{ invalid json }"); await GET(req as Request, res as Response); expect(res.status).toHaveBeenCalledWith(500); });libs/list-types/rcj-standard-daily-cause-list/src/pages/index.test.ts (1)
131-173: Consider usingit.eachfor parameterised tests.The loop-based approach works but
it.eachprovides clearer test output, showing eachlistTypeIdas a separate test case in reports. This improves debugging when a specific list type fails.♻️ Suggested refactor using it.each
- it("should render the list successfully for all supported list type IDs (10-17)", async () => { - const listTypeIds = [10, 11, 12, 13, 14, 15, 16, 17]; - - for (const listTypeId of listTypeIds) { - vi.clearAllMocks(); - - const mockArtefact = { - artefactId: `test-artefact-${listTypeId}`, - listTypeId, - displayFrom: new Date("2026-01-15"), - displayTo: new Date("2026-01-15"), - lastReceivedDate: new Date("2026-01-14T12:00:00Z"), - provenance: "MANUAL_UPLOAD" - }; - - const mockJsonData = []; - const mockRenderedData = { - header: { - listTitle: "Test List", - listDate: "List for 15 January 2026", - lastUpdated: "Last updated 14 January 2026 at 12pm" - }, - hearings: [] - }; - - req.query = { artefactId: `test-artefact-${listTypeId}` }; - - vi.mocked(prisma.artefact.findUnique).mockResolvedValue(mockArtefact as any); - vi.mocked(readFile).mockResolvedValue(JSON.stringify(mockJsonData)); - mockValidate.mockReturnValue({ isValid: true, errors: [] }); - vi.mocked(renderStandardDailyCauseList).mockReturnValue(mockRenderedData); - - await GET(req as Request, res as Response); - - expect(renderStandardDailyCauseList).toHaveBeenCalledWith( - mockJsonData, - expect.objectContaining({ - listTypeId - }) - ); - expect(res.render).toHaveBeenCalled(); - } - }); + it.each([10, 11, 12, 13, 14, 15, 16, 17])( + "should render the list successfully for listTypeId %i", + async (listTypeId) => { + const mockArtefact = { + artefactId: `test-artefact-${listTypeId}`, + listTypeId, + displayFrom: new Date("2026-01-15"), + displayTo: new Date("2026-01-15"), + lastReceivedDate: new Date("2026-01-14T12:00:00Z"), + provenance: "MANUAL_UPLOAD" + }; + + const mockJsonData: unknown[] = []; + const mockRenderedData = { + header: { + listTitle: "Test List", + listDate: "List for 15 January 2026", + lastUpdated: "Last updated 14 January 2026 at 12pm" + }, + hearings: [] + }; + + req.query = { artefactId: `test-artefact-${listTypeId}` }; + + vi.mocked(prisma.artefact.findUnique).mockResolvedValue(mockArtefact as any); + vi.mocked(readFile).mockResolvedValue(JSON.stringify(mockJsonData)); + mockValidate.mockReturnValue({ isValid: true, errors: [] }); + vi.mocked(renderStandardDailyCauseList).mockReturnValue(mockRenderedData); + + await GET(req as Request, res as Response); + + expect(renderStandardDailyCauseList).toHaveBeenCalledWith( + mockJsonData, + expect.objectContaining({ + listTypeId + }) + ); + expect(res.render).toHaveBeenCalled(); + } + );libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.test.ts (2)
116-116: Consider asserting the file path argument.The current assertion only verifies
readFilewas called, but doesn't confirm it was called with the correct path. For consistency with other assertions in this test, consider:- expect(readFile).toHaveBeenCalled(); + expect(readFile).toHaveBeenCalledWith( + expect.stringContaining("test-artefact-123.json"), + "utf-8" + );
254-270: Consider adding a test for malformed JSON.The server error test covers database failures, but there's no test for when
readFilereturns content that failsJSON.parse. This would exercise a different code path to the 500 handler.it("should return 500 when JSON parsing fails", async () => { const mockArtefact = { artefactId: "test-artefact-123", listTypeId: 18, displayFrom: new Date("2026-01-15"), displayTo: new Date("2026-01-15"), lastReceivedDate: new Date("2026-01-14T12:00:00Z"), provenance: "MANUAL_UPLOAD" }; req.query = { artefactId: "test-artefact-123" }; vi.mocked(prisma.artefact.findUnique).mockResolvedValue(mockArtefact as any); vi.mocked(readFile).mockResolvedValue("{ invalid json }"); await GET(req as Request, res as Response); expect(res.status).toHaveBeenCalledWith(500); });libs/list-types/rcj-standard-daily-cause-list/src/conversion/rcj-standard-daily-cause-list-config.test.ts (1)
46-49: Minor: Test description doesn't fully match test cases.Line 47 tests
"9:30am "(trailing space after "am"), whilst the test name references "spaces before am/pm". Consider renaming to "should accept times with surrounding spaces" or adjusting the test cases.libs/list-types/court-of-appeal-civil-daily-cause-list/src/config.test.ts (1)
20-24: Missing existence check for assets path.The
assetstest validates the string contains "assets" but doesn't verify the directory actually exists, unlike themoduleRootandpageRoutes.pathtests.Consider adding existence check
it("should export assets as a valid directory path", () => { expect(assets).toBeDefined(); expect(typeof assets).toBe("string"); expect(assets).toContain("assets"); + expect(existsSync(assets)).toBe(true); });libs/list-types/london-administrative-court-daily-cause-list/src/config.test.ts (1)
1-33: Consistent test structure across config modules.This follows the same pattern as other config tests. The same observations apply regarding the missing
existsSynccheck forassets(line 23) and theassets.startsWith(moduleRoot)assertion (line 31) that may not hold if assets is a sibling directory.
| it("should format last updated without minutes when on the hour", () => { | ||
| const result = renderAdminCourt(mockHearings, { | ||
| locale: "en", | ||
| listTypeId: 20, | ||
| listTitle: "Test List", | ||
| displayFrom: new Date(2025, 0, 15), | ||
| displayTo: new Date(2025, 0, 15), | ||
| lastReceivedDate: "2025-01-15T14:00:00Z" | ||
| }); | ||
|
|
||
| expect(result.header.lastUpdated).toMatch(/2pm|14pm/); | ||
| }); |
There was a problem hiding this comment.
Incorrect regex pattern for 24-hour time format.
The pattern 14pm at line 130 is not a valid time format. If testing 24-hour format, it should be 14:00 not 14pm.
Suggested fix
- expect(result.header.lastUpdated).toMatch(/2pm|14pm/);
+ expect(result.header.lastUpdated).toMatch(/2pm|14:00/);📝 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.
| it("should format last updated without minutes when on the hour", () => { | |
| const result = renderAdminCourt(mockHearings, { | |
| locale: "en", | |
| listTypeId: 20, | |
| listTitle: "Test List", | |
| displayFrom: new Date(2025, 0, 15), | |
| displayTo: new Date(2025, 0, 15), | |
| lastReceivedDate: "2025-01-15T14:00:00Z" | |
| }); | |
| expect(result.header.lastUpdated).toMatch(/2pm|14pm/); | |
| }); | |
| it("should format last updated without minutes when on the hour", () => { | |
| const result = renderAdminCourt(mockHearings, { | |
| locale: "en", | |
| listTypeId: 20, | |
| listTitle: "Test List", | |
| displayFrom: new Date(2025, 0, 15), | |
| displayTo: new Date(2025, 0, 15), | |
| lastReceivedDate: "2025-01-15T14:00:00Z" | |
| }); | |
| expect(result.header.lastUpdated).toMatch(/2pm|14:00/); | |
| }); |
| it("should handle hearing with undefined additional information", () => { | ||
| const hearingsWithUndefined: StandardHearingList = [ | ||
| { | ||
| venue: "Court 1", | ||
| judge: "Judge Smith", | ||
| time: "10:00am", | ||
| caseNumber: "T20257890", | ||
| caseDetails: "R v Jones", | ||
| hearingType: "Trial", | ||
| additionalInformation: "" | ||
| } | ||
| ]; | ||
|
|
||
| const result = renderAdminCourt(hearingsWithUndefined, { | ||
| locale: "en", | ||
| listTypeId: 20, | ||
| listTitle: "Test List", | ||
| displayFrom: new Date(2025, 0, 15), | ||
| displayTo: new Date(2025, 0, 15), | ||
| lastReceivedDate: "2025-01-15T09:30:00Z" | ||
| }); | ||
|
|
||
| expect(result.hearings[0].additionalInformation).toBe(""); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find type definition files
find libs/list-types/administrative-court-daily-cause-list/src -name "*.ts" -type f | head -20Repository: hmcts/cath-service
Length of output: 1038
🏁 Script executed:
# Search for StandardHearingList type definition
rg -n "type StandardHearing|interface StandardHearing" libs/list-types/administrative-court-daily-cause-list/src --type ts -A 15Repository: hmcts/cath-service
Length of output: 1141
🏁 Script executed:
# Look at the test file to see line 74 test and lines 133-156
sed -n '60,160p' libs/list-types/administrative-court-daily-cause-list/src/rendering/renderer.test.ts | cat -nRepository: hmcts/cath-service
Length of output: 3978
🏁 Script executed:
#!/bin/bash
# Get lines 133-156 from the test file
sed -n '133,156p' libs/list-types/administrative-court-daily-cause-list/src/rendering/renderer.test.ts | cat -nRepository: hmcts/cath-service
Length of output: 938
Test description does not match test data, and test duplicates the existing check at line 74.
The test claims to handle "undefined additional information" but uses additionalInformation: "" (empty string), not undefined. The additionalInformation property is also required in the StandardHearing type, not optional, so the suggested fix to remove the property entirely would not work without first making it optional in the type definition.
Rename the test description to "should handle empty additional information" to match the test data, or if genuinely testing the undefined case is needed, make additionalInformation optional in the StandardHearing interface and update the test accordingly.
| import { existsSync } from "node:fs"; | ||
| import { describe, expect, it } from "vitest"; | ||
| import { moduleRoot, pageRoutes } from "./config.js"; | ||
|
|
||
| describe("config", () => { | ||
| it("should export moduleRoot as a valid directory path", () => { | ||
| expect(moduleRoot).toBeDefined(); | ||
| expect(typeof moduleRoot).toBe("string"); | ||
| expect(existsSync(moduleRoot)).toBe(true); | ||
| }); | ||
|
|
||
| it("should export pageRoutes with a valid path", () => { | ||
| expect(pageRoutes).toBeDefined(); | ||
| expect(pageRoutes.path).toBeDefined(); | ||
| expect(typeof pageRoutes.path).toBe("string"); | ||
| expect(pageRoutes.path).toContain("pages"); | ||
| expect(existsSync(pageRoutes.path)).toBe(true); | ||
| }); | ||
|
|
||
| it("should have pageRoutes.path as a subdirectory of moduleRoot", () => { | ||
| expect(pageRoutes.path.startsWith(moduleRoot)).toBe(true); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat libs/list-types/rcj-standard-daily-cause-list/src/config.ts 2>/dev/null || echo "File not found"Repository: hmcts/cath-service
Length of output: 336
config.ts must export standardized interfaces: apiRoutes, prismaSchemas, and assets.
The test correctly validates the exports that are currently present (moduleRoot and pageRoutes). However, according to standardised module requirements, config.ts must also export apiRoutes, prismaSchemas, and assets. Once these are added to config.ts, the test should be expanded to validate them.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
libs/list-types/rcj-standard-daily-cause-list/src/pages/en.ts (1)
89-99: Previously flagged: English and Welsh page titles may be misaligned for key 16.This was identified in a previous review. The English
pageTitleis "Civil Daily Cause List" whilst the Welsh translation appears to include "Mayor and City" context. Please verify this is intentional.libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (3)
48-48: ValidateartefactIdtype before use.
req.query.artefactIdcan bestring | string[] | undefined. The cast silently allows arrays through, which could cause unexpected behaviour.Proposed fix
- const artefactId = req.query.artefactId as string; + const artefactId = typeof req.query.artefactId === "string" ? req.query.artefactId : undefined;
85-85: Path traversal risk: sanitiseartefactIdbefore file access.User-supplied
artefactIdis interpolated directly into the file path. Validate the format (e.g., UUID regex) before constructing the path to prevent directory traversal attacks.Proposed fix
+ // Validate artefactId format (UUID expected) + const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + if (!UUID_REGEX.test(artefactId)) { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Bad Request", + errorMessage: "Invalid artefactId format" + }); + } + const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`);
100-111: Malformed JSON returns 500 instead of 400.
JSON.parseis outside the inner try-catch, so invalid JSON throws and is caught by the outer handler, returning a generic 500 error. Move the parse inside the try-catch to return a proper 400 for malformed data.Proposed fix
let jsonContent: string; + let jsonData: StandardHearingList; try { jsonContent = await readFile(jsonFilePath, "utf-8"); + jsonData = JSON.parse(jsonContent); } catch (error) { - console.error(`Error reading JSON file at ${jsonFilePath}:`, error); - return res.status(404).render("errors/common", { - en, - cy, - errorTitle: "Not Found", - errorMessage: "The requested list could not be found" - }); + if ((error as NodeJS.ErrnoException).code === "ENOENT") { + console.error(`Error reading JSON file at ${jsonFilePath}:`, error); + return res.status(404).render("errors/common", { + en, + cy, + errorTitle: "Not Found", + errorMessage: "The requested list could not be found" + }); + } + console.error("Error parsing JSON:", error); + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Invalid Data", + errorMessage: "The list data could not be parsed" + }); } - - const jsonData: StandardHearingList = JSON.parse(jsonContent);libs/list-types/common/src/mock-list-types.ts (1)
103-111: Naming inconsistency:namefield missing "CENTRAL".The
englishFriendlyNameandurlPathboth reference "Central London" but thenamefield isCOUNTY_COURT_LONDON_CIVIL_DAILY_CAUSE_LIST. Consider updating toCOUNTY_COURT_CENTRAL_LONDON_CIVIL_DAILY_CAUSE_LISTfor consistency.libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/court-of-appeal-civil-daily-cause-list.njk (1)
67-71: Inconsistent section heading when no hearings are present.When
dailyHearingsis empty, no section heading is displayed, whereas the empty state forfutureJudgments(lines 108-113) includes the section heading. Consider adding a heading for consistency, particularly for screen reader users who benefit from consistent page structure.
🧹 Nitpick comments (11)
libs/list-types/rcj-standard-daily-cause-list/src/pages/civil-courts-rcj-daily-cause-list.njk (3)
29-32: Redundant accessibility attributes on search input.The input has both a
<label for="case-search-input">and anaria-labelattribute. When a proper<label>association exists, thearia-labelis superfluous and may cause screen readers to announce the label twice.♻️ Suggested fix
<label class="govuk-label govuk-visually-hidden" for="case-search-input"> {{ common.searchCasesLabel }} </label> - <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text" aria-label="{{ common.searchCasesLabel }}"> + <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text"> </div>
36-36: Redundantrole="table"attribute.The
<table>element already has an implicit ARIA role oftable. The explicitrole="table"is unnecessary.♻️ Suggested fix
- <table class="govuk-table" id="hearings-table" role="table" aria-label="{{ header.listTitle }}"> + <table class="govuk-table" id="hearings-table" aria-label="{{ header.listTitle }}">
48-60: Consider adding empty state handling for hearings.If the
hearingsarray is empty, the table renders with headers only and an empty body. Adding an empty state message would improve UX.♻️ Suggested enhancement
<tbody class="govuk-table__body"> + {% if hearings | length == 0 %} + <tr class="govuk-table__row"> + <td class="govuk-table__cell" colspan="7">{{ common.noHearings }}</td> + </tr> + {% endif %} {% for hearing in hearings %} <tr class="govuk-table__row"> <td class="govuk-table__cell">{{ hearing.venue }}</td>Ensure
common.noHearingsis defined in your strings file.libs/list-types/rcj-standard-daily-cause-list/src/pages/kings-bench-masters-daily-cause-list.njk (2)
29-33: Redundantaria-labelattribute.The
<label for="case-search-input">element already provides an accessible name for the input. The additionalaria-labelon line 32 is redundant and could be removed.Suggested fix
<div class="govuk-form-group search-container"> <h2 class="govuk-heading-s">{{ common.searchCasesTitle }}</h2> <label class="govuk-label govuk-visually-hidden" for="case-search-input"> {{ common.searchCasesLabel }} </label> - <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text" aria-label="{{ common.searchCasesLabel }}"> + <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text"> </div>
21-25: Consider extracting complex HTML into a partial.The inline HTML string in
govukDetailsis lengthy and difficult to maintain. If this pattern is reused, consider extracting it into a Nunjucks partial or macro for better readability.libs/list-types/administrative-court-daily-cause-list/src/pages/administrative-court-daily-cause-list.njk (2)
2-2: Unused import:govukTablemacro is imported but not used.The template imports
govukTablebut constructs the table manually. Either use the macro or remove the unused import.Suggested fix
{% extends "layouts/base-template.njk" %} -{% from "govuk/components/table/macro.njk" import govukTable %} {% from "govuk/components/details/macro.njk" import govukDetails %}
29-35: Redundantaria-labelattribute.Same as the King's Bench template: the
<label>element already provides an accessible name, making thearia-labelredundant.Suggested fix
- <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text" aria-label="{{ common.searchCasesLabel }}"> + <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text">libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (1)
128-128: Justify or avoidanycast.The
(t as any)[listTypeId]cast bypasses type safety. Consider defining a proper type for the locale objects or using a type guard.Suggested approach
- const listContent = (t as any)[listTypeId] || {}; + const listContent = (t as Record<number, unknown>)[listTypeId] || {};Alternatively, define proper typing for
enandcythat includes numeric keys.libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/court-of-appeal-civil-daily-cause-list.njk (2)
29-32: Remove redundantaria-labelattribute.The visually hidden
<label>element already provides the accessible name for the input. Thearia-labelattribute duplicates this and is unnecessary.♻️ Suggested fix
<label class="govuk-label govuk-visually-hidden" for="case-search-input"> {{ t.searchCasesLabel }} </label> - <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text" aria-label="{{ t.searchCasesLabel }}"> + <input class="govuk-input govuk-!-width-one-half" id="case-search-input" name="search" type="text">
39-39: Remove redundantrole="table"attribute.The
role="table"is implicit for HTML<table>elements and doesn't need to be explicitly declared. The same applies to line 78.libs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.ts (1)
4-34: Module ordering: interfaces should be at the bottom.Per coding guidelines, the preferred ordering is: constants at the top, exported functions next, other functions in order of use, interfaces and types at the bottom. Currently, interfaces are defined before the functions.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (33)
libs/list-types/administrative-court-daily-cause-list/src/pages/administrative-court-daily-cause-list.njklibs/list-types/administrative-court-daily-cause-list/src/pages/cy.tslibs/list-types/administrative-court-daily-cause-list/src/pages/en.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/rendering/renderer.test.tslibs/list-types/administrative-court-daily-cause-list/src/rendering/renderer.tslibs/list-types/common/src/mock-list-types.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/court-of-appeal-civil-daily-cause-list.njklibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/cy.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.test.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/cy.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/en.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.test.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/london-administrative-court-daily-cause-list.njklibs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.test.tslibs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/civil-courts-rcj-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/county-court-central-london-civil-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/court-of-appeal-criminal-division-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/cy.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/en.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/family-division-high-court-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/kings-bench-division-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/kings-bench-masters-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/mayor-city-civil-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/senior-courts-costs-office-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/rendering/renderer.test.tslibs/list-types/rcj-standard-daily-cause-list/src/rendering/renderer.ts
🚧 Files skipped from review as they are similar to previous changes (19)
- libs/list-types/london-administrative-court-daily-cause-list/src/pages/cy.ts
- libs/list-types/rcj-standard-daily-cause-list/src/pages/mayor-city-civil-daily-cause-list.njk
- libs/list-types/administrative-court-daily-cause-list/src/rendering/renderer.test.ts
- libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/cy.ts
- libs/list-types/rcj-standard-daily-cause-list/src/rendering/renderer.test.ts
- libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts
- libs/list-types/rcj-standard-daily-cause-list/src/pages/cy.ts
- libs/list-types/rcj-standard-daily-cause-list/src/rendering/renderer.ts
- libs/list-types/rcj-standard-daily-cause-list/src/pages/senior-courts-costs-office-daily-cause-list.njk
- libs/list-types/administrative-court-daily-cause-list/src/pages/en.ts
- libs/list-types/rcj-standard-daily-cause-list/src/pages/family-division-high-court-daily-cause-list.njk
- libs/list-types/london-administrative-court-daily-cause-list/src/pages/london-administrative-court-daily-cause-list.njk
- libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.ts
- libs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.ts
- libs/list-types/administrative-court-daily-cause-list/src/rendering/renderer.ts
- libs/list-types/rcj-standard-daily-cause-list/src/pages/county-court-central-london-civil-daily-cause-list.njk
- libs/list-types/rcj-standard-daily-cause-list/src/pages/kings-bench-division-daily-cause-list.njk
- libs/list-types/rcj-standard-daily-cause-list/src/pages/court-of-appeal-criminal-division-daily-cause-list.njk
- libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use camelCase for TypeScript variables:userId,caseDetails,documentId
Use PascalCase for classes and interfaces:UserService,CaseRepository. NOIprefix.
Use kebab-case for file and directory names:user-service.ts,case-management/
Use boolean variable names withis/has/canprefix:isActive,hasAccess,canEdit
Module ordering: constants at the top, exported functions next, other functions in order of use, interfaces and types at the bottom
Always add.jsextension to relative imports in ES modules (e.g.,import { foo } from "./bar.js")
Use workspace aliases for imports (@hmcts/*) instead of relative paths across packages
Use strict TypeScript mode enabled; noanywithout justification
Use parameterized database queries with Prisma (no raw SQL string concatenation)
Do not include sensitive data in logs
Files:
libs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.test.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.tslibs/list-types/common/src/mock-list-types.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.test.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/cy.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/en.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.test.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/en.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use SCREAMING_SNAKE_CASE for constants:
MAX_FILE_SIZE,DEFAULT_TIMEOUT
Files:
libs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.test.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.tslibs/list-types/common/src/mock-list-types.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.test.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/cy.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/en.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.test.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/en.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Test files must be co-located with source code using
*.test.tsor*.spec.tsnaming pattern
Files:
libs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.test.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.test.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.test.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Input validation must be performed on all endpoints
Files:
libs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.test.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.tslibs/list-types/common/src/mock-list-types.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.test.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/cy.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/en.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.test.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/en.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not use CommonJS - ES modules only with import/export syntax
Files:
libs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.test.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.tslibs/list-types/common/src/mock-list-types.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.test.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/cy.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/en.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.test.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/en.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 192
File: libs/list-types/common/src/mock-list-types.ts:93-110
Timestamp: 2025-12-19T15:19:47.640Z
Learning: For Single Justice Procedure (SJP) list types in libs/list-types/common/src/mock-list-types.ts, the Welsh translation "Gweithdrefn Ynad Sengl" is the preferred terminology for "Single Justice Procedure" rather than "Gweithdrefn Cyfiawnder Sengl".
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to e2e-tests/**/*.spec.ts : Include validation checks, Welsh translation checks, accessibility checks, and keyboard navigation within a single E2E test journey
Applied to files:
libs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.test.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.test.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.test.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/*.{ts,tsx} : Module ordering: constants at the top, exported functions next, other functions in order of use, interfaces and types at the bottom
Applied to files:
libs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.njk : Nunjucks templates must extend 'layouts/base-templates.njk' and use GOV.UK Frontend component macros
Applied to files:
libs/list-types/rcj-standard-daily-cause-list/src/pages/civil-courts-rcj-daily-cause-list.njklibs/list-types/administrative-court-daily-cause-list/src/pages/administrative-court-daily-cause-list.njklibs/list-types/rcj-standard-daily-cause-list/src/pages/kings-bench-masters-daily-cause-list.njklibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/court-of-appeal-civil-daily-cause-list.njk
📚 Learning: 2025-11-20T10:19:35.873Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 106
File: libs/system-admin-pages/src/reference-data-upload/services/download-service.ts:25-27
Timestamp: 2025-11-20T10:19:35.873Z
Learning: In the HMCTS cath-service project, region and sub-jurisdiction names are managed manually and must not contain semicolons, as semicolons are used as delimiters when joining multiple values in CSV export/import operations (specifically in libs/system-admin-pages/src/reference-data-upload/services/download-service.ts).
Applied to files:
libs/list-types/common/src/mock-list-types.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.ts : Provide both `en` and `cy` language objects in page controllers for English and Welsh support
Applied to files:
libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.test.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/cy.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/en.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/en.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.ts : Create page controller files with GET and POST exports following the pattern: `export const GET = async (req, res) => { ... }`
Applied to files:
libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.test.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/routes/**/*.ts : API endpoints should use plural for collections (/api/cases), singular for specific (/api/case/:id), and singular for creation (POST /api/case)
Applied to files:
libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts
📚 Learning: 2025-11-20T09:59:16.776Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 106
File: libs/system-admin-pages/src/pages/reference-data-upload/index.test.ts:84-160
Timestamp: 2025-11-20T09:59:16.776Z
Learning: In the cath-service repository, Welsh localization (lng=cy) is not required for admin screens (system-admin-pages), so locale preservation in admin screen redirects is not necessary.
Applied to files:
libs/list-types/administrative-court-daily-cause-list/src/pages/cy.ts
📚 Learning: 2025-12-19T15:19:47.640Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 192
File: libs/list-types/common/src/mock-list-types.ts:93-110
Timestamp: 2025-12-19T15:19:47.640Z
Learning: For Single Justice Procedure (SJP) list types in libs/list-types/common/src/mock-list-types.ts, the Welsh translation "Gweithdrefn Ynad Sengl" is the preferred terminology for "Single Justice Procedure" rather than "Gweithdrefn Cyfiawnder Sengl".
Applied to files:
libs/list-types/administrative-court-daily-cause-list/src/pages/cy.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/en.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Welsh translations are required for all user-facing text
Applied to files:
libs/list-types/administrative-court-daily-cause-list/src/pages/cy.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/en.ts
📚 Learning: 2025-11-27T14:18:22.932Z
Learnt from: junaidiqbalmoj
Repo: hmcts/cath-service PR: 137
File: e2e-tests/tests/create-media-account.spec.ts:51-64
Timestamp: 2025-11-27T14:18:22.932Z
Learning: For the create-media-account form in libs/public-pages, the English email validation error message (errorEmailInvalid) should be: "There is a problem - Enter a valid email address, e.g. nameexample.com" to match the Welsh translation and clearly indicate the format requirement rather than suggesting the field is empty.
Applied to files:
libs/list-types/rcj-standard-daily-cause-list/src/pages/en.ts
🧬 Code graph analysis (4)
libs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.test.ts (2)
libs/list-types/london-administrative-court-daily-cause-list/src/models/types.ts (1)
LondonAdminCourtData(11-14)libs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.ts (1)
renderLondonAdminCourt(75-90)
libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.test.ts (2)
libs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.ts (1)
renderLondonAdminCourt(75-90)libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.ts (1)
GET(23-118)
libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (2)
libs/list-types/common/src/validation/json-validator.ts (1)
createJsonValidator(12-36)libs/list-types/rcj-standard-daily-cause-list/src/rendering/renderer.ts (1)
renderStandardDailyCauseList(62-85)
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.ts (3)
libs/list-types/administrative-court-daily-cause-list/src/pages/en.ts (1)
en(1-62)libs/list-types/london-administrative-court-daily-cause-list/src/pages/en.ts (1)
en(1-40)libs/list-types/rcj-standard-daily-cause-list/src/pages/en.ts (1)
en(1-138)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests
🔇 Additional comments (21)
libs/list-types/rcj-standard-daily-cause-list/src/pages/civil-courts-rcj-daily-cause-list.njk (3)
1-2: Template setup looks correct.The template correctly extends the base layout and imports the GOV.UK details macro. This aligns with the coding guidelines for Nunjucks templates.
21-25: HTML concatenation in govukDetails.The inline HTML string construction works but could be fragile if the content fields contain special characters. Ensure the
listContentvalues are pre-sanitised by the renderer.
35-68: Table and navigation structure are well implemented.Good use of semantic HTML with proper table headers using
scope="col", appropriate GOV.UK styling classes, and accessible back-to-top navigation.libs/list-types/administrative-court-daily-cause-list/src/pages/cy.ts (1)
1-62: LGTM!Welsh localization file is well-structured with translations for all four Administrative Court locations (Birmingham, Leeds, Bristol/Cardiff, Manchester) plus common UI strings. The provenance labels and table headers are properly translated.
libs/list-types/rcj-standard-daily-cause-list/src/pages/en.ts (1)
1-137: LGTM overall.Comprehensive English localization covering eight list types with appropriate location details, contact information, and guidance text. The common block provides consistent UI strings across all list types.
libs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.test.ts (1)
1-250: LGTM!Comprehensive test suite covering header rendering (English/Welsh), hearing field mapping, time normalisation (dots to colons), last updated formatting variations, and edge cases (empty data, missing minutes). Good use of shared
baseOptionsto reduce duplication.libs/list-types/london-administrative-court-daily-cause-list/src/pages/en.ts (1)
1-40: LGTM!English locale data is well-structured with appropriate keys for page metadata, table headers, and provenance labels. The structure aligns with the Welsh counterpart and rendering requirements.
libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.test.ts (5)
1-54: LGTM!Test setup with
vi.hoistedfor the validator mock and proper mock ordering follows Vitest best practices. ThebeforeEachblock correctly resets state between tests.
57-132: LGTM!Thorough success path test validating the complete flow: artefact lookup, file read, JSON validation, rendering, and response assertions.
134-270: LGTM!Error path coverage is comprehensive—missing artefactId (400), not found artefact (404), invalid list type (400), missing JSON file (404), validation failure (400), and server error (500) are all tested with appropriate status codes and error messages.
272-356: LGTM!Locale handling tests correctly verify Welsh locale propagation and English default fallback behaviour.
358-436: LGTM!Provenance label tests cover both the mapped label scenario (
LIST_ASSIST→"List Assist") and the raw fallback scenario (UNKNOWN_SOURCE).libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (2)
13-22: LGTM!Route definitions are well-structured and follow kebab-case naming convention.
33-42: LGTM!
LIST_TYPE_CONFIGprovides a clean mapping of list type IDs to localised titles and templates, supporting the multi-list-type architecture.libs/list-types/common/src/mock-list-types.ts (4)
92-102: LGTM!The RCJ Standard Format section opener and first entry (ID 10) are well-structured with consistent naming across
name,englishFriendlyName, andurlPath.
121-165: LGTM!Entries ID 13-17 (Family Division, King's Bench Division, King's Bench Masters, Mayor & City, Senior Courts Costs Office) are well-structured with consistent naming conventions across all fields.
166-175: LGTM!London Administrative Court entry (ID 18) follows consistent naming conventions.
186-222: LGTM!Administrative Court entries (IDs 20-23) for Birmingham, Leeds, Bristol/Cardiff, and Manchester are well-structured with consistent naming conventions. Welsh translations appear appropriate for the court names.
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.ts (1)
1-46: LGTM!Localization structure is consistent with other list-type modules in the codebase. All required keys for the Court of Appeal (Civil Division) Daily Cause List are present and well-organised.
libs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.test.ts (1)
1-333: Comprehensive test coverage.The test suite thoroughly exercises the renderer across English and Welsh locales, time normalisation, date formatting, and edge cases. Structure aligns with other renderer tests in the codebase.
libs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.ts (1)
102-116: LGTM!The renderer function is clean, delegates appropriately to helper functions, and produces a well-structured output. Locale handling for titles and dates is correctly implemented.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
e2e-tests/tests/rcj-lists-viewing.spec.ts (2)
7-15: Same duplication concern as other test files.The
authenticateSystemAdminhelper is duplicated here as well. Consider a shared utility extraction as previously noted.
41-72: Upload helper duplication.Same pattern as other files. Consolidation opportunity as noted previously.
🧹 Nitpick comments (12)
e2e-tests/tests/court-of-appeal-civil-viewing.spec.ts (5)
10-14: Non-null assertions on environment variables may fail unhelpfully.If these environment variables are not set, the code will throw a cryptic error. Consider adding explicit validation with a meaningful error message.
Suggested improvement
if (page.url().includes("login.microsoftonline.com")) { - const systemAdminEmail = process.env.SSO_TEST_SYSTEM_ADMIN_EMAIL!; - const systemAdminPassword = process.env.SSO_TEST_SYSTEM_ADMIN_PASSWORD!; + const systemAdminEmail = process.env.SSO_TEST_SYSTEM_ADMIN_EMAIL; + const systemAdminPassword = process.env.SSO_TEST_SYSTEM_ADMIN_PASSWORD; + if (!systemAdminEmail || !systemAdminPassword) { + throw new Error("SSO_TEST_SYSTEM_ADMIN_EMAIL and SSO_TEST_SYSTEM_ADMIN_PASSWORD must be set"); + } await loginWithSSO(page, systemAdminEmail, systemAdminPassword); }
57-71: Prefer role-based or label-based selectors over attribute selectors.Per coding guidelines, E2E tests should use selectors in priority order:
getByRole(),getByLabel(),getByText(),getByTestId(). Several form interactions here usepage.fill('input[name="..."]')andpage.selectOption('select[name="..."]').Also,
waitForTimeout(1000)on line 58 is flaky—consider waiting for a specific element to be visible instead.Example for date inputs
- await page.waitForTimeout(1000); + await page.waitForLoadState("domcontentloaded"); - await page.fill('input[name="hearingStartDate-day"]', "15"); + await page.getByLabel(/hearing.*start.*day/i).fill("15");
60-60: Magic number "19" for listType lacks context.Consider extracting to a named constant for clarity.
Suggested improvement
+const COURT_OF_APPEAL_CIVIL_LIST_TYPE = "19"; + // In uploadCourtOfAppealCivilList: -await page.selectOption('select[name="listType"]', "19"); +await page.selectOption('select[name="listType"]', COURT_OF_APPEAL_CIVIL_LIST_TYPE);
99-99: ReplacewaitForTimeout()with explicit waits.Hardcoded timeouts are flaky and slow tests unnecessarily. Use
waitForLoadState(),waitForSelector(), orexpect().toBeVisible()instead.Example replacement
- await page.waitForTimeout(1000); + await page.locator('.govuk-list').waitFor({ state: "visible" });- await page.waitForTimeout(500); + await expect(page.locator("tbody tr:visible")).toHaveCount(1);Also applies to: 159-159, 168-168, 207-207, 235-235, 253-253, 269-269
249-274: Keyboard navigation test is good but could include accessibility scan.The test correctly verifies Tab navigation and back-to-top functionality. Consider adding an Axe check for completeness, or merge this into the main journey test.
e2e-tests/tests/london-administrative-court-viewing.spec.ts (5)
7-15: Consider extracting shared authentication helper.The
authenticateSystemAdminfunction is duplicated across all three E2E test files. Extract it toe2e-tests/utils/alongsidesso-helpers.tsto reduce duplication.Additionally, the non-null assertions on lines 11-12 will cause runtime errors if the environment variables are not set. Consider adding a guard with a descriptive error message.
Suggested improvement
async function authenticateSystemAdmin(page: Page) { await page.goto("/system-admin-dashboard"); if (page.url().includes("login.microsoftonline.com")) { - const systemAdminEmail = process.env.SSO_TEST_SYSTEM_ADMIN_EMAIL!; - const systemAdminPassword = process.env.SSO_TEST_SYSTEM_ADMIN_PASSWORD!; + const systemAdminEmail = process.env.SSO_TEST_SYSTEM_ADMIN_EMAIL; + const systemAdminPassword = process.env.SSO_TEST_SYSTEM_ADMIN_PASSWORD; + if (!systemAdminEmail || !systemAdminPassword) { + throw new Error("SSO_TEST_SYSTEM_ADMIN_EMAIL and SSO_TEST_SYSTEM_ADMIN_PASSWORD must be set"); + } await loginWithSSO(page, systemAdminEmail, systemAdminPassword); } }
54-85: Prefer explicit wait conditions over fixed timeouts.
waitForTimeout(1000)at line 57 is flaky. Consider waiting for a specific element or network state instead.Per coding guidelines, prefer
getByLabel()for form inputs over CSS attribute selectors where labels exist.Example adjustment for timeout
async function uploadLondonAdminCourtList(page: Page) { await page.goto("/manual-upload?locationId=9001"); - await page.waitForTimeout(1000); + await page.waitForLoadState("networkidle");
92-153: Consider incorporating keyboard navigation into main test journey.Per coding guidelines, validation checks, Welsh translation checks, accessibility checks, and keyboard navigation should be included within a single E2E test journey. The keyboard navigation test at line 178 is currently separate.
Also,
waitForTimeout(500)at line 150 could be replaced with a more deterministic wait (e.g., waiting for the row count to change).Based on learnings.
155-176: Consider adding accessibility check to this test.This test lacks an axe-core accessibility scan. Given it tests a different UI state (expanded details section), including an accessibility check would improve coverage.
178-198: Keyboard focus test may be brittle.The assumption that the search input is focused after exactly 2 Tab presses (lines 189-192) is fragile and could break if the DOM structure changes. Consider a more resilient approach or documenting the expected tab order.
Missing accessibility scan in this test as well.
e2e-tests/tests/administrative-court-lists-viewing.spec.ts (2)
7-15: Duplicated helper function.This
authenticateSystemAdminfunction is identical to the one inlondon-administrative-court-viewing.spec.ts. Extract to a shared utility ine2e-tests/utils/.Same concern about non-null assertions on environment variables.
41-72: Upload helper is highly similar across test files.This
uploadAdminCourtListfunction shares ~90% code withuploadLondonAdminCourtListanduploadRCJList. Consider extracting a generic upload helper that accepts list type ID and JSON content as parameters.Possible extraction
// e2e-tests/utils/upload-helpers.ts export async function uploadList( page: Page, listTypeId: string, jsonContent: string, filename: string ) { await page.goto("/manual-upload?locationId=9001"); await page.waitForLoadState("networkidle"); // ... common upload logic }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
e2e-tests/tests/administrative-court-lists-viewing.spec.tse2e-tests/tests/court-of-appeal-civil-viewing.spec.tse2e-tests/tests/london-administrative-court-viewing.spec.tse2e-tests/tests/rcj-lists-viewing.spec.tslibs/web-core/package.jsonlibs/web-core/src/assets/js/search-highlight.test.tspackage.json
💤 Files with no reviewable changes (2)
- libs/web-core/package.json
- libs/web-core/src/assets/js/search-highlight.test.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
**/package.json: Use@hmctsscope for package names:@hmcts/auth,@hmcts/case-management
All package.json files must use"type": "module"for ES modules support
Express version must be 5.x only ("express": "5.2.0")
All packages must use"test": "vitest run"script in package.json
Dependencies must use specific versions only (e.g.,"express": "5.2.0"), except for peer dependencies
Module build script must include"build:nunjucks"script if module contains Nunjucks templates in pages/ directory
Files:
package.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use camelCase for TypeScript variables:userId,caseDetails,documentId
Use PascalCase for classes and interfaces:UserService,CaseRepository. NOIprefix.
Use kebab-case for file and directory names:user-service.ts,case-management/
Use boolean variable names withis/has/canprefix:isActive,hasAccess,canEdit
Module ordering: constants at the top, exported functions next, other functions in order of use, interfaces and types at the bottom
Always add.jsextension to relative imports in ES modules (e.g.,import { foo } from "./bar.js")
Use workspace aliases for imports (@hmcts/*) instead of relative paths across packages
Use strict TypeScript mode enabled; noanywithout justification
Use parameterized database queries with Prisma (no raw SQL string concatenation)
Do not include sensitive data in logs
Files:
e2e-tests/tests/london-administrative-court-viewing.spec.tse2e-tests/tests/administrative-court-lists-viewing.spec.tse2e-tests/tests/rcj-lists-viewing.spec.tse2e-tests/tests/court-of-appeal-civil-viewing.spec.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use SCREAMING_SNAKE_CASE for constants:
MAX_FILE_SIZE,DEFAULT_TIMEOUT
Files:
e2e-tests/tests/london-administrative-court-viewing.spec.tse2e-tests/tests/administrative-court-lists-viewing.spec.tse2e-tests/tests/rcj-lists-viewing.spec.tse2e-tests/tests/court-of-appeal-civil-viewing.spec.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Test files must be co-located with source code using
*.test.tsor*.spec.tsnaming pattern
Files:
e2e-tests/tests/london-administrative-court-viewing.spec.tse2e-tests/tests/administrative-court-lists-viewing.spec.tse2e-tests/tests/rcj-lists-viewing.spec.tse2e-tests/tests/court-of-appeal-civil-viewing.spec.ts
e2e-tests/**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
e2e-tests/**/*.spec.ts: E2E tests must be located ine2e-tests/directory with*.spec.tsnaming pattern
Tag nightly-only E2E tests with@nightlyin the test title
E2E tests should use selectors in priority order: getByRole(), getByLabel(), getByText(), getByTestId()
Accessibility testing (WCAG 2.2 AA) is mandatory - include axe-core checks in E2E tests
Include validation checks, Welsh translation checks, accessibility checks, and keyboard navigation within a single E2E test journey
Do not test visual styling (fonts, colors, margins, padding) in E2E tests
Files:
e2e-tests/tests/london-administrative-court-viewing.spec.tse2e-tests/tests/administrative-court-lists-viewing.spec.tse2e-tests/tests/rcj-lists-viewing.spec.tse2e-tests/tests/court-of-appeal-civil-viewing.spec.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Input validation must be performed on all endpoints
Files:
e2e-tests/tests/london-administrative-court-viewing.spec.tse2e-tests/tests/administrative-court-lists-viewing.spec.tse2e-tests/tests/rcj-lists-viewing.spec.tse2e-tests/tests/court-of-appeal-civil-viewing.spec.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not use CommonJS - ES modules only with import/export syntax
Files:
e2e-tests/tests/london-administrative-court-viewing.spec.tse2e-tests/tests/administrative-court-lists-viewing.spec.tse2e-tests/tests/rcj-lists-viewing.spec.tse2e-tests/tests/court-of-appeal-civil-viewing.spec.ts
🧠 Learnings (6)
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to e2e-tests/**/*.spec.ts : Include validation checks, Welsh translation checks, accessibility checks, and keyboard navigation within a single E2E test journey
Applied to files:
e2e-tests/tests/london-administrative-court-viewing.spec.tse2e-tests/tests/administrative-court-lists-viewing.spec.tse2e-tests/tests/rcj-lists-viewing.spec.tse2e-tests/tests/court-of-appeal-civil-viewing.spec.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to e2e-tests/**/*.spec.ts : Accessibility testing (WCAG 2.2 AA) is mandatory - include axe-core checks in E2E tests
Applied to files:
e2e-tests/tests/london-administrative-court-viewing.spec.tse2e-tests/tests/administrative-court-lists-viewing.spec.tse2e-tests/tests/rcj-lists-viewing.spec.tse2e-tests/tests/court-of-appeal-civil-viewing.spec.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to e2e-tests/**/*.spec.ts : E2E tests must be located in `e2e-tests/` directory with `*.spec.ts` naming pattern
Applied to files:
e2e-tests/tests/london-administrative-court-viewing.spec.tse2e-tests/tests/administrative-court-lists-viewing.spec.tse2e-tests/tests/court-of-appeal-civil-viewing.spec.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to e2e-tests/**/*.spec.ts : Tag nightly-only E2E tests with `nightly` in the test title
Applied to files:
e2e-tests/tests/london-administrative-court-viewing.spec.tse2e-tests/tests/administrative-court-lists-viewing.spec.tse2e-tests/tests/rcj-lists-viewing.spec.tse2e-tests/tests/court-of-appeal-civil-viewing.spec.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to e2e-tests/**/*.spec.ts : Do not test visual styling (fonts, colors, margins, padding) in E2E tests
Applied to files:
e2e-tests/tests/london-administrative-court-viewing.spec.tse2e-tests/tests/administrative-court-lists-viewing.spec.tse2e-tests/tests/rcj-lists-viewing.spec.tse2e-tests/tests/court-of-appeal-civil-viewing.spec.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to e2e-tests/**/*.spec.ts : E2E tests should use selectors in priority order: getByRole(), getByLabel(), getByText(), getByTestId()
Applied to files:
e2e-tests/tests/london-administrative-court-viewing.spec.ts
🧬 Code graph analysis (4)
e2e-tests/tests/london-administrative-court-viewing.spec.ts (1)
e2e-tests/utils/sso-helpers.ts (1)
loginWithSSO(10-46)
e2e-tests/tests/administrative-court-lists-viewing.spec.ts (1)
e2e-tests/utils/sso-helpers.ts (1)
loginWithSSO(10-46)
e2e-tests/tests/rcj-lists-viewing.spec.ts (1)
e2e-tests/utils/sso-helpers.ts (1)
loginWithSSO(10-46)
e2e-tests/tests/court-of-appeal-civil-viewing.spec.ts (1)
e2e-tests/utils/sso-helpers.ts (1)
loginWithSSO(10-46)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests
🔇 Additional comments (7)
e2e-tests/tests/court-of-appeal-civil-viewing.spec.ts (3)
1-4: Imports look correct.Proper use of
.jsextension on relative import per ES modules guidelines.
93-162: Comprehensive test covering validation, Welsh, accessibility, and search.This test correctly consolidates multiple checks (content validation, Welsh translation, accessibility, table search) into a single journey per the coding guidelines. Well structured.
145-148: Verify disabled accessibility rules are intentional.Disabling
target-sizeandlink-namerules should be documented or tracked. If these are known framework issues, consider adding a comment explaining why.package.json (1)
70-72: Version resolutions appropriately pinned and secure.
qs6.14.1 is the current latest version and includes the critical fix for CVE-2025-15284.undici6.23.0 is secure with patches for all identified CVEs in the 6.x line. The practice of pinning specific versions aligns with dependency management best practices.e2e-tests/tests/london-administrative-court-viewing.spec.ts (1)
17-52: LGTM!Test fixture data is well-structured with representative fields for both main hearings and planning court sections.
e2e-tests/tests/rcj-lists-viewing.spec.ts (2)
17-39: LGTM!Test fixture data is appropriate for RCJ Standard lists.
129-155: LGTM with suggestion.Test validates page structure, headers, and accessibility. Consider adding Welsh translation checks for consistency with other tests in this suite.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| test.describe("Administrative Court Daily Cause Lists - Viewing @nightly", () => { | ||
| test.beforeEach(async ({ page }) => { | ||
| await authenticateSystemAdmin(page); | ||
| }); | ||
|
|
||
| test("should view Birmingham Administrative Court list with English and Welsh content", async ({ page }) => { | ||
| // Upload list | ||
| await uploadAdminCourtList(page, "20"); | ||
|
|
||
| // Navigate to summary of publications | ||
| await page.goto("/summary-of-publications?locationId=9001"); | ||
| await page.waitForTimeout(1000); | ||
|
|
||
| // Find and click the publication link | ||
| const publicationLinks = page.locator('.govuk-list a[href*="artefactId="]'); | ||
| await expect(publicationLinks.first()).toBeVisible(); | ||
| const firstLinkHref = await publicationLinks.first().getAttribute("href"); | ||
| expect(firstLinkHref).toContain("/birmingham-administrative-court-daily-cause-list?artefactId="); | ||
|
|
||
| await publicationLinks.first().click(); | ||
| await page.waitForLoadState("networkidle"); | ||
|
|
||
| // Verify English content | ||
| await expect(page.locator("h1")).toContainText("Birmingham Administrative Court Daily Cause List"); | ||
| await expect(page.locator("body")).toContainText("List for 15 January 2026"); | ||
| await expect(page.locator("body")).toContainText("Last updated"); | ||
| await expect(page.locator("body")).toContainText("Court 1"); | ||
| await expect(page.locator("body")).toContainText("Mr Justice Williams"); | ||
| await expect(page.locator("body")).toContainText("R (Smith) v Secretary of State"); | ||
|
|
||
| // Verify time normalization (dot replaced with colon) | ||
| await expect(page.locator("tbody")).toContainText("10:00am"); | ||
| await expect(page.locator("tbody")).toContainText("2:30pm"); | ||
|
|
||
| // Test Welsh translation | ||
| await page.getByRole("link", { name: "Cymraeg" }).click(); | ||
| await page.waitForLoadState("networkidle"); | ||
| await expect(page.locator("body")).toContainText("Rhestr ar gyfer 15 Ionawr 2026"); | ||
| await expect(page.locator("body")).toContainText("Diweddarwyd ddiwethaf"); | ||
| await expect(page.locator("body")).toContainText("Lleoliad"); | ||
|
|
||
| // Test accessibility | ||
| const accessibilityScanResults = await new AxeBuilder({ page }) | ||
| .disableRules(["target-size", "link-name"]) | ||
| .analyze(); | ||
| expect(accessibilityScanResults.violations).toEqual([]); | ||
|
|
||
| // Test table search functionality | ||
| const searchInput = page.locator('input[id="case-search-input"]'); | ||
| await expect(searchInput).toBeVisible(); | ||
| await searchInput.fill("Smith"); | ||
| await page.waitForTimeout(500); | ||
| await expect(page.locator("tbody tr:visible")).toHaveCount(1); | ||
| }); |
There was a problem hiding this comment.
Good coverage but missing keyboard navigation.
This test includes Welsh translation, accessibility, and search validation. However, per coding guidelines, keyboard navigation should be included within a single E2E test journey. Consider adding keyboard navigation checks to this test.
Based on learnings.
| test("should view Manchester Administrative Court list and verify data source", async ({ page }) => { | ||
| await uploadAdminCourtList(page, "23"); | ||
|
|
||
| await page.goto("/summary-of-publications?locationId=9001"); | ||
| await page.waitForTimeout(1000); | ||
|
|
||
| const publicationLinks = page.locator('.govuk-list a[href*="artefactId="]'); | ||
| await publicationLinks.first().click(); | ||
| await page.waitForLoadState("networkidle"); | ||
|
|
||
| // Verify data source is shown | ||
| await expect(page.locator("body")).toContainText("Data source"); | ||
| await expect(page.locator("body")).toContainText("Manual Upload"); | ||
|
|
||
| // Test Welsh data source | ||
| await page.getByRole("link", { name: "Cymraeg" }).click(); | ||
| await page.waitForLoadState("networkidle"); | ||
| await expect(page.locator("body")).toContainText("Ffynhonnell data"); | ||
| await expect(page.locator("body")).toContainText("Llwytho â Llaw"); | ||
| }); |
There was a problem hiding this comment.
Missing accessibility check in Manchester test.
This test validates Welsh data source labels but lacks an axe-core accessibility scan, unlike the Birmingham and Leeds tests.
Add accessibility check
await expect(page.locator("body")).toContainText("Ffynhonnell data");
await expect(page.locator("body")).toContainText("Llwytho â Llaw");
+
+ // Test accessibility
+ const accessibilityScanResults = await new AxeBuilder({ page })
+ .disableRules(["target-size", "link-name"])
+ .analyze();
+ expect(accessibilityScanResults.violations).toEqual([]);
});
});| test("should display important information with live streaming and judgments sections", async ({ page }) => { | ||
| await uploadCourtOfAppealCivilList(page); | ||
|
|
||
| await page.goto("/summary-of-publications?locationId=9001"); | ||
| await page.waitForTimeout(1000); | ||
|
|
||
| const publicationLinks = page.locator('.govuk-list a[href*="artefactId="]'); | ||
| await publicationLinks.first().click(); | ||
| await page.waitForLoadState("networkidle"); | ||
|
|
||
| // Check for important information section | ||
| const importantInfoDetails = page.locator(".govuk-details"); | ||
| await expect(importantInfoDetails).toBeVisible(); | ||
| await expect(importantInfoDetails.locator(".govuk-details__summary-text")).toContainText("Important information"); | ||
|
|
||
| // Expand the details section | ||
| await importantInfoDetails.locator(".govuk-details__summary").click(); | ||
|
|
||
| // Check for live streaming section | ||
| await expect(importantInfoDetails).toContainText("Live streaming of Court of Appeal hearings"); | ||
|
|
||
| // Check for judgments section | ||
| await expect(importantInfoDetails).toContainText("Judgments"); | ||
| }); |
There was a problem hiding this comment.
Missing accessibility checks in separate tests.
Per coding guidelines, accessibility testing (WCAG 2.2 AA) is mandatory. These tests don't include Axe checks. Consider either:
- Adding accessibility checks to each test, or
- Consolidating test scenarios into fewer comprehensive journeys
Based on learnings, including validation, Welsh, accessibility, and keyboard navigation within a single E2E test journey is preferred.
Also applies to: 189-247
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (12)
libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.ts (2)
77-88: Malformed JSON returns 500 instead of 400.
JSON.parseon line 77 is outside the inner try-catch. If the file contains invalid JSON, the error propagates to the outer catch block and returns a 500 status code. It should return 400 (bad request).Proposed fix
let jsonContent: string; + let jsonData: LondonAdminCourtData; try { jsonContent = await readFile(jsonFilePath, "utf-8"); + jsonData = JSON.parse(jsonContent); } catch (error) { - console.error(`Error reading JSON file at ${jsonFilePath}:`, error); - return res.status(404).render("errors/common", { + console.error(`Error reading or parsing JSON file for artefact ${artefactId}:`, error); + return res.status(400).render("errors/common", { en, cy, - errorTitle: "Not Found", - errorMessage: "The requested list could not be found" + errorTitle: "Invalid Data", + errorMessage: "The list data could not be read or parsed" }); } - - const jsonData: LondonAdminCourtData = JSON.parse(jsonContent);
68-68: Avoid logging full file paths.Logging
jsonFilePathexposes internal directory structure. Log only theartefactIdor a sanitised reference. As per coding guidelines, sensitive data should not appear in logs.Proposed fix
- console.error(`Error reading JSON file at ${jsonFilePath}:`, error); + console.error(`Error reading JSON file for artefact ${artefactId}:`, error);libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts (3)
62-62: Path traversal vulnerability remains unaddressed.User-supplied
artefactIdis interpolated directly into the file path without validation. Sanitise the input or validate that the resolved path remains withinTEMP_UPLOAD_DIR.Proposed fix
+ // Prevent path traversal attacks + const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + if (!UUID_REGEX.test(artefactId)) { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Bad Request", + errorMessage: "Invalid artefactId format" + }); + } + const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`);
68-68: Avoid logging full file paths.Logging
jsonFilePathexposes internal directory structure. Log only theartefactId.Proposed fix
- console.error(`Error reading JSON file at ${jsonFilePath}:`, error); + console.error(`Error reading JSON file for artefact ${artefactId}:`, error);
77-88: Malformed JSON returns 500 instead of 400.
JSON.parsecan throw on invalid JSON, which falls through to the outer catch block returning a 500 error. This should be a 400 Bad Request.Proposed fix
- const jsonData: CourtOfAppealCivilData = JSON.parse(jsonContent); + let jsonData: CourtOfAppealCivilData; + try { + jsonData = JSON.parse(jsonContent); + } catch { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Invalid Data", + errorMessage: "The list data could not be parsed" + }); + }libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (4)
40-40: ValidateartefactIdtype before use.
req.query.artefactIdcan bestring | string[] | undefined. The cast tostringsilently allows arrays through.Proposed fix
- const artefactId = req.query.artefactId as string; + const artefactId = typeof req.query.artefactId === "string" ? req.query.artefactId : undefined;
77-77: Path traversal risk: sanitiseartefactIdbefore file access.User-supplied
artefactIdis interpolated directly into the file path. Validate the format (e.g., UUID regex) before constructing the path.Proposed fix
+ // Validate artefactId format (UUID expected) + const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + if (!UUID_REGEX.test(artefactId)) { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Bad Request", + errorMessage: "Invalid artefactId format" + }); + } + const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`);
92-103: Malformed JSON returns 500 instead of 400.Same issue as other modules:
JSON.parseis outside the inner try-catch. Move it inside to return 400 for invalid JSON.Proposed fix
let jsonContent: string; + let jsonData: StandardHearingList; try { jsonContent = await readFile(jsonFilePath, "utf-8"); + jsonData = JSON.parse(jsonContent); } catch (error) { - console.error(`Error reading JSON file at ${jsonFilePath}:`, error); + console.error(`Error reading or parsing JSON file for artefact ${artefactId}:`, error); return res.status(404).render("errors/common", { en, cy, errorTitle: "Not Found", errorMessage: "The requested list could not be found" }); } - - const jsonData: StandardHearingList = JSON.parse(jsonContent);
83-83: Avoid logging full file paths.Logging
jsonFilePathexposes internal directory structure.Proposed fix
- console.error(`Error reading JSON file at ${jsonFilePath}:`, error); + console.error(`Error reading JSON file for artefact ${artefactId}:`, error);libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (3)
48-48: ValidateartefactIdtype before use.Same issue as the administrative court module:
req.query.artefactIdcan bestring | string[] | undefined. The cast silently allows arrays through.Proposed fix
- const artefactId = req.query.artefactId as string; + const artefactId = typeof req.query.artefactId === "string" ? req.query.artefactId : undefined;
85-85: Path traversal risk: sanitiseartefactIdbefore file access.User-supplied
artefactIdis interpolated directly into the file path without validation. Validate the format (e.g., UUID regex) before constructing the path.Proposed fix
+ // Validate artefactId format (UUID expected) + const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + if (!UUID_REGEX.test(artefactId)) { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Bad Request", + errorMessage: "Invalid artefactId format" + }); + } + const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`);
100-111: Malformed JSON returns 500 instead of 400.Consistent with the other modules,
JSON.parseshould be moved inside the try-catch to return 400 for invalid JSON rather than 500.Proposed fix
let jsonContent: string; + let jsonData: StandardHearingList; try { jsonContent = await readFile(jsonFilePath, "utf-8"); + jsonData = JSON.parse(jsonContent); } catch (error) { - console.error(`Error reading JSON file at ${jsonFilePath}:`, error); + console.error(`Error reading or parsing JSON file for artefact ${artefactId}:`, error); return res.status(404).render("errors/common", { en, cy, errorTitle: "Not Found", errorMessage: "The requested list could not be found" }); } - - const jsonData: StandardHearingList = JSON.parse(jsonContent);
🧹 Nitpick comments (3)
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.test.ts (1)
81-83: Consider usingpath.relative()for more robust path verification.
startsWith()on strings can produce false positives (e.g.,/module/assets-oldwould match/module/assets). Usingpath.relative()as done forpageRoutes.pathon line 77-78 would be more reliable.♻️ Suggested improvement
it("assets should be subdirectory of moduleRoot", () => { - expect(assets.startsWith(moduleRoot)).toBe(true); + const relativePath = path.relative(moduleRoot, assets); + expect(relativePath).toBe("assets"); });libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (1)
123-123: Consider adding type safety to list content lookup.The
(t as any)[listTypeId]cast bypasses TypeScript's type checking. Consider defining a proper type for the locale objects that includes the numeric keys.Example approach
// In en.ts / cy.ts, add a type: type ListTypeLocale = { [key: number]: { pageTitle: string; importantInfoText: string; judgmentsTitle: string; judgmentsText: string }; common: { /* ... */ }; }; // Then in the handler: const listContent = t[listTypeId as keyof typeof t] || {};libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (1)
1-42: Consider extracting common handler logic into a shared utility.All four route handlers in this PR share identical patterns for artefact retrieval, file reading, JSON parsing, validation, and error handling. Consider extracting this into a reusable factory or helper in
@hmcts/list-types-commonto reduce duplication and centralise security fixes.// Example: libs/list-types/common/src/page-handler.ts export function createListPageHandler<TData>(options: { schemaPath: string; listTypeIds: number[]; renderer: (data: TData, opts: RenderOptions) => RenderedData; templateName: string; }) { /* ... */ }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
libs/list-types/administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.test.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.test.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/list-types/care-standards-tribunal-weekly-hearing-list/src/pages/index.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use camelCase for TypeScript variables:userId,caseDetails,documentId
Use PascalCase for classes and interfaces:UserService,CaseRepository. NOIprefix.
Use kebab-case for file and directory names:user-service.ts,case-management/
Use boolean variable names withis/has/canprefix:isActive,hasAccess,canEdit
Module ordering: constants at the top, exported functions next, other functions in order of use, interfaces and types at the bottom
Always add.jsextension to relative imports in ES modules (e.g.,import { foo } from "./bar.js")
Use workspace aliases for imports (@hmcts/*) instead of relative paths across packages
Use strict TypeScript mode enabled; noanywithout justification
Use parameterized database queries with Prisma (no raw SQL string concatenation)
Do not include sensitive data in logs
Files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.test.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use SCREAMING_SNAKE_CASE for constants:
MAX_FILE_SIZE,DEFAULT_TIMEOUT
Files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.test.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Test files must be co-located with source code using
*.test.tsor*.spec.tsnaming pattern
Files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.test.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Input validation must be performed on all endpoints
Files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.test.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not use CommonJS - ES modules only with import/export syntax
Files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.test.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.ts
**/config.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Module config.ts must export standardized interfaces: pageRoutes, apiRoutes, prismaSchemas, assets
Files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.ts
🧠 Learnings (8)
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/config.ts : Module config.ts must export standardized interfaces: pageRoutes, apiRoutes, prismaSchemas, assets
Applied to files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.test.tslibs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.tslibs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/*.{test,spec}.ts : Test files must be co-located with source code using `*.test.ts` or `*.spec.ts` naming pattern
Applied to files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.test.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to e2e-tests/**/*.spec.ts : E2E tests must be located in `e2e-tests/` directory with `*.spec.ts` naming pattern
Applied to files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.test.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/tsconfig.json : Module tsconfig.json must extend root tsconfig and configure outDir, rootDir, declaration, and declarationMap
Applied to files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.test.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to **/*.{ts,tsx} : Use workspace aliases for imports (`hmcts/*`) instead of relative paths across packages
Applied to files:
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.ts : Create page controller files with GET and POST exports following the pattern: `export const GET = async (req, res) => { ... }`
Applied to files:
libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/routes/**/*.ts : API endpoints should use plural for collections (/api/cases), singular for specific (/api/case/:id), and singular for creation (POST /api/case)
Applied to files:
libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.ts
📚 Learning: 2025-12-15T13:38:57.497Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-15T13:38:57.497Z
Learning: Applies to libs/*/src/pages/**/*.ts : Provide both `en` and `cy` language objects in page controllers for English and Welsh support
Applied to files:
libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.tslibs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.tslibs/list-types/rcj-standard-daily-cause-list/src/pages/index.tslibs/list-types/administrative-court-daily-cause-list/src/pages/index.ts
🧬 Code graph analysis (6)
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.test.ts (1)
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.ts (2)
assets(12-12)moduleRoot(7-7)
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.ts (1)
e2e-tests/run-with-credentials.js (1)
__dirname(11-11)
libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.ts (5)
libs/list-types/common/src/validation/json-validator.ts (1)
createJsonValidator(12-36)libs/list-types/london-administrative-court-daily-cause-list/src/pages/cy.ts (1)
cy(1-40)libs/list-types/london-administrative-court-daily-cause-list/src/pages/en.ts (1)
en(1-40)libs/list-types/london-administrative-court-daily-cause-list/src/models/types.ts (1)
LondonAdminCourtData(11-14)libs/list-types/london-administrative-court-daily-cause-list/src/rendering/renderer.ts (1)
renderLondonAdminCourt(75-90)
libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts (6)
libs/list-types/common/src/index.ts (1)
createJsonValidator(18-18)libs/list-types/common/src/validation/json-validator.ts (1)
createJsonValidator(12-36)libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/cy.ts (1)
cy(1-46)libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/en.ts (1)
en(1-46)libs/list-types/court-of-appeal-civil-daily-cause-list/src/models/types.ts (1)
CourtOfAppealCivilData(15-18)libs/list-types/court-of-appeal-civil-daily-cause-list/src/rendering/renderer.ts (1)
renderCourtOfAppealCivil(102-116)
libs/list-types/rcj-standard-daily-cause-list/src/pages/index.ts (3)
libs/list-types/common/src/index.ts (1)
createJsonValidator(18-18)libs/list-types/common/src/validation/json-validator.ts (1)
createJsonValidator(12-36)libs/list-types/rcj-standard-daily-cause-list/src/rendering/renderer.ts (1)
renderStandardDailyCauseList(62-85)
libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (4)
libs/list-types/common/src/validation/json-validator.ts (1)
createJsonValidator(12-36)libs/list-types/administrative-court-daily-cause-list/src/pages/en.ts (1)
en(1-62)libs/list-types/administrative-court-daily-cause-list/src/pages/cy.ts (1)
cy(1-62)libs/list-types/administrative-court-daily-cause-list/src/rendering/renderer.ts (1)
renderAdminCourt(62-85)
🪛 GitHub Actions: Test
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.test.ts
[error] 20-20: Test failed: should export assets with correct path. AssertionError: expected undefined to be defined.
[error] 36-36: Test failed: should have assets path as sibling to src. AssertionError: expected undefined to be '/home/runner/work/cath-service/cath-service/libs/list-types/civil-and-family-daily-cause-list/assets/'
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests
🔇 Additional comments (5)
libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.test.ts (1)
1-4: Pipeline failures originate fromcivil-and-family-daily-cause-listmodule, not this file.The
civil-and-family-daily-cause-list/src/config.tsdoes not exportassets, but its tests expect it to exist and equalpath.join(__dirname, "../assets/"). Add the missing export to that module's config file.libs/list-types/care-standards-tribunal-weekly-hearing-list/src/config.ts (1)
7-12: The exports in this file align with the established pattern across all list-type modules in the codebase. Each module exports only what is needed:pageRoutesfor page-based modules,apiRoutesfor API modules, and shared exports likemoduleRootandassets. Adding unusedapiRoutesandprismaSchemaswould be inconsistent with the actual codebase conventions.Likely an incorrect or invalid review comment.
libs/list-types/london-administrative-court-daily-cause-list/src/pages/index.ts (1)
1-11: LGTM on imports and module structure.Imports follow ES module syntax with
.jsextensions on relative imports, workspace aliases are used correctly, and bothenandcylocale objects are provided as per coding guidelines. Based on learnings, this follows the expected page controller pattern.libs/list-types/court-of-appeal-civil-daily-cause-list/src/pages/index.ts (1)
27-27: LGTM on artefactId type validation.The type check
typeof req.query.artefactId === "string"properly validates the query parameter before use.libs/list-types/administrative-court-daily-cause-list/src/pages/index.ts (1)
13-18: LGTM on ROUTES and LIST_TYPE_CONFIG structure.The ROUTES array and LIST_TYPE_CONFIG mapping provide a clean configuration-driven approach for handling multiple city-specific Administrative Court lists with localised titles.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| }); | ||
| } | ||
|
|
||
| const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`); |
There was a problem hiding this comment.
Path traversal risk: sanitise artefactId before file access.
User-supplied artefactId is interpolated directly into the file path. A malicious value such as ../../etc/passwd could read arbitrary files. Validate the format (e.g., UUID regex) or verify the resolved path stays within TEMP_UPLOAD_DIR.
Proposed fix
+ // Validate artefactId format (UUID expected)
+ const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
+ if (!UUID_REGEX.test(artefactId)) {
+ return res.status(400).render("errors/common", {
+ en,
+ cy,
+ errorTitle: "Bad Request",
+ errorMessage: "Invalid artefactId format"
+ });
+ }
+
const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`);📝 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 jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`); | |
| // Validate artefactId format (UUID expected) | |
| const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; | |
| if (!UUID_REGEX.test(artefactId)) { | |
| return res.status(400).render("errors/common", { | |
| en, | |
| cy, | |
| errorTitle: "Bad Request", | |
| errorMessage: "Invalid artefactId format" | |
| }); | |
| } | |
| const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`); |
| try { | ||
| jsonContent = await readFile(jsonFilePath, "utf-8"); | ||
| } catch (error) { | ||
| console.error(`Error reading JSON file at ${jsonFilePath}:`, error); |
There was a problem hiding this comment.
Avoid logging full file paths.
Logging jsonFilePath exposes internal directory structure.
Proposed fix
- console.error(`Error reading JSON file at ${jsonFilePath}:`, error);
+ console.error(`Error reading JSON file for artefact ${artefactId}:`, error);📝 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.
| console.error(`Error reading JSON file at ${jsonFilePath}:`, error); | |
| console.error(`Error reading JSON file for artefact ${artefactId}:`, error); |
…e-rcj-hearing-lists
…e-rcj-hearing-lists # Conflicts: # apps/web/src/app.test.ts # package.json # yarn.lock
| const listType = mockListTypes.find((lt) => lt.id === artefact.listTypeId); | ||
| const location = await getLocationById(Number.parseInt(artefact.locationId)); | ||
| const location = await getLocationById(Number.parseInt(artefact.locationId, 10)); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find Prisma schema and search for Artefact model definition
find . -name "schema.prisma" -o -name "*.prisma" | head -20Repository: hmcts/cath-service
Length of output: 219
🏁 Script executed:
# Search for Artefact model and locationId in Prisma schemas
rg -n 'model Artefact' --type prisma -A 20Repository: hmcts/cath-service
Length of output: 92
🏁 Script executed:
# Read the actual code at lines 207-209 with context
cat -n libs/publication/src/repository/queries.ts | sed -n '200,220p'Repository: hmcts/cath-service
Length of output: 937
🏁 Script executed:
# Search for createArtefact patterns and locationId usage
rg -n 'createArtefact|locationId' libs/publication/src/repository/ -A 3 -B 1 --type tsRepository: hmcts/cath-service
Length of output: 29642
🏁 Script executed:
# Check types/interfaces for Artefact definition
rg -n 'interface Artefact|type Artefact' -A 15 --type tsRepository: hmcts/cath-service
Length of output: 5111
🏁 Script executed:
# Read the Prisma schema to check Artefact model and locationId constraints
cat -n ./apps/postgres/prisma/schema.prisma | grep -A 20 "model Artefact"Repository: hmcts/cath-service
Length of output: 1115
🏁 Script executed:
# Find getLocationById function definition
rg -n 'getLocationById' libs/publication/src/ -A 5 -B 2 --type tsRepository: hmcts/cath-service
Length of output: 6903
🏁 Script executed:
# Check if there are any validation utilities for locationId
rg -n 'isNumeric|parseNumber|parseInt' libs/publication/src/ --type ts | head -20Repository: hmcts/cath-service
Length of output: 192
Validate locationId before parsing or add a runtime guard.
The Prisma schema stores locationId as a plain String with no numeric constraints, yet the code calls Number.parseInt(artefact.locationId, 10) without verification. If a non-numeric value enters the database, this will result in NaN being passed to getLocationById. Add a guard clause or validation to handle this gracefully (e.g., log a warning, return early, or ensure database constraints enforce numeric-only strings).
…g-lists # Conflicts: # package.json # yarn.lock
|
2 participants
Jira link
https://tools.hmcts.net/jira/browse/VIBE-317
Change description
Add RCJ list types
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.