CCM-12685: Validate routing config submit #806

ClareJonesBJSS · 2026-01-27T17:01:20Z

Description

Retrieve routing config before submit (implicitly validating name etc via zod parsing)
Explicitly check for null defaultTemplateIds
Add missing/deleted/unproofed template check to ddb txn

Removes the option of null language/accessibility templateIds, as this was not actually being leveraged and getting rid simplifies both this validation and core events

Context

Type of changes

Refactoring (non-breaking change)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would change existing functionality)
Bug fix (non-breaking change which fixes an issue)

Checklist

I am familiar with the contributing guidelines
I have followed the code style of the project
I have added tests to cover my changes
I have updated the documentation accordingly
This PR is a result of pair or mob programming
If I have used the 'skip-trivy-package' label I have done so responsibly and in the knowledge that this is being fixed as part of a separate ticket/PR.

Sensitive Information Declaration

To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.

I confirm that neither PII/PID nor sensitive data are included in this PR and the codebase changes.

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

chris-elliott-nhsd · 2026-01-28T10:59:29Z

tests/test-team/template-mgmt-event-tests/routing-config.event.spec.ts

+      'Test Template for Submit'
+    );
+
+    await templateStorageHelper.seedTemplateData([template]);


do we want to do a cleanup of this template data at the end? we're doing that in other places in the test suite

lambdas/backend-api/src/__tests__/infra/routing-config-repository/repository.test.ts

lambdas/backend-api/src/infra/routing-config-repository/repository.ts

nicki-nhs · 2026-01-28T14:49:02Z

lambdas/backend-api/src/infra/routing-config-repository/repository.ts

+    if (currentLockNumber !== lockNumber) {
+      return failure(
+        ErrorCase.CONFLICT,
+        'Lock number mismatch - Message Plan has been modified since last read'


Suggested change

'Lock number mismatch - Message Plan has been modified since last read'

'Lock number mismatch - Routing configuration has been modified since last read'

nicki-nhs · 2026-01-28T14:51:18Z

lambdas/backend-api/src/infra/routing-config-repository/repository.ts

+    }
+
+    const submittableValidationError =
+      this.parseSubmittableRoutingConfig(cascade);


I feel like it might be clearer to do this inline rather than extracting into this other function?
and also, the 400 it returns if its invalid will clash with the 400 for already submitted, does that matter?

nicki-nhs · 2026-01-28T14:54:05Z

lambdas/backend-api/src/infra/routing-config-repository/repository.ts

+              Update: update,
+            },
+            // Template existence check + For LETTER templates, check they have PROOF_APPROVED or SUBMITTED status
+            // Also exclude DELETED templates for all template types


should we also be validating that the templates belong to the same client? or, given we do that when we add them to the message plan, are we assuming you couldnt have got in that state?

The condition check is doing this - both the update and the condition check use the same user.clientId as the owner partition key.

lambdas/backend-api/src/infra/routing-config-repository/repository.ts

nicki-nhs · 2026-01-28T14:58:57Z

lambdas/backend-api/src/infra/routing-config-repository/repository.ts

+            ('DELETED' satisfies RoutingConfigStatus)
+        ) {
+          missingTemplateIds.push(templateId);
+        } else {


is it not possible that you get ConditionalCheckFailed errors for other template types?
or letters that failed for reasons other than not being in submitted/proof approved?

I don't think so? In the case where the template ID doesn't exist or isn't owned by the client, !reason.Item will apply (and I feel treating ownership mismatch as not found is correct), and DELETED and invalid LETTER statuses are the only other conditions in the statement.

nicki-nhs · 2026-01-28T15:04:15Z

lambdas/backend-api/src/__tests__/infra/routing-config-repository/repository.test.ts

+          actualError: err,
+          errorMeta: {
+            code: 500,
+            description: 'Error retrieving Routing Config',


did you not update these error messages to be routing configuration?

Oh fair spot, it was some 404s that I renamed as we had two different wordings and it was bugging me, so I went with the switch that touched the fewest lines, might as well do it here too

nicki-nhs · 2026-01-28T15:05:25Z

lambdas/backend-api/src/__tests__/infra/routing-config-repository/repository.test.ts

+      });
+    });
+
+    test('returns 404 failure if transaction fails because routing config does not exist', async () => {


from the two descriptions, its not massively clear to me how this test and the one above are different , do they maybe need rewording, if we need them both?

I think this covering is the logically possible but massively unlikely scenario where the routing config item is deleted from DynamoDB between sending the GetItem command and sending the TransactWrite command. I think the test is valuable but I agree the descriptions could be a little clearer - maybe include the command names in them?

nicki-nhs · 2026-01-28T15:07:29Z

lambdas/backend-api/src/__tests__/infra/routing-config-repository/repository.test.ts

+      expect(mocks.dynamo).not.toHaveReceivedCommand(TransactWriteCommand);
+    });
+
+    test('returns validation failure if defaultTemplateId is null with no conditionalTemplates', async () => {


I think similar here - I feel like these two test descriptions sound the same?

nicki-nhs · 2026-01-28T15:12:57Z

lambdas/backend-client/src/__tests__/schemas/routing-config.test.ts

+    });
+    expect(res.success).toBe(false);
+  });
+});


what about test for invalid with conditional template with a null template ID?

harrim91 · 2026-01-29T10:11:31Z

lambdas/backend-api/src/__tests__/infra/routing-config-repository/repository.test.ts

+      });
+    });
+
+    test('returns 404 failure if transaction fails because routing config does not exist', async () => {


I think this covering is the logically possible but massively unlikely scenario where the routing config item is deleted from DynamoDB between sending the GetItem command and sending the TransactWrite command. I think the test is valuable but I agree the descriptions could be a little clearer - maybe include the command names in them?

harrim91 · 2026-01-29T10:21:22Z

lambdas/backend-api/src/infra/routing-config-repository/repository.ts

+              Update: update,
+            },
+            // Template existence check + For LETTER templates, check they have PROOF_APPROVED or SUBMITTED status
+            // Also exclude DELETED templates for all template types


The condition check is doing this - both the update and the condition check use the same user.clientId as the owner partition key.

lambdas/backend-api/src/infra/routing-config-repository/repository.ts

…CCM-12685_validate-submitted-config

ClareJonesBJSS requested review from a team as code owners January 27, 2026 17:01

ClareJonesBJSS force-pushed the feature/CCM-12685_validate-submitted-config branch 6 times, most recently from 8bab989 to b2f4edc Compare January 28, 2026 09:47

alexnuttall reviewed Jan 28, 2026

View reviewed changes

infrastructure/terraform/modules/backend-api/module_submit_routing_config_lambda.tf Show resolved Hide resolved

chris-elliott-nhsd reviewed Jan 28, 2026

View reviewed changes

alexnuttall reviewed Jan 28, 2026

View reviewed changes

lambdas/backend-api/src/__tests__/infra/routing-config-repository/repository.test.ts Outdated Show resolved Hide resolved

chris-elliott-nhsd previously approved these changes Jan 28, 2026

View reviewed changes

alexnuttall reviewed Jan 28, 2026

View reviewed changes

lambdas/backend-api/src/infra/routing-config-repository/repository.ts Show resolved Hide resolved

alexnuttall reviewed Jan 28, 2026

View reviewed changes

lambdas/backend-api/src/infra/routing-config-repository/repository.ts Outdated Show resolved Hide resolved

alexnuttall previously approved these changes Jan 28, 2026

View reviewed changes

ClareJonesBJSS dismissed stale reviews from alexnuttall and chris-elliott-nhsd via d2f1081 January 28, 2026 12:07

alexnuttall previously approved these changes Jan 28, 2026

View reviewed changes

nicki-nhs reviewed Jan 28, 2026

View reviewed changes

lambdas/backend-api/src/infra/routing-config-repository/repository.ts Show resolved Hide resolved

nicki-nhs reviewed Jan 28, 2026

View reviewed changes

harrim91 reviewed Jan 29, 2026

View reviewed changes

ClareJonesBJSS added 2 commits February 2, 2026 15:46

CCM-12685: Validate routing config submit

0c16557

Review comments

7bc828f

ClareJonesBJSS dismissed alexnuttall’s stale review via 5bd3af2 February 2, 2026 15:51

ClareJonesBJSS force-pushed the feature/CCM-12685_validate-submitted-config branch from d2f1081 to 5bd3af2 Compare February 2, 2026 15:51

Code review

f1ae2d3

ClareJonesBJSS force-pushed the feature/CCM-12685_validate-submitted-config branch from 5bd3af2 to f1ae2d3 Compare February 2, 2026 17:11

ClareJonesBJSS and others added 2 commits February 2, 2026 17:32

Merge branch 'main' into feature/CCM-12685_validate-submitted-config

6f4e34b

Merge commit 'dcec6b2e170cace2de2776a42624bb32f0841970' into feature/…

cf16126

…CCM-12685_validate-submitted-config

alexnuttall approved these changes Feb 3, 2026

View reviewed changes

	'Lock number mismatch - Message Plan has been modified since last read'
	'Lock number mismatch - Routing configuration has been modified since last read'

CCM-12685: Validate routing config submit #806

Are you sure you want to change the base?

CCM-12685: Validate routing config submit #806

Conversation

ClareJonesBJSS commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Context

Type of changes

Checklist

Sensitive Information Declaration

Uh oh!

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!

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ClareJonesBJSS commented Jan 27, 2026 •

edited

Loading