CCM-12858: fixing tests and tidy up

Author: nhsd-angel-pastor

lambdas/core-notifier-lambda/src/__tests__/apis/sqs-handler.test.ts

@@ -1,23 +1,48 @@
 import type { SQSEvent, SQSRecord } from 'aws-lambda';
 import { mock } from 'jest-mock-extended';
 import { Logger, Sender } from 'utils';
-import { PDMResourceAvailable } from 'digital-letters-events';
 import { NotifyMessageProcessor } from 'app/notify-message-processor';
-import { SenderManagement } from 'sender-management';
+import { ISenderManagement } from 'sender-management';
 import { EventPublisherFacade } from 'infra/event-publisher-facade';
 import { SqsHandlerDependencies, createHandler } from 'apis/sqs-handler';
 import { parseSqsRecord } from 'app/parse-sqs-message';
 import { InvalidPdmResourceAvailableEvent } from 'domain/invalid-pdm-resource-available-event';
 import { RequestNotifyError } from 'domain/request-notify-error';
+import { validPdmEvent, validSender } from '__tests__/constants';
 
 jest.mock('app/parse-sqs-message');
 
 const mockLogger = mock<Logger>();
 const mockNotifyMessageProcessor = mock<NotifyMessageProcessor>();
-const mockSenderManagement = mock<SenderManagement>();
+const mockSenderManagement = mock<ISenderManagement>();
 const mockEventPublisherFacade = mock<EventPublisherFacade>();
 const mockParseSqsRecord = jest.mocked(parseSqsRecord);
 
+const createSqsRecord = (messageId: string): SQSRecord => ({
+  messageId,
+  receiptHandle: 'receipt-handle',
+  body: JSON.stringify({
+    detail: validPdmEvent,
+  }),
+  attributes: {
+    ApproximateReceiveCount: '1',
+    SentTimestamp: '1234567890',
+    SenderId: 'sender-id',
+    ApproximateFirstReceiveTimestamp: '1234567890',
+  },
+  messageAttributes: {},
+  md5OfBody: 'md5',
+  eventSource: 'aws:sqs',
+  eventSourceARN: 'arn:aws:sqs:region:account:queue',
+  awsRegion: 'eu-west-2',
+});
+
+const createSqsEvent = (recordCount: number): SQSEvent => ({
+  Records: Array.from({ length: recordCount }, (_, i) =>
+    createSqsRecord(`message-id-${i + 1}`),
+  ),
+});
+
 describe('createHandler', () => {
   const dependencies: SqsHandlerDependencies = {
     logger: mockLogger,
@@ -27,60 +52,9 @@ describe('createHandler', () => {
   };
 
   const senderId = 'sender-123';
-  const routingConfigId = 'routing-config-123';
   const messageReference = 'msg-ref-123';
   const notifyId = 'notify-id-123';
 
-  const mockSender: Sender = {
-    senderId,
-    routingConfigId,
-    senderName: 'Test Sender',
-    meshMailboxSenderId: 'meshMailBoxSender-123',
-    meshMailboxReportsId: 'meshMailBoxReports-123',
-    fallbackWaitTimeSeconds: 100,
-  };
-
-  const mockPdmEvent: PDMResourceAvailable = {
-    id: 'event-id-123',
-    source: 'urn:nhs:names:services:notify:pdm',
-    specversion: '1.0',
-    type: 'uk.nhs.notify.digital.letters.pdm.resource.available.v1',
-    time: '2025-12-15T10:00:00Z',
-    datacontenttype: 'application/json',
-    data: {
-      senderId,
-      messageReference,
-      resourceId: 'ResourceId-123',
-      nhsNumber: '9991234566',
-      odsCode: 'A12345',
-    },
-  };
-
-  const createSqsRecord = (messageId: string): SQSRecord => ({
-    messageId,
-    receiptHandle: 'receipt-handle',
-    body: JSON.stringify({
-      detail: mockPdmEvent,
-    }),
-    attributes: {
-      ApproximateReceiveCount: '1',
-      SentTimestamp: '1234567890',
-      SenderId: 'sender-id',
-      ApproximateFirstReceiveTimestamp: '1234567890',
-    },
-    messageAttributes: {},
-    md5OfBody: 'md5',
-    eventSource: 'aws:sqs',
-    eventSourceARN: 'arn:aws:sqs:region:account:queue',
-    awsRegion: 'eu-west-2',
-  });
-
-  const createSqsEvent = (recordCount: number): SQSEvent => ({
-    Records: Array.from({ length: recordCount }, (_, i) =>
-      createSqsRecord(`message-id-${i + 1}`),
-    ),
-  });
-
   beforeEach(() => {
     jest.clearAllMocks();
   });
@@ -90,8 +64,8 @@ describe('createHandler', () => {
       const sqsEvent = createSqsEvent(1);
       const handler = createHandler(dependencies);
 
-      mockParseSqsRecord.mockReturnValueOnce(mockPdmEvent);
-      mockSenderManagement.getSender.mockReturnValueOnce(mockSender);
+      mockParseSqsRecord.mockReturnValueOnce(validPdmEvent);
+      mockSenderManagement.getSender.mockResolvedValue(validSender);
       mockNotifyMessageProcessor.process.mockResolvedValueOnce(notifyId);
 
       const result = await handler(sqsEvent);
@@ -107,46 +81,63 @@ describe('createHandler', () => {
         sqsEvent.Records[0],
         mockLogger,
       );
-      expect(mockSenderManagement.getSender).toHaveBeenCalledWith(senderId);
+      expect(mockSenderManagement.getSender).toHaveBeenCalledWith({
+        senderId,
+      });
       expect(mockNotifyMessageProcessor.process).toHaveBeenCalledTimes(1);
       expect(
         mockEventPublisherFacade.publishMessageRequestSubmitted,
       ).toHaveBeenCalledTimes(1);
     });
-  });
 
-  describe('when sender has no routing config', () => {
     it('skips the message and publishes a skipped event', async () => {
       const sqsEvent = createSqsEvent(1);
       const handler = createHandler(dependencies);
       const senderWithoutRouting: Sender = {
-        ...mockSender,
+        ...validSender,
         routingConfigId: undefined,
       };
 
-      mockParseSqsRecord.mockReturnValueOnce(mockPdmEvent);
-      mockSenderManagement.getSender.mockReturnValueOnce(senderWithoutRouting);
+      mockParseSqsRecord.mockReturnValueOnce(validPdmEvent);
+      mockSenderManagement.getSender.mockResolvedValue(senderWithoutRouting);
 
       const result = await handler(sqsEvent);
 
       expect(result).toEqual({ batchItemFailures: [] });
-      expect(mockLogger.debug).toHaveBeenCalledWith(
-        `No routing config for sender ${senderId}, skipping message`,
-      );
       expect(mockNotifyMessageProcessor.process).not.toHaveBeenCalled();
       expect(
         mockEventPublisherFacade.publishMessageRequestSkipped,
       ).toHaveBeenCalledTimes(1);
     });
+
+    it('throws an error when sender is not found', async () => {
+      const sqsEvent = createSqsEvent(1);
+      const handler = createHandler(dependencies);
+
+      mockParseSqsRecord.mockReturnValueOnce(validPdmEvent);
+      mockSenderManagement.getSender.mockResolvedValue(null);
+
+      const result = await handler(sqsEvent);
+
+      expect(result).toEqual({
+        batchItemFailures: [{ itemIdentifier: sqsEvent.Records[0].messageId }],
+      });
+      expect(mockLogger.warn).toHaveBeenCalledWith(
+        expect.objectContaining({
+          description: 'Failed processing message',
+          messageId: sqsEvent.Records[0].messageId,
+        }),
+      );
+    });
   });
 
   describe('when processing multiple SQS records', () => {
     it('processes all records successfully', async () => {
       const sqsEvent = createSqsEvent(3);
       const handler = createHandler(dependencies);
 
-      mockParseSqsRecord.mockReturnValue(mockPdmEvent);
-      mockSenderManagement.getSender.mockReturnValue(mockSender);
+      mockParseSqsRecord.mockReturnValue(validPdmEvent);
+      mockSenderManagement.getSender.mockResolvedValue(validSender);
       mockNotifyMessageProcessor.process.mockResolvedValue(notifyId);
 
       const result = await handler(sqsEvent);
@@ -161,6 +152,30 @@ describe('createHandler', () => {
       expect(mockParseSqsRecord).toHaveBeenCalledTimes(3);
       expect(mockNotifyMessageProcessor.process).toHaveBeenCalledTimes(3);
     });
+
+    it('returns only failed message IDs', async () => {
+      const sqsEvent = createSqsEvent(3);
+      const handler = createHandler(dependencies);
+
+      mockParseSqsRecord
+        .mockReturnValueOnce(validPdmEvent)
+        .mockImplementationOnce(() => {
+          throw new Error('Parse error');
+        })
+        .mockReturnValueOnce(validPdmEvent);
+
+      mockSenderManagement.getSender.mockResolvedValue(validSender);
+      mockNotifyMessageProcessor.process.mockResolvedValue(notifyId);
+
+      const result = await handler(sqsEvent);
+
+      expect(result).toEqual({
+        batchItemFailures: [{ itemIdentifier: 'message-id-2' }],
+      });
+      expect(mockLogger.info).toHaveBeenCalledWith(
+        '2 of 3 records processed successfully',
+      );
+    });
   });
 
   describe('when parseSqsRecord throws InvalidPdmResourceAvailableEvent', () => {
@@ -189,21 +204,15 @@ describe('createHandler', () => {
     });
   });
 
-  describe('when processing throws RequestNotifyError', () => {
-    it('marks the message as failed for retry since error lacks messageReference', async () => {
+  describe('when processing throws error', () => {
+    it('marks the message as failed as is not a RequestNotifyError', async () => {
       const sqsEvent = createSqsEvent(1);
       const handler = createHandler(dependencies);
       const { messageId } = sqsEvent.Records[0];
-      const errorCode = 'VALIDATION_ERROR';
-      const correlationId = 'corr-123';
-      const error = new RequestNotifyError(
-        new Error('Validation failed'),
-        correlationId,
-        errorCode,
-      );
+      const error = new Error('Validation failed');
 
-      mockParseSqsRecord.mockReturnValueOnce(mockPdmEvent);
-      mockSenderManagement.getSender.mockReturnValueOnce(mockSender);
+      mockParseSqsRecord.mockReturnValueOnce(validPdmEvent);
+      mockSenderManagement.getSender.mockResolvedValue(validSender);
       mockNotifyMessageProcessor.process.mockRejectedValueOnce(error);
 
       const result = await handler(sqsEvent);
@@ -237,8 +246,8 @@ describe('createHandler', () => {
       // Add messageReference property dynamically to trigger the terminal error path
       (error as any).messageReference = messageReference;
 
-      mockParseSqsRecord.mockReturnValueOnce(mockPdmEvent);
-      mockSenderManagement.getSender.mockReturnValueOnce(mockSender);
+      mockParseSqsRecord.mockReturnValueOnce(validPdmEvent);
+      mockSenderManagement.getSender.mockResolvedValue(validSender);
       mockNotifyMessageProcessor.process.mockRejectedValueOnce(error);
 
       const result = await handler(sqsEvent);
@@ -266,75 +275,4 @@ describe('createHandler', () => {
       );
     });
   });
-
-  describe('when processing throws a generic error', () => {
-    it('marks the message as failed for retry', async () => {
-      const sqsEvent = createSqsEvent(1);
-      const handler = createHandler(dependencies);
-      const { messageId } = sqsEvent.Records[0];
-      const error = new Error('Unexpected error');
-
-      mockParseSqsRecord.mockReturnValueOnce(mockPdmEvent);
-      mockSenderManagement.getSender.mockReturnValueOnce(mockSender);
-      mockNotifyMessageProcessor.process.mockRejectedValueOnce(error);
-
-      const result = await handler(sqsEvent);
-
-      expect(result).toEqual({
-        batchItemFailures: [{ itemIdentifier: messageId }],
-      });
-      expect(mockLogger.warn).toHaveBeenCalledWith({
-        error: error.message,
-        description: 'Failed processing message',
-        messageId,
-      });
-      expect(
-        mockEventPublisherFacade.publishMessageRequestRejected,
-      ).not.toHaveBeenCalled();
-    });
-  });
-
-  describe('when processing mixed success and failure records', () => {
-    it('returns only failed message IDs', async () => {
-      const sqsEvent = createSqsEvent(3);
-      const handler = createHandler(dependencies);
-
-      mockParseSqsRecord
-        .mockReturnValueOnce(mockPdmEvent)
-        .mockImplementationOnce(() => {
-          throw new Error('Parse error');
-        })
-        .mockReturnValueOnce(mockPdmEvent);
-
-      mockSenderManagement.getSender.mockReturnValue(mockSender);
-      mockNotifyMessageProcessor.process.mockResolvedValue(notifyId);
-
-      const result = await handler(sqsEvent);
-
-      expect(result).toEqual({
-        batchItemFailures: [{ itemIdentifier: 'message-id-2' }],
-      });
-      expect(mockLogger.info).toHaveBeenCalledWith(
-        '2 of 3 records processed successfully',
-      );
-    });
-  });
-
-  describe('when notifyMessageProcessor returns undefined', () => {
-    it('does not publish submitted event', async () => {
-      const sqsEvent = createSqsEvent(1);
-      const handler = createHandler(dependencies);
-
-      mockParseSqsRecord.mockReturnValueOnce(mockPdmEvent);
-      mockSenderManagement.getSender.mockReturnValueOnce(mockSender);
-      mockNotifyMessageProcessor.process.mockResolvedValueOnce();
-
-      const result = await handler(sqsEvent);
-
-      expect(result).toEqual({ batchItemFailures: [] });
-      expect(
-        mockEventPublisherFacade.publishMessageRequestSubmitted,
-      ).not.toHaveBeenCalled();
-    });
-  });
 });

lambdas/core-notifier-lambda/src/__tests__/app/notify-message-processor.test.ts

@@ -62,15 +62,17 @@ describe('NotifyMessageProcessor', () => {
     });
   });
 
-  it('does not re-throw when a RequestAlreadyReceivedError is thrown by the API client', async () => {
+  it('re-throw when a RequestAlreadyReceivedError is thrown by the API client', async () => {
     const { messageReference } = mockRequest1.data.attributes;
     const err = new RequestAlreadyReceivedError(
       new Error('Request was already received!'),
       messageReference,
     );
     mockClient.sendRequest.mockRejectedValue(err);
 
-    expect(await notifyMessageProcessor.process(mockRequest1)).toBeUndefined();
+    await expect(notifyMessageProcessor.process(mockRequest1)).rejects.toThrow(
+      err,
+    );
 
     expect(mockLogger.info).toHaveBeenCalledWith(
       'Request has already been received by Notify',

lambdas/core-notifier-lambda/src/__tests__/app/parse-sqs-message.test.ts

@@ -3,7 +3,7 @@ import { mock } from 'jest-mock-extended';
 import { Logger } from 'utils';
 import { parseSqsRecord } from 'app/parse-sqs-message';
 import { InvalidPdmResourceAvailableEvent } from 'domain/invalid-pdm-resource-available-event';
-import { PDMResourceAvailable } from 'digital-letters-events';
+import { validPdmEvent } from '__tests__/constants';
 
 // Import the mocked validator after the mock setup
 import { messageDownloadedValidator } from 'digital-letters-events/PDMResourceAvailable.js';
@@ -16,21 +16,6 @@ const mockLogger = mock<Logger>();
 
 describe('parseSqsRecord', () => {
   const messageId = 'test-message-id-123';
-  const validPdmEvent: PDMResourceAvailable = {
-    id: 'event-id-123',
-    source: 'urn:nhs:names:services:notify:pdm',
-    specversion: '1.0',
-    type: 'uk.nhs.notify.digital.letters.pdm.resource.available.v1',
-    time: '2025-12-15T10:00:00Z',
-    datacontenttype: 'application/json',
-    data: {
-      senderId: 'sender-123',
-      messageReference: 'ref-123',
-      resourceId: 'ResourceId-123',
-      nhsNumber: '9991234566',
-      odsCode: 'A12345',
-    },
-  };
 
   const createSqsRecord = (detail: any): SQSRecord => ({
     messageId,