Skip to content

Commit 8de7174

Browse files
authored
Fix: [AEA-0000] - Fix minor bugs in notifications (#1583)
## Summary - Routine Change ### Details Some issues have been found in QA with the current notifications code - The initial logging message when the producer lambda checks if anything is to be added to SQS was confusing. Updated it - The status checking logic was case-sensitive (it should be case insensitive) - The lambda name was confusing This is a small PR to address these problems.
1 parent 8c57dfa commit 8de7174

3 files changed

Lines changed: 21 additions & 21 deletions

File tree

SAMtemplates/functions/main.yaml

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ Resources:
343343
SplunkSubscriptionFilterRole: !ImportValue lambda-resources:SplunkSubscriptionFilterRole
344344
SplunkDeliveryStreamArn: !ImportValue lambda-resources:SplunkDeliveryStream
345345

346-
NHSNotifyLambdaScheduleEventRole:
346+
NotifyProcessorScheduleEventRole:
347347
Type: AWS::IAM::Role
348348
Properties:
349349
AssumeRolePolicyDocument:
@@ -356,9 +356,9 @@ Resources:
356356
Action:
357357
- sts:AssumeRole
358358
ManagedPolicyArns:
359-
- !Ref NHSNotifyLambdaScheduleEventRolePolicy
359+
- !Ref NotifyProcessorScheduleEventRolePolicy
360360

361-
NHSNotifyLambdaScheduleEventRolePolicy:
361+
NotifyProcessorScheduleEventRolePolicy:
362362
Type: AWS::IAM::ManagedPolicy
363363
Properties:
364364
PolicyDocument:
@@ -368,15 +368,15 @@ Resources:
368368
Action:
369369
- lambda:InvokeFunction
370370
Resource:
371-
- !GetAtt NHSNotifyLambda.Arn
371+
- !GetAtt NotifyProcessor.Arn
372372

373-
NHSNotifyLambda:
373+
NotifyProcessor:
374374
Type: AWS::Serverless::Function
375375
Properties:
376-
FunctionName: !Sub ${StackName}-NHSNotifyLambda
376+
FunctionName: !Sub ${StackName}-NotifyProcessor
377377
CodeUri: ../../packages/
378378
Handler: nhsNotifyLambda.handler
379-
Role: !GetAtt NHSNotifyLambdaResources.Outputs.LambdaRoleArn
379+
Role: !GetAtt NotifyProcessorResources.Outputs.LambdaRoleArn
380380
Environment:
381381
Variables:
382382
LOG_LEVEL: !Ref LogLevel
@@ -388,7 +388,7 @@ Resources:
388388
Properties:
389389
Name: !Sub ${StackName}-NHSNotifySchedule
390390
ScheduleExpression: "rate(1 minute)"
391-
RoleArn: !GetAtt NHSNotifyLambdaScheduleEventRole.Arn
391+
RoleArn: !GetAtt NotifyProcessorScheduleEventRole.Arn
392392
Metadata:
393393
BuildMethod: esbuild
394394
guard:
@@ -405,14 +405,14 @@ Resources:
405405
EntryPoints:
406406
- nhsNotifyLambda/src/nhsNotifyLambda.ts
407407

408-
NHSNotifyLambdaResources:
408+
NotifyProcessorResources:
409409
Type: AWS::Serverless::Application
410410
Properties:
411411
Location: lambda_resources.yaml
412412
Parameters:
413413
StackName: !Ref StackName
414-
LambdaName: !Sub ${StackName}-NHSNotifyLambda
415-
LambdaArn: !Sub arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:function:${StackName}-NHSNotifyLambda
414+
LambdaName: !Sub ${StackName}-NotifyProcessor
415+
LambdaArn: !Sub arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:function:${StackName}-NotifyProcessor
416416
LogRetentionInDays: !Ref LogRetentionInDays
417417
CloudWatchKMSKeyId: !ImportValue account-resources:CloudwatchLogsKmsKeyArn
418418
EnableSplunk: !Ref EnableSplunk
@@ -487,10 +487,10 @@ Outputs:
487487
- !GetAtt CheckPrescriptionStatusUpdates.Arn
488488
- ""
489489

490-
NHSNotifyLambdaFunctionName:
490+
NotifyProcessorFunctionName:
491491
Description: The function name of the NHS Notify lambda
492-
Value: !Ref NHSNotifyLambda
492+
Value: !Ref NotifyProcessor
493493

494-
NHSNotifyLambdaFunctionArn:
494+
NotifyProcessorFunctionArn:
495495
Description: The function ARN of the NHS Notify lambda
496-
Value: !GetAtt NHSNotifyLambda.Arn
496+
Value: !GetAtt NotifyProcessor.Arn

packages/updatePrescriptionStatus/src/utils/sqsClient.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export async function pushPrescriptionToNotificationSQS(
5555
data: Array<DataItem>,
5656
logger: Logger
5757
) {
58-
logger.info("Pushing data items up to the notifications SQS", {count: data.length, sqsUrl})
58+
logger.info("Checking if any items require notifications", {numItemsToBeChecked: data.length, sqsUrl})
5959

6060
if (!sqsUrl) {
6161
logger.error("Notifications SQS URL not found in environment variables")
@@ -73,7 +73,7 @@ export async function pushPrescriptionToNotificationSQS(
7373

7474
for (const batch of batches) {
7575
const entries = batch
76-
.filter((item) => updateStatuses.includes(item.Status))
76+
.filter((item) => updateStatuses.includes(item.Status.toLowerCase()))
7777
// Build SQS batch entries with FIFO parameters
7878
.map((item, idx) => ({
7979
Id: idx.toString(),
@@ -92,7 +92,7 @@ export async function pushPrescriptionToNotificationSQS(
9292
}
9393

9494
logger.info(
95-
"Notification required. Pushing prescriptions with deduplication IDs",
95+
"Notification required. Pushing prescriptions to the notifications SQS with the following SQS message IDs",
9696
{deduplicationIds: entries.map(e => e.MessageDeduplicationId), requestId}
9797
)
9898

packages/updatePrescriptionStatus/tests/testSqsClient.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ describe("Unit tests for pushPrescriptionToNotificationSQS", () => {
6464

6565
// It logs the initial push attempt, but never actually sends
6666
expect(infoSpy).toHaveBeenCalledWith(
67-
"Pushing data items up to the notifications SQS",
68-
{count: data.length, sqsUrl: process.env.NHS_NOTIFY_PRESCRIPTIONS_SQS_QUEUE_URL}
67+
"Checking if any items require notifications",
68+
{numItemsToBeChecked: data.length, sqsUrl: process.env.NHS_NOTIFY_PRESCRIPTIONS_SQS_QUEUE_URL}
6969
)
7070
expect(mockSend).not.toHaveBeenCalled()
7171
})
@@ -110,7 +110,7 @@ describe("Unit tests for pushPrescriptionToNotificationSQS", () => {
110110
})
111111

112112
expect(infoSpy).toHaveBeenCalledWith(
113-
"Notification required. Pushing prescriptions with deduplication IDs",
113+
"Notification required. Pushing prescriptions to the notifications SQS with the following SQS message IDs",
114114
expect.objectContaining({requestId: "req-789", deduplicationIds: expect.any(Array)})
115115
)
116116
expect(infoSpy).toHaveBeenCalledWith(

0 commit comments

Comments
 (0)