review changes

Author: nhsd-david-wass

infrastructure/terraform/components/api/module_lambda_supplier_allocator.tf

@@ -83,7 +83,22 @@ data "aws_iam_policy_document" "supplier_allocator_lambda" {
   }
 
   statement {
-    sid    = "AllowDynamoDBAccess"
+    sid    = "AllowConfigDynamoDBAccess"
+    effect = "Allow"
+
+    actions = [
+      "dynamodb:GetItem",
+      "dynamodb:Query",
+    ]
+
+    resources = [
+      aws_dynamodb_table.supplier-configuration.arn,
+      "${aws_dynamodb_table.supplier-configuration.arn}/index/*",
+    ]
+  }
+
+  statement {
+    sid    = "AllowQuotaDynamoDBAccess"
     effect = "Allow"
 
     actions = [
@@ -94,9 +109,7 @@ data "aws_iam_policy_document" "supplier_allocator_lambda" {
     ]
 
     resources = [
-      aws_dynamodb_table.supplier-configuration.arn,
       aws_dynamodb_table.supplier-quotas.arn,
-      "${aws_dynamodb_table.supplier-configuration.arn}/index/*",
       "${aws_dynamodb_table.supplier-quotas.arn}/index/*"
     ]
   }

lambdas/supplier-allocator/package.json

@@ -12,6 +12,8 @@
     "@types/aws-lambda": "^8.10.148",
     "aws-embedded-metrics": "^4.2.1",
     "aws-lambda": "^1.0.7",
+    "date-fns": "^3.0.0",
+    "date-fns-tz": "^3.0.0",
     "esbuild": "^0.27.2",
     "pino": "^9.7.0",
     "zod": "^4.1.11"

lambdas/supplier-allocator/src/handler/__tests__/allocate-handler.test.ts

@@ -577,6 +577,58 @@ describe("createSupplierAllocatorHandler", () => {
     },
   );
 
+  test("returns batch failure when no suppliers are found for pack specification", async () => {
+    const preparedEvent = createPreparedV2Event();
+    const evt: SQSEvent = createSQSEvent([
+      createSqsRecord("msg1", JSON.stringify(preparedEvent)),
+    ]);
+
+    process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue";
+
+    setupDefaultMocks();
+    (allocationConfig.suppliersWithValidPack as jest.Mock).mockResolvedValue(
+      [],
+    );
+
+    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).toHaveLength(2);
+    expect((mockedDeps.logger.error as jest.Mock).mock.calls[0][0]).toEqual(
+      expect.objectContaining({
+        description: "Error fetching supplier from config",
+        err: new Error("No suppliers found for pack specification spec1"),
+        variantId: "lv1",
+      }),
+    );
+  });
+
+  test("does not call selectSupplierByFactor for suppliers with capacity when there are no suppliers with capacity", async () => {
+    setupDefaultMocks();
+    (
+      allocationConfig.filterSuppliersWithCapacity as jest.Mock
+    ).mockResolvedValue([]);
+    process.env.UPSERT_LETTERS_QUEUE_URL = "https://sqs.test.queue";
+
+    const evt: SQSEvent = createSQSEvent([
+      createSqsRecord("msg1", JSON.stringify(createPreparedV2Event())),
+    ]);
+
+    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(0);
+    expect(allocationConfig.selectSupplierByFactor).toHaveBeenCalledWith(
+      expect.any(Array),
+      expect.any(Array),
+      mockedDeps,
+    );
+  });
+
   test("falls back to the second selectSupplierByFactor call when the first returns undefined", async () => {
     setupDefaultMocks();
     (allocationConfig.selectSupplierByFactor as jest.Mock)

lambdas/supplier-allocator/src/handler/__tests__/allocation-config.test.ts

@@ -961,4 +961,14 @@ describe("selectSupplierByFactor", () => {
 
     expect(result).toBe("supplier-5");
   });
+
+  it("should throw an error if no supplier factors are returned", async () => {
+    (
+      supplierQuotasService.calculateSupplierAllocatedFactor as jest.Mock
+    ).mockResolvedValue([]);
+
+    await expect(
+      selectSupplierByFactor(mockSuppliers, mockSupplierAllocations, mockDeps),
+    ).rejects.toThrow("No supplier factors could be calculated for allocation");
+  });
 });

lambdas/supplier-allocator/src/handler/allocate-handler.ts

@@ -73,16 +73,25 @@ async function getSupplierFromConfig(
       deps,
     );
 
+    if (allSuppliersForPack.length === 0) {
+      throw new Error(
+        `No suppliers found for pack specification ${preferredPack.id}`,
+      );
+    }
+
     const suppliersForPackWithCapacity: Supplier[] =
       await filterSuppliersWithCapacity(allSuppliersForPack, deps);
 
-    // selected supplier id is determined by first calling selectSupplierByFactor for suppliers with capacity and if nothing is returned tryong again with all suppliers for pack
+    // selected supplier id is determined by first calling selectSupplierByFactor for suppliers with capacity
+    // and if that returns nothing, try again with all suppliers for the pack
     const selectedSupplierId =
-      (await selectSupplierByFactor(
-        suppliersForPackWithCapacity,
-        supplierAllocations,
-        deps,
-      )) ??
+      (suppliersForPackWithCapacity.length > 0
+        ? await selectSupplierByFactor(
+            suppliersForPackWithCapacity,
+            supplierAllocations,
+            deps,
+          )
+        : undefined) ??
       (await selectSupplierByFactor(
         allSuppliersForPack,
         supplierAllocations,

lambdas/supplier-allocator/src/handler/allocation-config.ts

@@ -5,6 +5,8 @@ import {
   SupplierPack,
   VolumeGroup,
 } from "@nhsdigital/nhs-notify-event-schemas-supplier-config";
+import { format } from "date-fns";
+import { toZonedTime } from "date-fns-tz";
 import {
   filterPacksForLetter,
   getPackSpecification,
@@ -14,6 +16,7 @@ import {
   getSupplierPacks,
 } from "../services/supplier-config";
 import { calculateSupplierAllocatedFactor } from "../services/supplier-quotas";
+
 import { Deps } from "../config/deps";
 import { PreparedEvents } from "./types";
 
@@ -79,7 +82,10 @@ export async function filterSuppliersWithCapacity(
   suppliers: Supplier[],
   deps: Deps,
 ): Promise<Supplier[]> {
-  const dailyAllocationDate = new Date().toISOString().split("T")[0]; // Get current date in YYYY-MM-DD format
+  const dailyAllocationDate = format(
+    toZonedTime(new Date(), "Europe/London"),
+    "yyyy-MM-dd",
+  ); // Get current date in YYYY-MM-DD format (London timezone)
   const dailyAllocation =
     await deps.supplierQuotasRepo.getDailyAllocation(dailyAllocationDate);
   if (dailyAllocation) {
@@ -103,6 +109,10 @@ export async function selectSupplierByFactor(
   const supplierFactors: { supplierId: string; factor: number }[] =
     await calculateSupplierAllocatedFactor(supplierAllocationsForPack, deps);
 
+  if (supplierFactors.length === 0) {
+    throw new Error("No supplier factors could be calculated for allocation");
+  }
+
   deps.logger.info({
     description: "Calculated supplier factors for allocation",
     supplierFactors,