diff --git a/config/suppliers/letter-variant/notify-standard-colour.json b/config/suppliers/letter-variant/notify-standard-colour.json index 772a4146c..056e56f67 100644 --- a/config/suppliers/letter-variant/notify-standard-colour.json +++ b/config/suppliers/letter-variant/notify-standard-colour.json @@ -29,6 +29,7 @@ ], "priority": 99, "status": "INT", + "supplierId": "supplier1", "type": "STANDARD", - "volumeGroupId": "volumeGroup-test3" + "volumeGroupId": "volumeGroup-test1" } diff --git a/infrastructure/terraform/components/api/module_lambda_upsert_letter.tf b/infrastructure/terraform/components/api/module_lambda_upsert_letter.tf index 7519d1556..eeaed3420 100644 --- a/infrastructure/terraform/components/api/module_lambda_upsert_letter.tf +++ b/infrastructure/terraform/components/api/module_lambda_upsert_letter.tf @@ -67,11 +67,26 @@ data "aws_iam_policy_document" "upsert_letter_lambda" { resources = [ aws_dynamodb_table.letters.arn, - aws_dynamodb_table.idempotency.arn, "${aws_dynamodb_table.letters.arn}/index/supplierStatus-index" ] } + statement { + sid = "AllowIdempotencyTableWrite" + effect = "Allow" + + actions = [ + "dynamodb:PutItem", + "dynamodb:GetItem", + "dynamodb:UpdateItem", + "dynamodb:DeleteItem" + ] + + resources = [ + aws_dynamodb_table.idempotency.arn, + ] + } + statement { sid = "AllowSQSRead" effect = "Allow" diff --git a/lambdas/supplier-allocator/src/handler/__tests__/allocation-config.test.ts b/lambdas/supplier-allocator/src/handler/__tests__/allocation-config.test.ts index 17ce944a5..87b58584f 100644 --- a/lambdas/supplier-allocator/src/handler/__tests__/allocation-config.test.ts +++ b/lambdas/supplier-allocator/src/handler/__tests__/allocation-config.test.ts @@ -196,6 +196,37 @@ describe("eligibleSuppliers", () => { "Supplier service error", ); }); + it("should filter allocations by letterVariantSupplierId if provided", async () => { + ( + supplierConfigService.getSupplierAllocationsForVolumeGroup as jest.Mock + ).mockResolvedValue(mockSupplierAllocations); + (supplierConfigService.getSupplierDetails as jest.Mock).mockResolvedValue( + mockSuppliers, + ); + + const letterVariantSupplierId = "supplier-1"; + await eligibleSuppliers(mockVolumeGroup, mockDeps, letterVariantSupplierId); + expect(supplierConfigService.getSupplierDetails).toHaveBeenCalledWith( + ["supplier-1"], + mockDeps, + ); + }); + it("should log a warning if no allocations found for specified letter variant supplier", async () => { + ( + supplierConfigService.getSupplierAllocationsForVolumeGroup as jest.Mock + ).mockResolvedValue([]); + (supplierConfigService.getSupplierDetails as jest.Mock).mockResolvedValue( + [], + ); + + const letterVariantSupplierId = "supplier-1"; + await eligibleSuppliers(mockVolumeGroup, mockDeps, letterVariantSupplierId); + expect(mockDeps.logger.warn).toHaveBeenCalledWith({ + description: "No allocations found for specified letter variant supplier", + volumeGroupId: mockVolumeGroup.id, + letterVariantSupplierId, + }); + }); }); describe("preferredSupplierPack", () => { diff --git a/lambdas/supplier-allocator/src/handler/allocate-handler.ts b/lambdas/supplier-allocator/src/handler/allocate-handler.ts index 397a53abe..326f20bb2 100644 --- a/lambdas/supplier-allocator/src/handler/allocate-handler.ts +++ b/lambdas/supplier-allocator/src/handler/allocate-handler.ts @@ -29,7 +29,6 @@ import { selectSupplierByFactor, suppliersWithValidPack, } from "./allocation-config"; - import { Deps } from "../config/deps"; import { PreparedEventSchema, PreparedEvents, SupplierDetails } from "./types"; @@ -94,7 +93,7 @@ async function getSupplierFromConfig( ); const { supplierAllocations, suppliers: allocatedSuppliers } = - await eligibleSuppliers(volumeGroup, deps); + await eligibleSuppliers(volumeGroup, deps, letterVariant.supplierId); const preferredPack: PackSpecification = await preferredSupplierPack( letterEvent, @@ -136,6 +135,7 @@ async function getSupplierFromConfig( deps.logger.info({ description: "Fetched supplier details for supplier allocations", + domainId: letterEvent.data.domainId, variantId: letterEvent.data.letterVariantId, volumeGroupId: volumeGroup.id, supplierAllocationIds: supplierAllocations.map((a) => a.id), @@ -391,6 +391,9 @@ export default function createSupplierAllocatorHandler(deps: Deps): SQSHandler { ({ priority, supplier } = supplierAllocationResult); } catch (error) { + console.log( + `Error processing allocation of record ${record.messageId}: ${error}`, + ); deps.logger.error({ description: "Error processing allocation of record", err: error, diff --git a/lambdas/supplier-allocator/src/handler/allocation-config.ts b/lambdas/supplier-allocator/src/handler/allocation-config.ts index f029b619a..f912fe0ec 100644 --- a/lambdas/supplier-allocator/src/handler/allocation-config.ts +++ b/lambdas/supplier-allocator/src/handler/allocation-config.ts @@ -23,6 +23,7 @@ import { PreparedEvents } from "./types"; export async function eligibleSuppliers( volumeGroup: VolumeGroup, deps: Deps, + letterVariantSupplierId?: string, ): Promise<{ supplierAllocations: SupplierAllocation[]; suppliers: Supplier[]; @@ -31,6 +32,32 @@ export async function eligibleSuppliers( volumeGroup.id, deps, ); + if (letterVariantSupplierId) { + deps.logger.info({ + description: "Filtering allocations for letter variant supplier", + volumeGroupId: volumeGroup.id, + letterVariantSupplierId, + }); + const filteredAllocations = supplierAllocations.filter( + (alloc) => alloc.supplier === letterVariantSupplierId, + ); + if (filteredAllocations.length === 0) { + deps.logger.warn({ + description: + "No allocations found for specified letter variant supplier", + volumeGroupId: volumeGroup.id, + letterVariantSupplierId, + }); + } + return { + supplierAllocations: filteredAllocations, + suppliers: await getSupplierDetails( + filteredAllocations.map((alloc) => alloc.supplier), + deps, + ), + }; + } + const allocationPercentageSum = supplierAllocations.reduce( (sum, alloc) => sum + alloc.allocationPercentage, 0, diff --git a/tests/component-tests/allocation-tests/allocation-letter-variant.spec.ts b/tests/component-tests/allocation-tests/allocation-letter-variant.spec.ts new file mode 100644 index 000000000..880fc543d --- /dev/null +++ b/tests/component-tests/allocation-tests/allocation-letter-variant.spec.ts @@ -0,0 +1,35 @@ +import { randomUUID } from "node:crypto"; +import { expect, test } from "@playwright/test"; +import { + FetchedSupplierLog, + getAllocationDetailsForDomainId, + getVariantsForAllocation, +} from "tests/helpers/allocation-helper"; + +import { createPreparedV1Event } from "tests/helpers/event-fixtures"; +import { sendSnsEvent } from "tests/helpers/send-sns-event"; + +test.describe("Letter Variant Tests", () => { + test.setTimeout(180_000); // 3 minutes for long running polling + + test("Verify that supplierId on letter variant takes pecedence over volume group", async () => { + const letterVariant = getVariantsForAllocation(3); + const domainId = randomUUID(); + const preparedEvent = createPreparedV1Event({ + domainId, + letterVariantId: letterVariant, + }); + + const response = await sendSnsEvent(preparedEvent); + + expect(response.MessageId).toBeTruthy(); + + const allocationDetails: FetchedSupplierLog = + await getAllocationDetailsForDomainId(domainId); + + expect(allocationDetails.allocatedSuppliers).toHaveLength(1); + + const allocatedSupplierId = allocationDetails.allocatedSuppliers?.[0]?.id; + expect(allocatedSupplierId).toBe("supplier1"); + }); +}); diff --git a/tests/component-tests/allocation-tests/supplier-allocation.spec.ts b/tests/component-tests/allocation-tests/supplier-allocation.spec.ts index d33a6cc7c..fc1f03c27 100644 --- a/tests/component-tests/allocation-tests/supplier-allocation.spec.ts +++ b/tests/component-tests/allocation-tests/supplier-allocation.spec.ts @@ -117,6 +117,8 @@ test.describe("Supplier Allocation Tests", () => { logger.info( `New total daily allocation for date ${allocationDate}: ${newTotalDailyAllocation}`, ); - expect(newTotalDailyAllocation).toBe(originalTotalDailyAllocation + 1); + expect(newTotalDailyAllocation).toBeGreaterThanOrEqual( + originalTotalDailyAllocation + 1, + ); }); }); diff --git a/tests/helpers/allocation-helper.ts b/tests/helpers/allocation-helper.ts index bed146780..e73ccb1fc 100644 --- a/tests/helpers/allocation-helper.ts +++ b/tests/helpers/allocation-helper.ts @@ -9,6 +9,7 @@ import { import { envName } from "tests/constants/api-constants"; import { pollAllocatorLogWithOptions, + pollSupplierAllocatorLogForAllocationDetails, pollSupplierAllocatorLogForExceededDailyCapacity, pollSupplierAllocatorLogForResolvedSpec, } from "./aws-cloudwatch-helper"; @@ -83,6 +84,22 @@ export type AllocationLogOptions = { extraPatterns?: string[]; }; +export type AllocatedSuppliersLogEntry = { + id: string; + name: string; + channelType: string; + dailyCapacity: number; + status: string; +}; + +export type FetchedSupplierLog = { + supplierAllocationIds?: string[]; + allocatedSuppliers?: AllocatedSuppliersLogEntry[]; + allSuppliersForPack?: string[]; + supplierForPackWithCapacity?: string[]; + selectedSupplierId?: string; +}; + type LetterVariantConfig = { id: string; packSpecificationIds: string[]; @@ -137,6 +154,15 @@ export async function getAllocationLogForDomainId( return supplierAllocatorLog; } +export async function getAllocationDetailsForDomainId( + domainId: string, +): Promise { + const message = await pollSupplierAllocatorLogForAllocationDetails(domainId); + const fetchedSupplierLog = JSON.parse(message) as FetchedSupplierLog; + + return fetchedSupplierLog; +} + export async function getAllocationLog< TLog extends { description: string } = PackSpecificationLog, >(description: string, options?: AllocationLogOptions): Promise { diff --git a/tests/helpers/aws-cloudwatch-helper.ts b/tests/helpers/aws-cloudwatch-helper.ts index af9c56553..c31d0933b 100644 --- a/tests/helpers/aws-cloudwatch-helper.ts +++ b/tests/helpers/aws-cloudwatch-helper.ts @@ -60,6 +60,16 @@ export async function pollSupplierAllocatorLogForResolvedSpec( `"${domainId}"`, ]); } + +export async function pollSupplierAllocatorLogForAllocationDetails( + domainId: string, +): Promise { + return pollLambdaLog("supplier-allocator", [ + '"Fetched supplier details for supplier allocations"', + `"${domainId}"`, + ]); +} + export async function pollSupplierAllocatorLogForError( msgToCheck: string, domainId?: string,