Conversation
…t behaves how I think it does
|
This PR is linked to a ticket in an NHS Digital JIRA Project. Here's a handy link to the ticket: AEA-6428 |
There was a problem hiding this comment.
Pull request overview
Updates the PSU pipeline to support identifying notification suppliers by application/product ID (via nhsd-application-id) instead of relying on application name, and propagates the new identifier through persisted data and filtering.
Changes:
- Add
ApplicationIDtoPSUDataItemand populate it from thenhsd-application-idrequest header. - Switch notification enablement filtering from
ApplicationNametoApplicationID. - Update API specification and unit tests to reflect the new header/field.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/updatePrescriptionStatus/src/updatePrescriptionStatus.ts | Reads nhsd-application-id header and persists it on generated data items. |
| packages/updatePrescriptionStatus/src/validation/notificationSiteAndSystemFilters.ts | Changes supplier filtering to use ApplicationID instead of ApplicationName. |
| packages/common/commonTypes/src/index.ts | Adds ApplicationID to the shared PSUDataItem type. |
| packages/specification/eps-prescription-status-update-api.yaml | Enables product ID checking and declares nhsd-application-id as a target attribute/header. |
| packages/updatePrescriptionStatus/tests/utils/testUtils.ts | Adds ApplicationID to mock data item helper. |
| packages/updatePrescriptionStatus/tests/testUpdatePrescriptionStatus.test.ts | Updates buildDataItems calls for the new signature. |
| packages/updatePrescriptionStatus/tests/testSqsClient.test.ts | Updates mock data to include ApplicationID where needed. |
| packages/updatePrescriptionStatus/tests/testDatabaseClient.test.ts | Updates persisted item fixtures to include ApplicationID. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…n id and name checkign
| release_qa: | ||
| needs: [tag_release, release_dev, package_code, get_commit_id, get_config_values] | ||
| needs: | ||
| [tag_release, release_dev, package_code, get_commit_id, get_config_values] |
There was a problem hiding this comment.
gratuitious whitespace change?
|




Summary
Details
We need to update the code to filter notification suppliers based on product ID rather than by application name (which may change without us knowing).
However, there needs to be a switchover period. I will implement the logic for the new way in parallel to the old, and have an environment variable
USE_PRODUCT_ID_FOR_NOTIFICATIONS_FILTERINGthat toggles from one to the other. The enabled product IDs will be a new parameter also.The order of operations will be:
EnabledSystemsParameter, and the environment variableUSE_PRODUCT_ID_FOR_NOTIFICATIONS_FILTERING.