From 9cc6815c1ac7b695c753ee86a3f36ed11947fd63 Mon Sep 17 00:00:00 2001 From: David Wass Date: Thu, 19 Mar 2026 15:34:59 +0000 Subject: [PATCH 1/2] CCM-13372 - Select Preferred Pack --- .../api/ddb_table_supplier_configuration.tf | 12 ++ .../api/module_lambda_supplier_allocator.tf | 3 +- internal/datastore/src/__test__/db.ts | 11 + .../supplier-config-repository.test.ts | 82 ++++++++ .../src/supplier-config-repository.ts | 39 ++++ .../__tests__/allocate-handler.test.ts | 13 +- .../src/handler/allocate-handler.ts | 38 +++- .../__tests__/supplier-config.test.ts | 188 ++++++++++++++---- .../src/services/supplier-config.ts | 73 ++++++- 9 files changed, 411 insertions(+), 48 deletions(-) diff --git a/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf b/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf index 8d089a19..b16295ad 100644 --- a/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf +++ b/infrastructure/terraform/components/api/ddb_table_supplier_configuration.tf @@ -30,6 +30,11 @@ resource "aws_dynamodb_table" "supplier-configuration" { type = "S" } + attribute { + name = "packSpecificationId" + 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" @@ -45,6 +50,13 @@ resource "aws_dynamodb_table" "supplier-configuration" { projection_type = "ALL" } + global_secondary_index { + name = "packSpecificationId-index" + hash_key = "PK" + range_key = "packSpecificationId" + projection_type = "ALL" + } + point_in_time_recovery { enabled = true } diff --git a/infrastructure/terraform/components/api/module_lambda_supplier_allocator.tf b/infrastructure/terraform/components/api/module_lambda_supplier_allocator.tf index b568307c..c2013fb6 100644 --- a/infrastructure/terraform/components/api/module_lambda_supplier_allocator.tf +++ b/infrastructure/terraform/components/api/module_lambda_supplier_allocator.tf @@ -94,8 +94,7 @@ data "aws_iam_policy_document" "supplier_allocator_lambda" { resources = [ aws_dynamodb_table.supplier-configuration.arn, - "${aws_dynamodb_table.supplier-configuration.arn}/index/volumeGroup-index" - + "${aws_dynamodb_table.supplier-configuration.arn}/index/*" ] } } diff --git a/internal/datastore/src/__test__/db.ts b/internal/datastore/src/__test__/db.ts index 481166e6..2e251113 100644 --- a/internal/datastore/src/__test__/db.ts +++ b/internal/datastore/src/__test__/db.ts @@ -165,11 +165,22 @@ const createSupplierConfigTableCommand = new CreateTableCommand({ ProjectionType: "ALL", }, }, + { + IndexName: "packSpecificationId-index", + KeySchema: [ + { AttributeName: "PK", KeyType: "HASH" }, // Partition key for GSI + { AttributeName: "packSpecificationId", KeyType: "RANGE" }, // Sort key for GSI + ], + Projection: { + ProjectionType: "ALL", + }, + }, ], AttributeDefinitions: [ { AttributeName: "PK", AttributeType: "S" }, { AttributeName: "SK", AttributeType: "S" }, { AttributeName: "volumeGroup", AttributeType: "S" }, + { AttributeName: "packSpecificationId", AttributeType: "S" }, ], }); diff --git a/internal/datastore/src/__test__/supplier-config-repository.test.ts b/internal/datastore/src/__test__/supplier-config-repository.test.ts index 9fde74f9..fba53e5e 100644 --- a/internal/datastore/src/__test__/supplier-config-repository.test.ts +++ b/internal/datastore/src/__test__/supplier-config-repository.test.ts @@ -263,4 +263,86 @@ describe("SupplierConfigRepository", () => { `Supplier with id ${supplierId} not found`, ); }); + + test("getSupplierPacksForPackSpecification returns correct supplier packs", async () => { + const packSpecId = "pack-spec-123"; + const supplierId = "supplier-123"; + const supplierPackId = "supplier-pack-123"; + + await dbContext.docClient.send( + new PutCommand({ + TableName: dbContext.config.supplierConfigTableName, + Item: { + PK: "SUPPLIER_PACK", + SK: supplierPackId, + id: supplierPackId, + packSpecificationId: packSpecId, + supplierId, + status: "PROD", + approval: "APPROVED", + }, + }), + ); + + const result = + await repository.getSupplierPacksForPackSpecification(packSpecId); + expect(result).toEqual([ + { + approval: "APPROVED", + id: supplierPackId, + packSpecificationId: packSpecId, + supplierId, + status: "PROD", + }, + ]); + }); + + test("getSupplierPacksForPackSpecification returns empty array for non-existent pack specification", async () => { + const packSpecId = "non-existent-pack-spec"; + const result = + await repository.getSupplierPacksForPackSpecification(packSpecId); + expect(result).toEqual([]); + }); + + test("getPackSpecification returns correct pack specification details", async () => { + const packSpecId = "pack-spec-123"; + + await dbContext.docClient.send( + new PutCommand({ + TableName: dbContext.config.supplierConfigTableName, + Item: { + PK: "PACK_SPECIFICATION", + SK: packSpecId, + id: packSpecId, + name: `Pack Specification ${packSpecId}`, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + version: 1, + billingId: `billing-${packSpecId}`, + postage: { id: "postageId", size: "STANDARD" }, + status: "PROD", + }, + }), + ); + + const result = await repository.getPackSpecification(packSpecId); + expect(result).toEqual({ + billingId: `billing-${packSpecId}`, + createdAt: expect.any(String), + id: packSpecId, + name: `Pack Specification ${packSpecId}`, + postage: { id: "postageId", size: "STANDARD" }, + updatedAt: expect.any(String), + version: 1, + status: "PROD", + }); + }); + + test("getPackSpecification throws error for non-existent pack specification", async () => { + const packSpecId = "non-existent-pack-spec"; + + await expect(repository.getPackSpecification(packSpecId)).rejects.toThrow( + `No pack specification found for id ${packSpecId}`, + ); + }); }); diff --git a/internal/datastore/src/supplier-config-repository.ts b/internal/datastore/src/supplier-config-repository.ts index 1f82bb0c..d4698e5c 100644 --- a/internal/datastore/src/supplier-config-repository.ts +++ b/internal/datastore/src/supplier-config-repository.ts @@ -5,12 +5,16 @@ import { } from "@aws-sdk/lib-dynamodb"; import { $LetterVariant, + $PackSpecification, $Supplier, $SupplierAllocation, + $SupplierPack, $VolumeGroup, LetterVariant, + PackSpecification, Supplier, SupplierAllocation, + SupplierPack, VolumeGroup, } from "@nhsdigital/nhs-notify-event-schemas-supplier-config"; @@ -97,4 +101,39 @@ export class SupplierConfigRepository { } return suppliers; } + + async getSupplierPacksForPackSpecification( + packSpecId: string, + ): Promise { + const result = await this.ddbClient.send( + new QueryCommand({ + TableName: this.config.supplierConfigTableName, + IndexName: "packSpecificationId-index", + KeyConditionExpression: "#pk = :pk AND #packSpecId = :packSpecId", + ExpressionAttributeNames: { + "#pk": "PK", + "#packSpecId": "packSpecificationId", + }, + ExpressionAttributeValues: { + ":pk": "SUPPLIER_PACK", + ":packSpecId": packSpecId, + }, + }), + ); + + return $SupplierPack.array().parse(result.Items); + } + + async getPackSpecification(packSpecId: string): Promise { + const result = await this.ddbClient.send( + new GetCommand({ + TableName: this.config.supplierConfigTableName, + Key: { PK: "PACK_SPECIFICATION", SK: packSpecId }, + }), + ); + if (!result.Item) { + throw new Error(`No pack specification found for id ${packSpecId}`); + } + return $PackSpecification.parse(result.Item); + } } 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 2cea9f7e..e3d07518 100644 --- a/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts +++ b/lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts @@ -1,6 +1,6 @@ +import { SQSClient, SendMessageCommand } from "@aws-sdk/client-sqs"; import { SQSEvent, SQSRecord } from "aws-lambda"; import pino from "pino"; -import { SQSClient, 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 { @@ -158,6 +158,17 @@ function setupDefaultMocks() { priority: 1, billingId: "billing-1", }); + (supplierConfig.getPreferredSupplierPacks as jest.Mock).mockResolvedValue([ + { + packSpecificationId: "pack-spec-1", + }, + ]); + (supplierConfig.getPackSpecification as jest.Mock).mockResolvedValue({ + id: "pack-spec-1", + type: "A4", + colour: false, + duplex: false, + }); } describe("createSupplierAllocatorHandler", () => { diff --git a/lambdas/supplier-allocator/src/handler/allocate-handler.ts b/lambdas/supplier-allocator/src/handler/allocate-handler.ts index 2288fae1..ebfede74 100644 --- a/lambdas/supplier-allocator/src/handler/allocate-handler.ts +++ b/lambdas/supplier-allocator/src/handler/allocate-handler.ts @@ -3,8 +3,10 @@ import { SendMessageCommand } from "@aws-sdk/client-sqs"; import { LetterRequestPreparedEvent } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering-v1"; import { LetterVariant, + PackSpecification, Supplier, SupplierAllocation, + SupplierPack, VolumeGroup, } from "@nhsdigital/nhs-notify-event-schemas-supplier-config"; import { LetterRequestPreparedEventV2 } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering"; @@ -12,8 +14,11 @@ import z from "zod"; import { Unit } from "aws-embedded-metrics"; import { MetricEntry, MetricStatus, buildEMFObject } from "@internal/helpers"; import { + getPackSpecification, + getPreferredSupplierPacks, getSupplierAllocationsForVolumeGroup, getSupplierDetails, + getSuppliersWithValidPack, getVariantDetails, getVolumeGroupDetails, } from "../services/supplier-config"; @@ -83,19 +88,44 @@ async function getSupplierFromConfig(letterEvent: PreparedEvents, deps: Deps) { variantDetails.supplierId, ); - const supplierDetails: Supplier[] = await getSupplierDetails( - supplierAllocations, + const supplierIds = supplierAllocations.map((alloc) => alloc.supplier); + + const allocatedSuppliers: Supplier[] = await getSupplierDetails( + supplierIds, deps, ); + + const preferredSupplierPacks: SupplierPack[] = + await getPreferredSupplierPacks( + variantDetails.packSpecificationIds, + allocatedSuppliers, + deps, + ); + + const preferredPack: PackSpecification = await getPackSpecification( + preferredSupplierPacks[0].packSpecificationId, + deps, + ); + + const suppliersForPack: Supplier[] = await getSuppliersWithValidPack( + allocatedSuppliers, + preferredPack.id, + 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, + allocatedSuppliers, + eligiblePacks: variantDetails.packSpecificationIds, + preferredSupplierPacks, + preferredPack, + suppliersForPack, }); - return supplierDetails; + return allocatedSuppliers; } catch (error) { deps.logger.error({ description: "Error fetching supplier from config", 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 7941d1f0..08cee6be 100644 --- a/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts +++ b/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts @@ -1,6 +1,9 @@ import { + getPackSpecification, + getPreferredSupplierPacks, getSupplierAllocationsForVolumeGroup, getSupplierDetails, + getSuppliersWithValidPack, getVariantDetails, getVolumeGroupDetails, } from "../supplier-config"; @@ -31,7 +34,7 @@ describe("supplier-config service", () => { afterEach(() => jest.resetAllMocks()); describe("getVariantDetails", () => { - it("returns variant details", async () => { + it("returns variant details for valid id", async () => { const variant = { id: "v1", volumeGroupId: "g1" } as any; const deps = makeDeps(); deps.supplierConfigRepo.getLetterVariant = jest @@ -188,10 +191,7 @@ describe("supplier-config service", () => { describe("getSupplierDetails", () => { it("returns supplier details when found", async () => { - const allocations = [ - { supplier: "s1", variantId: "v1" }, - { supplier: "s2", variantId: "v2" }, - ] as any[]; + const supplierIds = ["s1", "s2"]; const suppliers = [ { id: "s1", name: "Supplier 1", status: "PROD" }, { id: "s2", name: "Supplier 2", status: "PROD" }, @@ -201,7 +201,7 @@ describe("supplier-config service", () => { .fn() .mockResolvedValue(suppliers); - const result = await getSupplierDetails(allocations, deps); + const result = await getSupplierDetails(supplierIds, deps); expect(result).toEqual(suppliers); expect(deps.supplierConfigRepo.getSuppliersDetails).toHaveBeenCalledWith([ @@ -211,23 +211,19 @@ describe("supplier-config service", () => { }); it("throws when no supplier details found", async () => { - const allocations = [{ supplier: "s1", variantId: "v1" }] as any[]; + const supplierIds = ["s1"]; const deps = makeDeps(); deps.supplierConfigRepo.getSuppliersDetails = jest .fn() .mockResolvedValue([]); - await expect(getSupplierDetails(allocations, deps)).rejects.toThrow( + await expect(getSupplierDetails(supplierIds, deps)).rejects.toThrow( /No supplier details found/, ); }); 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 supplierIds = ["s1", "s3", "s5"]; const suppliers = [ { id: "s1", name: "Supplier 1", status: "PROD" }, { id: "s3", name: "Supplier 3", status: "PROD" }, @@ -238,7 +234,7 @@ describe("supplier-config service", () => { .fn() .mockResolvedValue(suppliers); - await getSupplierDetails(allocations, deps); + await getSupplierDetails(supplierIds, deps); expect(deps.supplierConfigRepo.getSuppliersDetails).toHaveBeenCalledWith([ "s1", @@ -248,11 +244,7 @@ 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 supplierIds = ["s1", "s2", "s3"]; const suppliers = [ { id: "s1", name: "Supplier 1", status: "PROD" }, { id: "s2", name: "Supplier 2", status: "PROD" }, @@ -262,7 +254,7 @@ describe("supplier-config service", () => { .fn() .mockResolvedValue(suppliers); - await getSupplierDetails(allocations, deps); + await getSupplierDetails(supplierIds, deps); expect(deps.logger.warn).toHaveBeenCalledWith({ description: "Mismatch between supplier allocations and supplier details", @@ -273,10 +265,7 @@ describe("supplier-config service", () => { }); it("does not log a warning when counts match", async () => { - const allocations = [ - { supplier: "s1", variantId: "v1" }, - { supplier: "s2", variantId: "v2" }, - ] as any[]; + const supplierIds = ["s1", "s2"]; const suppliers = [ { id: "s1", name: "Supplier 1", status: "PROD" }, { id: "s2", name: "Supplier 2", status: "PROD" }, @@ -286,16 +275,13 @@ describe("supplier-config service", () => { .fn() .mockResolvedValue(suppliers); - await getSupplierDetails(allocations, deps); + await getSupplierDetails(supplierIds, deps); expect(deps.logger.warn).not.toHaveBeenCalled(); }); it("throws when no active suppliers found", async () => { - const allocations = [ - { supplier: "s1", variantId: "v1" }, - { supplier: "s2", variantId: "v2" }, - ] as any[]; + const supplierIds = ["s1", "s2"]; const suppliers = [ { id: "s1", name: "Supplier 1", status: "DRAFT" }, { id: "s2", name: "Supplier 2", status: "DRAFT" }, @@ -305,7 +291,7 @@ describe("supplier-config service", () => { .fn() .mockResolvedValue(suppliers); - await expect(getSupplierDetails(allocations, deps)).rejects.toThrow( + await expect(getSupplierDetails(supplierIds, deps)).rejects.toThrow( /No active suppliers found/, ); expect(deps.logger.error).toHaveBeenCalledWith( @@ -316,11 +302,7 @@ describe("supplier-config service", () => { }); it("filters to return only active suppliers with PROD status", async () => { - const allocations = [ - { supplier: "s1", variantId: "v1" }, - { supplier: "s2", variantId: "v2" }, - { supplier: "s3", variantId: "v3" }, - ] as any[]; + const supplierIds = ["s1", "s2", "s3"]; const suppliers = [ { id: "s1", name: "Supplier 1", status: "PROD" }, { id: "s2", name: "Supplier 2", status: "DRAFT" }, @@ -331,9 +313,143 @@ describe("supplier-config service", () => { .fn() .mockResolvedValue(suppliers); - const result = await getSupplierDetails(allocations, deps); + const result = await getSupplierDetails(supplierIds, deps); expect(result).toEqual([suppliers[0], suppliers[2]]); expect(result.every((s) => s.status === "PROD")).toBe(true); }); + describe("getPreferredSupplierPacks", () => { + it("returns preferred supplier packs when found", async () => { + const suppliers = [ + { id: "s1", name: "Supplier 1", status: "PROD" }, + { id: "s2", name: "Supplier 2", status: "PROD" }, + ] as any[]; + const supplierPacks = [ + { id: "p1", supplierId: "s1", packSpecificationId: "spec1" }, + { id: "p2", supplierId: "s2", packSpecificationId: "spec1" }, + { id: "p3", supplierId: "s3", packSpecificationId: "spec1" }, + ] as any[]; + const deps = makeDeps(); + deps.supplierConfigRepo.getSupplierPacksForPackSpecification = jest + .fn() + .mockResolvedValue(supplierPacks); + + const result = await getPreferredSupplierPacks( + ["spec1"], + suppliers, + deps, + ); + + expect(result).toEqual([ + { id: "p1", supplierId: "s1", packSpecificationId: "spec1" }, + { id: "p2", supplierId: "s2", packSpecificationId: "spec1" }, + ]); + }); + + it("throws when no preferred supplier packs found", async () => { + const suppliers = [ + { id: "s1", name: "Supplier 1", status: "PROD" }, + { id: "s2", name: "Supplier 2", status: "PROD" }, + ] as any[]; + const deps = makeDeps(); + deps.supplierConfigRepo.getSupplierPacksForPackSpecification = jest + .fn() + .mockResolvedValue([]); + + await expect( + getPreferredSupplierPacks(["spec1"], suppliers, deps), + ).rejects.toThrow(/No preferred supplier packs found/); + expect(deps.logger.error).toHaveBeenCalledWith( + expect.objectContaining({ + description: + "No preferred supplier packs found for pack specification ids and suppliers", + }), + ); + }); + it("does not error when at least 1 pack specification has a preferred supplier pack", async () => { + const suppliers = [ + { id: "s1", name: "Supplier 1", status: "PROD" }, + { id: "s2", name: "Supplier 2", status: "PROD" }, + ] as any[]; + const deps = makeDeps(); + deps.supplierConfigRepo.getSupplierPacksForPackSpecification = jest + .fn() + .mockResolvedValueOnce([]) // no packs for spec1 + .mockResolvedValueOnce([ + { id: "p2", supplierId: "s2", packSpecificationId: "spec2" }, + ]); // preferred pack for spec2 + + const result = await getPreferredSupplierPacks( + ["spec1", "spec2"], + suppliers, + deps, + ); + + expect(result).toEqual([ + { id: "p2", supplierId: "s2", packSpecificationId: "spec2" }, + ]); + }); + + it("throws an error when no suppliers match the pack specification", async () => { + const suppliers = [ + { id: "s4", name: "Supplier 1", status: "PROD" }, + { id: "s5", name: "Supplier 2", status: "PROD" }, + ] as any[]; + const supplierPacks = [ + { id: "p1", supplierId: "s1", packSpecificationId: "spec1" }, + { id: "p2", supplierId: "s2", packSpecificationId: "spec1" }, + { id: "p3", supplierId: "s3", packSpecificationId: "spec1" }, + ] as any[]; + const deps = makeDeps(); + deps.supplierConfigRepo.getSupplierPacksForPackSpecification = jest + .fn() + .mockResolvedValue(supplierPacks); + + await expect( + getPreferredSupplierPacks(["spec1"], suppliers, deps), + ).rejects.toThrow(/No preferred supplier packs found/); + expect(deps.logger.error).toHaveBeenCalledWith( + expect.objectContaining({ + description: + "No preferred supplier packs found for pack specification ids and suppliers", + }), + ); + }); + }); + + describe("getPackSpecification", () => { + it("returns pack specification when found", async () => { + const packSpec = { id: "spec1", name: "Pack Spec 1" } as any; + const deps = makeDeps(); + deps.supplierConfigRepo.getPackSpecification = jest + .fn() + .mockResolvedValue(packSpec); + + const result = await getPackSpecification("spec1", deps); + + expect(result).toBe(packSpec); + }); + }); + + describe("getSuppliersWithValidPack", () => { + it("returns suppliers that have the valid pack specification", async () => { + const suppliers = [ + { id: "s1", name: "Supplier 1", status: "PROD" }, + { id: "s2", name: "Supplier 2", status: "PROD" }, + ] as any[]; + const supplierPacks = [ + { id: "p1", supplierId: "s1", packSpecificationId: "spec1" }, + ] as any[]; + const deps = makeDeps(); + deps.supplierConfigRepo.getSupplierPacksForPackSpecification = jest + .fn() + .mockResolvedValue(supplierPacks); + + const result = await getSuppliersWithValidPack(suppliers, "spec1", deps); + + expect(result).toEqual([ + { id: "s1", name: "Supplier 1", status: "PROD" }, + ]); + }); + }); }); diff --git a/lambdas/supplier-allocator/src/services/supplier-config.ts b/lambdas/supplier-allocator/src/services/supplier-config.ts index 9710a68b..87d750aa 100644 --- a/lambdas/supplier-allocator/src/services/supplier-config.ts +++ b/lambdas/supplier-allocator/src/services/supplier-config.ts @@ -1,7 +1,9 @@ import { LetterVariant, + PackSpecification, Supplier, SupplierAllocation, + SupplierPack, VolumeGroup, } from "@nhsdigital/nhs-notify-event-schemas-supplier-config"; import { Deps } from "../config/deps"; @@ -75,11 +77,9 @@ export async function getSupplierAllocationsForVolumeGroup( } export async function getSupplierDetails( - supplierAllocations: SupplierAllocation[], + supplierIds: string[], deps: Deps, ): Promise { - const supplierIds = supplierAllocations.map((alloc) => alloc.supplier); - const supplierDetails: Supplier[] = await deps.supplierConfigRepo.getSuppliersDetails(supplierIds); @@ -93,14 +93,14 @@ export async function getSupplierDetails( ); } // Log a warning if some supplier details are missing compared to allocations - if (supplierAllocations.length !== supplierDetails.length) { + if (supplierIds.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, + allocationsCount: supplierIds.length, detailsCount: supplierDetails.length, missingSuppliers: missingSupplierIds, }); @@ -117,3 +117,66 @@ export async function getSupplierDetails( } return activeSuppliers; } + +export async function getPreferredSupplierPacks( + packSpecificationIds: string[], + suppliers: Supplier[], + deps: Deps, +): Promise { + for (const packSpecId of packSpecificationIds) { + const supplierPacks = + await deps.supplierConfigRepo.getSupplierPacksForPackSpecification( + packSpecId, + ); + if (supplierPacks.length > 0) { + const preferredPacks = supplierPacks.filter((pack) => + suppliers.some((supplier) => supplier.id === pack.supplierId), + ); + if (preferredPacks.length > 0) { + return preferredPacks; + } + } + } + deps.logger.error({ + description: + "No preferred supplier packs found for pack specification ids and suppliers", + packSpecificationIds, + supplierIds: suppliers.map((s) => s.id), + }); + throw new Error( + `No preferred supplier packs found for pack specification ids ${packSpecificationIds.join(", ")} and suppliers ${suppliers.map((s) => s.id).join(", ")}`, + ); +} + +export async function getPackSpecification( + packSpecId: string, + deps: Deps, +): Promise { + const packSpec = + await deps.supplierConfigRepo.getPackSpecification(packSpecId); + return packSpec; +} + +// This function is used to filter the allocated suppliers based on those that support the supplied pack specification +export async function getSuppliersWithValidPack( + suppliers: Supplier[], + packSpecificationId: string, + deps: Deps, +): Promise { + const suppliersWithValidPack: Supplier[] = []; + const supplierPacks = + await deps.supplierConfigRepo.getSupplierPacksForPackSpecification( + packSpecificationId, + ); + + for (const supplier of suppliers) { + const hasValidPack = supplierPacks.some( + (pack) => pack.supplierId === supplier.id, + ); + if (hasValidPack) { + suppliersWithValidPack.push(supplier); + } + } + + return suppliersWithValidPack; +} From 056760b3091d56bba5c14c8342e4934118e12b92 Mon Sep 17 00:00:00 2001 From: David Wass Date: Fri, 27 Mar 2026 13:22:49 +0000 Subject: [PATCH 2/2] added more validity checks --- .../src/supplier-config-repository.ts | 5 ++++ .../__tests__/supplier-config.test.ts | 29 ++++++++++++++++++- .../src/services/supplier-config.ts | 8 +++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/internal/datastore/src/supplier-config-repository.ts b/internal/datastore/src/supplier-config-repository.ts index d4698e5c..fa9eb015 100644 --- a/internal/datastore/src/supplier-config-repository.ts +++ b/internal/datastore/src/supplier-config-repository.ts @@ -110,13 +110,18 @@ export class SupplierConfigRepository { TableName: this.config.supplierConfigTableName, IndexName: "packSpecificationId-index", KeyConditionExpression: "#pk = :pk AND #packSpecId = :packSpecId", + FilterExpression: "#status = :status AND #approval = :approval", ExpressionAttributeNames: { "#pk": "PK", "#packSpecId": "packSpecificationId", + "#status": "status", + "#approval": "approval", }, ExpressionAttributeValues: { ":pk": "SUPPLIER_PACK", ":packSpecId": packSpecId, + ":status": "PROD", + ":approval": "APPROVED", }, }), ); 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 08cee6be..f0ba4d4c 100644 --- a/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts +++ b/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts @@ -419,7 +419,11 @@ describe("supplier-config service", () => { describe("getPackSpecification", () => { it("returns pack specification when found", async () => { - const packSpec = { id: "spec1", name: "Pack Spec 1" } as any; + const packSpec = { + id: "spec1", + name: "Pack Spec 1", + status: "PROD", + } as any; const deps = makeDeps(); deps.supplierConfigRepo.getPackSpecification = jest .fn() @@ -429,6 +433,29 @@ describe("supplier-config service", () => { expect(result).toBe(packSpec); }); + + it("throws when pack specification is not active based on status", async () => { + const packSpec = { + id: "spec2", + name: "Pack Spec 2", + status: "DRAFT", + } as any; + const deps = makeDeps(); + deps.supplierConfigRepo.getPackSpecification = jest + .fn() + .mockResolvedValue(packSpec); + + await expect(getPackSpecification("spec2", deps)).rejects.toThrow( + /not active/, + ); + expect(deps.logger.error).toHaveBeenCalledWith( + expect.objectContaining({ + description: "Pack specification is not active based on status", + packSpecId: "spec2", + status: "DRAFT", + }), + ); + }); }); describe("getSuppliersWithValidPack", () => { diff --git a/lambdas/supplier-allocator/src/services/supplier-config.ts b/lambdas/supplier-allocator/src/services/supplier-config.ts index 87d750aa..31db9660 100644 --- a/lambdas/supplier-allocator/src/services/supplier-config.ts +++ b/lambdas/supplier-allocator/src/services/supplier-config.ts @@ -154,6 +154,14 @@ export async function getPackSpecification( ): Promise { const packSpec = await deps.supplierConfigRepo.getPackSpecification(packSpecId); + if (packSpec.status !== "PROD") { + deps.logger.error({ + description: "Pack specification is not active based on status", + packSpecId, + status: packSpec.status, + }); + throw new Error(`Pack specification with id ${packSpecId} is not active`); + } return packSpec; }