Skip to content

Commit 6e4b4f7

Browse files
authored
New: [AEA-5367] - Silence notifications (#1775)
## Summary - ✨ New Feature ### Details Adds two new parameters, which toggle on or off the notifications integration in two places: - `EnableNotificationExternalLink` - Does all the internal stuff, but instead of calling out to the notify service, it just waits a few milliseconds and returns OK - `EnableNotificationsInternal` - Skips the notify integration completely. Both default to `False` in the event where there is an error fetching them.
1 parent ba1b3a3 commit 6e4b4f7

9 files changed

Lines changed: 186 additions & 14 deletions

File tree

SAMtemplates/functions/main.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ Parameters:
5151
NotifyAPIBaseURLParam:
5252
Type: AWS::SSM::Parameter::Name<String>
5353

54+
EnableNotificationExternalLinkParam:
55+
Type: AWS::SSM::Parameter::Name<String>
56+
57+
EnableNotificationsInternalParam:
58+
Type: AWS::SSM::Parameter::Name<String>
59+
5460
LogLevel:
5561
Type: String
5662

@@ -93,6 +99,7 @@ Resources:
9399
ENABLED_SITE_ODS_CODES_PARAM: !Ref EnabledSiteODSCodesParam
94100
ENABLED_SYSTEMS_PARAM: !Ref EnabledSystemsParam
95101
BLOCKED_SITE_ODS_CODES_PARAM: !Ref BlockedSiteODSCodesParam
102+
ENABLE_NOTIFICATIONS_PARAM: !Ref EnableNotificationsInternalParam
96103
LOG_LEVEL: !Ref LogLevel
97104
ENVIRONMENT: !Ref Environment
98105
TEST_PRESCRIPTIONS_1: "None"
@@ -396,6 +403,7 @@ Resources:
396403
API_KEY_SECRET: secrets-PSU-Notify-API-Key
397404
NHS_NOTIFY_ROUTING_ID_PARAM: !Ref NotifyRoutingPlanIDParam
398405
NOTIFY_API_BASE_URL_PARAM: !Ref NotifyAPIBaseURLParam
406+
MAKE_REAL_NOTIFY_REQUESTS_PARAM: !Ref EnableNotificationExternalLinkParam
399407
Events:
400408
ScheduleEvent:
401409
Type: ScheduleV2

SAMtemplates/main_template.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,8 @@ Resources:
159159
BlockedSiteODSCodesParam: !GetAtt Parameters.Outputs.BlockedSiteODSCodesParameterName
160160
NotifyRoutingPlanIDParam: !GetAtt Parameters.Outputs.NotifyRoutingPlanIDParameterName
161161
NotifyAPIBaseURLParam: !GetAtt Parameters.Outputs.NotifyAPIBaseURLParameterName
162+
EnableNotificationExternalLinkParam: !GetAtt Parameters.Outputs.EnableNotificationExternalLinkName
163+
EnableNotificationsInternalParam: !GetAtt Parameters.Outputs.EnableNotificationsInternalName
162164
LogLevel: !Ref LogLevel
163165
LogRetentionInDays: !Ref LogRetentionInDays
164166
EnableSplunk: !Ref EnableSplunk

SAMtemplates/parameters/main.yaml

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,22 @@ Resources:
8383
# Non-prod API URL (sandbox) (INT environment: https://int.api.service.nhs.uk/comms)
8484
- https://sandbox.api.service.nhs.uk/comms
8585

86+
EnableNotificationExternalLink:
87+
Type: AWS::SSM::Parameter
88+
Properties:
89+
Name: !Sub ${StackName}-EnableNotificationExternalLink
90+
Description: "Toggle on or off if we make real requests to the NHS notify service"
91+
Type: String
92+
Value: "false"
93+
94+
EnableNotificationsInternal:
95+
Type: AWS::SSM::Parameter
96+
Properties:
97+
Name: !Sub ${StackName}-EnableNotificationsInternal
98+
Description: "Toggle the notifications integration entirely"
99+
Type: String
100+
Value: "true"
101+
86102
Outputs:
87103
EnabledSiteODSCodesParameterName:
88104
Description: "Name of the SSM parameter holding enabled site ODS codes"
@@ -113,3 +129,15 @@ Outputs:
113129
Value: !Ref NotifyAPIBaseURLParameter
114130
Export:
115131
Name: !Sub ${StackName}-PSUNotifyApiBaseUrlParam
132+
133+
EnableNotificationExternalLinkName:
134+
Description: "Name of the SSM parameter holding the Notify API Base URL"
135+
Value: !Ref EnableNotificationExternalLink
136+
Export:
137+
Name: !Sub ${StackName}-EnableNotificationExternalLinkName
138+
139+
EnableNotificationsInternalName:
140+
Description: "Name of the SSM parameter holding the Notify API Base URL"
141+
Value: !Ref EnableNotificationsInternal
142+
Export:
143+
Name: !Sub ${StackName}-EnableNotificationsInternalName

packages/nhsNotifyLambda/.jest/setEnvVars.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@ process.env.AWS_REGION = "eu-west-2";
55
process.env.NHS_NOTIFY_ROUTING_ID_PARAM = "NHS_NOTIFY_ROUTING_ID_PARAM"
66
process.env.NOTIFY_API_BASE_URL_PARAM = "NOTIFY_API_BASE_URL_PARAM"
77
process.env.API_KEY_SECRET = "API_KEY_SECRET"
8+
process.env.MAKE_REAL_NOTIFY_REQUESTS_PARAM = "true"

packages/nhsNotifyLambda/src/utils.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,12 @@ const TTL_DELTA = 60 * 60 * 24 * 14 // Keep records for 2 weeks
2424
// For making the notify requests
2525
const NOTIFY_REQUEST_MAX_ITEMS = 45000
2626
const NOTIFY_REQUEST_MAX_BYTES = 5 * 1024 * 1024 // 5 MB
27+
const DUMMY_NOTIFY_DELAY_MS = 150
2728

2829
// these are only ever changed by a deployment
2930
const dynamoTable = process.env.TABLE_NAME
3031
const sqsUrl = process.env.NHS_NOTIFY_PRESCRIPTIONS_SQS_QUEUE_URL
32+
const MAKE_REAL_NOTIFY_REQUESTS_PARAM = process.env.MAKE_REAL_NOTIFY_REQUESTS_PARAM
3133

3234
// AWS clients
3335
const sqs = new SQSClient({region: process.env.AWS_REGION})
@@ -468,6 +470,24 @@ export async function makeBatchNotifyRequest(
468470
onRetry: onAxiosRetry
469471
})
470472

473+
let doRealRequest: boolean = false
474+
if (MAKE_REAL_NOTIFY_REQUESTS_PARAM) doRealRequest = await getParameter(MAKE_REAL_NOTIFY_REQUESTS_PARAM) === "true"
475+
476+
if (!doRealRequest) {
477+
logger.info("Not doing real Notify requests. Simply waiting for some time and returning success on all messages")
478+
await new Promise(f => setTimeout(f, DUMMY_NOTIFY_DELAY_MS))
479+
480+
// Map each input item to a "successful" NotifyDataItemMessage
481+
return data.map(item => {
482+
return {
483+
...item,
484+
messageBatchReference,
485+
success: true,
486+
notifyMessageId: v4() // Create a dummy UUID
487+
}
488+
})
489+
}
490+
471491
try {
472492
const resp = await axiosInstance.post<CreateMessageBatchResponse>("/v1/message-batches", body)
473493

packages/nhsNotifyLambda/tests/testUtils.test.ts

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const mockGetParameter = jest.fn().mockImplementation((name) => {
1515
if (name === "NOTIFY_API_BASE_URL_PARAM") {
1616
return TEST_URL
1717
}
18-
return "parameter_value"
18+
return name
1919
})
2020
jest.unstable_mockModule(
2121
"@aws-lambda-powertools/parameters/ssm",
@@ -701,5 +701,59 @@ describe("NHS notify lambda helper functions", () => {
701701

702702
expect(result).toHaveLength(2)
703703
})
704+
705+
it("uses a dummy call when the MAKE_REAL_NOTIFY_REQUESTS_PARAM is false", async () => {
706+
process.env.MAKE_REAL_NOTIFY_REQUESTS_PARAM = "false"
707+
const {makeBatchNotifyRequest: fn} = await import("../src/utils")
708+
709+
const data = [
710+
constructPSUDataItemMessage({
711+
PSUDataItem: {
712+
RequestID: "r1",
713+
PatientNHSNumber: "n1",
714+
PharmacyODSCode: "o1",
715+
TaskID: "t1",
716+
Status: "s1"
717+
}
718+
}),
719+
constructPSUDataItemMessage({
720+
PSUDataItem: {
721+
RequestID: "r2",
722+
PatientNHSNumber: "n2",
723+
PharmacyODSCode: "o2",
724+
TaskID: "t2",
725+
Status: "s2"
726+
}
727+
})
728+
]
729+
730+
// nock the POST to fail, so if nock is called the test will fail
731+
nock(TEST_URL)
732+
.post("/v1/message-batches")
733+
.reply(500)
734+
735+
const result = await fn(
736+
logger,
737+
"plan-123",
738+
data
739+
)
740+
741+
// Should return all successes
742+
expect(result).toHaveLength(2)
743+
expect(result[0]).toMatchObject({
744+
PSUDataItem: data[0].PSUDataItem,
745+
success: true,
746+
notifyMessageId: expect.any(String), // it will be assigned a dummy ID
747+
messageBatchReference: expect.any(String),
748+
messageReference: expect.any(String)
749+
})
750+
expect(result[1]).toMatchObject({
751+
PSUDataItem: data[1].PSUDataItem,
752+
success: true,
753+
notifyMessageId: expect.any(String),
754+
messageBatchReference: expect.any(String),
755+
messageReference: expect.any(String)
756+
})
757+
})
704758
})
705759
})

packages/updatePrescriptionStatus/.jest/setEnvVars.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@ process.env.SQS_SALT = "the quick brown fox something something"
55
process.env.ENABLED_SITE_ODS_CODES_PARAM = "ENABLED_SITE_ODS_CODES_PARAM"
66
process.env.ENABLED_SYSTEMS_PARAM = "ENABLED_SYSTEMS_PARAM"
77
process.env.BLOCKED_SITE_ODS_CODES_PARAM = "BLOCKED_SITE_ODS_CODES_PARAM"
8+
process.env.ENABLE_NOTIFICATIONS_PARAM = "ENABLE_NOTIFICATIONS_PARAM"

packages/updatePrescriptionStatus/src/updatePrescriptionStatus.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {APIGatewayProxyEvent, APIGatewayProxyResult} from "aws-lambda"
33
import {Logger} from "@aws-lambda-powertools/logger"
44
import {injectLambdaContext} from "@aws-lambda-powertools/logger/middleware"
55
import {TransactionCanceledException} from "@aws-sdk/client-dynamodb"
6+
import {getParameter} from "@aws-lambda-powertools/parameters/ssm"
67

78
import middy from "@middy/core"
89
import inputOutputLogger from "@middy/input-output-logger"
@@ -44,6 +45,8 @@ export const TEST_PRESCRIPTIONS_1 = (process.env.TEST_PRESCRIPTIONS_1 ?? "")
4445
export const TEST_PRESCRIPTIONS_2 = (process.env.TEST_PRESCRIPTIONS_2 ?? "")
4546
.split(",").map(item => item.trim()) || []
4647

48+
const ENABLE_NOTIFICATIONS_PARAM = process.env.ENABLE_NOTIFICATIONS_PARAM
49+
4750
const lambdaHandler = async (event: APIGatewayProxyEvent): Promise<APIGatewayProxyResult> => {
4851
logger.appendKeys({
4952
"nhsd-correlation-id": event.headers["nhsd-correlation-id"],
@@ -152,12 +155,26 @@ const lambdaHandler = async (event: APIGatewayProxyEvent): Promise<APIGatewayPro
152155

153156
// This prescription was handled successfully,
154157
// so add a message to the notifications SQS
158+
let enableNotifications: boolean = false
155159
try {
156-
const requestId = event.headers["x-request-id"] ?? "x-request-id-not-found"
157-
await pushPrescriptionToNotificationSQS(requestId, dataItems, logger)
158-
} catch (err) {
159-
logger.error("Failed to push prescriptions to the notifications SQS", {err})
160-
// DO NOT throw an error here, since we want to still return the update!
160+
if (ENABLE_NOTIFICATIONS_PARAM) enableNotifications = await getParameter(ENABLE_NOTIFICATIONS_PARAM) === "true"
161+
} catch {
162+
logger.error("Failed to fetch ENABLE_NOTIFICATIONS_PARAM. Defaulting to false.", {ENABLE_NOTIFICATIONS_PARAM})
163+
}
164+
165+
if (enableNotifications) {
166+
try {
167+
const requestId = event.headers["x-request-id"] ?? "x-request-id-not-found"
168+
await pushPrescriptionToNotificationSQS(requestId, dataItems, logger)
169+
} catch (err) {
170+
logger.error("Failed to push prescriptions to the notifications SQS", {err})
171+
// DO NOT throw an error here, since we want to still return the update!
172+
}
173+
} else {
174+
logger.info(
175+
"enableNotifications is not true, skipping the notification request.",
176+
{enableNotifications}
177+
)
161178
}
162179

163180
return response(201, responseEntries)

packages/updatePrescriptionStatus/tests/testHandler.test.ts

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import {
1717
generateExpectedItems,
1818
generateMockEvent,
1919
mockDynamoDBClient,
20-
mockSQSClient,
2120
TASK_VALUES
2221
} from "./utils/testUtils"
2322

@@ -38,7 +37,21 @@ import {
3837
import {QueryCommand, TransactionCanceledException, TransactWriteItemsCommand} from "@aws-sdk/client-dynamodb"
3938

4039
const {mockSend: dynamoDBMockSend} = mockDynamoDBClient()
41-
const {mockSend: sqsMockSend} = mockSQSClient()
40+
41+
const mockPushPrescriptionToNotificationSQS = jest.fn().mockImplementation(async () => Promise.resolve())
42+
jest.unstable_mockModule("../src/utils/sqsClient", async () => ({
43+
__esModule: true,
44+
pushPrescriptionToNotificationSQS: mockPushPrescriptionToNotificationSQS
45+
}))
46+
47+
export const mockGetParameter = jest.fn(async () => Promise.resolve("false"))
48+
jest.unstable_mockModule(
49+
"@aws-lambda-powertools/parameters/ssm",
50+
async () => ({
51+
__esModule: true,
52+
getParameter: mockGetParameter
53+
})
54+
)
4255

4356
const {handler, logger} = await import("../src/updatePrescriptionStatus")
4457

@@ -416,7 +429,8 @@ describe("Integration tests for updatePrescriptionStatus handler", () => {
416429
})
417430

418431
it("when the notification SQS push fails, the response still succeeds", async () => {
419-
sqsMockSend.mockImplementation(
432+
mockGetParameter.mockImplementation(async () => Promise.resolve("true"))
433+
mockPushPrescriptionToNotificationSQS.mockImplementation(
420434
async () => {
421435
throw new Error("Test error")
422436
}
@@ -425,13 +439,12 @@ describe("Integration tests for updatePrescriptionStatus handler", () => {
425439
const event: APIGatewayProxyEvent = generateMockEvent(requestDispatched)
426440
const response: APIGatewayProxyResult = await handler(event, {})
427441
expect(response.statusCode).toBe(201)
442+
expect(mockPushPrescriptionToNotificationSQS).toHaveBeenCalled()
428443
})
429444

430-
it("when SQS environment variables are not set, the response still succeeds", async () => {
431-
process.env.NHS_NOTIFY_PRESCRIPTIONS_SQS_QUEUE_URL = undefined
432-
process.env.AWS_REGION = undefined
433-
434-
sqsMockSend.mockImplementation(
445+
it("when SQS push throws an error, the response still succeeds", async () => {
446+
mockGetParameter.mockImplementation(async () => Promise.resolve("true"))
447+
mockPushPrescriptionToNotificationSQS.mockImplementation(
435448
async () => {
436449
throw new Error("Test error")
437450
}
@@ -440,5 +453,33 @@ describe("Integration tests for updatePrescriptionStatus handler", () => {
440453
const event: APIGatewayProxyEvent = generateMockEvent(requestDispatched)
441454
const response: APIGatewayProxyResult = await handler(event, {})
442455
expect(response.statusCode).toBe(201)
456+
expect(mockPushPrescriptionToNotificationSQS).toHaveBeenCalled()
457+
})
458+
459+
it("When the get parameter call throws an error, the request succeeds and the sqs queue is untouched", async () => {
460+
mockGetParameter.mockImplementation(async () => Promise.reject(new Error("Failed")))
461+
462+
const rejected_event: APIGatewayProxyEvent = generateMockEvent(requestDispatched)
463+
const rejected_response: APIGatewayProxyResult = await handler(rejected_event, {})
464+
expect(rejected_response.statusCode).toBe(201)
465+
expect(mockPushPrescriptionToNotificationSQS).not.toHaveBeenCalled()
466+
})
467+
468+
it("When the enable notifications parameter is false, the push to SQS is skipped", async () => {
469+
mockGetParameter.mockImplementation(async () => Promise.resolve("false"))
470+
471+
const bypass_event: APIGatewayProxyEvent = generateMockEvent(requestDispatched)
472+
const bypass_response: APIGatewayProxyResult = await handler(bypass_event, {})
473+
expect(bypass_response.statusCode).toBe(201)
474+
expect(mockPushPrescriptionToNotificationSQS).not.toHaveBeenCalled()
475+
})
476+
477+
it("When the enable notifications parameter is true, the push to SQS is done", async () => {
478+
mockGetParameter.mockImplementation(async () => Promise.resolve("true"))
479+
480+
const successful_event: APIGatewayProxyEvent = generateMockEvent(requestDispatched)
481+
const successful_response: APIGatewayProxyResult = await handler(successful_event, {})
482+
expect(successful_response.statusCode).toBe(201)
483+
expect(mockPushPrescriptionToNotificationSQS).toHaveBeenCalled()
443484
})
444485
})

0 commit comments

Comments
 (0)