Skip to content

Commit 47a9ce5

Browse files
Merge branch 'main' into feature/CCM-17943
2 parents 44d4bec + cd6e693 commit 47a9ce5

9 files changed

Lines changed: 2824 additions & 3059 deletions

File tree

.github/workflows/manual-combine-dependabot-prs.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ jobs:
1515
steps:
1616
- name: combine-prs
1717
id: combine-prs
18-
uses: github/combine-prs@2909f404763c3177a456e052bdb7f2e85d3a7cb3 # v5.2.0
18+
uses: NHSDigital/nhs-notify-shared-modules/.github/actions/combine-dependabot-prs@4.0.6
1919
with:
2020
ci_required: false
2121
labels: dependencies

lambdas/supplier-allocator/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
"@aws-sdk/lib-dynamodb": "^3.1044.0",
66
"@internal/datastore": "*",
77
"@internal/helpers": "^0.1.0",
8-
"@nhsdigital/nhs-notify-event-schemas-letter-rendering": "2.0.1",
8+
"@nhsdigital/nhs-notify-event-schemas-letter-rendering": "2.0.2",
99
"@nhsdigital/nhs-notify-event-schemas-letter-rendering-v1": "npm:@nhsdigital/nhs-notify-event-schemas-letter-rendering@^1.1.5",
1010
"@nhsdigital/nhs-notify-event-schemas-supplier-api": "^1.0.8",
1111
"@nhsdigital/nhs-notify-event-schemas-supplier-config": "^1.1.0",
@@ -28,4 +28,4 @@
2828
"typecheck": "tsc --noEmit"
2929
},
3030
"version": "0.0.1"
31-
}
31+
}

lambdas/supplier-allocator/src/services/__tests__/supplier-config.test.ts

Lines changed: 166 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,8 @@ describe("supplier-config service", () => {
483483

484484
expect(result).toEqual(["spec1"]);
485485
});
486-
it("throws when no eligible packs found for letter", async () => {
486+
487+
it("throws when no eligible packs found for letter based on sheets", async () => {
487488
const deps = makeDeps();
488489
deps.supplierConfigRepo.getPackSpecification = jest
489490
.fn()
@@ -505,11 +506,18 @@ describe("supplier-config service", () => {
505506
"No eligible pack specifications found for letter variant id undefined and pack specification ids spec1",
506507
);
507508
expect(deps.logger.info).toHaveBeenCalledWith({
508-
description: "Pack specification filtered out based on constraints",
509+
description:
510+
"Pack specification filtered out based on pageCount constraints",
509511
packSpecId: "spec1",
510512
pageCount: 3,
511-
constraintValue: 2,
512-
constraintOperator: "LESS_THAN",
513+
violatedConstraints: expect.arrayContaining([
514+
expect.objectContaining({
515+
actualValue: 3,
516+
constraint: "sheets",
517+
constraintValue: 2,
518+
operator: "LESS_THAN",
519+
}),
520+
]),
513521
});
514522
expect(deps.logger.error).toHaveBeenCalledWith(
515523
expect.objectContaining({
@@ -519,6 +527,160 @@ describe("supplier-config service", () => {
519527
}),
520528
);
521529
});
530+
531+
it("does not filter pack specification when duplex reduces sheets and constraint is LESS_THAN_OR_EQUAL", async () => {
532+
const deps = makeDeps();
533+
deps.supplierConfigRepo.getPackSpecification = jest
534+
.fn()
535+
.mockResolvedValue({
536+
id: "spec1",
537+
assembly: { duplex: true },
538+
constraints: {
539+
sheets: { operator: "LESS_THAN_OR_EQUAL", value: 2 },
540+
},
541+
} as any);
542+
543+
const letterEvent = {
544+
data: {
545+
pageCount: 3,
546+
letterVariantId: "variant-1",
547+
},
548+
} as any;
549+
550+
const result = await filterPacksForLetter(letterEvent, ["spec1"], deps);
551+
552+
expect(result).toEqual(["spec1"]);
553+
expect(deps.logger.info).not.toHaveBeenCalled();
554+
});
555+
556+
it("filters pack specification when sides fail LESS_THAN_OR_EQUAL", async () => {
557+
const deps = makeDeps();
558+
deps.supplierConfigRepo.getPackSpecification = jest
559+
.fn()
560+
.mockResolvedValue({
561+
id: "spec1",
562+
constraints: {
563+
sides: { operator: "LESS_THAN_OR_EQUAL", value: 2 },
564+
},
565+
} as any);
566+
567+
const letterEvent = {
568+
data: {
569+
pageCount: 3,
570+
letterVariantId: "variant-1",
571+
},
572+
} as any;
573+
574+
await expect(
575+
filterPacksForLetter(letterEvent, ["spec1"], deps),
576+
).rejects.toThrow(/No eligible pack specifications found/);
577+
578+
expect(deps.logger.info).toHaveBeenCalledWith({
579+
description:
580+
"Pack specification filtered out based on pageCount constraints",
581+
packSpecId: "spec1",
582+
pageCount: 3,
583+
violatedConstraints: expect.arrayContaining([
584+
expect.objectContaining({
585+
constraint: "sides",
586+
actualValue: 3,
587+
constraintValue: 2,
588+
operator: "LESS_THAN_OR_EQUAL",
589+
}),
590+
]),
591+
});
592+
});
593+
594+
it("filters by sides using page count even when duplex is enabled", async () => {
595+
const deps = makeDeps();
596+
deps.supplierConfigRepo.getPackSpecification = jest
597+
.fn()
598+
.mockResolvedValue({
599+
id: "spec1",
600+
assembly: { duplex: true },
601+
constraints: {
602+
sheets: { operator: "LESS_THAN_OR_EQUAL", value: 2 },
603+
sides: { operator: "LESS_THAN_OR_EQUAL", value: 2 },
604+
},
605+
} as any);
606+
607+
const letterEvent = {
608+
data: {
609+
pageCount: 3,
610+
letterVariantId: "variant-1",
611+
},
612+
} as any;
613+
614+
await expect(
615+
filterPacksForLetter(letterEvent, ["spec1"], deps),
616+
).rejects.toThrow(/No eligible pack specifications found/);
617+
618+
expect(deps.logger.info).toHaveBeenCalledWith({
619+
description:
620+
"Pack specification filtered out based on pageCount constraints",
621+
packSpecId: "spec1",
622+
pageCount: 3,
623+
violatedConstraints: expect.arrayContaining([
624+
expect.objectContaining({
625+
constraint: "sides",
626+
actualValue: 3,
627+
constraintValue: 2,
628+
operator: "LESS_THAN_OR_EQUAL",
629+
}),
630+
]),
631+
});
632+
});
633+
634+
it("does not filter pack specification when sides is valid", async () => {
635+
const deps = makeDeps();
636+
deps.supplierConfigRepo.getPackSpecification = jest
637+
.fn()
638+
.mockResolvedValue({
639+
id: "spec1",
640+
assembly: { duplex: true },
641+
constraints: {
642+
sheets: { operator: "LESS_THAN_OR_EQUAL", value: 4 },
643+
},
644+
} as any);
645+
646+
const letterEvent = {
647+
data: {
648+
pageCount: 3,
649+
letterVariantId: "variant-1",
650+
},
651+
} as any;
652+
653+
const result = await filterPacksForLetter(letterEvent, ["spec1"], deps);
654+
655+
expect(result).toEqual(["spec1"]);
656+
expect(deps.logger.info).not.toHaveBeenCalled();
657+
});
658+
659+
it("keeps pack specification when sides constraint passes", async () => {
660+
const deps = makeDeps();
661+
deps.supplierConfigRepo.getPackSpecification = jest
662+
.fn()
663+
.mockResolvedValue({
664+
id: "spec1",
665+
constraints: {
666+
sides: { operator: "LESS_THAN_OR_EQUAL", value: 3 },
667+
},
668+
} as any);
669+
670+
const letterEvent = {
671+
data: {
672+
pageCount: 3,
673+
letterVariantId: "variant-1",
674+
},
675+
} as any;
676+
677+
const result = await filterPacksForLetter(letterEvent, ["spec1"], deps);
678+
679+
expect(result).toEqual(["spec1"]);
680+
expect(deps.logger.info).not.toHaveBeenCalled();
681+
expect(deps.logger.error).not.toHaveBeenCalled();
682+
});
683+
522684
it("returns eligible packs for all constraint types", async () => {
523685
const deps = makeDeps();
524686
const constraints: { operator: string; value: number }[] = [

lambdas/supplier-allocator/src/services/supplier-config.ts

Lines changed: 94 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -212,41 +212,109 @@ function evaluateContraint(
212212

213213
// This function is used to filter the pack specifications for a letter based on the letter data pages and pack specification constraints sheets
214214

215+
type ConstraintName = "sheets" | "sides";
216+
217+
type ViolatedConstraint = {
218+
constraint: ConstraintName;
219+
actualValue: number;
220+
constraintValue: number;
221+
operator: string;
222+
};
223+
224+
function hasConstraint(constraint?: {
225+
value?: number;
226+
operator?: string;
227+
}): constraint is { value: number; operator: string } {
228+
return constraint?.value !== undefined && constraint.operator !== undefined;
229+
}
230+
231+
function getViolatedConstraints(
232+
pageCount: number,
233+
duplex: boolean | undefined,
234+
constraints?: {
235+
sheets?: { value?: number; operator?: string };
236+
sides?: { value?: number; operator?: string };
237+
},
238+
): ViolatedConstraint[] {
239+
if (!constraints) {
240+
return [];
241+
}
242+
243+
const violations: ViolatedConstraint[] = [];
244+
245+
if (hasConstraint(constraints.sheets)) {
246+
const sheetCount = duplex ? Math.ceil(pageCount / 2) : pageCount;
247+
const passes = evaluateContraint(
248+
sheetCount,
249+
constraints.sheets.value,
250+
constraints.sheets.operator,
251+
);
252+
253+
if (!passes) {
254+
violations.push({
255+
constraint: "sheets",
256+
actualValue: sheetCount,
257+
constraintValue: constraints.sheets.value,
258+
operator: constraints.sheets.operator,
259+
});
260+
}
261+
}
262+
263+
if (hasConstraint(constraints.sides)) {
264+
const passes = evaluateContraint(
265+
pageCount,
266+
constraints.sides.value,
267+
constraints.sides.operator,
268+
);
269+
270+
if (!passes) {
271+
violations.push({
272+
constraint: "sides",
273+
actualValue: pageCount,
274+
constraintValue: constraints.sides.value,
275+
operator: constraints.sides.operator,
276+
});
277+
}
278+
}
279+
280+
return violations;
281+
}
282+
215283
export async function filterPacksForLetter(
216284
letterEvent: PreparedEvents,
217285
packSpecificationIds: string[],
218286
deps: Deps,
219287
): Promise<string[]> {
220-
const filteredPackIds: string[] = [];
221-
for (const packSpecId of packSpecificationIds) {
222-
const packSpec =
223-
await deps.supplierConfigRepo.getPackSpecification(packSpecId);
224-
if (
225-
!packSpec.constraints ||
226-
!packSpec.constraints.sheets ||
227-
!packSpec.constraints.sheets.value ||
228-
!packSpec.constraints.sheets.operator
229-
) {
230-
filteredPackIds.push(packSpecId);
231-
} else {
232-
const isValid = evaluateContraint(
233-
letterEvent.data.pageCount,
234-
packSpec.constraints.sheets.value,
235-
packSpec.constraints.sheets.operator,
288+
const { pageCount } = letterEvent.data;
289+
290+
const evaluationResults = await Promise.all(
291+
packSpecificationIds.map(async (packSpecId) => {
292+
const packSpec =
293+
await deps.supplierConfigRepo.getPackSpecification(packSpecId);
294+
295+
const violatedConstraints = getViolatedConstraints(
296+
pageCount,
297+
packSpec?.assembly?.duplex,
298+
packSpec.constraints,
236299
);
237-
if (isValid) {
238-
filteredPackIds.push(packSpecId);
239-
} else {
300+
301+
if (violatedConstraints.length > 0) {
240302
deps.logger.info({
241-
description: "Pack specification filtered out based on constraints",
303+
description: `Pack specification filtered out based on pageCount constraints`,
242304
packSpecId,
243-
pageCount: letterEvent.data.pageCount,
244-
constraintValue: packSpec.constraints.sheets.value,
245-
constraintOperator: packSpec.constraints.sheets.operator,
305+
pageCount,
306+
violatedConstraints,
246307
});
247308
}
248-
}
249-
}
309+
310+
return { packSpecId, isValid: violatedConstraints.length === 0 };
311+
}),
312+
);
313+
314+
const filteredPackIds = evaluationResults
315+
.filter((result) => result.isValid)
316+
.map((result) => result.packSpecId);
317+
250318
if (filteredPackIds.length === 0) {
251319
deps.logger.error({
252320
description: "No eligible pack specifications found for letter",
@@ -257,5 +325,6 @@ export async function filterPacksForLetter(
257325
`No eligible pack specifications found for letter variant id ${letterEvent.data.letterVariantId} and pack specification ids ${packSpecificationIds.join(", ")}`,
258326
);
259327
}
328+
260329
return filteredPackIds;
261330
}

lambdas/supplier-config-ingress/src/handler/supplier-config-ingress-handler.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,12 @@ const $EventEnvelope = z.object({
2323
});
2424

2525
const entitySchemas: Record<SupplierConfigEntity, z.ZodType<{ id: string }>> = {
26-
"letter-variant": $LetterVariant as unknown as z.ZodType<{ id: string }>,
27-
"volume-group": $VolumeGroup as unknown as z.ZodType<{ id: string }>,
28-
"supplier-allocation": $SupplierAllocation as unknown as z.ZodType<{
29-
id: string;
30-
}>,
31-
supplier: $Supplier as unknown as z.ZodType<{ id: string }>,
32-
"pack-specification": $PackSpecification as unknown as z.ZodType<{
33-
id: string;
34-
}>,
35-
"supplier-pack": $SupplierPack as unknown as z.ZodType<{ id: string }>,
26+
"letter-variant": $LetterVariant,
27+
"volume-group": $VolumeGroup,
28+
"supplier-allocation": $SupplierAllocation,
29+
supplier: $Supplier,
30+
"pack-specification": $PackSpecification,
31+
"supplier-pack": $SupplierPack,
3632
};
3733

3834
type UpsertResult = Awaited<

lambdas/upsert-letter/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
"@aws-sdk/lib-dynamodb": "^3.1044.0",
66
"@internal/datastore": "*",
77
"@internal/helpers": "*",
8-
"@nhsdigital/nhs-notify-event-schemas-letter-rendering": "2.0.1",
8+
"@nhsdigital/nhs-notify-event-schemas-letter-rendering": "2.0.2",
99
"@nhsdigital/nhs-notify-event-schemas-letter-rendering-v1": "npm:@nhsdigital/nhs-notify-event-schemas-letter-rendering@^1.1.5",
1010
"@nhsdigital/nhs-notify-event-schemas-supplier-api": "^1.0.8",
1111
"@types/aws-lambda": "^8.10.148",

0 commit comments

Comments
 (0)