CCM-11343: remove filters from pipes and implement in code

Author: bhansell1

infrastructure/terraform/modules/backend-api/pipes_pipe_template_table_events.tf

@@ -19,43 +19,6 @@ resource "aws_pipes_pipe" "template_table_events" {
         arn = module.sqs_template_table_events_pipe_dlq.sqs_queue_arn
       }
     }
-
-    filter_criteria {
-      filter {
-        pattern = jsonencode({
-          "$or" = [
-            {
-              # Allow all non-LETTER templates
-              dynamodb = {
-                NewImage = {
-                  templateType = {
-                    S = [{ "anything-but" = "LETTER" }]
-                  }
-                }
-              }
-              eventName = ["INSERT", "MODIFY"]
-            },
-            {
-              # Allow LETTER templates only when proofingEnabled and valid status
-              dynamodb = {
-                NewImage = {
-                  templateType = {
-                    S = ["LETTER"]
-                  }
-                  templateStatus = {
-                    S = ["PROOF_AVAILABLE", "SUBMITTED", "DELETED"]
-                  }
-                  proofingEnabled = {
-                    BOOL = [true]
-                  }
-                }
-              }
-              eventName = ["INSERT", "MODIFY"]
-            }
-          ]
-        })
-      }
-    }
   }
 
   target_parameters {

lambdas/event-publisher/src/__tests__/domain/event-builder.test.ts

@@ -2,12 +2,22 @@ import { mockDeep } from 'jest-mock-extended';
 import { EventBuilder } from '../../domain/event-builder';
 import { Logger } from 'nhs-notify-web-template-management-utils/logger';
 import { PublishableEventRecord } from '../../domain/input-schemas';
+import { shouldPublish } from '../../domain/should-publish';
+
+jest.mock('../../domain/should-publish');
 
 beforeAll(() => {
   jest.useFakeTimers();
   jest.setSystemTime(new Date('2022-01-01 09:00'));
 });
 
+beforeEach(() => {
+  jest.resetAllMocks();
+  shouldPublishMock.mockReturnValueOnce(true);
+});
+
+const shouldPublishMock = jest.mocked(shouldPublish);
+
 const mockLogger = mockDeep<Logger>();
 
 const eventBuilder = new EventBuilder('table-name', 'event-source', mockLogger);
@@ -52,6 +62,9 @@ const publishableEventRecord = (status: string): PublishableEventRecord => ({
       letterType: {
         S: 'x0',
       },
+      proofingEnabled: {
+        BOOL: true,
+      },
       files: {
         M: {
           pdfTemplate: {
@@ -175,6 +188,17 @@ test('builds template deleted event', () => {
   );
 });
 
+test('should return undefined when not a publishable event', () => {
+  shouldPublishMock.mockReset();
+  shouldPublishMock.mockReturnValueOnce(false);
+
+  const event = eventBuilder.buildEvent(
+    publishableEventRecord('PROOF_AVAILABLE')
+  );
+
+  expect(event).toEqual(undefined);
+});
+
 test('does not build template event on hard delete', () => {
   const hardDeletePublishableEventRecord = {
     ...publishableEventRecord('SUBMITTED'),

lambdas/event-publisher/src/__tests__/domain/should-publish.test.ts

@@ -0,0 +1,50 @@
+import {
+  LetterProperties,
+  TemplateDto,
+  TemplateStatus,
+  TemplateType,
+} from 'nhs-notify-backend-client';
+import { shouldPublish } from '../../domain/should-publish';
+
+describe('shouldPublish', () => {
+  test.each(['EMAIL', 'SMS', 'NHS_APP'] satisfies TemplateType[])(
+    'templateType %p should return true',
+    (type) => {
+      expect(shouldPublish({ templateType: type } as TemplateDto)).toEqual(
+        true
+      );
+    }
+  );
+
+  test.each([false, undefined])(
+    'templateType LETTER should return false when proofingEnabled %p',
+    (proofingEnabled) => {
+      expect(
+        shouldPublish({
+          templateType: 'LETTER',
+          templateStatus: 'SUBMITTED',
+          proofingEnabled,
+        } as TemplateDto & LetterProperties)
+      ).toEqual(false);
+    }
+  );
+
+  test.each([
+    'DELETED',
+    'PENDING_PROOF_REQUEST',
+    'PROOF_AVAILABLE',
+    'SUBMITTED',
+    'WAITING_FOR_PROOF',
+  ] satisfies TemplateStatus[])(
+    'templateType LETTER should return true when templateStatus is %p and proofingEnabled is true',
+    (templateStatus) => {
+      expect(
+        shouldPublish({
+          templateType: 'LETTER',
+          templateStatus,
+          proofingEnabled: true,
+        } as TemplateDto & LetterProperties)
+      ).toEqual(true);
+    }
+  );
+});

lambdas/event-publisher/src/domain/event-builder.ts

@@ -6,6 +6,8 @@ import {
   EventMetadata,
   EventMetadataVersionInformation,
 } from './base-metadata-schemas';
+import { shouldPublish } from './should-publish';
+import { $TemplateDtoSchema } from 'nhs-notify-backend-client';
 
 export class EventBuilder {
   constructor(
@@ -73,6 +75,22 @@ export class EventBuilder {
 
     const dynamoRecord = unmarshall(publishableEventRecord.dynamodb.NewImage);
 
+    // This isn't strictly correct. the DTO schema is slightly different from the DB schema.
+    // I guess this should be a Zod schema of the database schema
+    // Also doing this may cause this lambda to stop being backwards compatible? I.E. we add a new field
+    // then older templates being updated without the field will cause errors.
+    // Or I just rely on the any typing...
+    const databaseTemplate = $TemplateDtoSchema.parse(dynamoRecord);
+
+    if (!shouldPublish(databaseTemplate)) {
+      this.logger.debug({
+        description: 'Not publishing event',
+        publishableEventRecord,
+      });
+
+      return undefined;
+    }
+
     return $Event.parse({
       ...this.buildTemplateSavedEventMetadata(
         publishableEventRecord.eventID,

lambdas/event-publisher/src/domain/should-publish.ts

@@ -0,0 +1,29 @@
+import {
+  TemplateStatus,
+  TemplateDto,
+  LetterProperties,
+} from 'nhs-notify-backend-client';
+
+type LetterTemplateDto = TemplateDto & LetterProperties;
+
+const publishableLetterStatuses = new Set<TemplateStatus>([
+  'DELETED',
+  'PENDING_PROOF_REQUEST',
+  'PROOF_AVAILABLE',
+  'SUBMITTED',
+  'WAITING_FOR_PROOF',
+]);
+
+function shouldPublishLetter(data: LetterTemplateDto): boolean {
+  return (
+    publishableLetterStatuses.has(data.templateStatus) && !!data.proofingEnabled
+  );
+}
+
+export function shouldPublish(data: TemplateDto) {
+  if (data.templateType === 'LETTER') {
+    return shouldPublishLetter(data);
+  }
+
+  return true;
+}