Feature/431 sscs tribunal non strategic publishing - #701
Conversation
- Add 8 new SSCS Tribunal locations across 4 new regions (South West, Scotland, North East, North West) - Add 8 new SSCS list types (SSCS_MIDLANDS through SSCS_LIVERPOOL) as non-strategic with MANUAL_UPLOAD provenance - Implement new sscs-daily-hearing-list library module with page controller, Nunjucks template, PDF generator, email summary builder, Excel converter, JSON schema, and English/Welsh translations - Register module in apps/web and root tsconfig 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
It was showing error on screen and not displaying anything
|
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:
📝 WalkthroughWalkthroughAdds a new ChangesSSCS Daily Hearing List Implementation
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (5)
libs/system-admin-pages/src/user-management/validation.ts (1)
1-1: ⚡ Quick winConsider deleting unused constant.
The underscore prefix suppresses the linter warning but retains dead code. Per YAGNI principle, this constant should be removed entirely.
(Note:
docs/tickets/431/review.mdalready documents this as a suggestion.)🧹 Proposed fix
-const _ALPHANUMERIC_REGEX = /^[a-zA-Z0-9]+$/; const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;libs/publication/src/processing/service.test.ts (1)
287-308: 💤 Low valueConsider verifying
importantInformationTextin the Welsh test.The Welsh locale test verifies
listTitleandcourtNamebut notimportantInformationText. Whilst the English test already covers this field, adding the assertion here would make the test more complete and self-contained.📝 Suggested addition
expect(generateSscsDailyHearingListPdf).toHaveBeenCalledWith( expect.objectContaining({ listTitle: "Rhestr Gwrandawiadau Dyddiol Tribiwnlys Nawdd Cymdeithasol a Chynhaliaeth Plant Gogledd Ddwyrain Lloegr", - courtName: "Rhestr Gwrandawiadau Dyddiol Tribiwnlys Nawdd Cymdeithasol a Chynhaliaeth Plant Gogledd Ddwyrain Lloegr" + courtName: "Rhestr Gwrandawiadau Dyddiol Tribiwnlys Nawdd Cymdeithasol a Chynhaliaeth Plant Gogledd Ddwyrain Lloegr", + importantInformationText: "Important information for North East" }) );libs/list-types/sscs-daily-hearing-list/package.json (1)
17-17: ⚖️ Poor tradeoffConsider simplifying the Nunjucks build script.
The nested shell commands with
find,mkdir -p, and variable substitution are fragile and may fail on non-Unix platforms or with unusual file paths.Alternative approach using simpler commands
- "build:nunjucks": "mkdir -p dist/pages && cd src/pages && find . -name '*.njk' -exec sh -c 'mkdir -p ../../dist/pages/$(dirname {}) && cp {} ../../dist/pages/{}' \\; && cd ../.. && mkdir -p dist/pdf && cd src/pdf && find . -name '*.njk' -exec sh -c 'mkdir -p ../../dist/pdf/$(dirname {}) && cp {} ../../dist/pdf/{}' \\;", + "build:nunjucks": "mkdir -p dist/pages dist/pdf && cp -R src/pages/*.njk dist/pages/ && cp -R src/pdf/*.njk dist/pdf/",Or use a Node.js script for cross-platform reliability.
libs/list-types/sscs-daily-hearing-list/src/conversion/sscs-config.test.ts (1)
33-44: ⚡ Quick winTest coverage gap: should explicitly verify optional fields.
The test "should mark most fields as required" only verifies that certain fields are marked required, but doesn't check that
additionalInformationis marked optional. This gap means the schema/converter mismatch wouldn't be caught by tests.🧪 Proposed test improvement
it("should mark most fields as required", () => { const requiredFields = SSCS_EXCEL_CONFIG.fields.filter((f) => f.required).map((f) => f.fieldName); + const optionalFields = SSCS_EXCEL_CONFIG.fields.filter((f) => !f.required).map((f) => f.fieldName); expect(requiredFields).toContain("venue"); expect(requiredFields).toContain("appealReferenceNumber"); expect(requiredFields).toContain("hearingType"); expect(requiredFields).toContain("appellant"); expect(requiredFields).toContain("courtroom"); expect(requiredFields).toContain("hearingTime"); expect(requiredFields).toContain("tribunal"); expect(requiredFields).toContain("respondent"); + + expect(optionalFields).toContain("additionalInformation"); + expect(optionalFields.length).toBe(1); });libs/list-types/sscs-daily-hearing-list/src/pages/index.ts (1)
89-92: ⚡ Quick winSimplify nested ternary for better readability.
The nested ternary operator for determining
listTitleis difficult to follow. Consider extracting this logic into a helper function or using if-else statements.♻️ Proposed refactor
- const listTitle = - locale === "cy" - ? (listTypeEntry?.welshFriendlyName ?? listTypeEntry?.englishFriendlyName ?? t.listForDate) - : (listTypeEntry?.englishFriendlyName ?? t.listForDate); + const listTitle = + locale === "cy" + ? listTypeEntry?.welshFriendlyName || listTypeEntry?.englishFriendlyName || t.listForDate + : listTypeEntry?.englishFriendlyName || t.listForDate;Alternatively, extract to a helper function:
function getListTitle(listTypeEntry: any, locale: string, fallback: string): string { if (locale === "cy") { return listTypeEntry?.welshFriendlyName || listTypeEntry?.englishFriendlyName || fallback; } return listTypeEntry?.englishFriendlyName || fallback; } const listTitle = getListTitle(listTypeEntry, locale, t.listForDate);
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3fcf502f-a42f-4c81-90a1-14b098e496c2
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (36)
apps/web/src/app.test.tsapps/web/src/app.tsdocs/tickets/431/review.mddocs/tickets/431/tasks.mdlibs/admin-pages/package.jsonlibs/admin-pages/src/pages/non-strategic-upload-summary/index.tslibs/admin-pages/src/pages/non-strategic-upload/index.tslibs/list-types/sscs-daily-hearing-list/package.jsonlibs/list-types/sscs-daily-hearing-list/src/config.test.tslibs/list-types/sscs-daily-hearing-list/src/config.tslibs/list-types/sscs-daily-hearing-list/src/conversion/sscs-config.test.tslibs/list-types/sscs-daily-hearing-list/src/conversion/sscs-config.tslibs/list-types/sscs-daily-hearing-list/src/email-summary/summary-builder.test.tslibs/list-types/sscs-daily-hearing-list/src/email-summary/summary-builder.tslibs/list-types/sscs-daily-hearing-list/src/index.tslibs/list-types/sscs-daily-hearing-list/src/models/types.tslibs/list-types/sscs-daily-hearing-list/src/pages/cy.tslibs/list-types/sscs-daily-hearing-list/src/pages/en.tslibs/list-types/sscs-daily-hearing-list/src/pages/index.test.tslibs/list-types/sscs-daily-hearing-list/src/pages/index.tslibs/list-types/sscs-daily-hearing-list/src/pages/sscs-daily-hearing-list.njklibs/list-types/sscs-daily-hearing-list/src/pdf/pdf-generator.test.tslibs/list-types/sscs-daily-hearing-list/src/pdf/pdf-generator.tslibs/list-types/sscs-daily-hearing-list/src/pdf/pdf-template.njklibs/list-types/sscs-daily-hearing-list/src/rendering/renderer.test.tslibs/list-types/sscs-daily-hearing-list/src/rendering/renderer.tslibs/list-types/sscs-daily-hearing-list/src/schemas/sscs-daily-hearing-list.jsonlibs/list-types/sscs-daily-hearing-list/tsconfig.jsonlibs/location/src/list-type-data.tslibs/location/src/location-data.tslibs/publication/package.jsonlibs/publication/src/processing/service.test.tslibs/publication/src/processing/service.tslibs/subscriptions/src/repository/service.test.tslibs/system-admin-pages/src/user-management/validation.tstsconfig.json
| path: path.join(__dirname, "pages"), | ||
| prefix: "/sscs-daily-hearing-list" | ||
| }; | ||
| export const assets = path.join(__dirname, "assets/"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if assets directory exists and if the assets export is used anywhere
# Check if assets directory exists
if [ -d "libs/list-types/sscs-daily-hearing-list/src/assets" ]; then
echo "✓ Assets directory exists"
fd -t f . libs/list-types/sscs-daily-hearing-list/src/assets
else
echo "✗ Assets directory not found"
fi
# Search for usage of the assets export
echo ""
echo "Searching for usage of 'assets' import from config..."
rg -n "assets.*from.*sscs-daily-hearing-list.*config" --type=tsRepository: hmcts/cath-service
Length of output: 146
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Locate any assets directory within the module path
echo "== assets directories under sscs-daily-hearing-list =="
fd -t d "assets" libs/list-types/sscs-daily-hearing-list || true
fd -t d "assets" libs/list-types/sscs-daily-hearing-list -a || true
# 2) Find the actual config.ts file path and search for imports of it
echo
echo "== Imports of config.ts (moduleRoot/pageRoutes/assets consumers) =="
rg -n "sscs-daily-hearing-list.*/config(\.js|\.ts)?|sscs-daily-hearing-list/src/config" . -S --type=ts --type=tsx || true
# 3) Search for symbol usage: `assets` import from this config module
echo
echo "== Imports that pull in `assets` from the config module =="
rg -n "from ['\"].*sscs-daily-hearing-list.*config['\"]" libs -S --type=ts --type=tsx | head -n 200 || true
rg -n "(\bassets\b)\s*[,}]" libs/list-types/sscs-daily-hearing-list -S --type=ts --type=tsx || true
# 4) Search for runtime reference to the exported value (string 'assets/' or path usage)
echo
echo "== Direct string/path references to 'assets/' in the repo =="
rg -n "assets/\"|\bassets\b\s*\)" . -S --type=ts --type=tsx --type=js --type=mjs || trueRepository: hmcts/cath-service
Length of output: 476
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Locate any assets directory within the module path
echo "== assets directories under sscs-daily-hearing-list =="
fd -t d "assets" libs/list-types/sscs-daily-hearing-list || true
# 2) Find imports of the config module from anywhere in the repo
echo
echo "== Imports of sscs-daily-hearing-list config module =="
rg -n "sscs-daily-hearing-list.*/config(\.js|\.ts)?|sscs-daily-hearing-list/src/config" . -S --type=ts --type=tsx || true
# 3) Search for consumers importing `assets` specifically
echo
echo "== Imports that include `assets` from the config module =="
rg -n "from ['\"].*sscs-daily-hearing-list.*config['\"]" . -S --type=ts --type=tsx | head -n 200 || true
rg -n "import\s*\{[^}]*\bassets\b[^}]*\}\s*from\s*['\"].*sscs-daily-hearing-list.*config['\"]" . -S --type=ts --type=tsx || true
# 4) Search for direct string/path references that might indicate usage
echo
echo "== Direct references to 'assets/' or `assets` path join =="
rg -n "assets/\"|path\.join\([^)]*['\"]assets\/?['\"]" . -S --type=ts --type=tsx || trueRepository: hmcts/cath-service
Length of output: 617
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate module config.ts
echo "== Locate config.ts =="
fd -t f "config.ts" libs/list-types/sscs-daily-hearing-list -a || true
# Inspect config.ts
CONFIG_PATH=$(fd -t f -a "config.ts" libs/list-types/sscs-daily-hearing-list | head -n 1 || true)
if [ -n "${CONFIG_PATH:-}" ]; then
echo
echo "== config.ts: content (first 200 lines) =="
sed -n '1,200p' "$CONFIG_PATH" | cat -n
fi
# Check for assets directories
echo
echo "== assets directories under sscs-daily-hearing-list =="
fd -t d -i "assets" libs/list-types/sscs-daily-hearing-list || true
echo
echo "== specifically src/assets under module =="
if [ -d "libs/list-types/sscs-daily-hearing-list/src/assets" ]; then
echo "✓ libs/list-types/sscs-daily-hearing-list/src/assets exists"
fd -t f . libs/list-types/sscs-daily-hearing-list/src/assets || true
else
echo "✗ libs/list-types/sscs-daily-hearing-list/src/assets does not exist"
fi
# Searches for imports / usage of `assets`
echo
echo "== Find imports that destructure `assets` from the module's config =="
rg -n --glob '*.ts' --glob '*.tsx' \
"\\{[^}]*\\bassets\\b[^}]*\\}\\s*from\\s*['\"][^'\"]*sscs-daily-hearing-list[^'\"]*config['\"]" \
libs || true
echo
echo "== Find any reference to `config.assets` in repo =="
rg -n --glob '*.ts' --glob '*.tsx' "\\bconfig\\.assets\\b" libs || true
echo
echo "== Find direct references to the exported string 'assets/' =="
rg -n --glob '*.ts' --glob '*.tsx' "\\bassets/\"" libs || true
echo
echo "== Find direct references to identifier `assets` within this list-type package =="
rg -n --glob '*.ts' --glob '*.tsx' "\\bassets\\b" libs/list-types/sscs-daily-hearing-list || trueRepository: hmcts/cath-service
Length of output: 4260
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate module config.ts
echo "== Locate config.ts =="
fd -t f "config.ts" libs/list-types/sscs-daily-hearing-list -a || true
# Inspect config.ts
CONFIG_PATH=$(fd -t f -a "config.ts" libs/list-types/sscs-daily-hearing-list | head -n 1 || true)
if [ -n "${CONFIG_PATH:-}" ]; then
echo
echo "== config.ts: content (first 200 lines) =="
sed -n '1,200p' "$CONFIG_PATH" | cat -n
fi
# Check for assets directories
echo
echo "== assets directories under sscs-daily-hearing-list =="
fd -t d -i "assets" libs/list-types/sscs-daily-hearing-list || true
echo
echo "== specifically src/assets under module =="
if [ -d "libs/list-types/sscs-daily-hearing-list/src/assets" ]; then
echo "✓ libs/list-types/sscs-daily-hearing-list/src/assets exists"
fd -t f . libs/list-types/sscs-daily-hearing-list/src/assets || true
else
echo "✗ libs/list-types/sscs-daily-hearing-list/src/assets does not exist"
fi
# Searches for imports / usage of `assets`
echo
echo "== Find imports that destructure `assets` from the module's config =="
rg -n --glob '*.ts' --glob '*.tsx' \
"\\{[^}]*\\bassets\\b[^}]*\\}\\s*from\\s*['\"][^'\"]*sscs-daily-hearing-list[^'\"]*config['\"]" \
libs || true
echo
echo "== Find any reference to `config.assets` in repo =="
rg -n --glob '*.ts' --glob '*.tsx' "\\bconfig\\.assets\\b" libs || true
echo
echo "== Find direct references to the string 'assets/' =="
rg -n --glob '*.ts' --glob '*.tsx' "\\bassets/\"" libs || true
echo
echo "== Find direct references to identifier `assets` within this list-type package =="
rg -n --glob '*.ts' --glob '*.tsx' "\\bassets\\b" libs/list-types/sscs-daily-hearing-list || trueRepository: hmcts/cath-service
Length of output: 4251
Check assets export usage in sscs-daily-hearing-list
libs/list-types/sscs-daily-hearing-list/src/config.tsexportsassetspointing at__dirname/assets/(trailing slash), but this module contains noassets/directory.- No repo references were found that import
{ assets }from this config module or useconfig.assets; onlylibs/list-types/sscs-daily-hearing-list/src/config.test.tsreferences the export.
Either add the missing assets directory/files or remove/adjust the assets export + its test to match actual usage.
| listTitle | ||
| }); | ||
|
|
||
| const importantInformationText = getImportantInformationText(listTypeName, en); |
There was a problem hiding this comment.
Pass locale-specific translations to getImportantInformationText.
The function is called with the hardcoded en object instead of the locale-specific t, meaning Welsh pages will incorrectly display English important information text.
🐛 Proposed fix
- const importantInformationText = getImportantInformationText(listTypeName, en);
+ const importantInformationText = getImportantInformationText(listTypeName, t);Also update the function signature at line 25:
-function getImportantInformationText(listTypeName: string | undefined, _t: typeof en): string {
+function getImportantInformationText(listTypeName: string | undefined, t: typeof en | typeof cy): string {Note: Currently the importantInformationByListType mapping only exists in en.ts. You'll need to create a Welsh version in cy.ts or handle the fallback appropriately.
📝 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 importantInformationText = getImportantInformationText(listTypeName, en); | |
| const importantInformationText = getImportantInformationText(listTypeName, t); |
| </span> | ||
| </summary> | ||
| <div class="govuk-details__text"> | ||
| {% for line in importantInformationText.split('\n') %} |
There was a problem hiding this comment.
Add safety check for importantInformationText.
The code calls .split('\n') on importantInformationText without checking if it's defined. If the text is missing for a list type, this will cause a runtime error.
🛡️ Proposed fix
- {% for line in importantInformationText.split('\n') %}
+ {% for line in (importantInformationText or '').split('\n') %}
{% if line %}📝 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.
| {% for line in importantInformationText.split('\n') %} | |
| {% for line in (importantInformationText or '').split('\n') %} |
| it("should pass correct render options to renderer", async () => { | ||
| vi.mocked(generatePdfFromHtml).mockResolvedValue({ | ||
| success: true, | ||
| pdfBuffer: Buffer.from("PDF"), | ||
| sizeBytes: 100 | ||
| }); | ||
|
|
||
| const contentDate = new Date("2026-01-01"); | ||
| const listTitle = "London Social Security and Child Support Tribunal Daily Hearing List"; | ||
|
|
||
| await generateSscsDailyHearingListPdf({ | ||
| artefactId: "test-render-options", | ||
| contentDate, | ||
| locale: "cy", | ||
| locationId: "19", | ||
| jsonData: mockHearingList, | ||
| listTitle, | ||
| courtName: "London Social Security and Child Support Tribunal", | ||
| importantInformationText: "Open justice is a fundamental principle." | ||
| }); | ||
|
|
||
| expect(renderSscsDailyHearingListData).toHaveBeenCalledWith(mockHearingList, { | ||
| locale: "cy", | ||
| courtName: "London Social Security and Child Support Tribunal", | ||
| contentDate, | ||
| lastReceivedDate: expect.any(String), | ||
| listTitle | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Test does not verify lastReceivedDate value, allowing timestamp bugs to slip through.
Line 150 uses expect.any(String) which accepts any string value, including the hardcoded new Date().toISOString() currently in the implementation. The test should verify that the lastReceivedDate passed to the renderer matches the value from the artefact options, not just that it's present.
♻️ Strengthen the assertion
await generateSscsDailyHearingListPdf({
artefactId: "test-render-options",
contentDate,
locale: "cy",
locationId: "19",
jsonData: mockHearingList,
listTitle,
courtName: "London Social Security and Child Support Tribunal",
- importantInformationText: "Open justice is a fundamental principle."
+ importantInformationText: "Open justice is a fundamental principle.",
+ lastReceivedDate: "2026-01-01T10:00:00Z"
});
expect(renderSscsDailyHearingListData).toHaveBeenCalledWith(mockHearingList, {
locale: "cy",
courtName: "London Social Security and Child Support Tribunal",
contentDate,
- lastReceivedDate: expect.any(String),
+ lastReceivedDate: "2026-01-01T10:00:00Z",
listTitle
});📝 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 pass correct render options to renderer", async () => { | |
| vi.mocked(generatePdfFromHtml).mockResolvedValue({ | |
| success: true, | |
| pdfBuffer: Buffer.from("PDF"), | |
| sizeBytes: 100 | |
| }); | |
| const contentDate = new Date("2026-01-01"); | |
| const listTitle = "London Social Security and Child Support Tribunal Daily Hearing List"; | |
| await generateSscsDailyHearingListPdf({ | |
| artefactId: "test-render-options", | |
| contentDate, | |
| locale: "cy", | |
| locationId: "19", | |
| jsonData: mockHearingList, | |
| listTitle, | |
| courtName: "London Social Security and Child Support Tribunal", | |
| importantInformationText: "Open justice is a fundamental principle." | |
| }); | |
| expect(renderSscsDailyHearingListData).toHaveBeenCalledWith(mockHearingList, { | |
| locale: "cy", | |
| courtName: "London Social Security and Child Support Tribunal", | |
| contentDate, | |
| lastReceivedDate: expect.any(String), | |
| listTitle | |
| }); | |
| }); | |
| it("should pass correct render options to renderer", async () => { | |
| vi.mocked(generatePdfFromHtml).mockResolvedValue({ | |
| success: true, | |
| pdfBuffer: Buffer.from("PDF"), | |
| sizeBytes: 100 | |
| }); | |
| const contentDate = new Date("2026-01-01"); | |
| const listTitle = "London Social Security and Child Support Tribunal Daily Hearing List"; | |
| await generateSscsDailyHearingListPdf({ | |
| artefactId: "test-render-options", | |
| contentDate, | |
| locale: "cy", | |
| locationId: "19", | |
| jsonData: mockHearingList, | |
| listTitle, | |
| courtName: "London Social Security and Child Support Tribunal", | |
| importantInformationText: "Open justice is a fundamental principle.", | |
| lastReceivedDate: "2026-01-01T10:00:00Z" | |
| }); | |
| expect(renderSscsDailyHearingListData).toHaveBeenCalledWith(mockHearingList, { | |
| locale: "cy", | |
| courtName: "London Social Security and Child Support Tribunal", | |
| contentDate, | |
| lastReceivedDate: "2026-01-01T10:00:00Z", | |
| listTitle | |
| }); | |
| }); |
| const renderedData = renderSscsDailyHearingListData(options.jsonData, { | ||
| locale: options.locale, | ||
| courtName: options.courtName, | ||
| contentDate: options.contentDate, | ||
| lastReceivedDate: new Date().toISOString(), | ||
| listTitle: options.listTitle | ||
| }); |
There was a problem hiding this comment.
Hardcoded lastReceivedDate timestamp produces incorrect "last updated" display.
Line 33 creates a fresh timestamp at PDF generation time rather than using the artefact's actual lastReceivedDate. This means the rendered PDF will show "last updated" as the generation time, not when the hearing list data was genuinely received, misrepresenting data freshness to users.
Add lastReceivedDate: string to PdfGenerationOptions (or ensure BasePdfGenerationOptions includes it) and pass the artefact's timestamp through.
🐛 Proposed fix
interface PdfGenerationOptions extends BasePdfGenerationOptions<SscsDailyHearingList> {
contentDate: Date;
listTitle: string;
courtName: string;
importantInformationText: string;
+ lastReceivedDate: string;
} const renderedData = renderSscsDailyHearingListData(options.jsonData, {
locale: options.locale,
courtName: options.courtName,
contentDate: options.contentDate,
- lastReceivedDate: new Date().toISOString(),
+ lastReceivedDate: options.lastReceivedDate,
listTitle: options.listTitle
});| @@ -0,0 +1,66 @@ | |||
| <!DOCTYPE html> | |||
| <html lang="en"> | |||
There was a problem hiding this comment.
Make the lang attribute dynamic to support Welsh.
The lang attribute is hardcoded to "en", but the template must support both English and Welsh locales. The locale is available in the rendering context and should be used here.
🌐 Proposed fix
-<html lang="en">
+<html lang="{{ locale }}">You'll need to ensure locale is passed to the template context in pdf-generator.ts.
As per coding guidelines, every user-facing page must support both English and Welsh languages.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <html lang="en"> | |
| <html lang="{{ locale }}"> |
Source: Coding guidelines
| </tbody> | ||
| </table> | ||
| {% else %} | ||
| <p>No hearings scheduled.</p> |
There was a problem hiding this comment.
Replace hardcoded English text with translated string.
The text "No hearings scheduled." is hardcoded in English, breaking Welsh language support. Use a translated string from the t object instead.
🌐 Proposed fix
- <p>No hearings scheduled.</p>
+ <p>{{ t.noHearingsScheduled }}</p>Add the corresponding translations to en.ts and cy.ts:
// en.ts
noHearingsScheduled: "No hearings scheduled."
// cy.ts
noHearingsScheduled: "Dim gwrandawiadau wedi'u trefnu."As per coding guidelines, every user-facing page must support both English and Welsh languages.
📝 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.
| <p>No hearings scheduled.</p> | |
| <p>{{ t.noHearingsScheduled }}</p> |
Source: Coding guidelines
| "type": "array", | ||
| "items": { | ||
| "type": "object", | ||
| "required": ["venue", "appealReferenceNumber", "hearingType", "appellant", "courtroom", "hearingTime", "tribunal", "respondent", "additionalInformation"], |
There was a problem hiding this comment.
Root cause: additionalInformation required/optional contract mismatch across schema and converter.
The JSON schema marks additionalInformation as required (line 8 of sscs-daily-hearing-list.json), whilst the Excel converter config marks it as optional (required: false on line 56 of sscs-config.ts). This contract violation will cause validation failures when Excel files lacking additional information are converted and then validated. Both files must agree on whether this field is required or optional.
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
🎭 Playwright E2E Test Results84 tests 52 ✅ 6m 18s ⏱️ Results for commit 8cb85f3. ♻️ This comment has been updated with latest results. |
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Fixes YN0028 lockfile immutability error in CI caused by vitest version mismatch after master merged chore(deps): update vitest monorepo to v4.1.8. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ishing Resolves yarn.lock conflict by regenerating lockfile after merging master. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
…blishing # Conflicts: # apps/web/src/app.test.ts # apps/web/src/app.ts # apps/web/src/pages/(admin)/non-strategic-upload-summary/index.ts # apps/web/src/pages/(admin)/non-strategic-upload/index.ts
…ng new architecture Move SSCS page controller, template, and tests from libs to apps/web/src/pages/(list-types) following the pattern established in master. Move cy/en translations to libs/locales/, export with camelCase aliases from index.ts, remove pageRoutes from config and add schemaPath. Fix TypeScript errors in non-strategic upload tests and remove stale @ts-expect-error directive. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/pages/(list-types)/sscs-daily-hearing-list/index.ts (1)
77-82:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle malformed JSON as invalid data, not a server error.
If JSON parsing fails, the request currently falls through to the outer catch and returns 500. This is bad input data and should return the existing 400 invalid-data response path.
Suggested fix
- const jsonData: SscsDailyHearingList = JSON.parse(jsonContent); + let jsonData: SscsDailyHearingList; + try { + jsonData = JSON.parse(jsonContent) as SscsDailyHearingList; + } catch { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Invalid Data", + errorMessage: "The list data is invalid" + }); + } const validationResult = validate(jsonData);
🧹 Nitpick comments (1)
apps/web/src/pages/(list-types)/sscs-daily-hearing-list/index.test.ts (1)
349-349: Avoidas anyin the render payload assertion.The
as anycast weakens strict typing in this test. UsetoMatchObjectwithexpect.stringContaininginstead to maintain type safety without the escape hatch.Suggested fix
- expect((renderCall[1] as any).importantInformationText).toContain("sscsa-sutton@justice.gov.uk"); + expect(renderCall[1]).toMatchObject({ + importantInformationText: expect.stringContaining("sscsa-sutton@justice.gov.uk") + });Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0688aea4-b548-4512-b7bd-f5d7e3eaa6b0
📒 Files selected for processing (9)
apps/web/src/app.test.tsapps/web/src/app.tsapps/web/src/pages/(admin)/non-strategic-upload-summary/index.test.tsapps/web/src/pages/(admin)/non-strategic-upload-summary/index.tsapps/web/src/pages/(admin)/non-strategic-upload/index.test.tsapps/web/src/pages/(admin)/non-strategic-upload/index.tsapps/web/src/pages/(list-types)/sscs-daily-hearing-list/index.test.tsapps/web/src/pages/(list-types)/sscs-daily-hearing-list/index.tsapps/web/src/pages/(list-types)/sscs-daily-hearing-list/sscs-daily-hearing-list.njk
💤 Files with no reviewable changes (1)
- apps/web/src/pages/(list-types)/sscs-daily-hearing-list/sscs-daily-hearing-list.njk
✅ Files skipped from review due to trivial changes (2)
- apps/web/src/pages/(admin)/non-strategic-upload-summary/index.test.ts
- apps/web/src/pages/(admin)/non-strategic-upload/index.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/app.test.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/pages/(list-types)/sscs-daily-hearing-list/index.ts (1)
77-82:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle malformed JSON as invalid data, not a server error.
If JSON parsing fails, the request currently falls through to the outer catch and returns 500. This is bad input data and should return the existing 400 invalid-data response path.
Suggested fix
- const jsonData: SscsDailyHearingList = JSON.parse(jsonContent); + let jsonData: SscsDailyHearingList; + try { + jsonData = JSON.parse(jsonContent) as SscsDailyHearingList; + } catch { + return res.status(400).render("errors/common", { + en, + cy, + errorTitle: "Invalid Data", + errorMessage: "The list data is invalid" + }); + } const validationResult = validate(jsonData);
🧹 Nitpick comments (1)
apps/web/src/pages/(list-types)/sscs-daily-hearing-list/index.test.ts (1)
349-349: Avoidas anyin the render payload assertion.The
as anycast weakens strict typing in this test. UsetoMatchObjectwithexpect.stringContaininginstead to maintain type safety without the escape hatch.Suggested fix
- expect((renderCall[1] as any).importantInformationText).toContain("sscsa-sutton@justice.gov.uk"); + expect(renderCall[1]).toMatchObject({ + importantInformationText: expect.stringContaining("sscsa-sutton@justice.gov.uk") + });Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0688aea4-b548-4512-b7bd-f5d7e3eaa6b0
📒 Files selected for processing (9)
apps/web/src/app.test.tsapps/web/src/app.tsapps/web/src/pages/(admin)/non-strategic-upload-summary/index.test.tsapps/web/src/pages/(admin)/non-strategic-upload-summary/index.tsapps/web/src/pages/(admin)/non-strategic-upload/index.test.tsapps/web/src/pages/(admin)/non-strategic-upload/index.tsapps/web/src/pages/(list-types)/sscs-daily-hearing-list/index.test.tsapps/web/src/pages/(list-types)/sscs-daily-hearing-list/index.tsapps/web/src/pages/(list-types)/sscs-daily-hearing-list/sscs-daily-hearing-list.njk
💤 Files with no reviewable changes (1)
- apps/web/src/pages/(list-types)/sscs-daily-hearing-list/sscs-daily-hearing-list.njk
✅ Files skipped from review due to trivial changes (2)
- apps/web/src/pages/(admin)/non-strategic-upload-summary/index.test.ts
- apps/web/src/pages/(admin)/non-strategic-upload/index.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/app.test.ts
🛑 Comments failed to post (2)
apps/web/src/pages/(admin)/non-strategic-upload-summary/index.ts (1)
24-24:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse friendly English name as fallback before raw list type value.
At Line 24, the current fallback can render a raw identifier when Welsh text is missing, even if
friendlyNameexists. PreferfriendlyNamebeforeuploadData.listTypeto keep user-facing text readable.As per coding guidelines, "Implement Welsh language support on all user-facing text with separate
en.tsandcy.tscontent files".Suggested patch
- const listTypeName = listType ? (locale === "cy" ? listType.welshFriendlyName : listType.friendlyName) || uploadData.listType : uploadData.listType; + const listTypeName = listType + ? (locale === "cy" ? listType.welshFriendlyName : listType.friendlyName) || listType.friendlyName || uploadData.listType + : uploadData.listType;📝 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 listTypeName = listType ? (locale === "cy" ? listType.welshFriendlyName : listType.friendlyName) || listType.friendlyName || uploadData.listType : uploadData.listType;Source: Coding guidelines
apps/web/src/pages/(list-types)/sscs-daily-hearing-list/index.ts (1)
20-21:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove upload-directory configuration to environment variables.
MONOREPO_ROOT/TEMP_UPLOAD_DIRare derived from a hardcoded relative filesystem layout. This is fragile across deployment layouts and breaks the repo’s configuration rule.Suggested fix
-const MONOREPO_ROOT = path.join(__dirname, "..", "..", "..", "..", "..", ".."); -const TEMP_UPLOAD_DIR = path.join(MONOREPO_ROOT, "storage", "temp", "uploads"); +const TEMP_UPLOAD_DIR = process.env.TEMP_UPLOAD_DIR; + +if (!TEMP_UPLOAD_DIR) { + throw new Error("TEMP_UPLOAD_DIR environment variable must be set"); +}As per coding guidelines, "Use environment variables for all configuration values and secrets, never hardcode them".
Source: Coding guidelines
…-list module The civil-daily-cause-list module was added to the monorepo so the test for 'no JSON schema available' could no longer use CIVIL_DAILY_CAUSE_LIST (ID 1). Switch to CROWN_COURT_DAILY_LIST (ID 99) which has no corresponding package, and add a mock for @hmcts/civil-daily-cause-list to prevent the real module import from causing test timeouts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The @hmcts/sscs-daily-hearing-list module registers converters at import time via sscs-config.ts, which requires createConverter, registerConverter, registerConverterByName and validateNoHtmlTags from @hmcts/list-types-common. Using importOriginal spreads the real module so all exports are available, matching the pattern used in care-standards-tribunal tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Register all 8 SSCS list type variants in EMAIL_BUILDER_REGISTRY so Gov Notifier emails include the case summary and PDF download link. Fixes missing email summary (HEARING_TIME, HEARING_TYPE, APPEAL_REFERENCE_NUMBER) and missing PDF download link for SSCS publications. Also adds missing @hmcts/civil-daily-cause-list, @hmcts/family-daily-cause-list, and @hmcts/sscs-daily-hearing-list to notifications package dependencies. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a94ef4f-b226-4651-9f51-365507b67981
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
apps/web/src/pages/(list-types)/sscs-daily-hearing-list/index.test.tslibs/list-types/common/src/validation/list-type-validator.test.tslibs/notifications/package.jsonlibs/notifications/src/notification/notification-service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/pages/(list-types)/sscs-daily-hearing-list/index.test.ts
|
|
||
| const rcjStandardConfig: EmailBuilderConfig = { extract: extractRcjSummary as SummaryExtractor, format: formatRcjSummaryForEmail }; | ||
| const adminCourtConfig: EmailBuilderConfig = { extract: extractAdminCourtSummary as SummaryExtractor, format: formatAdminCourtSummaryForEmail }; | ||
| const sscsConfig: EmailBuilderConfig = { extract: extractSscsSummary as SummaryExtractor, format: formatSscsSummaryForEmail }; |
There was a problem hiding this comment.
Use SCREAMING_SNAKE_CASE for the new constant.
Line 60 introduces sscsConfig, which breaks the repository constant naming rule. Rename it and its usages (e.g. SSCS_CONFIG) to keep lint/compliance consistent.
As per coding guidelines, "Use SCREAMING_SNAKE_CASE for constant declarations (e.g., MAX_FILE_SIZE, DEFAULT_TIMEOUT)."
Suggested patch
-const sscsConfig: EmailBuilderConfig = { extract: extractSscsSummary as SummaryExtractor, format: formatSscsSummaryForEmail };
+const SSCS_CONFIG: EmailBuilderConfig = { extract: extractSscsSummary as SummaryExtractor, format: formatSscsSummaryForEmail };
@@
- SSCS_LONDON_DAILY_HEARING_LIST: sscsConfig,
- SSCS_MIDLANDS_DAILY_HEARING_LIST: sscsConfig,
- SSCS_SOUTH_EAST_DAILY_HEARING_LIST: sscsConfig,
- SSCS_WALES_AND_SOUTH_WEST_DAILY_HEARING_LIST: sscsConfig,
- SSCS_SCOTLAND_DAILY_HEARING_LIST: sscsConfig,
- SSCS_NORTH_EAST_DAILY_HEARING_LIST: sscsConfig,
- SSCS_NORTH_WEST_DAILY_HEARING_LIST: sscsConfig,
- SSCS_LIVERPOOL_DAILY_HEARING_LIST: sscsConfig
+ SSCS_LONDON_DAILY_HEARING_LIST: SSCS_CONFIG,
+ SSCS_MIDLANDS_DAILY_HEARING_LIST: SSCS_CONFIG,
+ SSCS_SOUTH_EAST_DAILY_HEARING_LIST: SSCS_CONFIG,
+ SSCS_WALES_AND_SOUTH_WEST_DAILY_HEARING_LIST: SSCS_CONFIG,
+ SSCS_SCOTLAND_DAILY_HEARING_LIST: SSCS_CONFIG,
+ SSCS_NORTH_EAST_DAILY_HEARING_LIST: SSCS_CONFIG,
+ SSCS_NORTH_WEST_DAILY_HEARING_LIST: SSCS_CONFIG,
+ SSCS_LIVERPOOL_DAILY_HEARING_LIST: SSCS_CONFIGSource: Coding guidelines
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. |
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Resolves conflicts between SSCS tribunal list types (this branch) and the new tribunal list types added in master (SIAC, FTT, GRC, SEND, WPAFCC, UTIAC etc). Both sets of list types are preserved with non-overlapping IDs (SSCS renumbered to 49-55 in list-type-data.ts, locations 17-24 in location-data.ts). 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. |
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>
3 participants
Jira link
#431
Change description
Add SSCS Lists
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation