From c500c71cda1880037706439705d1fd247b742361 Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Thu, 8 Oct 2026 12:50:37 +0000 Subject: [PATCH 01/17] expose more dimensions for upsertLetter lambda --- .../terraform/components/api/README.md | 34 +++++++++---------- .../src/handler/upsert-handler.ts | 34 ++++++++++++------- 2 files changed, 39 insertions(+), 29 deletions(-) diff --git a/infrastructure/terraform/components/api/README.md b/infrastructure/terraform/components/api/README.md index b21ae6675..691e4db76 100644 --- a/infrastructure/terraform/components/api/README.md +++ b/infrastructure/terraform/components/api/README.md @@ -60,9 +60,9 @@ No requirements. | Name | Source | Version | | ---- | ------ | ------- | -| [amendment\_event\_transformer](#module\_amendment\_event\_transformer) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/4.0.2/terraform-lambda.zip | n/a | +| [amendment\_event\_transformer](#module\_amendment\_event\_transformer) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | | [amendments\_queue](#module\_amendments\_queue) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/3.1.5/terraform-sqs.zip | n/a | -| [authorizer\_lambda](#module\_authorizer\_lambda) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/4.0.2/terraform-lambda.zip | n/a | +| [authorizer\_lambda](#module\_authorizer\_lambda) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | | [ddb\_alarms\_idempotency](#module\_ddb\_alarms\_idempotency) | ../../modules/alarms-ddb | n/a | | [ddb\_alarms\_letter\_queue](#module\_ddb\_alarms\_letter\_queue) | ../../modules/alarms-ddb | n/a | | [ddb\_alarms\_letters](#module\_ddb\_alarms\_letters) | ../../modules/alarms-ddb | n/a | @@ -71,30 +71,30 @@ No requirements. | [domain\_truststore](#module\_domain\_truststore) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/3.0.6/terraform-s3bucket.zip | n/a | | [eventpub](#module\_eventpub) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/5.0.6/terraform-eventpub.zip | n/a | | [eventsub](#module\_eventsub) | ../../modules/eventsub | n/a | -| [get\_letter](#module\_get\_letter) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/4.0.2/terraform-lambda.zip | n/a | -| [get\_letter\_data](#module\_get\_letter\_data) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/4.0.2/terraform-lambda.zip | n/a | -| [get\_letters](#module\_get\_letters) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/4.0.2/terraform-lambda.zip | n/a | -| [get\_mi](#module\_get\_mi) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.29/terraform-lambda.zip | n/a | -| [get\_status](#module\_get\_status) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/4.0.2/terraform-lambda.zip | n/a | +| [get\_letter](#module\_get\_letter) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | +| [get\_letter\_data](#module\_get\_letter\_data) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | +| [get\_letters](#module\_get\_letters) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | +| [get\_mi](#module\_get\_mi) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | +| [get\_status](#module\_get\_status) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | | [kms](#module\_kms) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.26/terraform-kms.zip | n/a | | [lambda\_alarms](#module\_lambda\_alarms) | ../../modules/alarms-lambda | n/a | | [letter\_status\_updates\_queue](#module\_letter\_status\_updates\_queue) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/3.1.5/terraform-sqs.zip | n/a | -| [letter\_updates\_transformer](#module\_letter\_updates\_transformer) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/4.0.2/terraform-lambda.zip | n/a | -| [mi\_updates\_transformer](#module\_mi\_updates\_transformer) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/4.0.2/terraform-lambda.zip | n/a | -| [patch\_letter](#module\_patch\_letter) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/4.0.2/terraform-lambda.zip | n/a | -| [post\_letters](#module\_post\_letters) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/4.0.2/terraform-lambda.zip | n/a | -| [post\_mi](#module\_post\_mi) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/4.0.2/terraform-lambda.zip | n/a | +| [letter\_updates\_transformer](#module\_letter\_updates\_transformer) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | +| [mi\_updates\_transformer](#module\_mi\_updates\_transformer) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | +| [patch\_letter](#module\_patch\_letter) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | +| [post\_letters](#module\_post\_letters) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | +| [post\_mi](#module\_post\_mi) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | | [s3bucket\_test\_letters](#module\_s3bucket\_test\_letters) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.26/terraform-s3bucket.zip | n/a | | [sqs\_alarms](#module\_sqs\_alarms) | ../../modules/alarms-sqs | n/a | | [sqs\_letter\_updates](#module\_sqs\_letter\_updates) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/3.1.5/terraform-sqs.zip | n/a | | [sqs\_supplier\_allocator](#module\_sqs\_supplier\_allocator) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/3.1.5/terraform-sqs.zip | n/a | | [sqs\_supplier\_config](#module\_sqs\_supplier\_config) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/3.1.5/terraform-sqs.zip | n/a | -| [supplier\_allocator](#module\_supplier\_allocator) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/4.0.2/terraform-lambda.zip | n/a | -| [supplier\_config\_ingress](#module\_supplier\_config\_ingress) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.29/terraform-lambda.zip | n/a | -| [supplier\_mock](#module\_supplier\_mock) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/4.0.5/terraform-lambda.zip | n/a | +| [supplier\_allocator](#module\_supplier\_allocator) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | +| [supplier\_config\_ingress](#module\_supplier\_config\_ingress) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | +| [supplier\_mock](#module\_supplier\_mock) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | | [supplier\_ssl](#module\_supplier\_ssl) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.26/terraform-ssl.zip | n/a | -| [update\_letter\_queue](#module\_update\_letter\_queue) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/4.0.2/terraform-lambda.zip | n/a | -| [upsert\_letter](#module\_upsert\_letter) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/4.0.2/terraform-lambda.zip | n/a | +| [update\_letter\_queue](#module\_update\_letter\_queue) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | +| [upsert\_letter](#module\_upsert\_letter) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/6.0.5/terraform-lambda.zip | n/a | ## Outputs | Name | Description | diff --git a/lambdas/upsert-letter/src/handler/upsert-handler.ts b/lambdas/upsert-letter/src/handler/upsert-handler.ts index 6b5199b92..0a8808b5a 100644 --- a/lambdas/upsert-letter/src/handler/upsert-handler.ts +++ b/lambdas/upsert-letter/src/handler/upsert-handler.ts @@ -48,6 +48,15 @@ function getOperationFromType(type: string): UpsertOperation { preparedRequest, allocationDetails, ); + const { clientId, campaignId, templateId } = preparedRequest.data + const dimensions: Record = { + ClientId: clientId || "unknown", + CampaignId: campaignId || "unknown", + TemplateId: templateId || "unknown", + Supplier: letterToInsert.supplierId || "unknown", + GroupId: letterToInsert.groupId || "unknown", + Status: letterToInsert.status || "unknown" + }; try { await deps.letterRepo.putLetter(letterToInsert); @@ -62,17 +71,15 @@ function getOperationFromType(type: string): UpsertOperation { // emit success metric emitIndividualMetric( deps.logger, - letterToInsert.supplierId, MetricStatus.Success, - letterToInsert.groupId, + dimensions, ); } catch (error) { // emit failure metric emitIndividualMetric( deps.logger, - letterToInsert.supplierId, MetricStatus.Failure, - letterToInsert.groupId, + dimensions, ); if (error instanceof LetterAlreadyExistsError) { deps.logger.warn({ @@ -94,6 +101,10 @@ function getOperationFromType(type: string): UpsertOperation { handler: async (request, allocationDetails, deps) => { const supplierEvent = request as LetterStatusChangeEvent; const letterToUpdate: UpdateLetter = mapToUpdateLetter(supplierEvent); + const dimensions: Record = { + Supplier: letterToUpdate.supplierId || "unknown", + Status: letterToUpdate.status || "unknown" + }; await deps.letterRepo.updateLetterStatus(letterToUpdate); deps.logger.info({ @@ -106,8 +117,8 @@ function getOperationFromType(type: string): UpsertOperation { }); emitIndividualMetric( deps.logger, - letterToUpdate.supplierId, MetricStatus.Success, + dimensions, ); }, }; @@ -176,15 +187,10 @@ async function runUpsert( async function emitIndividualMetric( logger: Logger, - supplier: string, metricKey: MetricStatus, - groupId?: string, + dimensions: Record, ) { const namespace = process.env.AWS_LAMBDA_FUNCTION_NAME || "upsertLetter"; - const dimensions: Record = { - Supplier: supplier || "unknown", - GroupId: groupId || "unknown", - }; const metric: MetricEntry = { key: metricKey, @@ -261,10 +267,14 @@ export default function createUpsertLetterHandler(deps: Deps): SQSHandler { messageId: record.messageId, message: record.body, }); + const dimensions: Record = { + Supplier: "unknown", + Status: "unknown", + }; await emitIndividualMetric( deps.logger, - "unknown", MetricStatus.Failure, + dimensions, ); batchItemFailures.push({ itemIdentifier: record.messageId }); } From c0abe4e91a39b20dad758fe9dac1f582ff6bc59a Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Fri, 9 Oct 2026 15:21:43 +0000 Subject: [PATCH 02/17] fix lint issues --- lambdas/upsert-letter/src/handler/schemas.ts | 3 +-- .../src/handler/upsert-handler.ts | 24 +++++-------------- 2 files changed, 7 insertions(+), 20 deletions(-) diff --git a/lambdas/upsert-letter/src/handler/schemas.ts b/lambdas/upsert-letter/src/handler/schemas.ts index 760e7af92..45ee50153 100644 --- a/lambdas/upsert-letter/src/handler/schemas.ts +++ b/lambdas/upsert-letter/src/handler/schemas.ts @@ -11,8 +11,7 @@ import z from "zod"; import { Deps } from "../config/deps"; export type PreparedEvents = - | LetterRequestPreparedEventV2 - | LetterRequestPreparedEvent; + LetterRequestPreparedEventV2 | LetterRequestPreparedEvent; const SupplierSpecSchema = z.object({ supplierId: z.string().min(1), diff --git a/lambdas/upsert-letter/src/handler/upsert-handler.ts b/lambdas/upsert-letter/src/handler/upsert-handler.ts index 0a8808b5a..53838231d 100644 --- a/lambdas/upsert-letter/src/handler/upsert-handler.ts +++ b/lambdas/upsert-letter/src/handler/upsert-handler.ts @@ -48,14 +48,14 @@ function getOperationFromType(type: string): UpsertOperation { preparedRequest, allocationDetails, ); - const { clientId, campaignId, templateId } = preparedRequest.data + const { campaignId, clientId, templateId } = preparedRequest.data; const dimensions: Record = { ClientId: clientId || "unknown", CampaignId: campaignId || "unknown", TemplateId: templateId || "unknown", Supplier: letterToInsert.supplierId || "unknown", GroupId: letterToInsert.groupId || "unknown", - Status: letterToInsert.status || "unknown" + Status: letterToInsert.status || "unknown", }; try { await deps.letterRepo.putLetter(letterToInsert); @@ -69,18 +69,10 @@ function getOperationFromType(type: string): UpsertOperation { letterToInsert, }); // emit success metric - emitIndividualMetric( - deps.logger, - MetricStatus.Success, - dimensions, - ); + emitIndividualMetric(deps.logger, MetricStatus.Success, dimensions); } catch (error) { // emit failure metric - emitIndividualMetric( - deps.logger, - MetricStatus.Failure, - dimensions, - ); + emitIndividualMetric(deps.logger, MetricStatus.Failure, dimensions); if (error instanceof LetterAlreadyExistsError) { deps.logger.warn({ description: "Letter already exists", @@ -103,7 +95,7 @@ function getOperationFromType(type: string): UpsertOperation { const letterToUpdate: UpdateLetter = mapToUpdateLetter(supplierEvent); const dimensions: Record = { Supplier: letterToUpdate.supplierId || "unknown", - Status: letterToUpdate.status || "unknown" + Status: letterToUpdate.status || "unknown", }; await deps.letterRepo.updateLetterStatus(letterToUpdate); @@ -115,11 +107,7 @@ function getOperationFromType(type: string): UpsertOperation { letterUpdateRequest: request, letterToUpdate, }); - emitIndividualMetric( - deps.logger, - MetricStatus.Success, - dimensions, - ); + emitIndividualMetric(deps.logger, MetricStatus.Success, dimensions); }, }; } From d6ea2214ba907ce6d1fe8d883f2ee0e862df78f7 Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Sat, 10 Oct 2026 10:18:12 +0000 Subject: [PATCH 03/17] fix failing unit tests --- .../handler/__tests__/upsert-handler.test.ts | 67 ++++++++++++++++++- .../src/handler/upsert-handler.ts | 16 +++-- 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts index 1634efe21..4ab4fd9ff 100644 --- a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts +++ b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts @@ -37,7 +37,7 @@ jest.mock("@internal/helpers", () => { const renderingSchemaVersion: string = packageJson.dependencies[ - "@nhsdigital/nhs-notify-event-schemas-letter-rendering" + "@nhsdigital/nhs-notify-event-schemas-letter-rendering" ]; function createSQSEvent(records: SQSRecord[]): SQSEvent { @@ -312,6 +312,10 @@ describe("createUpsertLetterHandler", () => { { Supplier: "supplier1", GroupId: "client1_campaign1_template1", + CampaignId: "campaign1", + ClientId: "client1", + Status: "PENDING", + TemplateId: "template1", }, expect.objectContaining({ key: MetricStatus.Success, @@ -488,7 +492,11 @@ describe("createUpsertLetterHandler", () => { "upsertLetter", { Supplier: "unknown", + Status: "RETURNED", + Client: "unknown", + CampaignId: "unknown", GroupId: "unknown", + TemplateId: "unknown", }, expect.objectContaining({ key: MetricStatus.Success, @@ -498,6 +506,55 @@ describe("createUpsertLetterHandler", () => { ); }); + test("insert with missing client, campaign and template has metric emitted with 'unknown' dimensions", async () => { + const letterEvent = createPreparedV2Event(); + letterEvent.data.clientId = ""; + delete letterEvent.data.campaignId; + delete letterEvent.data.templateId; + const message = { + letterEvent, + allocationDetails: { + supplierSpec: { + supplierId: "supplier1", + specId: "spec1", + priority: 10, + billingId: "billing1", + }, + allocationStatus: { + status: "PENDING", + }, + }, + }; + + const evt: SQSEvent = createSQSEvent([ + createSqsRecord("missing-fields", JSON.stringify(message)), + ]); + + const result = await createUpsertLetterHandler(mockedDeps)( + evt, + {} as any, + {} as any, + ); + + expect(result!.batchItemFailures).toEqual([]); + expect(buildEMFObject as jest.Mock).toHaveBeenCalledWith( + "upsertLetter", + { + ClientId: "unknown", + CampaignId: "unknown", + TemplateId: "unknown", + Supplier: "supplier1", + GroupId: "_unknown_unknown", + Status: "PENDING", + }, + expect.objectContaining({ + key: MetricStatus.Failure, + value: 1, + unit: Unit.Count, + }), + ); + }); + test("invalid JSON produces batch failure and logs error", async () => { const evt: SQSEvent = createSQSEvent([ createSqsRecord("bad-json", "this-is-not-json"), @@ -525,7 +582,11 @@ describe("createUpsertLetterHandler", () => { "upsertLetter", { Supplier: "unknown", + Status: "unknown", + Client: "unknown", + CampaignId: "unknown", GroupId: "unknown", + TemplateId: "unknown", }, expect.objectContaining({ key: MetricStatus.Failure, @@ -624,7 +685,11 @@ describe("createUpsertLetterHandler", () => { "upsertLetter", { Supplier: "unknown", + Status: "unknown", + Client: "unknown", + CampaignId: "unknown", GroupId: "unknown", + TemplateId: "unknown", }, expect.objectContaining({ key: MetricStatus.Failure, diff --git a/lambdas/upsert-letter/src/handler/upsert-handler.ts b/lambdas/upsert-letter/src/handler/upsert-handler.ts index 53838231d..f9976ff43 100644 --- a/lambdas/upsert-letter/src/handler/upsert-handler.ts +++ b/lambdas/upsert-letter/src/handler/upsert-handler.ts @@ -53,9 +53,9 @@ function getOperationFromType(type: string): UpsertOperation { ClientId: clientId || "unknown", CampaignId: campaignId || "unknown", TemplateId: templateId || "unknown", - Supplier: letterToInsert.supplierId || "unknown", - GroupId: letterToInsert.groupId || "unknown", - Status: letterToInsert.status || "unknown", + Supplier: letterToInsert.supplierId, + GroupId: letterToInsert.groupId, + Status: letterToInsert.status, }; try { await deps.letterRepo.putLetter(letterToInsert); @@ -95,7 +95,11 @@ function getOperationFromType(type: string): UpsertOperation { const letterToUpdate: UpdateLetter = mapToUpdateLetter(supplierEvent); const dimensions: Record = { Supplier: letterToUpdate.supplierId || "unknown", - Status: letterToUpdate.status || "unknown", + Status: letterToUpdate.status, + Client: "unknown", + CampaignId: "unknown", + GroupId: "unknown", + TemplateId: "unknown", }; await deps.letterRepo.updateLetterStatus(letterToUpdate); @@ -258,6 +262,10 @@ export default function createUpsertLetterHandler(deps: Deps): SQSHandler { const dimensions: Record = { Supplier: "unknown", Status: "unknown", + Client: "unknown", + CampaignId: "unknown", + GroupId: "unknown", + TemplateId: "unknown", }; await emitIndividualMetric( deps.logger, From 6e6ba826203d36518165bb0369c47e7401a29bc4 Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Sat, 10 Oct 2026 10:26:50 +0000 Subject: [PATCH 04/17] fix lint issues --- .../upsert-letter/src/handler/__tests__/upsert-handler.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts index 4ab4fd9ff..d0351faca 100644 --- a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts +++ b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts @@ -37,7 +37,7 @@ jest.mock("@internal/helpers", () => { const renderingSchemaVersion: string = packageJson.dependencies[ - "@nhsdigital/nhs-notify-event-schemas-letter-rendering" + "@nhsdigital/nhs-notify-event-schemas-letter-rendering" ]; function createSQSEvent(records: SQSRecord[]): SQSEvent { From 893e80eb7bb11020779cfb80e5f8e15a35a24a3c Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Sat, 10 Oct 2026 10:52:55 +0000 Subject: [PATCH 05/17] revert schemas to main version --- lambdas/upsert-letter/src/handler/schemas.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lambdas/upsert-letter/src/handler/schemas.ts b/lambdas/upsert-letter/src/handler/schemas.ts index 45ee50153..760e7af92 100644 --- a/lambdas/upsert-letter/src/handler/schemas.ts +++ b/lambdas/upsert-letter/src/handler/schemas.ts @@ -11,7 +11,8 @@ import z from "zod"; import { Deps } from "../config/deps"; export type PreparedEvents = - LetterRequestPreparedEventV2 | LetterRequestPreparedEvent; + | LetterRequestPreparedEventV2 + | LetterRequestPreparedEvent; const SupplierSpecSchema = z.object({ supplierId: z.string().min(1), From ac80c8952df4f0ccb6a9dfbe0cf67564d5ef5375 Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Mon, 12 Oct 2026 15:26:47 +0000 Subject: [PATCH 06/17] only emit failed metrics once. Change groupId to use hyphens --- internal/helpers/src/group-id.ts | 2 +- .../handler/__tests__/upsert-handler.test.ts | 82 ++++++++++++++++--- lambdas/upsert-letter/src/handler/schemas.ts | 3 +- .../src/handler/upsert-handler.ts | 81 ++++++++++++++---- 4 files changed, 139 insertions(+), 29 deletions(-) diff --git a/internal/helpers/src/group-id.ts b/internal/helpers/src/group-id.ts index a5a6cff2a..0fc9e8d85 100644 --- a/internal/helpers/src/group-id.ts +++ b/internal/helpers/src/group-id.ts @@ -3,5 +3,5 @@ export default function formatGroupId( campaignId = "unknown", safeTemplateId = "unknown", ): string { - return `${clientId}_${campaignId}_${safeTemplateId}`; + return `${clientId}-${campaignId}-${safeTemplateId}`; } diff --git a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts index d0351faca..bc592ac2e 100644 --- a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts +++ b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts @@ -13,7 +13,9 @@ import { } from "@nhsdigital/nhs-notify-event-schemas-supplier-api/src/events/letter-events"; import { MetricStatus, buildEMFObject } from "@internal/helpers"; import { Unit } from "aws-embedded-metrics"; -import createUpsertLetterHandler from "../upsert-handler"; +import createUpsertLetterHandler, { + extractDimensionsFromLetterEvent, +} from "../upsert-handler"; import { Deps } from "../../config/deps"; import { EnvVars } from "../../config/env"; import packageJson from "../../../package.json"; @@ -281,7 +283,7 @@ describe("createUpsertLetterHandler", () => { expect(insertedV2Letter.billingRef).toBe("spec1"); expect(insertedV2Letter.url).toBe("s3://letterDataBucket/letter1.pdf"); expect(insertedV2Letter.status).toBe("PENDING"); - expect(insertedV2Letter.groupId).toBe("client1_campaign1_template1"); + expect(insertedV2Letter.groupId).toBe("client1-campaign1-template1"); expect(insertedV2Letter.source).toBe("/data-plane/letter-rendering/test"); expect(insertedV2Letter.specificationBillingId).toBe("billing1"); expect(insertedV2Letter.priority).toBe(10); @@ -294,7 +296,7 @@ describe("createUpsertLetterHandler", () => { expect(insertedV1Letter.billingRef).toBe("spec2"); expect(insertedV1Letter.url).toBe("s3://letterDataBucket/letter1.pdf"); expect(insertedV1Letter.status).toBe("PENDING"); - expect(insertedV1Letter.groupId).toBe("client1_campaign1_template1"); + expect(insertedV1Letter.groupId).toBe("client1-campaign1-template1"); expect(insertedV1Letter.source).toBe("/data-plane/letter-rendering/test"); expect(insertedV1Letter.specificationBillingId).toBe("billing2"); expect(insertedV1Letter.priority).toBe(10); @@ -311,7 +313,7 @@ describe("createUpsertLetterHandler", () => { "upsertLetter", { Supplier: "supplier1", - GroupId: "client1_campaign1_template1", + GroupId: "client1-campaign1-template1", CampaignId: "campaign1", ClientId: "client1", Status: "PENDING", @@ -392,7 +394,7 @@ describe("createUpsertLetterHandler", () => { expect(insertedV2Letter.reasonText).toBe( "No suppliers available for allocation of V2", ); - expect(insertedV2Letter.groupId).toBe("client1_campaign1_template1"); + expect(insertedV2Letter.groupId).toBe("client1-campaign1-template1"); expect(insertedV2Letter.source).toBe("/data-plane/letter-rendering/test"); expect(insertedV2Letter.specificationBillingId).toBe("unknown"); expect(insertedV2Letter.priority).toBe(0); @@ -409,7 +411,7 @@ describe("createUpsertLetterHandler", () => { expect(insertedV1Letter.reasonText).toBe( "No suppliers available for allocation of V1", ); - expect(insertedV1Letter.groupId).toBe("client1_campaign1_template1"); + expect(insertedV1Letter.groupId).toBe("client1-campaign1-template1"); expect(insertedV1Letter.source).toBe("/data-plane/letter-rendering/test"); expect(insertedV1Letter.specificationBillingId).toBe("unknown"); expect(insertedV1Letter.priority).toBe(0); @@ -493,9 +495,9 @@ describe("createUpsertLetterHandler", () => { { Supplier: "unknown", Status: "RETURNED", - Client: "unknown", + ClientId: "client_template", CampaignId: "unknown", - GroupId: "unknown", + GroupId: "client_template", TemplateId: "unknown", }, expect.objectContaining({ @@ -506,6 +508,64 @@ describe("createUpsertLetterHandler", () => { ); }); + test("extracts insert dimensions without allocation details", () => { + const letterEvent = createPreparedV2Event(); + + expect(extractDimensionsFromLetterEvent(letterEvent)).toEqual({ + Supplier: "unknown", + Status: "PREPARED", + ClientId: "client1", + CampaignId: "campaign1", + GroupId: "client1-campaign1-template1", + TemplateId: "template1", + }); + }); + + test("uses unknown update dimensions when group ID is empty or partial", async () => { + const emptyGroupEvent = createSupplierStatusChangeEvent(); + emptyGroupEvent.data.groupId = ""; + const partialGroupEvent = createSupplierStatusChangeEvent({ + id: "23f1f09c-a555-4d9b-8405-0b33490bc921", + }); + partialGroupEvent.data.groupId = "-campaign-template"; + const evt: SQSEvent = createSQSEvent([ + createSqsRecord("empty-group", JSON.stringify(emptyGroupEvent)), + createSqsRecord("partial-group", JSON.stringify(partialGroupEvent)), + ]); + + const result = await createUpsertLetterHandler(mockedDeps)( + evt, + {} as any, + {} as any, + ); + + expect(result!.batchItemFailures).toEqual([]); + expect(buildEMFObject as jest.Mock).toHaveBeenCalledWith( + "upsertLetter", + { + Supplier: "supplier1", + Status: "RETURNED", + ClientId: "unknown", + CampaignId: "unknown", + GroupId: "unknown", + TemplateId: "unknown", + }, + expect.objectContaining({ key: MetricStatus.Success }), + ); + expect(buildEMFObject as jest.Mock).toHaveBeenCalledWith( + "upsertLetter", + { + Supplier: "supplier1", + Status: "RETURNED", + ClientId: "unknown", + CampaignId: "campaign", + GroupId: "-campaign-template", + TemplateId: "template", + }, + expect.objectContaining({ key: MetricStatus.Success }), + ); + }); + test("insert with missing client, campaign and template has metric emitted with 'unknown' dimensions", async () => { const letterEvent = createPreparedV2Event(); letterEvent.data.clientId = ""; @@ -544,7 +604,7 @@ describe("createUpsertLetterHandler", () => { CampaignId: "unknown", TemplateId: "unknown", Supplier: "supplier1", - GroupId: "_unknown_unknown", + GroupId: "-unknown-unknown", Status: "PENDING", }, expect.objectContaining({ @@ -583,7 +643,7 @@ describe("createUpsertLetterHandler", () => { { Supplier: "unknown", Status: "unknown", - Client: "unknown", + ClientId: "unknown", CampaignId: "unknown", GroupId: "unknown", TemplateId: "unknown", @@ -686,7 +746,7 @@ describe("createUpsertLetterHandler", () => { { Supplier: "unknown", Status: "unknown", - Client: "unknown", + ClientId: "unknown", CampaignId: "unknown", GroupId: "unknown", TemplateId: "unknown", diff --git a/lambdas/upsert-letter/src/handler/schemas.ts b/lambdas/upsert-letter/src/handler/schemas.ts index 760e7af92..45ee50153 100644 --- a/lambdas/upsert-letter/src/handler/schemas.ts +++ b/lambdas/upsert-letter/src/handler/schemas.ts @@ -11,8 +11,7 @@ import z from "zod"; import { Deps } from "../config/deps"; export type PreparedEvents = - | LetterRequestPreparedEventV2 - | LetterRequestPreparedEvent; + LetterRequestPreparedEventV2 | LetterRequestPreparedEvent; const SupplierSpecSchema = z.object({ supplierId: z.string().min(1), diff --git a/lambdas/upsert-letter/src/handler/upsert-handler.ts b/lambdas/upsert-letter/src/handler/upsert-handler.ts index f9976ff43..4b9f4a086 100644 --- a/lambdas/upsert-letter/src/handler/upsert-handler.ts +++ b/lambdas/upsert-letter/src/handler/upsert-handler.ts @@ -31,6 +31,15 @@ import { UpsertOperation, } from "./schemas"; +interface UpsertDimensions extends Record { + Supplier: string; + Status: string; + ClientId: string; + CampaignId: string; + GroupId: string; + TemplateId: string; +} + const idempotencyConfig = new IdempotencyConfig({ eventKeyJmesPath: "id", }); @@ -49,7 +58,7 @@ function getOperationFromType(type: string): UpsertOperation { allocationDetails, ); const { campaignId, clientId, templateId } = preparedRequest.data; - const dimensions: Record = { + const dimensions: UpsertDimensions = { ClientId: clientId || "unknown", CampaignId: campaignId || "unknown", TemplateId: templateId || "unknown", @@ -92,14 +101,16 @@ function getOperationFromType(type: string): UpsertOperation { schemas: [$LetterStatusChangeEvent], handler: async (request, allocationDetails, deps) => { const supplierEvent = request as LetterStatusChangeEvent; + const groupId = supplierEvent.data?.groupId || "unknown"; + const [clientId, campaignId, templateId] = groupId.split("-"); const letterToUpdate: UpdateLetter = mapToUpdateLetter(supplierEvent); - const dimensions: Record = { + const dimensions: UpsertDimensions = { Supplier: letterToUpdate.supplierId || "unknown", Status: letterToUpdate.status, - Client: "unknown", - CampaignId: "unknown", - GroupId: "unknown", - TemplateId: "unknown", + ClientId: clientId || "unknown", + CampaignId: campaignId || "unknown", + GroupId: groupId, + TemplateId: templateId || "unknown", }; await deps.letterRepo.updateLetterStatus(letterToUpdate); @@ -116,6 +127,42 @@ function getOperationFromType(type: string): UpsertOperation { }; } +export function extractDimensionsFromLetterEvent( + letterEvent: LetterStatusChangeEvent | PreparedEvents, + allocationDetails: AllocationDetails | undefined, +): UpsertDimensions { + const { type } = letterEvent; + // Insert type + if ( + type.startsWith("uk.nhs.notify.letter-rendering.letter-request.prepared") + ) { + const preparedRequest = letterEvent as PreparedEvents; + const { campaignId, clientId, status, templateId } = preparedRequest.data; + const supplierId = allocationDetails?.supplierSpec.supplierId; + const groupId = formatGroupId(clientId, campaignId, templateId); + return { + Supplier: supplierId || "unknown", + Status: status, + ClientId: clientId || "unknown", + CampaignId: campaignId || "unknown", + GroupId: groupId, + TemplateId: templateId || "unknown", + }; + } + // Update type + const changeEvent = letterEvent as LetterStatusChangeEvent; + const groupId = changeEvent.data.groupId || "unknown"; + const [clientId, campaignId, templateId] = groupId.split("-"); + return { + Supplier: changeEvent.data.supplierId || "unknown", + Status: changeEvent.data.status, + ClientId: clientId || "unknown", + CampaignId: campaignId || "unknown", + GroupId: groupId, + TemplateId: templateId || "unknown", + }; +} + function mapToInsertLetter( upsertRequest: PreparedEvents, allocationDetails: AllocationDetails, @@ -180,7 +227,7 @@ async function runUpsert( async function emitIndividualMetric( logger: Logger, metricKey: MetricStatus, - dimensions: Record, + dimensions: UpsertDimensions, ) { const namespace = process.env.AWS_LAMBDA_FUNCTION_NAME || "upsertLetter"; @@ -221,6 +268,14 @@ export default function createUpsertLetterHandler(deps: Deps): SQSHandler { return async (event: SQSEvent, context: Context) => { const batchItemFailures: SQSBatchItemFailure[] = []; + let dimensions: UpsertDimensions = { + Supplier: "unknown", + Status: "unknown", + ClientId: "unknown", + CampaignId: "unknown", + GroupId: "unknown", + TemplateId: "unknown", + }; const tasks = event.Records.map(async (record) => { try { deps.logger.info({ @@ -251,6 +306,10 @@ export default function createUpsertLetterHandler(deps: Deps): SQSHandler { }); idempotencyConfig.registerLambdaContext(context); + dimensions = extractDimensionsFromLetterEvent( + letterEvent, + allocationDetails, + ); await processRecordIdempotently(letterEvent, allocationDetails, deps); } catch (error) { deps.logger.error({ @@ -259,14 +318,6 @@ export default function createUpsertLetterHandler(deps: Deps): SQSHandler { messageId: record.messageId, message: record.body, }); - const dimensions: Record = { - Supplier: "unknown", - Status: "unknown", - Client: "unknown", - CampaignId: "unknown", - GroupId: "unknown", - TemplateId: "unknown", - }; await emitIndividualMetric( deps.logger, MetricStatus.Failure, From a51ab9e9e2364eff96fc03a4354f1321f51d8af3 Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Mon, 12 Oct 2026 16:14:54 +0000 Subject: [PATCH 07/17] fix lint issues --- lambdas/supplier-allocator/src/handler/types.ts | 3 +-- tests/helpers/generate-fetch-test-data.ts | 8 ++------ .../letter-request-prepared.consumer.pact.test.ts | 2 +- .../sandbox/testCases/update-letter-status-test-cases.ts | 4 +--- 4 files changed, 5 insertions(+), 12 deletions(-) diff --git a/lambdas/supplier-allocator/src/handler/types.ts b/lambdas/supplier-allocator/src/handler/types.ts index 5c762ccea..c62b4f855 100644 --- a/lambdas/supplier-allocator/src/handler/types.ts +++ b/lambdas/supplier-allocator/src/handler/types.ts @@ -37,5 +37,4 @@ export const PreparedEventSchema = z.discriminatedUnion("type", [ ]); export type PreparedEvents = - | LetterRequestPreparedEventV2 - | LetterRequestPreparedEvent; + LetterRequestPreparedEventV2 | LetterRequestPreparedEvent; diff --git a/tests/helpers/generate-fetch-test-data.ts b/tests/helpers/generate-fetch-test-data.ts index c033bd68a..e5ca3abd6 100644 --- a/tests/helpers/generate-fetch-test-data.ts +++ b/tests/helpers/generate-fetch-test-data.ts @@ -112,9 +112,7 @@ type FetchLettersWithRetryOptions = { }; type GetLettersResponseBody = - | GetLettersResponse - | ErrorResponse - | Record; + GetLettersResponse | ErrorResponse | Record; type FetchLettersWithRetryResult = { statusCode: number; @@ -122,9 +120,7 @@ type FetchLettersWithRetryResult = { }; type GetMIResponseBody = - | GetMIResponse - | ErrorResponse - | Record; + GetMIResponse | ErrorResponse | Record; type FetchMiResult = { statusCode: number; diff --git a/tests/pact-tests/consumer/letter-request-prepared.consumer.pact.test.ts b/tests/pact-tests/consumer/letter-request-prepared.consumer.pact.test.ts index 0430bd129..82f1093ed 100644 --- a/tests/pact-tests/consumer/letter-request-prepared.consumer.pact.test.ts +++ b/tests/pact-tests/consumer/letter-request-prepared.consumer.pact.test.ts @@ -1,4 +1,4 @@ -/* eslint-disable security/detect-unsafe-regex, sonarjs/slow-regex */ +/* eslint-disable security/detect-unsafe-regex */ import path from "node:path"; import { MatchersV3, diff --git a/tests/sandbox/testCases/update-letter-status-test-cases.ts b/tests/sandbox/testCases/update-letter-status-test-cases.ts index 061e30b84..640981cba 100644 --- a/tests/sandbox/testCases/update-letter-status-test-cases.ts +++ b/tests/sandbox/testCases/update-letter-status-test-cases.ts @@ -16,9 +16,7 @@ export type ApiSandboxUpdateLetterStatusTestData = { body?: PatchMessageRequestBody; expectedStatus: number; expectedResponse?: - | PatchMessageResponseBody - | SandboxErrorResponse - | ErrorMessageBody; + PatchMessageResponseBody | SandboxErrorResponse | ErrorMessageBody; }; export const apiSandboxUpdateLetterStatusTestData: ApiSandboxUpdateLetterStatusTestData[] = From 1f782698e4a4f3001084dbf87d574fb8f7335128 Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Mon, 12 Oct 2026 16:26:13 +0000 Subject: [PATCH 08/17] revert lint fixes --- lambdas/supplier-allocator/src/handler/types.ts | 3 ++- tests/helpers/generate-fetch-test-data.ts | 8 ++++++-- .../sandbox/testCases/update-letter-status-test-cases.ts | 4 +++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/lambdas/supplier-allocator/src/handler/types.ts b/lambdas/supplier-allocator/src/handler/types.ts index c62b4f855..5c762ccea 100644 --- a/lambdas/supplier-allocator/src/handler/types.ts +++ b/lambdas/supplier-allocator/src/handler/types.ts @@ -37,4 +37,5 @@ export const PreparedEventSchema = z.discriminatedUnion("type", [ ]); export type PreparedEvents = - LetterRequestPreparedEventV2 | LetterRequestPreparedEvent; + | LetterRequestPreparedEventV2 + | LetterRequestPreparedEvent; diff --git a/tests/helpers/generate-fetch-test-data.ts b/tests/helpers/generate-fetch-test-data.ts index e5ca3abd6..c033bd68a 100644 --- a/tests/helpers/generate-fetch-test-data.ts +++ b/tests/helpers/generate-fetch-test-data.ts @@ -112,7 +112,9 @@ type FetchLettersWithRetryOptions = { }; type GetLettersResponseBody = - GetLettersResponse | ErrorResponse | Record; + | GetLettersResponse + | ErrorResponse + | Record; type FetchLettersWithRetryResult = { statusCode: number; @@ -120,7 +122,9 @@ type FetchLettersWithRetryResult = { }; type GetMIResponseBody = - GetMIResponse | ErrorResponse | Record; + | GetMIResponse + | ErrorResponse + | Record; type FetchMiResult = { statusCode: number; diff --git a/tests/sandbox/testCases/update-letter-status-test-cases.ts b/tests/sandbox/testCases/update-letter-status-test-cases.ts index 640981cba..061e30b84 100644 --- a/tests/sandbox/testCases/update-letter-status-test-cases.ts +++ b/tests/sandbox/testCases/update-letter-status-test-cases.ts @@ -16,7 +16,9 @@ export type ApiSandboxUpdateLetterStatusTestData = { body?: PatchMessageRequestBody; expectedStatus: number; expectedResponse?: - PatchMessageResponseBody | SandboxErrorResponse | ErrorMessageBody; + | PatchMessageResponseBody + | SandboxErrorResponse + | ErrorMessageBody; }; export const apiSandboxUpdateLetterStatusTestData: ApiSandboxUpdateLetterStatusTestData[] = From 33a7a172534cece45529c89244383be30efb0bfd Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Mon, 12 Oct 2026 16:46:39 +0000 Subject: [PATCH 09/17] fix typecheck issues --- .../handler/__tests__/upsert-handler.test.ts | 17 +++++++++++++++-- lambdas/upsert-letter/src/handler/schemas.ts | 3 ++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts index bc592ac2e..e40771570 100644 --- a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts +++ b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts @@ -39,7 +39,7 @@ jest.mock("@internal/helpers", () => { const renderingSchemaVersion: string = packageJson.dependencies[ - "@nhsdigital/nhs-notify-event-schemas-letter-rendering" + "@nhsdigital/nhs-notify-event-schemas-letter-rendering" ]; function createSQSEvent(records: SQSRecord[]): SQSEvent { @@ -510,8 +510,21 @@ describe("createUpsertLetterHandler", () => { test("extracts insert dimensions without allocation details", () => { const letterEvent = createPreparedV2Event(); + const allocationDetails = { + supplierSpec: { + supplierId: "supplier1", + specId: "spec1", + priority: 10, + billingId: "billing1", + }, + allocationStatus: { + status: "PENDING", + }, + }; - expect(extractDimensionsFromLetterEvent(letterEvent)).toEqual({ + expect( + extractDimensionsFromLetterEvent(letterEvent, allocationDetails), + ).toEqual({ Supplier: "unknown", Status: "PREPARED", ClientId: "client1", diff --git a/lambdas/upsert-letter/src/handler/schemas.ts b/lambdas/upsert-letter/src/handler/schemas.ts index 45ee50153..760e7af92 100644 --- a/lambdas/upsert-letter/src/handler/schemas.ts +++ b/lambdas/upsert-letter/src/handler/schemas.ts @@ -11,7 +11,8 @@ import z from "zod"; import { Deps } from "../config/deps"; export type PreparedEvents = - LetterRequestPreparedEventV2 | LetterRequestPreparedEvent; + | LetterRequestPreparedEventV2 + | LetterRequestPreparedEvent; const SupplierSpecSchema = z.object({ supplierId: z.string().min(1), From d0cf2213579a105918d191dd739a5fed1e5d8e6e Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Mon, 12 Oct 2026 21:37:56 +0000 Subject: [PATCH 10/17] fix unit test and lint... again --- .../src/handler/__tests__/upsert-handler.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts index e40771570..2e7bdd9b7 100644 --- a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts +++ b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts @@ -39,7 +39,7 @@ jest.mock("@internal/helpers", () => { const renderingSchemaVersion: string = packageJson.dependencies[ - "@nhsdigital/nhs-notify-event-schemas-letter-rendering" + "@nhsdigital/nhs-notify-event-schemas-letter-rendering" ]; function createSQSEvent(records: SQSRecord[]): SQSEvent { @@ -512,7 +512,7 @@ describe("createUpsertLetterHandler", () => { const letterEvent = createPreparedV2Event(); const allocationDetails = { supplierSpec: { - supplierId: "supplier1", + supplierId: "", specId: "spec1", priority: 10, billingId: "billing1", From 9f7dfff6c06387b97244e87ebbfae91265a91223 Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Tue, 13 Oct 2026 08:16:14 +0000 Subject: [PATCH 11/17] fix failure metric from logging twice --- lambdas/upsert-letter/src/handler/upsert-handler.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/lambdas/upsert-letter/src/handler/upsert-handler.ts b/lambdas/upsert-letter/src/handler/upsert-handler.ts index 4b9f4a086..788b61e34 100644 --- a/lambdas/upsert-letter/src/handler/upsert-handler.ts +++ b/lambdas/upsert-letter/src/handler/upsert-handler.ts @@ -80,8 +80,6 @@ function getOperationFromType(type: string): UpsertOperation { // emit success metric emitIndividualMetric(deps.logger, MetricStatus.Success, dimensions); } catch (error) { - // emit failure metric - emitIndividualMetric(deps.logger, MetricStatus.Failure, dimensions); if (error instanceof LetterAlreadyExistsError) { deps.logger.warn({ description: "Letter already exists", From e7f8f5c9f728558c03a2512278bcff862c4f4a92 Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Tue, 13 Oct 2026 09:10:06 +0000 Subject: [PATCH 12/17] still log failure metric for LetterAlreadyExistsError --- lambdas/upsert-letter/src/handler/upsert-handler.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/lambdas/upsert-letter/src/handler/upsert-handler.ts b/lambdas/upsert-letter/src/handler/upsert-handler.ts index 788b61e34..3f3cbe71a 100644 --- a/lambdas/upsert-letter/src/handler/upsert-handler.ts +++ b/lambdas/upsert-letter/src/handler/upsert-handler.ts @@ -86,6 +86,7 @@ function getOperationFromType(type: string): UpsertOperation { supplierId: letterToInsert.supplierId, letterId: letterToInsert.id, }); + emitIndividualMetric(deps.logger, MetricStatus.Failure, dimensions); } else { throw error; } From 935fc8c04b7d23b7bdb86952522fbf94450049fc Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Tue, 13 Oct 2026 10:32:57 +0000 Subject: [PATCH 13/17] reuse extract dimensions function --- .../handler/__tests__/upsert-handler.test.ts | 2 +- .../src/handler/upsert-handler.ts | 33 ++++++++----------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts index 2e7bdd9b7..de56c12a5 100644 --- a/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts +++ b/lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts @@ -526,7 +526,7 @@ describe("createUpsertLetterHandler", () => { extractDimensionsFromLetterEvent(letterEvent, allocationDetails), ).toEqual({ Supplier: "unknown", - Status: "PREPARED", + Status: "PENDING", ClientId: "client1", CampaignId: "campaign1", GroupId: "client1-campaign1-template1", diff --git a/lambdas/upsert-letter/src/handler/upsert-handler.ts b/lambdas/upsert-letter/src/handler/upsert-handler.ts index 3f3cbe71a..2b9310737 100644 --- a/lambdas/upsert-letter/src/handler/upsert-handler.ts +++ b/lambdas/upsert-letter/src/handler/upsert-handler.ts @@ -57,15 +57,10 @@ function getOperationFromType(type: string): UpsertOperation { preparedRequest, allocationDetails, ); - const { campaignId, clientId, templateId } = preparedRequest.data; - const dimensions: UpsertDimensions = { - ClientId: clientId || "unknown", - CampaignId: campaignId || "unknown", - TemplateId: templateId || "unknown", - Supplier: letterToInsert.supplierId, - GroupId: letterToInsert.groupId, - Status: letterToInsert.status, - }; + const dimensions: UpsertDimensions = extractDimensionsFromLetterEvent( + request, + allocationDetails, + ); try { await deps.letterRepo.putLetter(letterToInsert); @@ -100,17 +95,11 @@ function getOperationFromType(type: string): UpsertOperation { schemas: [$LetterStatusChangeEvent], handler: async (request, allocationDetails, deps) => { const supplierEvent = request as LetterStatusChangeEvent; - const groupId = supplierEvent.data?.groupId || "unknown"; - const [clientId, campaignId, templateId] = groupId.split("-"); const letterToUpdate: UpdateLetter = mapToUpdateLetter(supplierEvent); - const dimensions: UpsertDimensions = { - Supplier: letterToUpdate.supplierId || "unknown", - Status: letterToUpdate.status, - ClientId: clientId || "unknown", - CampaignId: campaignId || "unknown", - GroupId: groupId, - TemplateId: templateId || "unknown", - }; + const dimensions: UpsertDimensions = extractDimensionsFromLetterEvent( + request, + allocationDetails, + ); await deps.letterRepo.updateLetterStatus(letterToUpdate); deps.logger.info({ @@ -136,8 +125,12 @@ export function extractDimensionsFromLetterEvent( type.startsWith("uk.nhs.notify.letter-rendering.letter-request.prepared") ) { const preparedRequest = letterEvent as PreparedEvents; - const { campaignId, clientId, status, templateId } = preparedRequest.data; + const { campaignId, clientId, templateId } = preparedRequest.data; const supplierId = allocationDetails?.supplierSpec.supplierId; + const status = + allocationDetails.allocationStatus.status === "REJECTED" + ? "REJECTED" + : "PENDING"; const groupId = formatGroupId(clientId, campaignId, templateId); return { Supplier: supplierId || "unknown", From 8f4d2482d9f5025708c8af589e0ea956400e5723 Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Tue, 13 Oct 2026 11:05:17 +0000 Subject: [PATCH 14/17] fix typecheck --- lambdas/upsert-letter/src/handler/upsert-handler.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lambdas/upsert-letter/src/handler/upsert-handler.ts b/lambdas/upsert-letter/src/handler/upsert-handler.ts index 2b9310737..06c232987 100644 --- a/lambdas/upsert-letter/src/handler/upsert-handler.ts +++ b/lambdas/upsert-letter/src/handler/upsert-handler.ts @@ -58,7 +58,7 @@ function getOperationFromType(type: string): UpsertOperation { allocationDetails, ); const dimensions: UpsertDimensions = extractDimensionsFromLetterEvent( - request, + request as LetterStatusChangeEvent | PreparedEvents, allocationDetails, ); try { @@ -97,7 +97,7 @@ function getOperationFromType(type: string): UpsertOperation { const supplierEvent = request as LetterStatusChangeEvent; const letterToUpdate: UpdateLetter = mapToUpdateLetter(supplierEvent); const dimensions: UpsertDimensions = extractDimensionsFromLetterEvent( - request, + request as LetterStatusChangeEvent | PreparedEvents, allocationDetails, ); await deps.letterRepo.updateLetterStatus(letterToUpdate); @@ -128,7 +128,7 @@ export function extractDimensionsFromLetterEvent( const { campaignId, clientId, templateId } = preparedRequest.data; const supplierId = allocationDetails?.supplierSpec.supplierId; const status = - allocationDetails.allocationStatus.status === "REJECTED" + allocationDetails?.allocationStatus.status === "REJECTED" ? "REJECTED" : "PENDING"; const groupId = formatGroupId(clientId, campaignId, templateId); From af507ec20cd6d2d262fced2f9bb9649747cddcdc Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Wed, 14 Oct 2026 11:12:21 +0000 Subject: [PATCH 15/17] add timestamp to EMF metrics object logs --- internal/events/schemas/examples/letter.ACCEPTED.json | 2 +- internal/events/schemas/examples/letter.CANCELLED.json | 2 +- internal/events/schemas/examples/letter.DELIVERED.json | 2 +- internal/events/schemas/examples/letter.DISPATCHED.json | 2 +- internal/events/schemas/examples/letter.ENCLOSED.json | 2 +- internal/events/schemas/examples/letter.FAILED.json | 2 +- internal/events/schemas/examples/letter.FORWARDED.json | 2 +- internal/events/schemas/examples/letter.PENDING.json | 2 +- internal/events/schemas/examples/letter.PRINTED.json | 2 +- internal/events/schemas/examples/letter.REJECTED.json | 2 +- internal/events/schemas/examples/letter.RETURNED.json | 2 +- internal/helpers/src/metrics.ts | 8 ++++---- 12 files changed, 15 insertions(+), 15 deletions(-) diff --git a/internal/events/schemas/examples/letter.ACCEPTED.json b/internal/events/schemas/examples/letter.ACCEPTED.json index 62ea85bb9..e113c8151 100644 --- a/internal/events/schemas/examples/letter.ACCEPTED.json +++ b/internal/events/schemas/examples/letter.ACCEPTED.json @@ -2,7 +2,7 @@ "data": { "billingRef": "1y3q9v1zzzz", "domainId": "e9dc9ee9-b2f8-4ef2-8351-73a9a29f8e96_38U220Ghb1WXF8hCK6hL0LGya3Y_38U23YjtnKSZypvaCSb07adMOlq", - "groupId": "client_template", + "groupId": "client-campaign-template", "origin": { "domain": "letter-rendering", "event": "f47ac10b-58cc-4372-a567-0e02b2c3d479", diff --git a/internal/events/schemas/examples/letter.CANCELLED.json b/internal/events/schemas/examples/letter.CANCELLED.json index 95c7638d7..87a569d81 100644 --- a/internal/events/schemas/examples/letter.CANCELLED.json +++ b/internal/events/schemas/examples/letter.CANCELLED.json @@ -2,7 +2,7 @@ "data": { "billingRef": "1y3q9v1zzzz", "domainId": "e9dc9ee9-b2f8-4ef2-8351-73a9a29f8e96_38U220Ghb1WXF8hCK6hL0LGya3Y_38U23YjtnKSZypvaCSb07adMOlq", - "groupId": "client_template", + "groupId": "client-campaign-template", "origin": { "domain": "letter-rendering", "event": "f47ac10b-58cc-4372-a567-0e02b2c3d479", diff --git a/internal/events/schemas/examples/letter.DELIVERED.json b/internal/events/schemas/examples/letter.DELIVERED.json index a15fd21e9..d9acbc605 100644 --- a/internal/events/schemas/examples/letter.DELIVERED.json +++ b/internal/events/schemas/examples/letter.DELIVERED.json @@ -2,7 +2,7 @@ "data": { "billingRef": "1y3q9v1zzzz", "domainId": "e9dc9ee9-b2f8-4ef2-8351-73a9a29f8e96_38U220Ghb1WXF8hCK6hL0LGya3Y_38U23YjtnKSZypvaCSb07adMOlq", - "groupId": "client_template", + "groupId": "client-campaign-template", "origin": { "domain": "letter-rendering", "event": "f47ac10b-58cc-4372-a567-0e02b2c3d479", diff --git a/internal/events/schemas/examples/letter.DISPATCHED.json b/internal/events/schemas/examples/letter.DISPATCHED.json index 782ee5c15..da1b68edc 100644 --- a/internal/events/schemas/examples/letter.DISPATCHED.json +++ b/internal/events/schemas/examples/letter.DISPATCHED.json @@ -2,7 +2,7 @@ "data": { "billingRef": "1y3q9v1zzzz", "domainId": "e9dc9ee9-b2f8-4ef2-8351-73a9a29f8e96_38U220Ghb1WXF8hCK6hL0LGya3Y_38U23YjtnKSZypvaCSb07adMOlq", - "groupId": "client_template", + "groupId": "client-campaign-template", "origin": { "domain": "letter-rendering", "event": "f47ac10b-58cc-4372-a567-0e02b2c3d479", diff --git a/internal/events/schemas/examples/letter.ENCLOSED.json b/internal/events/schemas/examples/letter.ENCLOSED.json index 610d157da..4d9aad83b 100644 --- a/internal/events/schemas/examples/letter.ENCLOSED.json +++ b/internal/events/schemas/examples/letter.ENCLOSED.json @@ -2,7 +2,7 @@ "data": { "billingRef": "1y3q9v1zzzz", "domainId": "e9dc9ee9-b2f8-4ef2-8351-73a9a29f8e96_38U220Ghb1WXF8hCK6hL0LGya3Y_38U23YjtnKSZypvaCSb07adMOlq", - "groupId": "client_template", + "groupId": "client-campaign-template", "origin": { "domain": "letter-rendering", "event": "f47ac10b-58cc-4372-a567-0e02b2c3d479", diff --git a/internal/events/schemas/examples/letter.FAILED.json b/internal/events/schemas/examples/letter.FAILED.json index 9ef34aa14..5bdd1b841 100644 --- a/internal/events/schemas/examples/letter.FAILED.json +++ b/internal/events/schemas/examples/letter.FAILED.json @@ -2,7 +2,7 @@ "data": { "billingRef": "1y3q9v1zzzz", "domainId": "e9dc9ee9-b2f8-4ef2-8351-73a9a29f8e96_38U220Ghb1WXF8hCK6hL0LGya3Y_38U23YjtnKSZypvaCSb07adMOlq", - "groupId": "client_template", + "groupId": "client-campaign-template", "origin": { "domain": "letter-rendering", "event": "f47ac10b-58cc-4372-a567-0e02b2c3d479", diff --git a/internal/events/schemas/examples/letter.FORWARDED.json b/internal/events/schemas/examples/letter.FORWARDED.json index bc1f48cf3..2bbe69606 100644 --- a/internal/events/schemas/examples/letter.FORWARDED.json +++ b/internal/events/schemas/examples/letter.FORWARDED.json @@ -2,7 +2,7 @@ "data": { "billingRef": "1y3q9v1zzzz", "domainId": "e9dc9ee9-b2f8-4ef2-8351-73a9a29f8e96_38U220Ghb1WXF8hCK6hL0LGya3Y_38U23YjtnKSZypvaCSb07adMOlq", - "groupId": "client_template", + "groupId": "client-campaign-template", "origin": { "domain": "letter-rendering", "event": "f47ac10b-58cc-4372-a567-0e02b2c3d479", diff --git a/internal/events/schemas/examples/letter.PENDING.json b/internal/events/schemas/examples/letter.PENDING.json index c5c0fe697..798b71d23 100644 --- a/internal/events/schemas/examples/letter.PENDING.json +++ b/internal/events/schemas/examples/letter.PENDING.json @@ -2,7 +2,7 @@ "data": { "billingRef": "1y3q9v1zzzz", "domainId": "e9dc9ee9-b2f8-4ef2-8351-73a9a29f8e96_38U220Ghb1WXF8hCK6hL0LGya3Y_38U23YjtnKSZypvaCSb07adMOlq", - "groupId": "client_template", + "groupId": "client-campaign-template", "origin": { "domain": "letter-rendering", "event": "f47ac10b-58cc-4372-a567-0e02b2c3d479", diff --git a/internal/events/schemas/examples/letter.PRINTED.json b/internal/events/schemas/examples/letter.PRINTED.json index 9c8cf131d..f1c6412a3 100644 --- a/internal/events/schemas/examples/letter.PRINTED.json +++ b/internal/events/schemas/examples/letter.PRINTED.json @@ -2,7 +2,7 @@ "data": { "billingRef": "1y3q9v1zzzz", "domainId": "e9dc9ee9-b2f8-4ef2-8351-73a9a29f8e96_38U220Ghb1WXF8hCK6hL0LGya3Y_38U23YjtnKSZypvaCSb07adMOlq", - "groupId": "client_template", + "groupId": "client-campaign-template", "origin": { "domain": "letter-rendering", "event": "f47ac10b-58cc-4372-a567-0e02b2c3d479", diff --git a/internal/events/schemas/examples/letter.REJECTED.json b/internal/events/schemas/examples/letter.REJECTED.json index d8fee1b71..d608c8115 100644 --- a/internal/events/schemas/examples/letter.REJECTED.json +++ b/internal/events/schemas/examples/letter.REJECTED.json @@ -2,7 +2,7 @@ "data": { "billingRef": "1y3q9v1zzzz", "domainId": "e9dc9ee9-b2f8-4ef2-8351-73a9a29f8e96_38U220Ghb1WXF8hCK6hL0LGya3Y_38U23YjtnKSZypvaCSb07adMOlq", - "groupId": "client_template", + "groupId": "client-campaign-template", "origin": { "domain": "letter-rendering", "event": "f47ac10b-58cc-4372-a567-0e02b2c3d479", diff --git a/internal/events/schemas/examples/letter.RETURNED.json b/internal/events/schemas/examples/letter.RETURNED.json index 7b7121d8f..9396b8a36 100644 --- a/internal/events/schemas/examples/letter.RETURNED.json +++ b/internal/events/schemas/examples/letter.RETURNED.json @@ -2,7 +2,7 @@ "data": { "billingRef": "1y3q9v1zzzz", "domainId": "e9dc9ee9-b2f8-4ef2-8351-73a9a29f8e96_38U220Ghb1WXF8hCK6hL0LGya3Y_38U23YjtnKSZypvaCSb07adMOlq", - "groupId": "client_template", + "groupId": "client-campaign-template", "origin": { "domain": "letter-rendering", "event": "f47ac10b-58cc-4372-a567-0e02b2c3d479", diff --git a/internal/helpers/src/metrics.ts b/internal/helpers/src/metrics.ts index 1b3929b4e..3aa37c701 100644 --- a/internal/helpers/src/metrics.ts +++ b/internal/helpers/src/metrics.ts @@ -34,16 +34,16 @@ export function buildEMFObject( metric: MetricEntry, ) { const namespace = process.env.AWS_LAMBDA_FUNCTION_NAME || functionName; + const timeStamp = Date.now(); return { - LogGroup: namespace, - ServiceName: namespace, + TimeStamp: timeStamp, ...dimensions, _aws: { - Timestamp: Date.now(), + Timestamp: timeStamp, CloudWatchMetrics: [ { Namespace: namespace, - Dimensions: [[...Object.keys(dimensions), "ServiceName", "LogGroup"]], + Dimensions: [[...Object.keys(dimensions), "TimeStamp"]], Metrics: [ { Name: metric.key, Value: metric.value, Unit: metric.unit }, ], From dddf6b5a836b481f04851ca305ac8040bf63c2e7 Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Wed, 14 Oct 2026 11:24:07 +0000 Subject: [PATCH 16/17] increment internal events version --- internal/events/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/events/package.json b/internal/events/package.json index ca6177c5a..28cc9ffa2 100644 --- a/internal/events/package.json +++ b/internal/events/package.json @@ -36,5 +36,5 @@ "typecheck": "tsc --noEmit" }, "types": "dist/index.d.ts", - "version": "1.0.22" + "version": "1.0.23" } From 321c65e28edb025b5ed66af5099b5f1a8e81cbc4 Mon Sep 17 00:00:00 2001 From: Vlasis-Perdikidis Date: Wed, 14 Oct 2026 12:23:49 +0000 Subject: [PATCH 17/17] remove unused timestamp dimension --- internal/helpers/src/metrics.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/internal/helpers/src/metrics.ts b/internal/helpers/src/metrics.ts index 3aa37c701..74ed3622a 100644 --- a/internal/helpers/src/metrics.ts +++ b/internal/helpers/src/metrics.ts @@ -34,16 +34,14 @@ export function buildEMFObject( metric: MetricEntry, ) { const namespace = process.env.AWS_LAMBDA_FUNCTION_NAME || functionName; - const timeStamp = Date.now(); return { - TimeStamp: timeStamp, ...dimensions, _aws: { - Timestamp: timeStamp, + Timestamp: Date.now(), CloudWatchMetrics: [ { Namespace: namespace, - Dimensions: [[...Object.keys(dimensions), "TimeStamp"]], + Dimensions: [Object.keys(dimensions)], Metrics: [ { Name: metric.key, Value: metric.value, Unit: metric.unit }, ],