From 49da984bee5bd9ddacf922efba6a4e748b034bfd Mon Sep 17 00:00:00 2001 From: David Wass Date: Thu, 19 Feb 2026 15:18:40 +0000 Subject: [PATCH 01/24] Supplier Config table --- .../api/ddb_table_supplier_configuration.tf | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf diff --git a/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf b/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf new file mode 100644 index 00000000..5e228f30 --- /dev/null +++ b/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf @@ -0,0 +1,46 @@ +resource "aws_dynamodb_table" "supplier-configuration" { + name = "${local.csi}-supplier-config" + billing_mode = "PAY_PER_REQUEST" + + hash_key = "PK" + range_key = "SK" + + ttl { + attribute_name = "ttl" + enabled = true + } + + attribute { + name = "PK" + type = "S" + } + + attribute { + name = "SK" + type = "S" + } + + attribute { + name = "entityType" + type = "S" + } + // The type-index GSI allows us to query for all supplier configurations of a given type (e.g. all letter supplier configurations) + global_secondary_index { + name = "EntityTypeIndex" + hash_key = "entityType" + range_key = "SK" + projection_type = "ALL" + } + + point_in_time_recovery { + enabled = true + } + + tags = merge( + local.default_tags, + { + NHSE-Enable-Dynamo-Backup-Acct = "True" + } + ) + +} From cb4507123f5a514834c6d86520a40bf98d805d6a Mon Sep 17 00:00:00 2001 From: David Wass Date: Fri, 20 Feb 2026 15:40:52 +0000 Subject: [PATCH 02/24] Get the variant details --- .../terraform/components/api/locals.tf | 19 +++++++------ .../api/module_lambda_supplier_allocator.tf | 14 ++++++++++ internal/datastore/src/index.ts | 1 + .../src/supplier-config-repository.ts | 28 +++++++++++++++++++ .../src/config/__tests__/deps.test.ts | 16 +++++++++-- .../src/config/__tests__/env.test.ts | 2 ++ lambdas/supplier-allocator/src/config/deps.ts | 22 +++++++++++++++ lambdas/supplier-allocator/src/config/env.ts | 1 + .../__tests__/allocate-handler.test.ts | 9 ++++++ .../src/handler/allocate-handler.ts | 18 +++++++++++- 10 files changed, 118 insertions(+), 12 deletions(-) create mode 100644 internal/datastore/src/supplier-config-repository.ts diff --git a/infrastructure/terraform/components/api/locals.tf b/infrastructure/terraform/components/api/locals.tf index 46c7aca7..8edd8fa8 100644 --- a/infrastructure/terraform/components/api/locals.tf +++ b/infrastructure/terraform/components/api/locals.tf @@ -20,15 +20,16 @@ locals { destination_arn = "arn:aws:logs:${var.region}:${var.shared_infra_account_id}:destination:nhs-main-obs-firehose-logs" common_lambda_env_vars = { - LETTERS_TABLE_NAME = aws_dynamodb_table.letters.name, - MI_TABLE_NAME = aws_dynamodb_table.mi.name, - LETTER_TTL_HOURS = 12960, # 18 months * 30 days * 24 hours - MI_TTL_HOURS = 2160 # 90 days * 24 hours - SUPPLIER_ID_HEADER = "nhsd-supplier-id", - APIM_CORRELATION_HEADER = "nhsd-correlation-id", - DOWNLOAD_URL_TTL_SECONDS = 60 - SNS_TOPIC_ARN = "${module.eventsub.sns_topic.arn}", - EVENT_SOURCE = "/data-plane/supplier-api/${var.group}/${var.environment}/letters" + LETTERS_TABLE_NAME = aws_dynamodb_table.letters.name, + MI_TABLE_NAME = aws_dynamodb_table.mi.name, + LETTER_TTL_HOURS = 12960, # 18 months * 30 days * 24 hours + MI_TTL_HOURS = 2160 # 90 days * 24 hours + SUPPLIER_ID_HEADER = "nhsd-supplier-id", + APIM_CORRELATION_HEADER = "nhsd-correlation-id", + DOWNLOAD_URL_TTL_SECONDS = 60 + SNS_TOPIC_ARN = "${module.eventsub.sns_topic.arn}", + EVENT_SOURCE = "/data-plane/supplier-api/${var.group}/${var.environment}/letters" + SUPPLIER_CONFIG_TABLE_NAME = aws_dynamodb_table.supplier-configuration.name } core_pdf_bucket_arn = "arn:aws:s3:::comms-${var.core_account_id}-eu-west-2-${var.core_environment}-api-stg-pdf-pipeline" diff --git a/infrastructure/terraform/components/api/module_lambda_supplier_allocator.tf b/infrastructure/terraform/components/api/module_lambda_supplier_allocator.tf index 76cfb22a..b8f073a0 100644 --- a/infrastructure/terraform/components/api/module_lambda_supplier_allocator.tf +++ b/infrastructure/terraform/components/api/module_lambda_supplier_allocator.tf @@ -82,4 +82,18 @@ data "aws_iam_policy_document" "supplier_allocator_lambda" { module.sqs_letter_updates.sqs_queue_arn ] } + + statement { + sid = "AllowDynamoDBAccess" + effect = "Allow" + + actions = [ + "dynamodb:GetItem", + "dynamodb:Query" + ] + + resources = [ + aws_dynamodb_table.supplier-configuration.arn + ] + } } diff --git a/internal/datastore/src/index.ts b/internal/datastore/src/index.ts index 7ee912c2..6c32e4bc 100644 --- a/internal/datastore/src/index.ts +++ b/internal/datastore/src/index.ts @@ -3,5 +3,6 @@ export * from "./errors"; export * from "./mi-repository"; export * from "./letter-repository"; export * from "./supplier-repository"; +export * from "./supplier-config-repository"; export { default as LetterQueueRepository } from "./letter-queue-repository"; export { default as DBHealthcheck } from "./healthcheck"; diff --git a/internal/datastore/src/supplier-config-repository.ts b/internal/datastore/src/supplier-config-repository.ts new file mode 100644 index 00000000..73b247e2 --- /dev/null +++ b/internal/datastore/src/supplier-config-repository.ts @@ -0,0 +1,28 @@ +import { DynamoDBDocumentClient, GetCommand } from "@aws-sdk/lib-dynamodb"; +import { Logger } from "pino"; +import { $LetterVariant, LetterVariant } from "./SupplierConfigDomain"; + +export type SupplierConfigRepositoryConfig = { + supplierConfigTableName: string; +}; + +export class SupplierConfigRepository { + constructor( + readonly ddbClient: DynamoDBDocumentClient, + readonly log: Logger, + readonly config: SupplierConfigRepositoryConfig, + ) {} + + async getLetterVariant(variantId: string): Promise { + const result = await this.ddbClient.send( + new GetCommand({ + TableName: this.config.supplierConfigTableName, + Key: { PK: "LETTER_VARIANT", SK: variantId }, + }), + ); + if (!result.Item) { + throw new Error(`Letter variant with id ${variantId} not found`); + } + return $LetterVariant.parse(result.Item); + } +} diff --git a/lambdas/supplier-allocator/src/config/__tests__/deps.test.ts b/lambdas/supplier-allocator/src/config/__tests__/deps.test.ts index b8b7b736..f07f63aa 100644 --- a/lambdas/supplier-allocator/src/config/__tests__/deps.test.ts +++ b/lambdas/supplier-allocator/src/config/__tests__/deps.test.ts @@ -2,6 +2,7 @@ import type { Deps } from "lambdas/supplier-allocator/src/config/deps"; describe("createDependenciesContainer", () => { const env = { + SUPPLIER_CONFIG_TABLE_NAME: "SupplierConfigTable", VARIANT_MAP: { lv1: { supplierId: "supplier1", @@ -25,6 +26,11 @@ describe("createDependenciesContainer", () => { })), })); + // Repo client + jest.mock("@internal/datastore", () => ({ + SupplierConfigRepository: jest.fn(), + })); + // Env jest.mock("../env", () => ({ envVars: env })); }); @@ -32,12 +38,18 @@ describe("createDependenciesContainer", () => { test("constructs deps and wires repository config correctly", async () => { // get current mock instances const { createLogger } = jest.requireMock("@internal/helpers"); - + const { SupplierConfigRepository } = jest.requireMock( + "@internal/datastore", + ); // eslint-disable-next-line @typescript-eslint/no-require-imports const { createDependenciesContainer } = require("../deps"); const deps: Deps = createDependenciesContainer(); expect(createLogger).toHaveBeenCalledTimes(1); - + expect(SupplierConfigRepository).toHaveBeenCalledTimes(1); + const supplierConfigRepoCtorArgs = SupplierConfigRepository.mock.calls[0]; + expect(supplierConfigRepoCtorArgs[2]).toEqual({ + supplierConfigTableName: "SupplierConfigTable", + }); expect(deps.env).toEqual(env); }); }); diff --git a/lambdas/supplier-allocator/src/config/__tests__/env.test.ts b/lambdas/supplier-allocator/src/config/__tests__/env.test.ts index dd013aeb..1d8e3a1f 100644 --- a/lambdas/supplier-allocator/src/config/__tests__/env.test.ts +++ b/lambdas/supplier-allocator/src/config/__tests__/env.test.ts @@ -15,6 +15,7 @@ describe("lambdaEnv", () => { }); it("should load all environment variables successfully", () => { + process.env.SUPPLIER_CONFIG_TABLE_NAME = "SupplierConfigTable"; process.env.VARIANT_MAP = `{ "lv1": { "supplierId": "supplier1", @@ -25,6 +26,7 @@ describe("lambdaEnv", () => { const { envVars } = require("../env"); expect(envVars).toEqual({ + SUPPLIER_CONFIG_TABLE_NAME: "SupplierConfigTable", VARIANT_MAP: { lv1: { supplierId: "supplier1", diff --git a/lambdas/supplier-allocator/src/config/deps.ts b/lambdas/supplier-allocator/src/config/deps.ts index 1a5f6ce5..5758b6df 100644 --- a/lambdas/supplier-allocator/src/config/deps.ts +++ b/lambdas/supplier-allocator/src/config/deps.ts @@ -1,18 +1,40 @@ +import { DynamoDBClient } from "@aws-sdk/client-dynamodb"; +import { DynamoDBDocumentClient } from "@aws-sdk/lib-dynamodb"; import { SQSClient } from "@aws-sdk/client-sqs"; import { Logger } from "pino"; import { createLogger } from "@internal/helpers"; +import { SupplierConfigRepository } from "@internal/datastore"; import { EnvVars, envVars } from "./env"; export type Deps = { + supplierConfigRepo: SupplierConfigRepository; logger: Logger; env: EnvVars; sqsClient: SQSClient; }; +function createDocumentClient(): DynamoDBDocumentClient { + const ddbClient = new DynamoDBClient({}); + return DynamoDBDocumentClient.from(ddbClient); +} + +function createSupplierConfigRepository( + log: Logger, + // eslint-disable-next-line @typescript-eslint/no-shadow + envVars: EnvVars, +): SupplierConfigRepository { + const config = { + supplierConfigTableName: envVars.SUPPLIER_CONFIG_TABLE_NAME, + }; + + return new SupplierConfigRepository(createDocumentClient(), log, config); +} + export function createDependenciesContainer(): Deps { const log = createLogger({ logLevel: envVars.PINO_LOG_LEVEL }); return { + supplierConfigRepo: createSupplierConfigRepository(log, envVars), logger: log, env: envVars, sqsClient: new SQSClient({}), diff --git a/lambdas/supplier-allocator/src/config/env.ts b/lambdas/supplier-allocator/src/config/env.ts index 0adc3920..5d924430 100644 --- a/lambdas/supplier-allocator/src/config/env.ts +++ b/lambdas/supplier-allocator/src/config/env.ts @@ -10,6 +10,7 @@ const LetterVariantSchema = z.record( export type LetterVariant = z.infer; const EnvVarsSchema = z.object({ + SUPPLIER_CONFIG_TABLE_NAME: z.string(), PINO_LOG_LEVEL: z.coerce.string().optional(), VARIANT_MAP: z.string().transform((str, _) => { const parsed = JSON.parse(str); diff --git a/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts b/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts index 23fc5981..6ccf00ec 100644 --- a/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts +++ b/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts @@ -7,6 +7,7 @@ import { $LetterEvent, LetterEvent, } from "@nhsdigital/nhs-notify-event-schemas-supplier-api/src/events/letter-events"; +import { SupplierConfigRepository } from "@internal/datastore"; import createSupplierAllocatorHandler from "../allocate-handler"; import { Deps } from "../../config/deps"; import { EnvVars } from "../../config/env"; @@ -139,8 +140,16 @@ describe("createSupplierAllocatorHandler", () => { } as unknown as jest.Mocked; mockedDeps = { + supplierConfigRepo: { + getLetterVariant: jest.fn().mockResolvedValue({ + id: "variant1", + supplierId: "supplier1", + specId: "spec1", + }), + } as unknown as SupplierConfigRepository, logger: { error: jest.fn(), info: jest.fn() } as unknown as pino.Logger, env: { + SUPPLIER_CONFIG_TABLE_NAME: "SupplierConfigTable", VARIANT_MAP: { lv1: { supplierId: "supplier1", diff --git a/lambdas/supplier-allocator/src/handler/allocate-handler.ts b/lambdas/supplier-allocator/src/handler/allocate-handler.ts index 0402e284..e00d3fb7 100644 --- a/lambdas/supplier-allocator/src/handler/allocate-handler.ts +++ b/lambdas/supplier-allocator/src/handler/allocate-handler.ts @@ -1,7 +1,7 @@ import { SQSBatchItemFailure, SQSEvent, SQSHandler } from "aws-lambda"; import { SendMessageCommand } from "@aws-sdk/client-sqs"; import { LetterRequestPreparedEvent } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering-v1"; - +import { LetterVariant } from "internal/datastore/src/SupplierConfigDomain"; import { LetterRequestPreparedEventV2 } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering"; import z from "zod"; import { Deps } from "../config/deps"; @@ -46,7 +46,23 @@ function validateType(event: unknown) { } } +async function getVariantDetails(variantId: string, deps: Deps) { + deps.logger.info({ + description: "Fetching letter variant details from database", + variantId, + }); + + const variantDetails: LetterVariant = + await deps.supplierConfigRepo.getLetterVariant(variantId); + deps.logger.info({ + description: "Fetched letter variant details", + variantId, + variantDetails, + }); +} + function getSupplier(letterEvent: PreparedEvents, deps: Deps): SupplierSpec { + getVariantDetails(letterEvent.data.letterVariantId, deps); return resolveSupplierForVariant(letterEvent.data.letterVariantId, deps); } From 651f80e3249fa119236a49459e4f91b519ed70e7 Mon Sep 17 00:00:00 2001 From: David Wass Date: Mon, 23 Feb 2026 15:46:20 +0000 Subject: [PATCH 03/24] get rest of dbdata --- .../api/ddb_table_supplier_configuration.tf | 12 +++ .../src/supplier-config-repository.ts | 50 +++++++++++- .../__tests__/allocate-handler.test.ts | 2 +- .../src/handler/allocate-handler.ts | 81 +++++++++++++++++-- 4 files changed, 137 insertions(+), 8 deletions(-) diff --git a/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf b/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf index 5e228f30..b3d8b462 100644 --- a/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf +++ b/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf @@ -24,6 +24,11 @@ resource "aws_dynamodb_table" "supplier-configuration" { name = "entityType" type = "S" } + + attribute { + name = "volumeGroup" + type = "S" + } // The type-index GSI allows us to query for all supplier configurations of a given type (e.g. all letter supplier configurations) global_secondary_index { name = "EntityTypeIndex" @@ -32,6 +37,13 @@ resource "aws_dynamodb_table" "supplier-configuration" { projection_type = "ALL" } + global_secondary_index { + name = "volumeGroupIndex" + hash_key = "PK" + range_key = "volumeGroup" + projection_type = "ALL" + } + point_in_time_recovery { enabled = true } diff --git a/internal/datastore/src/supplier-config-repository.ts b/internal/datastore/src/supplier-config-repository.ts index 73b247e2..ce78759c 100644 --- a/internal/datastore/src/supplier-config-repository.ts +++ b/internal/datastore/src/supplier-config-repository.ts @@ -1,6 +1,17 @@ -import { DynamoDBDocumentClient, GetCommand } from "@aws-sdk/lib-dynamodb"; +import { + DynamoDBDocumentClient, + GetCommand, + QueryCommand, +} from "@aws-sdk/lib-dynamodb"; import { Logger } from "pino"; -import { $LetterVariant, LetterVariant } from "./SupplierConfigDomain"; +import { + $LetterVariant, + LetterVariant, + $VolumeGroup, + VolumeGroup, + $SupplierAllocation, + SupplierAllocation, +} from "./SupplierConfigDomain"; export type SupplierConfigRepositoryConfig = { supplierConfigTableName: string; @@ -25,4 +36,39 @@ export class SupplierConfigRepository { } return $LetterVariant.parse(result.Item); } + + async getVolumeGroup(groupId: string): Promise { + const result = await this.ddbClient.send( + new GetCommand({ + TableName: this.config.supplierConfigTableName, + Key: { PK: "VOLUME_GROUP", SK: groupId }, + }), + ); + if (!result.Item) { + throw new Error(`Volume group with id ${groupId} not found`); + } + return $VolumeGroup.parse(result.Item); + } + + async getSupplierAllocationsForVolumeGroup( + groupId: string, + ): Promise { + const result = await this.ddbClient.send( + new QueryCommand({ + TableName: this.config.supplierConfigTableName, + IndexName: "VolumeGroupIndex", + KeyConditionExpression: "PK = :pk AND volumeGroup = :groupId", + ExpressionAttributeValues: { + ":pk": "SUPPLIER_ALLOCATIONS", + ":groupId": groupId, + }, + }), + ); + if (!result.Item) { + throw new Error( + `Supplier allocations for volume group with id ${groupId} not found`, + ); + } + return $SupplierAllocation.array().parse(result.Item.items); + } } diff --git a/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts b/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts index 6ccf00ec..33acc25d 100644 --- a/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts +++ b/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts @@ -130,7 +130,7 @@ function createSupplierStatusChangeEvent( }); } -describe("createSupplierAllocatorHandler", () => { +describe.skip("createSupplierAllocatorHandler", () => { let mockSqsClient: jest.Mocked; let mockedDeps: jest.Mocked; diff --git a/lambdas/supplier-allocator/src/handler/allocate-handler.ts b/lambdas/supplier-allocator/src/handler/allocate-handler.ts index e00d3fb7..1802c14c 100644 --- a/lambdas/supplier-allocator/src/handler/allocate-handler.ts +++ b/lambdas/supplier-allocator/src/handler/allocate-handler.ts @@ -1,7 +1,11 @@ import { SQSBatchItemFailure, SQSEvent, SQSHandler } from "aws-lambda"; import { SendMessageCommand } from "@aws-sdk/client-sqs"; import { LetterRequestPreparedEvent } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering-v1"; -import { LetterVariant } from "internal/datastore/src/SupplierConfigDomain"; +import { + LetterVariant, + SupplierAllocation, + VolumeGroup, +} from "internal/datastore/src/SupplierConfigDomain"; import { LetterRequestPreparedEventV2 } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering"; import z from "zod"; import { Deps } from "../config/deps"; @@ -46,7 +50,10 @@ function validateType(event: unknown) { } } -async function getVariantDetails(variantId: string, deps: Deps) { +async function getVariantDetails( + variantId: string, + deps: Deps, +): Promise { deps.logger.info({ description: "Fetching letter variant details from database", variantId, @@ -54,15 +61,77 @@ async function getVariantDetails(variantId: string, deps: Deps) { const variantDetails: LetterVariant = await deps.supplierConfigRepo.getLetterVariant(variantId); + + if (variantDetails) { + deps.logger.info({ + description: "Fetched letter variant details", + variantId, + variantDetails, + }); + } else { + deps.logger.error({ + description: "No letter variant found for id", + variantId, + }); + } + return variantDetails; +} + +async function getVolumeGroupDetails( + groupId: string, + deps: Deps, +): Promise { deps.logger.info({ - description: "Fetched letter variant details", - variantId, + description: "Fetching volume group details from database", + groupId, + }); + + const groupDetails = await deps.supplierConfigRepo.getVolumeGroup(groupId); + if (groupDetails) { + deps.logger.info({ + description: "Fetched volume group details", + groupId, + groupDetails, + }); + } else { + deps.logger.error({ + description: "No volume group found for id", + groupId, + }); + } + return groupDetails; +} + +async function getSupplierFromConfig(letterEvent: PreparedEvents, deps: Deps) { + const variantDetails: LetterVariant = await getVariantDetails( + letterEvent.data.letterVariantId, + deps, + ); + deps.logger.info({ + description: "Fetched variant details for letter variant", variantDetails, }); + + const volumeGroupDetails: VolumeGroup = await getVolumeGroupDetails( + variantDetails.volumeGroupId, + deps, + ); + deps.logger.info({ + description: "Fetched volume group details for letter variant", + volumeGroupDetails, + }); + + const supplierAllocations: SupplierAllocation[] = + await deps.supplierConfigRepo.getSupplierAllocationsForVolumeGroup( + variantDetails.volumeGroupId, + ); + deps.logger.info({ + description: "Fetched supplier allocations for volume group", + supplierAllocations, + }); } function getSupplier(letterEvent: PreparedEvents, deps: Deps): SupplierSpec { - getVariantDetails(letterEvent.data.letterVariantId, deps); return resolveSupplierForVariant(letterEvent.data.letterVariantId, deps); } @@ -81,6 +150,8 @@ export default function createSupplierAllocatorHandler(deps: Deps): SQSHandler { validateType(letterEvent); + await getSupplierFromConfig(letterEvent as PreparedEvents, deps); + const supplierSpec = getSupplier(letterEvent as PreparedEvents, deps); deps.logger.info({ From 3cf12dca60a9241d922f4387726359b93374a5af Mon Sep 17 00:00:00 2001 From: David Wass Date: Mon, 23 Feb 2026 16:26:30 +0000 Subject: [PATCH 04/24] fix typescript --- internal/datastore/src/supplier-config-repository.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/datastore/src/supplier-config-repository.ts b/internal/datastore/src/supplier-config-repository.ts index ce78759c..7d4ba16c 100644 --- a/internal/datastore/src/supplier-config-repository.ts +++ b/internal/datastore/src/supplier-config-repository.ts @@ -6,11 +6,11 @@ import { import { Logger } from "pino"; import { $LetterVariant, - LetterVariant, - $VolumeGroup, - VolumeGroup, $SupplierAllocation, + $VolumeGroup, + LetterVariant, SupplierAllocation, + VolumeGroup, } from "./SupplierConfigDomain"; export type SupplierConfigRepositoryConfig = { @@ -64,11 +64,11 @@ export class SupplierConfigRepository { }, }), ); - if (!result.Item) { + if (!result.Items) { throw new Error( - `Supplier allocations for volume group with id ${groupId} not found`, + `No supplier allocations found for volume group id ${groupId}`, ); } - return $SupplierAllocation.array().parse(result.Item.items); + return $SupplierAllocation.array().parse(result.Items); } } From 1c3c32bff195f5b7bbabe499eb4d104e0ff432b4 Mon Sep 17 00:00:00 2001 From: David Wass Date: Tue, 24 Feb 2026 08:47:48 +0000 Subject: [PATCH 05/24] index rename and permissions --- .../components/api/ddb_table_supplier_configuration.tf | 2 +- .../components/api/module_lambda_supplier_allocator.tf | 4 +++- internal/datastore/src/supplier-config-repository.ts | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf b/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf index b3d8b462..9618cfd8 100644 --- a/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf +++ b/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf @@ -38,7 +38,7 @@ resource "aws_dynamodb_table" "supplier-configuration" { } global_secondary_index { - name = "volumeGroupIndex" + name = "volumeGroup-index" hash_key = "PK" range_key = "volumeGroup" projection_type = "ALL" diff --git a/infrastructure/terraform/components/api/module_lambda_supplier_allocator.tf b/infrastructure/terraform/components/api/module_lambda_supplier_allocator.tf index b8f073a0..b568307c 100644 --- a/infrastructure/terraform/components/api/module_lambda_supplier_allocator.tf +++ b/infrastructure/terraform/components/api/module_lambda_supplier_allocator.tf @@ -93,7 +93,9 @@ data "aws_iam_policy_document" "supplier_allocator_lambda" { ] resources = [ - aws_dynamodb_table.supplier-configuration.arn + aws_dynamodb_table.supplier-configuration.arn, + "${aws_dynamodb_table.supplier-configuration.arn}/index/volumeGroup-index" + ] } } diff --git a/internal/datastore/src/supplier-config-repository.ts b/internal/datastore/src/supplier-config-repository.ts index 7d4ba16c..a1624a24 100644 --- a/internal/datastore/src/supplier-config-repository.ts +++ b/internal/datastore/src/supplier-config-repository.ts @@ -56,7 +56,7 @@ export class SupplierConfigRepository { const result = await this.ddbClient.send( new QueryCommand({ TableName: this.config.supplierConfigTableName, - IndexName: "VolumeGroupIndex", + IndexName: "volumeGroup-index", KeyConditionExpression: "PK = :pk AND volumeGroup = :groupId", ExpressionAttributeValues: { ":pk": "SUPPLIER_ALLOCATIONS", From 5b6d30e9b9e8458362dc16e9fb9631389d0c803b Mon Sep 17 00:00:00 2001 From: David Wass Date: Tue, 24 Feb 2026 09:57:38 +0000 Subject: [PATCH 06/24] repo logging --- .../api/ddb_table_supplier_configuration.tf | 1 + internal/datastore/src/supplier-config-repository.ts | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf b/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf index 9618cfd8..8d089a19 100644 --- a/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf +++ b/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf @@ -29,6 +29,7 @@ resource "aws_dynamodb_table" "supplier-configuration" { name = "volumeGroup" type = "S" } + // The type-index GSI allows us to query for all supplier configurations of a given type (e.g. all letter supplier configurations) global_secondary_index { name = "EntityTypeIndex" diff --git a/internal/datastore/src/supplier-config-repository.ts b/internal/datastore/src/supplier-config-repository.ts index a1624a24..84a3f5ec 100644 --- a/internal/datastore/src/supplier-config-repository.ts +++ b/internal/datastore/src/supplier-config-repository.ts @@ -53,6 +53,11 @@ export class SupplierConfigRepository { async getSupplierAllocationsForVolumeGroup( groupId: string, ): Promise { + this.log.info({ + description: + "Fetching supplier allocations for volume group from database", + groupId, + }); const result = await this.ddbClient.send( new QueryCommand({ TableName: this.config.supplierConfigTableName, @@ -64,6 +69,12 @@ export class SupplierConfigRepository { }, }), ); + this.log.info({ + description: + "Fetched supplier allocations for volume group from database", + groupId, + count: result.Items?.length ?? 0, + }); if (!result.Items) { throw new Error( `No supplier allocations found for volume group id ${groupId}`, From 636862aa03e7899944323377ba43daabf4e5618b Mon Sep 17 00:00:00 2001 From: David Wass Date: Tue, 24 Feb 2026 11:05:39 +0000 Subject: [PATCH 07/24] Fix PK name --- internal/datastore/src/supplier-config-repository.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/datastore/src/supplier-config-repository.ts b/internal/datastore/src/supplier-config-repository.ts index 84a3f5ec..62231cc7 100644 --- a/internal/datastore/src/supplier-config-repository.ts +++ b/internal/datastore/src/supplier-config-repository.ts @@ -62,9 +62,13 @@ export class SupplierConfigRepository { new QueryCommand({ TableName: this.config.supplierConfigTableName, IndexName: "volumeGroup-index", - KeyConditionExpression: "PK = :pk AND volumeGroup = :groupId", + KeyConditionExpression: "#pk = :pk AND #group = :groupId", + ExpressionAttributeNames: { + "#pk": "PK", // make sure this is the GSI's PK attribute name + "#group": "volumeGroup", // <-- use the **actual** GSI key name + }, ExpressionAttributeValues: { - ":pk": "SUPPLIER_ALLOCATIONS", + ":pk": "SUPPLIER_ALLOCATION", ":groupId": groupId, }, }), From 04ef19b4735305a5081c5936926a551e3a3601fa Mon Sep 17 00:00:00 2001 From: David Wass Date: Tue, 24 Feb 2026 13:44:15 +0000 Subject: [PATCH 08/24] add filters and error checking --- .../src/supplier-config-repository.ts | 9 ++- .../src/handler/allocate-handler.ts | 66 ++++++++++++++++++- 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/internal/datastore/src/supplier-config-repository.ts b/internal/datastore/src/supplier-config-repository.ts index 62231cc7..94dba22b 100644 --- a/internal/datastore/src/supplier-config-repository.ts +++ b/internal/datastore/src/supplier-config-repository.ts @@ -47,6 +47,7 @@ export class SupplierConfigRepository { if (!result.Item) { throw new Error(`Volume group with id ${groupId} not found`); } + return $VolumeGroup.parse(result.Item); } @@ -63,13 +64,16 @@ export class SupplierConfigRepository { TableName: this.config.supplierConfigTableName, IndexName: "volumeGroup-index", KeyConditionExpression: "#pk = :pk AND #group = :groupId", + FilterExpression: "#status = :status ", ExpressionAttributeNames: { - "#pk": "PK", // make sure this is the GSI's PK attribute name - "#group": "volumeGroup", // <-- use the **actual** GSI key name + "#pk": "PK", + "#group": "volumeGroup", + "#status": "status", }, ExpressionAttributeValues: { ":pk": "SUPPLIER_ALLOCATION", ":groupId": groupId, + ":status": "PROD", }, }), ); @@ -84,6 +88,7 @@ export class SupplierConfigRepository { `No supplier allocations found for volume group id ${groupId}`, ); } + return $SupplierAllocation.array().parse(result.Items); } } diff --git a/lambdas/supplier-allocator/src/handler/allocate-handler.ts b/lambdas/supplier-allocator/src/handler/allocate-handler.ts index 1802c14c..f753a40f 100644 --- a/lambdas/supplier-allocator/src/handler/allocate-handler.ts +++ b/lambdas/supplier-allocator/src/handler/allocate-handler.ts @@ -99,9 +99,71 @@ async function getVolumeGroupDetails( groupId, }); } + + if ( + groupDetails.status !== "PROD" && + (new Date(groupDetails.startDate) > new Date() || + (groupDetails.endDate && new Date(groupDetails.endDate) < new Date())) + ) { + deps.logger.error({ + description: "Volume group is not active based on status and dates", + groupId, + status: groupDetails.status, + startDate: groupDetails.startDate, + endDate: groupDetails.endDate, + }); + throw new Error(`Volume group with id ${groupId} is not active`); + } return groupDetails; } +async function getSupplierAllocationsForVolumeGroup( + groupId: string, + supplierId: string, + deps: Deps, +): Promise { + deps.logger.info({ + description: "Fetching supplier allocations for volume group from database", + groupId, + }); + const allocations = + await deps.supplierConfigRepo.getSupplierAllocationsForVolumeGroup(groupId); + + if (allocations.length > 0) { + deps.logger.info({ + description: + "Fetched supplier allocations for volume group from database", + groupId, + count: allocations.length, + }); + } else { + deps.logger.error({ + description: "No supplier allocations found for volume group id", + groupId, + }); + } + + if (supplierId) { + const filteredAllocations = allocations.filter( + (alloc) => alloc.supplier === supplierId, + ); + if (filteredAllocations.length === 0) { + deps.logger.error({ + description: + "No supplier allocations found for variantsupplier id in volume group", + groupId, + supplierId, + }); + throw new Error( + `No supplier allocations found for variant supplier id ${supplierId} in volume group ${groupId}`, + ); + } + return filteredAllocations; + } + + return allocations; +} + async function getSupplierFromConfig(letterEvent: PreparedEvents, deps: Deps) { const variantDetails: LetterVariant = await getVariantDetails( letterEvent.data.letterVariantId, @@ -122,8 +184,10 @@ async function getSupplierFromConfig(letterEvent: PreparedEvents, deps: Deps) { }); const supplierAllocations: SupplierAllocation[] = - await deps.supplierConfigRepo.getSupplierAllocationsForVolumeGroup( + await getSupplierAllocationsForVolumeGroup( variantDetails.volumeGroupId, + variantDetails.supplierId ?? "", + deps, ); deps.logger.info({ description: "Fetched supplier allocations for volume group", From 33783ccbfb151f1df507ed876c30378f9986f891 Mon Sep 17 00:00:00 2001 From: David Wass Date: Tue, 24 Feb 2026 14:45:29 +0000 Subject: [PATCH 09/24] refactor to supplier-config services --- .../src/handler/allocate-handler.ts | 119 +---------------- .../src/services/supplier-config.ts | 120 ++++++++++++++++++ 2 files changed, 125 insertions(+), 114 deletions(-) create mode 100644 lambdas/supplier-allocator/src/services/supplier-config.ts diff --git a/lambdas/supplier-allocator/src/handler/allocate-handler.ts b/lambdas/supplier-allocator/src/handler/allocate-handler.ts index f753a40f..63d09c63 100644 --- a/lambdas/supplier-allocator/src/handler/allocate-handler.ts +++ b/lambdas/supplier-allocator/src/handler/allocate-handler.ts @@ -8,6 +8,11 @@ import { } from "internal/datastore/src/SupplierConfigDomain"; import { LetterRequestPreparedEventV2 } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering"; import z from "zod"; +import { + getSupplierAllocationsForVolumeGroup, + getVariantDetails, + getVolumeGroupDetails, +} from "../services/supplier-config"; import { Deps } from "../config/deps"; type SupplierSpec = { supplierId: string; specId: string }; @@ -50,120 +55,6 @@ function validateType(event: unknown) { } } -async function getVariantDetails( - variantId: string, - deps: Deps, -): Promise { - deps.logger.info({ - description: "Fetching letter variant details from database", - variantId, - }); - - const variantDetails: LetterVariant = - await deps.supplierConfigRepo.getLetterVariant(variantId); - - if (variantDetails) { - deps.logger.info({ - description: "Fetched letter variant details", - variantId, - variantDetails, - }); - } else { - deps.logger.error({ - description: "No letter variant found for id", - variantId, - }); - } - return variantDetails; -} - -async function getVolumeGroupDetails( - groupId: string, - deps: Deps, -): Promise { - deps.logger.info({ - description: "Fetching volume group details from database", - groupId, - }); - - const groupDetails = await deps.supplierConfigRepo.getVolumeGroup(groupId); - if (groupDetails) { - deps.logger.info({ - description: "Fetched volume group details", - groupId, - groupDetails, - }); - } else { - deps.logger.error({ - description: "No volume group found for id", - groupId, - }); - } - - if ( - groupDetails.status !== "PROD" && - (new Date(groupDetails.startDate) > new Date() || - (groupDetails.endDate && new Date(groupDetails.endDate) < new Date())) - ) { - deps.logger.error({ - description: "Volume group is not active based on status and dates", - groupId, - status: groupDetails.status, - startDate: groupDetails.startDate, - endDate: groupDetails.endDate, - }); - throw new Error(`Volume group with id ${groupId} is not active`); - } - return groupDetails; -} - -async function getSupplierAllocationsForVolumeGroup( - groupId: string, - supplierId: string, - deps: Deps, -): Promise { - deps.logger.info({ - description: "Fetching supplier allocations for volume group from database", - groupId, - }); - const allocations = - await deps.supplierConfigRepo.getSupplierAllocationsForVolumeGroup(groupId); - - if (allocations.length > 0) { - deps.logger.info({ - description: - "Fetched supplier allocations for volume group from database", - groupId, - count: allocations.length, - }); - } else { - deps.logger.error({ - description: "No supplier allocations found for volume group id", - groupId, - }); - } - - if (supplierId) { - const filteredAllocations = allocations.filter( - (alloc) => alloc.supplier === supplierId, - ); - if (filteredAllocations.length === 0) { - deps.logger.error({ - description: - "No supplier allocations found for variantsupplier id in volume group", - groupId, - supplierId, - }); - throw new Error( - `No supplier allocations found for variant supplier id ${supplierId} in volume group ${groupId}`, - ); - } - return filteredAllocations; - } - - return allocations; -} - async function getSupplierFromConfig(letterEvent: PreparedEvents, deps: Deps) { const variantDetails: LetterVariant = await getVariantDetails( letterEvent.data.letterVariantId, diff --git a/lambdas/supplier-allocator/src/services/supplier-config.ts b/lambdas/supplier-allocator/src/services/supplier-config.ts new file mode 100644 index 00000000..e9d680c1 --- /dev/null +++ b/lambdas/supplier-allocator/src/services/supplier-config.ts @@ -0,0 +1,120 @@ +import { + LetterVariant, + SupplierAllocation, + VolumeGroup, +} from "internal/datastore/src/SupplierConfigDomain"; +import { Deps } from "../config/deps"; + +export async function getVariantDetails( + variantId: string, + deps: Deps, +): Promise { + deps.logger.info({ + description: "Fetching letter variant details from database", + variantId, + }); + + const variantDetails: LetterVariant = + await deps.supplierConfigRepo.getLetterVariant(variantId); + + if (variantDetails) { + deps.logger.info({ + description: "Fetched letter variant details", + variantId, + variantDetails, + }); + } else { + deps.logger.error({ + description: "No letter variant found for id", + variantId, + }); + } + return variantDetails; +} + +export async function getVolumeGroupDetails( + groupId: string, + deps: Deps, +): Promise { + deps.logger.info({ + description: "Fetching volume group details from database", + groupId, + }); + + const groupDetails = await deps.supplierConfigRepo.getVolumeGroup(groupId); + if (groupDetails) { + deps.logger.info({ + description: "Fetched volume group details", + groupId, + groupDetails, + }); + } else { + deps.logger.error({ + description: "No volume group found for id", + groupId, + }); + } + + if ( + groupDetails.status !== "PROD" && + (new Date(groupDetails.startDate) > new Date() || + (groupDetails.endDate && new Date(groupDetails.endDate) < new Date())) + ) { + deps.logger.error({ + description: "Volume group is not active based on status and dates", + groupId, + status: groupDetails.status, + startDate: groupDetails.startDate, + endDate: groupDetails.endDate, + }); + throw new Error(`Volume group with id ${groupId} is not active`); + } + return groupDetails; +} + +export async function getSupplierAllocationsForVolumeGroup( + groupId: string, + supplierId: string, + deps: Deps, +): Promise { + deps.logger.info({ + description: "Fetching supplier allocations for volume group from database", + groupId, + }); + const allocations = + await deps.supplierConfigRepo.getSupplierAllocationsForVolumeGroup(groupId); + + if (allocations.length > 0) { + deps.logger.info({ + description: + "Fetched supplier allocations for volume group from database", + groupId, + count: allocations.length, + }); + } else { + deps.logger.error({ + description: "No supplier allocations found for volume group id", + groupId, + }); + } + + if (supplierId) { + const filteredAllocations = allocations.filter( + (alloc) => alloc.supplier === supplierId, + ); + if (filteredAllocations.length === 0) { + deps.logger.error({ + description: + "No supplier allocations found for variantsupplier id in volume group", + groupId, + supplierId, + }); + throw new Error( + `No supplier allocations found for variant supplier id ${supplierId} in volume group ${groupId}`, + ); + } + return filteredAllocations; + } + + return allocations; +} From d2afc98a28195e20ffce9e5818a720cc7e3cc881 Mon Sep 17 00:00:00 2001 From: David Wass Date: Tue, 24 Feb 2026 15:27:34 +0000 Subject: [PATCH 10/24] finally get the pool of supplier details --- .../src/supplier-config-repository.ts | 19 +++++++++++++ .../src/handler/allocate-handler.ts | 9 ++++++ .../src/services/supplier-config.ts | 28 +++++++++++++++++++ 3 files changed, 56 insertions(+) diff --git a/internal/datastore/src/supplier-config-repository.ts b/internal/datastore/src/supplier-config-repository.ts index 94dba22b..8663bd00 100644 --- a/internal/datastore/src/supplier-config-repository.ts +++ b/internal/datastore/src/supplier-config-repository.ts @@ -6,9 +6,11 @@ import { import { Logger } from "pino"; import { $LetterVariant, + $Supplier, $SupplierAllocation, $VolumeGroup, LetterVariant, + Supplier, SupplierAllocation, VolumeGroup, } from "./SupplierConfigDomain"; @@ -91,4 +93,21 @@ export class SupplierConfigRepository { return $SupplierAllocation.array().parse(result.Items); } + + async getSuppliersDetails(supplierIds: string[]): Promise { + const suppliers: Supplier[] = []; + for (const supplierId of supplierIds) { + const result = await this.ddbClient.send( + new GetCommand({ + TableName: this.config.supplierConfigTableName, + Key: { PK: "SUPPLIER", SK: supplierId }, + }), + ); + if (!result.Item) { + throw new Error(`Supplier with id ${supplierId} not found`); + } + suppliers.push($Supplier.parse(result.Item)); + } + return suppliers; + } } diff --git a/lambdas/supplier-allocator/src/handler/allocate-handler.ts b/lambdas/supplier-allocator/src/handler/allocate-handler.ts index 63d09c63..373fffc5 100644 --- a/lambdas/supplier-allocator/src/handler/allocate-handler.ts +++ b/lambdas/supplier-allocator/src/handler/allocate-handler.ts @@ -10,6 +10,7 @@ import { LetterRequestPreparedEventV2 } from "@nhsdigital/nhs-notify-event-schem import z from "zod"; import { getSupplierAllocationsForVolumeGroup, + getSupplierDetails, getVariantDetails, getVolumeGroupDetails, } from "../services/supplier-config"; @@ -84,6 +85,14 @@ async function getSupplierFromConfig(letterEvent: PreparedEvents, deps: Deps) { description: "Fetched supplier allocations for volume group", supplierAllocations, }); + + const supplierDetails = await getSupplierDetails(supplierAllocations, deps); + deps.logger.info({ + description: "Fetched supplier details for supplier allocations", + supplierDetails, + }); + + return supplierDetails; } function getSupplier(letterEvent: PreparedEvents, deps: Deps): SupplierSpec { diff --git a/lambdas/supplier-allocator/src/services/supplier-config.ts b/lambdas/supplier-allocator/src/services/supplier-config.ts index e9d680c1..e92a5811 100644 --- a/lambdas/supplier-allocator/src/services/supplier-config.ts +++ b/lambdas/supplier-allocator/src/services/supplier-config.ts @@ -1,5 +1,6 @@ import { LetterVariant, + Supplier, SupplierAllocation, VolumeGroup, } from "internal/datastore/src/SupplierConfigDomain"; @@ -118,3 +119,30 @@ export async function getSupplierAllocationsForVolumeGroup( return allocations; } + +export async function getSupplierDetails( + supplierAllocations: SupplierAllocation[], + deps: Deps, +): Promise { + const supplierIds = supplierAllocations.map((alloc) => alloc.supplier); + + const supplierDetails: Supplier[] = + await deps.supplierConfigRepo.getSuppliersDetails(supplierIds); + + deps.logger.info({ + description: "Fetched supplier details for supplier allocations", + supplierIds, + supplierDetails, + }); + + if (Object.keys(supplierDetails).length === 0) { + deps.logger.error({ + description: "No supplier details found for supplier allocations", + supplierIds, + }); + throw new Error( + `No supplier details found for supplier ids ${supplierIds.join(", ")}`, + ); + } + return supplierDetails; +} From 77fa51889bf81ddd5395fd2c2c5247b232be5ca6 Mon Sep 17 00:00:00 2001 From: David Wass Date: Wed, 25 Feb 2026 13:16:41 +0000 Subject: [PATCH 11/24] import schemas from nhs-notify-event-schemas-supplier-config --- internal/datastore/package.json | 1 + .../src/supplier-config-repository.ts | 2 +- lambdas/supplier-allocator/package.json | 1 + .../src/handler/allocate-handler.ts | 2 +- .../__tests__/supplier-config.test.ts | 134 ++++++++++++++++++ .../src/services/supplier-config.ts | 2 +- package-lock.json | 122 +++++++++++----- 7 files changed, 228 insertions(+), 36 deletions(-) create mode 100644 lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts diff --git a/internal/datastore/package.json b/internal/datastore/package.json index 23531375..f3e56167 100644 --- a/internal/datastore/package.json +++ b/internal/datastore/package.json @@ -3,6 +3,7 @@ "@aws-sdk/client-dynamodb": "^3.984.0", "@aws-sdk/lib-dynamodb": "^3.984.0", "@internal/helpers": "*", + "@nhsdigital/nhs-notify-event-schemas-supplier-config": "^1.0.1", "pino": "^10.3.0", "zod": "^4.1.11", "zod-mermaid": "^1.0.9" diff --git a/internal/datastore/src/supplier-config-repository.ts b/internal/datastore/src/supplier-config-repository.ts index 8663bd00..53461d50 100644 --- a/internal/datastore/src/supplier-config-repository.ts +++ b/internal/datastore/src/supplier-config-repository.ts @@ -13,7 +13,7 @@ import { Supplier, SupplierAllocation, VolumeGroup, -} from "./SupplierConfigDomain"; +} from "@nhsdigital/nhs-notify-event-schemas-supplier-config"; export type SupplierConfigRepositoryConfig = { supplierConfigTableName: string; diff --git a/lambdas/supplier-allocator/package.json b/lambdas/supplier-allocator/package.json index 1b21d001..2ea6cb67 100644 --- a/lambdas/supplier-allocator/package.json +++ b/lambdas/supplier-allocator/package.json @@ -8,6 +8,7 @@ "@nhsdigital/nhs-notify-event-schemas-letter-rendering": "^2.0.1", "@nhsdigital/nhs-notify-event-schemas-letter-rendering-v1": "npm:@nhsdigital/nhs-notify-event-schemas-letter-rendering@^1.1.5", "@nhsdigital/nhs-notify-event-schemas-supplier-api": "^1.0.8", + "@nhsdigital/nhs-notify-event-schemas-supplier-config": "^1.0.1", "@types/aws-lambda": "^8.10.148", "aws-lambda": "^1.0.7", "esbuild": "^0.27.2", diff --git a/lambdas/supplier-allocator/src/handler/allocate-handler.ts b/lambdas/supplier-allocator/src/handler/allocate-handler.ts index 373fffc5..74df928f 100644 --- a/lambdas/supplier-allocator/src/handler/allocate-handler.ts +++ b/lambdas/supplier-allocator/src/handler/allocate-handler.ts @@ -5,7 +5,7 @@ import { LetterVariant, SupplierAllocation, VolumeGroup, -} from "internal/datastore/src/SupplierConfigDomain"; +} from "@nhsdigital/nhs-notify-event-schemas-supplier-config"; import { LetterRequestPreparedEventV2 } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering"; import z from "zod"; import { diff --git a/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts b/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts new file mode 100644 index 00000000..17c46023 --- /dev/null +++ b/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts @@ -0,0 +1,134 @@ +import { + getSupplierAllocationsForVolumeGroup, + getVariantDetails, + getVolumeGroupDetails, +} from "../supplier-config"; +import { Deps } from "../../config/deps"; + +function makeDeps(overrides: Partial = {}): Deps { + const logger = { + info: jest.fn(), + error: jest.fn(), + } as unknown as Deps["logger"]; + + const supplierConfigRepo = { + getLetterVariant: jest.fn(), + getVolumeGroup: jest.fn(), + getSupplierAllocationsForVolumeGroup: jest.fn(), + } as unknown as Deps["supplierConfigRepo"]; + + const base: Partial = { + logger: logger as any, + supplierConfigRepo: supplierConfigRepo as any, + }; + + return { ...(base as Deps), ...overrides } as Deps; +} + +describe("supplier-config service", () => { + afterEach(() => jest.resetAllMocks()); + + describe("getVariantDetails", () => { + it("returns variant details and logs when found", async () => { + const variant = { id: "v1", volumeGroupId: "g1" } as any; + const deps = makeDeps(); + deps.supplierConfigRepo.getLetterVariant = jest + .fn() + .mockResolvedValue(variant); + + const result = await getVariantDetails("v1", deps); + + expect(result).toBe(variant); + expect(deps.logger.info).toHaveBeenCalled(); + }); + + it("logs an error and returns undefined when not found", async () => { + const deps = makeDeps(); + deps.supplierConfigRepo.getLetterVariant = jest.fn().mockResolvedValue(); + + const result = await getVariantDetails("missing", deps); + + expect(result).toBeUndefined(); + expect(deps.logger.error).toHaveBeenCalled(); + }); + }); + + describe("getVolumeGroupDetails", () => { + it("returns group details when active", async () => { + const group = { + id: "g1", + status: "PROD", + startDate: "2020-01-01", + } as any; + const deps = makeDeps(); + deps.supplierConfigRepo.getVolumeGroup = jest + .fn() + .mockResolvedValue(group); + + const result = await getVolumeGroupDetails("g1", deps); + + expect(result).toBe(group); + expect(deps.logger.info).toHaveBeenCalled(); + }); + + it("throws when group is not active based on dates/status", async () => { + const future = new Date(Date.now() + 1000 * 60 * 60 * 24).toISOString(); + const group = { id: "g2", status: "DRAFT", startDate: future } as any; + const deps = makeDeps(); + deps.supplierConfigRepo.getVolumeGroup = jest + .fn() + .mockResolvedValue(group); + + await expect(getVolumeGroupDetails("g2", deps)).rejects.toThrow( + /not active/, + ); + expect(deps.logger.error).toHaveBeenCalled(); + }); + }); + + describe("getSupplierAllocationsForVolumeGroup", () => { + const allocations = [ + { supplier: "s1", variantId: "v1" }, + { supplier: "s2", variantId: "v2" }, + ] as any[]; + + it("returns all allocations when no supplierId provided", async () => { + const deps = makeDeps(); + deps.supplierConfigRepo.getSupplierAllocationsForVolumeGroup = jest + .fn() + .mockResolvedValue(allocations); + + const result = await getSupplierAllocationsForVolumeGroup("g1", "", deps); + + expect(result).toEqual(allocations); + expect(deps.logger.info).toHaveBeenCalled(); + }); + + it("filters by supplierId when provided", async () => { + const deps = makeDeps(); + deps.supplierConfigRepo.getSupplierAllocationsForVolumeGroup = jest + .fn() + .mockResolvedValue(allocations); + + const result = await getSupplierAllocationsForVolumeGroup( + "g1", + "s2", + deps, + ); + + expect(result).toEqual([allocations[1]]); + }); + + it("throws when supplierId provided but no matching allocation", async () => { + const deps = makeDeps(); + deps.supplierConfigRepo.getSupplierAllocationsForVolumeGroup = jest + .fn() + .mockResolvedValue(allocations); + + await expect( + getSupplierAllocationsForVolumeGroup("g1", "missing", deps), + ).rejects.toThrow(/No supplier allocations found/); + expect(deps.logger.error).toHaveBeenCalled(); + }); + }); +}); diff --git a/lambdas/supplier-allocator/src/services/supplier-config.ts b/lambdas/supplier-allocator/src/services/supplier-config.ts index e92a5811..79f3dfea 100644 --- a/lambdas/supplier-allocator/src/services/supplier-config.ts +++ b/lambdas/supplier-allocator/src/services/supplier-config.ts @@ -3,7 +3,7 @@ import { Supplier, SupplierAllocation, VolumeGroup, -} from "internal/datastore/src/SupplierConfigDomain"; +} from "@nhsdigital/nhs-notify-event-schemas-supplier-config"; import { Deps } from "../config/deps"; export async function getVariantDetails( diff --git a/package-lock.json b/package-lock.json index 84ca6562..0b7ea9d4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -93,6 +93,7 @@ "@aws-sdk/client-dynamodb": "^3.984.0", "@aws-sdk/lib-dynamodb": "^3.984.0", "@internal/helpers": "*", + "@nhsdigital/nhs-notify-event-schemas-supplier-config": "^1.0.1", "pino": "^10.3.0", "zod": "^4.1.11", "zod-mermaid": "^1.0.9" @@ -294,6 +295,7 @@ "@nhsdigital/nhs-notify-event-schemas-letter-rendering": "^2.0.1", "@nhsdigital/nhs-notify-event-schemas-letter-rendering-v1": "npm:@nhsdigital/nhs-notify-event-schemas-letter-rendering@^1.1.5", "@nhsdigital/nhs-notify-event-schemas-supplier-api": "^1.0.8", + "@nhsdigital/nhs-notify-event-schemas-supplier-config": "^1.0.1", "@types/aws-lambda": "^8.10.148", "aws-lambda": "^1.0.7", "esbuild": "^0.27.2", @@ -2953,28 +2955,43 @@ "node": "^18.18.0 || ^20.9.0 || >=21.1.0" } }, + "node_modules/@eslint/config-array/node_modules/balanced-match": { + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", + "integrity": "sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==", + "dev": true, + "license": "MIT", + "engines": { + "node": "18 || 20 || >=22" + } + }, "node_modules/@eslint/config-array/node_modules/brace-expansion": { - "version": "1.1.12", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.12.tgz", - "integrity": "sha512-9T9UjW3r0UW5c1Q7GTwllptXwhvYmEzFhzMfZ9H7FQWt+uZePjZPjBP/W1ZEyZ1twGWom5/56TF4lPcqjnDHcg==", + "version": "5.0.4", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.4.tgz", + "integrity": "sha512-h+DEnpVvxmfVefa4jFbCf5HdH5YMDXRsmKflpf1pILZWRFlTbJpxeU55nJl4Smt5HQaGzg1o6RHFPJaOqnmBDg==", "dev": true, "license": "MIT", "dependencies": { - "balanced-match": "^1.0.0", - "concat-map": "0.0.1" + "balanced-match": "^4.0.2" + }, + "engines": { + "node": "18 || 20 || >=22" } }, "node_modules/@eslint/config-array/node_modules/minimatch": { - "version": "3.1.5", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.5.tgz", - "integrity": "sha512-VgjWUsnnT6n+NUk6eZq77zeFdpW2LWDzP6zFGrCbHXiYNul5Dzqk2HHQ5uFH2DNW5Xbp8+jVzaeNt94ssEEl4w==", + "version": "10.2.4", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.4.tgz", + "integrity": "sha512-oRjTw/97aTBN0RHbYCdtF1MQfvusSIBQM0IZEgzl6426+8jSC0nF1a/GmnVLpfB9yyr6g6FTqWqiZVbxrtaCIg==", "dev": true, - "license": "ISC", + "license": "BlueOak-1.0.0", "dependencies": { - "brace-expansion": "^1.1.7" + "brace-expansion": "^5.0.2" }, "engines": { - "node": "*" + "node": "18 || 20 || >=22" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" } }, "node_modules/@eslint/config-helpers": { @@ -3044,15 +3061,27 @@ "url": "https://github.com/sponsors/epoberezkin" } }, + "node_modules/@eslint/eslintrc/node_modules/balanced-match": { + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", + "integrity": "sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==", + "dev": true, + "license": "MIT", + "engines": { + "node": "18 || 20 || >=22" + } + }, "node_modules/@eslint/eslintrc/node_modules/brace-expansion": { - "version": "1.1.12", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.12.tgz", - "integrity": "sha512-9T9UjW3r0UW5c1Q7GTwllptXwhvYmEzFhzMfZ9H7FQWt+uZePjZPjBP/W1ZEyZ1twGWom5/56TF4lPcqjnDHcg==", + "version": "5.0.4", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.4.tgz", + "integrity": "sha512-h+DEnpVvxmfVefa4jFbCf5HdH5YMDXRsmKflpf1pILZWRFlTbJpxeU55nJl4Smt5HQaGzg1o6RHFPJaOqnmBDg==", "dev": true, "license": "MIT", "dependencies": { - "balanced-match": "^1.0.0", - "concat-map": "0.0.1" + "balanced-match": "^4.0.2" + }, + "engines": { + "node": "18 || 20 || >=22" } }, "node_modules/@eslint/eslintrc/node_modules/ignore": { @@ -3073,16 +3102,19 @@ "license": "MIT" }, "node_modules/@eslint/eslintrc/node_modules/minimatch": { - "version": "3.1.5", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.5.tgz", - "integrity": "sha512-VgjWUsnnT6n+NUk6eZq77zeFdpW2LWDzP6zFGrCbHXiYNul5Dzqk2HHQ5uFH2DNW5Xbp8+jVzaeNt94ssEEl4w==", + "version": "10.2.4", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.4.tgz", + "integrity": "sha512-oRjTw/97aTBN0RHbYCdtF1MQfvusSIBQM0IZEgzl6426+8jSC0nF1a/GmnVLpfB9yyr6g6FTqWqiZVbxrtaCIg==", "dev": true, - "license": "ISC", + "license": "BlueOak-1.0.0", "dependencies": { - "brace-expansion": "^1.1.7" + "brace-expansion": "^5.0.2" }, "engines": { - "node": "*" + "node": "18 || 20 || >=22" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" } }, "node_modules/@eslint/js": { @@ -4225,6 +4257,15 @@ "resolved": "internal/events", "link": true }, + "node_modules/@nhsdigital/nhs-notify-event-schemas-supplier-config": { + "version": "1.0.1", + "resolved": "https://npm.pkg.github.com/download/@nhsdigital/nhs-notify-event-schemas-supplier-config/1.0.1/ff1ce566201ae291825acd5e771537229d6aa9ca", + "integrity": "sha512-gIZgfzgvkCfZE+HCosrVJ3tBce2FJRGfwPmtYtZDBG+ox/KvbpJFWXzJ5Jkh/42YzcVn2GxT1fy1L1F6pxiYWA==", + "dependencies": { + "@asyncapi/bundler": "^0.6.4", + "zod": "^4.1.12" + } + }, "node_modules/@nhsdigital/notify-supplier-api-consumer-contracts": { "resolved": "pact-contracts", "link": true @@ -12800,15 +12841,27 @@ "url": "https://github.com/sponsors/epoberezkin" } }, + "node_modules/eslint/node_modules/balanced-match": { + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", + "integrity": "sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==", + "dev": true, + "license": "MIT", + "engines": { + "node": "18 || 20 || >=22" + } + }, "node_modules/eslint/node_modules/brace-expansion": { - "version": "1.1.12", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.12.tgz", - "integrity": "sha512-9T9UjW3r0UW5c1Q7GTwllptXwhvYmEzFhzMfZ9H7FQWt+uZePjZPjBP/W1ZEyZ1twGWom5/56TF4lPcqjnDHcg==", + "version": "5.0.4", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.4.tgz", + "integrity": "sha512-h+DEnpVvxmfVefa4jFbCf5HdH5YMDXRsmKflpf1pILZWRFlTbJpxeU55nJl4Smt5HQaGzg1o6RHFPJaOqnmBDg==", "dev": true, "license": "MIT", "dependencies": { - "balanced-match": "^1.0.0", - "concat-map": "0.0.1" + "balanced-match": "^4.0.2" + }, + "engines": { + "node": "18 || 20 || >=22" } }, "node_modules/eslint/node_modules/glob-parent": { @@ -12842,16 +12895,19 @@ "license": "MIT" }, "node_modules/eslint/node_modules/minimatch": { - "version": "3.1.5", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.5.tgz", - "integrity": "sha512-VgjWUsnnT6n+NUk6eZq77zeFdpW2LWDzP6zFGrCbHXiYNul5Dzqk2HHQ5uFH2DNW5Xbp8+jVzaeNt94ssEEl4w==", + "version": "10.2.4", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.4.tgz", + "integrity": "sha512-oRjTw/97aTBN0RHbYCdtF1MQfvusSIBQM0IZEgzl6426+8jSC0nF1a/GmnVLpfB9yyr6g6FTqWqiZVbxrtaCIg==", "dev": true, - "license": "ISC", + "license": "BlueOak-1.0.0", "dependencies": { - "brace-expansion": "^1.1.7" + "brace-expansion": "^5.0.2" }, "engines": { - "node": "*" + "node": "18 || 20 || >=22" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" } }, "node_modules/espree": { From 79959d7d3a57aef53014c999066f46ab0cd49e00 Mon Sep 17 00:00:00 2001 From: David Wass Date: Thu, 26 Feb 2026 09:58:32 +0000 Subject: [PATCH 12/24] new unit tests --- .../__tests__/allocate-handler.test.ts | 662 ++++++++---------- .../src/handler/allocate-handler.ts | 3 +- .../__tests__/supplier-config.test.ts | 136 +++- .../src/services/supplier-config.ts | 10 +- 4 files changed, 430 insertions(+), 381 deletions(-) diff --git a/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts b/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts index 33acc25d..9c33b3e0 100644 --- a/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts +++ b/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts @@ -1,447 +1,365 @@ -import { SQSEvent, SQSRecord } from "aws-lambda"; -import pino from "pino"; -import { SQSClient, SendMessageCommand } from "@aws-sdk/client-sqs"; +import { SQSBatchResponse, SQSEvent, SQSRecord } from "aws-lambda"; +import { SendMessageCommand } from "@aws-sdk/client-sqs"; import { LetterRequestPreparedEventV2 } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering"; -import { LetterRequestPreparedEvent } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering-v1"; -import { - $LetterEvent, - LetterEvent, -} from "@nhsdigital/nhs-notify-event-schemas-supplier-api/src/events/letter-events"; -import { SupplierConfigRepository } from "@internal/datastore"; import createSupplierAllocatorHandler from "../allocate-handler"; +import * as supplierConfig from "../../services/supplier-config"; import { Deps } from "../../config/deps"; -import { EnvVars } from "../../config/env"; -function createSQSEvent(records: SQSRecord[]): SQSEvent { - return { - Records: records, +jest.mock("@aws-sdk/client-sqs"); +jest.mock("../../services/supplier-config"); + +function makeDeps(overrides: Partial = {}): Deps { + const logger = { + info: jest.fn(), + error: jest.fn(), }; -} -function createSqsRecord(msgId: string, body: string): SQSRecord { - return { - messageId: msgId, - receiptHandle: "", - body, - attributes: { - ApproximateReceiveCount: "", - SentTimestamp: "", - SenderId: "", - ApproximateFirstReceiveTimestamp: "", - }, - messageAttributes: {}, - md5OfBody: "", - eventSource: "", - eventSourceARN: "", - awsRegion: "", + const sqsClient = { + send: jest.fn().mockResolvedValue({}), }; -} -function createPreparedV1Event( - overrides: Partial = {}, -): LetterRequestPreparedEvent { - const now = new Date().toISOString(); + const supplierConfigRepo = { + getLetterVariant: jest.fn(), + getVolumeGroup: jest.fn(), + getSupplierAllocationsForVolumeGroup: jest.fn(), + }; - return { - specversion: "1.0", - id: overrides.id ?? "7b9a03ca-342a-4150-b56b-989109c45613", - source: "/data-plane/letter-rendering/test", - subject: "client/client1/letter-request/letterRequest1", - type: "uk.nhs.notify.letter-rendering.letter-request.prepared.v1", - time: now, - dataschema: - "https://notify.nhs.uk/cloudevents/schemas/letter-rendering/letter-request.prepared.1.0.0.schema.json", - dataschemaversion: "1.0.0", - data: { - domainId: overrides.domainId ?? "letter1", - letterVariantId: "lv1", - requestId: "request1", - requestItemId: "requestItem1", - requestItemPlanId: "requestItemPlan1", - clientId: "client1", - campaignId: "campaign1", - templateId: "template1", - url: overrides.url ?? "s3://letterDataBucket/letter1.pdf", - sha256Hash: - "3a7bd3e2360a3d29eea436fcfb7e44c735d117c8f2f1d2d1e4f6e8f7e6e8f7e6", - createdAt: now, - pageCount: 1, - status: "PREPARED", + const env = { + VARIANT_MAP: { + "variant-1": { supplierId: "supplier-1", specId: "spec-1" }, + "variant-2": { supplierId: "supplier-2", specId: "spec-2" }, }, - traceparent: "00-0af7651916cd43dd8448eb211c803191-b7ad6b7169203331-01", - recordedtime: now, - severitynumber: 2, - severitytext: "INFO", - plane: "data", }; + + return { + logger: logger as any, + sqsClient: sqsClient as any, + supplierConfigRepo: supplierConfigRepo as any, + env: env as any, + ...overrides, + } as any; } -function createPreparedV2Event( - overrides: Partial = {}, +function makeLetterEventV2( + variantId = "variant-1", ): LetterRequestPreparedEventV2 { return { - ...createPreparedV1Event(overrides), type: "uk.nhs.notify.letter-rendering.letter-request.prepared.v2", - dataschema: - "https://notify.nhs.uk/cloudevents/schemas/letter-rendering/letter-request.prepared.2.0.1.schema.json", - dataschemaversion: "2.0.1", - }; + data: { + letterVariantId: variantId, + eventId: "event-123", + timestamp: new Date().toISOString(), + }, + } as any; } -function createSupplierStatusChangeEvent( - overrides: Partial = {}, -): LetterEvent { - const now = new Date().toISOString(); - - return $LetterEvent.parse({ - data: { - domainId: overrides.domainId ?? "f47ac10b-58cc-4372-a567-0e02b2c3d479", - groupId: "client_template", - origin: { - domain: "letter-rendering", - event: "f47ac10b-58cc-4372-a567-0e02b2c3d479", - source: "/data-plane/letter-rendering/prod/render-pdf", - subject: - "client/00f3b388-bbe9-41c9-9e76-052d37ee8988/letter-request/0o5Fs0EELR0fUjHjbCnEtdUwQe4_0o5Fs0EELR0fUjHjbCnEtdUwQe5", - }, - reasonCode: "R07", - reasonText: "No such address", - specificationId: "1y3q9v1zzzz", - billingRef: "1y3q9v1zzzz", - status: "RETURNED", - supplierId: "supplier1", +function makeSQSRecord(body: any): SQSRecord { + return { + messageId: "message-123", + receiptHandle: "receipt-handle-123", + body: JSON.stringify(body), + attributes: { + ApproximateReceiveCount: "1", + SentTimestamp: Date.now().toString(), + SenderId: "sender-123", + ApproximateFirstReceiveTimestamp: Date.now().toString(), }, - datacontenttype: "application/json", - dataschema: - "https://notify.nhs.uk/cloudevents/schemas/supplier-api/letter.RETURNED.1.0.0.schema.json", - dataschemaversion: "1.0.0", - id: overrides.id ?? "23f1f09c-a555-4d9b-8405-0b33490bc920", - plane: "data", - recordedtime: now, - severitynumber: 2, - severitytext: "INFO", - source: "/data-plane/supplier-api/prod/update-status", - specversion: "1.0", - subject: - "letter-origin/letter-rendering/letter/f47ac10b-58cc-4372-a567-0e02b2c3d479", - time: now, - traceparent: "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01", - type: "uk.nhs.notify.supplier-api.letter.RETURNED.v1", - }); + messageAttributes: {}, + md5OfBody: "md5", + eventSource: "aws:sqs", + eventSourceARN: "arn:aws:sqs:region:account:queue", + awsRegion: "us-east-1", + } as any; } -describe.skip("createSupplierAllocatorHandler", () => { - let mockSqsClient: jest.Mocked; - let mockedDeps: jest.Mocked; +function setupDefaultMocks() { + (supplierConfig.getVariantDetails as jest.Mock).mockResolvedValue({ + id: "v1", + volumeGroupId: "g1", + }); + (supplierConfig.getVolumeGroupDetails as jest.Mock).mockResolvedValue({ + id: "g1", + status: "PROD", + }); + ( + supplierConfig.getSupplierAllocationsForVolumeGroup as jest.Mock + ).mockResolvedValue([{ supplier: "s1" }]); + (supplierConfig.getSupplierDetails as jest.Mock).mockResolvedValue({ + supplierId: "supplier-1", + specId: "spec-1", + }); +} +describe("allocate-handler", () => { beforeEach(() => { - mockSqsClient = { - send: jest.fn(), - } as unknown as jest.Mocked; - - mockedDeps = { - supplierConfigRepo: { - getLetterVariant: jest.fn().mockResolvedValue({ - id: "variant1", - supplierId: "supplier1", - specId: "spec1", - }), - } as unknown as SupplierConfigRepository, - logger: { error: jest.fn(), info: jest.fn() } as unknown as pino.Logger, - env: { - SUPPLIER_CONFIG_TABLE_NAME: "SupplierConfigTable", - VARIANT_MAP: { - lv1: { - supplierId: "supplier1", - specId: "spec1", - }, - }, - } as EnvVars, - sqsClient: mockSqsClient, - } as jest.Mocked; - jest.clearAllMocks(); + process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.queue"; }); - test("parses SNS notification and sends message to SQS queue for v2 event", async () => { - const preparedEvent = createPreparedV2Event(); - const evt: SQSEvent = createSQSEvent([ - createSqsRecord("msg1", JSON.stringify(preparedEvent)), - ]); - - process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue"; - - const handler = createSupplierAllocatorHandler(mockedDeps); - const result = await handler(evt, {} as any, {} as any); - - expect(result).toBeDefined(); - if (!result) throw new Error("expected BatchResponse, got void"); - - expect(result.batchItemFailures).toHaveLength(0); - - expect(mockSqsClient.send).toHaveBeenCalledTimes(1); - const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0]; - expect(sendCall).toBeInstanceOf(SendMessageCommand); - - const messageBody = JSON.parse(sendCall.input.MessageBody); - expect(messageBody.letterEvent).toEqual(preparedEvent); - expect(messageBody.supplierSpec).toEqual({ - supplierId: "supplier1", - specId: "spec1", - }); + afterEach(() => { + delete process.env.UPSERT_LETTERS_QUEUE_URL; }); - test("parses SNS notification and sends message to SQS queue for v1 event", async () => { - const preparedEvent = createPreparedV1Event(); - - const evt: SQSEvent = createSQSEvent([ - createSqsRecord("msg1", JSON.stringify(preparedEvent)), - ]); - - process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue"; - - const handler = createSupplierAllocatorHandler(mockedDeps); - const result = await handler(evt, {} as any, {} as any); - - expect(result).toBeDefined(); - if (!result) throw new Error("expected BatchResponse, got void"); - - expect(result.batchItemFailures).toHaveLength(0); - - expect(mockSqsClient.send).toHaveBeenCalledTimes(1); - const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0]; - const messageBody = JSON.parse(sendCall.input.MessageBody); - expect(messageBody.supplierSpec).toEqual({ - supplierId: "supplier1", - specId: "spec1", + describe("createSupplierAllocatorHandler", () => { + it("creates a handler function", () => { + const deps = makeDeps(); + const handler = createSupplierAllocatorHandler(deps); + expect(handler).toBeInstanceOf(Function); }); }); - test("returns batch failure for Update event", async () => { - const preparedEvent = createSupplierStatusChangeEvent(); - - const evt: SQSEvent = createSQSEvent([ - createSqsRecord("invalid-event", JSON.stringify(preparedEvent)), - ]); - - process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue"; - - const handler = createSupplierAllocatorHandler(mockedDeps); - const result = await handler(evt, {} as any, {} as any); - - expect(result).toBeDefined(); - if (!result) throw new Error("expected BatchResponse, got void"); - - expect(result.batchItemFailures).toHaveLength(1); - expect(result.batchItemFailures[0].itemIdentifier).toBe("invalid-event"); - expect((mockedDeps.logger.error as jest.Mock).mock.calls).toHaveLength(1); - }); + describe("handler execution", () => { + it("processes single record successfully", async () => { + const deps = makeDeps(); + const letterEvent = makeLetterEventV2(); + + setupDefaultMocks(); + + const handler = createSupplierAllocatorHandler(deps); + const event: SQSEvent = { + Records: [makeSQSRecord(letterEvent)], + }; + + const result = (await handler( + event, + {} as any, + () => {}, + )) as SQSBatchResponse; + + expect(result.batchItemFailures).toHaveLength(0); + expect(deps.sqsClient.send).toHaveBeenCalledWith( + expect.any(SendMessageCommand), + ); + expect(deps.logger.info).toHaveBeenCalled(); + }); - test("unwraps EventBridge envelope and extracts event details", async () => { - const preparedEvent = createPreparedV2Event({ domainId: "letter-test" }); + it("processes multiple records", async () => { + const deps = makeDeps(); + const letterEvent1 = makeLetterEventV2("variant-1"); + const letterEvent2 = makeLetterEventV2("variant-2"); - const evt: SQSEvent = createSQSEvent([ - createSqsRecord("msg1", JSON.stringify(preparedEvent)), - ]); + setupDefaultMocks(); - process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue"; + const handler = createSupplierAllocatorHandler(deps); + const event: SQSEvent = { + Records: [makeSQSRecord(letterEvent1), makeSQSRecord(letterEvent2)], + }; - const handler = createSupplierAllocatorHandler(mockedDeps); - await handler(evt, {} as any, {} as any); + const result = (await handler( + event, + {} as any, + () => {}, + )) as SQSBatchResponse; - const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0]; - const messageBody = JSON.parse(sendCall.input.MessageBody); - expect(messageBody.letterEvent.data.domainId).toBe("letter-test"); - }); + expect(result.batchItemFailures).toHaveLength(0); + expect(deps.sqsClient.send).toHaveBeenCalledTimes(2); + }); - test("resolves correct supplier spec from variant map", async () => { - const preparedEvent = createPreparedV2Event(); + it("records failures for invalid JSON", async () => { + const deps = makeDeps(); + const handler = createSupplierAllocatorHandler(deps); - const evt: SQSEvent = createSQSEvent([ - createSqsRecord("msg1", JSON.stringify(preparedEvent)), - ]); + const badRecord = makeSQSRecord(""); + badRecord.body = "invalid json {"; - process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue"; + const event: SQSEvent = { + Records: [badRecord], + }; - const handler = createSupplierAllocatorHandler(mockedDeps); - await handler(evt, {} as any, {} as any); + const result = (await handler( + event, + {} as any, + () => {}, + )) as SQSBatchResponse; - const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0]; - const messageBody = JSON.parse(sendCall.input.MessageBody); - expect(messageBody.supplierSpec.supplierId).toBe("supplier1"); - expect(messageBody.supplierSpec.specId).toBe("spec1"); - }); + expect(result.batchItemFailures).toHaveLength(1); + expect(result.batchItemFailures[0].itemIdentifier).toBe("message-123"); + expect(deps.logger.error).toHaveBeenCalled(); + }); - test("processes multiple messages in batch", async () => { - const evt: SQSEvent = createSQSEvent([ - createSqsRecord( - "msg1", - JSON.stringify(createPreparedV2Event({ domainId: "letter1" })), - ), - createSqsRecord( - "msg2", - JSON.stringify(createPreparedV2Event({ domainId: "letter2" })), - ), - ]); + it("records failures for invalid event type", async () => { + const deps = makeDeps(); + const handler = createSupplierAllocatorHandler(deps); - process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue"; + const invalidEvent = { type: "invalid.type" }; + const event: SQSEvent = { + Records: [makeSQSRecord(invalidEvent)], + }; - const handler = createSupplierAllocatorHandler(mockedDeps); - const result = await handler(evt, {} as any, {} as any); + const result = (await handler( + event, + {} as any, + () => {}, + )) as SQSBatchResponse; - expect(result).toBeDefined(); - if (!result) throw new Error("expected BatchResponse, got void"); + expect(result.batchItemFailures).toHaveLength(1); + expect(deps.logger.error).toHaveBeenCalled(); + }); - expect(result.batchItemFailures).toHaveLength(0); - expect(mockSqsClient.send).toHaveBeenCalledTimes(2); - }); + it("records failures for missing envelope type", async () => { + const deps = makeDeps(); + const handler = createSupplierAllocatorHandler(deps); - test("returns batch failure for invalid JSON", async () => { - const evt: SQSEvent = createSQSEvent([ - createSqsRecord("bad-json", "this-is-not-json"), - ]); + const invalidEvent = { data: { letterVariantId: "v1" } }; + const event: SQSEvent = { + Records: [makeSQSRecord(invalidEvent)], + }; - process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue"; + const result = (await handler( + event, + {} as any, + () => {}, + )) as SQSBatchResponse; - const handler = createSupplierAllocatorHandler(mockedDeps); - const result = await handler(evt, {} as any, {} as any); + expect(result.batchItemFailures).toHaveLength(1); + expect(deps.logger.error).toHaveBeenCalled(); + }); - expect(result).toBeDefined(); - if (!result) throw new Error("expected BatchResponse, got void"); + it("records failures for missing variant mapping", async () => { + const deps = makeDeps(); + const letterEvent = makeLetterEventV2("unknown-variant"); + + const handler = createSupplierAllocatorHandler(deps); + const event: SQSEvent = { + Records: [makeSQSRecord(letterEvent)], + }; + + const result = (await handler( + event, + {} as any, + () => {}, + )) as SQSBatchResponse; + + expect(result.batchItemFailures).toHaveLength(1); + expect(deps.logger.error).toHaveBeenCalledWith( + expect.objectContaining({ + description: "Error processing allocation of record", + }), + ); + }); - expect(result.batchItemFailures).toHaveLength(1); - expect(result.batchItemFailures[0].itemIdentifier).toBe("bad-json"); - expect((mockedDeps.logger.error as jest.Mock).mock.calls).toHaveLength(1); - }); + it("records failures when supplier config resolution fails", async () => { + const deps = makeDeps(); + const letterEvent = makeLetterEventV2(); - test("returns batch failure when event type is missing", async () => { - const event = { no: "type" }; + (supplierConfig.getVariantDetails as jest.Mock).mockRejectedValue( + new Error("Database error"), + ); - const evt: SQSEvent = createSQSEvent([ - createSqsRecord("no-type", JSON.stringify(event)), - ]); + const handler = createSupplierAllocatorHandler(deps); + const event: SQSEvent = { + Records: [makeSQSRecord(letterEvent)], + }; - process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue"; + const result = (await handler( + event, + {} as any, + () => {}, + )) as SQSBatchResponse; - const handler = createSupplierAllocatorHandler(mockedDeps); - const result = await handler(evt, {} as any, {} as any); - if (!result) throw new Error("expected BatchResponse, got void"); + expect(result.batchItemFailures).toHaveLength(1); + expect(deps.logger.error).toHaveBeenCalled(); + }); - expect(result.batchItemFailures).toHaveLength(1); - expect(result.batchItemFailures[0].itemIdentifier).toBe("no-type"); - }); + it("records failures when UPSERT_LETTERS_QUEUE_URL not configured", async () => { + delete process.env.UPSERT_LETTERS_QUEUE_URL; - test("returns batch failure when UPSERT_LETTERS_QUEUE_URL is not set", async () => { - const preparedEvent = createPreparedV2Event(); + const deps = makeDeps(); + const letterEvent = makeLetterEventV2(); - const evt: SQSEvent = createSQSEvent([ - createSqsRecord("msg1", JSON.stringify(preparedEvent)), - ]); + setupDefaultMocks(); - delete process.env.UPSERT_LETTERS_QUEUE_URL; + const handler = createSupplierAllocatorHandler(deps); + const event: SQSEvent = { + Records: [makeSQSRecord(letterEvent)], + }; - const handler = createSupplierAllocatorHandler(mockedDeps); - const result = await handler(evt, {} as any, {} as any); - if (!result) throw new Error("expected BatchResponse, got void"); - - expect(result.batchItemFailures).toHaveLength(1); - expect(result.batchItemFailures[0].itemIdentifier).toBe("msg1"); - expect((mockedDeps.logger.error as jest.Mock).mock.calls[0][0].err).toEqual( - expect.objectContaining({ - message: "UPSERT_LETTERS_QUEUE_URL not configured", - }), - ); - }); + const result = (await handler( + event, + {} as any, + () => {}, + )) as SQSBatchResponse; - test("returns batch failure when variant mapping is missing", async () => { - const preparedEvent = createPreparedV2Event(); - preparedEvent.data.letterVariantId = "missing-variant"; - - const evt: SQSEvent = createSQSEvent([ - createSqsRecord("msg1", JSON.stringify(preparedEvent)), - ]); - - process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue"; - - // Override variant map to be empty for this test - mockedDeps.env.VARIANT_MAP = {} as any; - - const handler = createSupplierAllocatorHandler(mockedDeps); - const result = await handler(evt, {} as any, {} as any); - if (!result) throw new Error("expected BatchResponse, got void"); - - expect(result.batchItemFailures).toHaveLength(1); - expect(result.batchItemFailures[0].itemIdentifier).toBe("msg1"); - expect( - (mockedDeps.logger.error as jest.Mock).mock.calls.length, - ).toBeGreaterThan(0); - expect((mockedDeps.logger.error as jest.Mock).mock.calls[0][0]).toEqual( - expect.objectContaining({ - description: "No supplier mapping found for variant", - }), - ); - }); + expect(result.batchItemFailures).toHaveLength(1); + expect(deps.logger.error).toHaveBeenCalled(); + }); - test("handles SQS send errors and returns batch failure", async () => { - const preparedEvent = createPreparedV2Event(); + it("records failures when SQS send fails", async () => { + const deps = makeDeps(); + const letterEvent = makeLetterEventV2(); - const evt: SQSEvent = createSQSEvent([ - createSqsRecord("msg1", JSON.stringify(preparedEvent)), - ]); + setupDefaultMocks(); - process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue"; + deps.sqsClient.send = jest.fn().mockRejectedValue(new Error("SQS error")); - const sqsError = new Error("SQS send failed"); - (mockSqsClient.send as jest.Mock).mockRejectedValueOnce(sqsError); + const handler = createSupplierAllocatorHandler(deps); + const event: SQSEvent = { + Records: [makeSQSRecord(letterEvent)], + }; - const handler = createSupplierAllocatorHandler(mockedDeps); - const result = await handler(evt, {} as any, {} as any); - if (!result) throw new Error("expected BatchResponse, got void"); + const result = (await handler( + event, + {} as any, + () => {}, + )) as SQSBatchResponse; - expect(result.batchItemFailures).toHaveLength(1); - expect(result.batchItemFailures[0].itemIdentifier).toBe("msg1"); - expect((mockedDeps.logger.error as jest.Mock).mock.calls).toHaveLength(1); - }); + expect(result.batchItemFailures).toHaveLength(1); + expect(deps.logger.error).toHaveBeenCalled(); + }); - test("processes mixed batch with successes and failures", async () => { - const evt: SQSEvent = createSQSEvent([ - createSqsRecord( - "ok-msg", - JSON.stringify(createPreparedV2Event({ domainId: "letter1" })), - ), - createSqsRecord("fail-msg", "invalid-json"), - createSqsRecord( - "ok-msg-2", - JSON.stringify(createPreparedV2Event({ domainId: "letter2" })), - ), - ]); - - process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue"; - - const handler = createSupplierAllocatorHandler(mockedDeps); - const result = await handler(evt, {} as any, {} as any); - if (!result) throw new Error("expected BatchResponse, got void"); - - expect(result.batchItemFailures).toHaveLength(1); - expect(result.batchItemFailures[0].itemIdentifier).toBe("fail-msg"); - - expect(mockSqsClient.send).toHaveBeenCalledTimes(2); - }); + it("handles mixed success and failure records", async () => { + const deps = makeDeps(); + const letterEventGood = makeLetterEventV2("variant-1"); + const letterEventBad = { type: "invalid.type" }; + + setupDefaultMocks(); + + const handler = createSupplierAllocatorHandler(deps); + const event: SQSEvent = { + Records: [ + makeSQSRecord(letterEventGood), + makeSQSRecord(letterEventBad), + ], + }; + + const result = (await handler( + event, + {} as any, + () => {}, + )) as SQSBatchResponse; + + expect(result.batchItemFailures).toHaveLength(1); + expect(deps.sqsClient.send).toHaveBeenCalledTimes(1); + }); - test("sends correct queue URL in SQS message command", async () => { - const preparedEvent = createPreparedV2Event(); + it("logs extraction and resolution for successful records", async () => { + const deps = makeDeps(); + const letterEvent = makeLetterEventV2(); - const evt: SQSEvent = createSQSEvent([ - createSqsRecord("msg1", JSON.stringify(preparedEvent)), - ]); + setupDefaultMocks(); - const queueUrl = "https://sqs.eu-west-2.amazonaws.com/123456789/test-queue"; - process.env.UPSERT_LETTERS_QUEUE_URL = queueUrl; + const handler = createSupplierAllocatorHandler(deps); + const event: SQSEvent = { + Records: [makeSQSRecord(letterEvent)], + }; - const handler = createSupplierAllocatorHandler(mockedDeps); - await handler(evt, {} as any, {} as any); + await handler(event, {} as any, () => {}); - const sendCall = (mockSqsClient.send as jest.Mock).mock.calls[0][0]; - expect(sendCall.input.QueueUrl).toBe(queueUrl); + expect(deps.logger.info).toHaveBeenCalledWith( + expect.objectContaining({ + description: "Extracted letter event", + }), + ); + expect(deps.logger.info).toHaveBeenCalledWith( + expect.objectContaining({ + description: "Resolved supplier spec", + }), + ); + expect(deps.logger.info).toHaveBeenCalledWith( + expect.objectContaining({ + description: "Sending message to upsert letter queue", + }), + ); + }); }); }); diff --git a/lambdas/supplier-allocator/src/handler/allocate-handler.ts b/lambdas/supplier-allocator/src/handler/allocate-handler.ts index 74df928f..3733d4a2 100644 --- a/lambdas/supplier-allocator/src/handler/allocate-handler.ts +++ b/lambdas/supplier-allocator/src/handler/allocate-handler.ts @@ -114,9 +114,8 @@ export default function createSupplierAllocatorHandler(deps: Deps): SQSHandler { validateType(letterEvent); - await getSupplierFromConfig(letterEvent as PreparedEvents, deps); - const supplierSpec = getSupplier(letterEvent as PreparedEvents, deps); + await getSupplierFromConfig(letterEvent as PreparedEvents, deps); deps.logger.info({ description: "Resolved supplier spec", diff --git a/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts b/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts index 17c46023..f505bef1 100644 --- a/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts +++ b/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts @@ -1,5 +1,6 @@ import { getSupplierAllocationsForVolumeGroup, + getSupplierDetails, getVariantDetails, getVolumeGroupDetails, } from "../supplier-config"; @@ -43,8 +44,11 @@ describe("supplier-config service", () => { }); it("logs an error and returns undefined when not found", async () => { + const variant = undefined; const deps = makeDeps(); - deps.supplierConfigRepo.getLetterVariant = jest.fn().mockResolvedValue(); + deps.supplierConfigRepo.getLetterVariant = jest + .fn() + .mockResolvedValue(variant); const result = await getVariantDetails("missing", deps); @@ -70,10 +74,25 @@ describe("supplier-config service", () => { expect(result).toBe(group); expect(deps.logger.info).toHaveBeenCalled(); }); + it("logs an error and returns group details when not found", async () => { + const group = undefined; + const deps = makeDeps(); + deps.supplierConfigRepo.getVolumeGroup = jest + .fn() + .mockResolvedValue(group); - it("throws when group is not active based on dates/status", async () => { - const future = new Date(Date.now() + 1000 * 60 * 60 * 24).toISOString(); - const group = { id: "g2", status: "DRAFT", startDate: future } as any; + const result = await getVolumeGroupDetails("missing", deps); + + expect(result).toBeUndefined(); + expect(deps.logger.error).toHaveBeenCalled(); + }); + + it("throws when group is not active based on status", async () => { + const group = { + id: "g2", + status: "DRAFT", + startDate: "2020-01-01", + } as any; const deps = makeDeps(); deps.supplierConfigRepo.getVolumeGroup = jest .fn() @@ -84,6 +103,39 @@ describe("supplier-config service", () => { ); expect(deps.logger.error).toHaveBeenCalled(); }); + + it("throws when group is not active based on start date", async () => { + const future = new Date(Date.now() + 1000 * 60 * 60 * 24).toISOString(); + const group = { id: "g3", status: "PROD", startDate: future } as any; + const deps = makeDeps(); + deps.supplierConfigRepo.getVolumeGroup = jest + .fn() + .mockResolvedValue(group); + + await expect(getVolumeGroupDetails("g3", deps)).rejects.toThrow( + /not active/, + ); + expect(deps.logger.error).toHaveBeenCalled(); + }); + + it("throws when group is not active based on end date", async () => { + const past = new Date(Date.now() - 1000 * 60 * 60 * 24).toISOString(); + const group = { + id: "g3", + status: "PROD", + startDate: "2020-01-01", + endDate: past, + } as any; + const deps = makeDeps(); + deps.supplierConfigRepo.getVolumeGroup = jest + .fn() + .mockResolvedValue(group); + + await expect(getVolumeGroupDetails("g3", deps)).rejects.toThrow( + /not active/, + ); + expect(deps.logger.error).toHaveBeenCalled(); + }); }); describe("getSupplierAllocationsForVolumeGroup", () => { @@ -119,6 +171,18 @@ describe("supplier-config service", () => { expect(result).toEqual([allocations[1]]); }); + it("throws when no allocations found for volume group id, and logs an error", async () => { + const deps = makeDeps(); + deps.supplierConfigRepo.getSupplierAllocationsForVolumeGroup = jest + .fn() + .mockResolvedValue([]); + + await expect( + getSupplierAllocationsForVolumeGroup("g1", "", deps), + ).rejects.toThrow(/No supplier allocations found/); + expect(deps.logger.error).toHaveBeenCalled(); + }); + it("throws when supplierId provided but no matching allocation", async () => { const deps = makeDeps(); deps.supplierConfigRepo.getSupplierAllocationsForVolumeGroup = jest @@ -131,4 +195,68 @@ describe("supplier-config service", () => { expect(deps.logger.error).toHaveBeenCalled(); }); }); + + describe("getSupplierDetails", () => { + it("returns supplier details when found", async () => { + const allocations = [ + { supplier: "s1", variantId: "v1" }, + { supplier: "s2", variantId: "v2" }, + ] as any[]; + const suppliers = [ + { id: "s1", name: "Supplier 1" }, + { id: "s2", name: "Supplier 2" }, + ] as any[]; + const deps = makeDeps(); + deps.supplierConfigRepo.getSuppliersDetails = jest + .fn() + .mockResolvedValue(suppliers); + + const result = await getSupplierDetails(allocations, deps); + + expect(result).toEqual(suppliers); + expect(deps.supplierConfigRepo.getSuppliersDetails).toHaveBeenCalledWith([ + "s1", + "s2", + ]); + expect(deps.logger.info).toHaveBeenCalled(); + }); + + it("throws when no supplier details found", async () => { + const allocations = [{ supplier: "s1", variantId: "v1" }] as any[]; + const deps = makeDeps(); + deps.supplierConfigRepo.getSuppliersDetails = jest + .fn() + .mockResolvedValue([]); + + await expect(getSupplierDetails(allocations, deps)).rejects.toThrow( + /No supplier details found/, + ); + expect(deps.logger.error).toHaveBeenCalled(); + }); + + it("extracts supplier ids from allocations and requests details", async () => { + const allocations = [ + { supplier: "s1", variantId: "v1" }, + { supplier: "s3", variantId: "v2" }, + { supplier: "s5", variantId: "v3" }, + ] as any[]; + const suppliers = [ + { id: "s1", name: "Supplier 1" }, + { id: "s3", name: "Supplier 3" }, + { id: "s5", name: "Supplier 5" }, + ] as any[]; + const deps = makeDeps(); + deps.supplierConfigRepo.getSuppliersDetails = jest + .fn() + .mockResolvedValue(suppliers); + + await getSupplierDetails(allocations, deps); + + expect(deps.supplierConfigRepo.getSuppliersDetails).toHaveBeenCalledWith([ + "s1", + "s3", + "s5", + ]); + }); + }); }); diff --git a/lambdas/supplier-allocator/src/services/supplier-config.ts b/lambdas/supplier-allocator/src/services/supplier-config.ts index 79f3dfea..37fa35e8 100644 --- a/lambdas/supplier-allocator/src/services/supplier-config.ts +++ b/lambdas/supplier-allocator/src/services/supplier-config.ts @@ -54,12 +54,13 @@ export async function getVolumeGroupDetails( description: "No volume group found for id", groupId, }); + return undefined as any; } if ( - groupDetails.status !== "PROD" && - (new Date(groupDetails.startDate) > new Date() || - (groupDetails.endDate && new Date(groupDetails.endDate) < new Date())) + groupDetails.status !== "PROD" || + new Date(groupDetails.startDate) > new Date() || + (groupDetails.endDate && new Date(groupDetails.endDate) < new Date()) ) { deps.logger.error({ description: "Volume group is not active based on status and dates", @@ -97,6 +98,9 @@ export async function getSupplierAllocationsForVolumeGroup( description: "No supplier allocations found for volume group id", groupId, }); + throw new Error( + `No supplier allocations found for volume group id ${groupId}`, + ); } if (supplierId) { From 3a68d4a4e2de4d08a3902e52e2d5999ef2b43879 Mon Sep 17 00:00:00 2001 From: David Wass Date: Thu, 26 Feb 2026 10:48:52 +0000 Subject: [PATCH 13/24] Fix tests due to ESM Error - Not sure why --- lambdas/api-handler/jest.config.ts | 3 +++ lambdas/letter-updates-transformer/jest.config.ts | 3 +++ lambdas/mi-updates-transformer/jest.config.ts | 3 +++ 3 files changed, 9 insertions(+) diff --git a/lambdas/api-handler/jest.config.ts b/lambdas/api-handler/jest.config.ts index 87279451..174e7f7f 100644 --- a/lambdas/api-handler/jest.config.ts +++ b/lambdas/api-handler/jest.config.ts @@ -9,6 +9,9 @@ export const baseJestConfig = { }, ], }, + transformIgnorePatterns: [ + "node_modules/(?!(@nhsdigital/nhs-notify-event-schemas-supplier-config)/)", + ], // Automatically clear mock calls, instances, contexts and results before every test clearMocks: true, diff --git a/lambdas/letter-updates-transformer/jest.config.ts b/lambdas/letter-updates-transformer/jest.config.ts index 87279451..174e7f7f 100644 --- a/lambdas/letter-updates-transformer/jest.config.ts +++ b/lambdas/letter-updates-transformer/jest.config.ts @@ -9,6 +9,9 @@ export const baseJestConfig = { }, ], }, + transformIgnorePatterns: [ + "node_modules/(?!(@nhsdigital/nhs-notify-event-schemas-supplier-config)/)", + ], // Automatically clear mock calls, instances, contexts and results before every test clearMocks: true, diff --git a/lambdas/mi-updates-transformer/jest.config.ts b/lambdas/mi-updates-transformer/jest.config.ts index f88e7277..93ec73a5 100644 --- a/lambdas/mi-updates-transformer/jest.config.ts +++ b/lambdas/mi-updates-transformer/jest.config.ts @@ -26,6 +26,9 @@ export const baseJestConfig: Config = { coveragePathIgnorePatterns: ["/__tests__/"], transform: { "^.+\\.ts$": "ts-jest" }, + transformIgnorePatterns: [ + "node_modules/(?!(@nhsdigital/nhs-notify-event-schemas-supplier-config)/)", + ], testPathIgnorePatterns: [".build"], testMatch: ["**/?(*.)+(spec|test).[jt]s?(x)"], From 184a41f253089c34f1aafc360505d1053d28463e Mon Sep 17 00:00:00 2001 From: David Wass Date: Thu, 26 Feb 2026 15:26:21 +0000 Subject: [PATCH 14/24] new unit tests --- internal/datastore/jest.config.ts | 3 + internal/datastore/src/__test__/db.ts | 28 ++ .../supplier-config-repository.test.ts | 302 ++++++++++++++++++ internal/datastore/src/config.ts | 1 + .../src/supplier-config-repository.ts | 37 ++- .../src/handler/allocate-handler.ts | 21 +- .../__tests__/supplier-config.test.ts | 23 +- .../src/services/supplier-config.ts | 70 +--- 8 files changed, 369 insertions(+), 116 deletions(-) create mode 100644 internal/datastore/src/__test__/supplier-config-repository.test.ts diff --git a/internal/datastore/jest.config.ts b/internal/datastore/jest.config.ts index 1fb73e6d..da6c0a86 100644 --- a/internal/datastore/jest.config.ts +++ b/internal/datastore/jest.config.ts @@ -31,6 +31,9 @@ export const baseJestConfig: Config = { coveragePathIgnorePatterns: ["/__tests__/"], transform: { "^.+\\.ts$": "ts-jest" }, + transformIgnorePatterns: [ + "node_modules/(?!(@nhsdigital/nhs-notify-event-schemas-supplier-config)/)", + ], testPathIgnorePatterns: [".build"], testMatch: ["**/?(*.)+(spec|test).[jt]s?(x)"], diff --git a/internal/datastore/src/__test__/db.ts b/internal/datastore/src/__test__/db.ts index f382add6..8f5e224b 100644 --- a/internal/datastore/src/__test__/db.ts +++ b/internal/datastore/src/__test__/db.ts @@ -36,6 +36,7 @@ export async function setupDynamoDBContainer() { lettersTtlHours: 1, letterQueueTtlHours: 1, miTtlHours: 1, + supplierConfigTableName: "supplier-config", }; return { @@ -145,6 +146,31 @@ const createLetterQueueTableCommand = new CreateTableCommand({ { AttributeName: "queueTimestamp", AttributeType: "S" }, ], }); +const createSupplierConfigTableCommand = new CreateTableCommand({ + TableName: "supplier-config", + BillingMode: "PAY_PER_REQUEST", + KeySchema: [ + { AttributeName: "PK", KeyType: "HASH" }, // Partition key + { AttributeName: "SK", KeyType: "RANGE" }, // Sort key + ], + GlobalSecondaryIndexes: [ + { + IndexName: "volumeGroup-index", + KeySchema: [ + { AttributeName: "PK", KeyType: "HASH" }, // Partition key for GSI + { AttributeName: "volumeGroup", KeyType: "RANGE" }, // Sort key for GSI + ], + Projection: { + ProjectionType: "ALL", + }, + }, + ], + AttributeDefinitions: [ + { AttributeName: "PK", AttributeType: "S" }, + { AttributeName: "SK", AttributeType: "S" }, + { AttributeName: "volumeGroup", AttributeType: "S" }, + ], +}); export async function createTables(context: DBContext) { const { ddbClient } = context; @@ -155,6 +181,7 @@ export async function createTables(context: DBContext) { await ddbClient.send(createMITableCommand); await ddbClient.send(createSupplierTableCommand); await ddbClient.send(createLetterQueueTableCommand); + await ddbClient.send(createSupplierConfigTableCommand); } export async function deleteTables(context: DBContext) { @@ -165,6 +192,7 @@ export async function deleteTables(context: DBContext) { "management-info", "suppliers", "letter-queue", + "supplier-config", ]) { await ddbClient.send( new DeleteTableCommand({ diff --git a/internal/datastore/src/__test__/supplier-config-repository.test.ts b/internal/datastore/src/__test__/supplier-config-repository.test.ts new file mode 100644 index 00000000..df209bd9 --- /dev/null +++ b/internal/datastore/src/__test__/supplier-config-repository.test.ts @@ -0,0 +1,302 @@ +import { Logger } from "pino"; +import { PutCommand } from "@aws-sdk/lib-dynamodb"; +import { + DBContext, + createTables, + deleteTables, + setupDynamoDBContainer, +} from "./db"; +import { LogStream, createTestLogger } from "./logs"; +import { SupplierConfigRepository } from "../supplier-config-repository"; + +function createLetterVariantItem(variantId: string) { + return { + PK: "LETTER_VARIANT", + SK: variantId, + id: variantId, + name: `Variant ${variantId}`, + description: `Description for variant ${variantId}`, + type: "STANDARD", + status: "PROD", + volumeGroupId: `group-${variantId}`, + packSpecificationIds: [`pack-spec-${variantId}`], + }; +} + +function createVolumeGroupItem(groupId: string, status = "PROD") { + const startDate = new Date(Date.now() - 24 * 1000 * 60 * 60) + .toISOString() + .split("T")[0]; // Started an hour ago to ensure it's active based on start date. Tests can override this if needed. + const endDate = new Date(Date.now() + 24 * 1000 * 60 * 60) + .toISOString() + .split("T")[0]; // Ends in an hour to ensure it's active based on end date. Tests can override this if needed. + return { + PK: "VOLUME_GROUP", + SK: groupId, + id: groupId, + name: `Volume Group ${groupId}`, + description: `Description for volume group ${groupId}`, + status, + startDate, + endDate, + }; +} + +function createSupplierAllocationItem( + allocationId: string, + groupId: string, + supplier: string, +) { + return { + PK: `SUPPLIER_ALLOCATION`, + SK: allocationId, + id: allocationId, + status: "PROD", + volumeGroup: groupId, + supplier, + allocationPercentage: 50, + }; +} + +function createSupplierItem(supplierId: string) { + return { + PK: "SUPPLIER", + SK: supplierId, + id: supplierId, + name: `Supplier ${supplierId}`, + channelType: "LETTER", + dailyCapacity: 1000, + status: "PROD", + }; +} + +jest.setTimeout(30_000); + +describe("SupplierConfigRepository", () => { + let dbContext: DBContext; + let repository: SupplierConfigRepository; + let logStream: LogStream; + let logger: Logger; + + // Database tests can take longer, especially with setup and teardown + beforeAll(async () => { + dbContext = await setupDynamoDBContainer(); + }); + + beforeEach(async () => { + await createTables(dbContext); + ({ logStream, logger } = createTestLogger()); + repository = new SupplierConfigRepository( + dbContext.docClient, + logger, + dbContext.config, + ); + }); + + afterEach(async () => { + await deleteTables(dbContext); + jest.useRealTimers(); + }); + + afterAll(async () => { + await dbContext.container.stop(); + }); + + test("getLetterVariant returns correct details for existing variant", async () => { + const variantId = "variant-123"; + await dbContext.docClient.send( + new PutCommand({ + TableName: dbContext.config.supplierConfigTableName, + Item: createLetterVariantItem(variantId), + }), + ); + + const result = await repository.getLetterVariant(variantId); + + expect(result.id).toBe(variantId); + expect(result.name).toBe(`Variant ${variantId}`); + expect(result.description).toBe(`Description for variant ${variantId}`); + expect(result.type).toBe("STANDARD"); + expect(result.status).toBe("PROD"); + expect(result.volumeGroupId).toBe(`group-${variantId}`); + expect(result.packSpecificationIds).toEqual([`pack-spec-${variantId}`]); + + expect(logStream.logs).toEqual([]); + }); + + test("getLetterVariant throws error for non-existent variant", async () => { + const variantId = "non-existent-variant"; + + await expect(repository.getLetterVariant(variantId)).rejects.toThrow( + `No letter variant details found for id ${variantId}`, + ); + + expect( + logStream.logs.some((log) => + log.includes("No letter variant found for id"), + ), + ).toBe(true); + }); + + test("getVolumeGroup returns correct details for existing group", async () => { + const groupId = "group-123"; + await dbContext.docClient.send( + new PutCommand({ + TableName: dbContext.config.supplierConfigTableName, + Item: createVolumeGroupItem(groupId), + }), + ); + + const result = await repository.getVolumeGroup(groupId); + + expect(result.id).toBe(groupId); + expect(result.name).toBe(`Volume Group ${groupId}`); + expect(result.description).toBe(`Description for volume group ${groupId}`); + expect(result.status).toBe("PROD"); + expect(new Date(result.startDate).getTime()).toBeLessThan(Date.now()); + expect(new Date(result.endDate!).getTime()).toBeGreaterThan(Date.now()); + + expect(logStream.logs).toEqual([]); + }); + + test("getVolumeGroup throws error for non-existent group", async () => { + const groupId = "non-existent-group"; + + await expect(repository.getVolumeGroup(groupId)).rejects.toThrow( + `No volume group details found for id ${groupId}`, + ); + + expect( + logStream.logs.some((log) => + log.includes("No volume group found for id"), + ), + ).toBe(true); + }); + + test("getSupplierAllocationsForVolumeGroup returns correct allocations", async () => { + const allocationId = "allocation-123"; + const groupId = "group-123"; + const supplierId = "supplier-123"; + + await dbContext.docClient.send( + new PutCommand({ + TableName: dbContext.config.supplierConfigTableName, + Item: createSupplierAllocationItem(allocationId, groupId, supplierId), + }), + ); + + const result = + await repository.getSupplierAllocationsForVolumeGroup(groupId); + + expect(result).toEqual([ + { + id: allocationId, + status: "PROD", + volumeGroup: groupId, + supplier: supplierId, + allocationPercentage: 50, + }, + ]); + + expect(logStream.logs).toEqual([]); + }); + + test("getSupplierAllocationsForVolumeGroup returns multiple allocations", async () => { + const allocationId1 = "allocation-123"; + const allocationId2 = "allocation-456"; + const groupId = "group-123"; + const supplierId1 = "supplier-123"; + const supplierId2 = "supplier-456"; + + await dbContext.docClient.send( + new PutCommand({ + TableName: dbContext.config.supplierConfigTableName, + Item: createSupplierAllocationItem(allocationId1, groupId, supplierId1), + }), + ); + + await dbContext.docClient.send( + new PutCommand({ + TableName: dbContext.config.supplierConfigTableName, + Item: createSupplierAllocationItem(allocationId2, groupId, supplierId2), + }), + ); + + const result = + await repository.getSupplierAllocationsForVolumeGroup(groupId); + + // order of allocations should not matter, just that both are present + expect(result).toHaveLength(2); + expect(result).toEqual( + expect.arrayContaining([ + { + id: allocationId1, + status: "PROD", + volumeGroup: groupId, + supplier: supplierId1, + allocationPercentage: 50, + }, + { + id: allocationId2, + status: "PROD", + volumeGroup: groupId, + supplier: supplierId2, + allocationPercentage: 50, + }, + ]), + ); + expect(logStream.logs).toEqual([]); + }); + + test("getSupplierAllocationsForVolumeGroup throws error for non-existent group", async () => { + const groupId = "non-existent-group"; + + await expect( + repository.getSupplierAllocationsForVolumeGroup(groupId), + ).rejects.toThrow( + `No active supplier allocations found for volume group id ${groupId}`, + ); + + expect( + logStream.logs.some((log) => + log.includes("No supplier allocations found for volume group id"), + ), + ).toBe(true); + }); + + test("getSuppliersDetails returns correct supplier details", async () => { + const supplierId = "supplier-123"; + + await dbContext.docClient.send( + new PutCommand({ + TableName: dbContext.config.supplierConfigTableName, + Item: createSupplierItem(supplierId), + }), + ); + + const result = await repository.getSuppliersDetails([supplierId]); + + expect(result).toEqual([ + { + id: supplierId, + name: `Supplier ${supplierId}`, + channelType: "LETTER", + dailyCapacity: 1000, + status: "PROD", + }, + ]); + expect(logStream.logs).toEqual([]); + }); + + test("getSuppliersDetails throws error for non-existent supplier", async () => { + const supplierId = "non-existent-supplier"; + + await expect(repository.getSuppliersDetails([supplierId])).rejects.toThrow( + `Supplier with id ${supplierId} not found`, + ); + + expect( + logStream.logs.some((log) => log.includes("No supplier found for id")), + ).toBe(true); + }); +}); diff --git a/internal/datastore/src/config.ts b/internal/datastore/src/config.ts index 4066101a..f0795402 100644 --- a/internal/datastore/src/config.ts +++ b/internal/datastore/src/config.ts @@ -8,4 +8,5 @@ export type DatastoreConfig = { lettersTtlHours: number; letterQueueTtlHours: number; miTtlHours: number; + supplierConfigTableName: string; }; diff --git a/internal/datastore/src/supplier-config-repository.ts b/internal/datastore/src/supplier-config-repository.ts index 53461d50..6b60b40f 100644 --- a/internal/datastore/src/supplier-config-repository.ts +++ b/internal/datastore/src/supplier-config-repository.ts @@ -34,8 +34,13 @@ export class SupplierConfigRepository { }), ); if (!result.Item) { - throw new Error(`Letter variant with id ${variantId} not found`); + this.log.error({ + description: "No letter variant found for id", + variantId, + }); + throw new Error(`No letter variant details found for id ${variantId}`); } + return $LetterVariant.parse(result.Item); } @@ -47,20 +52,18 @@ export class SupplierConfigRepository { }), ); if (!result.Item) { - throw new Error(`Volume group with id ${groupId} not found`); + this.log.error({ + description: "No volume group found for id", + groupId, + }); + throw new Error(`No volume group details found for id ${groupId}`); } - return $VolumeGroup.parse(result.Item); } async getSupplierAllocationsForVolumeGroup( groupId: string, ): Promise { - this.log.info({ - description: - "Fetching supplier allocations for volume group from database", - groupId, - }); const result = await this.ddbClient.send( new QueryCommand({ TableName: this.config.supplierConfigTableName, @@ -79,15 +82,13 @@ export class SupplierConfigRepository { }, }), ); - this.log.info({ - description: - "Fetched supplier allocations for volume group from database", - groupId, - count: result.Items?.length ?? 0, - }); - if (!result.Items) { + if (!result.Items || result.Items.length === 0) { + this.log.error({ + description: "No supplier allocations found for volume group id", + groupId, + }); throw new Error( - `No supplier allocations found for volume group id ${groupId}`, + `No active supplier allocations found for volume group id ${groupId}`, ); } @@ -104,6 +105,10 @@ export class SupplierConfigRepository { }), ); if (!result.Item) { + this.log.error({ + description: "No supplier found for id", + supplierId, + }); throw new Error(`Supplier with id ${supplierId} not found`); } suppliers.push($Supplier.parse(result.Item)); diff --git a/lambdas/supplier-allocator/src/handler/allocate-handler.ts b/lambdas/supplier-allocator/src/handler/allocate-handler.ts index 3733d4a2..f2e6c33e 100644 --- a/lambdas/supplier-allocator/src/handler/allocate-handler.ts +++ b/lambdas/supplier-allocator/src/handler/allocate-handler.ts @@ -3,6 +3,7 @@ import { SendMessageCommand } from "@aws-sdk/client-sqs"; import { LetterRequestPreparedEvent } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering-v1"; import { LetterVariant, + Supplier, SupplierAllocation, VolumeGroup, } from "@nhsdigital/nhs-notify-event-schemas-supplier-config"; @@ -61,19 +62,11 @@ async function getSupplierFromConfig(letterEvent: PreparedEvents, deps: Deps) { letterEvent.data.letterVariantId, deps, ); - deps.logger.info({ - description: "Fetched variant details for letter variant", - variantDetails, - }); const volumeGroupDetails: VolumeGroup = await getVolumeGroupDetails( variantDetails.volumeGroupId, deps, ); - deps.logger.info({ - description: "Fetched volume group details for letter variant", - volumeGroupDetails, - }); const supplierAllocations: SupplierAllocation[] = await getSupplierAllocationsForVolumeGroup( @@ -81,14 +74,16 @@ async function getSupplierFromConfig(letterEvent: PreparedEvents, deps: Deps) { variantDetails.supplierId ?? "", deps, ); - deps.logger.info({ - description: "Fetched supplier allocations for volume group", - supplierAllocations, - }); - const supplierDetails = await getSupplierDetails(supplierAllocations, deps); + const supplierDetails: Supplier[] = await getSupplierDetails( + supplierAllocations, + deps, + ); deps.logger.info({ description: "Fetched supplier details for supplier allocations", + variantId: letterEvent.data.letterVariantId, + volumeGroupId: volumeGroupDetails.id, + supplierAllocationIds: supplierAllocations.map((a) => a.id), supplierDetails, }); diff --git a/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts b/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts index f505bef1..3ffe9b10 100644 --- a/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts +++ b/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts @@ -30,7 +30,7 @@ describe("supplier-config service", () => { afterEach(() => jest.resetAllMocks()); describe("getVariantDetails", () => { - it("returns variant details and logs when found", async () => { + it("returns variant details", async () => { const variant = { id: "v1", volumeGroupId: "g1" } as any; const deps = makeDeps(); deps.supplierConfigRepo.getLetterVariant = jest @@ -40,10 +40,9 @@ describe("supplier-config service", () => { const result = await getVariantDetails("v1", deps); expect(result).toBe(variant); - expect(deps.logger.info).toHaveBeenCalled(); }); - it("logs an error and returns undefined when not found", async () => { + it("returns undefined when not found", async () => { const variant = undefined; const deps = makeDeps(); deps.supplierConfigRepo.getLetterVariant = jest @@ -53,7 +52,6 @@ describe("supplier-config service", () => { const result = await getVariantDetails("missing", deps); expect(result).toBeUndefined(); - expect(deps.logger.error).toHaveBeenCalled(); }); }); @@ -72,7 +70,6 @@ describe("supplier-config service", () => { const result = await getVolumeGroupDetails("g1", deps); expect(result).toBe(group); - expect(deps.logger.info).toHaveBeenCalled(); }); it("logs an error and returns group details when not found", async () => { const group = undefined; @@ -84,7 +81,6 @@ describe("supplier-config service", () => { const result = await getVolumeGroupDetails("missing", deps); expect(result).toBeUndefined(); - expect(deps.logger.error).toHaveBeenCalled(); }); it("throws when group is not active based on status", async () => { @@ -153,7 +149,6 @@ describe("supplier-config service", () => { const result = await getSupplierAllocationsForVolumeGroup("g1", "", deps); expect(result).toEqual(allocations); - expect(deps.logger.info).toHaveBeenCalled(); }); it("filters by supplierId when provided", async () => { @@ -171,18 +166,6 @@ describe("supplier-config service", () => { expect(result).toEqual([allocations[1]]); }); - it("throws when no allocations found for volume group id, and logs an error", async () => { - const deps = makeDeps(); - deps.supplierConfigRepo.getSupplierAllocationsForVolumeGroup = jest - .fn() - .mockResolvedValue([]); - - await expect( - getSupplierAllocationsForVolumeGroup("g1", "", deps), - ).rejects.toThrow(/No supplier allocations found/); - expect(deps.logger.error).toHaveBeenCalled(); - }); - it("throws when supplierId provided but no matching allocation", async () => { const deps = makeDeps(); deps.supplierConfigRepo.getSupplierAllocationsForVolumeGroup = jest @@ -218,7 +201,6 @@ describe("supplier-config service", () => { "s1", "s2", ]); - expect(deps.logger.info).toHaveBeenCalled(); }); it("throws when no supplier details found", async () => { @@ -231,7 +213,6 @@ describe("supplier-config service", () => { await expect(getSupplierDetails(allocations, deps)).rejects.toThrow( /No supplier details found/, ); - expect(deps.logger.error).toHaveBeenCalled(); }); it("extracts supplier ids from allocations and requests details", async () => { diff --git a/lambdas/supplier-allocator/src/services/supplier-config.ts b/lambdas/supplier-allocator/src/services/supplier-config.ts index 37fa35e8..18d5a3cd 100644 --- a/lambdas/supplier-allocator/src/services/supplier-config.ts +++ b/lambdas/supplier-allocator/src/services/supplier-config.ts @@ -10,26 +10,8 @@ export async function getVariantDetails( variantId: string, deps: Deps, ): Promise { - deps.logger.info({ - description: "Fetching letter variant details from database", - variantId, - }); - const variantDetails: LetterVariant = await deps.supplierConfigRepo.getLetterVariant(variantId); - - if (variantDetails) { - deps.logger.info({ - description: "Fetched letter variant details", - variantId, - variantDetails, - }); - } else { - deps.logger.error({ - description: "No letter variant found for id", - variantId, - }); - } return variantDetails; } @@ -37,30 +19,13 @@ export async function getVolumeGroupDetails( groupId: string, deps: Deps, ): Promise { - deps.logger.info({ - description: "Fetching volume group details from database", - groupId, - }); - const groupDetails = await deps.supplierConfigRepo.getVolumeGroup(groupId); - if (groupDetails) { - deps.logger.info({ - description: "Fetched volume group details", - groupId, - groupDetails, - }); - } else { - deps.logger.error({ - description: "No volume group found for id", - groupId, - }); - return undefined as any; - } if ( - groupDetails.status !== "PROD" || - new Date(groupDetails.startDate) > new Date() || - (groupDetails.endDate && new Date(groupDetails.endDate) < new Date()) + groupDetails && + (groupDetails.status !== "PROD" || + new Date(groupDetails.startDate) > new Date() || + (groupDetails.endDate && new Date(groupDetails.endDate) < new Date())) ) { deps.logger.error({ description: "Volume group is not active based on status and dates", @@ -79,30 +44,9 @@ export async function getSupplierAllocationsForVolumeGroup( supplierId: string, deps: Deps, ): Promise { - deps.logger.info({ - description: "Fetching supplier allocations for volume group from database", - groupId, - }); const allocations = await deps.supplierConfigRepo.getSupplierAllocationsForVolumeGroup(groupId); - if (allocations.length > 0) { - deps.logger.info({ - description: - "Fetched supplier allocations for volume group from database", - groupId, - count: allocations.length, - }); - } else { - deps.logger.error({ - description: "No supplier allocations found for volume group id", - groupId, - }); - throw new Error( - `No supplier allocations found for volume group id ${groupId}`, - ); - } - if (supplierId) { const filteredAllocations = allocations.filter( (alloc) => alloc.supplier === supplierId, @@ -133,12 +77,6 @@ export async function getSupplierDetails( const supplierDetails: Supplier[] = await deps.supplierConfigRepo.getSuppliersDetails(supplierIds); - deps.logger.info({ - description: "Fetched supplier details for supplier allocations", - supplierIds, - supplierDetails, - }); - if (Object.keys(supplierDetails).length === 0) { deps.logger.error({ description: "No supplier details found for supplier allocations", From 183c31595bcde1468829ac425f94b1c41425903b Mon Sep 17 00:00:00 2001 From: David Wass Date: Thu, 26 Feb 2026 15:52:11 +0000 Subject: [PATCH 15/24] transform ignore --- lambdas/update-letter-queue/jest.config.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lambdas/update-letter-queue/jest.config.ts b/lambdas/update-letter-queue/jest.config.ts index e173d5e3..c6f65159 100644 --- a/lambdas/update-letter-queue/jest.config.ts +++ b/lambdas/update-letter-queue/jest.config.ts @@ -9,6 +9,9 @@ export const baseJestConfig = { }, ], }, + transformIgnorePatterns: [ + "node_modules/(?!(@nhsdigital/nhs-notify-event-schemas-supplier-config)/)", + ], // Automatically clear mock calls, instances, contexts and results before every test clearMocks: true, From e4b737c2fd3bd96ce3758a3de430d020351b85f1 Mon Sep 17 00:00:00 2001 From: David Wass Date: Mon, 9 Mar 2026 14:13:51 +0000 Subject: [PATCH 16/24] review changes --- .../supplier-config-repository.test.ts | 40 +----- .../src/supplier-config-repository.ts | 18 --- .../src/config/__tests__/deps.test.ts | 2 +- lambdas/supplier-allocator/src/config/deps.ts | 2 +- .../src/handler/allocate-handler.ts | 2 +- .../__tests__/supplier-config.test.ts | 51 +++++++- .../src/services/supplier-config.ts | 15 ++- package-lock.json | 122 +++++------------- 8 files changed, 100 insertions(+), 152 deletions(-) diff --git a/internal/datastore/src/__test__/supplier-config-repository.test.ts b/internal/datastore/src/__test__/supplier-config-repository.test.ts index df209bd9..9fde74f9 100644 --- a/internal/datastore/src/__test__/supplier-config-repository.test.ts +++ b/internal/datastore/src/__test__/supplier-config-repository.test.ts @@ -1,4 +1,3 @@ -import { Logger } from "pino"; import { PutCommand } from "@aws-sdk/lib-dynamodb"; import { DBContext, @@ -6,7 +5,6 @@ import { deleteTables, setupDynamoDBContainer, } from "./db"; -import { LogStream, createTestLogger } from "./logs"; import { SupplierConfigRepository } from "../supplier-config-repository"; function createLetterVariantItem(variantId: string) { @@ -26,10 +24,10 @@ function createLetterVariantItem(variantId: string) { function createVolumeGroupItem(groupId: string, status = "PROD") { const startDate = new Date(Date.now() - 24 * 1000 * 60 * 60) .toISOString() - .split("T")[0]; // Started an hour ago to ensure it's active based on start date. Tests can override this if needed. + .split("T")[0]; // Started a day ago to ensure it's active based on start date. Tests can override this if needed. const endDate = new Date(Date.now() + 24 * 1000 * 60 * 60) .toISOString() - .split("T")[0]; // Ends in an hour to ensure it's active based on end date. Tests can override this if needed. + .split("T")[0]; // Ends in a day to ensure it's active based on end date. Tests can override this if needed. return { PK: "VOLUME_GROUP", SK: groupId, @@ -75,8 +73,6 @@ jest.setTimeout(30_000); describe("SupplierConfigRepository", () => { let dbContext: DBContext; let repository: SupplierConfigRepository; - let logStream: LogStream; - let logger: Logger; // Database tests can take longer, especially with setup and teardown beforeAll(async () => { @@ -85,10 +81,8 @@ describe("SupplierConfigRepository", () => { beforeEach(async () => { await createTables(dbContext); - ({ logStream, logger } = createTestLogger()); repository = new SupplierConfigRepository( dbContext.docClient, - logger, dbContext.config, ); }); @@ -120,8 +114,6 @@ describe("SupplierConfigRepository", () => { expect(result.status).toBe("PROD"); expect(result.volumeGroupId).toBe(`group-${variantId}`); expect(result.packSpecificationIds).toEqual([`pack-spec-${variantId}`]); - - expect(logStream.logs).toEqual([]); }); test("getLetterVariant throws error for non-existent variant", async () => { @@ -130,12 +122,6 @@ describe("SupplierConfigRepository", () => { await expect(repository.getLetterVariant(variantId)).rejects.toThrow( `No letter variant details found for id ${variantId}`, ); - - expect( - logStream.logs.some((log) => - log.includes("No letter variant found for id"), - ), - ).toBe(true); }); test("getVolumeGroup returns correct details for existing group", async () => { @@ -155,8 +141,6 @@ describe("SupplierConfigRepository", () => { expect(result.status).toBe("PROD"); expect(new Date(result.startDate).getTime()).toBeLessThan(Date.now()); expect(new Date(result.endDate!).getTime()).toBeGreaterThan(Date.now()); - - expect(logStream.logs).toEqual([]); }); test("getVolumeGroup throws error for non-existent group", async () => { @@ -165,12 +149,6 @@ describe("SupplierConfigRepository", () => { await expect(repository.getVolumeGroup(groupId)).rejects.toThrow( `No volume group details found for id ${groupId}`, ); - - expect( - logStream.logs.some((log) => - log.includes("No volume group found for id"), - ), - ).toBe(true); }); test("getSupplierAllocationsForVolumeGroup returns correct allocations", async () => { @@ -197,8 +175,6 @@ describe("SupplierConfigRepository", () => { allocationPercentage: 50, }, ]); - - expect(logStream.logs).toEqual([]); }); test("getSupplierAllocationsForVolumeGroup returns multiple allocations", async () => { @@ -245,7 +221,6 @@ describe("SupplierConfigRepository", () => { }, ]), ); - expect(logStream.logs).toEqual([]); }); test("getSupplierAllocationsForVolumeGroup throws error for non-existent group", async () => { @@ -256,12 +231,6 @@ describe("SupplierConfigRepository", () => { ).rejects.toThrow( `No active supplier allocations found for volume group id ${groupId}`, ); - - expect( - logStream.logs.some((log) => - log.includes("No supplier allocations found for volume group id"), - ), - ).toBe(true); }); test("getSuppliersDetails returns correct supplier details", async () => { @@ -285,7 +254,6 @@ describe("SupplierConfigRepository", () => { status: "PROD", }, ]); - expect(logStream.logs).toEqual([]); }); test("getSuppliersDetails throws error for non-existent supplier", async () => { @@ -294,9 +262,5 @@ describe("SupplierConfigRepository", () => { await expect(repository.getSuppliersDetails([supplierId])).rejects.toThrow( `Supplier with id ${supplierId} not found`, ); - - expect( - logStream.logs.some((log) => log.includes("No supplier found for id")), - ).toBe(true); }); }); diff --git a/internal/datastore/src/supplier-config-repository.ts b/internal/datastore/src/supplier-config-repository.ts index 6b60b40f..1f82bb0c 100644 --- a/internal/datastore/src/supplier-config-repository.ts +++ b/internal/datastore/src/supplier-config-repository.ts @@ -3,7 +3,6 @@ import { GetCommand, QueryCommand, } from "@aws-sdk/lib-dynamodb"; -import { Logger } from "pino"; import { $LetterVariant, $Supplier, @@ -22,7 +21,6 @@ export type SupplierConfigRepositoryConfig = { export class SupplierConfigRepository { constructor( readonly ddbClient: DynamoDBDocumentClient, - readonly log: Logger, readonly config: SupplierConfigRepositoryConfig, ) {} @@ -34,10 +32,6 @@ export class SupplierConfigRepository { }), ); if (!result.Item) { - this.log.error({ - description: "No letter variant found for id", - variantId, - }); throw new Error(`No letter variant details found for id ${variantId}`); } @@ -52,10 +46,6 @@ export class SupplierConfigRepository { }), ); if (!result.Item) { - this.log.error({ - description: "No volume group found for id", - groupId, - }); throw new Error(`No volume group details found for id ${groupId}`); } return $VolumeGroup.parse(result.Item); @@ -83,10 +73,6 @@ export class SupplierConfigRepository { }), ); if (!result.Items || result.Items.length === 0) { - this.log.error({ - description: "No supplier allocations found for volume group id", - groupId, - }); throw new Error( `No active supplier allocations found for volume group id ${groupId}`, ); @@ -105,10 +91,6 @@ export class SupplierConfigRepository { }), ); if (!result.Item) { - this.log.error({ - description: "No supplier found for id", - supplierId, - }); throw new Error(`Supplier with id ${supplierId} not found`); } suppliers.push($Supplier.parse(result.Item)); diff --git a/lambdas/supplier-allocator/src/config/__tests__/deps.test.ts b/lambdas/supplier-allocator/src/config/__tests__/deps.test.ts index f07f63aa..6069dc07 100644 --- a/lambdas/supplier-allocator/src/config/__tests__/deps.test.ts +++ b/lambdas/supplier-allocator/src/config/__tests__/deps.test.ts @@ -47,7 +47,7 @@ describe("createDependenciesContainer", () => { expect(createLogger).toHaveBeenCalledTimes(1); expect(SupplierConfigRepository).toHaveBeenCalledTimes(1); const supplierConfigRepoCtorArgs = SupplierConfigRepository.mock.calls[0]; - expect(supplierConfigRepoCtorArgs[2]).toEqual({ + expect(supplierConfigRepoCtorArgs[1]).toEqual({ supplierConfigTableName: "SupplierConfigTable", }); expect(deps.env).toEqual(env); diff --git a/lambdas/supplier-allocator/src/config/deps.ts b/lambdas/supplier-allocator/src/config/deps.ts index 5758b6df..4d51f9a0 100644 --- a/lambdas/supplier-allocator/src/config/deps.ts +++ b/lambdas/supplier-allocator/src/config/deps.ts @@ -27,7 +27,7 @@ function createSupplierConfigRepository( supplierConfigTableName: envVars.SUPPLIER_CONFIG_TABLE_NAME, }; - return new SupplierConfigRepository(createDocumentClient(), log, config); + return new SupplierConfigRepository(createDocumentClient(), config); } export function createDependenciesContainer(): Deps { diff --git a/lambdas/supplier-allocator/src/handler/allocate-handler.ts b/lambdas/supplier-allocator/src/handler/allocate-handler.ts index f2e6c33e..a3ef61ed 100644 --- a/lambdas/supplier-allocator/src/handler/allocate-handler.ts +++ b/lambdas/supplier-allocator/src/handler/allocate-handler.ts @@ -71,8 +71,8 @@ async function getSupplierFromConfig(letterEvent: PreparedEvents, deps: Deps) { const supplierAllocations: SupplierAllocation[] = await getSupplierAllocationsForVolumeGroup( variantDetails.volumeGroupId, - variantDetails.supplierId ?? "", deps, + variantDetails.supplierId, ); const supplierDetails: Supplier[] = await getSupplierDetails( diff --git a/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts b/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts index 3ffe9b10..278ff2af 100644 --- a/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts +++ b/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts @@ -5,11 +5,13 @@ import { getVolumeGroupDetails, } from "../supplier-config"; import { Deps } from "../../config/deps"; +import { warn } from "node:console"; function makeDeps(overrides: Partial = {}): Deps { const logger = { info: jest.fn(), error: jest.fn(), + warn: jest.fn(), } as unknown as Deps["logger"]; const supplierConfigRepo = { @@ -146,7 +148,7 @@ describe("supplier-config service", () => { .fn() .mockResolvedValue(allocations); - const result = await getSupplierAllocationsForVolumeGroup("g1", "", deps); + const result = await getSupplierAllocationsForVolumeGroup("g1", deps); expect(result).toEqual(allocations); }); @@ -159,8 +161,8 @@ describe("supplier-config service", () => { const result = await getSupplierAllocationsForVolumeGroup( "g1", - "s2", deps, + "s2", ); expect(result).toEqual([allocations[1]]); @@ -173,7 +175,7 @@ describe("supplier-config service", () => { .mockResolvedValue(allocations); await expect( - getSupplierAllocationsForVolumeGroup("g1", "missing", deps), + getSupplierAllocationsForVolumeGroup("g1", deps, "missing"), ).rejects.toThrow(/No supplier allocations found/); expect(deps.logger.error).toHaveBeenCalled(); }); @@ -240,4 +242,47 @@ describe("supplier-config service", () => { ]); }); }); + it("logs a warning when supplier allocations count differs from supplier details count", async () => { + const allocations = [ + { supplier: "s1", variantId: "v1" }, + { supplier: "s2", variantId: "v2" }, + { supplier: "s3", variantId: "v3" }, + ] as any[]; + const suppliers = [ + { id: "s1", name: "Supplier 1" }, + { id: "s2", name: "Supplier 2" }, + ] as any[]; + const deps = makeDeps(); + deps.supplierConfigRepo.getSuppliersDetails = jest + .fn() + .mockResolvedValue(suppliers); + + await getSupplierDetails(allocations, deps); + + expect(deps.logger.warn).toHaveBeenCalledWith({ + description: "Mismatch between supplier allocations and supplier details", + allocationsCount: 3, + detailsCount: 2, + missingSuppliers: ["s3"], + }); + }); + + it("does not log a warning when counts match", async () => { + const allocations = [ + { supplier: "s1", variantId: "v1" }, + { supplier: "s2", variantId: "v2" }, + ] as any[]; + const suppliers = [ + { id: "s1", name: "Supplier 1" }, + { id: "s2", name: "Supplier 2" }, + ] as any[]; + const deps = makeDeps(); + deps.supplierConfigRepo.getSuppliersDetails = jest + .fn() + .mockResolvedValue(suppliers); + + await getSupplierDetails(allocations, deps); + + expect(deps.logger.warn).not.toHaveBeenCalled(); + }); }); diff --git a/lambdas/supplier-allocator/src/services/supplier-config.ts b/lambdas/supplier-allocator/src/services/supplier-config.ts index 18d5a3cd..f8619520 100644 --- a/lambdas/supplier-allocator/src/services/supplier-config.ts +++ b/lambdas/supplier-allocator/src/services/supplier-config.ts @@ -41,8 +41,8 @@ export async function getVolumeGroupDetails( export async function getSupplierAllocationsForVolumeGroup( groupId: string, - supplierId: string, deps: Deps, + supplierId?: string, ): Promise { const allocations = await deps.supplierConfigRepo.getSupplierAllocationsForVolumeGroup(groupId); @@ -86,5 +86,18 @@ export async function getSupplierDetails( `No supplier details found for supplier ids ${supplierIds.join(", ")}`, ); } + // Log a warning if some supplier details are missing compared to allocations + if (supplierAllocations.length !== supplierDetails.length) { + const foundSupplierIds = new Set(supplierDetails.map((s) => s.id)); + const missingSupplierIds = supplierIds.filter( + (id) => !foundSupplierIds.has(id), + ); + deps.logger.warn({ + description: "Mismatch between supplier allocations and supplier details", + allocationsCount: supplierAllocations.length, + detailsCount: supplierDetails.length, + missingSuppliers: missingSupplierIds, + }); + } return supplierDetails; } diff --git a/package-lock.json b/package-lock.json index 0b7ea9d4..84ca6562 100644 --- a/package-lock.json +++ b/package-lock.json @@ -93,7 +93,6 @@ "@aws-sdk/client-dynamodb": "^3.984.0", "@aws-sdk/lib-dynamodb": "^3.984.0", "@internal/helpers": "*", - "@nhsdigital/nhs-notify-event-schemas-supplier-config": "^1.0.1", "pino": "^10.3.0", "zod": "^4.1.11", "zod-mermaid": "^1.0.9" @@ -295,7 +294,6 @@ "@nhsdigital/nhs-notify-event-schemas-letter-rendering": "^2.0.1", "@nhsdigital/nhs-notify-event-schemas-letter-rendering-v1": "npm:@nhsdigital/nhs-notify-event-schemas-letter-rendering@^1.1.5", "@nhsdigital/nhs-notify-event-schemas-supplier-api": "^1.0.8", - "@nhsdigital/nhs-notify-event-schemas-supplier-config": "^1.0.1", "@types/aws-lambda": "^8.10.148", "aws-lambda": "^1.0.7", "esbuild": "^0.27.2", @@ -2955,43 +2953,28 @@ "node": "^18.18.0 || ^20.9.0 || >=21.1.0" } }, - "node_modules/@eslint/config-array/node_modules/balanced-match": { - "version": "4.0.4", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", - "integrity": "sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==", - "dev": true, - "license": "MIT", - "engines": { - "node": "18 || 20 || >=22" - } - }, "node_modules/@eslint/config-array/node_modules/brace-expansion": { - "version": "5.0.4", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.4.tgz", - "integrity": "sha512-h+DEnpVvxmfVefa4jFbCf5HdH5YMDXRsmKflpf1pILZWRFlTbJpxeU55nJl4Smt5HQaGzg1o6RHFPJaOqnmBDg==", + "version": "1.1.12", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.12.tgz", + "integrity": "sha512-9T9UjW3r0UW5c1Q7GTwllptXwhvYmEzFhzMfZ9H7FQWt+uZePjZPjBP/W1ZEyZ1twGWom5/56TF4lPcqjnDHcg==", "dev": true, "license": "MIT", "dependencies": { - "balanced-match": "^4.0.2" - }, - "engines": { - "node": "18 || 20 || >=22" + "balanced-match": "^1.0.0", + "concat-map": "0.0.1" } }, "node_modules/@eslint/config-array/node_modules/minimatch": { - "version": "10.2.4", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.4.tgz", - "integrity": "sha512-oRjTw/97aTBN0RHbYCdtF1MQfvusSIBQM0IZEgzl6426+8jSC0nF1a/GmnVLpfB9yyr6g6FTqWqiZVbxrtaCIg==", + "version": "3.1.5", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.5.tgz", + "integrity": "sha512-VgjWUsnnT6n+NUk6eZq77zeFdpW2LWDzP6zFGrCbHXiYNul5Dzqk2HHQ5uFH2DNW5Xbp8+jVzaeNt94ssEEl4w==", "dev": true, - "license": "BlueOak-1.0.0", + "license": "ISC", "dependencies": { - "brace-expansion": "^5.0.2" + "brace-expansion": "^1.1.7" }, "engines": { - "node": "18 || 20 || >=22" - }, - "funding": { - "url": "https://github.com/sponsors/isaacs" + "node": "*" } }, "node_modules/@eslint/config-helpers": { @@ -3061,27 +3044,15 @@ "url": "https://github.com/sponsors/epoberezkin" } }, - "node_modules/@eslint/eslintrc/node_modules/balanced-match": { - "version": "4.0.4", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", - "integrity": "sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==", - "dev": true, - "license": "MIT", - "engines": { - "node": "18 || 20 || >=22" - } - }, "node_modules/@eslint/eslintrc/node_modules/brace-expansion": { - "version": "5.0.4", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.4.tgz", - "integrity": "sha512-h+DEnpVvxmfVefa4jFbCf5HdH5YMDXRsmKflpf1pILZWRFlTbJpxeU55nJl4Smt5HQaGzg1o6RHFPJaOqnmBDg==", + "version": "1.1.12", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.12.tgz", + "integrity": "sha512-9T9UjW3r0UW5c1Q7GTwllptXwhvYmEzFhzMfZ9H7FQWt+uZePjZPjBP/W1ZEyZ1twGWom5/56TF4lPcqjnDHcg==", "dev": true, "license": "MIT", "dependencies": { - "balanced-match": "^4.0.2" - }, - "engines": { - "node": "18 || 20 || >=22" + "balanced-match": "^1.0.0", + "concat-map": "0.0.1" } }, "node_modules/@eslint/eslintrc/node_modules/ignore": { @@ -3102,19 +3073,16 @@ "license": "MIT" }, "node_modules/@eslint/eslintrc/node_modules/minimatch": { - "version": "10.2.4", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.4.tgz", - "integrity": "sha512-oRjTw/97aTBN0RHbYCdtF1MQfvusSIBQM0IZEgzl6426+8jSC0nF1a/GmnVLpfB9yyr6g6FTqWqiZVbxrtaCIg==", + "version": "3.1.5", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.5.tgz", + "integrity": "sha512-VgjWUsnnT6n+NUk6eZq77zeFdpW2LWDzP6zFGrCbHXiYNul5Dzqk2HHQ5uFH2DNW5Xbp8+jVzaeNt94ssEEl4w==", "dev": true, - "license": "BlueOak-1.0.0", + "license": "ISC", "dependencies": { - "brace-expansion": "^5.0.2" + "brace-expansion": "^1.1.7" }, "engines": { - "node": "18 || 20 || >=22" - }, - "funding": { - "url": "https://github.com/sponsors/isaacs" + "node": "*" } }, "node_modules/@eslint/js": { @@ -4257,15 +4225,6 @@ "resolved": "internal/events", "link": true }, - "node_modules/@nhsdigital/nhs-notify-event-schemas-supplier-config": { - "version": "1.0.1", - "resolved": "https://npm.pkg.github.com/download/@nhsdigital/nhs-notify-event-schemas-supplier-config/1.0.1/ff1ce566201ae291825acd5e771537229d6aa9ca", - "integrity": "sha512-gIZgfzgvkCfZE+HCosrVJ3tBce2FJRGfwPmtYtZDBG+ox/KvbpJFWXzJ5Jkh/42YzcVn2GxT1fy1L1F6pxiYWA==", - "dependencies": { - "@asyncapi/bundler": "^0.6.4", - "zod": "^4.1.12" - } - }, "node_modules/@nhsdigital/notify-supplier-api-consumer-contracts": { "resolved": "pact-contracts", "link": true @@ -12841,27 +12800,15 @@ "url": "https://github.com/sponsors/epoberezkin" } }, - "node_modules/eslint/node_modules/balanced-match": { - "version": "4.0.4", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", - "integrity": "sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==", - "dev": true, - "license": "MIT", - "engines": { - "node": "18 || 20 || >=22" - } - }, "node_modules/eslint/node_modules/brace-expansion": { - "version": "5.0.4", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.4.tgz", - "integrity": "sha512-h+DEnpVvxmfVefa4jFbCf5HdH5YMDXRsmKflpf1pILZWRFlTbJpxeU55nJl4Smt5HQaGzg1o6RHFPJaOqnmBDg==", + "version": "1.1.12", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.12.tgz", + "integrity": "sha512-9T9UjW3r0UW5c1Q7GTwllptXwhvYmEzFhzMfZ9H7FQWt+uZePjZPjBP/W1ZEyZ1twGWom5/56TF4lPcqjnDHcg==", "dev": true, "license": "MIT", "dependencies": { - "balanced-match": "^4.0.2" - }, - "engines": { - "node": "18 || 20 || >=22" + "balanced-match": "^1.0.0", + "concat-map": "0.0.1" } }, "node_modules/eslint/node_modules/glob-parent": { @@ -12895,19 +12842,16 @@ "license": "MIT" }, "node_modules/eslint/node_modules/minimatch": { - "version": "10.2.4", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.4.tgz", - "integrity": "sha512-oRjTw/97aTBN0RHbYCdtF1MQfvusSIBQM0IZEgzl6426+8jSC0nF1a/GmnVLpfB9yyr6g6FTqWqiZVbxrtaCIg==", + "version": "3.1.5", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.5.tgz", + "integrity": "sha512-VgjWUsnnT6n+NUk6eZq77zeFdpW2LWDzP6zFGrCbHXiYNul5Dzqk2HHQ5uFH2DNW5Xbp8+jVzaeNt94ssEEl4w==", "dev": true, - "license": "BlueOak-1.0.0", + "license": "ISC", "dependencies": { - "brace-expansion": "^5.0.2" + "brace-expansion": "^1.1.7" }, "engines": { - "node": "18 || 20 || >=22" - }, - "funding": { - "url": "https://github.com/sponsors/isaacs" + "node": "*" } }, "node_modules/espree": { From ab9c89b61a1a1a03fb9522ed82d7804852e733ee Mon Sep 17 00:00:00 2001 From: David Wass Date: Mon, 9 Mar 2026 15:34:36 +0000 Subject: [PATCH 17/24] package lock --- package-lock.json | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/package-lock.json b/package-lock.json index 84ca6562..fc88f6af 100644 --- a/package-lock.json +++ b/package-lock.json @@ -93,6 +93,7 @@ "@aws-sdk/client-dynamodb": "^3.984.0", "@aws-sdk/lib-dynamodb": "^3.984.0", "@internal/helpers": "*", + "@nhsdigital/nhs-notify-event-schemas-supplier-config": "^1.0.1", "pino": "^10.3.0", "zod": "^4.1.11", "zod-mermaid": "^1.0.9" @@ -294,6 +295,7 @@ "@nhsdigital/nhs-notify-event-schemas-letter-rendering": "^2.0.1", "@nhsdigital/nhs-notify-event-schemas-letter-rendering-v1": "npm:@nhsdigital/nhs-notify-event-schemas-letter-rendering@^1.1.5", "@nhsdigital/nhs-notify-event-schemas-supplier-api": "^1.0.8", + "@nhsdigital/nhs-notify-event-schemas-supplier-config": "^1.0.1", "@types/aws-lambda": "^8.10.148", "aws-lambda": "^1.0.7", "esbuild": "^0.27.2", @@ -4225,6 +4227,15 @@ "resolved": "internal/events", "link": true }, + "node_modules/@nhsdigital/nhs-notify-event-schemas-supplier-config": { + "version": "1.0.1", + "resolved": "https://npm.pkg.github.com/download/@nhsdigital/nhs-notify-event-schemas-supplier-config/1.0.1/ff1ce566201ae291825acd5e771537229d6aa9ca", + "integrity": "sha512-gIZgfzgvkCfZE+HCosrVJ3tBce2FJRGfwPmtYtZDBG+ox/KvbpJFWXzJ5Jkh/42YzcVn2GxT1fy1L1F6pxiYWA==", + "dependencies": { + "@asyncapi/bundler": "^0.6.4", + "zod": "^4.1.12" + } + }, "node_modules/@nhsdigital/notify-supplier-api-consumer-contracts": { "resolved": "pact-contracts", "link": true From 9240b8d816ebd0be0ec5c7a658a12006b4826d87 Mon Sep 17 00:00:00 2001 From: David Wass Date: Mon, 9 Mar 2026 15:46:37 +0000 Subject: [PATCH 18/24] lint fix --- .../src/services/__tests__/supplier-config.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts b/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts index 278ff2af..b6fa8e1e 100644 --- a/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts +++ b/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts @@ -5,7 +5,6 @@ import { getVolumeGroupDetails, } from "../supplier-config"; import { Deps } from "../../config/deps"; -import { warn } from "node:console"; function makeDeps(overrides: Partial = {}): Deps { const logger = { From 07325d8cd604b34950857959a2aefeef5d1a1815 Mon Sep 17 00:00:00 2001 From: David Wass Date: Thu, 12 Mar 2026 14:19:56 +0000 Subject: [PATCH 19/24] Log errors but do not fail for new allocations --- .../__tests__/allocate-handler.test.ts | 2 +- .../src/handler/allocate-handler.ts | 59 +++++++++++-------- 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts b/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts index 9c33b3e0..b86ab5dd 100644 --- a/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts +++ b/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts @@ -257,7 +257,7 @@ describe("allocate-handler", () => { () => {}, )) as SQSBatchResponse; - expect(result.batchItemFailures).toHaveLength(1); + expect(result.batchItemFailures).toHaveLength(0); expect(deps.logger.error).toHaveBeenCalled(); }); diff --git a/lambdas/supplier-allocator/src/handler/allocate-handler.ts b/lambdas/supplier-allocator/src/handler/allocate-handler.ts index a3ef61ed..401d1270 100644 --- a/lambdas/supplier-allocator/src/handler/allocate-handler.ts +++ b/lambdas/supplier-allocator/src/handler/allocate-handler.ts @@ -58,36 +58,45 @@ function validateType(event: unknown) { } async function getSupplierFromConfig(letterEvent: PreparedEvents, deps: Deps) { - const variantDetails: LetterVariant = await getVariantDetails( - letterEvent.data.letterVariantId, - deps, - ); - - const volumeGroupDetails: VolumeGroup = await getVolumeGroupDetails( - variantDetails.volumeGroupId, - deps, - ); - - const supplierAllocations: SupplierAllocation[] = - await getSupplierAllocationsForVolumeGroup( + try { + const variantDetails: LetterVariant = await getVariantDetails( + letterEvent.data.letterVariantId, + deps, + ); + + const volumeGroupDetails: VolumeGroup = await getVolumeGroupDetails( variantDetails.volumeGroupId, deps, - variantDetails.supplierId, ); - const supplierDetails: Supplier[] = await getSupplierDetails( - supplierAllocations, - deps, - ); - deps.logger.info({ - description: "Fetched supplier details for supplier allocations", - variantId: letterEvent.data.letterVariantId, - volumeGroupId: volumeGroupDetails.id, - supplierAllocationIds: supplierAllocations.map((a) => a.id), - supplierDetails, - }); + const supplierAllocations: SupplierAllocation[] = + await getSupplierAllocationsForVolumeGroup( + variantDetails.volumeGroupId, + deps, + variantDetails.supplierId, + ); + + const supplierDetails: Supplier[] = await getSupplierDetails( + supplierAllocations, + deps, + ); + deps.logger.info({ + description: "Fetched supplier details for supplier allocations", + variantId: letterEvent.data.letterVariantId, + volumeGroupId: volumeGroupDetails.id, + supplierAllocationIds: supplierAllocations.map((a) => a.id), + supplierDetails, + }); - return supplierDetails; + return supplierDetails; + } catch (error) { + deps.logger.error({ + description: "Error fetching supplier from config", + err: error, + variantId: letterEvent.data.letterVariantId, + }); + return []; + } } function getSupplier(letterEvent: PreparedEvents, deps: Deps): SupplierSpec { From e59635eec22385e01860aeac6e94e0776877d3d2 Mon Sep 17 00:00:00 2001 From: David Wass Date: Wed, 11 Mar 2026 13:20:54 +0000 Subject: [PATCH 20/24] CCM-13752 Added specificationBillingId --- .../src/__test__/letter-repository.test.ts | 1 + internal/datastore/src/types.ts | 1 + .../mappers/__tests__/letter-mapper.test.ts | 5 ++++ .../__tests__/letter-operations.test.ts | 1 + .../letter-updates-transformer.test.ts | 1 + .../__tests__/allocate-handler.test.ts | 10 ++++++-- .../src/__tests__/update-letter-queue.test.ts | 1 + .../handler/__tests__/upsert-handler.test.ts | 24 +++++++++++++++---- .../src/handler/upsert-handler.ts | 6 ++++- .../helpers/create-letter-helpers.test.ts | 3 +++ .../src/helpers/create-letter-helpers.ts | 2 ++ 11 files changed, 48 insertions(+), 7 deletions(-) diff --git a/internal/datastore/src/__test__/letter-repository.test.ts b/internal/datastore/src/__test__/letter-repository.test.ts index 193c1c07..286d7975 100644 --- a/internal/datastore/src/__test__/letter-repository.test.ts +++ b/internal/datastore/src/__test__/letter-repository.test.ts @@ -30,6 +30,7 @@ function createLetter( source: "/data-plane/letter-rendering/pdf", subject: `client/1/letter-request/${letterId}`, billingRef: "specification1", + specificationBillingId: "specification1", }; } diff --git a/internal/datastore/src/types.ts b/internal/datastore/src/types.ts index bb0843f8..62a3f611 100644 --- a/internal/datastore/src/types.ts +++ b/internal/datastore/src/types.ts @@ -53,6 +53,7 @@ export const LetterSchema = LetterSchemaBase.extend({ source: z.string(), subject: z.string(), billingRef: z.string(), + specificationBillingId: z.string(), }).describe("Letter"); /** diff --git a/lambdas/api-handler/src/mappers/__tests__/letter-mapper.test.ts b/lambdas/api-handler/src/mappers/__tests__/letter-mapper.test.ts index fa7f9f81..d6440eee 100644 --- a/lambdas/api-handler/src/mappers/__tests__/letter-mapper.test.ts +++ b/lambdas/api-handler/src/mappers/__tests__/letter-mapper.test.ts @@ -28,6 +28,7 @@ describe("letter-mapper", () => { ttl: 123, source: "/data-plane/letter-rendering/pdf", subject: "letter-rendering/source/letter/letter-id", + specificationBillingId: "billing123", }; const result: PatchLetterResponse = mapToPatchLetterResponse(letter); @@ -64,6 +65,7 @@ describe("letter-mapper", () => { reasonText: "Reason text", source: "/data-plane/letter-rendering/pdf", subject: "letter-rendering/source/letter/letter-id", + specificationBillingId: "billing123", }; const result: PatchLetterResponse = mapToPatchLetterResponse(letter); @@ -100,6 +102,7 @@ describe("letter-mapper", () => { ttl: 123, source: "/data-plane/letter-rendering/pdf", subject: "letter-rendering/source/letter/letter-id", + specificationBillingId: "billing123", }; const result: GetLetterResponse = mapToGetLetterResponse(letter); @@ -136,6 +139,7 @@ describe("letter-mapper", () => { reasonText: "Reason text", source: "/data-plane/letter-rendering/pdf", subject: "letter-rendering/source/letter/letter-id", + specificationBillingId: "billing123", }; const result: GetLetterResponse = mapToGetLetterResponse(letter); @@ -174,6 +178,7 @@ describe("letter-mapper", () => { reasonText: "Reason text", source: "/data-plane/letter-rendering/pdf", subject: "letter-rendering/source/letter/letter-id", + specificationBillingId: "billing123", }; const result: GetLettersResponse = mapToGetLettersResponse([ diff --git a/lambdas/api-handler/src/services/__tests__/letter-operations.test.ts b/lambdas/api-handler/src/services/__tests__/letter-operations.test.ts index 08e103ab..be69387e 100644 --- a/lambdas/api-handler/src/services/__tests__/letter-operations.test.ts +++ b/lambdas/api-handler/src/services/__tests__/letter-operations.test.ts @@ -41,6 +41,7 @@ function makeLetter(id: string, status: Letter["status"]): Letter { reasonText: "Reason text", source: "/data-plane/letter-rendering/pdf", subject: "letter-rendering/source/letter/letter-id", + specificationBillingId: "billing123", }; } diff --git a/lambdas/letter-updates-transformer/src/__tests__/letter-updates-transformer.test.ts b/lambdas/letter-updates-transformer/src/__tests__/letter-updates-transformer.test.ts index fd11de13..9a805f44 100644 --- a/lambdas/letter-updates-transformer/src/__tests__/letter-updates-transformer.test.ts +++ b/lambdas/letter-updates-transformer/src/__tests__/letter-updates-transformer.test.ts @@ -339,6 +339,7 @@ function generateLetter(status: LetterStatus, id?: string): Letter { supplierStatus: `supplier1#${status}`, supplierStatusSk: "2025-12-10T11:12:54Z#1", ttl: 1_234_567_890, + specificationBillingId: "billing1", }; } diff --git a/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts b/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts index b86ab5dd..4fc54643 100644 --- a/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts +++ b/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts @@ -26,8 +26,14 @@ function makeDeps(overrides: Partial = {}): Deps { const env = { VARIANT_MAP: { - "variant-1": { supplierId: "supplier-1", specId: "spec-1" }, - "variant-2": { supplierId: "supplier-2", specId: "spec-2" }, + "variant-1": { + supplierId: "supplier-1", + specId: "spec-1", + }, + "variant-2": { + supplierId: "supplier-2", + specId: "spec-2", + }, }, }; diff --git a/lambdas/update-letter-queue/src/__tests__/update-letter-queue.test.ts b/lambdas/update-letter-queue/src/__tests__/update-letter-queue.test.ts index 03f9ff72..f7475a8c 100644 --- a/lambdas/update-letter-queue/src/__tests__/update-letter-queue.test.ts +++ b/lambdas/update-letter-queue/src/__tests__/update-letter-queue.test.ts @@ -46,6 +46,7 @@ function generateLetter(status: LetterStatus, id?: string): Letter { source: "test-source", subject: "test-subject", billingRef: "billing-ref-1", + specificationBillingId: "billing1", }; } diff --git a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts index 6292de0f..6ddaa280 100644 --- a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts +++ b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts @@ -97,6 +97,7 @@ function createSupplierStatusChangeEventWithoutSupplier( billingRef: "1y3q9v1zzzz", status: "RETURNED", supplierId: "", + specificationBillingId: "billing1", }, datacontenttype: "application/json", dataschema: @@ -151,6 +152,7 @@ function createSupplierStatusChangeEvent( billingRef: "1y3q9v1zzzz", status: "RETURNED", supplierId: "supplier1", + specificationBillingId: "billing1", }, datacontenttype: "application/json", dataschema: @@ -211,11 +213,17 @@ describe("createUpsertLetterHandler", () => { test("processes all records successfully and returns no batch failures", async () => { const v2message = { letterEvent: createPreparedV2Event(), - supplierSpec: { supplierId: "supplier1", specId: "spec1" }, + supplierSpec: { + supplierId: "supplier1", + specId: "spec1", + }, }; const v1message = { letterEvent: createPreparedV1Event(), - supplierSpec: { supplierId: "supplier1", specId: "spec1" }, + supplierSpec: { + supplierId: "supplier1", + specId: "spec1", + }, }; const evt: SQSEvent = createSQSEvent([ @@ -249,6 +257,7 @@ describe("createUpsertLetterHandler", () => { expect(insertedV2Letter.status).toBe("PENDING"); expect(insertedV2Letter.groupId).toBe("client1campaign1template1"); expect(insertedV2Letter.source).toBe("/data-plane/letter-rendering/test"); + expect(insertedV2Letter.specificationBillingId).toBe("spec1"); const insertedV1Letter = (mockedDeps.letterRepo.putLetter as jest.Mock).mock .calls[1][0]; @@ -260,6 +269,7 @@ describe("createUpsertLetterHandler", () => { expect(insertedV1Letter.status).toBe("PENDING"); expect(insertedV1Letter.groupId).toBe("client1campaign1template1"); expect(insertedV1Letter.source).toBe("/data-plane/letter-rendering/test"); + expect(insertedV1Letter.specificationBillingId).toBe("spec1"); const updatedLetter = ( mockedDeps.letterRepo.updateLetterStatus as jest.Mock @@ -472,14 +482,20 @@ describe("createUpsertLetterHandler", () => { id: "7b9a03ca-342a-4150-b56b-989109c45615", domainId: "ok", }), - supplierSpec: { supplierId: "supplier1", specId: "spec1" }, + supplierSpec: { + supplierId: "supplier1", + specId: "spec1", + }, }; const message2 = { letterEvent: createPreparedV2Event({ id: "7b9a03ca-342a-4150-b56b-989109c45616", domainId: "fail", }), - supplierSpec: { supplierId: "supplier1", specId: "spec1" }, + supplierSpec: { + supplierId: "supplier1", + specId: "spec1", + }, }; const evt: SQSEvent = createSQSEvent([ createSqsRecord("ok-msg", JSON.stringify(message1)), diff --git a/lambdas/upsert-letter/src/handler/upsert-handler.ts b/lambdas/upsert-letter/src/handler/upsert-handler.ts index ada416ec..fd8ddcd6 100644 --- a/lambdas/upsert-letter/src/handler/upsert-handler.ts +++ b/lambdas/upsert-letter/src/handler/upsert-handler.ts @@ -117,6 +117,7 @@ function mapToInsertLetter( createdAt: now, updatedAt: now, billingRef, + specificationBillingId: spec, }; } @@ -235,7 +236,10 @@ export default function createUpsertLetterHandler(deps: Deps): SQSHandler { await runUpsert( operation, letterEvent, - supplierSpec ?? { supplierId: "unknown", specId: "unknown" }, + supplierSpec ?? { + supplierId: "unknown", + specId: "unknown", + }, deps, ); diff --git a/scripts/utilities/letter-test-data/src/__test__/helpers/create-letter-helpers.test.ts b/scripts/utilities/letter-test-data/src/__test__/helpers/create-letter-helpers.test.ts index 9f366d2d..9722d2ff 100644 --- a/scripts/utilities/letter-test-data/src/__test__/helpers/create-letter-helpers.test.ts +++ b/scripts/utilities/letter-test-data/src/__test__/helpers/create-letter-helpers.test.ts @@ -62,6 +62,7 @@ describe("Create letter helpers", () => { source: "/data-plane/letter-rendering/letter-test-data", subject: "supplier-api/letter-test-data/letterId", billingRef: "specificationId", + specificationBillingId: "specificationId", }); }); @@ -110,6 +111,7 @@ describe("Create letter helpers", () => { billingRef: "specificationId", source: "/data-plane/letter-rendering/letter-test-data", subject: "supplier-api/letter-test-data/letterId", + specificationBillingId: "specificationId", }); }); @@ -140,6 +142,7 @@ describe("Create letter helpers", () => { source: "/data-plane/letter-rendering/letter-test-data", subject: "supplier-api/letter-test-data/testLetterId", billingRef: "testSpecId", + specificationBillingId: "testSpecId", }); }); }); diff --git a/scripts/utilities/letter-test-data/src/helpers/create-letter-helpers.ts b/scripts/utilities/letter-test-data/src/helpers/create-letter-helpers.ts index 20131b56..4b1983e3 100644 --- a/scripts/utilities/letter-test-data/src/helpers/create-letter-helpers.ts +++ b/scripts/utilities/letter-test-data/src/helpers/create-letter-helpers.ts @@ -49,6 +49,7 @@ export async function createLetter(params: { source: "/data-plane/letter-rendering/letter-test-data", subject: `supplier-api/letter-test-data/${letterId}`, billingRef: specificationId, + specificationBillingId: specificationId, }; const letterRecord = await letterRepository.putLetter(letter); @@ -78,6 +79,7 @@ export function createLetterDto(params: { source: "/data-plane/letter-rendering/letter-test-data", subject: `supplier-api/letter-test-data/${letterId}`, billingRef: specificationId, + specificationBillingId: specificationId, }; return letter; From 366a7b7f39117694b3bbc7ceb22074cc0fab82c1 Mon Sep 17 00:00:00 2001 From: David Wass Date: Wed, 11 Mar 2026 15:52:50 +0000 Subject: [PATCH 21/24] add to cloudevents --- internal/events/package.json | 2 +- internal/events/src/domain/letter.ts | 7 +++++++ internal/events/src/events/__tests__/letter-mapper.test.ts | 2 ++ .../events/__tests__/letter-status-change-events.test.ts | 1 + .../letter.ACCEPTED-with-invalid-major-version.json | 1 + .../testData/letter.ACCEPTED-with-missing-fields.json | 1 + .../src/events/__tests__/testData/letter.ACCEPTED.json | 1 + .../src/events/__tests__/testData/letter.FORWARDED.json | 1 + .../src/events/__tests__/testData/letter.RETURNED.json | 1 + internal/events/src/events/letter-mapper.ts | 1 + package-lock.json | 2 +- 11 files changed, 18 insertions(+), 2 deletions(-) diff --git a/internal/events/package.json b/internal/events/package.json index d8b6626c..cf2e4f0e 100644 --- a/internal/events/package.json +++ b/internal/events/package.json @@ -37,5 +37,5 @@ "typecheck": "tsc --noEmit" }, "types": "dist/index.d.ts", - "version": "1.0.13" + "version": "1.0.14" } diff --git a/internal/events/src/domain/letter.ts b/internal/events/src/domain/letter.ts index 67ed8bbe..c5aa0059 100644 --- a/internal/events/src/domain/letter.ts +++ b/internal/events/src/domain/letter.ts @@ -83,6 +83,13 @@ The identifier will be included as the origin domain in the subject of any corre examples: ["1y3q9v1zzzz"], }), + specificationBillingId: z.string().meta({ + title: "Specification Billing ID", + description: + "The billing ID from the letter specification which was used to produce a letter pack for this request.", + examples: ["1y3q9v1zzzz"], + }), + supplierId: z.string().meta({ title: "Supplier ID", description: "Supplier ID allocated to the letter during creation.", diff --git a/internal/events/src/events/__tests__/letter-mapper.test.ts b/internal/events/src/events/__tests__/letter-mapper.test.ts index c870dc91..26ea6d00 100644 --- a/internal/events/src/events/__tests__/letter-mapper.test.ts +++ b/internal/events/src/events/__tests__/letter-mapper.test.ts @@ -16,6 +16,7 @@ describe("letter-mapper", () => { updatedAt: "2025-11-24T15:55:18.000Z", source: "letter-rendering/source/test", subject: "letter-rendering/source/letter/letter-id", + specificationBillingId: "billing123", } as Letter; const source = "/data-plane/supplier-api/nhs-supplier-api-dev/main/letters"; const event = mapLetterToCloudEvent(letter, source); @@ -35,6 +36,7 @@ describe("letter-mapper", () => { status: "PRINTED", specificationId: "spec1", billingRef: "spec1", + specificationBillingId: "billing123", supplierId: "supplier1", groupId: "group1", reasonCode: "R02", diff --git a/internal/events/src/events/__tests__/letter-status-change-events.test.ts b/internal/events/src/events/__tests__/letter-status-change-events.test.ts index 48215545..6ccd4020 100644 --- a/internal/events/src/events/__tests__/letter-status-change-events.test.ts +++ b/internal/events/src/events/__tests__/letter-status-change-events.test.ts @@ -40,6 +40,7 @@ describe("LetterStatus event validations", () => { billingRef: "1y3q9v1zzzz", groupId: "client_template", status, + specificationBillingId: "billing123", }), }), ); diff --git a/internal/events/src/events/__tests__/testData/letter.ACCEPTED-with-invalid-major-version.json b/internal/events/src/events/__tests__/testData/letter.ACCEPTED-with-invalid-major-version.json index 192ea5e2..81d4ded6 100644 --- a/internal/events/src/events/__tests__/testData/letter.ACCEPTED-with-invalid-major-version.json +++ b/internal/events/src/events/__tests__/testData/letter.ACCEPTED-with-invalid-major-version.json @@ -9,6 +9,7 @@ "source": "/data-plane/letter-rendering/prod/render-pdf", "subject": "client/00f3b388-bbe9-41c9-9e76-052d37ee8988/letter-request/0o5Fs0EELR0fUjHjbCnEtdUwQe4_0o5Fs0EELR0fUjHjbCnEtdUwQe5" }, + "specificationBillingId": "billing123", "specificationId": "1y3q9v1zzzz", "status": "ACCEPTED", "supplierId": "supplier1" diff --git a/internal/events/src/events/__tests__/testData/letter.ACCEPTED-with-missing-fields.json b/internal/events/src/events/__tests__/testData/letter.ACCEPTED-with-missing-fields.json index 54000422..0f03dc39 100644 --- a/internal/events/src/events/__tests__/testData/letter.ACCEPTED-with-missing-fields.json +++ b/internal/events/src/events/__tests__/testData/letter.ACCEPTED-with-missing-fields.json @@ -8,6 +8,7 @@ "event": "f47ac10b-58cc-4372-a567-0e02b2c3d479", "source": "/data-plane/letter-rendering/prod/render-pdf" }, + "specificationBillingId": "billing123", "specificationId": "1y3q9v1zzzz", "status": "ACCEPTED", "supplierId": "supplier1" diff --git a/internal/events/src/events/__tests__/testData/letter.ACCEPTED.json b/internal/events/src/events/__tests__/testData/letter.ACCEPTED.json index 7ffac10f..8182cc1f 100644 --- a/internal/events/src/events/__tests__/testData/letter.ACCEPTED.json +++ b/internal/events/src/events/__tests__/testData/letter.ACCEPTED.json @@ -9,6 +9,7 @@ "source": "/data-plane/letter-rendering/prod/render-pdf", "subject": "client/00f3b388-bbe9-41c9-9e76-052d37ee8988/letter-request/0o5Fs0EELR0fUjHjbCnEtdUwQe4_0o5Fs0EELR0fUjHjbCnEtdUwQe5" }, + "specificationBillingId": "billing123", "specificationId": "1y3q9v1zzzz", "status": "ACCEPTED", "supplierId": "supplier1" diff --git a/internal/events/src/events/__tests__/testData/letter.FORWARDED.json b/internal/events/src/events/__tests__/testData/letter.FORWARDED.json index 28c6111f..fe53b766 100644 --- a/internal/events/src/events/__tests__/testData/letter.FORWARDED.json +++ b/internal/events/src/events/__tests__/testData/letter.FORWARDED.json @@ -11,6 +11,7 @@ }, "reasonCode": "RNIB", "reasonText": "RNIB", + "specificationBillingId": "billing123", "specificationId": "1y3q9v1zzzz", "status": "FORWARDED", "supplierId": "supplier1" diff --git a/internal/events/src/events/__tests__/testData/letter.RETURNED.json b/internal/events/src/events/__tests__/testData/letter.RETURNED.json index 07b28154..f35440e4 100644 --- a/internal/events/src/events/__tests__/testData/letter.RETURNED.json +++ b/internal/events/src/events/__tests__/testData/letter.RETURNED.json @@ -11,6 +11,7 @@ }, "reasonCode": "R07", "reasonText": "No such address", + "specificationBillingId": "billing123", "specificationId": "1y3q9v1zzzz", "status": "RETURNED", "supplierId": "supplier1" diff --git a/internal/events/src/events/letter-mapper.ts b/internal/events/src/events/letter-mapper.ts index 91f72988..4b6781e1 100644 --- a/internal/events/src/events/letter-mapper.ts +++ b/internal/events/src/events/letter-mapper.ts @@ -25,6 +25,7 @@ export function mapLetterToCloudEvent( status: letter.status, specificationId: letter.specificationId, billingRef: letter.billingRef, + specificationBillingId: letter.specificationBillingId, supplierId: letter.supplierId, groupId: letter.groupId, reasonCode: letter.reasonCode, diff --git a/package-lock.json b/package-lock.json index fc88f6af..7724b633 100644 --- a/package-lock.json +++ b/package-lock.json @@ -101,7 +101,7 @@ }, "internal/events": { "name": "@nhsdigital/nhs-notify-event-schemas-supplier-api", - "version": "1.0.13", + "version": "1.0.14", "license": "MIT", "dependencies": { "@asyncapi/bundler": "^0.6.4", From 7d4f544d3a1eb7e6f4a16ec77532d2fe86032dfd Mon Sep 17 00:00:00 2001 From: David Wass Date: Thu, 12 Mar 2026 10:46:18 +0000 Subject: [PATCH 22/24] target specific internal dev branch - DO NOT MERGE --- .github/workflows/stage-3-build.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/stage-3-build.yaml b/.github/workflows/stage-3-build.yaml index a8441e7f..766b9bfd 100644 --- a/.github/workflows/stage-3-build.yaml +++ b/.github/workflows/stage-3-build.yaml @@ -161,7 +161,8 @@ jobs: --targetAccountGroup "nhs-notify-supplier-api-dev" \ --terraformAction "apply" \ --overrideProjectName "nhs" \ - --overrideRoleName "nhs-main-acct-supplier-api-github-deploy" + --overrideRoleName "nhs-main-acct-supplier-api-github-deploy" \ + --internalRef "feature/CCM-15148" artefact-proxies: name: "Build proxies" runs-on: ubuntu-latest From 3dd83f42481ab9bc7e75f7f94ab3f40e434060c0 Mon Sep 17 00:00:00 2001 From: David Wass Date: Thu, 12 Mar 2026 11:44:09 +0000 Subject: [PATCH 23/24] CCM-15148 - Add BillingId to variant map --- .../terraform/components/api/README.md | 2 +- .../terraform/components/api/variables.tf | 8 +++--- .../src/config/__tests__/deps.test.ts | 1 + .../src/config/__tests__/env.test.ts | 4 ++- lambdas/supplier-allocator/src/config/env.ts | 1 + .../__tests__/allocate-handler.test.ts | 2 ++ .../src/handler/allocate-handler.ts | 2 +- .../handler/__tests__/upsert-handler.test.ts | 25 +++++++++++++------ .../src/handler/upsert-handler.ts | 8 ++++-- 9 files changed, 36 insertions(+), 17 deletions(-) diff --git a/infrastructure/terraform/components/api/README.md b/infrastructure/terraform/components/api/README.md index f1f278c5..0e6b2dbe 100644 --- a/infrastructure/terraform/components/api/README.md +++ b/infrastructure/terraform/components/api/README.md @@ -34,7 +34,7 @@ No requirements. | [group](#input\_group) | The group variables are being inherited from (often synonmous with account short-name) | `string` | n/a | yes | | [kms\_deletion\_window](#input\_kms\_deletion\_window) | When a kms key is deleted, how long should it wait in the pending deletion state? | `string` | `"30"` | no | | [letter\_table\_ttl\_hours](#input\_letter\_table\_ttl\_hours) | Number of hours to set as TTL on letters table | `number` | `24` | no | -| [letter\_variant\_map](#input\_letter\_variant\_map) | n/a | `map(object({ supplierId = string, specId = string }))` | 
{
  "lv1": {
    "specId": "spec1",
    "supplierId": "supplier1"
  },
  "lv2": {
    "specId": "spec2",
    "supplierId": "supplier1"
  },
  "lv3": {
    "specId": "spec3",
    "supplierId": "supplier2"
  }
}
| no | +| [letter\_variant\_map](#input\_letter\_variant\_map) | n/a | `map(object({ supplierId = string, specId = string, billingId = string }))` | 
{
  "lv1": {
    "billingId": "billing1",
    "specId": "spec1",
    "supplierId": "supplier1"
  },
  "lv2": {
    "billingId": "billing2",
    "specId": "spec2",
    "supplierId": "supplier1"
  },
  "lv3": {
    "billingId": "billing3",
    "specId": "spec3",
    "supplierId": "supplier2"
  }
}
| no | | [log\_level](#input\_log\_level) | The log level to be used in lambda functions within the component. Any log with a lower severity than the configured value will not be logged: https://docs.python.org/3/library/logging.html#levels | `string` | `"INFO"` | no | | [log\_retention\_in\_days](#input\_log\_retention\_in\_days) | The retention period in days for the Cloudwatch Logs events to be retained, default of 0 is indefinite | `number` | `0` | no | | [manually\_configure\_mtls\_truststore](#input\_manually\_configure\_mtls\_truststore) | Manually manage the truststore used for API Gateway mTLS (e.g. for prod environment) | `bool` | `false` | no | diff --git a/infrastructure/terraform/components/api/variables.tf b/infrastructure/terraform/components/api/variables.tf index 97169278..d7faa9a8 100644 --- a/infrastructure/terraform/components/api/variables.tf +++ b/infrastructure/terraform/components/api/variables.tf @@ -136,11 +136,11 @@ variable "eventpub_control_plane_bus_arn" { } variable "letter_variant_map" { - type = map(object({ supplierId = string, specId = string })) + type = map(object({ supplierId = string, specId = string, billingId = string })) default = { - "lv1" = { supplierId = "supplier1", specId = "spec1" }, - "lv2" = { supplierId = "supplier1", specId = "spec2" }, - "lv3" = { supplierId = "supplier2", specId = "spec3" } + "lv1" = { supplierId = "supplier1", specId = "spec1", billingId = "billing1" }, + "lv2" = { supplierId = "supplier1", specId = "spec2", billingId = "billing2" }, + "lv3" = { supplierId = "supplier2", specId = "spec3", billingId = "billing3" } } } diff --git a/lambdas/supplier-allocator/src/config/__tests__/deps.test.ts b/lambdas/supplier-allocator/src/config/__tests__/deps.test.ts index 6069dc07..7c2767f1 100644 --- a/lambdas/supplier-allocator/src/config/__tests__/deps.test.ts +++ b/lambdas/supplier-allocator/src/config/__tests__/deps.test.ts @@ -7,6 +7,7 @@ describe("createDependenciesContainer", () => { lv1: { supplierId: "supplier1", specId: "spec1", + billingId: "billing1", }, }, }; diff --git a/lambdas/supplier-allocator/src/config/__tests__/env.test.ts b/lambdas/supplier-allocator/src/config/__tests__/env.test.ts index 1d8e3a1f..d54c2c4f 100644 --- a/lambdas/supplier-allocator/src/config/__tests__/env.test.ts +++ b/lambdas/supplier-allocator/src/config/__tests__/env.test.ts @@ -19,7 +19,8 @@ describe("lambdaEnv", () => { process.env.VARIANT_MAP = `{ "lv1": { "supplierId": "supplier1", - "specId": "spec1" + "specId": "spec1", + "billingId": "billing1" } }`; @@ -31,6 +32,7 @@ describe("lambdaEnv", () => { lv1: { supplierId: "supplier1", specId: "spec1", + billingId: "billing1", }, }, }); diff --git a/lambdas/supplier-allocator/src/config/env.ts b/lambdas/supplier-allocator/src/config/env.ts index 5d924430..a3ef6bf0 100644 --- a/lambdas/supplier-allocator/src/config/env.ts +++ b/lambdas/supplier-allocator/src/config/env.ts @@ -5,6 +5,7 @@ const LetterVariantSchema = z.record( z.object({ supplierId: z.string(), specId: z.string(), + billingId: z.string(), }), ); export type LetterVariant = z.infer; diff --git a/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts b/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts index 4fc54643..646a2d2f 100644 --- a/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts +++ b/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts @@ -29,10 +29,12 @@ function makeDeps(overrides: Partial = {}): Deps { "variant-1": { supplierId: "supplier-1", specId: "spec-1", + billingId: "billing-1", }, "variant-2": { supplierId: "supplier-2", specId: "spec-2", + billingId: "billing-2", }, }, }; diff --git a/lambdas/supplier-allocator/src/handler/allocate-handler.ts b/lambdas/supplier-allocator/src/handler/allocate-handler.ts index 401d1270..89f37080 100644 --- a/lambdas/supplier-allocator/src/handler/allocate-handler.ts +++ b/lambdas/supplier-allocator/src/handler/allocate-handler.ts @@ -17,7 +17,7 @@ import { } from "../services/supplier-config"; import { Deps } from "../config/deps"; -type SupplierSpec = { supplierId: string; specId: string }; +type SupplierSpec = { supplierId: string; specId: string; billingId: string }; type PreparedEvents = LetterRequestPreparedEventV2 | LetterRequestPreparedEvent; // small envelope that must exist in all inputs diff --git a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts index 6ddaa280..e32d47d9 100644 --- a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts +++ b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts @@ -216,13 +216,15 @@ describe("createUpsertLetterHandler", () => { supplierSpec: { supplierId: "supplier1", specId: "spec1", + billingId: "billing1", }, }; const v1message = { letterEvent: createPreparedV1Event(), supplierSpec: { - supplierId: "supplier1", - specId: "spec1", + supplierId: "supplier2", + specId: "spec2", + billingId: "billing2", }, }; @@ -257,19 +259,19 @@ describe("createUpsertLetterHandler", () => { expect(insertedV2Letter.status).toBe("PENDING"); expect(insertedV2Letter.groupId).toBe("client1campaign1template1"); expect(insertedV2Letter.source).toBe("/data-plane/letter-rendering/test"); - expect(insertedV2Letter.specificationBillingId).toBe("spec1"); + expect(insertedV2Letter.specificationBillingId).toBe("billing1"); const insertedV1Letter = (mockedDeps.letterRepo.putLetter as jest.Mock).mock .calls[1][0]; expect(insertedV1Letter.id).toBe("letter1"); - expect(insertedV1Letter.supplierId).toBe("supplier1"); - expect(insertedV1Letter.specificationId).toBe("spec1"); - expect(insertedV1Letter.billingRef).toBe("spec1"); + expect(insertedV1Letter.supplierId).toBe("supplier2"); + expect(insertedV1Letter.specificationId).toBe("spec2"); + expect(insertedV1Letter.billingRef).toBe("spec2"); expect(insertedV1Letter.url).toBe("s3://letterDataBucket/letter1.pdf"); expect(insertedV1Letter.status).toBe("PENDING"); expect(insertedV1Letter.groupId).toBe("client1campaign1template1"); expect(insertedV1Letter.source).toBe("/data-plane/letter-rendering/test"); - expect(insertedV1Letter.specificationBillingId).toBe("spec1"); + expect(insertedV1Letter.specificationBillingId).toBe("billing2"); const updatedLetter = ( mockedDeps.letterRepo.updateLetterStatus as jest.Mock @@ -285,7 +287,12 @@ describe("createUpsertLetterHandler", () => { }); expect(mockMetrics.putMetric).toHaveBeenCalledWith( "MessagesProcessed", - 3, + 2, + "Count", + ); + expect(mockMetrics.putMetric).toHaveBeenCalledWith( + "MessagesProcessed", + 1, "Count", ); }); @@ -485,6 +492,7 @@ describe("createUpsertLetterHandler", () => { supplierSpec: { supplierId: "supplier1", specId: "spec1", + billingId: "billing1", }, }; const message2 = { @@ -495,6 +503,7 @@ describe("createUpsertLetterHandler", () => { supplierSpec: { supplierId: "supplier1", specId: "spec1", + billingId: "billing1", }, }; const evt: SQSEvent = createSQSEvent([ diff --git a/lambdas/upsert-letter/src/handler/upsert-handler.ts b/lambdas/upsert-letter/src/handler/upsert-handler.ts index fd8ddcd6..e332096f 100644 --- a/lambdas/upsert-letter/src/handler/upsert-handler.ts +++ b/lambdas/upsert-letter/src/handler/upsert-handler.ts @@ -16,12 +16,13 @@ import z from "zod"; import { MetricsLogger, Unit, metricScope } from "aws-embedded-metrics"; import { Deps } from "../config/deps"; -type SupplierSpec = { supplierId: string; specId: string }; +type SupplierSpec = { supplierId: string; specId: string; billingId: string }; type PreparedEvents = LetterRequestPreparedEventV2 | LetterRequestPreparedEvent; const SupplierSpecSchema = z.object({ supplierId: z.string().min(1), specId: z.string().min(1), + billingId: z.string().min(1), }); const PreparedEventUnionSchema = z.discriminatedUnion("type", [ @@ -63,6 +64,7 @@ function getOperationFromType(type: string): UpsertOperation { supplierSpec.supplierId, supplierSpec.specId, supplierSpec.specId, // use specId for now + supplierSpec.billingId, // use billingId for now ); await deps.letterRepo.putLetter(letterToInsert); @@ -99,6 +101,7 @@ function mapToInsertLetter( supplier: string, spec: string, billingRef: string, + billingId: string, ): InsertLetter { const now = new Date().toISOString(); return { @@ -117,7 +120,7 @@ function mapToInsertLetter( createdAt: now, updatedAt: now, billingRef, - specificationBillingId: spec, + specificationBillingId: billingId, }; } @@ -239,6 +242,7 @@ export default function createUpsertLetterHandler(deps: Deps): SQSHandler { supplierSpec ?? { supplierId: "unknown", specId: "unknown", + billingId: "unknown", }, deps, ); From 506107306b5b28816047ce11d0cb740795394f7e Mon Sep 17 00:00:00 2001 From: David Wass Date: Thu, 12 Mar 2026 14:44:16 +0000 Subject: [PATCH 24/24] update test utility --- .../src/__test__/letter-repository.test.ts | 2 +- scripts/utilities/letter-test-data/README.md | 2 ++ .../helpers/create-letter-helpers.test.ts | 11 ++++++++--- .../letter-test-data/src/cli/index.ts | 12 ++++++++++++ .../src/helpers/create-letter-helpers.ts | 18 ++++++++++++++---- 5 files changed, 37 insertions(+), 8 deletions(-) diff --git a/internal/datastore/src/__test__/letter-repository.test.ts b/internal/datastore/src/__test__/letter-repository.test.ts index 286d7975..0f614971 100644 --- a/internal/datastore/src/__test__/letter-repository.test.ts +++ b/internal/datastore/src/__test__/letter-repository.test.ts @@ -30,7 +30,7 @@ function createLetter( source: "/data-plane/letter-rendering/pdf", subject: `client/1/letter-request/${letterId}`, billingRef: "specification1", - specificationBillingId: "specification1", + specificationBillingId: "billing1", }; } diff --git a/scripts/utilities/letter-test-data/README.md b/scripts/utilities/letter-test-data/README.md index 28426888..52ed6616 100644 --- a/scripts/utilities/letter-test-data/README.md +++ b/scripts/utilities/letter-test-data/README.md @@ -16,6 +16,7 @@ npm run cli -- create-letter \ --letter-id letter-id \ --group-id group-id \ --specification-id specification-id \ + --billing-id billing-id \ --status PENDING ``` @@ -26,6 +27,7 @@ npm run cli -- create-letter-batch \ --awsAccountId 820178564574 \ --group-id group-id \ --specification-id specification-id \ + --billing-id billing-id \ --status PENDING \ --count 10 ``` diff --git a/scripts/utilities/letter-test-data/src/__test__/helpers/create-letter-helpers.test.ts b/scripts/utilities/letter-test-data/src/__test__/helpers/create-letter-helpers.test.ts index 9722d2ff..66af9c01 100644 --- a/scripts/utilities/letter-test-data/src/__test__/helpers/create-letter-helpers.test.ts +++ b/scripts/utilities/letter-test-data/src/__test__/helpers/create-letter-helpers.test.ts @@ -29,6 +29,7 @@ describe("Create letter helpers", () => { const targetFilename = "targetFilename"; const groupId = "groupId"; const specificationId = "specificationId"; + const billingId = "billingId"; const status = "PENDING" as LetterStatusType; const testLetter = "test-letter-standard"; @@ -39,6 +40,7 @@ describe("Create letter helpers", () => { targetFilename, groupId, specificationId, + billingId, status, letterRepository: mockedLetterRepository, testLetter, @@ -62,7 +64,7 @@ describe("Create letter helpers", () => { source: "/data-plane/letter-rendering/letter-test-data", subject: "supplier-api/letter-test-data/letterId", billingRef: "specificationId", - specificationBillingId: "specificationId", + specificationBillingId: "billingId", }); }); @@ -82,6 +84,7 @@ describe("Create letter helpers", () => { const targetFilename = "targetFilename"; const groupId = "groupId"; const specificationId = "specificationId"; + const billingId = "billingId"; const status = "PENDING" as LetterStatusType; const testLetter = "none"; @@ -92,6 +95,7 @@ describe("Create letter helpers", () => { targetFilename, groupId, specificationId, + billingId, status, letterRepository: mockedLetterRepository, testLetter, @@ -111,7 +115,7 @@ describe("Create letter helpers", () => { billingRef: "specificationId", source: "/data-plane/letter-rendering/letter-test-data", subject: "supplier-api/letter-test-data/letterId", - specificationBillingId: "specificationId", + specificationBillingId: "billingId", }); }); @@ -123,6 +127,7 @@ describe("Create letter helpers", () => { letterId: "testLetterId", supplierId: "testSupplierId", specificationId: "testSpecId", + billingId: "testBillingId", groupId: "testGroupId", status: "PENDING" as LetterStatusType, url: "s3://bucket/testSupplierId/testLetter.pdf", @@ -142,7 +147,7 @@ describe("Create letter helpers", () => { source: "/data-plane/letter-rendering/letter-test-data", subject: "supplier-api/letter-test-data/testLetterId", billingRef: "testSpecId", - specificationBillingId: "testSpecId", + specificationBillingId: "testBillingId", }); }); }); diff --git a/scripts/utilities/letter-test-data/src/cli/index.ts b/scripts/utilities/letter-test-data/src/cli/index.ts index 131fe224..982e9e4e 100644 --- a/scripts/utilities/letter-test-data/src/cli/index.ts +++ b/scripts/utilities/letter-test-data/src/cli/index.ts @@ -44,6 +44,10 @@ async function main() { type: "string", demandOption: false, }, + "billing-id": { + type: "string", + demandOption: false, + }, "ttl-hours": { type: "number", demandOption: false, @@ -83,6 +87,7 @@ async function main() { const targetFilename = `${letterId}.pdf`; const groupId = argv.groupId ?? randomUUID(); const specificationId = argv.specificationId ?? randomUUID(); + const billingId = argv.billingId ?? randomUUID(); const { status } = argv; const { environment } = argv; const { ttlHours } = argv; @@ -96,6 +101,7 @@ async function main() { targetFilename, groupId, specificationId, + billingId, status: status as LetterStatusType, letterRepository, testLetter, @@ -130,6 +136,10 @@ async function main() { type: "string", demandOption: false, }, + "billing-id": { + type: "string", + demandOption: false, + }, "ttl-hours": { type: "number", demandOption: false, @@ -174,6 +184,7 @@ async function main() { const { supplierId } = argv; const groupId = argv.groupId ?? randomUUID(); const specificationId = argv.specificationId ?? randomUUID(); + const billingId = argv.billingId ?? randomUUID(); const { status } = argv; const { environment } = argv; const { ttlHours } = argv; @@ -206,6 +217,7 @@ async function main() { supplierId, groupId, specificationId, + billingId, status: status as LetterStatusType, url, }), diff --git a/scripts/utilities/letter-test-data/src/helpers/create-letter-helpers.ts b/scripts/utilities/letter-test-data/src/helpers/create-letter-helpers.ts index 4b1983e3..6278dbbe 100644 --- a/scripts/utilities/letter-test-data/src/helpers/create-letter-helpers.ts +++ b/scripts/utilities/letter-test-data/src/helpers/create-letter-helpers.ts @@ -11,12 +11,14 @@ export async function createLetter(params: { supplierId: string; targetFilename: string; specificationId: string; + billingId: string; groupId: string; status: LetterStatusType; letterRepository: LetterRepository; testLetter: string; }) { const { + billingId, bucketName, groupId, letterId, @@ -49,7 +51,7 @@ export async function createLetter(params: { source: "/data-plane/letter-rendering/letter-test-data", subject: `supplier-api/letter-test-data/${letterId}`, billingRef: specificationId, - specificationBillingId: specificationId, + specificationBillingId: billingId, }; const letterRecord = await letterRepository.putLetter(letter); @@ -60,12 +62,20 @@ export function createLetterDto(params: { letterId: string; supplierId: string; specificationId: string; + billingId: string; groupId: string; status: LetterStatusType; url: string; }) { - const { groupId, letterId, specificationId, status, supplierId, url } = - params; + const { + billingId, + groupId, + letterId, + specificationId, + status, + supplierId, + url, + } = params; const letter: Omit = { id: letterId, @@ -79,7 +89,7 @@ export function createLetterDto(params: { source: "/data-plane/letter-rendering/letter-test-data", subject: `supplier-api/letter-test-data/${letterId}`, billingRef: specificationId, - specificationBillingId: specificationId, + specificationBillingId: billingId, }; return letter;