Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-- AlterTable
ALTER TABLE "list_types" ADD COLUMN "location_type" VARCHAR(50);

-- CreateTable
CREATE TABLE "location_reference" (
"location_reference_id" TEXT NOT NULL,
"location_id" INTEGER NOT NULL,
"provenance" VARCHAR(50) NOT NULL,
"provenance_location_id" VARCHAR(255) NOT NULL,
"provenance_location_type" VARCHAR(50) NOT NULL,

CONSTRAINT "location_reference_pkey" PRIMARY KEY ("location_reference_id")
);

-- CreateIndex
CREATE UNIQUE INDEX "location_reference_provenance_provenance_location_id_key" ON "location_reference"("provenance", "provenance_location_id");

-- AddForeignKey
ALTER TABLE "location_reference" ADD CONSTRAINT "location_reference_location_id_fkey" FOREIGN KEY ("location_id") REFERENCES "location"("location_id") ON DELETE CASCADE ON UPDATE CASCADE;
338 changes: 338 additions & 0 deletions docs/tickets/582/plan.md

Large diffs are not rendered by default.

61 changes: 61 additions & 0 deletions docs/tickets/582/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Implementation Tasks

## Database Schema

- [x] Add `LocationReference` model to `libs/location/prisma/schema.prisma` with fields: `location_reference_id`, `location_id`, `provenance`, `provenance_location_id`, `provenance_location_type`; add `@@unique([provenance, provenanceLocationId])` constraint and `onDelete: Cascade` back-relation on `Location`
- [x] Add `locationType` field (nullable `String`) to `ListType` model in `libs/location/prisma/schema.prisma`
- [x] Create migration for `location_reference` table (`yarn db:migrate:dev`)
- [x] Create migration for `location_type` column on `list_types` table (`yarn db:migrate:dev`)
- [x] Run `yarn db:generate` to regenerate the Prisma client

## Location Reference Queries

- [x] Create `libs/location/src/location-reference/model.ts` with `LOCATION_REFERENCE_PROVENANCES` and `LOCATION_REFERENCE_TYPES` string literal union constants
- [x] Create `libs/location/src/location-reference/queries.ts` with `getLocationByProvenanceLocationId(provenance, provenanceLocationId, locationType?)` that queries `location_reference` joined to `location`, excluding soft-deleted locations
- [x] Export `getLocationByProvenanceLocationId` from `libs/location/src/index.ts`

## CSV Upload - Model & Parser

- [x] Add `PROVENANCE`, `PROVENANCE_LOCATION_ID`, `PROVENANCE_LOCATION_TYPE` fields to `CsvRow` interface in `libs/system-admin-pages/src/reference-data-upload/model.ts`
- [x] Add `provenance`, `provenanceLocationId`, `provenanceLocationType` fields to `ParsedLocationData` interface in the same file
- [x] Add the three new column names to `REQUIRED_HEADERS` in `libs/system-admin-pages/src/reference-data-upload/parsers/csv-parser.ts`
- [x] Parse and map the three new fields in the row-level loop in `csv-parser.ts`
- [x] Update `csv-parser.test.ts` to cover new fields present and new fields missing

## CSV Upload - Validation

- [x] Fix in-file location name duplicate check in `libs/system-admin-pages/src/reference-data-upload/validation/validation.ts` to key on `(locationName, locationId)` so identical names for the same ID are allowed
- [x] Fix in-file Welsh location name duplicate check with the same `(welshLocationName, locationId)` keying
- [x] Add validation that `provenance` is non-empty and is one of the allowed values from `LOCATION_REFERENCE_PROVENANCES`
- [x] Add validation that `provenanceLocationId` is non-empty
- [x] Add validation that `provenanceLocationType` is non-empty and is one of the allowed values from `LOCATION_REFERENCE_TYPES`
- [x] Add validation that `(provenance, provenanceLocationId)` is unique within the uploaded CSV
- [x] Update `validation.test.ts` to cover all new rules and the relaxed name-uniqueness rule

## CSV Upload - Repository

- [x] Update `upsertLocations` in `libs/system-admin-pages/src/reference-data-upload/repository/upload-repository.ts` to upsert a `location_reference` row for each CSV row using the `(provenance, provenanceLocationId)` unique key
- [x] Update `upload-repository.test.ts` to assert `locationReference` upsert is called with correct data

## Provenance Enum Update

- [x] Add `CP_CATH = "CP_CATH"` and `PDDA = "PDDA"` to the `Provenance` enum in `libs/publication/src/provenance.ts`
- [x] Add corresponding entries to `PROVENANCE_LABELS` in the same file

## Blob Ingestion - Model & Validation

- [x] Add `resolvedLocationId?: string` to `BlobValidationResult` interface in `libs/api/src/blob-ingestion/repository/model.ts`
- [x] Add `CP_CATH` and `PDDA` to `ALLOWED_PROVENANCES` in `libs/api/src/blob-ingestion/validation.ts`
- [x] Define `EXTERNAL_PROVENANCES` constant (`SNL`, `COMMON_PLATFORM`, `CP_CATH`, `PDDA`) in `validation.ts`
- [x] Update location validation in `validateBlobRequest` to branch on external vs internal provenance: external provenances call `getLocationByProvenanceLocationId(provenance, court_id, listTypeLocationType)`, internal provenances call `getLocationById(parseInt(court_id))`
- [x] Populate `resolvedLocationId` in the returned `BlobValidationResult` with the looked-up internal location ID
- [x] Update `validation.test.ts` to cover external provenance resolution and the new allowed provenance values

## Blob Ingestion - Service

- [x] Update `processBlobIngestion` in `libs/api/src/blob-ingestion/repository/service.ts` to use `validation.resolvedLocationId ?? request.court_id` as the `locationId` argument for `createArtefact` and `processPublication`
- [x] Update `service.test.ts` to assert the resolved location ID is used when provenance is external

## Tests

- [x] Verify all unit tests pass: `yarn test`
51 changes: 51 additions & 0 deletions docs/tickets/582/ticket.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# Issue #582: System admin reference data upload - Backend logic update

## Title
System admin reference data upload - Backend logic update

## Body

*Frontend update is not covered in this ticket.*

Currently the reference data upload does not have the concept of provenance. To support multiple provenances for a location:

- The following fields need to be added to the input CSV file format, the relevant model classes and the database:
- PROVENANCE
- PROVENANCE LOCATION ID
- PROVENANCE LOCATION TYPE
Comment on lines +13 to +15

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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use the exact CSV header names consistently across ticket docs.

Lines 13–15 use spaced names (PROVENANCE LOCATION ID, PROVENANCE LOCATION TYPE), while the plan/tasks and parser changes use underscore headers (PROVENANCE_LOCATION_ID, PROVENANCE_LOCATION_TYPE). Please standardise these to avoid ambiguity in implementation and test data.

🧰 Tools
🪛 LanguageTool

[duplication] ~13-~13: Possible typo: you repeated a word.
Context: ...ant model classes and the database: - PROVENANCE - PROVENANCE LOCATION ID - PROVENANCE LOCATION TYP...

(ENGLISH_WORD_REPEAT_RULE)

- The above fields should be mandatory and non-empty.
- A new `location_reference` table with the following fields added to store the location provenance:
- location_reference_id
- location_id
- provenance
- provenance_location_id
- provenance_location_type
- The possible values for provenance should be one of the enum values below:
- SNL
- COMMON_PLATFORM
- CP_CATH
- PDDA
- The possible values for provenance_location_type should be one of the enum values below:
- VENUE
- REGION
- OWNING_HEARING_LOCATION
- NATIONAL

### Reference data upload validation update:
- When the CSV file has multiple records with same location name or welsh location name but different location IDs, we should show the error on frontend.
- When the CSV file has records with the same location name or Welsh location name but different location IDs as existing records on the database, we should also error on frontend.
- Location name and Welsh location name on the location table should have unique constraint.
- The CSV files are allowed to have same location name or Welsh location name for records with the same location ID but different provenances.

### Changes required in publication upload processing to support the use of provenance location ID:
- Frontend manual upload should always use internal location ID to represent the court or tribunal the publication is uploaded to.
- Publication upload using the API currently only uses internal location ID for the `court_id` header. With the addition of provenance in reference data upload, external systems (SNL, COMMON_PLATFORM, CP_CATH, PDDA) should use their owned provenance location ID in the header when uploading publication.
- A new field called `location_type` needs to be added to the `list_type` table with one of the possible values below:
- VENUE
- REGION
- OWNING_HEARING_LOCATION
- NATIONAL
- When a publication is uploaded by external systems using the API, we should query the `location` table by the provenance location ID, provenance and the list type provenance location type to convert the provenance location ID to internal location ID before storing the publication in the database.

## Assignees
- KianKwa
22 changes: 11 additions & 11 deletions e2e-tests/fixtures/test-reference-data.csv
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
LOCATION_ID,LOCATION_NAME,WELSH_LOCATION_NAME,EMAIL,CONTACT_NO,SUB_JURISDICTION_NAME,REGION_NAME
9001,Test Court Alpha,Llys Prawf Alffa,test.alpha@test.hmcts.net,01234567890,Civil Court;Crown Court,South East
9002,Test Court Beta,Llys Prawf Beta,test.beta@test.hmcts.net,01234567891,Civil Court;Family Court,Midlands
9003,Test Court Gamma,Llys Prawf Gama,test.gamma@test.hmcts.net,01234567892,Civil Court,North
9004,Test Court Delta,Llys Prawf Delta,test.delta@test.hmcts.net,01234567893,Civil Court;Crown Court;Court of Appeal,London
9005,Test Court Epsilon,Llys Prawf Epsilon,test.epsilon@test.hmcts.net,01234567894,Civil Court;Family Court,Wales
9006,Test Court Zeta,Llys Prawf Zeta,test.zeta@test.hmcts.net,01234567895,Civil Court;Crown Court,North
9007,Test Court Eta,Llys Prawf Eta,test.eta@test.hmcts.net,01234567896,Civil Court;Family Court,South East
9008,Test Court Theta,Llys Prawf Theta,test.theta@test.hmcts.net,01234567897,Civil Court;Family Court,North
9,Test SJP Court,Llys Prawf SJP,sjp@test.hmcts.net,01234567898,Magistrates Court,London;Midlands;South East;North;Wales
9010,Test Court Iota,Llys Prawf Iota,test.iota@test.hmcts.net,01234567899,Civil Court;Crown Court,North
LOCATION_ID,LOCATION_NAME,WELSH_LOCATION_NAME,EMAIL,CONTACT_NO,SUB_JURISDICTION_NAME,REGION_NAME,PROVENANCE,PROVENANCE_LOCATION_ID,PROVENANCE_LOCATION_TYPE
9001,Test Court Alpha,Llys Prawf Alffa,test.alpha@test.hmcts.net,01234567890,Civil Court;Crown Court,South East,SNL,9001,VENUE
9002,Test Court Beta,Llys Prawf Beta,test.beta@test.hmcts.net,01234567891,Civil Court;Family Court,Midlands,SNL,9002,VENUE
9003,Test Court Gamma,Llys Prawf Gama,test.gamma@test.hmcts.net,01234567892,Civil Court,North,SNL,9003,VENUE
9004,Test Court Delta,Llys Prawf Delta,test.delta@test.hmcts.net,01234567893,Civil Court;Crown Court;Court of Appeal,London,SNL,9004,VENUE
9005,Test Court Epsilon,Llys Prawf Epsilon,test.epsilon@test.hmcts.net,01234567894,Civil Court;Family Court,Wales,SNL,9005,VENUE
9006,Test Court Zeta,Llys Prawf Zeta,test.zeta@test.hmcts.net,01234567895,Civil Court;Crown Court,North,SNL,9006,VENUE
9007,Test Court Eta,Llys Prawf Eta,test.eta@test.hmcts.net,01234567896,Civil Court;Family Court,South East,SNL,9007,VENUE
9008,Test Court Theta,Llys Prawf Theta,test.theta@test.hmcts.net,01234567897,Civil Court;Family Court,North,SNL,9008,VENUE
9,Test SJP Court,Llys Prawf SJP,sjp@test.hmcts.net,01234567898,Magistrates Court,London;Midlands;South East;North;Wales,SNL,9,VENUE
9010,Test Court Iota,Llys Prawf Iota,test.iota@test.hmcts.net,01234567899,Civil Court;Crown Court,North,SNL,9010,VENUE
2 changes: 1 addition & 1 deletion e2e-tests/tests/api/blob-ingestion-notifications.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
function createValidPayload(locationId: number, locationName: string) {
return {
court_id: locationId.toString(),
provenance: "XHIBIT",
provenance: "PDDA",
content_date: "2024-12-01",
list_type: "CIVIL_AND_FAMILY_DAILY_CAUSE_LIST",
sensitivity: "PUBLIC",
Expand Down Expand Up @@ -101,7 +101,7 @@
const notifications = await waitForNotifications(result.artefact_id, 15, 1000, true);

// Verify notification was created
expect(notifications.length).toBeGreaterThan(0);

Check failure on line 104 in e2e-tests/tests/api/blob-ingestion-notifications.spec.ts

View workflow job for this annotation

GitHub Actions / E2E Tests

[chromium] › tests/api/blob-ingestion-notifications.spec.ts:82:3 › Blob Ingestion - Notification E2E Tests › should verify GOV.UK Notify email content

1) [chromium] › tests/api/blob-ingestion-notifications.spec.ts:82:3 › Blob Ingestion - Notification E2E Tests › should verify GOV.UK Notify email content Retry #2 ─────────────────────────────────────────────────────────────────────────────────────── Error: expect(received).toBeGreaterThan(expected) Expected: > 0 Received: 0 102 | 103 | // Verify notification was created > 104 | expect(notifications.length).toBeGreaterThan(0); | ^ 105 | 106 | // Find the notification that was successfully sent (has govNotifyId) 107 | const sentNotification = notifications.find((n) => n.govNotifyId !== null); at /home/runner/work/cath-service/cath-service/e2e-tests/tests/api/blob-ingestion-notifications.spec.ts:104:34

Check failure on line 104 in e2e-tests/tests/api/blob-ingestion-notifications.spec.ts

View workflow job for this annotation

GitHub Actions / E2E Tests

[chromium] › tests/api/blob-ingestion-notifications.spec.ts:82:3 › Blob Ingestion - Notification E2E Tests › should verify GOV.UK Notify email content

1) [chromium] › tests/api/blob-ingestion-notifications.spec.ts:82:3 › Blob Ingestion - Notification E2E Tests › should verify GOV.UK Notify email content Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── Error: expect(received).toBeGreaterThan(expected) Expected: > 0 Received: 0 102 | 103 | // Verify notification was created > 104 | expect(notifications.length).toBeGreaterThan(0); | ^ 105 | 106 | // Find the notification that was successfully sent (has govNotifyId) 107 | const sentNotification = notifications.find((n) => n.govNotifyId !== null); at /home/runner/work/cath-service/cath-service/e2e-tests/tests/api/blob-ingestion-notifications.spec.ts:104:34

Check failure on line 104 in e2e-tests/tests/api/blob-ingestion-notifications.spec.ts

View workflow job for this annotation

GitHub Actions / E2E Tests

[chromium] › tests/api/blob-ingestion-notifications.spec.ts:82:3 › Blob Ingestion - Notification E2E Tests › should verify GOV.UK Notify email content

1) [chromium] › tests/api/blob-ingestion-notifications.spec.ts:82:3 › Blob Ingestion - Notification E2E Tests › should verify GOV.UK Notify email content Error: expect(received).toBeGreaterThan(expected) Expected: > 0 Received: 0 102 | 103 | // Verify notification was created > 104 | expect(notifications.length).toBeGreaterThan(0); | ^ 105 | 106 | // Find the notification that was successfully sent (has govNotifyId) 107 | const sentNotification = notifications.find((n) => n.govNotifyId !== null); at /home/runner/work/cath-service/cath-service/e2e-tests/tests/api/blob-ingestion-notifications.spec.ts:104:34

// Find the notification that was successfully sent (has govNotifyId)
const sentNotification = notifications.find((n) => n.govNotifyId !== null);
Expand Down
1 change: 1 addition & 0 deletions libs/api/src/blob-ingestion/repository/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export interface BlobValidationResult {
errors: ValidationError[];
locationExists: boolean;
listTypeId?: number;
resolvedLocationId?: string;
}

export interface IngestionLog {
Expand Down
10 changes: 5 additions & 5 deletions libs/api/src/blob-ingestion/repository/queries.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe("queries", async () => {
const log: IngestionLog = {
id: "test-id",
timestamp: new Date("2025-01-25T10:00:00Z"),
sourceSystem: "XHIBIT",
sourceSystem: "PDDA",
courtId: "123",
status: "SUCCESS",
artefactId: "artefact-123"
Expand Down Expand Up @@ -59,7 +59,7 @@ describe("queries", async () => {
const log: IngestionLog = {
id: "test-id",
timestamp: new Date("2025-01-25T10:00:00Z"),
sourceSystem: "XHIBIT",
sourceSystem: "PDDA",
courtId: "123",
status: "VALIDATION_ERROR",
errorMessage: "court_id is required"
Expand Down Expand Up @@ -100,7 +100,7 @@ describe("queries", async () => {
{
id: "log-1",
timestamp: new Date("2025-01-15T10:00:00Z"),
sourceSystem: "XHIBIT",
sourceSystem: "PDDA",
courtId: "123",
status: "SUCCESS",
errorMessage: null,
Expand Down Expand Up @@ -145,7 +145,7 @@ describe("queries", async () => {
{
id: "log-1",
timestamp: new Date("2025-01-25T10:00:00Z"),
sourceSystem: "XHIBIT",
sourceSystem: "PDDA",
courtId: "123",
status: "VALIDATION_ERROR",
errorMessage: "Invalid JSON",
Expand Down Expand Up @@ -178,7 +178,7 @@ describe("queries", async () => {
{
id: "log-1",
timestamp: new Date("2025-01-25T10:00:00Z"),
sourceSystem: "XHIBIT",
sourceSystem: "PDDA",
courtId: "123",
status: "SYSTEM_ERROR",
errorMessage: "Database connection failed",
Expand Down
63 changes: 56 additions & 7 deletions libs/api/src/blob-ingestion/repository/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@ vi.mock("@hmcts/publication", () => ({
processPublication: vi.fn(),
Provenance: {
MANUAL_UPLOAD: "MANUAL_UPLOAD",
XHIBIT: "XHIBIT",
LIBRA: "LIBRA",
SJP: "SJP",
SNL: "SNL",
COMMON_PLATFORM: "COMMON_PLATFORM"
COMMON_PLATFORM: "COMMON_PLATFORM",
CP_CATH: "CP_CATH",
PDDA: "PDDA"
}
}));

Expand Down Expand Up @@ -43,7 +42,7 @@ describe("processBlobIngestion", async () => {

const validRequest: BlobIngestionRequest = {
court_id: "123",
provenance: "XHIBIT",
provenance: "MANUAL_UPLOAD",
content_date: "2025-01-25",
list_type: "CIVIL_AND_FAMILY_DAILY_CAUSE_LIST",
sensitivity: "PUBLIC",
Expand Down Expand Up @@ -74,7 +73,7 @@ describe("processBlobIngestion", async () => {
expect(createIngestionLog).toHaveBeenCalledWith(
expect.objectContaining({
status: "SUCCESS",
sourceSystem: "XHIBIT",
sourceSystem: "MANUAL_UPLOAD",
courtId: "123"
})
);
Expand Down Expand Up @@ -310,11 +309,61 @@ describe("processBlobIngestion", async () => {
contentDate: expect.any(Date),
locale: "en",
jsonData: expect.anything(),
provenance: "XHIBIT",
provenance: "MANUAL_UPLOAD",
logPrefix: "[blob-ingestion]"
});
});

it("should use resolvedLocationId when provenance is external", async () => {
vi.mocked(validateBlobRequest).mockResolvedValue({
isValid: true,
errors: [],
locationExists: true,
listTypeId: 8,
resolvedLocationId: "456"
});

vi.mocked(createArtefact).mockResolvedValue("test-artefact-id");

const externalRequest = { ...validRequest, provenance: "SNL", court_id: "snl-ext-id" };
await processBlobIngestion(externalRequest, 1000);

expect(createArtefact).toHaveBeenCalledWith(
expect.objectContaining({
locationId: "456"
})
);

// Wait for async processing
await new Promise((resolve) => setTimeout(resolve, 50));

expect(processPublication).toHaveBeenCalledWith(
expect.objectContaining({
locationId: "456"
})
);
});

it("should fall back to court_id when resolvedLocationId is undefined", async () => {
vi.mocked(validateBlobRequest).mockResolvedValue({
isValid: true,
errors: [],
locationExists: false,
listTypeId: 8,
resolvedLocationId: undefined
});

vi.mocked(createArtefact).mockResolvedValue("test-artefact-id");

await processBlobIngestion(validRequest, 1000);

expect(createArtefact).toHaveBeenCalledWith(
expect.objectContaining({
locationId: "123"
})
);
});

it("should not call processPublication when noMatch is true", async () => {
vi.mocked(validateBlobRequest).mockResolvedValue({
isValid: true,
Expand Down
11 changes: 7 additions & 4 deletions libs/api/src/blob-ingestion/repository/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ import type { BlobIngestionRequest, BlobIngestionResponse } from "./model.js";
import { createIngestionLog } from "./queries.js";

const PROVENANCE_MAP: Record<string, string> = {
XHIBIT: Provenance.XHIBIT,
MANUAL_UPLOAD: Provenance.MANUAL_UPLOAD,
SNL: Provenance.SNL,
COMMON_PLATFORM: Provenance.COMMON_PLATFORM
COMMON_PLATFORM: Provenance.COMMON_PLATFORM,
CP_CATH: Provenance.CP_CATH,
PDDA: Provenance.PDDA
};

export async function processBlobIngestion(request: BlobIngestionRequest, rawBodySize: number): Promise<BlobIngestionResponse> {
Expand Down Expand Up @@ -61,11 +62,13 @@ export async function processBlobIngestion(request: BlobIngestionRequest, rawBod
};
}

const locationId = validation.resolvedLocationId ?? request.court_id;

try {
// Create artefact in database (returns actual artefact ID - either new or existing)
const artefactId = await createArtefact({
artefactId: newArtefactId,
locationId: request.court_id,
locationId,
listTypeId: validation.listTypeId,
contentDate: new Date(request.content_date),
sensitivity: request.sensitivity,
Expand Down Expand Up @@ -99,7 +102,7 @@ export async function processBlobIngestion(request: BlobIngestionRequest, rawBod
if (!noMatch) {
processPublication({
artefactId,
locationId: request.court_id,
locationId,
listTypeId: validation.listTypeId,
contentDate: new Date(request.content_date),
locale: request.language === "WELSH" ? "cy" : "en",
Expand Down
Loading
Loading