CCM-12858: fixing tests and tidy up

Author: nhsd-angel-pastor

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

@@ -1,20 +1,20 @@
 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);
 
@@ -31,36 +31,11 @@ describe('createHandler', () => {
   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,
+      detail: validPdmEvent,
     }),
     attributes: {
       ApproximateReceiveCount: '1',
@@ -90,8 +65,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,32 +82,27 @@ describe('createHandler', () => {
         sqsEvent.Records[0],
         mockLogger,
       );
-      expect(mockSenderManagement.getSender).toHaveBeenCalledWith(senderId);
+      expect(mockSenderManagement.getSender).toHaveBeenCalledWith({ senderId: 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,
@@ -145,8 +115,8 @@ describe('createHandler', () => {
       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 +131,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 +183,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 +225,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 +254,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,7 @@ 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,

lambdas/core-notifier-lambda/src/__tests__/constants.ts

@@ -2,6 +2,9 @@ import type {
   SingleMessageRequest,
   SingleMessageResponse,
 } from 'domain/request';
+import { Sender } from 'utils';
+
+import { PDMResourceAvailable } from 'digital-letters-events';
 
 export const mockRoutingPlanId = 'routing-plan-id';
 
@@ -49,3 +52,32 @@ export const mockResponse: SingleMessageResponse = {
     },
   },
 };
+
+export 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',
+  subject: 'message-subject-123',
+  traceparent: '00-trace-parent',
+  recordedtime: '2025-12-15T10:00:00Z',
+  severitynumber: 2,
+  data: {
+    senderId : 'sender-123',
+    messageReference: 'msg-ref-123',
+    resourceId: 'ResourceId-123',
+    nhsNumber: '9991234566',
+    odsCode: 'A12345',
+  },
+};
+
+export const validSender: Sender = {
+    senderId: 'sender-123',
+    routingConfigId: 'routing-config-123',
+    senderName: 'Test Sender',
+    meshMailboxSenderId: 'meshMailBoxSender-123',
+    meshMailboxReportsId: 'meshMailBoxReports-123',
+    fallbackWaitTimeSeconds: 100,
+  };