From d6e607bfc5eb204fdb4f8ba42603f487b7c0d893 Mon Sep 17 00:00:00 2001 From: Mike Houston Date: Thu, 11 Jun 2026 17:18:57 +0100 Subject: [PATCH 1/3] Extract config-store validation changes Cherry-pick the shared validation and validation-reporting work onto CCM-19694 without the supplier reports package. Includes config-store integrity validation, excel-parser mapping validation/reporting, and publisher error-detail propagation. --- .../ddb-publisher/src/__tests__/run.test.ts | 78 ++- packages/ddb-publisher/src/run.ts | 27 +- .../src/__tests__/run.test.ts | 74 ++- packages/eventbridge-publisher/src/run.ts | 27 +- packages/excel-parser/README.md | 12 + packages/excel-parser/package.json | 1 + .../__tests__/cli-parse.formatting.test.ts | 72 ++ .../mapping-validation-report.test.ts | 94 +++ .../mapping-validation.specifications.test.ts | 107 +++ .../src/__tests__/mapping-validation.test.ts | 189 ++++++ packages/excel-parser/src/cli-parse.ts | 210 ++++-- packages/excel-parser/src/index.ts | 9 + .../src/mapping-validation-report.ts | 47 ++ .../excel-parser/src/mapping-validation.ts | 103 +++ .../config-store-integrity-validator.test.ts | 614 ++++++++++++++++++ .../__tests__/config-store-validator.test.ts | 39 +- packages/file-store/src/index.ts | 1 + packages/file-store/src/types.ts | 1 + .../config-store-integrity-validator.ts | 359 ++++++++++ .../src/validation/config-store-validator.ts | 21 +- .../src/validation/entity-schemas.ts | 17 + 21 files changed, 1999 insertions(+), 103 deletions(-) create mode 100644 packages/excel-parser/src/__tests__/cli-parse.formatting.test.ts create mode 100644 packages/excel-parser/src/__tests__/mapping-validation-report.test.ts create mode 100644 packages/excel-parser/src/__tests__/mapping-validation.specifications.test.ts create mode 100644 packages/excel-parser/src/__tests__/mapping-validation.test.ts create mode 100644 packages/excel-parser/src/mapping-validation-report.ts create mode 100644 packages/excel-parser/src/mapping-validation.ts create mode 100644 packages/file-store/src/__tests__/config-store-integrity-validator.test.ts create mode 100644 packages/file-store/src/validation/config-store-integrity-validator.ts create mode 100644 packages/file-store/src/validation/entity-schemas.ts diff --git a/packages/ddb-publisher/src/__tests__/run.test.ts b/packages/ddb-publisher/src/__tests__/run.test.ts index ab642f3..d6e3992 100644 --- a/packages/ddb-publisher/src/__tests__/run.test.ts +++ b/packages/ddb-publisher/src/__tests__/run.test.ts @@ -9,6 +9,7 @@ jest.mock("@supplier-config/file-store", () => { return { loadConfigStore: jest.fn(), validateConfigStore: jest.fn(), + validateConfigStoreIntegrity: jest.fn(), }; }); @@ -43,6 +44,7 @@ const fileStore = jest.requireMock( ) as unknown as { loadConfigStore: jest.Mock; validateConfigStore: jest.Mock; + validateConfigStoreIntegrity: jest.Mock; }; const audit = jest.requireMock("../ddb/audit") as unknown as { @@ -58,13 +60,17 @@ const awsClient = jest.requireMock("@aws-sdk/client-dynamodb") as unknown as { }; describe("runPublisher", () => { + beforeEach(() => { + const validResult: ValidationResult = { ok: true, issues: [] }; + + fileStore.validateConfigStore.mockReturnValue(validResult); + fileStore.validateConfigStoreIntegrity.mockReturnValue(validResult); + }); + it("should stop after validation when dryRun=true", async () => { const store: LoadedConfigStore = { rootPath: "/tmp", records: [] }; fileStore.loadConfigStore.mockResolvedValue(store); - const validation: ValidationResult = { ok: true, issues: [] }; - fileStore.validateConfigStore.mockReturnValue(validation); - await runPublisher({ sourcePath: "/tmp", env: "draft", @@ -107,6 +113,72 @@ describe("runPublisher", () => { ).rejects.toThrow("Config store validation failed"); }); + it("should throw with a helpful message when integrity validation fails", async () => { + const store: LoadedConfigStore = { + rootPath: "/tmp", + records: [], + }; + fileStore.loadConfigStore.mockResolvedValue(store); + + fileStore.validateConfigStoreIntegrity.mockReturnValue({ + ok: false, + issues: [ + { + entity: "letter-variant", + sourceFilePath: "/tmp/letter-variant/variant-1.json", + message: "missing promoted supplier pack", + path: ["packSpecificationIds"], + }, + ], + }); + + await expect( + runPublisher({ + sourcePath: "/tmp", + env: "draft", + tableName: "tbl", + dryRun: true, + force: false, + }), + ).rejects.toThrow("Config store integrity validation failed"); + }); + + it("should include issue detail lines in integrity validation errors", async () => { + const store: LoadedConfigStore = { + rootPath: "/tmp", + records: [], + }; + fileStore.loadConfigStore.mockResolvedValue(store); + + fileStore.validateConfigStoreIntegrity.mockReturnValue({ + ok: false, + issues: [ + { + entity: "letter-variant", + sourceFilePath: "/tmp/letter-variant/variant-1.json", + message: "missing promoted supplier pack", + path: ["packSpecificationIds"], + details: [ + "packSpecification=pack-1 status=PROD => valid", + "supplierPack=supplier-pack-1 supplier=supplier-1 status=INT approval=APPROVED => invalid", + ], + }, + ], + }); + + await expect( + runPublisher({ + sourcePath: "/tmp", + env: "draft", + tableName: "tbl", + dryRun: true, + force: false, + }), + ).rejects.toThrow( + "supplierPack=supplier-pack-1 supplier=supplier-1 status=INT approval=APPROVED => invalid", + ); + }); + it("should block upload when audit reports blocking items and force=false", async () => { const store: LoadedConfigStore = { rootPath: "/tmp", diff --git a/packages/ddb-publisher/src/run.ts b/packages/ddb-publisher/src/run.ts index 5494fbe..aa5d174 100644 --- a/packages/ddb-publisher/src/run.ts +++ b/packages/ddb-publisher/src/run.ts @@ -4,6 +4,7 @@ import { DynamoDBDocumentClient } from "@aws-sdk/lib-dynamodb"; import { loadConfigStore, validateConfigStore, + validateConfigStoreIntegrity, } from "@supplier-config/file-store"; import type { ConfigRecord, @@ -19,9 +20,13 @@ function issueLabel(i: { sourceFilePath: string; path?: (string | number)[]; message: string; + details?: string[]; }): string { const pathPart = i.path?.length ? `:${i.path.join(".")}` : ""; - return `${i.entity} ${i.sourceFilePath}${pathPart} - ${i.message}`; + const formattedDetails = i.details?.map((detail) => ` ${detail}`).join("\n"); + const detailsPart = i.details?.length ? `\n${formattedDetails}` : ""; + + return `${i.entity} ${i.sourceFilePath}${pathPart} - ${i.message}${detailsPart}`; } function logStep(message: string): void { @@ -81,6 +86,26 @@ async function runPublisher(plan: LoadPlan): Promise { logStep("Validation passed."); + logStep("Running config-store integrity checks..."); + const integrityValidation = validateConfigStoreIntegrity(store); + + if (!integrityValidation.ok) { + logStep( + `Integrity validation failed with ${integrityValidation.issues.length} issue(s).`, + ); + + const summary = integrityValidation.issues + .slice(0, 20) + .map((i: ValidationIssue) => issueLabel(i)) + .join("\n"); + + throw new Error( + `Config store integrity validation failed with ${integrityValidation.issues.length} issue(s).\n${summary}`, + ); + } + + logStep("Integrity validation passed."); + if (plan.dryRun) { logStep("Dry-run enabled; skipping DynamoDB audit and publish."); return; diff --git a/packages/eventbridge-publisher/src/__tests__/run.test.ts b/packages/eventbridge-publisher/src/__tests__/run.test.ts index 4f429ff..6a9c141 100644 --- a/packages/eventbridge-publisher/src/__tests__/run.test.ts +++ b/packages/eventbridge-publisher/src/__tests__/run.test.ts @@ -9,6 +9,7 @@ jest.mock("@supplier-config/file-store", () => { return { loadConfigStore: jest.fn(), validateConfigStore: jest.fn(), + validateConfigStoreIntegrity: jest.fn(), }; }); @@ -29,6 +30,7 @@ const fileStore = jest.requireMock( ) as unknown as { loadConfigStore: jest.Mock; validateConfigStore: jest.Mock; + validateConfigStoreIntegrity: jest.Mock; }; const publish = jest.requireMock("../eventbridge/publish") as unknown as { @@ -42,13 +44,17 @@ const eventBridgeClient = jest.requireMock( }; describe("runPublisher", () => { + beforeEach(() => { + const validResult: ValidationResult = { ok: true, issues: [] }; + + fileStore.validateConfigStore.mockReturnValue(validResult); + fileStore.validateConfigStoreIntegrity.mockReturnValue(validResult); + }); + it("should stop after validation when dryRun=true", async () => { const store: LoadedConfigStore = { rootPath: "/tmp", records: [] }; fileStore.loadConfigStore.mockResolvedValue(store); - const validation: ValidationResult = { ok: true, issues: [] }; - fileStore.validateConfigStore.mockReturnValue(validation); - await runPublisher({ sourcePath: "/tmp", eventBusArn: "eventBus", @@ -86,6 +92,68 @@ describe("runPublisher", () => { ).rejects.toThrow("Config store validation failed"); }); + it("should throw with a helpful message when integrity validation fails", async () => { + const store: LoadedConfigStore = { + rootPath: "/tmp", + records: [], + }; + fileStore.loadConfigStore.mockResolvedValue(store); + + fileStore.validateConfigStoreIntegrity.mockReturnValue({ + ok: false, + issues: [ + { + entity: "letter-variant", + sourceFilePath: "/tmp/letter-variant/variant-1.json", + message: "missing promoted supplier pack", + path: ["packSpecificationIds"], + }, + ], + }); + + await expect( + runPublisher({ + sourcePath: "/tmp", + eventBusArn: "event-bus", + dryRun: true, + }), + ).rejects.toThrow("Config store integrity validation failed"); + }); + + it("should include issue detail lines in integrity validation errors", async () => { + const store: LoadedConfigStore = { + rootPath: "/tmp", + records: [], + }; + fileStore.loadConfigStore.mockResolvedValue(store); + + fileStore.validateConfigStoreIntegrity.mockReturnValue({ + ok: false, + issues: [ + { + entity: "letter-variant", + sourceFilePath: "/tmp/letter-variant/variant-1.json", + message: "missing promoted supplier pack", + path: ["packSpecificationIds"], + details: [ + "packSpecification=pack-1 status=PROD => valid", + "supplierPack=supplier-pack-1 supplier=supplier-1 status=INT approval=APPROVED => invalid", + ], + }, + ], + }); + + await expect( + runPublisher({ + sourcePath: "/tmp", + eventBusArn: "event-bus", + dryRun: true, + }), + ).rejects.toThrow( + "supplierPack=supplier-pack-1 supplier=supplier-1 status=INT approval=APPROVED => invalid", + ); + }); + it("should publish events", async () => { const store: LoadedConfigStore = { rootPath: "/tmp", diff --git a/packages/eventbridge-publisher/src/run.ts b/packages/eventbridge-publisher/src/run.ts index 567bd02..78e9452 100644 --- a/packages/eventbridge-publisher/src/run.ts +++ b/packages/eventbridge-publisher/src/run.ts @@ -1,6 +1,7 @@ import { loadConfigStore, validateConfigStore, + validateConfigStoreIntegrity, } from "@supplier-config/file-store"; import type { ConfigRecord, @@ -19,9 +20,13 @@ function issueLabel(i: { sourceFilePath: string; path?: (string | number)[]; message: string; + details?: string[]; }): string { const pathPart = i.path?.length ? `:${i.path.join(".")}` : ""; - return `${i.entity} ${i.sourceFilePath}${pathPart} - ${i.message}`; + const formattedDetails = i.details?.map((detail) => ` ${detail}`).join("\n"); + const detailsPart = i.details?.length ? `\n${formattedDetails}` : ""; + + return `${i.entity} ${i.sourceFilePath}${pathPart} - ${i.message}${detailsPart}`; } function logStep(message: string): void { @@ -106,6 +111,26 @@ async function runPublisher(plan: LoadPlan): Promise { logStep("Validation passed."); + logStep("Running config-store integrity checks..."); + const integrityValidation = validateConfigStoreIntegrity(store); + + if (!integrityValidation.ok) { + logStep( + `Integrity validation failed with ${integrityValidation.issues.length} issue(s).`, + ); + + const summary = integrityValidation.issues + .slice(0, 20) + .map((i: ValidationIssue) => issueLabel(i)) + .join("\n"); + + throw new Error( + `Config store integrity validation failed with ${integrityValidation.issues.length} issue(s).\n${summary}`, + ); + } + + logStep("Integrity validation passed."); + if (plan.dryRun) { logStep("Dry-run enabled; skipping publish to EventBridge."); return; diff --git a/packages/excel-parser/README.md b/packages/excel-parser/README.md index 495d172..62c2d59 100644 --- a/packages/excel-parser/README.md +++ b/packages/excel-parser/README.md @@ -7,6 +7,7 @@ A package for parsing and generating Excel files for NHS Notify supplier configu - Parse Excel files containing supplier configuration data (PackSpecification, LetterVariant, VolumeGroup, Supplier, SupplierAllocation, SupplierPack) - Generate template Excel files with the correct sheet structure and headers - Validate parsed data against Zod schemas +- Warn about incomplete config-store mappings for promoted variants ## Usage @@ -32,6 +33,9 @@ npm run parse -- config.xlsx --pretty --output output.json # Parse and write a file-store-compatible directory of JSON files npm run parse -- config.xlsx --output-dir ./config-store +# Parse and warn about incomplete mappings for INT/PROD variants +npm run parse -- config.xlsx --check-mappings + # Pretty-print each JSON file in the generated config store npm run parse -- config.xlsx --output-dir ./config-store --pretty @@ -43,6 +47,7 @@ npm run parse -- --help - `-o, --output ` - Write output to a file instead of stdout - `-d, --output-dir ` - Write one JSON file per record into a directory compatible with `@supplier-config/file-store` +- `--check-mappings` - Warn when promoted variants are missing promoted pack, supplier-pack, volume-group, or supplier-allocation mappings - `-p, --pretty` - Pretty-print the JSON output - `-h, --help` - Show help message @@ -114,10 +119,12 @@ The template includes the following sheets with proper column headers: ```typescript import { parseExcelFile, + validateIncompleteMappings, writeParseResultToConfigStore, } from "@supplier-config/excel-parser"; const result = parseExcelFile("./specifications.xlsx"); +const mappingValidation = validateIncompleteMappings(result); console.log(result.packs); // Record console.log(result.variants); // Record @@ -125,10 +132,15 @@ console.log(result.volumeGroups); // Record console.log(result.suppliers); // Record console.log(result.allocations); // Record console.log(result.supplierPacks); // Record +console.log(mappingValidation); // { ok: boolean, warnings: [...] } await writeParseResultToConfigStore(result, "./config-store", { pretty: true, }); + +if (!mappingValidation.ok) { + console.warn(mappingValidation.warnings); +} ``` ### Generating a template Excel file diff --git a/packages/excel-parser/package.json b/packages/excel-parser/package.json index 3646a88..889ecc6 100644 --- a/packages/excel-parser/package.json +++ b/packages/excel-parser/package.json @@ -1,6 +1,7 @@ { "dependencies": { "@nhsdigital/nhs-notify-event-schemas-supplier-config": "*", + "@supplier-config/file-store": "*", "xlsx": "^0.18.5", "yargs": "^17.7.2", "zod": "^4.1.12" diff --git a/packages/excel-parser/src/__tests__/cli-parse.formatting.test.ts b/packages/excel-parser/src/__tests__/cli-parse.formatting.test.ts new file mode 100644 index 0000000..4f6f20e --- /dev/null +++ b/packages/excel-parser/src/__tests__/cli-parse.formatting.test.ts @@ -0,0 +1,72 @@ +import { validateIncompleteMappings } from "../mapping-validation"; +import { + makeAllocationRow, + makePackRow, + makeSupplierPackRow, + makeSupplierRow, + makeVariantRow, + makeVolumeGroupRow, + parseWorkbook, +} from "./helpers/parse-excel"; + +describe("grouped mapping warning formatting inputs", () => { + it("produces repeated variant warnings that share the same detail lines for grouping", () => { + const result = parseWorkbook({ + packs: [ + makePackRow({ + id: "pack-cli-grouped", + name: "Pack CLI Grouped", + billingId: "billing-pack-cli-grouped", + status: "PROD", + }), + ], + variants: [ + makeVariantRow({ + id: "variant-cli-grouped", + name: "Variant CLI Grouped", + packSpecificationIds: "pack-cli-grouped", + supplierId: "supplier-cli-grouped", + status: "PROD", + }), + ], + volumeGroups: [makeVolumeGroupRow()], + suppliers: [ + makeSupplierRow({ + id: "supplier-cli-grouped", + name: "Supplier CLI Grouped", + }), + ], + allocations: [makeAllocationRow({ supplier: "supplier-cli-grouped" })], + supplierPacks: [ + makeSupplierPackRow({ + id: "supplier-pack-cli-grouped", + packSpecificationId: "pack-cli-grouped", + supplierId: "supplier-cli-grouped", + approval: "SUBMITTED", + status: "PROD", + }), + ], + }); + + const validation = validateIncompleteMappings(result); + const variantWarnings = validation.warnings.filter( + (warning) => warning.id === "variant-cli-grouped", + ); + + expect(variantWarnings).toHaveLength(2); + expect(variantWarnings.map((warning) => warning.path)).toEqual([ + ["packSpecificationIds"], + ["supplierId"], + ]); + expect(variantWarnings.map((warning) => warning.details)).toEqual([ + [ + "packSpecification=pack-cli-grouped status=PROD => valid", + "supplierPack=supplier-pack-cli-grouped supplier=supplier-cli-grouped status=PROD approval=SUBMITTED => invalid", + ], + [ + "packSpecification=pack-cli-grouped status=PROD => valid", + "supplierPack=supplier-pack-cli-grouped supplier=supplier-cli-grouped status=PROD approval=SUBMITTED => invalid", + ], + ]); + }); +}); diff --git a/packages/excel-parser/src/__tests__/mapping-validation-report.test.ts b/packages/excel-parser/src/__tests__/mapping-validation-report.test.ts new file mode 100644 index 0000000..bb66d53 --- /dev/null +++ b/packages/excel-parser/src/__tests__/mapping-validation-report.test.ts @@ -0,0 +1,94 @@ +import { + formatGroupedMappingWarnings, + groupMappingWarningsByVariant, +} from "../mapping-validation-report"; + +describe("mapping validation report formatting", () => { + const warnings = [ + { + entity: "letter-variant" as const, + id: "variant-b", + message: "Variant B warning one", + path: ["packSpecificationIds"], + details: [ + "packSpecification=pack-b status=PROD => valid", + "supplierPack=supplier-pack-b supplier=supplier-b status=PROD approval=SUBMITTED => invalid", + ], + }, + { + entity: "letter-variant" as const, + id: "variant-a", + message: "Variant A warning one", + path: ["packSpecificationIds"], + details: [ + "packSpecification=pack-a status=INT => invalid", + "supplierPack=", + ], + }, + { + entity: "letter-variant" as const, + id: "variant-b", + message: "Variant B warning two", + path: ["supplierId"], + details: [ + "packSpecification=pack-b status=PROD => valid", + "supplierPack=supplier-pack-b supplier=supplier-b status=PROD approval=SUBMITTED => invalid", + ], + }, + ]; + + it("groups warnings by variant id in sorted order and deduplicates detail lines", () => { + expect(groupMappingWarningsByVariant(warnings)).toEqual([ + { + variantId: "variant-a", + warnings: [warnings[1]], + details: [ + "packSpecification=pack-a status=INT => invalid", + "supplierPack=", + ], + }, + { + variantId: "variant-b", + warnings: [warnings[0], warnings[2]], + details: [ + "packSpecification=pack-b status=PROD => valid", + "supplierPack=supplier-pack-b supplier=supplier-b status=PROD approval=SUBMITTED => invalid", + ], + }, + ]); + }); + + it("formats grouped warnings with a single mapped-combinations section per variant", () => { + expect(formatGroupedMappingWarnings(warnings)).toEqual([ + "- variant-a", + " • Variant A warning one", + " mapped combinations:", + " - packSpecification=pack-a status=INT => invalid", + " - supplierPack=", + "- variant-b", + " • Variant B warning one", + " • Variant B warning two", + " mapped combinations:", + " - packSpecification=pack-b status=PROD => valid", + " - supplierPack=supplier-pack-b supplier=supplier-b status=PROD approval=SUBMITTED => invalid", + ]); + }); + + it("returns no grouped warnings when the warning list is empty", () => { + expect(groupMappingWarningsByVariant([])).toEqual([]); + expect(formatGroupedMappingWarnings([])).toEqual([]); + }); + + it("omits the mapped-combinations section when a grouped warning has no details", () => { + expect( + formatGroupedMappingWarnings([ + { + entity: "letter-variant", + id: "variant-no-details", + message: "Variant without detail lines", + path: ["volumeGroupId"], + }, + ]), + ).toEqual(["- variant-no-details", " • Variant without detail lines"]); + }); +}); diff --git a/packages/excel-parser/src/__tests__/mapping-validation.specifications.test.ts b/packages/excel-parser/src/__tests__/mapping-validation.specifications.test.ts new file mode 100644 index 0000000..378e9ca --- /dev/null +++ b/packages/excel-parser/src/__tests__/mapping-validation.specifications.test.ts @@ -0,0 +1,107 @@ +import { + makeAllocationRow, + makePackRow, + makeSupplierPackRow, + makeSupplierRow, + makeVariantRow, + makeVolumeGroupRow, + parseWorkbook, +} from "./helpers/parse-excel"; +import { validateIncompleteMappings } from "../mapping-validation"; + +describe("validateIncompleteMappings detailed diagnostics", () => { + it("reports synthetic invalid variants with mapped pack and supplier-pack combinations", () => { + const result = parseWorkbook({ + packs: [ + makePackRow({ + id: "pack-alpha-test", + name: "Pack Alpha Test", + billingId: "billing-pack-alpha-test", + status: "INT", + }), + makePackRow({ + id: "pack-beta-test", + name: "Pack Beta Test", + billingId: "billing-pack-beta-test", + status: "PROD", + }), + ], + variants: [ + makeVariantRow({ + id: "variant-alpha-test", + name: "Variant Alpha Test", + packSpecificationIds: "pack-alpha-test", + supplierId: "supplier-alpha-test", + status: "PROD", + }), + makeVariantRow({ + id: "variant-beta-test", + name: "Variant Beta Test", + packSpecificationIds: "pack-beta-test", + supplierId: "supplier-beta-test", + status: "PROD", + }), + ], + volumeGroups: [makeVolumeGroupRow()], + suppliers: [ + makeSupplierRow({ + id: "supplier-alpha-test", + name: "Supplier Alpha Test", + }), + makeSupplierRow({ + id: "supplier-beta-test", + name: "Supplier Beta Test", + }), + ], + allocations: [makeAllocationRow({ supplier: "supplier-alpha-test" })], + supplierPacks: [ + makeSupplierPackRow({ + id: "supplier-pack-alpha-test", + packSpecificationId: "pack-alpha-test", + supplierId: "supplier-alpha-test", + status: "INT", + }), + makeSupplierPackRow({ + id: "supplier-pack-beta-test", + packSpecificationId: "pack-beta-test", + supplierId: "supplier-beta-test", + approval: "SUBMITTED", + status: "PROD", + }), + ], + }); + const validation = validateIncompleteMappings(result); + const warningIds = validation.warnings.map((warning) => warning.id); + + expect(validation.ok).toBe(false); + expect(warningIds).toEqual( + expect.arrayContaining(["variant-alpha-test", "variant-beta-test"]), + ); + expect(validation.warnings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: "variant-alpha-test", + path: ["packSpecificationIds"], + message: expect.stringContaining( + "none of its referenced pack specifications are promoted to PROD or higher", + ), + details: [ + "packSpecification=pack-alpha-test status=INT => invalid", + "supplierPack=supplier-pack-alpha-test supplier=supplier-alpha-test status=INT approval=APPROVED => invalid", + ], + }), + expect.objectContaining({ + id: "variant-beta-test", + path: ["packSpecificationIds"], + message: expect.stringContaining( + "none of its promoted pack specifications have a supplier pack in PROD and APPROVED state", + ), + details: [ + "packSpecification=pack-beta-test status=PROD => valid", + "supplierPack=supplier-pack-beta-test supplier=supplier-beta-test status=PROD approval=SUBMITTED => invalid", + ], + }), + ]), + ); + }); +}); diff --git a/packages/excel-parser/src/__tests__/mapping-validation.test.ts b/packages/excel-parser/src/__tests__/mapping-validation.test.ts new file mode 100644 index 0000000..8e3d7e9 --- /dev/null +++ b/packages/excel-parser/src/__tests__/mapping-validation.test.ts @@ -0,0 +1,189 @@ +import { validateIncompleteMappings } from "../mapping-validation"; +import { + makeAllocationRow, + makePackRow, + makeSupplierPackRow, + makeSupplierRow, + makeVariantRow, + makeVolumeGroupRow, + parseWorkbook, +} from "./helpers/parse-excel"; + +describe("validateIncompleteMappings", () => { + it("returns no warnings when a promoted variant has complete promoted mappings", () => { + const result = parseWorkbook({ + packs: [makePackRow()], + variants: [makeVariantRow({ supplierId: "supplier-1" })], + volumeGroups: [makeVolumeGroupRow()], + suppliers: [makeSupplierRow()], + allocations: [makeAllocationRow()], + supplierPacks: [makeSupplierPackRow()], + }); + + expect(validateIncompleteMappings(result)).toEqual({ + ok: true, + warnings: [], + }); + }); + + it("warns when no referenced pack specification is promoted enough", () => { + const result = parseWorkbook({ + packs: [makePackRow({ status: "INT" })], + variants: [makeVariantRow({ status: "PROD" })], + volumeGroups: [makeVolumeGroupRow()], + suppliers: [makeSupplierRow()], + allocations: [makeAllocationRow()], + supplierPacks: [makeSupplierPackRow({ status: "INT" })], + }); + + expect(validateIncompleteMappings(result)).toEqual({ + ok: false, + warnings: [ + expect.objectContaining({ + entity: "letter-variant", + id: "variant-1", + message: + "LetterVariant 'variant-1' is in PROD, but none of its referenced pack specifications are promoted to PROD or higher.", + path: ["packSpecificationIds"], + details: [ + "packSpecification=pack-1 status=INT => invalid", + "supplierPack=supplier-pack-1 supplier=supplier-1 status=INT approval=APPROVED => invalid", + ], + }), + ], + }); + }); + + it("warns when promoted packs do not have a supplier pack at the same promotion level", () => { + const result = parseWorkbook({ + packs: [makePackRow({ status: "PROD" })], + variants: [makeVariantRow({ status: "PROD" })], + volumeGroups: [makeVolumeGroupRow()], + suppliers: [makeSupplierRow()], + allocations: [makeAllocationRow()], + supplierPacks: [makeSupplierPackRow({ status: "INT" })], + }); + + expect(validateIncompleteMappings(result)).toEqual({ + ok: false, + warnings: [ + expect.objectContaining({ + entity: "letter-variant", + id: "variant-1", + message: + "LetterVariant 'variant-1' is in PROD, but none of its promoted pack specifications have a supplier pack in PROD and APPROVED state.", + path: ["packSpecificationIds"], + details: [ + "packSpecification=pack-1 status=PROD => valid", + "supplierPack=supplier-pack-1 supplier=supplier-1 status=INT approval=APPROVED => invalid", + ], + }), + ], + }); + }); + + it("warns when a supplier-scoped variant has no promoted supplier pack for that supplier", () => { + const result = parseWorkbook({ + packs: [makePackRow({ status: "INT" })], + variants: [makeVariantRow({ status: "INT", supplierId: "supplier-1" })], + volumeGroups: [makeVolumeGroupRow()], + suppliers: [ + makeSupplierRow({ id: "supplier-1" }), + makeSupplierRow({ id: "supplier-2", name: "Supplier 2" }), + ], + allocations: [makeAllocationRow({ supplier: "supplier-1" })], + supplierPacks: [ + makeSupplierPackRow({ + status: "INT", + supplierId: "supplier-2", + }), + ], + }); + + expect(validateIncompleteMappings(result)).toEqual({ + ok: false, + warnings: [ + expect.objectContaining({ + entity: "letter-variant", + id: "variant-1", + message: + "LetterVariant 'variant-1' is scoped to supplier 'supplier-1', but no promoted supplier pack exists for that supplier and variant pack specifications at INT or higher.", + path: ["supplierId"], + details: [ + "packSpecification=pack-1 status=INT => valid", + "supplierPack=supplier-pack-1 supplier=supplier-2 status=INT approval=APPROVED => valid", + ], + }), + ], + }); + }); + + it("warns when a promoted variant references a missing volume group", () => { + const result = parseWorkbook({ + packs: [makePackRow()], + variants: [makeVariantRow()], + suppliers: [makeSupplierRow()], + allocations: [makeAllocationRow()], + supplierPacks: [makeSupplierPackRow()], + }); + + expect(validateIncompleteMappings(result)).toEqual({ + ok: false, + warnings: [ + expect.objectContaining({ + entity: "letter-variant", + id: "variant-1", + message: + "LetterVariant 'variant-1' references volume group 'volume-group-1', but that volume group does not exist.", + path: ["volumeGroupId"], + }), + ], + }); + }); + + it("warns when a promoted variant volume group has no supplier allocation", () => { + const result = parseWorkbook({ + packs: [makePackRow()], + variants: [makeVariantRow()], + volumeGroups: [makeVolumeGroupRow()], + suppliers: [makeSupplierRow()], + supplierPacks: [makeSupplierPackRow()], + }); + + expect(validateIncompleteMappings(result)).toEqual({ + ok: false, + warnings: [ + expect.objectContaining({ + entity: "letter-variant", + id: "variant-1", + message: + "LetterVariant 'variant-1' references volume group 'volume-group-1', but no supplier allocation is associated with it.", + path: ["volumeGroupId"], + }), + ], + }); + }); + + it.each(["DRAFT", "DISABLED"])( + "ignores variants that are not promoted beyond draft when status is %s", + (status) => { + const result = parseWorkbook({ + packs: [makePackRow({ status: "DRAFT" })], + variants: [ + makeVariantRow({ + status, + supplierId: "supplier-1", + volumeGroupId: "missing-volume-group", + }), + ], + suppliers: [makeSupplierRow()], + supplierPacks: [makeSupplierPackRow({ status: "DRAFT" })], + }); + + expect(validateIncompleteMappings(result)).toEqual({ + ok: true, + warnings: [], + }); + }, + ); +}); diff --git a/packages/excel-parser/src/cli-parse.ts b/packages/excel-parser/src/cli-parse.ts index 862eaf3..a5439b5 100644 --- a/packages/excel-parser/src/cli-parse.ts +++ b/packages/excel-parser/src/cli-parse.ts @@ -1,32 +1,118 @@ #!/usr/bin/env node import fs from "node:fs"; import path from "node:path"; + import yargs from "yargs"; import { hideBin } from "yargs/helpers"; + import { writeParseResultToConfigStore } from "./config-store-output"; import { stringifyJsonWithSortedKeys } from "./json-output"; +import { validateIncompleteMappings } from "./mapping-validation"; +import { formatGroupedMappingWarnings } from "./mapping-validation-report"; import { parseExcelFile } from "./parse-excel"; interface Arguments { + checkMappings: boolean; input: string; output?: string; outputDir?: string; pretty: boolean; } -async function main() { +function resolveCliPath(filePath: string): string { + return path.isAbsolute(filePath) + ? filePath + : path.join(process.cwd(), filePath); +} + +function assertValidInputFile(inputFile: string): void { + // eslint-disable-next-line security/detect-non-literal-fs-filename + if (!fs.existsSync(inputFile)) { + throw new Error(`Input file not found: ${inputFile}`); + } + + if (!/\.xlsx?$/iu.test(inputFile)) { + throw new Error("Input file must be an Excel file (.xlsx or .xls)"); + } +} + +async function writeParseOutput(args: { + jsonOutput: string; + output?: string; + outputDir?: string; + pretty: boolean; + result: ReturnType; +}): Promise { + const { jsonOutput, output, outputDir, pretty, result } = args; + + if (outputDir) { + const resolvedOutputDir = resolveCliPath(outputDir); + + await writeParseResultToConfigStore(result, resolvedOutputDir, { + pretty, + }); + // eslint-disable-next-line no-console + console.error( + `✓ Successfully parsed and wrote file-store output to: ${resolvedOutputDir}`, + ); + + return; + } + + if (output) { + const resolvedOutput = resolveCliPath(output); + + // eslint-disable-next-line security/detect-non-literal-fs-filename + fs.writeFileSync(resolvedOutput, jsonOutput, "utf8"); + // eslint-disable-next-line no-console + console.error( + `✓ Successfully parsed and wrote output to: ${resolvedOutput}`, + ); + + return; + } + + // eslint-disable-next-line no-console + console.log(jsonOutput); +} + +function reportMappingValidation( + mappingValidation: ReturnType | undefined, +): void { + if (!mappingValidation) { + return; + } + + if (mappingValidation.ok) { + // eslint-disable-next-line no-console + console.error("✓ Mapping validation found no incomplete mappings."); + + return; + } + + // eslint-disable-next-line no-console + console.error( + `⚠ Mapping validation found ${mappingValidation.warnings.length} warning(s):`, + ); + + for (const line of formatGroupedMappingWarnings(mappingValidation.warnings)) { + // eslint-disable-next-line no-console + console.error(line); + } +} + +async function parseArguments(): Promise { const argv = await yargs(hideBin(process.argv)) .usage("Usage: $0 [options]") .command( "$0 ", "Parse an Excel file and output the parsed JSON data", - (args) => { - return args.positional("input", { + (args) => + args.positional("input", { describe: "Path to the Excel file to parse", type: "string", demandOption: true, - }); - }, + }), ) .option("output", { alias: "o", @@ -39,90 +125,70 @@ async function main() { description: "Write one JSON file per record into a file-store-compatible directory", }) - .conflicts("output", "output-dir") .option("pretty", { alias: "p", type: "boolean", default: false, - description: "Pretty-print the JSON output", + description: "Pretty-print JSON output", }) - .example("$0 config.xlsx", "Parse and output to stdout") - .example("$0 config.xlsx --pretty", "Parse and pretty-print to stdout") - .example("$0 config.xlsx -o output.json", "Parse and save to file") - .example( - "$0 config.xlsx --output-dir ./config-store", - "Parse and save one JSON file per record for file-store consumption", - ) + .option("check-mappings", { + type: "boolean", + default: false, + description: + "Warn when promoted variants are missing promoted pack, supplier-pack, volume-group, or supplier-allocation mappings", + }) + .conflicts("output", "output-dir") .example( - "$0 config.xlsx --pretty --output output.json", - "Parse with pretty formatting and save to file", + "$0 config.xlsx --check-mappings", + "Parse and warn about incomplete config-store mappings for promoted variants", ) .help("h") .alias("h", "help") .strict() .parseAsync(); - const { input, output, outputDir, pretty } = argv as unknown as Arguments; - - // Resolve input file path - const resolvedInput = path.isAbsolute(input) - ? input - : path.join(process.cwd(), input); + return { + checkMappings: Boolean(argv.checkMappings), + input: String(argv.input), + output: typeof argv.output === "string" ? argv.output : undefined, + outputDir: typeof argv.outputDir === "string" ? argv.outputDir : undefined, + pretty: Boolean(argv.pretty), + }; +} - // Check if input file exists - // eslint-disable-next-line security/detect-non-literal-fs-filename - if (!fs.existsSync(resolvedInput)) { - // eslint-disable-next-line no-console - console.error(`Error: Input file not found: ${resolvedInput}`); - process.exit(1); - } +async function runParser({ + checkMappings, + input, + output, + outputDir, + pretty, +}: Arguments): Promise { + const resolvedInput = resolveCliPath(input); - // Check if input file is an Excel file - if (!/\.xlsx?$/i.test(resolvedInput)) { - // eslint-disable-next-line no-console - console.error(`Error: Input file must be an Excel file (.xlsx or .xls)`); - process.exit(1); - } + assertValidInputFile(resolvedInput); - try { - // Parse the Excel file - const result = parseExcelFile(resolvedInput); + const result = parseExcelFile(resolvedInput); + const mappingValidation = checkMappings + ? validateIncompleteMappings(result) + : undefined; + const jsonOutput = stringifyJsonWithSortedKeys( + result, + pretty ? 2 : undefined, + ); - // Format the output - const jsonOutput = stringifyJsonWithSortedKeys( - result, - pretty ? 2 : undefined, - ); + await writeParseOutput({ + jsonOutput, + output, + outputDir, + pretty, + result, + }); + reportMappingValidation(mappingValidation); +} - if (outputDir) { - const resolvedOutputDir = path.isAbsolute(outputDir) - ? outputDir - : path.join(process.cwd(), outputDir); - - await writeParseResultToConfigStore(result, resolvedOutputDir, { - pretty, - }); - // eslint-disable-next-line no-console - console.error( - `✓ Successfully parsed and wrote file-store output to: ${resolvedOutputDir}`, - ); - } else if (output) { - // Write to file - const resolvedOutput = path.isAbsolute(output) - ? output - : path.join(process.cwd(), output); - - // eslint-disable-next-line security/detect-non-literal-fs-filename - fs.writeFileSync(resolvedOutput, jsonOutput, "utf8"); - // eslint-disable-next-line no-console - console.error( - `✓ Successfully parsed and wrote output to: ${resolvedOutput}`, - ); - } else { - // Write to stdout - // eslint-disable-next-line no-console - console.log(jsonOutput); - } +async function main(): Promise { + try { + await runParser(await parseArguments()); } catch (error) { // eslint-disable-next-line no-console console.error( diff --git a/packages/excel-parser/src/index.ts b/packages/excel-parser/src/index.ts index 71b1f4c..0688cdd 100644 --- a/packages/excel-parser/src/index.ts +++ b/packages/excel-parser/src/index.ts @@ -2,4 +2,13 @@ export { parseExcelFile } from "./parse-excel"; export type { ParseResult } from "./parse-excel"; export { writeParseResultToConfigStore } from "./config-store-output"; export type { WriteConfigStoreOptions } from "./config-store-output"; +export { validateIncompleteMappings } from "./mapping-validation"; +export { + formatGroupedMappingWarnings, + groupMappingWarningsByVariant, +} from "./mapping-validation-report"; +export type { + MappingValidationResult, + MappingValidationWarning, +} from "./mapping-validation"; export { generateTemplateExcel } from "./template"; diff --git a/packages/excel-parser/src/mapping-validation-report.ts b/packages/excel-parser/src/mapping-validation-report.ts new file mode 100644 index 0000000..8f3a37c --- /dev/null +++ b/packages/excel-parser/src/mapping-validation-report.ts @@ -0,0 +1,47 @@ +import type { MappingValidationWarning } from "./mapping-validation"; + +export interface GroupedMappingWarnings { + details: string[]; + variantId: string; + warnings: MappingValidationWarning[]; +} + +export function groupMappingWarningsByVariant( + warnings: MappingValidationWarning[], +): GroupedMappingWarnings[] { + const warningsByVariant = new Map(); + + for (const warning of warnings) { + warningsByVariant.set(warning.id, [ + ...(warningsByVariant.get(warning.id) ?? []), + warning, + ]); + } + + return [...warningsByVariant.entries()] + .toSorted(([left], [right]) => left.localeCompare(right)) + .map(([variantId, groupedWarnings]) => ({ + variantId, + warnings: groupedWarnings, + details: [ + ...new Set(groupedWarnings.flatMap((warning) => warning.details ?? [])), + ], + })); +} + +export function formatGroupedMappingWarnings( + warnings: MappingValidationWarning[], +): string[] { + return groupMappingWarningsByVariant(warnings).flatMap( + ({ details, variantId, warnings: groupedWarnings }) => [ + `- ${variantId}`, + ...groupedWarnings.map((warning) => ` • ${warning.message}`), + ...(details.length > 0 + ? [ + " mapped combinations:", + ...details.map((detail) => ` - ${detail}`), + ] + : []), + ], + ); +} diff --git a/packages/excel-parser/src/mapping-validation.ts b/packages/excel-parser/src/mapping-validation.ts new file mode 100644 index 0000000..268255d --- /dev/null +++ b/packages/excel-parser/src/mapping-validation.ts @@ -0,0 +1,103 @@ +import type { + DomainEntityName, + LoadedConfigStore, +} from "@supplier-config/file-store"; +import { validateConfigStoreIntegrity } from "@supplier-config/file-store"; + +import type { ParseResult } from "./parse-excel"; + +export interface MappingValidationWarning { + entity: "letter-variant"; + id: string; + message: string; + path?: (string | number)[]; + details?: string[]; +} + +export interface MappingValidationResult { + ok: boolean; + warnings: MappingValidationWarning[]; +} + +const inMemoryRootPath = ""; + +function buildInMemorySourceFilePath( + entity: DomainEntityName, + id: string, +): string { + return `${inMemoryRootPath}/${entity}/${encodeURIComponent(id)}.json`; +} + +function buildInMemoryConfigStore(result: ParseResult): LoadedConfigStore { + return { + rootPath: inMemoryRootPath, + records: [ + ...Object.values(result.volumeGroups).map((data) => ({ + entity: "volume-group" as const, + id: data.id, + sourceFilePath: buildInMemorySourceFilePath("volume-group", data.id), + data, + })), + ...Object.values(result.variants).map((data) => ({ + entity: "letter-variant" as const, + id: data.id, + sourceFilePath: buildInMemorySourceFilePath("letter-variant", data.id), + data, + })), + ...Object.values(result.packs).map((data) => ({ + entity: "pack-specification" as const, + id: data.id, + sourceFilePath: buildInMemorySourceFilePath( + "pack-specification", + data.id, + ), + data, + })), + ...Object.values(result.suppliers).map((data) => ({ + entity: "supplier" as const, + id: data.id, + sourceFilePath: buildInMemorySourceFilePath("supplier", data.id), + data, + })), + ...Object.values(result.allocations).map((data) => ({ + entity: "supplier-allocation" as const, + id: data.id, + sourceFilePath: buildInMemorySourceFilePath( + "supplier-allocation", + data.id, + ), + data, + })), + ...Object.values(result.supplierPacks).map((data) => ({ + entity: "supplier-pack" as const, + id: data.id, + sourceFilePath: buildInMemorySourceFilePath("supplier-pack", data.id), + data, + })), + ], + }; +} + +export function validateIncompleteMappings( + result: ParseResult, +): MappingValidationResult { + const store = buildInMemoryConfigStore(result); + const sourcePathToVariantId = new Map( + store.records + .filter((record) => record.entity === "letter-variant") + .map((record) => [record.sourceFilePath, record.id] as const), + ); + const integrityValidation = validateConfigStoreIntegrity(store); + const warnings = integrityValidation.issues.map((issue) => ({ + entity: "letter-variant" as const, + id: sourcePathToVariantId.get(issue.sourceFilePath)!, + message: issue.message, + path: issue.path, + details: issue.details, + })); + + return { + ok: warnings.length === 0, + warnings, + }; +} diff --git a/packages/file-store/src/__tests__/config-store-integrity-validator.test.ts b/packages/file-store/src/__tests__/config-store-integrity-validator.test.ts new file mode 100644 index 0000000..25ab4f2 --- /dev/null +++ b/packages/file-store/src/__tests__/config-store-integrity-validator.test.ts @@ -0,0 +1,614 @@ +import { z } from "zod"; + +import type { DomainEntityName } from "../types"; +import { validateConfigStoreIntegrity } from "../validation/config-store-integrity-validator"; + +jest.mock( + "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/volume-group", + () => ({ + $VolumeGroup: z.object({ + id: z.string(), + name: z.string(), + status: z.enum(["DRAFT", "INT", "PROD", "DISABLED"]), + }), + }), +); + +jest.mock( + "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/letter-variant", + () => ({ + $LetterVariant: z.object({ + id: z.string(), + name: z.string(), + status: z.enum(["DRAFT", "INT", "PROD", "DISABLED"]), + volumeGroupId: z.string(), + packSpecificationIds: z.array(z.string()).min(1), + supplierId: z.string().optional(), + }), + }), +); + +jest.mock( + "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/pack-specification", + () => ({ + $PackSpecification: z.object({ + id: z.string(), + name: z.string(), + status: z.enum(["DRAFT", "INT", "PROD", "DISABLED"]), + }), + }), +); + +jest.mock( + "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/supplier-allocation", + () => ({ + $SupplierAllocation: z.object({ + id: z.string(), + volumeGroup: z.string(), + supplier: z.string(), + allocationPercentage: z.number(), + status: z.enum(["DRAFT", "INT", "PROD", "DISABLED"]), + }), + }), +); + +jest.mock( + "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/supplier-pack", + () => ({ + $SupplierPack: z.object({ + id: z.string(), + packSpecificationId: z.string(), + supplierId: z.string(), + approval: z.enum([ + "DRAFT", + "SUBMITTED", + "PROOF_RECEIVED", + "APPROVED", + "REJECTED", + "DISABLED", + ]), + status: z.enum(["DRAFT", "INT", "PROD", "DISABLED"]), + }), + }), +); +const rootPath = "/config-store"; + +function makeRecord(entity: DomainEntityName, id: string, data: unknown) { + return { + entity, + id, + data, + sourceFilePath: `${rootPath}/${entity}/${id}.json`, + }; +} + +describe("validateConfigStoreIntegrity", () => { + it("returns ok=true when a promoted variant has complete promoted mappings", () => { + const result = validateConfigStoreIntegrity({ + rootPath, + records: [ + makeRecord("volume-group", "volume-group-1", { + id: "volume-group-1", + name: "Volume group 1", + status: "PROD", + }), + makeRecord("pack-specification", "pack-1", { + id: "pack-1", + name: "Pack 1", + status: "PROD", + }), + makeRecord("supplier-allocation", "allocation-1", { + id: "allocation-1", + volumeGroup: "volume-group-1", + supplier: "supplier-1", + allocationPercentage: 100, + status: "PROD", + }), + makeRecord("supplier-pack", "supplier-pack-1", { + id: "supplier-pack-1", + packSpecificationId: "pack-1", + supplierId: "supplier-1", + approval: "APPROVED", + status: "PROD", + }), + makeRecord("letter-variant", "variant-1", { + id: "variant-1", + name: "Variant 1", + status: "PROD", + volumeGroupId: "volume-group-1", + packSpecificationIds: ["pack-1"], + supplierId: "supplier-1", + }), + ], + }); + + expect(result).toEqual({ + ok: true, + issues: [], + }); + }); + + it("reports a missing sufficiently promoted pack specification", () => { + const result = validateConfigStoreIntegrity({ + rootPath, + records: [ + makeRecord("pack-specification", "pack-1", { + id: "pack-1", + name: "Pack 1", + status: "INT", + }), + makeRecord("letter-variant", "variant-1", { + id: "variant-1", + name: "Variant 1", + status: "PROD", + volumeGroupId: "volume-group-1", + packSpecificationIds: ["pack-1"], + }), + ], + }); + + expect(result).toEqual({ + ok: false, + issues: [ + { + entity: "letter-variant", + sourceFilePath: "/config-store/letter-variant/variant-1.json", + message: + "LetterVariant 'variant-1' is in PROD, but none of its referenced pack specifications are promoted to PROD or higher.", + path: ["packSpecificationIds"], + details: [ + "packSpecification=pack-1 status=INT => invalid", + "supplierPack=", + ], + }, + { + entity: "letter-variant", + sourceFilePath: "/config-store/letter-variant/variant-1.json", + message: + "LetterVariant 'variant-1' references volume group 'volume-group-1', but that volume group does not exist.", + path: ["volumeGroupId"], + }, + ], + }); + }); + + it("reports a missing promoted supplier pack for promoted packs", () => { + const result = validateConfigStoreIntegrity({ + rootPath, + records: [ + makeRecord("volume-group", "volume-group-1", { + id: "volume-group-1", + name: "Volume group 1", + status: "PROD", + }), + makeRecord("pack-specification", "pack-1", { + id: "pack-1", + name: "Pack 1", + status: "PROD", + }), + makeRecord("supplier-allocation", "allocation-1", { + id: "allocation-1", + volumeGroup: "volume-group-1", + supplier: "supplier-1", + allocationPercentage: 100, + status: "PROD", + }), + makeRecord("supplier-pack", "supplier-pack-1", { + id: "supplier-pack-1", + packSpecificationId: "pack-1", + supplierId: "supplier-1", + approval: "APPROVED", + status: "INT", + }), + makeRecord("letter-variant", "variant-1", { + id: "variant-1", + name: "Variant 1", + status: "PROD", + volumeGroupId: "volume-group-1", + packSpecificationIds: ["pack-1"], + }), + ], + }); + + expect(result.issues).toContainEqual({ + entity: "letter-variant", + sourceFilePath: "/config-store/letter-variant/variant-1.json", + message: + "LetterVariant 'variant-1' is in PROD, but none of its promoted pack specifications have a supplier pack in PROD and APPROVED state.", + path: ["packSpecificationIds"], + details: [ + "packSpecification=pack-1 status=PROD => valid", + "supplierPack=supplier-pack-1 supplier=supplier-1 status=INT approval=APPROVED => invalid", + ], + }); + }); + + it("reports a missing sufficiently promoted supplier pack for INT variants", () => { + const result = validateConfigStoreIntegrity({ + rootPath, + records: [ + makeRecord("volume-group", "volume-group-1", { + id: "volume-group-1", + name: "Volume group 1", + status: "INT", + }), + makeRecord("pack-specification", "pack-1", { + id: "pack-1", + name: "Pack 1", + status: "INT", + }), + makeRecord("supplier-allocation", "allocation-1", { + id: "allocation-1", + volumeGroup: "volume-group-1", + supplier: "supplier-1", + allocationPercentage: 100, + status: "INT", + }), + makeRecord("supplier-pack", "supplier-pack-1", { + id: "supplier-pack-1", + packSpecificationId: "pack-1", + supplierId: "supplier-1", + approval: "SUBMITTED", + status: "DRAFT", + }), + makeRecord("letter-variant", "variant-1", { + id: "variant-1", + name: "Variant 1", + status: "INT", + volumeGroupId: "volume-group-1", + packSpecificationIds: ["pack-1"], + }), + ], + }); + + expect(result.issues).toContainEqual({ + entity: "letter-variant", + sourceFilePath: "/config-store/letter-variant/variant-1.json", + message: + "LetterVariant 'variant-1' is in INT, but none of its promoted pack specifications have a supplier pack promoted to INT or higher.", + path: ["packSpecificationIds"], + details: [ + "packSpecification=pack-1 status=INT => valid", + "supplierPack=supplier-pack-1 supplier=supplier-1 status=DRAFT approval=SUBMITTED => invalid", + ], + }); + }); + + it("treats DISABLED supplier packs as invalid for promoted variants", () => { + const result = validateConfigStoreIntegrity({ + rootPath, + records: [ + makeRecord("volume-group", "volume-group-1", { + id: "volume-group-1", + name: "Volume group 1", + status: "INT", + }), + makeRecord("pack-specification", "pack-1", { + id: "pack-1", + name: "Pack 1", + status: "INT", + }), + makeRecord("supplier-allocation", "allocation-1", { + id: "allocation-1", + volumeGroup: "volume-group-1", + supplier: "supplier-1", + allocationPercentage: 100, + status: "INT", + }), + makeRecord("supplier-pack", "supplier-pack-1", { + id: "supplier-pack-1", + packSpecificationId: "pack-1", + supplierId: "supplier-1", + approval: "APPROVED", + status: "DISABLED", + }), + makeRecord("letter-variant", "variant-disabled-supplier-pack", { + id: "variant-disabled-supplier-pack", + name: "Variant Disabled Supplier Pack", + status: "INT", + volumeGroupId: "volume-group-1", + packSpecificationIds: ["pack-1"], + }), + ], + }); + + expect(result.issues).toContainEqual({ + entity: "letter-variant", + sourceFilePath: + "/config-store/letter-variant/variant-disabled-supplier-pack.json", + message: + "LetterVariant 'variant-disabled-supplier-pack' is in INT, but none of its promoted pack specifications have a supplier pack promoted to INT or higher.", + path: ["packSpecificationIds"], + details: [ + "packSpecification=pack-1 status=INT => valid", + "supplierPack=supplier-pack-1 supplier=supplier-1 status=DISABLED approval=APPROVED => invalid", + ], + }); + }); + + it("reports a missing promoted supplier pack for a supplier-scoped variant", () => { + const result = validateConfigStoreIntegrity({ + rootPath, + records: [ + makeRecord("volume-group", "volume-group-1", { + id: "volume-group-1", + name: "Volume group 1", + status: "INT", + }), + makeRecord("pack-specification", "pack-1", { + id: "pack-1", + name: "Pack 1", + status: "INT", + }), + makeRecord("supplier-allocation", "allocation-1", { + id: "allocation-1", + volumeGroup: "volume-group-1", + supplier: "supplier-1", + allocationPercentage: 100, + status: "INT", + }), + makeRecord("supplier-pack", "supplier-pack-1", { + id: "supplier-pack-1", + packSpecificationId: "pack-1", + supplierId: "supplier-2", + approval: "APPROVED", + status: "INT", + }), + makeRecord("letter-variant", "variant-1", { + id: "variant-1", + name: "Variant 1", + status: "INT", + volumeGroupId: "volume-group-1", + packSpecificationIds: ["pack-1"], + supplierId: "supplier-1", + }), + ], + }); + + expect(result.issues).toContainEqual({ + entity: "letter-variant", + sourceFilePath: "/config-store/letter-variant/variant-1.json", + message: + "LetterVariant 'variant-1' is scoped to supplier 'supplier-1', but no promoted supplier pack exists for that supplier and variant pack specifications at INT or higher.", + path: ["supplierId"], + details: [ + "packSpecification=pack-1 status=INT => valid", + "supplierPack=supplier-pack-1 supplier=supplier-2 status=INT approval=APPROVED => valid", + ], + }); + }); + + it("includes missing-pack diagnostics when the variant references an unknown pack specification", () => { + const result = validateConfigStoreIntegrity({ + rootPath, + records: [ + makeRecord("volume-group", "volume-group-1", { + id: "volume-group-1", + name: "Volume group 1", + status: "PROD", + }), + makeRecord("supplier-allocation", "allocation-1", { + id: "allocation-1", + volumeGroup: "volume-group-1", + supplier: "supplier-1", + allocationPercentage: 100, + status: "PROD", + }), + makeRecord("letter-variant", "variant-missing-pack", { + id: "variant-missing-pack", + name: "Variant Missing Pack", + status: "PROD", + volumeGroupId: "volume-group-1", + packSpecificationIds: ["pack-missing"], + }), + ], + }); + + expect(result.issues).toContainEqual({ + entity: "letter-variant", + sourceFilePath: "/config-store/letter-variant/variant-missing-pack.json", + message: + "LetterVariant 'variant-missing-pack' is in PROD, but none of its referenced pack specifications are promoted to PROD or higher.", + path: ["packSpecificationIds"], + details: [ + "packSpecification=pack-missing status= => missing", + "supplierPack=", + ], + }); + }); + + it("reports a missing supplier allocation when the volume group exists", () => { + const result = validateConfigStoreIntegrity({ + rootPath, + records: [ + makeRecord("volume-group", "volume-group-1", { + id: "volume-group-1", + name: "Volume group 1", + status: "PROD", + }), + makeRecord("pack-specification", "pack-1", { + id: "pack-1", + name: "Pack 1", + status: "PROD", + }), + makeRecord("supplier-pack", "supplier-pack-1", { + id: "supplier-pack-1", + packSpecificationId: "pack-1", + supplierId: "supplier-1", + approval: "APPROVED", + status: "PROD", + }), + makeRecord("letter-variant", "variant-1", { + id: "variant-1", + name: "Variant 1", + status: "PROD", + volumeGroupId: "volume-group-1", + packSpecificationIds: ["pack-1"], + }), + ], + }); + + expect(result.issues).toContainEqual({ + entity: "letter-variant", + sourceFilePath: "/config-store/letter-variant/variant-1.json", + message: + "LetterVariant 'variant-1' references volume group 'volume-group-1', but no supplier allocation is associated with it.", + path: ["volumeGroupId"], + }); + }); + + it.each(["DRAFT", "DISABLED"])( + "ignores non-promoted variants when status is %s", + (status) => { + const result = validateConfigStoreIntegrity({ + rootPath, + records: [ + makeRecord("letter-variant", "variant-1", { + id: "variant-1", + name: "Variant 1", + status, + volumeGroupId: "missing-volume-group", + packSpecificationIds: ["missing-pack"], + supplierId: "supplier-1", + }), + ], + }); + + expect(result).toEqual({ + ok: true, + issues: [], + }); + }, + ); + + it("requires supplier packs to be APPROVED when validating PROD variants", () => { + const result = validateConfigStoreIntegrity({ + rootPath, + records: [ + makeRecord("volume-group", "volume-group-1", { + id: "volume-group-1", + name: "Volume group 1", + status: "PROD", + }), + makeRecord("pack-specification", "pack-1", { + id: "pack-1", + name: "Pack 1", + status: "PROD", + }), + makeRecord("supplier-allocation", "allocation-1", { + id: "allocation-1", + volumeGroup: "volume-group-1", + supplier: "supplier-1", + allocationPercentage: 100, + status: "PROD", + }), + makeRecord("supplier-pack", "supplier-pack-1", { + id: "supplier-pack-1", + packSpecificationId: "pack-1", + supplierId: "supplier-1", + approval: "SUBMITTED", + status: "PROD", + }), + makeRecord("letter-variant", "variant-1", { + id: "variant-1", + name: "Variant 1", + status: "PROD", + volumeGroupId: "volume-group-1", + packSpecificationIds: ["pack-1"], + supplierId: "supplier-1", + }), + ], + }); + + expect(result.issues).toEqual( + expect.arrayContaining([ + { + entity: "letter-variant", + sourceFilePath: "/config-store/letter-variant/variant-1.json", + message: + "LetterVariant 'variant-1' is in PROD, but none of its promoted pack specifications have a supplier pack in PROD and APPROVED state.", + path: ["packSpecificationIds"], + details: [ + "packSpecification=pack-1 status=PROD => valid", + "supplierPack=supplier-pack-1 supplier=supplier-1 status=PROD approval=SUBMITTED => invalid", + ], + }, + { + entity: "letter-variant", + sourceFilePath: "/config-store/letter-variant/variant-1.json", + message: + "LetterVariant 'variant-1' is scoped to supplier 'supplier-1', but no supplier pack in PROD and APPROVED state exists for that supplier and variant pack specifications.", + path: ["supplierId"], + details: [ + "packSpecification=pack-1 status=PROD => valid", + "supplierPack=supplier-pack-1 supplier=supplier-1 status=PROD approval=SUBMITTED => invalid", + ], + }, + ]), + ); + }); + + it("does not require supplier packs to be APPROVED when validating INT variants", () => { + const result = validateConfigStoreIntegrity({ + rootPath, + records: [ + makeRecord("volume-group", "volume-group-1", { + id: "volume-group-1", + name: "Volume group 1", + status: "INT", + }), + makeRecord("pack-specification", "pack-1", { + id: "pack-1", + name: "Pack 1", + status: "INT", + }), + makeRecord("supplier-allocation", "allocation-1", { + id: "allocation-1", + volumeGroup: "volume-group-1", + supplier: "supplier-1", + allocationPercentage: 100, + status: "INT", + }), + makeRecord("supplier-pack", "supplier-pack-1", { + id: "supplier-pack-1", + packSpecificationId: "pack-1", + supplierId: "supplier-1", + approval: "SUBMITTED", + status: "INT", + }), + makeRecord("letter-variant", "variant-1", { + id: "variant-1", + name: "Variant 1", + status: "INT", + volumeGroupId: "volume-group-1", + packSpecificationIds: ["pack-1"], + supplierId: "supplier-1", + }), + ], + }); + + expect(result).toEqual({ + ok: true, + issues: [], + }); + }); + + it("ignores records that do not parse against the entity schema", () => { + const result = validateConfigStoreIntegrity({ + rootPath, + records: [ + makeRecord("letter-variant", "variant-1", { + id: "variant-1", + name: "Variant 1", + volumeGroupId: "volume-group-1", + packSpecificationIds: ["pack-1"], + }), + ], + }); + + expect(result).toEqual({ + ok: true, + issues: [], + }); + }); +}); diff --git a/packages/file-store/src/__tests__/config-store-validator.test.ts b/packages/file-store/src/__tests__/config-store-validator.test.ts index d731ade..e753b4b 100644 --- a/packages/file-store/src/__tests__/config-store-validator.test.ts +++ b/packages/file-store/src/__tests__/config-store-validator.test.ts @@ -6,17 +6,30 @@ import { validateConfigStore, } from "@supplier-config/file-store"; -jest.mock("@nhsdigital/nhs-notify-event-schemas-supplier-config", () => { - return { +jest.mock( + "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/volume-group", + () => ({ $VolumeGroup: z.object({ id: z.string(), name: z.string(), }), + }), +); + +jest.mock( + "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/letter-variant", + () => ({ $LetterVariant: z.object({ id: z.string(), priority: z.number().int().min(1).max(99).default(50), packSpecificationIds: z.array(z.string()).min(1), }), + }), +); + +jest.mock( + "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/pack-specification", + () => ({ $PackSpecification: z.object({ id: z.string(), version: z.number().int(), @@ -24,20 +37,38 @@ jest.mock("@nhsdigital/nhs-notify-event-schemas-supplier-config", () => { id: z.string(), }), }), + }), +); + +jest.mock( + "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/supplier", + () => ({ $Supplier: z.object({ id: z.string(), dailyCapacity: z.number().int(), }), + }), +); + +jest.mock( + "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/supplier-allocation", + () => ({ $SupplierAllocation: z.object({ id: z.string(), allocationPercentage: z.number().min(0).max(100), }), + }), +); + +jest.mock( + "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/supplier-pack", + () => ({ $SupplierPack: z.object({ id: z.string(), approval: z.enum(["APPROVED", "DRAFT"]), }), - }; -}); + }), +); const mockRootPath = "/config-store"; diff --git a/packages/file-store/src/index.ts b/packages/file-store/src/index.ts index 665e697..b332b58 100644 --- a/packages/file-store/src/index.ts +++ b/packages/file-store/src/index.ts @@ -1,3 +1,4 @@ export { default as loadConfigStore } from "./loader/config-store-loader"; +export * from "./validation/config-store-integrity-validator"; export * from "./validation/config-store-validator"; export * from "./types"; diff --git a/packages/file-store/src/types.ts b/packages/file-store/src/types.ts index 79a481e..c035d69 100644 --- a/packages/file-store/src/types.ts +++ b/packages/file-store/src/types.ts @@ -28,6 +28,7 @@ export type ValidationIssue = { sourceFilePath: string; message: string; path?: (string | number)[]; + details?: string[]; }; export type ValidationResult = { diff --git a/packages/file-store/src/validation/config-store-integrity-validator.ts b/packages/file-store/src/validation/config-store-integrity-validator.ts new file mode 100644 index 0000000..dc37007 --- /dev/null +++ b/packages/file-store/src/validation/config-store-integrity-validator.ts @@ -0,0 +1,359 @@ +import type { ZodType } from "zod"; +import { + $LetterVariant, + LetterVariant, +} from "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/letter-variant"; +import { + $PackSpecification, + PackSpecification, +} from "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/pack-specification"; +import { + $SupplierAllocation, + SupplierAllocation, +} from "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/supplier-allocation"; +import { + $SupplierPack, + SupplierPack, +} from "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/supplier-pack"; +import { + $VolumeGroup, + VolumeGroup, +} from "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/volume-group"; + +import type { + ConfigRecord, + LoadedConfigStore, + ValidationIssue, + ValidationResult, +} from "../types"; + +type PromotionStatus = "INT" | "PROD"; +type EnvironmentStatus = "DRAFT" | "INT" | "PROD" | "DISABLED"; +type TypedRecord = Omit & { data: T }; + +const promotionRankByStatus: Record = { + DISABLED: -1, + DRAFT: 0, + INT: 1, + PROD: 2, +}; + +function isPromotionStatus(status: string): status is PromotionStatus { + return status === "INT" || status === "PROD"; +} + +function getPromotionRank(status: EnvironmentStatus): number { + if (status === "DISABLED") { + return promotionRankByStatus.DISABLED; + } + + if (status === "DRAFT") { + return promotionRankByStatus.DRAFT; + } + + if (status === "INT") { + return promotionRankByStatus.INT; + } + + return promotionRankByStatus.PROD; +} + +function isStatusAtLeast( + status: EnvironmentStatus, + minimum: PromotionStatus, +): boolean { + return getPromotionRank(status) >= getPromotionRank(minimum); +} + +function isSupplierPackValidAtPromotion(args: { + minimumStatus: PromotionStatus; + supplierPack: SupplierPack; +}): boolean { + const { minimumStatus, supplierPack } = args; + + return ( + isStatusAtLeast(supplierPack.status, minimumStatus) && + (minimumStatus !== "PROD" || supplierPack.approval === "APPROVED") + ); +} + +function buildIssue( + record: Pick, + message: string, + path?: (string | number)[], + details?: string[], +): ValidationIssue { + return { + entity: record.entity, + sourceFilePath: record.sourceFilePath, + message, + path, + ...(details ? { details } : {}), + }; +} + +function parseTypedRecords( + store: LoadedConfigStore, + entity: ConfigRecord["entity"], + schema: ZodType, +): TypedRecord[] { + return store.records.flatMap((record) => { + if (record.entity !== entity) { + return []; + } + + const parsed = schema.safeParse(record.data); + + return parsed.success ? [{ ...record, data: parsed.data }] : []; + }); +} + +function getPromotedPackIds( + minimumStatus: PromotionStatus, + packById: Map>, + packSpecificationIds: string[], +): Set { + return new Set( + packSpecificationIds.filter((packSpecificationId) => { + const packSpecification = packById.get(packSpecificationId); + + return ( + packSpecification !== undefined && + isStatusAtLeast(packSpecification.data.status, minimumStatus) + ); + }), + ); +} + +function formatSupplierPackLine(args: { + minimumStatus: PromotionStatus; + supplierPack: SupplierPack; +}): string { + const { minimumStatus, supplierPack } = args; + const validity = isSupplierPackValidAtPromotion({ + minimumStatus, + supplierPack, + }) + ? "valid" + : "invalid"; + + return `supplierPack=${supplierPack.id} supplier=${supplierPack.supplierId} status=${supplierPack.status} approval=${supplierPack.approval} => ${validity}`; +} + +function buildVariantMappingDetails(args: { + minimumStatus: PromotionStatus; + packById: Map>; + supplierPacks: TypedRecord[]; + variant: LetterVariant; +}): string[] { + const { minimumStatus, packById, supplierPacks, variant } = args; + + return variant.packSpecificationIds.flatMap((packSpecificationId) => { + const packSpecification = packById.get(packSpecificationId); + const packStatus = packSpecification?.data.status ?? ""; + let packValidity: "missing" | "valid" | "invalid" = "missing"; + + if (packSpecification !== undefined) { + packValidity = isStatusAtLeast( + packSpecification.data.status, + minimumStatus, + ) + ? "valid" + : "invalid"; + } + + const matchingSupplierPacks = supplierPacks.filter( + (supplierPack) => + supplierPack.data.packSpecificationId === packSpecificationId, + ); + const supplierPackLines = + matchingSupplierPacks.length === 0 + ? ["supplierPack="] + : matchingSupplierPacks.map((supplierPack) => + formatSupplierPackLine({ + minimumStatus, + supplierPack: supplierPack.data, + }), + ); + + return [ + `packSpecification=${packSpecificationId} status=${packStatus} => ${packValidity}`, + ...supplierPackLines, + ]; + }); +} + +function getPromotedSupplierPacks(args: { + minimumStatus: PromotionStatus; + promotedPackIds: Set; + supplierPacks: TypedRecord[]; +}): TypedRecord[] { + const { minimumStatus, promotedPackIds, supplierPacks } = args; + + return supplierPacks.filter( + (supplierPack) => + promotedPackIds.has(supplierPack.data.packSpecificationId) && + isSupplierPackValidAtPromotion({ + minimumStatus, + supplierPack: supplierPack.data, + }), + ); +} + +function validateSupplierPackMappings(args: { + minimumStatus: PromotionStatus; + packById: Map>; + supplierPacks: TypedRecord[]; + variantRecord: TypedRecord; +}): ValidationIssue[] { + const { minimumStatus, packById, supplierPacks, variantRecord } = args; + const { data: variant } = variantRecord; + const warnings: ValidationIssue[] = []; + const mappingDetails = buildVariantMappingDetails({ + minimumStatus, + packById, + supplierPacks, + variant, + }); + const promotedPackIds = getPromotedPackIds( + minimumStatus, + packById, + variant.packSpecificationIds, + ); + + if (promotedPackIds.size === 0) { + warnings.push( + buildIssue( + variantRecord, + `LetterVariant '${variant.id}' is in ${variant.status}, but none of its referenced pack specifications are promoted to ${variant.status} or higher.`, + ["packSpecificationIds"], + mappingDetails, + ), + ); + + return warnings; + } + + const promotedSupplierPacks = getPromotedSupplierPacks({ + minimumStatus, + promotedPackIds, + supplierPacks, + }); + + if (promotedSupplierPacks.length === 0) { + warnings.push( + buildIssue( + variantRecord, + minimumStatus === "PROD" + ? `LetterVariant '${variant.id}' is in PROD, but none of its promoted pack specifications have a supplier pack in PROD and APPROVED state.` + : `LetterVariant '${variant.id}' is in ${variant.status}, but none of its promoted pack specifications have a supplier pack promoted to ${variant.status} or higher.`, + ["packSpecificationIds"], + mappingDetails, + ), + ); + } + + if ( + variant.supplierId && + !promotedSupplierPacks.some( + (supplierPack) => supplierPack.data.supplierId === variant.supplierId, + ) + ) { + warnings.push( + buildIssue( + variantRecord, + minimumStatus === "PROD" + ? `LetterVariant '${variant.id}' is scoped to supplier '${variant.supplierId}', but no supplier pack in PROD and APPROVED state exists for that supplier and variant pack specifications.` + : `LetterVariant '${variant.id}' is scoped to supplier '${variant.supplierId}', but no promoted supplier pack exists for that supplier and variant pack specifications at ${variant.status} or higher.`, + ["supplierId"], + mappingDetails, + ), + ); + } + + return warnings; +} + +function validateVolumeGroupMappings(args: { + allocations: TypedRecord[]; + variantRecord: TypedRecord; + volumeGroupById: Map>; +}): ValidationIssue[] { + const { allocations, variantRecord, volumeGroupById } = args; + const { data: variant } = variantRecord; + + if (!volumeGroupById.has(variant.volumeGroupId)) { + return [ + buildIssue( + variantRecord, + `LetterVariant '${variant.id}' references volume group '${variant.volumeGroupId}', but that volume group does not exist.`, + ["volumeGroupId"], + ), + ]; + } + + return allocations.some( + (allocation) => allocation.data.volumeGroup === variant.volumeGroupId, + ) + ? [] + : [ + buildIssue( + variantRecord, + `LetterVariant '${variant.id}' references volume group '${variant.volumeGroupId}', but no supplier allocation is associated with it.`, + ["volumeGroupId"], + ), + ]; +} + +export function validateConfigStoreIntegrity( + store: LoadedConfigStore, +): ValidationResult { + const issues: ValidationIssue[] = []; + const packById = new Map( + parseTypedRecords(store, "pack-specification", $PackSpecification).map( + (record) => [record.data.id, record] as const, + ), + ); + const supplierPacks = parseTypedRecords( + store, + "supplier-pack", + $SupplierPack, + ); + const allocations = parseTypedRecords( + store, + "supplier-allocation", + $SupplierAllocation, + ); + const volumeGroupById = new Map( + parseTypedRecords(store, "volume-group", $VolumeGroup).map( + (record) => [record.data.id, record] as const, + ), + ); + + for (const variantRecord of parseTypedRecords( + store, + "letter-variant", + $LetterVariant, + )) { + if (isPromotionStatus(variantRecord.data.status)) { + issues.push( + ...validateSupplierPackMappings({ + minimumStatus: variantRecord.data.status, + packById, + supplierPacks, + variantRecord, + }), + ...validateVolumeGroupMappings({ + allocations, + variantRecord, + volumeGroupById, + }), + ); + } + } + + return { + ok: issues.length === 0, + issues, + }; +} diff --git a/packages/file-store/src/validation/config-store-validator.ts b/packages/file-store/src/validation/config-store-validator.ts index 41808ce..aed4148 100644 --- a/packages/file-store/src/validation/config-store-validator.ts +++ b/packages/file-store/src/validation/config-store-validator.ts @@ -1,28 +1,11 @@ -import { - $LetterVariant, - $PackSpecification, - $Supplier, - $SupplierAllocation, - $SupplierPack, - $VolumeGroup, -} from "@nhsdigital/nhs-notify-event-schemas-supplier-config"; - import type { DomainEntityName, - EntitySchemaMap, LoadedConfigStore, ValidationIssue, ValidationResult, -} from "@supplier-config/file-store"; +} from "../types"; -const schemaByEntity: EntitySchemaMap = { - "volume-group": $VolumeGroup, - "letter-variant": $LetterVariant, - "pack-specification": $PackSpecification, - supplier: $Supplier, - "supplier-allocation": $SupplierAllocation, - "supplier-pack": $SupplierPack, -}; +import { schemaByEntity } from "./entity-schemas"; export function normalizeIssuePath(path: PropertyKey[]): (string | number)[] { return path diff --git a/packages/file-store/src/validation/entity-schemas.ts b/packages/file-store/src/validation/entity-schemas.ts new file mode 100644 index 0000000..4bec899 --- /dev/null +++ b/packages/file-store/src/validation/entity-schemas.ts @@ -0,0 +1,17 @@ +import { $LetterVariant } from "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/letter-variant"; +import { $PackSpecification } from "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/pack-specification"; +import { $Supplier } from "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/supplier"; +import { $SupplierAllocation } from "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/supplier-allocation"; +import { $SupplierPack } from "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/supplier-pack"; +import { $VolumeGroup } from "@nhsdigital/nhs-notify-event-schemas-supplier-config/src/domain/volume-group"; + +import type { EntitySchemaMap } from "../types"; + +export const schemaByEntity: EntitySchemaMap = { + "volume-group": $VolumeGroup, + "letter-variant": $LetterVariant, + "pack-specification": $PackSpecification, + supplier: $Supplier, + "supplier-allocation": $SupplierAllocation, + "supplier-pack": $SupplierPack, +}; From 90d15f8e307130021c0804502c46894a949e4b28 Mon Sep 17 00:00:00 2001 From: Mike Houston Date: Thu, 11 Jun 2026 17:41:17 +0100 Subject: [PATCH 2/3] Enhance action.yml to resolve commit SHA to tag for release asset downloads --- actions/ddb-publish/action.yml | 46 +++++++++++++++++++++----- actions/eventbridge-publish/action.yml | 46 +++++++++++++++++++++----- 2 files changed, 74 insertions(+), 18 deletions(-) diff --git a/actions/ddb-publish/action.yml b/actions/ddb-publish/action.yml index 1933b5e..5ca2401 100644 --- a/actions/ddb-publish/action.yml +++ b/actions/ddb-publish/action.yml @@ -55,18 +55,46 @@ runs: exit 1 fi - # Prefer release assets when the action ref is a tag with a matching release in the action repo. - if gh release view "${ACTION_REF}" --repo "${ACTION_REPO}" >/dev/null 2>&1; then - echo "[ddb-publish] Found release for ref '${ACTION_REF}' in '${ACTION_REPO}'. Downloading '${RELEASE_ASSET_NAME}'." - gh release download "${ACTION_REF}" --repo "${ACTION_REPO}" --pattern "${RELEASE_ASSET_NAME}" --dir "${download_dir}" + resolved_release_ref="${ACTION_REF}" + run_list_scope_description="" + run_list_scope_args=() + + if [[ "${ACTION_REF}" =~ ^[0-9a-fA-F]{40}$ ]]; then + echo "[ddb-publish] action_ref looks like a commit SHA. Attempting to resolve it to a tag before checking releases." + + resolved_tag="$({ + gh api \ + --paginate \ + "repos/${ACTION_REPO}/tags?per_page=100" \ + --jq ".[] | select(.commit.sha == \"${ACTION_REF}\") | .name" + } | head -n 1)" + + if [[ -n "${resolved_tag}" ]]; then + resolved_release_ref="${resolved_tag}" + echo "[ddb-publish] Resolved commit '${ACTION_REF}' to tag '${resolved_release_ref}'." + else + echo "[ddb-publish] No tag matched commit '${ACTION_REF}'." + fi + + run_list_scope_description="commit '${ACTION_REF}'" + run_list_scope_args=(--commit "${ACTION_REF}") + else + branch="${ACTION_REF#refs/heads/}" + run_list_scope_description="branch '${branch}'" + run_list_scope_args=(--branch "${branch}") + fi + + # Prefer release assets when the action ref is, or resolves to, a tag with a matching release in the action repo. + if gh release view "${resolved_release_ref}" --repo "${ACTION_REPO}" >/dev/null 2>&1; then + echo "[ddb-publish] Found release for ref '${resolved_release_ref}' in '${ACTION_REPO}'. Downloading '${RELEASE_ASSET_NAME}'." + gh release download "${resolved_release_ref}" --repo "${ACTION_REPO}" --pattern "${RELEASE_ASSET_NAME}" --dir "${download_dir}" tar -xzf "${download_dir}/${RELEASE_ASSET_NAME}" -C "${unpack_dir}" echo "[ddb-publish] Bundle extracted from release asset." exit 0 fi - # Otherwise treat the ref as a branch-like ref and fetch the latest successful CI artifact from the action repo. - branch="${ACTION_REF#refs/heads/}" - echo "[ddb-publish] No release found for ref '${ACTION_REF}'. Falling back to latest workflow artifact on branch '${branch}' from '${ACTION_REPO}'." + # Otherwise fetch the latest successful CI artifact using the original branch or commit from the action repo. + echo "[ddb-publish] No release found for ref '${resolved_release_ref}'. Falling back to latest workflow artifact on ${run_list_scope_description} from '${ACTION_REPO}'." run_id="" workflow_used="" @@ -78,7 +106,7 @@ runs: run_id_candidate="$(gh run list \ --repo "${ACTION_REPO}" \ --workflow "${workflow_file}" \ - --branch "${branch}" \ + "${run_list_scope_args[@]}" \ --status success \ --json databaseId \ --jq '.[0].databaseId' 2>/tmp/ddb_publish_run_list_err.log)" @@ -105,7 +133,7 @@ runs: done if [[ -z "${run_id}" || "${run_id}" == "null" ]]; then - echo "ERROR: Could not find a successful run on branch '${branch}' in '${ACTION_REPO}' containing artifact '${WORKFLOW_ARTIFACT_NAME}'. Checked workflows: stage-3-build.yaml, cicd-1-pull-request.yaml." >&2 + echo "ERROR: Could not find a successful run for ${run_list_scope_description} in '${ACTION_REPO}' containing artifact '${WORKFLOW_ARTIFACT_NAME}'. Checked workflows: stage-3-build.yaml, cicd-1-pull-request.yaml." >&2 exit 1 fi diff --git a/actions/eventbridge-publish/action.yml b/actions/eventbridge-publish/action.yml index eeae959..cdc253c 100644 --- a/actions/eventbridge-publish/action.yml +++ b/actions/eventbridge-publish/action.yml @@ -48,18 +48,46 @@ runs: exit 1 fi - # Prefer release assets when the action ref is a tag with a matching release in the action repo. - if gh release view "${ACTION_REF}" --repo "${ACTION_REPO}" >/dev/null 2>&1; then - echo "[eventbridge-publish] Found release for ref '${ACTION_REF}' in '${ACTION_REPO}'. Downloading '${RELEASE_ASSET_NAME}'." - gh release download "${ACTION_REF}" --repo "${ACTION_REPO}" --pattern "${RELEASE_ASSET_NAME}" --dir "${download_dir}" + resolved_release_ref="${ACTION_REF}" + run_list_scope_description="" + run_list_scope_args=() + + if [[ "${ACTION_REF}" =~ ^[0-9a-fA-F]{40}$ ]]; then + echo "[eventbridge-publish] action_ref looks like a commit SHA. Attempting to resolve it to a tag before checking releases." + + resolved_tag="$({ + gh api \ + --paginate \ + "repos/${ACTION_REPO}/tags?per_page=100" \ + --jq ".[] | select(.commit.sha == \"${ACTION_REF}\") | .name" + } | head -n 1)" + + if [[ -n "${resolved_tag}" ]]; then + resolved_release_ref="${resolved_tag}" + echo "[eventbridge-publish] Resolved commit '${ACTION_REF}' to tag '${resolved_release_ref}'." + else + echo "[eventbridge-publish] No tag matched commit '${ACTION_REF}'." + fi + + run_list_scope_description="commit '${ACTION_REF}'" + run_list_scope_args=(--commit "${ACTION_REF}") + else + branch="${ACTION_REF#refs/heads/}" + run_list_scope_description="branch '${branch}'" + run_list_scope_args=(--branch "${branch}") + fi + + # Prefer release assets when the action ref is, or resolves to, a tag with a matching release in the action repo. + if gh release view "${resolved_release_ref}" --repo "${ACTION_REPO}" >/dev/null 2>&1; then + echo "[eventbridge-publish] Found release for ref '${resolved_release_ref}' in '${ACTION_REPO}'. Downloading '${RELEASE_ASSET_NAME}'." + gh release download "${resolved_release_ref}" --repo "${ACTION_REPO}" --pattern "${RELEASE_ASSET_NAME}" --dir "${download_dir}" tar -xzf "${download_dir}/${RELEASE_ASSET_NAME}" -C "${unpack_dir}" echo "[eventbridge-publish] Bundle extracted from release asset." exit 0 fi - # Otherwise treat the ref as a branch-like ref and fetch the latest successful CI artifact from the action repo. - branch="${ACTION_REF#refs/heads/}" - echo "[eventbridge-publish] No release found for ref '${ACTION_REF}'. Falling back to latest workflow artifact on branch '${branch}' from '${ACTION_REPO}'." + # Otherwise fetch the latest successful CI artifact using the original branch or commit from the action repo. + echo "[eventbridge-publish] No release found for ref '${resolved_release_ref}'. Falling back to latest workflow artifact on ${run_list_scope_description} from '${ACTION_REPO}'." run_id="" workflow_used="" @@ -71,7 +99,7 @@ runs: run_id_candidate="$(gh run list \ --repo "${ACTION_REPO}" \ --workflow "${workflow_file}" \ - --branch "${branch}" \ + "${run_list_scope_args[@]}" \ --status success \ --json databaseId \ --jq '.[0].databaseId' 2>/tmp/eventbridge_publish_run_list_err.log)" @@ -98,7 +126,7 @@ runs: done if [[ -z "${run_id}" || "${run_id}" == "null" ]]; then - echo "ERROR: Could not find a successful run on branch '${branch}' in '${ACTION_REPO}' containing artifact '${WORKFLOW_ARTIFACT_NAME}'. Checked workflows: stage-3-build.yaml, cicd-1-pull-request.yaml." >&2 + echo "ERROR: Could not find a successful run for ${run_list_scope_description} in '${ACTION_REPO}' containing artifact '${WORKFLOW_ARTIFACT_NAME}'. Checked workflows: stage-3-build.yaml, cicd-1-pull-request.yaml." >&2 exit 1 fi From e41d2ff08b27ed063e835ad939994bb9d894dec0 Mon Sep 17 00:00:00 2001 From: Mike Houston Date: Fri, 12 Jun 2026 10:01:42 +0100 Subject: [PATCH 3/3] Add make target to run validation against local specifications.xlsx file --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index 09c3dc2..84343a3 100644 --- a/Makefile +++ b/Makefile @@ -30,6 +30,9 @@ internal-config: npm run parse --workspace=@supplier-config/excel-parser -- \ "$(PWD)/specifications.xlsx" --output-dir "$(PWD)/artifacts/config-store" --pretty +validate-config: + npm run parse --workspace=@supplier-config/excel-parser -- \ + "$(PWD)/specifications.xlsx" --check-mappings # ============================================================================== ${VERBOSE}.SILENT: \