VED-982: Create POST MNS Notification by Akol125 · Pull Request #1211 · NHSDigital/immunisation-fhir-api

Akol125 · 2026-02-16T11:58:07Z

Summary

Routine Change
Summary
Implemented MNS integration to publish real-time CloudEvents notifications for immunisation record changes (create/update/delete).

Key Components

Lambda Handler
Processes SQS events from DynamoDB streams
Publishes to MNS with partial batch failure handling
Structured logging with trace IDs (message, immunisation, notification)
Notification Creation
Builds CloudEvents-compliant payloads from DynamoDB stream data
Fetches GP ODS codes from PDS
Calculates patient age at vaccination
Populates filtering attributes (organisation, application, vaccine type, action)
Utilities
Recursive extraction from nested DynamoDB events with type descriptor unwrapping
MNS service class for API interactions (subscribe, publish, delete)
Enum-based field mappings for type safety
Testing
Unit test coverage using real SQS payloads
Mocked external dependencies (PDS, MNS)
Comprehensive edge case and error handling tests

Add any other relevant notes or explanations here. Remove this line if you have nothing to add.

Reviews Required

Dev
Test
Tech Author
Product Owner

Review Checklist

ℹ️ This section is to be filled in by the reviewer.

I have reviewed the changes in this PR and they fill all of the acceptance criteria of the ticket.
If there were infrastructure, operational, or build changes, I have made sure there is sufficient evidence that the changes will work.
If there were changes that are outside of the regular release processes e.g. account infrastructure to setup, manual setup for external API integrations, secrets to set, then I have checked that the developer has flagged this to the Tech Lead as release steps.
I have checked that no Personal Identifiable Data (PID) is logged as part of the changes.

github-actions · 2026-02-16T11:58:23Z

This branch is working on a ticket in the NHS England VED JIRA Project. Here's a handy link to the ticket:

VED-982

…tifications' into VED-982-Notification-Lambda-MNS

lambdas/mns_publisher/src/create_notification.py

dlzhry2nhs

Thanks for resolving. I have resolved my existing comments. Just a couple of additional raised but getting there.

infrastructure/instance/environments/prod/green/variables.tfvars

infrastructure/instance/mns_publisher.tf

lambdas/shared/src/common/api_clients/mns_service.py

lambdas/mns_publisher/src/sqs_dynamo_utils.py

lambdas/mns_publisher/src/process_records.py

lambdas/mns_publisher/src/lambda_handler.py

lambdas/mns_publisher/src/process_records.py

dlzhry2nhs · 2026-02-24T16:07:10Z

lambdas/mns_publisher/src/process_records.py

+    Returns: List of failed item identifiers for partial batch failure
+    """
+    batch_item_failures = []
+    mns_service = get_mns_service(mns_env=mns_env)


This is performant enough because of the hacky cache used by the existing authenticator.

However, I really recommend looking at Matt's performance test PR and how we will manage auth going forward. Maybe we can try get that merged first, or use a similar approach on this branch.

With this current approach, we will end up initialised a new Authenticator and MNS Service each time. Instead we should have a long-lived class which allows us to make use of reused Lambda execution environments.

We could have this resolved in a seperate ticket

Yes, potentially or as a sub task. As long as we do not move the 8 pointer into testing until all those changes are ready. We will likely not be able to move it there for a while anyway as we have the dependency on MNS.

lambdas/mns_publisher/tests/test_lambda_handler.py

dlzhry2nhs · 2026-02-24T21:54:27Z

You have 1 Sonar issue. Only minor, will be a quick fix.

lambdas/mns_publisher/src/process_records.py

lambdas/mns_publisher/src/create_notification.py

dlzhry2nhs

Is not too far away now, before we can maybe say dev work is ready pending discussions and onboarding to MNS.

Testers can also pick up setting up automation tests in the meantime.

Think there are some old comments to do with the terraform which still need looking at. Then have raised some further code change requests in this review, along with a request to look at the tests.

lambdas/mns_publisher/src/process_records.py

lambdas/mns_publisher/src/sqs_dynamo_utils.py

lambdas/mns_publisher/src/create_notification.py

dlzhry2nhs · 2026-02-26T09:00:27Z

lambdas/mns_publisher/src/create_notification.py

+
+    patient_age = calculate_age_at_vaccination(imms_data["person_dob"], imms_data["date_and_time"])
+
+    gp_ods_code = get_practitioner_details_from_pds(imms_data["nhs_number"])


Not a comment for action yet, but let's keep this under review. I understand MNS may offer a service where they enrich the payload with further filtering information, so we may not even need to make this call. But let's see.

dlzhry2nhs · 2026-02-26T09:09:01Z

lambdas/mns_publisher/tests/test_create_notification.py

+class TestGetPractitionerDetailsFromPds(unittest.TestCase):
+    """Tests for get_practitioner_details_from_pds function."""
+
+    @patch("create_notification.pds_get_patient_details")


We are mocking very far away from the actual external services, and mocking a lot of our own helper functions.
If we change this helper function - pds_get_patient_details - and accidentally introduce a breaking change, we won't know about it (unless we have an e2e test), as we will have mocked the output.

Again, I would highly recommend having some tests that mock at the Python requests level. The responses library I shared earlier might be good for this, but fine if you feel more comfortable setting up manual mocks.

Finally, I think it's okay to leave the unit tests as they are here for create_notification, but I would advocate having some higher-level i.e. integration tests in test_lambda_handler otherwise we just have fairly brittle unit tests that do not guard very well against regressions.

dlzhry2nhs

Also you have a new failure, please resolve and test. The test would be that everything in your Lambda should succeed up to publishing the message to MNS. We expect this to fail, as they do not accept this event type. We should see all failed messages delivered to the Dead Letter Queue.

dlzhry2nhs

These are my last comments, I promise. Then we can resolve and open for the new team to review. I'm happy with the code changes, just a couple of minor additional pointers on the tests.

dlzhry2nhs · 2026-02-26T23:14:21Z

lambdas/mns_publisher/tests/test_create_notification.py

+
+    @patch("create_notification.get_practitioner_details_from_pds")
+    @patch("create_notification.get_service_url")
+    def test_create_mns_notification_uuid_generated(self, mock_get_service_url, mock_get_gp):


Not entirely sure this test is required.

dlzhry2nhs · 2026-02-26T23:15:19Z

lambdas/mns_publisher/tests/test_create_notification.py

+        self.assertEqual(result, [{"S": "item1"}, {"S": "item2"}])
+
+
+if __name__ == "__main__":


Not required.

dlzhry2nhs · 2026-02-26T23:17:16Z

lambdas/mns_publisher/tests/test_create_notification.py

+    @patch("create_notification.get_practitioner_details_from_pds")
+    @patch("create_notification.get_service_url")
+    def test_create_mns_notification_dataref_format_real_payload(self, mock_get_service_url, mock_get_gp):
+        mock_get_service_url.return_value = self.expected_immunisation_url
+        mock_get_gp.return_value = self.expected_gp_ods_code
+
+        result = create_mns_notification(self.sample_sqs_event)
+
+        expected_dataref = f"{self.expected_immunisation_url}/Immunization/d058014c-b0fd-4471-8db9-3316175eb825"
+        self.assertEqual(result["dataref"], expected_dataref)
+
+    @patch("create_notification.get_practitioner_details_from_pds")
+    @patch("create_notification.get_service_url")
+    def test_create_mns_notification_filtering_fields_real_payload(self, mock_get_service_url, mock_get_gp):
+        mock_get_service_url.return_value = self.expected_immunisation_url
+        mock_get_gp.return_value = self.expected_gp_ods_code
+
+        result = create_mns_notification(self.sample_sqs_event)
+
+        filtering = result["filtering"]
+        self.assertEqual(filtering["generalpractitioner"], self.expected_gp_ods_code)
+        self.assertEqual(filtering["sourceorganisation"], "B0C4P")
+        self.assertEqual(filtering["sourceapplication"], "TPP")
+        self.assertEqual(filtering["immunisationtype"], "hib")
+        self.assertEqual(filtering["action"], "CREATE")
+        self.assertIsInstance(filtering["subjectage"], str)
+
+    @patch("create_notification.get_practitioner_details_from_pds")
+    @patch("create_notification.get_service_url")
+    def test_create_mns_notification_age_calculation_real_payload(self, mock_get_service_url, mock_get_gp):
+        mock_get_service_url.return_value = self.expected_immunisation_url
+        mock_get_gp.return_value = self.expected_gp_ods_code
+
+        result = create_mns_notification(self.sample_sqs_event)
+
+        self.assertEqual(result["filtering"]["subjectage"], "21")
+
+    @patch("create_notification.get_practitioner_details_from_pds")
+    @patch("create_notification.get_service_url")
+    def test_create_mns_notification_calls_get_practitioner_real_payload(self, mock_get_service_url, mock_get_gp):
+        mock_get_service_url.return_value = self.expected_immunisation_url
+        mock_get_gp.return_value = self.expected_gp_ods_code
+
+        create_mns_notification(self.sample_sqs_event)
+
+        mock_get_gp.assert_called_once_with("9481152782")


All of these have the same setup and are testing the same thing. It's just that we seem to be splitting the assertions across different tests. This is an inefficient pattern.

We should do one test and then assert the expected mocks were called and assert against the full payload. We could even construct an expected payload and assertDictEquals. But am equally fine with us splitting the assertions into different sections. But we just need 1 test.

dlzhry2nhs · 2026-02-26T23:17:37Z

lambdas/mns_publisher/tests/test_sqs_dynamo_utils.py

No longer needed?

dlzhry2nhs · 2026-02-26T23:25:01Z

lambdas/mns_publisher/tests/sample_data/sqs_event.json

Worth putting in a test_data or sample_data directory. We appear to use that pattern in other Lambda test directories.

dlzhry2nhs · 2026-02-26T23:29:14Z

lambdas/mns_publisher/tests/test_lambda_handler.py

+        mock_get_mns.assert_called_once()
+
+
+class TestLambdaHandler(unittest.TestCase):


Think you are already on with this. I'm a bit tied up in the morning, but will keep the afternoon free to pair if help needed.

If we set up some nice integration level tests using moto for secrets, responses for external HTTP calls, asserting against everything including logs, and demonstrating the scenarios mentioned in the ticket ACs, I think we could potentially just have TestLambdaHandler and everything below it will get tested.

The unit tests that exist for test_create_notification provide us good unit test coverage already :)

lambdas/mns_publisher/tests/test_lambda_handler.py

sonarqubecloud · 2026-03-03T10:37:26Z

Quality Gate passed

Issues
11 New issues
0 Accepted issues

Measures
0 Security Hotspots
98.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

build base schema

01fa03c

Akol125 temporarily deployed to dev February 16, 2026 11:58 — with GitHub Actions Inactive

Akol125 had a problem deploying to dev February 16, 2026 11:58 — with GitHub Actions Failure

Akol125 changed the title ~~build base schema~~ VED-982: Create MNS Notification Feb 16, 2026

Akol125 marked this pull request as draft February 17, 2026 09:13

Akol125 added 2 commits February 17, 2026 22:10

setup base_schema from vaccs data

953d861

make pds call and move pds details to shared folder

32330cc

Akol125 changed the base branch from staging/VED-16-mns-vacc-event-notifications to master February 18, 2026 11:35

Akol125 changed the base branch from master to staging/VED-16-mns-vacc-event-notifications February 18, 2026 11:35

Akol125 marked this pull request as ready for review February 18, 2026 15:00

fetch details from pds and add tests

dcf31ef

Akol125 had a problem deploying to dev February 18, 2026 21:24 — with GitHub Actions Failure

Akol125 changed the base branch from staging/VED-16-mns-vacc-event-notifications to master February 18, 2026 21:26

Akol125 changed the base branch from master to staging/VED-921-New-Vacc-Types February 18, 2026 21:27

Akol125 changed the base branch from staging/VED-921-New-Vacc-Types to staging/VED-16-mns-vacc-event-notifications February 18, 2026 21:27

Merge remote-tracking branch 'origin/staging/VED-16-mns-vacc-event-no…

5de42d2

…tifications' into VED-982-Notification-Lambda-MNS

Akol125 had a problem deploying to dev February 18, 2026 21:37 — with GitHub Actions Failure

fix terraform and test issues

8337ff0

Akol125 temporarily deployed to dev February 18, 2026 22:14 — with GitHub Actions Inactive

Akol125 had a problem deploying to dev February 18, 2026 22:15 — with GitHub Actions Failure

add test for lambda

02945a1

Akol125 temporarily deployed to dev February 18, 2026 22:30 — with GitHub Actions Inactive

Akol125 temporarily deployed to dev February 18, 2026 22:31 — with GitHub Actions Inactive

NHSDigital deleted a comment from sonarqubecloud bot Feb 18, 2026

Akol125 temporarily deployed to internal-dev February 18, 2026 22:35 — with GitHub Actions Inactive

Akol125 had a problem deploying to internal-dev-sandbox February 18, 2026 22:35 — with GitHub Actions Failure

Akol125 temporarily deployed to internal-dev February 18, 2026 22:35 — with GitHub Actions Inactive

Akol125 temporarily deployed to internal-dev February 18, 2026 22:45 — with GitHub Actions Inactive

Akol125 had a problem deploying to internal-dev-sandbox February 18, 2026 22:50 — with GitHub Actions Failure

dlzhry2nhs reviewed Feb 23, 2026

View reviewed changes

lambdas/mns_publisher/src/create_notification.py Outdated Show resolved Hide resolved

Akol125 added 6 commits February 23, 2026 14:51

add gp identifier

565a765

make payload dynamic

9a58846

add typedicts, remove recursion

9cc38bd

refactor lambda and add env vars

ecdf384

fix sonar and terraform issues

afe7bc9

remove pdsSync found in id_sync

0297ee5

dlzhry2nhs reviewed Feb 24, 2026

View reviewed changes

resolve type hinting and remove try catch

a96582a

dlzhry2nhs reviewed Feb 24, 2026

View reviewed changes

lambdas/mns_publisher/src/process_records.py Outdated Show resolved Hide resolved

dlzhry2nhs reviewed Feb 24, 2026

View reviewed changes

lambdas/mns_publisher/src/create_notification.py Show resolved Hide resolved

Akol125 added 7 commits February 25, 2026 09:37

add secrets for tf and resolve sonar issues

a925afd

secret configurations

1ef1124

fix e2e as base isn't master

96a4a13

reflect new permission in e2e test

a8c3782

add conditional for patient_detials

1c92884

remove test for attribute error

22fb4ed

check logs to pinpoint pds bad request

f13b423

dlzhry2nhs reviewed Feb 26, 2026

View reviewed changes

remove dynamo utils and add tests

023942a

dlzhry2nhs reviewed Feb 27, 2026

View reviewed changes

Akol125 added 2 commits February 27, 2026 17:13

fail fast with no nhs_no and lambda int test

9305138

integration test

7ab3bb3

dlzhry2nhs reviewed Mar 3, 2026

View reviewed changes

lambdas/mns_publisher/tests/test_lambda_handler.py Outdated Show resolved Hide resolved

dlzhry2nhs reviewed Mar 3, 2026

View reviewed changes

lambdas/mns_publisher/tests/test_lambda_handler.py Outdated Show resolved Hide resolved

dlzhry2nhs reviewed Mar 3, 2026

View reviewed changes

lambdas/mns_publisher/tests/test_lambda_handler.py Outdated Show resolved Hide resolved

change subject age to integer in test

8e22a92


		patient_age = calculate_age_at_vaccination(imms_data["person_dob"], imms_data["date_and_time"])

		gp_ods_code = get_practitioner_details_from_pds(imms_data["nhs_number"])

		self.assertEqual(result, [{"S": "item1"}, {"S": "item2"}])


		if __name__ == "__main__":

		mock_get_mns.assert_called_once()


		class TestLambdaHandler(unittest.TestCase):

Conversation

Akol125 commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Reviews Required

Review Checklist

Uh oh!

github-actions bot commented Feb 16, 2026

Uh oh!

Uh oh!

dlzhry2nhs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dlzhry2nhs commented Feb 24, 2026

Uh oh!

Uh oh!

Uh oh!

dlzhry2nhs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dlzhry2nhs left a comment

Choose a reason for hiding this comment

Uh oh!

dlzhry2nhs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Mar 3, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Akol125 commented Feb 16, 2026 •

edited

Loading