Review fixes

Author: nhsd-david-wass

lambdas/supplier-allocator/package.json

@@ -4,6 +4,7 @@
     "@aws-sdk/client-sqs": "^3.984.0",
     "@aws-sdk/lib-dynamodb": "^3.858.0",
     "@internal/datastore": "*",
+    "@internal/helpers": "^0.1.0",
     "@nhsdigital/nhs-notify-event-schemas-letter-rendering": "^2.0.1",
     "@nhsdigital/nhs-notify-event-schemas-letter-rendering-v1": "npm:@nhsdigital/nhs-notify-event-schemas-letter-rendering@^1.1.5",
     "@nhsdigital/nhs-notify-event-schemas-supplier-api": "^1.0.8",

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

@@ -14,35 +14,30 @@ describe("createDependenciesContainer", () => {
     jest.clearAllMocks();
     jest.resetModules();
 
-    // pino
-    jest.mock("pino", () => ({
-      __esModule: true,
-      default: jest.fn(() => ({
+    // @internal/helpers - createLogger
+    jest.mock("@internal/helpers", () => ({
+      createLogger: jest.fn(() => ({
         info: jest.fn(),
         error: jest.fn(),
         warn: jest.fn(),
         debug: jest.fn(),
+        level: "info",
       })),
     }));
 
-    // Repo client
-    jest.mock("@internal/datastore", () => ({
-      LetterRepository: jest.fn(),
-    }));
-
     // Env
     jest.mock("../env", () => ({ envVars: env }));
   });
 
   test("constructs deps and wires repository config correctly", async () => {
     // get current mock instances
-    const pinoMock = jest.requireMock("pino");
+    const { createLogger } = jest.requireMock("@internal/helpers");
 
     // eslint-disable-next-line @typescript-eslint/no-require-imports
     const { createDependenciesContainer } = require("../deps");
     const deps: Deps = createDependenciesContainer();
+    expect(createLogger).toHaveBeenCalledTimes(1);
 
-    expect(pinoMock.default).toHaveBeenCalledTimes(1);
     expect(deps.env).toEqual(env);
   });
 });

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

@@ -1,15 +1,16 @@
 import { SQSClient } from "@aws-sdk/client-sqs";
-import pino from "pino";
+import { Logger } from "pino";
+import { createLogger } from "@internal/helpers";
 import { EnvVars, envVars } from "./env";
 
 export type Deps = {
-  logger: pino.Logger;
+  logger: Logger;
   env: EnvVars;
   sqsClient: SQSClient;
 };
 
 export function createDependenciesContainer(): Deps {
-  const log = pino();
+  const log = createLogger({ logLevel: envVars.PINO_LOG_LEVEL });
 
   return {
     logger: log,

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

@@ -10,6 +10,7 @@ const LetterVariantSchema = z.record(
 export type LetterVariant = z.infer<typeof LetterVariantSchema>;
 
 const EnvVarsSchema = z.object({
+  PINO_LOG_LEVEL: z.coerce.string().optional(),
   VARIANT_MAP: z.string().transform((str, _) => {
     const parsed = JSON.parse(str);
     return LetterVariantSchema.parse(parsed);

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

@@ -16,7 +16,10 @@ function resolveSupplierForVariant(
   variantId: string,
   deps: Deps,
 ): SupplierSpec {
-  deps.logger.info({ variantId }, "Resolving supplier for letter variant");
+  deps.logger.info({
+    description: "Resolving supplier for letter variant",
+    variantId,
+  });
   return deps.env.VARIANT_MAP[variantId];
 }
 
@@ -71,10 +74,11 @@ export default function createSupplierAllocatorHandler(deps: Deps): SQSHandler {
           supplierSpec,
         };
 
-        deps.logger.info(
-          { msg: queueMessage, url: queueUrl },
-          "Sending message to upsert letter queue",
-        );
+        deps.logger.info({
+          description: "Sending message to upsert letter queue",
+          msg: queueMessage,
+          url: queueUrl,
+        });
 
         await deps.sqsClient.send(
           new SendMessageCommand({
@@ -83,10 +87,12 @@ export default function createSupplierAllocatorHandler(deps: Deps): SQSHandler {
           }),
         );
       } catch (error) {
-        deps.logger.error(
-          { err: error, message: record.body },
-          `Error processing allocation of record ${record.messageId}`,
-        );
+        deps.logger.error({
+          description: "Error processing allocation of record",
+          err: error,
+          messageId: record.messageId,
+          message: record.body,
+        });
         batchItemFailures.push({ itemIdentifier: record.messageId });
       }
     });

lambdas/upsert-letter/src/handler/upsert-handler.ts

@@ -37,6 +37,8 @@ const QueueMessageSchema = z.union([
   }),
 ]);
 
+type QueueMessage = z.infer<typeof QueueMessageSchema>;
+
 type UpsertOperation = {
   name: "Insert" | "Update";
   schemas: z.ZodSchema[];
@@ -173,7 +175,7 @@ function getSupplierIdFromEvent(letterEvent: any): string {
   return "unknown";
 }
 
-function parseQueueMessage(queueMessage: string): any {
+function parseQueueMessage(queueMessage: string): QueueMessage {
   const result = QueueMessageSchema.safeParse(queueMessage);
 
   if (!result.success) {
@@ -201,18 +203,18 @@ export default function createUpsertLetterHandler(deps: Deps): SQSHandler {
             messageId: record.messageId,
             message: record.body,
           });
-          const queueMessage = JSON.parse(record.body);
+          const sqsMessage = JSON.parse(record.body);
 
-          const sqsEvent = parseQueueMessage(queueMessage);
+          const queueMessage: QueueMessage = parseQueueMessage(sqsMessage);
 
-          let letterEvent: any;
+          let letterEvent: LetterEvent | PreparedEvents;
           let supplierSpec: SupplierSpec | undefined;
 
-          if ("letterEvent" in sqsEvent) {
-            letterEvent = sqsEvent.letterEvent;
-            supplierSpec = sqsEvent.supplierSpec;
+          if ("letterEvent" in queueMessage) {
+            letterEvent = queueMessage.letterEvent;
+            supplierSpec = queueMessage.supplierSpec;
           } else {
-            letterEvent = sqsEvent;
+            letterEvent = queueMessage;
             supplierSpec = undefined;
           }