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 6663d120f..1f6807dba 100644 --- a/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts +++ b/lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts @@ -483,7 +483,8 @@ describe("supplier-config service", () => { expect(result).toEqual(["spec1"]); }); - it("throws when no eligible packs found for letter", async () => { + + it("throws when no eligible packs found for letter based on sheets", async () => { const deps = makeDeps(); deps.supplierConfigRepo.getPackSpecification = jest .fn() @@ -505,11 +506,18 @@ describe("supplier-config service", () => { "No eligible pack specifications found for letter variant id undefined and pack specification ids spec1", ); expect(deps.logger.info).toHaveBeenCalledWith({ - description: "Pack specification filtered out based on constraints", + description: + "Pack specification filtered out based on pageCount constraints", packSpecId: "spec1", pageCount: 3, - constraintValue: 2, - constraintOperator: "LESS_THAN", + violatedConstraints: expect.arrayContaining([ + expect.objectContaining({ + actualValue: 3, + constraint: "sheets", + constraintValue: 2, + operator: "LESS_THAN", + }), + ]), }); expect(deps.logger.error).toHaveBeenCalledWith( expect.objectContaining({ @@ -519,6 +527,160 @@ describe("supplier-config service", () => { }), ); }); + + it("does not filter pack specification when duplex reduces sheets and constraint is LESS_THAN_OR_EQUAL", async () => { + const deps = makeDeps(); + deps.supplierConfigRepo.getPackSpecification = jest + .fn() + .mockResolvedValue({ + id: "spec1", + assembly: { duplex: true }, + constraints: { + sheets: { operator: "LESS_THAN_OR_EQUAL", value: 2 }, + }, + } as any); + + const letterEvent = { + data: { + pageCount: 3, + letterVariantId: "variant-1", + }, + } as any; + + const result = await filterPacksForLetter(letterEvent, ["spec1"], deps); + + expect(result).toEqual(["spec1"]); + expect(deps.logger.info).not.toHaveBeenCalled(); + }); + + it("filters pack specification when sides fail LESS_THAN_OR_EQUAL", async () => { + const deps = makeDeps(); + deps.supplierConfigRepo.getPackSpecification = jest + .fn() + .mockResolvedValue({ + id: "spec1", + constraints: { + sides: { operator: "LESS_THAN_OR_EQUAL", value: 2 }, + }, + } as any); + + const letterEvent = { + data: { + pageCount: 3, + letterVariantId: "variant-1", + }, + } as any; + + await expect( + filterPacksForLetter(letterEvent, ["spec1"], deps), + ).rejects.toThrow(/No eligible pack specifications found/); + + expect(deps.logger.info).toHaveBeenCalledWith({ + description: + "Pack specification filtered out based on pageCount constraints", + packSpecId: "spec1", + pageCount: 3, + violatedConstraints: expect.arrayContaining([ + expect.objectContaining({ + constraint: "sides", + actualValue: 3, + constraintValue: 2, + operator: "LESS_THAN_OR_EQUAL", + }), + ]), + }); + }); + + it("filters by sides using page count even when duplex is enabled", async () => { + const deps = makeDeps(); + deps.supplierConfigRepo.getPackSpecification = jest + .fn() + .mockResolvedValue({ + id: "spec1", + assembly: { duplex: true }, + constraints: { + sheets: { operator: "LESS_THAN_OR_EQUAL", value: 2 }, + sides: { operator: "LESS_THAN_OR_EQUAL", value: 2 }, + }, + } as any); + + const letterEvent = { + data: { + pageCount: 3, + letterVariantId: "variant-1", + }, + } as any; + + await expect( + filterPacksForLetter(letterEvent, ["spec1"], deps), + ).rejects.toThrow(/No eligible pack specifications found/); + + expect(deps.logger.info).toHaveBeenCalledWith({ + description: + "Pack specification filtered out based on pageCount constraints", + packSpecId: "spec1", + pageCount: 3, + violatedConstraints: expect.arrayContaining([ + expect.objectContaining({ + constraint: "sides", + actualValue: 3, + constraintValue: 2, + operator: "LESS_THAN_OR_EQUAL", + }), + ]), + }); + }); + + it("does not filter pack specification when sides is valid", async () => { + const deps = makeDeps(); + deps.supplierConfigRepo.getPackSpecification = jest + .fn() + .mockResolvedValue({ + id: "spec1", + assembly: { duplex: true }, + constraints: { + sheets: { operator: "LESS_THAN_OR_EQUAL", value: 4 }, + }, + } as any); + + const letterEvent = { + data: { + pageCount: 3, + letterVariantId: "variant-1", + }, + } as any; + + const result = await filterPacksForLetter(letterEvent, ["spec1"], deps); + + expect(result).toEqual(["spec1"]); + expect(deps.logger.info).not.toHaveBeenCalled(); + }); + + it("keeps pack specification when sides constraint passes", async () => { + const deps = makeDeps(); + deps.supplierConfigRepo.getPackSpecification = jest + .fn() + .mockResolvedValue({ + id: "spec1", + constraints: { + sides: { operator: "LESS_THAN_OR_EQUAL", value: 3 }, + }, + } as any); + + const letterEvent = { + data: { + pageCount: 3, + letterVariantId: "variant-1", + }, + } as any; + + const result = await filterPacksForLetter(letterEvent, ["spec1"], deps); + + expect(result).toEqual(["spec1"]); + expect(deps.logger.info).not.toHaveBeenCalled(); + expect(deps.logger.error).not.toHaveBeenCalled(); + }); + it("returns eligible packs for all constraint types", async () => { const deps = makeDeps(); const constraints: { operator: string; value: number }[] = [ diff --git a/lambdas/supplier-allocator/src/services/supplier-config.ts b/lambdas/supplier-allocator/src/services/supplier-config.ts index d5a3774cd..52ac1c434 100644 --- a/lambdas/supplier-allocator/src/services/supplier-config.ts +++ b/lambdas/supplier-allocator/src/services/supplier-config.ts @@ -212,41 +212,109 @@ function evaluateContraint( // This function is used to filter the pack specifications for a letter based on the letter data pages and pack specification constraints sheets +type ConstraintName = "sheets" | "sides"; + +type ViolatedConstraint = { + constraint: ConstraintName; + actualValue: number; + constraintValue: number; + operator: string; +}; + +function hasConstraint(constraint?: { + value?: number; + operator?: string; +}): constraint is { value: number; operator: string } { + return constraint?.value !== undefined && constraint.operator !== undefined; +} + +function getViolatedConstraints( + pageCount: number, + duplex: boolean | undefined, + constraints?: { + sheets?: { value?: number; operator?: string }; + sides?: { value?: number; operator?: string }; + }, +): ViolatedConstraint[] { + if (!constraints) { + return []; + } + + const violations: ViolatedConstraint[] = []; + + if (hasConstraint(constraints.sheets)) { + const sheetCount = duplex ? Math.ceil(pageCount / 2) : pageCount; + const passes = evaluateContraint( + sheetCount, + constraints.sheets.value, + constraints.sheets.operator, + ); + + if (!passes) { + violations.push({ + constraint: "sheets", + actualValue: sheetCount, + constraintValue: constraints.sheets.value, + operator: constraints.sheets.operator, + }); + } + } + + if (hasConstraint(constraints.sides)) { + const passes = evaluateContraint( + pageCount, + constraints.sides.value, + constraints.sides.operator, + ); + + if (!passes) { + violations.push({ + constraint: "sides", + actualValue: pageCount, + constraintValue: constraints.sides.value, + operator: constraints.sides.operator, + }); + } + } + + return violations; +} + export async function filterPacksForLetter( letterEvent: PreparedEvents, packSpecificationIds: string[], deps: Deps, ): Promise { - const filteredPackIds: string[] = []; - for (const packSpecId of packSpecificationIds) { - const packSpec = - await deps.supplierConfigRepo.getPackSpecification(packSpecId); - if ( - !packSpec.constraints || - !packSpec.constraints.sheets || - !packSpec.constraints.sheets.value || - !packSpec.constraints.sheets.operator - ) { - filteredPackIds.push(packSpecId); - } else { - const isValid = evaluateContraint( - letterEvent.data.pageCount, - packSpec.constraints.sheets.value, - packSpec.constraints.sheets.operator, + const { pageCount } = letterEvent.data; + + const evaluationResults = await Promise.all( + packSpecificationIds.map(async (packSpecId) => { + const packSpec = + await deps.supplierConfigRepo.getPackSpecification(packSpecId); + + const violatedConstraints = getViolatedConstraints( + pageCount, + packSpec?.assembly?.duplex, + packSpec.constraints, ); - if (isValid) { - filteredPackIds.push(packSpecId); - } else { + + if (violatedConstraints.length > 0) { deps.logger.info({ - description: "Pack specification filtered out based on constraints", + description: `Pack specification filtered out based on pageCount constraints`, packSpecId, - pageCount: letterEvent.data.pageCount, - constraintValue: packSpec.constraints.sheets.value, - constraintOperator: packSpec.constraints.sheets.operator, + pageCount, + violatedConstraints, }); } - } - } + + return { packSpecId, isValid: violatedConstraints.length === 0 }; + }), + ); + + const filteredPackIds = evaluationResults + .filter((result) => result.isValid) + .map((result) => result.packSpecId); + if (filteredPackIds.length === 0) { deps.logger.error({ description: "No eligible pack specifications found for letter", @@ -257,5 +325,6 @@ export async function filterPacksForLetter( `No eligible pack specifications found for letter variant id ${letterEvent.data.letterVariantId} and pack specification ids ${packSpecificationIds.join(", ")}`, ); } + return filteredPackIds; } diff --git a/tests/component-tests/allocation-tests/letter-allocation-capacity.spec.ts b/tests/component-tests/allocation-tests/letter-allocation-capacity.spec.ts index e2353c3d8..7a172a4a8 100644 --- a/tests/component-tests/allocation-tests/letter-allocation-capacity.spec.ts +++ b/tests/component-tests/allocation-tests/letter-allocation-capacity.spec.ts @@ -58,14 +58,14 @@ test.describe("Allocator Lambda Tests", () => { const preparedEvent = createPreparedV1Event({ domainId, letterVariantId: letterVariant, - pageCount: 6, // pagecount that makes notify-c5 ineligible and notify-c4 eligible based on their pack configs + pageCount: 11, // pagecount that makes notify-c5 ineligible and notify-c4 eligible based on their pack configs (note packs are duplex) }); const response = await sendSnsEvent(preparedEvent); expect(response.MessageId).toBeTruthy(); const supplierAllocatorLog = await getAllocationLog( - "Pack specification filtered out based on constraints", + "Pack specification filtered out based on pageCount constraints", ); const filteredPackSpecId = supplierAllocatorLog.packSpecId; logger.info(`Pack spec filtered out ${filteredPackSpecId}`);