feat(#428): Add non-strategic hearing list types for SIAC, POAC, PAAC, FTT Tax Chamber, FTT LRT and FTT RPT - #749
Conversation
…, FTT Tax Chamber, FTT LRT and FTT RPT Adds 10 new list types (IDs 28–37) to the non-strategic publishing route, each with a validation schema, Excel converter, email summary, PDF template, Nunjucks page template, and English/Welsh locales. List types added: - SIAC Weekly Hearing List (ID 28, London) - POAC Weekly Hearing List (ID 29, London) - PAAC Weekly Hearing List (ID 30, London) - FTT Tax Chamber Weekly Hearing List (ID 31, National) - FTT Lands Registration Tribunal Weekly Hearing List (ID 32, National) - FTT RPT Eastern Region Weekly Hearing List (ID 33) - FTT RPT London Region Weekly Hearing List (ID 34) - FTT RPT Midlands Region Weekly Hearing List (ID 35) - FTT RPT Northern Region Weekly Hearing List (ID 36) - FTT RPT Southern Region Weekly Hearing List (ID 37) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds ten new tribunal weekly hearing list types (SIAC, POAC, PAAC, FTT Tax Chamber, FTT LRT, and five FTT RPT regions) as new lib packages with schemas, converters, renderers, PDF generators, locales, and templates, plus corresponding web page controllers, list-type registry entries, and notification/publication wiring. ChangesTribunal weekly hearing list rollout
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Publication
participant renderSiacPoacPaacData
Browser->>Publication: getArtefactById(artefactId)
Publication-->>Browser: artefact
Browser->>Publication: getPublicationJson(artefactId)
Publication-->>Browser: hearing list JSON
Browser->>renderSiacPoacPaacData: jsonData + options
renderSiacPoacPaacData-->>Browser: header + hearings
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎭 Playwright E2E Test Results84 tests 52 ✅ 6m 8s ⏱️ Results for commit f1b27ab. ♻️ This comment has been updated with latest results. |
- Add no-op migration file for 20260605141023_add_jurisdiction_soft_delete_and_audit_log - Fix summary-of-publications "for week commencing" text for all WEEKLY_HEARING_LIST types - Register all 10 new list types in PDF_GENERATOR_REGISTRY and EMAIL_BUILDER_REGISTRY - Add workspace dependencies for 4 new list type libs to publication and notifications - Split FTT Tax Chamber important information from single paragraph into 4 paragraphs - Fix FTT RPT short names: "RPT X" -> "FTT (RPT) X Weekly Hearing List" - Fix FTT RPT full names: remove "and", add colon, lowercase "region" across all files - Exclude .claude analytics files from Biome linting Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (36)
libs/location/src/list-type-data.ts-383-390 (1)
383-390:⚠️ Potential issue | 🟠 MajorFTT RPT friendly names deviate from ticket specification.
Lines 383, 395, 407, 419, and 431 show
englishFriendlyNameas "First-tier Tribunal (Residential Property Tribunal): Eastern region Weekly Hearing List" (missing "and", added colon, lowercase "region"), but ticket 428 specifies "First-tier Tribunal (Residential and Property Tribunal) Eastern Region Weekly Hearing List".Additionally,
shortenedFriendlyNameis "FTT (RPT) Eastern Weekly Hearing List", but the ticket specifies "RPT Eastern Weekly Hearing List" for the upload form label.These deviations affect user-visible text in the upload form. Align with the approved specification or update the ticket if these changes were intentional.
libs/list-types/siac-poac-paac-weekly-hearing-list/src/schemas/siac-poac-paac-weekly-hearing-list.json-13-13 (1)
13-13:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStrengthen
datevalidation to reject impossible calendar dates.Line 13 only validates shape, so invalid dates can pass validation and render as wrong dates downstream.
Suggested change
- "pattern": "^\\d{2}/\\d{2}/\\d{4}$", + "pattern": "^(0[1-9]|[12][0-9]|3[01])\\/(0[1-9]|1[0-2])\\/\\d{4}$",apps/web/src/pages/(list-types)/poac-weekly-hearing-list/index.ts-26-27 (1)
26-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate and constrain
artefactIdbefore building the JSON path.At Line 26 and Line 49, user input is interpolated into a filesystem path without explicit format validation or path containment checks. Add an allow-list check (e.g. UUID) and verify the resolved path remains under
TEMP_UPLOAD_DIRbefore reading.Also applies to: 49-49
apps/web/src/pages/(list-types)/poac-weekly-hearing-list/index.ts-32-34 (1)
32-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLocalise error title/message content for Welsh requests.
These user-facing error strings are hard-coded in English, so Welsh journeys still receive English copy on error branches. Route-level error text should be locale-driven like the rest of the page content.
As per coding guidelines, "Implement Welsh language support on all user-facing text with separate
en.tsandcy.tscontent files".Also applies to: 44-46, 72-74, 103-105
Source: Coding guidelines
libs/list-types/siac-poac-paac-weekly-hearing-list/src/locales/cy.ts-4-35 (1)
4-35:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftReplace English placeholders with Welsh copy for the
cylocale.Line 4 through Line 35 are still English, so Welsh journeys render untranslated content when
?lng=cyis used.As per coding guidelines, "Implement Welsh language support on all user-facing text with separate
en.tsandcy.tscontent files" and "Test pages with?lng=cyquery parameter to verify Welsh content during development."Source: Coding guidelines
libs/list-types/siac-poac-paac-weekly-hearing-list/src/pdf/pdf-generator.ts-62-62 (1)
62-62:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid non-null assertion on
sizeBytesfrom external PDF generation.Line 62 uses
pdfResult.sizeBytes!; ifsizeBytesis absent on a successful response, this can propagate invalid data into storage logic.Suggested fix
- return await savePdfToStorage(options.artefactId, pdfResult.pdfBuffer, pdfResult.sizeBytes!); + const sizeBytes = pdfResult.sizeBytes ?? pdfResult.pdfBuffer.length; + return await savePdfToStorage(options.artefactId, pdfResult.pdfBuffer, sizeBytes);libs/list-types/siac-poac-paac-weekly-hearing-list/src/pdf/pdf-generator.ts-32-33 (1)
32-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse artefact last-received time, not generation time, for PDF header metadata.
Line 32 currently uses
new Date().toISOString(), which makes “last updated” reflect PDF generation time rather than publication data time.Suggested fix
interface PdfGenerationOptions extends BasePdfGenerationOptions<SiacPoacPaacHearingList> { contentDate: Date; + lastReceivedDate: string; courtName: string; listTitle: string; } // ... - lastReceivedDate: new Date().toISOString(), + lastReceivedDate: options.lastReceivedDate,apps/web/src/pages/(list-types)/siac-weekly-hearing-list/index.ts-26-50 (1)
26-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
artefactIdbefore file-path construction to block traversal.Line 49 builds a filesystem path directly from user-controlled
artefactId. Without strict format validation, crafted values can escapeTEMP_UPLOAD_DIR.Suggested change
+const ARTEFACT_ID_PATTERN = /^[a-zA-Z0-9-]+$/; + export const GET = async (req: Request, res: Response) => { @@ const artefactId = req.query.artefactId as string; @@ if (!artefactId) { @@ } + + if (!ARTEFACT_ID_PATTERN.test(artefactId)) { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Bad Request", + errorMessage: "Invalid artefactId parameter" + }); + } @@ - const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`); + const jsonFilePath = path.resolve(TEMP_UPLOAD_DIR, `${artefactId}.json`); + if (!jsonFilePath.startsWith(`${TEMP_UPLOAD_DIR}${path.sep}`)) { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Bad Request", + errorMessage: "Invalid artefactId parameter" + }); + }libs/list-types/ftt-tax-chamber-weekly-hearing-list/src/models/types.ts-1-11 (1)
1-11:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake
membersoptional in the domain model.
membersis currently mandatory, but this list type is described as having an optional Members field. Keeping it required creates a contract mismatch and pushes callers to inject placeholder empty strings.Suggested change
export interface FttTaxChamberHearing { date: string; hearingTime: string; caseName: string; caseReferenceNumber: string; judges: string; - members: string; + members?: string; venuePlatform: string; }libs/list-types/ftt-tax-chamber-weekly-hearing-list/src/schemas/ftt-tax-chamber-weekly-hearing-list.json-8-8 (1)
8-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
membersshould not be inrequiredfor this schema.Line 8 currently makes
membersmandatory, which conflicts with the stated optional Members behaviour and will reject otherwise valid records at ingest time.Suggested change
- "required": ["date", "hearingTime", "caseName", "caseReferenceNumber", "judges", "members", "venuePlatform"], + "required": ["date", "hearingTime", "caseName", "caseReferenceNumber", "judges", "venuePlatform"],libs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/pdf/pdf-template.njk-2-2 (1)
2-2:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove hard-coded English in the Welsh-capable PDF template.
Line [2] fixes document language to
en, and Line [51] hard-codes “No hearings scheduled.”. This will produce mixed-language output when Welsh content is selected.Suggested patch
-<html lang="en"> +<html lang="{{ t.languageCode or 'en' }}"> @@ - <p>No hearings scheduled.</p> + <p>{{ t.noHearingsScheduled }}</p>Also applies to: 51-51
apps/web/src/pages/(list-types)/ftt-tax-chamber-weekly-hearing-list/index.ts-18-19 (1)
18-19:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace hard-coded repository traversal with configuration.
This path derivation is deployment-fragile and can break as soon as runtime layout changes. Read the upload root from an environment variable instead.
As per coding guidelines: “Use environment variables for all configuration values and secrets, never hardcode them”.
Source: Coding guidelines
apps/web/src/pages/(list-types)/ftt-tax-chamber-weekly-hearing-list/index.ts-26-26 (1)
26-26:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSanitise
artefactIdbefore using it in a filesystem path.Line [26] trusts query input via cast, and Line [49] interpolates it into a file path. Add strict type/format validation (and path containment checks) before file access.
Suggested patch
- const artefactId = req.query.artefactId as string; + const artefactIdRaw = req.query.artefactId; + if (typeof artefactIdRaw !== "string" || !/^[a-f0-9-]{36}$/i.test(artefactIdRaw)) { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Bad Request", + errorMessage: "Invalid artefactId parameter" + }); + } + const artefactId = artefactIdRaw; @@ - const jsonFilePath = path.join(TEMP_UPLOAD_DIR, `${artefactId}.json`); + const jsonFilePath = path.resolve(TEMP_UPLOAD_DIR, `${artefactId}.json`); + if (!jsonFilePath.startsWith(`${path.resolve(TEMP_UPLOAD_DIR)}${path.sep}`)) { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Bad Request", + errorMessage: "Invalid artefactId parameter" + }); + }Also applies to: 49-49
apps/web/src/pages/(list-types)/ftt-tax-chamber-weekly-hearing-list/index.ts-52-62 (1)
52-62:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not collapse all file-read failures into 404.
This catch treats permission/I/O faults as “Not Found”, which obscures operational failures. Return 404 only for missing files (ENOENT); return 500 for other read errors.
apps/web/src/pages/(list-types)/ftt-tax-chamber-weekly-hearing-list/index.ts-55-55 (1)
55-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid logging raw errors and validation payload details.
These logs may expose sensitive case data or internal paths. Log sanitised, structured messages instead.
As per coding guidelines: “Never include sensitive data in application logs”.
Also applies to: 68-68, 99-99
Source: Coding guidelines
libs/list-types/ftt-tax-chamber-weekly-hearing-list/src/locales/cy.ts-4-36 (1)
4-36:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftWelsh locale is still English placeholder content.
For Welsh journeys, users on
?lng=cywill see English copy across headings, body content, and table labels. Please replace these placeholders with actual Welsh strings before release.libs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/locales/en.ts-12-13 (1)
12-13:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUser-facing contact email is unresolved placeholder text.
Line 13 still contains
[insert office email], so users cannot follow the published contact instruction. Please replace with the confirmed tribunal mailbox before shipping.libs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/locales/cy.ts-4-13 (1)
4-13:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftWelsh locale remains placeholder content, including unresolved office email token.
This file is still English and includes
[insert office email], so the Welsh path is incomplete and not actionable for users.apps/web/src/pages/(list-types)/ftt-lands-registration-tribunal-weekly-hearing-list/index.ts-26-26 (1)
26-26:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
artefactIdneeds strict validation before file-path usage.Line 49 interpolates unvalidated query input into a filesystem path. A crafted value (for example containing path segments) can escape the intended upload directory. Validate
artefactIdagainst a strict allowlist (e.g., UUID) and reject anything else before path construction.Suggested hardening
+const ARTEFACT_ID_PATTERN = /^[0-9a-fA-F-]{36}$/; + export const GET = async (req: Request, res: Response) => { @@ - const artefactId = req.query.artefactId as string; + const artefactId = req.query.artefactId as string; @@ - if (!artefactId) { + if (!artefactId || !ARTEFACT_ID_PATTERN.test(artefactId)) { return res.status(400).render("errors/common", { en, cy, errorTitle: "Bad Request", - errorMessage: "Missing artefactId parameter" + errorMessage: "Invalid artefactId parameter" }); }Also applies to: 49-49
apps/web/src/pages/(list-types)/ftt-lands-registration-tribunal-weekly-hearing-list/index.ts-55-55 (1)
55-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid logging raw errors and validation details from this request flow.
These logs can expose operational paths and data-bearing validation details. Log a sanitised error code/context only.
As per coding guidelines, "Never include sensitive data in application logs".
Also applies to: 68-68, 99-99
Source: Coding guidelines
libs/list-types/ftt-tax-chamber-weekly-hearing-list/src/conversion/ftt-tax-config.ts-45-48 (1)
45-48:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Member(s)is incorrectly mandatory in the Tax converter.Line 47 sets
required: true, but this list type is defined with an optional Members field. This will fail otherwise valid uploads.Suggested fix
{ header: "Member(s)", fieldName: "members", - required: true, + required: false, validators: [(value, rowNumber) => validateNoHtmlTags(value, "Member(s)", rowNumber)] },libs/list-types/ftt-tax-chamber-weekly-hearing-list/src/pdf/pdf-template.njk-2-2 (1)
2-2:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove hardcoded English from language metadata and empty-state content.
lang="en"andNo hearings scheduled.are fixed English strings, so Welsh/localised PDF output is incomplete.Also applies to: 55-55
libs/list-types/ftt-tax-chamber-weekly-hearing-list/src/pdf/pdf-generator.ts-60-60 (1)
60-60:⚠️ Potential issue | 🟠 MajorRemove non-null assertion and handle optional
sizeBytesdefensively.Line 60 uses a non-null assertion on
pdfResult.sizeBytes!despite the type signature marking it optional. While the implementation always providessizeByteswhen generation succeeds, the code should treat the type as authoritative. Use a fallback to the buffer length if needed:Suggested fix
- return await savePdfToStorage(options.artefactId, pdfResult.pdfBuffer, pdfResult.sizeBytes!); + const sizeBytes = pdfResult.sizeBytes ?? pdfResult.pdfBuffer.length; + return await savePdfToStorage(options.artefactId, pdfResult.pdfBuffer, sizeBytes);libs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/pdf/pdf-generator.ts-30-31 (1)
30-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse artefact/event
lastReceivedDateinstead of generation-timenow().
lastUpdatedDate/Timein the PDF header is currently derived fromnew Date()at generation time, so it can drift from the actual received timestamp used elsewhere and produce inconsistent publication metadata.Suggested fix
interface PdfGenerationOptions extends BasePdfGenerationOptions<FttLrtHearingList> { contentDate: Date; + lastReceivedDate?: string; } @@ - lastReceivedDate: new Date().toISOString(), + lastReceivedDate: options.lastReceivedDate ?? new Date().toISOString(),libs/list-types/ftt-rpt-weekly-hearing-list/src/locales/en.ts-11-13 (1)
11-13:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace unresolved contact placeholder before release.
Line 12 still contains
[insert office email]in user-facing guidance. That leaves no actionable contact route for observers/media and breaks the core instruction flow.apps/web/src/pages/(list-types)/ftt-rpt-eastern-weekly-hearing-list/index.ts-59-70 (1)
59-70:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle JSON parse failures as 400 Invalid Data, not 500.
Line 59 can throw on malformed JSON and currently falls into the Line 94 catch, returning a server error. Parse failures should be mapped to the same 400 invalid-data path as schema failures.
Suggested patch
- const jsonData: FttRptHearingList = JSON.parse(jsonContent); + let jsonData: FttRptHearingList; + try { + jsonData = JSON.parse(jsonContent); + } catch (error) { + console.error("Invalid JSON content:", error); + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Invalid Data", + errorMessage: "The list data is invalid" + }); + }Also applies to: 94-101
libs/list-types/ftt-rpt-weekly-hearing-list/src/pdf/pdf-template.njk-57-57 (1)
57-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLocalise the empty-state message instead of hardcoding English text.
No hearings scheduled.is hardcoded and will stay English in Welsh output. Use a translation key from locale files (for examplet.noHearingsScheduled) to keep bilingual rendering consistent.libs/list-types/ftt-rpt-weekly-hearing-list/src/pdf/pdf-template.njk-2-2 (1)
2-2:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDo not hardcode document language to English.
<html lang="en">is incorrect for Welsh PDFs. Pass the active locale into the template and setlangdynamically.libs/list-types/ftt-rpt-weekly-hearing-list/src/locales/cy.ts-4-39 (1)
4-39:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
cylocale is not Welsh content.Lines 4–39 are English strings, so
?lng=cyusers won’t receive Welsh content. This is a localisation defect for a user-facing route.As per coding guidelines, "Implement Welsh language support on all user-facing text with separate en.ts and cy.ts content files".
Source: Coding guidelines
apps/web/src/pages/(list-types)/ftt-rpt-midlands-weekly-hearing-list/index.ts-21-24 (1)
21-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd runtime validation for
artefactIdinput shape.Line 21 relies on a type assertion. Non-string query values (e.g. repeated params) can still reach downstream calls and cause a 500 path instead of a 400.
apps/web/src/pages/(list-types)/ftt-rpt-midlands-weekly-hearing-list/index.ts-59-64 (1)
59-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle invalid JSON as client error (400).
Line 59 parse failures currently escape to the outer catch and return 500. Malformed list payload should return the same 400 Invalid Data response used for schema failures.
apps/web/src/pages/(list-types)/ftt-rpt-london-weekly-hearing-list/index.ts-59-64 (1)
59-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMalformed JSON currently returns 500 instead of 400.
Line 59 can throw on
JSON.parse, and that is caught by the outer handler (Line 94), returning Server Error. This should return Invalid Data (400), same as schema failures.Suggested fix
- const jsonData: FttRptHearingList = JSON.parse(jsonContent); + let jsonData: FttRptHearingList; + try { + jsonData = JSON.parse(jsonContent) as FttRptHearingList; + } catch { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Invalid Data", + errorMessage: "The list data is invalid" + }); + }apps/web/src/pages/(list-types)/ftt-rpt-london-weekly-hearing-list/index.ts-21-24 (1)
21-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
artefactIdruntime type, not just TS assertion.Line 21 uses
as stringwithout runtime checks. Repeated query params (array value) can flow through and end up as a 500 instead of a 400.Suggested fix
- const artefactId = req.query.artefactId as string; - - if (!artefactId) { + const artefactIdParam = req.query.artefactId; + if (typeof artefactIdParam !== "string" || artefactIdParam.trim().length === 0) { return res.status(400).render("errors/common", { en, cy, errorTitle: "Bad Request", errorMessage: "Missing artefactId parameter" }); } + const artefactId = artefactIdParam;libs/list-types/ftt-rpt-weekly-hearing-list/src/locales/cy.ts-11-13 (1)
11-13:⚠️ Potential issue | 🟠 MajorReplace
[insert office email]with the actual tribunal contact address.Line 12 contains an unresolved placeholder that leaves the instructions unusable for observers and media requesting hearing access. The same issue appears in the English version and other tribunal configuration files.
libs/list-types/ftt-rpt-weekly-hearing-list/src/pdf/pdf-generator.ts-28-33 (1)
28-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse artefact update time for renderer metadata, not generation time.
Line 32 hardcodes
new Date().toISOString(), which can misstate the list’s actual update timestamp in the rendered output. Please pass through the artefact’slastReceivedDatefrom options so PDF and page views stay consistent for the same artefact.Suggested fix
const renderedData = renderFttRptData(options.jsonData, { locale: options.locale, courtName: options.courtName, contentDate: options.contentDate, - lastReceivedDate: new Date().toISOString(), + lastReceivedDate: options.lastReceivedDate.toISOString(), listTitle: options.listTitle });libs/list-types/ftt-rpt-weekly-hearing-list/src/pdf/pdf-generator.ts-55-63 (1)
55-63:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
sizeBytesbefore persisting the PDF.Line 62 uses a non-null assertion for
sizeBytes. If the PDF service returns a buffer without size metadata, this can pass invalid data into storage handling.Suggested fix
- if (!pdfResult.success || !pdfResult.pdfBuffer) { + if (!pdfResult.success || !pdfResult.pdfBuffer || typeof pdfResult.sizeBytes !== "number") { return { success: false, error: pdfResult.error || "PDF generation failed" }; } - return await savePdfToStorage(options.artefactId, pdfResult.pdfBuffer, pdfResult.sizeBytes!); + return await savePdfToStorage(options.artefactId, pdfResult.pdfBuffer, pdfResult.sizeBytes);
🟡 Minor comments (6)
libs/list-types/siac-poac-paac-weekly-hearing-list/src/pdf/pdf-template.njk-53-53 (1)
53-53:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalise the empty-state message instead of hardcoding English.
Line 53 uses
"No hearings scheduled."directly, so Welsh output will still contain English text for empty lists.Suggested change
- <p>No hearings scheduled.</p> + <p>{{ t.noHearingsScheduled }}</p>libs/list-types/siac-poac-paac-weekly-hearing-list/src/config.test.ts-40-42 (1)
40-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the trailing-separator assertion cross-platform.
At Line 41,
/\/$/only matches POSIX separators. This can fail on Windows-style paths.
Use a platform-agnostic check instead.Suggested diff
- it("should end with trailing slash", () => { - expect(assets).toMatch(/\/$/); - }); + it("should end with a trailing path separator", () => { + expect(/[\\/]$/.test(assets)).toBe(true); + });apps/web/src/pages/(list-types)/siac-weekly-hearing-list/index.ts-64-75 (1)
64-75:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTreat malformed JSON as client data error, not server error.
Line 64 can throw on invalid JSON, but that currently maps to the generic 500 path (Lines 98-105). This should return the same 400 “Invalid Data” response as schema failures.
Suggested change
- const jsonData: SiacPoacPaacHearingList = JSON.parse(jsonContent); + let jsonData: SiacPoacPaacHearingList; + try { + jsonData = JSON.parse(jsonContent) as SiacPoacPaacHearingList; + } catch { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Invalid Data", + errorMessage: "The list data is invalid" + }); + }Also applies to: 98-105
apps/web/src/pages/(list-types)/ftt-tax-chamber-weekly-hearing-list/ftt-tax-chamber-weekly-hearing-list.njk-72-72 (1)
72-72:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRender
dataSourceconditionally to avoid empty label output.If
dataSourceis missing, the page will still display a blank value (Data source:). Guard this block with a conditional.apps/web/src/pages/(list-types)/ftt-lands-registration-tribunal-weekly-hearing-list/ftt-lands-registration-tribunal-weekly-hearing-list.njk-68-68 (1)
68-68:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard the data source paragraph when no provenance label is available.
Without a conditional, users can see an empty
data sourcevalue.libs/list-types/ftt-rpt-weekly-hearing-list/src/rendering/renderer.ts-4-22 (1)
4-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMove exported interfaces below exported functions to match repo export-order rules.
RenderOptions/RenderedDataare currently declared beforerenderFttRptData; this breaks the project’s required export ordering for*.tsfiles.As per coding guidelines, "
**/*.ts: Order module exports: top-level constants first, then exported functions, then other functions ordered by usage, with interfaces and types at the bottom."Source: Coding guidelines
🧹 Nitpick comments (21)
libs/list-types/siac-poac-paac-weekly-hearing-list/src/rendering/renderer.ts (1)
4-22: ⚡ Quick winReorder exports to match repository TypeScript ordering convention.
Lines 4–21 export interfaces before the exported function on Line 22. Please move interfaces/types to the bottom of the module.
As per coding guidelines "**/*.ts: Order module exports: top-level constants first, then exported functions, then other functions ordered by usage, with interfaces and types at the bottom".
Source: Coding guidelines
libs/list-types/siac-poac-paac-weekly-hearing-list/src/conversion/siac-poac-paac-config.ts (1)
62-62: ⚡ Quick winRename converter constant to SCREAMING_SNAKE_CASE.
Line 62 uses
siacPoacPaacConverter, which breaks the constant naming rule used in this repo.Suggested change
-const siacPoacPaacConverter = createConverter(SIAC_POAC_PAAC_EXCEL_CONFIG); +const SIAC_POAC_PAAC_CONVERTER = createConverter(SIAC_POAC_PAAC_EXCEL_CONFIG); -registerConverter(28, siacPoacPaacConverter); -registerConverterByName("SIAC_WEEKLY_HEARING_LIST", siacPoacPaacConverter); +registerConverter(28, SIAC_POAC_PAAC_CONVERTER); +registerConverterByName("SIAC_WEEKLY_HEARING_LIST", SIAC_POAC_PAAC_CONVERTER); -registerConverter(29, siacPoacPaacConverter); -registerConverterByName("POAC_WEEKLY_HEARING_LIST", siacPoacPaacConverter); +registerConverter(29, SIAC_POAC_PAAC_CONVERTER); +registerConverterByName("POAC_WEEKLY_HEARING_LIST", SIAC_POAC_PAAC_CONVERTER); -registerConverter(30, siacPoacPaacConverter); -registerConverterByName("PAAC_WEEKLY_HEARING_LIST", siacPoacPaacConverter); +registerConverter(30, SIAC_POAC_PAAC_CONVERTER); +registerConverterByName("PAAC_WEEKLY_HEARING_LIST", SIAC_POAC_PAAC_CONVERTER);As per coding guidelines, "Use SCREAMING_SNAKE_CASE for constant declarations (e.g.,
MAX_FILE_SIZE,DEFAULT_TIMEOUT)."Source: Coding guidelines
apps/web/src/pages/(list-types)/siac-weekly-hearing-list/index.test.ts (1)
187-217: ⚡ Quick winAdd a malformed-JSON test case for the GET handler.
Current tests cover schema-invalid JSON, but not syntactically invalid JSON (e.g. truncated payload). A dedicated case will lock in the expected error response for parse failures.
apps/web/src/pages/(list-types)/poac-weekly-hearing-list/index.test.ts (1)
55-142: ⚡ Quick winAlign POAC negative-path coverage with the SIAC suite.
This suite currently misses JSON-file read failure and schema-validation failure scenarios, which are already covered in the SIAC equivalent tests.
apps/web/src/pages/(list-types)/paac-weekly-hearing-list/index.test.ts (1)
45-109: ⚡ Quick winExpand PAAC error-path tests for parity with sibling controllers.
Please add cases for JSON file read failure and schema-validation failure so PAAC coverage matches SIAC behaviour checks.
libs/list-types/ftt-tax-chamber-weekly-hearing-list/src/config.test.ts (1)
4-4: ⚡ Quick winAdd
schemaPathassertions to cover the runtime validation contract.Current tests only cover
moduleRootandassets.schemaPathis consumed by page validation, so a missing/misplaced schema would fail at runtime without test coverage.Suggested patch
-import { assets, moduleRoot } from "./config.js"; +import { assets, moduleRoot, schemaPath } from "./config.js"; @@ describe("assets", () => { @@ }); + + describe("schemaPath", () => { + it("should be an absolute path", () => { + expect(path.isAbsolute(schemaPath)).toBe(true); + }); + + it("should point to an existing schema file", () => { + expect(existsSync(schemaPath)).toBe(true); + }); + }); });Also applies to: 13-47
libs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/config.test.ts (1)
4-4: ⚡ Quick winCover
schemaPathin config tests to protect validator wiring.This suite does not assert
schemaPath, even though downstream handlers depend on it for JSON validation initialisation.Suggested patch
-import { assets, moduleRoot } from "./config.js"; +import { assets, moduleRoot, schemaPath } from "./config.js"; @@ describe("assets", () => { @@ }); + + describe("schemaPath", () => { + it("should be an absolute path", () => { + expect(path.isAbsolute(schemaPath)).toBe(true); + }); + + it("should point to an existing schema file", () => { + expect(existsSync(schemaPath)).toBe(true); + }); + }); });Also applies to: 13-47
libs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/index.ts (1)
4-11: ⚡ Quick winReorder barrel exports to match the repo export-order convention.
ValidationResult(type export) is currently above value exports. Move type/interface exports to the bottom to keep module surfaces consistent.As per coding guidelines: “Order module exports: top-level constants first, then exported functions, then other functions ordered by usage, with interfaces and types at the bottom”.
Source: Coding guidelines
libs/list-types/ftt-tax-chamber-weekly-hearing-list/src/rendering/renderer.ts (1)
4-22: ⚡ Quick winMove interfaces/types to the bottom to match repository export ordering.
RenderOptionsandRenderedDataare declared before the exported function; the repo rule requires constants first, then exported functions, with interfaces/types at the bottom.As per coding guidelines,
**/*.ts: "Order module exports: top-level constants first, then exported functions, then other functions ordered by usage, with interfaces and types at the bottom".Source: Coding guidelines
libs/list-types/ftt-tax-chamber-weekly-hearing-list/src/pdf/pdf-generator.ts (1)
20-24: ⚡ Quick winReorder declarations to keep interfaces/types at the bottom.
PdfGenerationOptionsis declared before the exported function; this breaks the repository export ordering rule.As per coding guidelines,
**/*.ts: "Order module exports: top-level constants first, then exported functions, then other functions ordered by usage, with interfaces and types at the bottom".Source: Coding guidelines
apps/web/src/pages/(list-types)/ftt-tax-chamber-weekly-hearing-list/index.test.ts (1)
76-76: ⚡ Quick winAvoid
as anyin test fixtures; use typed mock artefacts.The
as anycasts weaken type-safety and can mask contract drift in controller tests.As per coding guidelines,
**/*.{ts,tsx}: "Enable TypeScript strict mode and avoidanytype without justification".Also applies to: 111-111
Source: Coding guidelines
apps/web/src/pages/(list-types)/ftt-lands-registration-tribunal-weekly-hearing-list/index.test.ts (1)
75-75: ⚡ Quick winReplace
as anycasts with typed test doubles.Using
as anyhere reduces confidence that mocked artefacts still match the real controller contract.As per coding guidelines,
**/*.{ts,tsx}: "Enable TypeScript strict mode and avoidanytype without justification".Also applies to: 110-110
Source: Coding guidelines
libs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/rendering/renderer.ts (1)
4-22: ⚡ Quick winReorder exports to match the module ordering convention.
This file currently exports interfaces before the exported function; your project rule requires exported functions before interfaces/types.
Suggested reordering
-export interface RenderOptions { - locale: string; - courtName: string; - contentDate: Date; - lastReceivedDate: string; - listTitle: string; -} - -export interface RenderedData { - header: { - listTitle: string; - weekCommencingDate: string; - lastUpdatedDate: string; - lastUpdatedTime: string; - }; - hearings: FttLrtHearing[]; -} - export function renderFttLrtData(hearingList: FttLrtHearingList, options: RenderOptions): RenderedData { const weekCommencingDate = formatDisplayDate(options.contentDate, options.locale); const { date: lastUpdatedDate, time: lastUpdatedTime } = formatLastUpdatedDateTime(options.lastReceivedDate, options.locale); @@ }; } + +export interface RenderOptions { + locale: string; + courtName: string; + contentDate: Date; + lastReceivedDate: string; + listTitle: string; +} + +export interface RenderedData { + header: { + listTitle: string; + weekCommencingDate: string; + lastUpdatedDate: string; + lastUpdatedTime: string; + }; + hearings: FttLrtHearing[]; +}As per coding guidelines:
**/*.ts: “Order module exports: top-level constants first, then exported functions, then other functions ordered by usage, with interfaces and types at the bottom”.Source: Coding guidelines
apps/web/src/pages/(list-types)/ftt-rpt-london-weekly-hearing-list/index.ts (1)
17-103: 🏗️ Heavy liftExtract a shared regional handler factory to prevent logic drift.
This controller is structurally duplicated across regional pages; defects like the JSON/type handling are likely to recur. A shared
createFttRptRegionalHandler({ template, pageTitleKey, courtName })would reduce repeat bugs.libs/list-types/ftt-rpt-weekly-hearing-list/src/pdf/pdf-generator.test.ts (1)
83-160: ⚡ Quick winAdd a regression test for successful PDF generation with missing
sizeBytes.The suite currently does not cover the case where
generatePdfFromHtmlreturns{ success: true, pdfBuffer }withoutsizeBytes. Adding this test will lock in expected error handling for a real edge case.apps/web/src/pages/(list-types)/ftt-rpt-eastern-weekly-hearing-list/index.test.ts (2)
78-78: ⚡ Quick winReplace
anyartefact casts with a typed fixture.Line 78 and Line 116 use
as any, which weakens strict test guarantees and can hide shape mismatches against controller expectations.
As per coding guidelines, "Enable TypeScript strict mode and avoidanytype without justification".Also applies to: 111-116
Source: Coding guidelines
45-128: ⚡ Quick winAdd coverage for invalid JSON validation path.
The controller has a dedicated 400 branch for schema validation failures, but this suite does not assert it. Please add a case with
mockValidatereturning{ isValid: false }and verify the 400 response path.apps/web/src/pages/(list-types)/ftt-rpt-london-weekly-hearing-list/index.test.ts (1)
78-78: ⚡ Quick winReplace
anyartefact casts with a typed fixture.Line 78 and Line 116 use
as any, which reduces confidence in controller contract checks and can mask future type regressions.
As per coding guidelines, "Enable TypeScript strict mode and avoidanytype without justification".Also applies to: 111-116
Source: Coding guidelines
apps/web/src/pages/(list-types)/ftt-rpt-midlands-weekly-hearing-list/index.test.ts (1)
78-78: ⚡ Quick winReplace
anyartefact casts with a typed fixture.Line 78 and Line 116 use
as any, which weakens strict typing in these controller tests.
As per coding guidelines, "Enable TypeScript strict mode and avoidanytype without justification".Also applies to: 111-116
Source: Coding guidelines
apps/web/src/pages/(list-types)/ftt-rpt-northern-weekly-hearing-list/index.test.ts (1)
78-78: ⚡ Quick winReplace
anyartefact casts with a typed fixture.Line 78 and Line 116 use
as any, which weakens type safety and can hide contract drift in test setup.
As per coding guidelines, "Enable TypeScript strict mode and avoidanytype without justification".Also applies to: 111-116
Source: Coding guidelines
apps/web/src/pages/(list-types)/ftt-rpt-southern-weekly-hearing-list/index.test.ts (1)
78-78: ⚡ Quick winReplace
anyartefact casts with a typed fixture.Line 78 and Line 116 use
as any, which undermines strict typing in this test suite.
As per coding guidelines, "Enable TypeScript strict mode and avoidanytype without justification".Also applies to: 111-116
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 069a8d55-5181-4079-a4a1-9240284d4092
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (112)
apps/postgres/prisma/migrations/20260605141023_add_jurisdiction_soft_delete_and_audit_log/migration.sqlapps/web/src/app.tsapps/web/src/pages/(list-types)/ftt-lands-registration-tribunal-weekly-hearing-list/ftt-lands-registration-tribunal-weekly-hearing-list.njkapps/web/src/pages/(list-types)/ftt-lands-registration-tribunal-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/ftt-lands-registration-tribunal-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/ftt-rpt-eastern-weekly-hearing-list/ftt-rpt-eastern-weekly-hearing-list.njkapps/web/src/pages/(list-types)/ftt-rpt-eastern-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/ftt-rpt-eastern-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/ftt-rpt-london-weekly-hearing-list/ftt-rpt-london-weekly-hearing-list.njkapps/web/src/pages/(list-types)/ftt-rpt-london-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/ftt-rpt-london-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/ftt-rpt-midlands-weekly-hearing-list/ftt-rpt-midlands-weekly-hearing-list.njkapps/web/src/pages/(list-types)/ftt-rpt-midlands-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/ftt-rpt-midlands-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/ftt-rpt-northern-weekly-hearing-list/ftt-rpt-northern-weekly-hearing-list.njkapps/web/src/pages/(list-types)/ftt-rpt-northern-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/ftt-rpt-northern-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/ftt-rpt-southern-weekly-hearing-list/ftt-rpt-southern-weekly-hearing-list.njkapps/web/src/pages/(list-types)/ftt-rpt-southern-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/ftt-rpt-southern-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/ftt-tax-chamber-weekly-hearing-list/ftt-tax-chamber-weekly-hearing-list.njkapps/web/src/pages/(list-types)/ftt-tax-chamber-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/ftt-tax-chamber-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/paac-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/paac-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/paac-weekly-hearing-list/paac-weekly-hearing-list.njkapps/web/src/pages/(list-types)/poac-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/poac-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/poac-weekly-hearing-list/poac-weekly-hearing-list.njkapps/web/src/pages/(list-types)/siac-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/siac-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/siac-weekly-hearing-list/siac-weekly-hearing-list.njkapps/web/src/pages/(public)/summary-of-publications/index.tsbiome.jsondocs/tickets/428/plan.mddocs/tickets/428/review.mddocs/tickets/428/tasks.mddocs/tickets/428/ticket.mdlibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/package.jsonlibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/config.test.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/config.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/conversion/ftt-lrt-config.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/email-summary/summary-builder.test.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/email-summary/summary-builder.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/index.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/locales/cy.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/locales/en.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/models/types.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/pdf/pdf-generator.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/pdf/pdf-template.njklibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/rendering/renderer.test.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/rendering/renderer.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/schemas/ftt-lands-registration-tribunal-weekly-hearing-list.jsonlibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/tsconfig.jsonlibs/list-types/ftt-rpt-weekly-hearing-list/package.jsonlibs/list-types/ftt-rpt-weekly-hearing-list/src/config.test.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/config.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/conversion/ftt-rpt-config.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/email-summary/summary-builder.test.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/email-summary/summary-builder.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/index.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/locales/cy.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/locales/en.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/models/types.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/pdf/pdf-generator.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/pdf/pdf-template.njklibs/list-types/ftt-rpt-weekly-hearing-list/src/rendering/renderer.test.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/rendering/renderer.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/schemas/ftt-rpt-weekly-hearing-list.jsonlibs/list-types/ftt-rpt-weekly-hearing-list/tsconfig.jsonlibs/list-types/ftt-tax-chamber-weekly-hearing-list/package.jsonlibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/config.test.tslibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/config.tslibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/conversion/ftt-tax-config.tslibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/email-summary/summary-builder.test.tslibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/email-summary/summary-builder.tslibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/index.tslibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/locales/cy.tslibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/locales/en.tslibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/models/types.tslibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/pdf/pdf-generator.tslibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/pdf/pdf-template.njklibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/rendering/renderer.test.tslibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/rendering/renderer.tslibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/schemas/ftt-tax-chamber-weekly-hearing-list.jsonlibs/list-types/ftt-tax-chamber-weekly-hearing-list/tsconfig.jsonlibs/list-types/siac-poac-paac-weekly-hearing-list/package.jsonlibs/list-types/siac-poac-paac-weekly-hearing-list/src/config.test.tslibs/list-types/siac-poac-paac-weekly-hearing-list/src/config.tslibs/list-types/siac-poac-paac-weekly-hearing-list/src/conversion/siac-poac-paac-config.tslibs/list-types/siac-poac-paac-weekly-hearing-list/src/email-summary/summary-builder.test.tslibs/list-types/siac-poac-paac-weekly-hearing-list/src/email-summary/summary-builder.tslibs/list-types/siac-poac-paac-weekly-hearing-list/src/index.tslibs/list-types/siac-poac-paac-weekly-hearing-list/src/locales/cy.tslibs/list-types/siac-poac-paac-weekly-hearing-list/src/locales/en.tslibs/list-types/siac-poac-paac-weekly-hearing-list/src/models/types.tslibs/list-types/siac-poac-paac-weekly-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/siac-poac-paac-weekly-hearing-list/src/pdf/pdf-generator.tslibs/list-types/siac-poac-paac-weekly-hearing-list/src/pdf/pdf-template.njklibs/list-types/siac-poac-paac-weekly-hearing-list/src/rendering/renderer.test.tslibs/list-types/siac-poac-paac-weekly-hearing-list/src/rendering/renderer.tslibs/list-types/siac-poac-paac-weekly-hearing-list/src/schemas/siac-poac-paac-weekly-hearing-list.jsonlibs/list-types/siac-poac-paac-weekly-hearing-list/tsconfig.jsonlibs/location/src/list-type-data.tslibs/notifications/package.jsonlibs/notifications/src/notification/notification-service.tslibs/publication/package.jsonlibs/publication/src/processing/service.tstsconfig.json
| id: 28, | ||
| name: "SIAC_WEEKLY_HEARING_LIST", | ||
| englishFriendlyName: "Special Immigration Appeals Commission Weekly Hearing List", | ||
| welshFriendlyName: "Special Immigration Appeals Commission Weekly Hearing List", | ||
| provenance: "MANUAL_UPLOAD", | ||
| urlPath: "siac-weekly-hearing-list", | ||
| isNonStrategic: true, | ||
| defaultSensitivity: "Public", | ||
| shortenedFriendlyName: "SIAC Weekly Hearing List", | ||
| subJurisdictionIds: [25] | ||
| }, | ||
| { | ||
| id: 29, | ||
| name: "POAC_WEEKLY_HEARING_LIST", | ||
| englishFriendlyName: "Proscribed Organisations Appeal Commission Weekly Hearing List", | ||
| welshFriendlyName: "Proscribed Organisations Appeal Commission Weekly Hearing List", | ||
| provenance: "MANUAL_UPLOAD", | ||
| urlPath: "poac-weekly-hearing-list", | ||
| isNonStrategic: true, | ||
| defaultSensitivity: "Public", | ||
| shortenedFriendlyName: "POAC Weekly Hearing List", | ||
| subJurisdictionIds: [23] | ||
| }, | ||
| { | ||
| id: 30, | ||
| name: "PAAC_WEEKLY_HEARING_LIST", | ||
| englishFriendlyName: "Pathogens Access Appeal Commission Weekly Hearing List", | ||
| welshFriendlyName: "Pathogens Access Appeal Commission Weekly Hearing List", | ||
| provenance: "MANUAL_UPLOAD", | ||
| urlPath: "paac-weekly-hearing-list", | ||
| isNonStrategic: true, | ||
| defaultSensitivity: "Public", | ||
| shortenedFriendlyName: "PACC Weekly Hearing List", | ||
| subJurisdictionIds: [21] | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file and check its structure
find . -name "list-type-data.ts" -type fRepository: hmcts/cath-service
Length of output: 100
🏁 Script executed:
# Also look for plan.md and any ticket files
find . -name "plan.md" -type f
find . -name "*.md" -type f | head -20Repository: hmcts/cath-service
Length of output: 1702
🏁 Script executed:
# Search for references to SIAC, POAC, PAAC in the codebase to understand context
rg -i "SIAC|POAC|PAAC" --type ts --type md -A 2 -B 2Repository: hmcts/cath-service
Length of output: 50374
🏁 Script executed:
# Read the plan.md file to find the specific lines 111-113 mentioned in the review comment
head -120 ./docs/tickets/428/plan.md | tail -20Repository: hmcts/cath-service
Length of output: 3449
🏁 Script executed:
# Also check the ticket.md file to find line 23 and the requirement mentioned
head -30 ./docs/tickets/428/ticket.md | tail -15Repository: hmcts/cath-service
Length of output: 1681
🏁 Script executed:
# Check if there's any documented approval or resolution of the sensitivity level question
rg "defaultSensitivity|sensitivity.*level|Public.*SIAC|SIAC.*Public" ./docs/tickets/428/ -A 2 -B 2Repository: hmcts/cath-service
Length of output: 6838
🏁 Script executed:
# Check the actual current state of list-type-data.ts to see the exact sensitivity values
sed -n '320,360p' ./libs/location/src/list-type-data.tsRepository: hmcts/cath-service
Length of output: 1636
🏁 Script executed:
# Check if there's any resolution or product owner decision documented in any follow-up files
rg "SIAC.*sensitivity|sensitivity.*SIAC|product owner.*SIAC|SIAC.*approval" ./docs --type mdRepository: hmcts/cath-service
Length of output: 1497
🏁 Script executed:
# Check the review.md file to see what the final recommendation was
grep -A 5 "Solution:" ./docs/tickets/428/review.md | grep -A 5 "defaultSensitivity"Repository: hmcts/cath-service
Length of output: 388
🏁 Script executed:
# Check the git history to see if this was addressed in follow-up commits
git log --oneline --all -- "libs/location/src/list-type-data.ts" | head -20Repository: hmcts/cath-service
Length of output: 750
🏁 Script executed:
# Check if there are any comments in the review.md or ticket.md about resolution after the review was written
tail -50 ./docs/tickets/428/review.mdRepository: hmcts/cath-service
Length of output: 5079
Obtain explicit written product owner approval before these entries are deployed to production.
Lines 328, 340, 352 set defaultSensitivity: "Public" for SIAC, POAC, and PAAC weekly hearing lists. However, the tribunal locales explicitly state: "The tribunal sometimes uses reference numbers or initials to protect the anonymity of those involved in the appeal", suggesting these lists contain protected personal information.
The ticket acceptance criterion requires that "List types are classified and user groups are decided based on authorised access", but no product owner confirmation is documented. The codebase review identified this as a critical issue requiring explicit written approval before go-live. If Public is the intended classification, document the decision in the ticket; otherwise, adjust the sensitivity level in consultation with the product owner.
…ngle shared template The 5 regional FTT RPT weekly hearing list templates were byte-for-byte identical. Move the single template into the lib's views directory and point all regional controllers at it, removing 410 lines of duplication. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace hardcoded courtName strings in the 5 regional FTT RPT controllers with locale file keys, consistent with how listTitle is already handled. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The build:nunjucks script used cd src/pdf before copying views, causing the src/views glob to resolve from the wrong directory and fail with "No matches found" on CI. Split into a separate build:views step using find instead of a glob. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
| @@ -0,0 +1 @@ | |||
| -- No-op: changes in this migration are applied by other in-progress tickets | |||
There was a problem hiding this comment.
Not sure why this change is needed. If not required we should remove it
| @@ -0,0 +1,103 @@ | |||
| import { readFile } from "node:fs/promises"; | |||
There was a problem hiding this comment.
There are 5 separate pages for the 5 RPT list types. For RCJ standard types, those list types are grouped together under the page rcj-standard-daily-cause-list. For SJP main list and SJP delta list, they are in separate pages.
I think we need to discuss and agree on a consistent approaches for this type of list types.
| @@ -0,0 +1,107 @@ | |||
| import { readFile } from "node:fs/promises"; | |||
There was a problem hiding this comment.
Same for SIAC, POAC and PAAC, they are in separate pages here but the PDF and summary stuffs in /libs/list-types/siac-poac-paac-weekly-hearing-list are shared.
…publishing Resolves conflicts from remote refactoring commits that: - Moved NJK template into libs/ftt-rpt-weekly-hearing-list/src/views/ - Moved court names into locale files - Fixed build:views script in package.json Local consolidation of regional pages into single handler is preserved. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…to a single shared handler Three identical NJK templates for siac, poac, and paac weekly hearing lists are replaced by a single siac-poac-paac-weekly-hearing-list template in the lib. The three individual page controllers are replaced by one handler that routes by listTypeId (28=SIAC, 29=POAC, 30=PAAC), following the same pattern used for FTT RPT. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All three list types now resolve to siac-poac-paac-weekly-hearing-list instead of the deleted individual routes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/pages/(list-types)/ftt-lands-registration-tribunal-weekly-hearing-list/index.test.ts (1)
63-145: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd the invalid-data branch to this suite.
The changed controller now relies on the shared validator path, but none of these tests drive
mockValidateto{ isValid: false }. A regression in that branch would still slip through here. Please add one case that asserts the 400 error render when validation fails.
🧹 Nitpick comments (2)
apps/web/src/pages/(list-types)/ftt-lands-registration-tribunal-weekly-hearing-list/index.test.ts (1)
77-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the
as anycasts in these fixtures.These casts sidestep the strict-mode safety the test should provide, so contract drift in the artefact or rendered payload can still compile unnoticed. Prefer typed fixtures or
satisfieshere. As per coding guidelines, "Enable TypeScript strict mode and avoidanytype without justification".Also applies to: 130-133
Source: Coding guidelines
apps/web/src/pages/(list-types)/ftt-rpt-weekly-hearing-list/index.test.ts (1)
111-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a typed
Artefactfixture instead ofas anyin these mocks. The casts in this file suppress interface drift across all fivegetArtefactByIdstubs, so a small shared helper orPartial<Artefact>would keep the test data strict without much extra noise.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 14efb396-3f0f-4c6a-8357-059a54992c08
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
apps/web/src/pages/(list-types)/ftt-lands-registration-tribunal-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/ftt-lands-registration-tribunal-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/ftt-rpt-weekly-hearing-list/ftt-rpt-weekly-hearing-list.njkapps/web/src/pages/(list-types)/ftt-rpt-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/ftt-rpt-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/ftt-tax-chamber-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/ftt-tax-chamber-weekly-hearing-list/index.tsapps/web/src/pages/(list-types)/siac-poac-paac-weekly-hearing-list/index.test.tsapps/web/src/pages/(list-types)/siac-poac-paac-weekly-hearing-list/index.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/conversion/ftt-lrt-config.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/locales/cy.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/locales/en.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/pdf/pdf-generator.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/rendering/renderer.test.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/schemas/ftt-lands-registration-tribunal-weekly-hearing-list.jsonlibs/list-types/ftt-rpt-weekly-hearing-list/package.jsonlibs/list-types/ftt-rpt-weekly-hearing-list/src/locales/cy.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/locales/en.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/views/ftt-rpt-weekly-hearing-list.njklibs/list-types/siac-poac-paac-weekly-hearing-list/package.jsonlibs/list-types/siac-poac-paac-weekly-hearing-list/src/views/siac-poac-paac-weekly-hearing-list.njklibs/location/src/list-type-data.tslibs/location/src/location-data.tslibs/notifications/package.jsonlibs/notifications/src/notification/notification-service.tslibs/publication/package.jsontemplates/tech-spec-references/welsh-translations-catalogue.jsontsconfig.json
💤 Files with no reviewable changes (3)
- libs/list-types/siac-poac-paac-weekly-hearing-list/src/views/siac-poac-paac-weekly-hearing-list.njk
- libs/list-types/ftt-rpt-weekly-hearing-list/src/views/ftt-rpt-weekly-hearing-list.njk
- apps/web/src/pages/(list-types)/ftt-rpt-weekly-hearing-list/ftt-rpt-weekly-hearing-list.njk
✅ Files skipped from review due to trivial changes (3)
- libs/location/src/location-data.ts
- libs/list-types/ftt-rpt-weekly-hearing-list/src/locales/en.ts
- tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (14)
- libs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/schemas/ftt-lands-registration-tribunal-weekly-hearing-list.json
- libs/list-types/ftt-rpt-weekly-hearing-list/src/locales/cy.ts
- libs/list-types/siac-poac-paac-weekly-hearing-list/package.json
- libs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/rendering/renderer.test.ts
- libs/list-types/ftt-rpt-weekly-hearing-list/package.json
- libs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/conversion/ftt-lrt-config.ts
- libs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/locales/en.ts
- libs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/locales/cy.ts
- libs/publication/package.json
- libs/location/src/list-type-data.ts
- libs/notifications/package.json
- libs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/pdf/pdf-generator.ts
- libs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/pdf/pdf-generator.test.ts
- libs/notifications/src/notification/notification-service.ts
| vi.mock("@hmcts/ftt-lands-registration-tribunal-weekly-hearing-list", () => ({ | ||
| fttLrtWeeklyHearingListEn: { | ||
| pageTitle: "First-tier Tribunal (Land Registration Tribunal) Weekly Hearing List", | ||
| provenanceLabels: { MANUAL_UPLOAD: "Manual Upload", SNL: "ListAssist" } | ||
| }, | ||
| fttLrtWeeklyHearingListCy: { | ||
| pageTitle: "First-tier Tribunal (Land Registration Tribunal) Weekly Hearing List", | ||
| provenanceLabels: { MANUAL_UPLOAD: "Lanlwytho â Llaw", SNL: "ListAssist" } | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a distinct Welsh title in the locale mock.
pageTitle is identical in both locale fixtures, so the Welsh test only proves the provenance label changed. If the controller stopped forwarding the Welsh title into renderFttLrtData, this test would still pass. Give the Welsh mock a different title and assert that listTitle matches it. As per coding guidelines, "Implement Welsh language support on all user-facing text with separate en.ts and cy.ts content files".
Also applies to: 121-143
Source: Coding guidelines
| fttRptWeeklyHearingListCy: { | ||
| provenanceLabels: { MANUAL_UPLOAD: "Lanlwytho â Llaw", SNL: "ListAssist" }, | ||
| rptEasternPageTitle: "First-tier Tribunal (Residential Property Tribunal): Eastern region Weekly Hearing List", | ||
| rptLondonPageTitle: "First-tier Tribunal (Residential Property Tribunal): London region Weekly Hearing List", | ||
| rptMidlandsPageTitle: "First-tier Tribunal (Residential Property Tribunal): Midlands region Weekly Hearing List", | ||
| rptNorthernPageTitle: "First-tier Tribunal (Residential Property Tribunal): Northern region Weekly Hearing List", | ||
| rptSouthernPageTitle: "First-tier Tribunal (Residential Property Tribunal): Southern region Weekly Hearing List" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the Welsh title path observable.
The mocked cy titles on Lines 27-33 are the same as en, so the Welsh test on Lines 160-185 would still pass if the controller accidentally used enTitle. Give the Welsh mock distinct values and assert the listTitle passed to renderFttRptData.
Suggested change
fttRptWeeklyHearingListCy: {
provenanceLabels: { MANUAL_UPLOAD: "Lanlwytho â Llaw", SNL: "ListAssist" },
- rptEasternPageTitle: "First-tier Tribunal (Residential Property Tribunal): Eastern region Weekly Hearing List",
- rptLondonPageTitle: "First-tier Tribunal (Residential Property Tribunal): London region Weekly Hearing List",
- rptMidlandsPageTitle: "First-tier Tribunal (Residential Property Tribunal): Midlands region Weekly Hearing List",
- rptNorthernPageTitle: "First-tier Tribunal (Residential Property Tribunal): Northern region Weekly Hearing List",
- rptSouthernPageTitle: "First-tier Tribunal (Residential Property Tribunal): Southern region Weekly Hearing List"
+ rptEasternPageTitle: "CY Eastern Weekly Hearing List",
+ rptLondonPageTitle: "CY London Weekly Hearing List",
+ rptMidlandsPageTitle: "CY Midlands Weekly Hearing List",
+ rptNorthernPageTitle: "CY Northern Weekly Hearing List",
+ rptSouthernPageTitle: "CY Southern Weekly Hearing List"
},- expect(renderFttRptData).toHaveBeenCalledWith(MOCK_JSON_DATA, expect.objectContaining({ locale: "cy" }));
+ expect(renderFttRptData).toHaveBeenCalledWith(
+ MOCK_JSON_DATA,
+ expect.objectContaining({ locale: "cy", listTitle: "CY Eastern Weekly Hearing List" })
+ );As per coding guidelines, "Implement Welsh language support on all user-facing text with separate en.ts and cy.ts content files" and "Test pages with ?lng=cy query parameter to verify Welsh content during development".
Also applies to: 160-185
Source: Coding guidelines
| import type { Request, Response } from "express"; | ||
| import { beforeEach, describe, expect, it, vi } from "vitest"; | ||
|
|
||
| const mockValidate = vi.hoisted(() => vi.fn()); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename the test constants to match the repo rule.
mockValidate and both mockRenderedData fixtures are const declarations but are not SCREAMING_SNAKE_CASE. As per coding guidelines, "Use SCREAMING_SNAKE_CASE for constant declarations (e.g., MAX_FILE_SIZE, DEFAULT_TIMEOUT)".
Also applies to: 67-76, 134-143
Source: Coding guidelines
| import { createJsonValidator } from "@hmcts/list-types-common"; | ||
| import { createSimpleListTypeHandler, resolveDataSource } from "../list-type-handler.js"; | ||
|
|
||
| const validate = createJsonValidator(schemaPath); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename these constants to match the repo rule.
validate, t, and dataSource are const declarations but are not SCREAMING_SNAKE_CASE. As per coding guidelines, "Use SCREAMING_SNAKE_CASE for constant declarations (e.g., MAX_FILE_SIZE, DEFAULT_TIMEOUT)".
Also applies to: 20-20, 30-30
Source: Coding guidelines
| cy, | ||
| validate, | ||
| logPrefix: "ftt-tax-chamber-weekly-hearing-list", | ||
| serverError: { errorTitle: "Server Error", errorMessage: "An error occurred while loading the list" }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localise the custom 500 copy.
These strings are hard-coded in English, so the Welsh route will still show English text on the server-error path. Either remove the override or make the shared handler accept locale-specific error content from en/cy. As per coding guidelines, "Provide both en and cy content objects in page controllers and render both language versions in templates" and "Implement Welsh language support on all user-facing text with separate en.ts and cy.ts content files".
Source: Coding guidelines
| res.status(400).render("errors/common", { | ||
| en, | ||
| cy, | ||
| errorTitle: "Invalid List Type", | ||
| errorMessage: "This list type is not supported by this module" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localise the custom error copy.
These branches hard-code English strings, so ?lng=cy will still show English text for the invalid-list-type and custom 500 paths. Move this copy into the locale content, or make the error payload locale-aware before rendering errors/common. As per coding guidelines, apps/web/src/pages/**/index.ts must “Provide both en and cy content objects in page controllers” and “Implement Welsh language support on all user-facing text with separate en.ts and cy.ts content files”.
Also applies to: 51-51
Source: Coding guidelines
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
…iacWeeklyHearingListPdf to eliminate duplication Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
libs/list-types/common/src/index.ts (1)
36-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the new type re-exports at the end of this barrel block.
generateFttSiacWeeklyHearingListPdfis a value export, soFttSiacWeeklyHearingListPdfOptionsandRenderedPdfDatashould sit after the value exports here to keep the public API ordering consistent. As per coding guidelines, "Order module exports: top-level constants first, then exported functions, then other functions ordered by usage, with interfaces and types at the bottom".Source: Coding guidelines
libs/list-types/siac-poac-paac-weekly-hearing-list/src/pdf/pdf-generator.test.ts (1)
3-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the shared test fixtures to SCREAMING_SNAKE_CASE.
mockUploadBlob,mockRenderedData, andmockHearingListare module-level constants, so they should follow the repo naming rule to keep these tests aligned with linting. As per coding guidelines, "Use SCREAMING_SNAKE_CASE for constant declarations (e.g., MAX_FILE_SIZE, DEFAULT_TIMEOUT)".Source: Coding guidelines
libs/list-types/ftt-rpt-weekly-hearing-list/src/pdf/pdf-generator.test.ts (1)
3-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the shared test fixtures to SCREAMING_SNAKE_CASE.
mockUploadBlob,mockRenderedData, andmockHearingListare module-level constants, so they should follow the repo naming rule to keep these tests aligned with linting. As per coding guidelines, "Use SCREAMING_SNAKE_CASE for constant declarations (e.g., MAX_FILE_SIZE, DEFAULT_TIMEOUT)".Source: Coding guidelines
libs/list-types/common/src/pdf/pdf-utilities.ts (2)
65-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the exported interfaces to the bottom of the module.
RenderedPdfDataandFttSiacWeeklyHearingListPdfOptionsnow sit above an exported function, which breaks the repo’s export ordering rule. As per coding guidelines, "Order module exports: top-level constants first, then exported functions, then other functions ordered by usage, with interfaces and types at the bottom".Source: Coding guidelines
78-79: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRequire
sizeBytesin the success shape.
savePdfToStorageneedssizeBytes, so a successfulgeneratePdfresult should not allow it to be omitted. Make the success branch carry bothpdfBufferandsizeBytes, or split success/failure into a discriminated union.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5de4a6a1-7559-41bf-ac97-e7d8c146fe90
📒 Files selected for processing (11)
libs/list-types/common/src/index.tslibs/list-types/common/src/pdf/pdf-utilities.test.tslibs/list-types/common/src/pdf/pdf-utilities.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/ftt-lands-registration-tribunal-weekly-hearing-list/src/pdf/pdf-generator.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/ftt-rpt-weekly-hearing-list/src/pdf/pdf-generator.tslibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/ftt-tax-chamber-weekly-hearing-list/src/pdf/pdf-generator.tslibs/list-types/siac-poac-paac-weekly-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/siac-poac-paac-weekly-hearing-list/src/pdf/pdf-generator.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/list-types/ftt-tax-chamber-weekly-hearing-list/src/pdf/pdf-generator.test.ts
| return generateFttSiacWeeklyHearingListPdf({ | ||
| ...options, | ||
| courtName: "First-tier Tribunal (Land Registration Tribunal)", | ||
| listTitle: "First-tier Tribunal (Land Registration Tribunal) Weekly Hearing List", | ||
| moduleDir: __dirname, | ||
| provenanceLabel: options.provenance ? PROVENANCE_LABELS[options.provenance as keyof typeof PROVENANCE_LABELS] || options.provenance : "", | ||
| importEn: () => import("../locales/en.js"), | ||
| importCy: () => import("../locales/cy.js"), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not hard-code the Welsh-facing title in the wrapper.
Line 20 always passes the English list title, so a locale: "cy" request can never render a Welsh header from this path. Please source listTitle (and ideally courtName) from locale data or caller input instead of fixing them here.
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Merges SIAC/POAC/PAAC/FTT list types (HEAD) with SEND/CIC/AST list types (master). Both PDF generation APIs kept: generateFttSiacWeeklyHearingListPdf and generateListPdf/buildPdfFromRenderedList. List type IDs renumbered so SEND=38, CIC=39, AST=40 to avoid collision with SIAC(28)-FTT_RPT_SOUTHERN(37). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ormat date-formatting.ts was updated to use day: "2-digit" (producing "02 January 2025" instead of "2 January 2025"). Align renderer tests for siac-poac-paac, ftt-rpt, ftt-tax-chamber, and ftt-lands-registration-tribunal to match the new format. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
…directories Restored migration.sql files from their original commits for migrations that had empty directories in the working tree, and removed orphan directories that were never committed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gration directories" This reverts commit f03d8d6.
|
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
10 STATUS + IMPL changes (closed issue + merged closing PR → verified): REQ-0078 (#301): implemented → verified (PR #458) REQ-0105 (#428): in_progress → verified (PR #749) REQ-0106 (#429): approved → verified (PR #761) REQ-0107 (#431): implemented → verified (PR #701) REQ-0108 (#434): approved → verified (PR #772) REQ-0109 (#436): implemented → verified (PR #727) REQ-0112 (#467): implemented → verified (PR #670) REQ-0124 (#563): approved → verified (PR #782) REQ-0135 (#569): in_progress → verified (PR #748) REQ-0137 (#729): approved → verified (PR #766) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 participants
Summary
siac-poac-paac-weekly-hearing-list,ftt-tax-chamber-weekly-hearing-list,ftt-lands-registration-tribunal-weekly-hearing-list,ftt-rpt-weekly-hearing-listapps/web/src/pages/(list-types)/following the existing non-strategic publishing patternImplementation Notes
taxappeals@justice.gov.ukcontact[insert office email]placeholder pending confirmation from the tribunal — seeTODOcomments in the relevanten.tsfilesTest plan
yarn test)yarn lint)members,additionalInformation) render blank when absent, populated when present?lng=cy) loads without errors on all new pages🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes