Skip to content

feat(#593): implement Employment Tribunal daily and fortnightly press lists - #864

Merged
junaidiqbalmoj merged 22 commits into
masterfrom
feat/593-employment-tribunal-lists
Jul 24, 2026
Merged

feat(#593): implement Employment Tribunal daily and fortnightly press lists#864
junaidiqbalmoj merged 22 commits into
masterfrom
feat/593-employment-tribunal-lists

Conversation

@junaidiqbalmoj

@junaidiqbalmoj junaidiqbalmoj commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements the two Employment Tribunal list types as strategic JSON cause lists, following the civil-and-family-daily-cause-list reference and reusing @hmcts/daily-cause-list-common for rendering, PDF, and email-summary logic.

Closes #593.

Field ET_DAILY_LIST ET_FORTNIGHTLY_PRESS_LIST
English friendly name Employment Tribunals Daily List Employment Tribunals Fortnightly Press List
Welsh friendly name Rhestr Ddyddiol y Tribiwnlysoedd Cyflogaeth Rhestr y Wasg Pob Pythefnos y Tribiwnlysoedd Cyflogaeth
url path et-daily-list et-fortnightly-list
Jurisdiction Employment Tribunal (sub-jurisdiction 3) Employment Tribunal (sub-jurisdiction 3)
Sensitivity / provenance Public / CFT_IDAM Public / CFT_IDAM

Sources of truth: pip-frontend listLookup.json + locales/, pip-data-management et_daily_list.json / et_fortnightly_press_list.json schemas and the EtDailyListSummaryData / EtFortnightlyPressListSummaryData summary classes.

Changes

  • Two new libslibs/list-types/et-daily-list/ and libs/list-types/et-fortnightly-list/, each with its own verbatim schema, validate* wrapper + tests (one it per required field at every nesting depth), locales (EN/CY), and thin re-export shims for renderer/PDF/email-summary over @hmcts/daily-cause-list-common.
  • PagesGET /et-daily-list?artefactId= and GET /et-fortnightly-list?artefactId= with the 7-column hearings table, Open Justice accordion (full openJustice1openJustice6 with dynamic venue contact), functional case search, and data-source attribution. Welsh via ?lng=cy.
  • PDF generation — per list type, delegating to the shared generateDailyCauseListPdf; the fortnightly list renders representative rows.
  • Email summaries — shared extractEtCaseSummary (Claimant, Respondent, Case reference, Hearing type), registered in the notification service EMAIL_BUILDER_REGISTRY.
  • RegistrationPDF_GENERATOR_REGISTRY (by listTypeName), list-type-data.ts catalogue, app.ts module paths, root tsconfig.json aliases, workspace deps, and a PACKAGE_ALIASES entry resolving the et-fortnightly-press-list kebab mismatch.
  • Production seed scripts — added ET rows to 001_insert_missing_list_types.sql and Employment Tribunal links to 003_upsert_sub_jurisdictions_and_list_type_links.sql (verified idempotent against a local Postgres). Documented this three-file sync as a mandatory default for future list types in CLAUDE.md.

Notes for reviewers

  • Titles override the upstream locale title values (which are buggy for the fortnightly list) with the listLookup.json friendly names, in both EN and CY.
  • All routing/registration keys on the stable listTypeName; no numeric listTypeId anywhere.
  • The PCOL_DAILY_CAUSE_LIST entry from Style Guide: PCOL Daily List  #438 was intentionally removed on this branch per instruction — confirm that is expected before merging (it was a separate list type).
  • Open Justice item 4 renders venue.venueContact dynamically; behaviour when the contact is absent (renders the intro line alone) is pending PO confirmation.

Testing

  • Full workspace: yarn lint, yarn build (tsc), and yarn test all pass (63/63 workspaces).
  • Coverage on changed code all >80% (daily-cause-list-common 96%, ET libs 85.7%, notifications 89%, publication 96.6%).
  • Project duplicate-code check: the ET libs contribute 0 of the project's clones.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added Employment Tribunal daily and fortnightly list pages, including English and Welsh content.
    • Added case search, structured court/hearing tables, and responsive table behaviour.
    • Added PDF generation and email summaries for both list types.
    • Added schema validation and access-controlled routes for the new pages.
  • Bug Fixes
    • Improved rendering when venue address details are missing.
    • Improved list catalogue reconciliation (restores re-added items and soft-deletes removed ones, with test/E2E exemptions).
  • Tests
    • Added comprehensive coverage for routes, rendering, validation, PDFs, and reference-data seeding.

junaidiqbalmoj and others added 5 commits July 16, 2026 13:35
… lists

Add two new list types following the civil-and-family-daily-cause-list
reference and reusing @hmcts/daily-cause-list-common for rendering/PDF:

- libs/list-types/et-daily-list (ET_DAILY_LIST)
- libs/list-types/et-fortnightly-list (ET_FORTNIGHTLY_PRESS_LIST)

Each lib ships its own verbatim schema, validator + tests, locales
(EN/CY), PDF generator/template, and config. Page controllers and
templates live in apps/web with the 7-column hearings table, Open
Justice accordion, and dynamic venue-contact injection. Registered by
listTypeName in PDF_GENERATOR_REGISTRY, the list-type catalogue, app.ts
module paths, tsconfig aliases, and workspace deps. Added a
PACKAGE_ALIASES entry to resolve the et-fortnightly-press-list kebab
mismatch to the et-fortnightly-list package.

Review fixes: made the case-search input functional (hearings-table
class), extracted inline template styles to a shared SCSS partial, and
replaced <em> rep labels with neutral spans. Removed the
PCOL_DAILY_CAUSE_LIST catalogue entry and its tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts:
#	libs/publication/package.json
#	libs/publication/src/processing/service.ts
#	yarn.lock
…ement

Register ET_DAILY_LIST and ET_FORTNIGHTLY_PRESS_LIST in the production
seed scripts (seeding is skipped in prod, so listTypeData alone does not
populate prod):

- 001_insert_missing_list_types.sql — two list_types rows
- 003_upsert_sub_jurisdictions_and_list_type_links.sql — Employment
  Tribunal (sub-jurisdiction id 3) links

Both scripts verified idempotent against a local Postgres instance.

Also add a mandatory checklist item to CLAUDE.md so every future list
type updates all three reference-data files (TS catalogue + both SQL
scripts) by default, without prompting.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ator

Both ET pdf-generator.ts files were byte-identical except the function
name, duplicating the render/translate/template/save flow already
provided by generateDailyCauseListPdf in @hmcts/daily-cause-list-common.
Reduce each to a thin wrapper that maps the provenance label and
delegates to the shared generator, cutting duplicated lines between the
two libs from 65 to 20 and removing the ET libs from the project's
duplicate-code report entirely. Tests updated to verify the delegation
(provenance-label mapping, option/locale forwarding, result passthrough).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both ET lists produce the same subscription-email summary (per
pip-data-management EtDailyListSummaryData / EtFortnightlyPressListSummaryData):
Claimant, Respondent, Case reference, and Hearing type per case.

Add a shared extractEtCaseSummary to @hmcts/daily-cause-list-common (with
tests) and re-export it as extractCaseSummary from each ET lib, mirroring
the renderer/pdf re-export pattern and avoiding duplication between the two
libs. Register both list types in the notification service
EMAIL_BUILDER_REGISTRY so subscription emails include the case summary.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f08d7d0a-d7b7-4e3b-8d15-77d89eb01e19

📥 Commits

Reviewing files that changed from the base of the PR and between b9082ed and b2addf2.

📒 Files selected for processing (2)
  • libs/list-types/common/src/list-type-data.ts
  • libs/notifications/src/notification/notification-service.ts

📝 Walkthrough

Walkthrough

Adds Employment Tribunal daily and fortnightly list modules with separate schemas, Welsh and English rendering, HTML/PDF templates, web routes, email summaries, publication integration, catalogue registration, and deploy-time reference-data seeding with reconciliation.

Changes

Employment Tribunal list modules

Layer / File(s) Summary
Shared contracts and helpers
libs/list-types/common/*, libs/list-types/et-list-common/*, libs/list-types/daily-cause-list-common/*
Adds shared cause-list types, ET party and hearing formatting, region resolution, case-summary extraction, and common exports.
Daily and fortnightly list packages
libs/list-types/et-daily-list/*, libs/list-types/et-fortnightly-list/*
Adds schemas, validators, renderers, locales, PDF generators, templates, package configuration, and unit tests for both list types.
Application routes and registries
apps/web/src/pages/..., apps/web/src/app.ts, libs/publication/*, libs/notifications/*, tsconfig.json, apps/web/package.json
Registers module paths, routes, PDF generators, email-summary builders, dependencies, and ET styles.
Reference-data seeding
apps/postgres/*, libs/location/src/seed-list-types.ts, .github/workflows/*, CLAUDE.md
Replaces SQL seed execution with deploy-time TypeScript seeding and adds list-type soft-delete/reactivation reconciliation.
Ticket documentation
docs/tickets/593/*
Adds the ticket requirements, implementation plan, task checklist, review record, acceptance mapping, and follow-ups.

Possibly related issues

  • #610 — Covers a similar tribunal list implementation with dedicated schemas, renderers, PDFs, routes, and localisation.
  • #612 — Covers another tribunal hearing-list implementation using the same list-type architecture.

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning Core list, page, PDF, and test work is present, but the PR implements email-summary wiring that the issue explicitly said to defer. Remove or isolate the email-summary changes until requirements are confirmed, and ensure any required converter registration is present.
Out of Scope Changes check ⚠️ Warning The email-summary extraction and notification registration are outside the linked issue scope, which asked to defer email summaries. Split the email-summary work into a separate PR or gate it behind confirmed requirements.
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarises the main change: adding ET daily and fortnightly press lists.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/593-employment-tribunal-lists

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

🎭 Playwright E2E Test Results

84 tests   52 ✅  6m 15s ⏱️
33 suites  32 💤
 1 files     0 ❌

Results for commit 29573c0.

♻️ This comment has been updated with latest results.

@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment Successful 🚀

Your preview environment is ready:

The environment will be automatically cleaned up when this PR is closed.

- Guard missing venueAddress in shared cause-list renderer (ET venue
  schema has no address), fixing the page/PDF crash and restoring the
  notification PDF download link
- Add "Find contact details..." fact-link intro to web and PDF
- Render venue contact (name, email, phone) on a single line
- Always show "Venue:" with courthouse name; address lines stay optional
- Make hearing table columns sortable on web (moj-sortable-table)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment Successful 🚀

Your preview environment is ready:

The environment will be automatically cleaned up when this PR is closed.

junaidiqbalmoj and others added 5 commits July 16, 2026 15:40
The ET fortnightly list was reusing the shared daily cause list renderer
and template, but the upstream reference groups by courthouse then by
sitting day (one accordion section per date) rather than by courtroom.

- Add standalone renderEtFortnightlyList that flattens to courts -> days
  -> rows, initialising applicant, respondent and representative names
- Rewrite HTML and PDF templates for the day-grouped accordion, showing
  Rep lines and the case sequence indicator in the duration column
- Make the fortnightly PDF generator standalone (no longer delegates to
  the shared daily generator)
- Rename displayed title to "Employment Tribunals Fortnightly List"
- Render representative text as plain cell text, matching party names

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
extractEtCaseSummary is ET-specific but lived in daily-cause-list-common.
Move it (and its test) into et-daily-list, the base ET lib, and have
et-fortnightly-list re-export it from @hmcts/et-daily-list. The genuinely
shared party-extractor stays in common.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The et-fortnightly -> et-daily edge (added to share extractEtCaseSummary)
plus a stray et-daily -> publication package dep closed two dependency
cycles through @hmcts/publication and @hmcts/notifications, failing
`yarn build`.

- Give et-fortnightly-list its own local et-summary-builder (no cross-ET
  package dependency)
- Drop the @hmcts/publication package.json dependency from et-daily-list;
  its source imports (PROVENANCE_LABELS, ValidationResult) resolve via
  tsconfig path aliases, matching how the other daily-cause-list libs
  avoid the same cycle

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment Successful 🚀

Your preview environment is ready:

The environment will be automatically cleaned up when this PR is closed.

…mmon

Removes the ~9.9% code duplication between et-daily-list and
et-fortnightly-list (identical summary builder plus renderer helpers)
reported by SonarQube on new code.

- New @hmcts/et-list-common lib holds the shared ET logic: party-name
  formatting (initialised), hearing/date/address/duration/channel
  formatting, region resolution, and the email case-summary builder
- Both ET libs now import these helpers instead of duplicating them;
  their renderers keep only their list-specific shaping (daily mutates
  the tree; fortnightly splits into courthouse -> day -> rows)
- The new lib depends only on location, list-types-common and
  daily-cause-list-common (types), none of which depend on ET libs, so
  no build cycle is introduced

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment Successful 🚀

Your preview environment is ready:

The environment will be automatically cleaned up when this PR is closed.

}
];

describe.each(ET_LIST_TYPES)("listTypeData Employment Tribunal list $name", (listType) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test file here is just testing the content of the static list type data array. I don't think we need to test the ET list specifically. May be can remove this test file?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as discussed, removed

* Used when a list type shares the same JSON schema as another list type.
*/
const PACKAGE_ALIASES: Record<string, string> = {
"et-fortnightly-press-list": "et-fortnightly-list",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this only added for fortnightly list but not the daily list?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The daily list's DB name ET_DAILY_LIST kebab-cases to et-daily-list, which matches its package @hmcts/et-daily-list exactly, so the import resolves with no alias. The fortnightly list's DB name is ET_FORTNIGHTLY_PRESS_LIST (→ et-fortnightly-press-list) but its package is @hmcts/et-fortnightly-list, so it needs the alias to bridge that name difference. Anything whose kebab-cased name already matches its package name is intentionally left out of PACKAGE_ALIASES.
Reason explained by AI and it makes sense.

@@ -0,0 +1,98 @@
import type { Session, Sitting } from "@hmcts/daily-cause-list-common";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see the daily-cause-list-common lib is included in this this file and other files within the et-list-common package. I can also see so far the models in daily-cause-list-common are only used by Civil, Family and mixed lists before the ET lists.
If the model classes/interface within daily-cause-list-common are meant to be used by other list-type libs, may be it should be within libs/list-types/common instead of daily-cause-list-common. Otherwise define ET list owned models within et-list-common. The cross list type dependencies are going to cause confusion.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create common list which is being used by different lists now.

The ET list libs imported cause-list model interfaces (Session, Sitting,
CauseListCase, etc.) from @hmcts/daily-cause-list-common, whose name only
reflected its original Civil/Family/mixed consumers. Relocate the shared
interfaces to @hmcts/list-types-common — the base package every list-type
lib already depends on — so the dependency graph matches the naming.

- Add libs/list-types/common/src/models/cause-list-types.ts as the single
  source for the nine shared interfaces; export from the package index.
- Drop the duplicate Party interface in crown-utilities.ts; import it from
  the new models file instead.
- Keep daily-cause-list-common working via a type re-export shim so the
  Civil/Family libs are untouched.
- Repoint the ET libs at list-types-common and update their package deps.
- Dedup date/time formatters: et-list-common now re-exports formatTime,
  formatContentDate and formatPublicationDateTime from list-types-common
  rather than keeping identical copies.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment Successful 🚀

Your preview environment is ready:

The environment will be automatically cleaned up when this PR is closed.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 469ec9b8-bae7-42de-9f6f-81b66a5c5e65

📥 Commits

Reviewing files that changed from the base of the PR and between 44d9ad8 and 3054af8.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (11)
  • CLAUDE.md
  • apps/postgres/prisma/scripts/005_insert_magistrates_adult_lists.sql
  • apps/postgres/start.sh
  • apps/web/package.json
  • apps/web/src/app.ts
  • libs/list-types/common/src/list-type-data.ts
  • libs/notifications/package.json
  • libs/notifications/src/notification/notification-service.ts
  • libs/publication/package.json
  • libs/publication/src/processing/service.ts
  • tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (9)
  • libs/publication/package.json
  • apps/web/src/app.ts
  • libs/publication/src/processing/service.ts
  • libs/notifications/package.json
  • apps/web/package.json
  • libs/notifications/src/notification/notification-service.ts
  • tsconfig.json
  • libs/list-types/common/src/list-type-data.ts
  • CLAUDE.md

Comment on lines +14 to +22
ON CONFLICT (name) DO UPDATE SET
friendly_name = EXCLUDED.friendly_name,
welsh_friendly_name = EXCLUDED.welsh_friendly_name,
shortened_friendly_name = EXCLUDED.shortened_friendly_name,
url = EXCLUDED.url,
default_sensitivity = EXCLUDED.default_sensitivity,
allowed_provenance = EXCLUDED.allowed_provenance,
is_non_strategic = EXCLUDED.is_non_strategic,
updated_at = EXCLUDED.updated_at;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Reactivate soft-deleted rows during the upsert.

Because list_types.deleted_at is not cleared, re-running this seed leaves a matching soft-deleted list type inactive. That contradicts the “safe to re-run at any time” guarantee and can prevent these lists from being discovered.

 ON CONFLICT (name) DO UPDATE SET
   friendly_name           = EXCLUDED.friendly_name,
   welsh_friendly_name     = EXCLUDED.welsh_friendly_name,
   shortened_friendly_name = EXCLUDED.shortened_friendly_name,
   url                     = EXCLUDED.url,
   default_sensitivity     = EXCLUDED.default_sensitivity,
   allowed_provenance      = EXCLUDED.allowed_provenance,
   is_non_strategic        = EXCLUDED.is_non_strategic,
+  deleted_at              = NULL,
   updated_at              = EXCLUDED.updated_at;
📝 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.

Suggested change
ON CONFLICT (name) DO UPDATE SET
friendly_name = EXCLUDED.friendly_name,
welsh_friendly_name = EXCLUDED.welsh_friendly_name,
shortened_friendly_name = EXCLUDED.shortened_friendly_name,
url = EXCLUDED.url,
default_sensitivity = EXCLUDED.default_sensitivity,
allowed_provenance = EXCLUDED.allowed_provenance,
is_non_strategic = EXCLUDED.is_non_strategic,
updated_at = EXCLUDED.updated_at;
ON CONFLICT (name) DO UPDATE SET
friendly_name = EXCLUDED.friendly_name,
welsh_friendly_name = EXCLUDED.welsh_friendly_name,
shortened_friendly_name = EXCLUDED.shortened_friendly_name,
url = EXCLUDED.url,
default_sensitivity = EXCLUDED.default_sensitivity,
allowed_provenance = EXCLUDED.allowed_provenance,
is_non_strategic = EXCLUDED.is_non_strategic,
deleted_at = NULL,
updated_at = EXCLUDED.updated_at;

@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment Successful 🚀

Your preview environment is ready:

The environment will be automatically cleaned up when this PR is closed.

Make libs/list-types/common/src/list-type-data.ts the single source of
truth for list types across local and STG. The postgres migration-runner
now runs the TypeScript seed (seedLocationData -> seedListTypes) on deploy
instead of hand-written SQL scripts, so adding or updating a listTypeData
entry is reflected automatically.

- start.sh: replace the 001-005 SQL script loop with `prisma generate`
  + `tsx prisma/seed-deploy.ts`
- add seed-deploy.ts deploy entry point calling seedLocationData()
- add @hmcts/location and tsx as deps of @hmcts/postgres so the focused
  runtime install resolves them
- seed-list-types.ts: soft-delete list types no longer in listTypeData
  (excludes TEST_/E2E_), and clear deletedAt on re-add
- seed.ts: delegate to seedLocationData(); drop the duplicated inline
  reference-data logic and the unused seedTestData fixtures
- remove seed.sql, the `seed` npm script, and the CI seed + ">=3 artefacts"
  gate from e2e/nightly workflows (Playwright global-setup seeds its own
  location-9 data; nothing referenced the fixed-UUID fixtures)
- delete apps/postgres/prisma/scripts/001-005
- CLAUDE.md: rewrite List Type item 7 for the single-source-of-truth model

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
apps/postgres/start.sh (1)

82-86: 🧹 Nitpick | 🔵 Trivial

Confirm restart semantics on seed failure.

set -e means a seedListTypes()/seedLocationData() failure aborts the whole script after the health proxy has already signalled Ready. Confirm the pod's restart policy is expected to retry the whole migration/seed job in this case, rather than leaving it in a partially-migrated-but-unseeded state.

libs/location/src/seed-list-types.test.ts (1)

95-111: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a regression test for an empty listTypeData.

Good coverage of the reconciliation where shape, but there's no test asserting behaviour when listTypeData is empty — the scenario flagged on libs/location/src/seed-list-types.ts (Lines 83-96) where notIn: [] would match every active list type. Once a guard is added there, a test here would lock in the safe behaviour.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 78e988e5-615c-472e-8275-74463fc85c9c

📥 Commits

Reviewing files that changed from the base of the PR and between 3054af8 and 21fe848.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (15)
  • .github/workflows/e2e.yml
  • .github/workflows/nightly.yml
  • CLAUDE.md
  • apps/postgres/package.json
  • apps/postgres/prisma/scripts/001_insert_missing_list_types.sql
  • apps/postgres/prisma/scripts/002_update_list_type_provenances.sql
  • apps/postgres/prisma/scripts/003_upsert_sub_jurisdictions_and_list_type_links.sql
  • apps/postgres/prisma/scripts/004_soft_delete_crime_daily_list.sql
  • apps/postgres/prisma/seed-deploy.ts
  • apps/postgres/prisma/seed.sql
  • apps/postgres/prisma/seed.test.ts
  • apps/postgres/prisma/seed.ts
  • apps/postgres/start.sh
  • libs/location/src/seed-list-types.test.ts
  • libs/location/src/seed-list-types.ts
💤 Files with no reviewable changes (7)
  • apps/postgres/prisma/scripts/004_soft_delete_crime_daily_list.sql
  • apps/postgres/prisma/scripts/002_update_list_type_provenances.sql
  • apps/postgres/prisma/seed.sql
  • apps/postgres/prisma/scripts/003_upsert_sub_jurisdictions_and_list_type_links.sql
  • .github/workflows/e2e.yml
  • .github/workflows/nightly.yml
  • apps/postgres/prisma/scripts/001_insert_missing_list_types.sql

"migrate:reset": "yarn build && npx prisma migrate reset",
"studio": "yarn build && npx prisma studio",
"seed": "psql $DATABASE_URL -f prisma/seed.sql",
"test": "echo 'No tests for migration runner'",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how tests are invoked across the workspace (root scripts, turbo/nx config, CI workflows)
cat package.json | jq '.scripts'
rg -n "vitest" -g 'package.json' -g 'turbo.json' -g 'nx.json'
fd -H 'e2e.yml|nightly.yml' .github/workflows --exec cat {}

Repository: hmcts/cath-service

Length of output: 2156


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## apps/postgres/package.json\n'
cat apps/postgres/package.json | jq '{name, scripts}'

printf '\n## turbo.json\n'
cat turbo.json

printf '\n## workspace package manifests mentioning postgres\n'
rg -n '"name"|test' apps/postgres -g 'package.json' -g 'turbo.json' -g 'nx.json' -g 'vitest*.{ts,js,mjs,cjs}' -g 'package.json'

printf '\n## test files under apps/postgres\n'
fd -t f 'seed.test.ts|*.test.ts|*.spec.ts' apps/postgres

Repository: hmcts/cath-service

Length of output: 2332


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## apps/postgres/package.json\n'
cat apps/postgres/package.json | jq '{name, scripts}'

printf '\n## turbo.json\n'
cat turbo.json

printf '\n## test files under apps/postgres\n'
fd -t f 'seed.test.ts|*.test.ts|*.spec.ts' apps/postgres

printf '\n## package.json references to apps/postgres\n'
rg -n '"apps/postgres|`@hmcts/postgres`|`@hmcts/postgres-prisma`|seed.test' -g 'package.json' -g 'turbo.json' -g 'nx.json' -g '.github/workflows/**' .

Repository: hmcts/cath-service

Length of output: 2018


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## apps/postgres/vitest.config.ts\n'
cat -n apps/postgres/vitest.config.ts

printf '\n## workspace references to a direct vitest invocation for apps/postgres\n'
rg -n "vitest run|vitest.config|apps/postgres|`@hmcts/postgres`" .github/workflows apps package.json turbo.json nx.json -g '!**/node_modules/**'

printf '\n## root package.json test-related scripts\n'
cat package.json | jq '.scripts'

Repository: hmcts/cath-service

Length of output: 14381


Run Vitest from apps/postgres
turbo test still dispatches this package’s test script, so seed.test.ts is skipped while the script just echoes a message. Switch it to vitest run (or add a package test task that does) so the new assertions run in CI.

Source: Coding guidelines

Comment on lines +83 to +96

// Reconcile removals: soft-delete any active list type no longer present in listTypeData
// (e.g. CRIME_DAILY_LIST). Test list types live outside listTypeData and must be preserved.
const activeNames = listTypeData.map((lt) => lt.name);
const { count } = await (prisma as any).listType.updateMany({
where: {
deletedAt: null,
name: { notIn: activeNames },
NOT: [{ name: { startsWith: "TEST_" } }, { name: { startsWith: "E2E_" } }]
},
data: { deletedAt: new Date() }
});
console.log(`Soft-deleted ${count} list types no longer in listTypeData`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Missing guard for empty listTypeData — fix at the source, add a regression test.

Root cause: the reconciliation updateMany uses notIn: activeNames, and an empty activeNames array matches every row, so an empty/broken listTypeData would soft-delete all non-TEST_/E2E_ list types in one deploy.

  • libs/location/src/seed-list-types.ts#L83-L96: add an explicit if (listTypeData.length === 0) throw ... guard before computing activeNames/running the reconciliation updateMany.
  • libs/location/src/seed-list-types.test.ts#L95-L111: add a test asserting seedListTypes() throws (and updateMany is never called) when listTypeData is empty, once the guard above lands.
📍 Affects 2 files
  • libs/location/src/seed-list-types.ts#L83-L96 (this comment)
  • libs/location/src/seed-list-types.test.ts#L95-L111

@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment Successful 🚀

Your preview environment is ready:

The environment will be automatically cleaned up when this PR is closed.

list-type-data.test.ts only asserted static catalogue content (names,
friendly names, sensitivity) already declared in list-type-data.ts, so it
tested the fixture against itself. No standing instruction requires it and
nothing imports it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment Successful 🚀

Your preview environment is ready:

The environment will be automatically cleaned up when this PR is closed.

@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment Successful 🚀

Your preview environment is ready:

The environment will be automatically cleaned up when this PR is closed.

# Conflicts:
#	apps/postgres/prisma/scripts/001_insert_missing_list_types.sql
#	apps/postgres/prisma/scripts/003_upsert_sub_jurisdictions_and_list_type_links.sql
@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment Successful 🚀

Your preview environment is ready:

The environment will be automatically cleaned up when this PR is closed.

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

Preview Deployment Successful 🚀

Your preview environment is ready:

The environment will be automatically cleaned up when this PR is closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement Employment Tribunal Lists style guide

2 participants