diff --git a/frontend/src/__tests__/utils/routing-utils.test.ts b/frontend/src/__tests__/utils/routing-utils.test.ts index 84413decb..696ef20ea 100644 --- a/frontend/src/__tests__/utils/routing-utils.test.ts +++ b/frontend/src/__tests__/utils/routing-utils.test.ts @@ -176,27 +176,6 @@ describe('getSelectedLanguageTemplateIds', () => { ]); }); - it('should filter out language templates with null templateId', () => { - const cascadeItem: CascadeItem = { - cascadeGroups: ['translations'], - channel: 'LETTER', - channelType: 'primary', - defaultTemplateId: 'default-template', - conditionalTemplates: [ - { templateId: 'template-1', language: 'fr' }, - { templateId: null, language: 'pl' }, - { templateId: 'template-3', language: 'es' }, - ], - }; - - const result = getSelectedLanguageTemplateIds(cascadeItem); - - expect(result).toEqual([ - { language: 'fr', templateId: 'template-1' }, - { language: 'es', templateId: 'template-3' }, - ]); - }); - it('should return empty array when no conditional templates exist', () => { const cascadeItem: CascadeItem = { cascadeGroups: ['standard'], @@ -287,20 +266,6 @@ describe('removeTemplatesFromConditionalTemplates', () => { expect(result).toEqual([{ templateId: 'template-2', language: 'es' }]); }); - it('should keep templates with null templateId', () => { - const conditionalTemplates: ConditionalTemplate[] = [ - { templateId: 'template-1', language: 'fr' }, - { templateId: null, language: 'es' }, - ]; - - const result = removeTemplatesFromConditionalTemplates( - conditionalTemplates, - ['template-1'] - ); - - expect(result).toEqual([{ templateId: null, language: 'es' }]); - }); - it('should return empty array when all templates are removed', () => { const conditionalTemplates: ConditionalTemplate[] = [ { templateId: 'template-1', language: 'fr' }, @@ -512,26 +477,6 @@ describe('getConditionalTemplatesForItem', () => { }); }); - it('should filter out templates with a missing/invalid templateId', () => { - const cascadeItem: CascadeItem = { - cascadeGroups: ['standard', 'translations'], - channel: 'LETTER', - channelType: 'primary', - defaultTemplateId: 'template-1', - conditionalTemplates: [ - { templateId: 'template-2', language: 'fr' }, - { templateId: null, language: 'es' }, - { accessibleFormat: 'x1' } as ConditionalTemplate, - ], - }; - - const result = getConditionalTemplatesForItem(cascadeItem, templates); - - expect(result).toEqual({ - 'template-2': templates['template-2'], - }); - }); - it('should not include templates that are missing from templates object', () => { const cascadeItem: CascadeItem = { cascadeGroups: ['standard', 'translations'], @@ -635,24 +580,6 @@ describe('buildCascadeGroupsForItem', () => { ]); }); - it('should return only standard group when conditional templates have missing templateIds', () => { - const cascadeItem: CascadeItem = { - cascadeGroups: ['standard'], - channel: 'LETTER', - channelType: 'primary', - defaultTemplateId: 'template-1', - conditionalTemplates: [ - { templateId: null, accessibleFormat: 'q4' }, - { templateId: 'template-2', language: 'fr' }, - ], - }; - - expect(buildCascadeGroupsForItem(cascadeItem)).toEqual([ - 'standard', - 'translations', - ]); - }); - it('should return only standard when conditional templates array is empty', () => { const cascadeItem: CascadeItem = { cascadeGroups: ['standard'], diff --git a/frontend/src/utils/routing-utils.ts b/frontend/src/utils/routing-utils.ts index a3efb13ac..f21a0deca 100644 --- a/frontend/src/utils/routing-utils.ts +++ b/frontend/src/utils/routing-utils.ts @@ -51,7 +51,7 @@ export function getConditionalTemplatesForItem( return Object.fromEntries( conditionalTemplateIds - .filter((id): id is string => id != null && id in templates) + .filter((id) => id in templates) .map((id) => [id, templates[id]]) ); } @@ -90,10 +90,8 @@ export function getSelectedLanguageTemplateIds( return cascadeItem.conditionalTemplates .filter( - ( - template - ): template is ConditionalTemplateLanguage & { templateId: string } => - 'language' in template && template.templateId !== null + (template): template is ConditionalTemplateLanguage => + 'language' in template ) .map(({ language, templateId }) => ({ language, @@ -137,10 +135,10 @@ export function buildCascadeGroupsForItem( cascadeItem.conditionalTemplates.length > 0 ) { const hasAccessibleFormat = cascadeItem.conditionalTemplates.some( - (template) => 'accessibleFormat' in template && template.templateId + (template) => 'accessibleFormat' in template ); const hasLanguage = cascadeItem.conditionalTemplates.some( - (template) => 'language' in template && template.templateId + (template) => 'language' in template ); if (hasAccessibleFormat) { @@ -190,7 +188,6 @@ export function removeTemplatesFromCascadeItem( return updatedCascadeItem; } - /** * Add default template to cascade at specific index */ diff --git a/infrastructure/terraform/modules/backend-api/module_submit_routing_config_lambda.tf b/infrastructure/terraform/modules/backend-api/module_submit_routing_config_lambda.tf index f3b5f1ec4..490ad0e96 100644 --- a/infrastructure/terraform/modules/backend-api/module_submit_routing_config_lambda.tf +++ b/infrastructure/terraform/modules/backend-api/module_submit_routing_config_lambda.tf @@ -41,6 +41,7 @@ data "aws_iam_policy_document" "submit_routing_config_lambda_policy" { effect = "Allow" actions = [ + "dynamodb:GetItem", "dynamodb:UpdateItem", ] @@ -49,6 +50,21 @@ data "aws_iam_policy_document" "submit_routing_config_lambda_policy" { ] } + statement { + sid = "AllowConditionCheckDynamoAccess" + effect = "Allow" + + actions = [ + "dynamodb:BatchGetItem", + "dynamodb:UpdateItem", + "dynamodb:ConditionCheckItem", + ] + + resources = [ + aws_dynamodb_table.templates.arn, + ] + } + statement { sid = "AllowKMSAccess" effect = "Allow" diff --git a/infrastructure/terraform/modules/backend-api/spec.tmpl.json b/infrastructure/terraform/modules/backend-api/spec.tmpl.json index 504c2b3ac..42d17a46c 100644 --- a/infrastructure/terraform/modules/backend-api/spec.tmpl.json +++ b/infrastructure/terraform/modules/backend-api/spec.tmpl.json @@ -385,7 +385,6 @@ }, "templateId": { "format": "uuid", - "nullable": true, "type": "string" } }, @@ -409,7 +408,6 @@ }, "templateId": { "format": "uuid", - "nullable": true, "type": "string" } }, diff --git a/lambdas/backend-api/src/__tests__/api/delete-routing-config.test.ts b/lambdas/backend-api/src/__tests__/api/delete-routing-config.test.ts index 43141ba18..aeb3b8e42 100644 --- a/lambdas/backend-api/src/__tests__/api/delete-routing-config.test.ts +++ b/lambdas/backend-api/src/__tests__/api/delete-routing-config.test.ts @@ -169,7 +169,7 @@ describe('Delete Routing Config Handler', () => { errorMeta: { code: 409, description: - 'Lock number mismatch - Message Plan has been modified since last read', + 'Lock number mismatch - Routing configuration has been modified since last read', }, }, }); @@ -192,7 +192,7 @@ describe('Delete Routing Config Handler', () => { body: JSON.stringify({ statusCode: 409, technicalMessage: - 'Lock number mismatch - Message Plan has been modified since last read', + 'Lock number mismatch - Routing configuration has been modified since last read', }), }); diff --git a/lambdas/backend-api/src/__tests__/api/submit-routing-config.test.ts b/lambdas/backend-api/src/__tests__/api/submit-routing-config.test.ts index 681255539..48d9abea6 100644 --- a/lambdas/backend-api/src/__tests__/api/submit-routing-config.test.ts +++ b/lambdas/backend-api/src/__tests__/api/submit-routing-config.test.ts @@ -173,7 +173,7 @@ describe('Submit Routing Config Handler', () => { errorMeta: { code: 409, description: - 'Lock number mismatch - Message Plan has been modified since last read', + 'Lock number mismatch - Routing configuration has been modified since last read', }, }, }); @@ -196,7 +196,7 @@ describe('Submit Routing Config Handler', () => { body: JSON.stringify({ statusCode: 409, technicalMessage: - 'Lock number mismatch - Message Plan has been modified since last read', + 'Lock number mismatch - Routing configuration has been modified since last read', }), }); diff --git a/lambdas/backend-api/src/__tests__/api/update-routing-config.test.ts b/lambdas/backend-api/src/__tests__/api/update-routing-config.test.ts index 99a38d6c8..ff4c44014 100644 --- a/lambdas/backend-api/src/__tests__/api/update-routing-config.test.ts +++ b/lambdas/backend-api/src/__tests__/api/update-routing-config.test.ts @@ -255,7 +255,7 @@ describe('Update Routing Config Handler', () => { errorMeta: { code: 409, description: - 'Lock number mismatch - Message Plan has been modified since last read', + 'Lock number mismatch - Routing configuration has been modified since last read', }, }, }); @@ -283,7 +283,7 @@ describe('Update Routing Config Handler', () => { body: JSON.stringify({ statusCode: 409, technicalMessage: - 'Lock number mismatch - Message Plan has been modified since last read', + 'Lock number mismatch - Routing configuration has been modified since last read', }), }); diff --git a/lambdas/backend-api/src/__tests__/app/routing-config-client.test.ts b/lambdas/backend-api/src/__tests__/app/routing-config-client.test.ts index 141ee19bb..f11478a74 100644 --- a/lambdas/backend-api/src/__tests__/app/routing-config-client.test.ts +++ b/lambdas/backend-api/src/__tests__/app/routing-config-client.test.ts @@ -102,7 +102,10 @@ describe('RoutingConfigClient', () => { expect(result).toEqual({ error: { - errorMeta: { code: 404, description: 'Routing Config not found' }, + errorMeta: { + code: 404, + description: 'Routing configuration not found', + }, }, }); @@ -516,6 +519,7 @@ describe('RoutingConfigClient', () => { const completed: RoutingConfig = { ...routingConfig, + id, status: 'COMPLETED', }; @@ -536,6 +540,36 @@ describe('RoutingConfigClient', () => { }); }); + test('returns failures from repository', async () => { + const { client, mocks } = setup(); + + mocks.clientConfigRepository.get.mockResolvedValueOnce({ + data: { features: { routing: true } }, + }); + + mocks.routingConfigRepository.submit.mockResolvedValueOnce({ + error: { + errorMeta: { + code: 400, + description: + 'All cascade items must have either a defaultTemplateId or conditionalTemplates', + }, + }, + }); + + const result = await client.submitRoutingConfig('some-id', user, '42'); + + expect(result).toEqual({ + error: { + errorMeta: { + code: 400, + description: + 'All cascade items must have either a defaultTemplateId or conditionalTemplates', + }, + }, + }); + }); + test('returns failures from client config repository', async () => { const { client, mocks } = setup(); diff --git a/lambdas/backend-api/src/__tests__/infra/routing-config-repository/repository.test.ts b/lambdas/backend-api/src/__tests__/infra/routing-config-repository/repository.test.ts index 43172a18f..73e99fa4b 100644 --- a/lambdas/backend-api/src/__tests__/infra/routing-config-repository/repository.test.ts +++ b/lambdas/backend-api/src/__tests__/infra/routing-config-repository/repository.test.ts @@ -1,4 +1,5 @@ import { + BatchGetCommand, DynamoDBDocumentClient, GetCommand, PutCommand, @@ -51,8 +52,29 @@ const TEMPLATE_TABLE_NAME = 'template-table-name'; const user = { internalUserId: 'user', clientId: 'nhs-notify-client-id' }; const clientOwnerKey = `CLIENT#${user.clientId}`; +const dynamo = mockClient(DynamoDBDocumentClient); + +// Helper to create valid template mocks that pass $TemplateDto validation +const makeTemplateMock = ( + overrides: { + id?: string; + templateType?: 'SMS' | 'EMAIL' | 'NHS_APP' | 'LETTER'; + templateStatus?: string; + lockNumber?: number; + } = {} +) => ({ + id: overrides.id ?? 'template-id', + name: 'Test Template', + templateType: overrides.templateType ?? 'SMS', + templateStatus: overrides.templateStatus ?? 'NOT_YET_SUBMITTED', + lockNumber: overrides.lockNumber ?? 1, + message: 'Test message content', + createdAt: date.toISOString(), + updatedAt: date.toISOString(), +}); + function setup() { - const dynamo = mockClient(DynamoDBDocumentClient); + dynamo.reset(); const mocks = { dynamo }; @@ -110,7 +132,10 @@ describe('RoutingConfigRepository', () => { expect(result).toEqual({ error: { - errorMeta: { code: 404, description: 'Routing Config not found' }, + errorMeta: { + code: 404, + description: 'Routing configuration not found', + }, }, }); @@ -249,54 +274,230 @@ describe('RoutingConfigRepository', () => { }); describe('submit', () => { - test('updates routing config to COMPLETED', async () => { + test('updates routing config to COMPLETED and templates to SUBMITTED', async () => { const { repo, mocks } = setup(); + const routingConfigWithLock: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + }; + const completed: RoutingConfig = { ...routingConfig, status: 'COMPLETED', + lockNumber: 3, }; - mocks.dynamo.on(UpdateCommand).resolves({ Attributes: completed }); + const template = makeTemplateMock({ + id: routingConfig.cascade[0].defaultTemplateId!, + templateType: 'SMS', + templateStatus: 'NOT_YET_SUBMITTED', + lockNumber: 1, + }); + + mocks.dynamo + .on(GetCommand) + .resolvesOnce({ Item: routingConfigWithLock }) + .resolvesOnce({ Item: completed }); + mocks.dynamo.on(BatchGetCommand).resolvesOnce({ + Responses: { + [TEMPLATE_TABLE_NAME]: [template], + }, + }); + mocks.dynamo.on(TransactWriteCommand).resolvesOnce({}); const result = await repo.submit(routingConfig.id, user, 2); expect(result).toEqual({ data: completed }); - expect(mocks.dynamo).toHaveReceivedCommandWith(UpdateCommand, { - ConditionExpression: - '#status = :condition_1_status AND #lockNumber = :condition_2_lockNumber', - ExpressionAttributeNames: { - '#status': 'status', - '#updatedAt': 'updatedAt', - '#updatedBy': 'updatedBy', - '#lockNumber': 'lockNumber', + expect(mocks.dynamo).toHaveReceivedCommandWith(BatchGetCommand, { + RequestItems: { + [TEMPLATE_TABLE_NAME]: { + Keys: [ + { + id: routingConfig.cascade[0].defaultTemplateId!, + owner: clientOwnerKey, + }, + ], + }, }, - ExpressionAttributeValues: { - ':condition_1_status': 'DRAFT', - ':condition_2_lockNumber': 2, - ':lockNumber': 1, - ':status': 'COMPLETED', - ':updatedAt': date.toISOString(), - ':updatedBy': `INTERNAL_USER#${user.internalUserId}`, + }); + + expect(mocks.dynamo).toHaveReceivedCommandWith(TransactWriteCommand, { + TransactItems: [ + { + Update: { + ConditionExpression: + '#status = :condition_1_status AND #lockNumber = :condition_2_lockNumber', + ExpressionAttributeNames: { + '#lockNumber': 'lockNumber', + '#status': 'status', + '#updatedAt': 'updatedAt', + '#updatedBy': 'updatedBy', + }, + ExpressionAttributeValues: { + ':condition_1_status': 'DRAFT', + ':condition_2_lockNumber': 2, + ':lockNumber': 1, + ':status': 'COMPLETED', + ':updatedAt': date.toISOString(), + ':updatedBy': `INTERNAL_USER#${user.internalUserId}`, + }, + Key: { + id: routingConfig.id, + owner: clientOwnerKey, + }, + ReturnValues: 'ALL_NEW', + ReturnValuesOnConditionCheckFailure: 'ALL_OLD', + TableName: 'routing-config-table-name', + UpdateExpression: + 'SET #status = :status, #updatedAt = :updatedAt, #updatedBy = :updatedBy ADD #lockNumber :lockNumber', + }, + }, + { + Update: { + ConditionExpression: + '#templateStatus = :condition_1_templateStatus AND (#lockNumber = :condition_2_1_lockNumber OR attribute_not_exists (#lockNumber))', + ExpressionAttributeNames: { + '#lockNumber': 'lockNumber', + '#templateStatus': 'templateStatus', + '#updatedAt': 'updatedAt', + '#updatedBy': 'updatedBy', + }, + ExpressionAttributeValues: { + ':condition_1_templateStatus': 'NOT_YET_SUBMITTED', + ':condition_2_1_lockNumber': 1, + ':lockNumber': 1, + ':templateStatus': 'SUBMITTED', + ':updatedAt': date.toISOString(), + ':updatedBy': `INTERNAL_USER#${user.internalUserId}`, + }, + Key: { + id: routingConfig.cascade[0].defaultTemplateId!, + owner: clientOwnerKey, + }, + ReturnValues: 'ALL_NEW', + ReturnValuesOnConditionCheckFailure: 'ALL_OLD', + TableName: 'template-table-name', + UpdateExpression: + 'SET #templateStatus = :templateStatus, #updatedAt = :updatedAt, #updatedBy = :updatedBy ADD #lockNumber :lockNumber', + }, + }, + ], + }); + }); + + test('uses ConditionCheck for templates already SUBMITTED', async () => { + const { repo, mocks } = setup(); + + const routingConfigWithLock: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + }; + + const completed: RoutingConfig = { + ...routingConfig, + status: 'COMPLETED', + lockNumber: 3, + }; + + const template = makeTemplateMock({ + id: routingConfig.cascade[0].defaultTemplateId!, + templateType: 'SMS', + templateStatus: 'SUBMITTED', + lockNumber: 5, + }); + + mocks.dynamo + .on(GetCommand) + .resolvesOnce({ Item: routingConfigWithLock }) + .resolvesOnce({ Item: completed }); + mocks.dynamo.on(BatchGetCommand).resolvesOnce({ + Responses: { + [TEMPLATE_TABLE_NAME]: [template], }, - Key: { - id: routingConfig.id, - owner: clientOwnerKey, + }); + mocks.dynamo.on(TransactWriteCommand).resolvesOnce({}); + + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ data: completed }); + + expect(mocks.dynamo).toHaveReceivedCommandWith(TransactWriteCommand, { + TransactItems: [ + { + Update: expect.objectContaining({ + TableName: TABLE_NAME, + Key: { + id: routingConfig.id, + owner: clientOwnerKey, + }, + }), + }, + { + ConditionCheck: { + TableName: TEMPLATE_TABLE_NAME, + Key: { + id: routingConfig.cascade[0].defaultTemplateId!, + owner: clientOwnerKey, + }, + ConditionExpression: + 'attribute_exists(id) AND lockNumber = :lockNumber AND templateStatus = :submitted', + ExpressionAttributeValues: { + ':lockNumber': 5, + ':submitted': 'SUBMITTED', + }, + ReturnValuesOnConditionCheckFailure: 'ALL_OLD', + }, + }, + ], + }); + }); + + test('returns failure on client error during initial get', async () => { + const { repo, mocks } = setup(); + + const err = new Error('ddb err'); + + mocks.dynamo.on(GetCommand).rejectsOnce(err); + + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ + error: { + actualError: err, + errorMeta: { + code: 500, + description: 'Error retrieving routing configuration', + }, }, - ReturnValues: 'ALL_NEW', - TableName: TABLE_NAME, - UpdateExpression: - 'SET #status = :status, #updatedAt = :updatedAt, #updatedBy = :updatedBy ADD #lockNumber :lockNumber', }); }); - test('returns failure on client error', async () => { + test('returns failure on client error during transaction', async () => { const { repo, mocks } = setup(); const err = new Error('ddb err'); - mocks.dynamo.on(UpdateCommand).rejects(err); + const routingConfigWithLock: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + }; + + const template = makeTemplateMock({ + id: routingConfig.cascade[0].defaultTemplateId!, + templateType: 'SMS', + templateStatus: 'NOT_YET_SUBMITTED', + lockNumber: 1, + }); + + mocks.dynamo.on(GetCommand).resolvesOnce({ Item: routingConfigWithLock }); + mocks.dynamo.on(BatchGetCommand).resolvesOnce({ + Responses: { + [TEMPLATE_TABLE_NAME]: [template], + }, + }); + mocks.dynamo.on(TransactWriteCommand).rejectsOnce(err); const result = await repo.submit(routingConfig.id, user, 2); @@ -314,13 +515,34 @@ describe('RoutingConfigRepository', () => { test('returns failure if updated template is invalid', async () => { const { repo, mocks } = setup(); + const routingConfigWithLock: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + }; + const completedInvalid: RoutingConfig = { ...routingConfig, status: 'COMPLETED', name: 0 as unknown as string, }; - mocks.dynamo.on(UpdateCommand).resolves({ Attributes: completedInvalid }); + const template = makeTemplateMock({ + id: routingConfig.cascade[0].defaultTemplateId!, + templateType: 'SMS', + templateStatus: 'NOT_YET_SUBMITTED', + lockNumber: 1, + }); + + mocks.dynamo + .on(GetCommand) + .resolvesOnce({ Item: routingConfigWithLock }) + .resolvesOnce({ Item: completedInvalid }); + mocks.dynamo.on(BatchGetCommand).resolvesOnce({ + Responses: { + [TEMPLATE_TABLE_NAME]: [template], + }, + }); + mocks.dynamo.on(TransactWriteCommand).resolvesOnce({}); const result = await repo.submit(routingConfig.id, user, 2); @@ -344,15 +566,51 @@ describe('RoutingConfigRepository', () => { }); }); - test('returns 404 failure if routing config does not exist', async () => { + test('returns failure if template from database is invalid', async () => { const { repo, mocks } = setup(); - const err = new ConditionalCheckFailedException({ - $metadata: {}, - message: 'msg', + const routingConfigWithLock: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + }; + + // Invalid template - missing required fields like name, createdAt, etc. + const invalidTemplate = { + id: routingConfig.cascade[0].defaultTemplateId!, + owner: clientOwnerKey, + templateType: 'SMS', + templateStatus: 'NOT_YET_SUBMITTED', + lockNumber: 1, + // Missing: name, message, createdAt, updatedAt + }; + + mocks.dynamo.on(GetCommand).resolvesOnce({ Item: routingConfigWithLock }); + mocks.dynamo.on(BatchGetCommand).resolvesOnce({ + Responses: { + [TEMPLATE_TABLE_NAME]: [invalidTemplate], + }, }); - mocks.dynamo.on(UpdateCommand).rejects(err); + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ + error: { + actualError: expect.any(Error), + errorMeta: { + code: 500, + description: 'Error parsing template from database', + details: undefined, + }, + }, + }); + + expect(mocks.dynamo).not.toHaveReceivedCommand(TransactWriteCommand); + }); + + test('returns 404 failure if routing config does not exist on get', async () => { + const { repo, mocks } = setup(); + + mocks.dynamo.on(GetCommand).resolvesOnce({ Item: undefined }); const result = await repo.submit(routingConfig.id, user, 2); @@ -366,16 +624,33 @@ describe('RoutingConfigRepository', () => { }); }); - test('returns 404 failure if routing config is DELETED', async () => { + test('returns 404 failure if routing config does not exist on write', async () => { const { repo, mocks } = setup(); - const err = new ConditionalCheckFailedException({ - Item: { status: { S: 'DELETED' satisfies RoutingConfigStatus } }, + const routingConfigWithLock: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + }; + + const template = makeTemplateMock({ + id: routingConfig.cascade[0].defaultTemplateId!, + templateStatus: 'NOT_YET_SUBMITTED', + lockNumber: 1, + }); + + const err = new TransactionCanceledException({ $metadata: {}, message: 'msg', + CancellationReasons: [{ Code: 'ConditionalCheckFailed' }], }); - mocks.dynamo.on(UpdateCommand).rejects(err); + mocks.dynamo.on(GetCommand).resolvesOnce({ Item: routingConfigWithLock }); + mocks.dynamo.on(BatchGetCommand).resolvesOnce({ + Responses: { + [TEMPLATE_TABLE_NAME]: [template], + }, + }); + mocks.dynamo.on(TransactWriteCommand).rejectsOnce(err); const result = await repo.submit(routingConfig.id, user, 2); @@ -389,16 +664,41 @@ describe('RoutingConfigRepository', () => { }); }); - test('returns 400 failure if routing config is COMPLETED', async () => { + test('returns 404 failure if routing config is DELETED', async () => { const { repo, mocks } = setup(); - const err = new ConditionalCheckFailedException({ - Item: { status: { S: 'COMPLETED' satisfies RoutingConfigStatus } }, - $metadata: {}, - message: 'msg', + const deletedRoutingConfig: RoutingConfig = { + ...routingConfig, + status: 'DELETED', + }; + + mocks.dynamo.on(GetCommand).resolvesOnce({ Item: deletedRoutingConfig }); + + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ + error: { + errorMeta: { + code: 404, + description: 'Routing configuration not found', + }, + }, }); - mocks.dynamo.on(UpdateCommand).rejects(err); + expect(mocks.dynamo).not.toHaveReceivedCommand(TransactWriteCommand); + }); + + test('returns 400 failure if routing config is COMPLETED', async () => { + const { repo, mocks } = setup(); + + const completedRoutingConfig: RoutingConfig = { + ...routingConfig, + status: 'COMPLETED', + }; + + mocks.dynamo + .on(GetCommand) + .resolvesOnce({ Item: completedRoutingConfig }); const result = await repo.submit(routingConfig.id, user, 2); @@ -411,31 +711,561 @@ describe('RoutingConfigRepository', () => { }, }, }); + + expect(mocks.dynamo).not.toHaveReceivedCommand(TransactWriteCommand); }); test("returns 409 failure if lock number doesn't match", async () => { const { repo, mocks } = setup(); - const err = new ConditionalCheckFailedException({ - Item: { - status: { S: 'DRAFT' satisfies RoutingConfigStatus }, - lockNumber: { N: '3' }, - }, - $metadata: {}, - message: 'msg', - }); + const mismatchedLockRoutingConfig: RoutingConfig = { + ...routingConfig, + lockNumber: 3, + }; - mocks.dynamo.on(UpdateCommand).rejects(err); + mocks.dynamo + .on(GetCommand) + .resolvesOnce({ Item: mismatchedLockRoutingConfig }); const result = await repo.submit(routingConfig.id, user, 2); expect(result).toEqual({ error: { - actualError: err, errorMeta: { code: 409, description: - 'Lock number mismatch - Message Plan has been modified since last read', + 'Lock number mismatch - Routing configuration has been modified since last read', + }, + }, + }); + + expect(mocks.dynamo).not.toHaveReceivedCommand(TransactWriteCommand); + }); + + test('returns validation failure if cascade item conditionalTemplates is empty with no defaultTemplateId', async () => { + const { repo, mocks } = setup(); + + const invalidRoutingConfig: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'NHSAPP', + channelType: 'primary', + conditionalTemplates: [], + }, + ], + }; + + mocks.dynamo.on(GetCommand).resolvesOnce({ Item: invalidRoutingConfig }); + + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ + error: { + actualError: expect.any(Error), + errorMeta: { + code: 400, + description: + 'Routing config is not ready for submission: all cascade items must have templates assigned', + }, + }, + }); + + expect(mocks.dynamo).not.toHaveReceivedCommand(TransactWriteCommand); + }); + + test('returns validation failure if defaultTemplateId is null', async () => { + const { repo, mocks } = setup(); + + const invalidRoutingConfig: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'NHSAPP', + channelType: 'primary', + defaultTemplateId: null, + }, + ], + }; + + mocks.dynamo.on(GetCommand).resolvesOnce({ Item: invalidRoutingConfig }); + + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ + error: { + actualError: expect.any(Error), + errorMeta: { + code: 400, + description: + 'Routing config is not ready for submission: all cascade items must have templates assigned', + }, + }, + }); + + expect(mocks.dynamo).not.toHaveReceivedCommand(TransactWriteCommand); + }); + + test('returns 400 failure if template not found during submit', async () => { + const { repo, mocks } = setup(); + + const routingConfigWithTemplate: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'SMS', + channelType: 'primary', + defaultTemplateId: 'missing-template-id', + }, + ], + }; + + mocks.dynamo + .on(GetCommand) + .resolvesOnce({ Item: routingConfigWithTemplate }); + mocks.dynamo.on(BatchGetCommand).resolvesOnce({ + Responses: { + [TEMPLATE_TABLE_NAME]: [], + }, + }); + + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ + error: { + errorMeta: { + code: 400, + description: 'Some templates not found', + details: { templateIds: 'missing-template-id' }, + }, + }, + }); + + expect(mocks.dynamo).not.toHaveReceivedCommand(TransactWriteCommand); + }); + + test('returns 400 failure if template has DELETED status', async () => { + const { repo, mocks } = setup(); + + const routingConfigWithTemplate: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'SMS', + channelType: 'primary', + defaultTemplateId: 'deleted-template-id', + }, + ], + }; + + mocks.dynamo + .on(GetCommand) + .resolvesOnce({ Item: routingConfigWithTemplate }); + // Template exists but is DELETED + mocks.dynamo.on(BatchGetCommand).resolvesOnce({ + Responses: { + [TEMPLATE_TABLE_NAME]: [ + makeTemplateMock({ + id: 'deleted-template-id', + templateType: 'SMS', + templateStatus: 'DELETED', + lockNumber: 1, + }), + ], + }, + }); + + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ + error: { + errorMeta: { + code: 400, + description: 'Some templates not found', + details: { templateIds: 'deleted-template-id' }, + }, + }, + }); + + expect(mocks.dynamo).not.toHaveReceivedCommand(TransactWriteCommand); + }); + + test('returns 400 failure if LETTER template has invalid status', async () => { + const { repo, mocks } = setup(); + + const routingConfigWithLetter: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'LETTER', + channelType: 'primary', + defaultTemplateId: 'letter-template-id', + }, + ], + }; + + mocks.dynamo + .on(GetCommand) + .resolvesOnce({ Item: routingConfigWithLetter }); + // LETTER template with invalid status (not PROOF_APPROVED or SUBMITTED) + mocks.dynamo.on(BatchGetCommand).resolvesOnce({ + Responses: { + [TEMPLATE_TABLE_NAME]: [ + { + id: 'letter-template-id', + owner: clientOwnerKey, + name: 'Test Letter Template', + templateType: 'LETTER', + templateStatus: 'NOT_YET_SUBMITTED', + lockNumber: 1, + letterType: 'x0', + language: 'en', + letterVersion: 'PDF', + files: { + pdfTemplate: { + fileName: 'test.pdf', + currentVersion: '1', + virusScanStatus: 'PASSED', + }, + }, + createdAt: date.toISOString(), + updatedAt: date.toISOString(), + }, + ], + }, + }); + + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ + error: { + errorMeta: { + code: 400, + description: + 'Letter templates must have status PROOF_APPROVED or SUBMITTED', + details: { templateIds: 'letter-template-id' }, + }, + }, + }); + + expect(mocks.dynamo).not.toHaveReceivedCommand(TransactWriteCommand); + }); + + test('returns 500 failure on unexpected TransactionCanceledException', async () => { + const { repo, mocks } = setup(); + + // Transaction cancelled with update succeeded but no template failures + // This is an edge case that shouldn't normally happen + const err = new TransactionCanceledException({ + $metadata: {}, + message: 'Unexpected cancellation', + CancellationReasons: [ + { Code: 'None' }, // Update succeeded + { Code: 'None' }, // Template update also passed (unexpected) + ], + }); + + const routingConfigWithTemplate: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'SMS', + channelType: 'primary', + defaultTemplateId: 'some-template-id', + }, + ], + }; + + const template = makeTemplateMock({ + id: 'some-template-id', + templateStatus: 'NOT_YET_SUBMITTED', + lockNumber: 1, + }); + + mocks.dynamo + .on(GetCommand) + .resolvesOnce({ Item: routingConfigWithTemplate }); + mocks.dynamo.on(BatchGetCommand).resolvesOnce({ + Responses: { + [TEMPLATE_TABLE_NAME]: [template], + }, + }); + mocks.dynamo.on(TransactWriteCommand).rejectsOnce(err); + + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ + error: { + actualError: err, + errorMeta: { + code: 500, + description: 'Failed to update routing config', + }, + }, + }); + }); + + test('returns 500 failure on TransactionCanceledException with undefined CancellationReasons', async () => { + const { repo, mocks } = setup(); + + const err = new TransactionCanceledException({ + $metadata: {}, + message: 'Transaction cancelled with no reasons', + CancellationReasons: undefined, + }); + + const routingConfigWithLock: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + }; + + const template = makeTemplateMock({ + id: routingConfig.cascade[0].defaultTemplateId!, + templateStatus: 'NOT_YET_SUBMITTED', + lockNumber: 1, + }); + + mocks.dynamo.on(GetCommand).resolvesOnce({ Item: routingConfigWithLock }); + mocks.dynamo.on(BatchGetCommand).resolvesOnce({ + Responses: { + [TEMPLATE_TABLE_NAME]: [template], + }, + }); + mocks.dynamo.on(TransactWriteCommand).rejectsOnce(err); + + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ + error: { + actualError: err, + errorMeta: { + code: 500, + description: 'Failed to update routing config', + }, + }, + }); + }); + + test('returns 409 failure if template was modified since retrieval', async () => { + const { repo, mocks } = setup(); + + const routingConfigWithTemplate: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'SMS', + channelType: 'primary', + defaultTemplateId: 'modified-template-id', + }, + ], + }; + + const template = makeTemplateMock({ + id: 'modified-template-id', + templateStatus: 'NOT_YET_SUBMITTED', + lockNumber: 1, + }); + + const err = new TransactionCanceledException({ + $metadata: {}, + message: 'Transaction cancelled', + CancellationReasons: [ + { Code: 'None' }, // Routing config update succeeded + { Code: 'ConditionalCheckFailed' }, // Template lock number mismatch + ], + }); + + mocks.dynamo + .on(GetCommand) + .resolvesOnce({ Item: routingConfigWithTemplate }); + mocks.dynamo.on(BatchGetCommand).resolvesOnce({ + Responses: { + [TEMPLATE_TABLE_NAME]: [template], + }, + }); + mocks.dynamo.on(TransactWriteCommand).rejectsOnce(err); + + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ + error: { + actualError: err, + errorMeta: { + code: 409, + description: + 'Some templates have been modified since they were retrieved', + details: { templateIds: 'modified-template-id' }, + }, + }, + }); + }); + + test('returns 500 failure if BatchGetCommand fails', async () => { + const { repo, mocks } = setup(); + + const routingConfigWithLock: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + }; + + const err = new Error('BatchGet failed'); + + mocks.dynamo.on(GetCommand).resolvesOnce({ Item: routingConfigWithLock }); + mocks.dynamo.on(BatchGetCommand).rejectsOnce(err); + + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ + error: { + actualError: err, + errorMeta: { + code: 500, + description: 'Failed to retrieve templates', + }, + }, + }); + + expect(mocks.dynamo).not.toHaveReceivedCommand(TransactWriteCommand); + }); + + test('succeeds when routing config has no templates (empty cascade)', async () => { + const { repo, mocks } = setup(); + + const routingConfigNoTemplates: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'NHSAPP', + channelType: 'primary', + conditionalTemplates: [ + { language: 'en', templateId: 'english-template' }, + ], + }, + ], + }; + + const completed: RoutingConfig = { + ...routingConfigNoTemplates, + status: 'COMPLETED', + lockNumber: 3, + }; + + const template = makeTemplateMock({ + id: 'english-template', + templateType: 'NHS_APP', + templateStatus: 'NOT_YET_SUBMITTED', + lockNumber: 1, + }); + + mocks.dynamo + .on(GetCommand) + .resolvesOnce({ Item: routingConfigNoTemplates }) + .resolvesOnce({ Item: completed }); + mocks.dynamo.on(BatchGetCommand).resolvesOnce({ + Responses: { + [TEMPLATE_TABLE_NAME]: [template], + }, + }); + mocks.dynamo.on(TransactWriteCommand).resolvesOnce({}); + + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ data: completed }); + }); + + test('handles template with undefined lockNumber', async () => { + const { repo, mocks } = setup(); + + const routingConfigWithLock: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + }; + + const completed: RoutingConfig = { + ...routingConfig, + status: 'COMPLETED', + lockNumber: 3, + }; + + const template = { + id: routingConfig.cascade[0].defaultTemplateId!, + owner: clientOwnerKey, + templateType: 'SMS', + templateStatus: 'NOT_YET_SUBMITTED', + name: 'Test Template', + message: 'Test message content', + createdAt: date.toISOString(), + updatedAt: date.toISOString(), + }; + + mocks.dynamo + .on(GetCommand) + .resolvesOnce({ Item: routingConfigWithLock }) + .resolvesOnce({ Item: completed }); + mocks.dynamo.on(BatchGetCommand).resolvesOnce({ + Responses: { + [TEMPLATE_TABLE_NAME]: [template], + }, + }); + mocks.dynamo.on(TransactWriteCommand).resolvesOnce({}); + + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ data: completed }); + + expect(mocks.dynamo).toHaveReceivedCommandWith(TransactWriteCommand, { + TransactItems: expect.arrayContaining([ + expect.objectContaining({ + Update: expect.objectContaining({ + ExpressionAttributeValues: expect.objectContaining({ + ':condition_2_1_lockNumber': 0, + }), + }), + }), + ]), + }); + }); + + test('handles BatchGetCommand returning empty Responses', async () => { + const { repo, mocks } = setup(); + + const routingConfigWithLock: RoutingConfig = { + ...routingConfig, + lockNumber: 2, + }; + + mocks.dynamo.on(GetCommand).resolvesOnce({ Item: routingConfigWithLock }); + mocks.dynamo.on(BatchGetCommand).resolvesOnce({ + Responses: {}, + }); + + const result = await repo.submit(routingConfig.id, user, 2); + + expect(result).toEqual({ + error: { + actualError: undefined, + errorMeta: { + code: 400, + description: 'Some templates not found', + details: { + templateIds: routingConfig.cascade[0].defaultTemplateId, + }, }, }, }); @@ -631,7 +1461,7 @@ describe('RoutingConfigRepository', () => { errorMeta: { code: 409, description: - 'Lock number mismatch - Message Plan has been modified since last read', + 'Lock number mismatch - Routing configuration has been modified since last read', }, }, }); @@ -1495,7 +2325,7 @@ describe('RoutingConfigRepository', () => { errorMeta: { code: 409, description: - 'Lock number mismatch - Message Plan has been modified since last read', + 'Lock number mismatch - Routing configuration has been modified since last read', }, }, }); diff --git a/lambdas/backend-api/src/app/routing-config-client.ts b/lambdas/backend-api/src/app/routing-config-client.ts index 7e220e1ac..32b46dacb 100644 --- a/lambdas/backend-api/src/app/routing-config-client.ts +++ b/lambdas/backend-api/src/app/routing-config-client.ts @@ -189,7 +189,7 @@ export class RoutingConfigClient { const result = await this.routingConfigRepository.get(id, user.clientId); if (result.data?.status === 'DELETED') { - return failure(ErrorCase.NOT_FOUND, 'Routing Config not found'); + return failure(ErrorCase.NOT_FOUND, 'Routing configuration not found'); } return result; diff --git a/lambdas/backend-api/src/infra/routing-config-repository/repository.ts b/lambdas/backend-api/src/infra/routing-config-repository/repository.ts index 5d34a209a..0dbd21ea0 100644 --- a/lambdas/backend-api/src/infra/routing-config-repository/repository.ts +++ b/lambdas/backend-api/src/infra/routing-config-repository/repository.ts @@ -1,5 +1,6 @@ import { randomUUID } from 'node:crypto'; import { + BatchGetCommand, GetCommand, PutCommand, TransactWriteCommand, @@ -13,19 +14,26 @@ import { } from '@backend-api/utils/result'; import { $RoutingConfig, + $SubmittableCascade, + $TemplateDto, CascadeItem, type CreateRoutingConfig, ErrorCase, type RoutingConfig, type RoutingConfigReference, RoutingConfigStatus, + type TemplateDto, type UpdateRoutingConfig, } from 'nhs-notify-backend-client'; import type { User } from 'nhs-notify-web-template-management-utils'; import { RoutingConfigQuery } from './query'; -import { RoutingConfigUpdateBuilder } from 'nhs-notify-entity-update-command-builder'; +import { + RoutingConfigUpdateBuilder, + TemplateUpdateBuilder, +} from 'nhs-notify-entity-update-command-builder'; import { ConditionalCheckFailedException, + ReturnValuesOnConditionCheckFailure, TransactionCanceledException, } from '@aws-sdk/client-dynamodb'; import { calculateTTL } from '@backend-api/utils/calculate-ttl'; @@ -168,7 +176,89 @@ export class RoutingConfigRepository { user: User, lockNumber: number ): Promise> { - const cmdInput = new RoutingConfigUpdateBuilder( + const existingConfig = await this.get(id, user.clientId); + + if (existingConfig.error) { + return existingConfig; + } + + const { + status, + cascade, + lockNumber: currentLockNumber, + } = existingConfig.data; + + if (status === 'DELETED') { + return failure(ErrorCase.NOT_FOUND, 'Routing configuration not found'); + } + + if (status === 'COMPLETED') { + return failure( + ErrorCase.ALREADY_SUBMITTED, + 'Routing configuration with status COMPLETED cannot be updated' + ); + } + + if (currentLockNumber !== lockNumber) { + return failure( + ErrorCase.CONFLICT, + 'Lock number mismatch - Routing configuration has been modified since last read' + ); + } + + const submittableCascadeValidation = $SubmittableCascade.safeParse(cascade); + + if (!submittableCascadeValidation.success) { + return failure( + ErrorCase.VALIDATION_FAILED, + 'Routing config is not ready for submission: all cascade items must have templates assigned', + submittableCascadeValidation.error + ); + } + + const templateIds = this.extractTemplateIds(cascade); + + const templatesResult = await this.getTemplates(templateIds, user.clientId); + + if (templatesResult.error) { + return templatesResult; + } + + const templates = templatesResult.data; + + const missingTemplateIds = templateIds.filter( + (tid) => + !templates.some((t) => t.id === tid && t.templateStatus !== 'DELETED') + ); + + if (missingTemplateIds.length > 0) { + return failure( + ErrorCase.ROUTING_CONFIG_TEMPLATES_NOT_FOUND, + 'Some templates not found', + undefined, + { templateIds: missingTemplateIds.join(',') } + ); + } + + const invalidLetterTemplateIds = templates + .filter( + (t) => + t.templateType === 'LETTER' && + t.templateStatus !== 'PROOF_APPROVED' && + t.templateStatus !== 'SUBMITTED' + ) + .map((t) => t.id); + + if (invalidLetterTemplateIds.length > 0) { + return failure( + ErrorCase.VALIDATION_FAILED, + 'Letter templates must have status PROOF_APPROVED or SUBMITTED', + undefined, + { templateIds: invalidLetterTemplateIds.join(',') } + ); + } + + const routingConfigUpdate = new RoutingConfigUpdateBuilder( this.tableName, user.clientId, id, @@ -181,10 +271,62 @@ export class RoutingConfigRepository { .incrementLockNumber() .build(); + // add an update to each template to set status to SUBMITTED, or a condition check if already SUBMITTED + const templateUpdatesAndChecks = templates.map((template) => + template.templateStatus === 'SUBMITTED' + ? { + ConditionCheck: { + TableName: this.templateTableName, + Key: { + id: template.id, + owner: this.clientOwnerKey(user.clientId), + }, + ConditionExpression: + 'attribute_exists(id) AND lockNumber = :lockNumber AND templateStatus = :submitted', + ExpressionAttributeValues: { + ':lockNumber': template.lockNumber, + ':submitted': 'SUBMITTED', + }, + ReturnValuesOnConditionCheckFailure: + ReturnValuesOnConditionCheckFailure.ALL_OLD, + }, + } + : { + Update: new TemplateUpdateBuilder( + this.templateTableName, + user.clientId, + template.id, + this.updateCmdOpts + ) + .setStatus('SUBMITTED') + .expectStatus(template.templateStatus) + .expectLockNumber(template.lockNumber) + .incrementLockNumber() + .setUpdatedByUserAt(this.internalUserKey(user)) + .build(), + } + ); + try { - const result = await this.client.send(new UpdateCommand(cmdInput)); + await this.client.send( + new TransactWriteCommand({ + TransactItems: [ + { + Update: routingConfigUpdate, + }, + ...templateUpdatesAndChecks, + ], + }) + ); - const parsed = $RoutingConfig.safeParse(result.Attributes); + const getResult = await this.client.send( + new GetCommand({ + TableName: this.tableName, + Key: { id, owner: this.clientOwnerKey(user.clientId) }, + }) + ); + + const parsed = $RoutingConfig.safeParse(getResult.Item); if (!parsed.success) { return failure( @@ -196,7 +338,49 @@ export class RoutingConfigRepository { return success(parsed.data); } catch (error) { - return this.handleUpdateError(error, lockNumber); + return this.handleSubmitTransactionError(error, lockNumber, templateIds); + } + } + + private async getTemplates( + templateIds: string[], + clientId: string + ): Promise> { + try { + const result = await this.client.send( + new BatchGetCommand({ + RequestItems: { + [this.templateTableName]: { + Keys: templateIds.map((tid) => ({ + id: tid, + owner: this.clientOwnerKey(clientId), + })), + }, + }, + }) + ); + + const rawTemplates = result.Responses?.[this.templateTableName] ?? []; + + const templates: TemplateDto[] = []; + + for (const item of rawTemplates) { + const parsed = $TemplateDto.safeParse(item); + + if (!parsed.success) { + return failure( + ErrorCase.INTERNAL, + 'Error parsing template from database', + parsed.error + ); + } + + templates.push(parsed.data); + } + + return success(templates); + } catch (error) { + return failure(ErrorCase.INTERNAL, 'Failed to retrieve templates', error); } } @@ -254,7 +438,7 @@ export class RoutingConfigRepository { ); if (!result.Item) { - return failure(ErrorCase.NOT_FOUND, 'Routing Config not found'); + return failure(ErrorCase.NOT_FOUND, 'Routing configuration not found'); } const parsed = $RoutingConfig.parse(result.Item); @@ -263,7 +447,7 @@ export class RoutingConfigRepository { } catch (error) { return failure( ErrorCase.INTERNAL, - 'Error retrieving Routing Config', + 'Error retrieving routing configuration', error ); } @@ -298,7 +482,7 @@ export class RoutingConfigRepository { if (item.lockNumber !== expectedLockNumber) { return failure( ErrorCase.CONFLICT, - 'Lock number mismatch - Message Plan has been modified since last read', + 'Lock number mismatch - Routing configuration has been modified since last read', err ); } @@ -401,4 +585,51 @@ export class RoutingConfigRepository { ]) .filter((id): id is string => id != null); } + + private handleSubmitTransactionError( + err: unknown, + lockNumber: number, + templateIds: string[] + ): ApplicationResult { + if (!(err instanceof TransactionCanceledException)) { + return this.handleUpdateError(err, lockNumber); + } + + // Note: The first item will always be the routing config update + // https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_CancellationReason.html + const [routingConfigReason, ...templateReasons] = + err.CancellationReasons ?? []; + + if (routingConfigReason && routingConfigReason.Code !== 'None') { + return this.handleUpdateError( + new ConditionalCheckFailedException({ + message: routingConfigReason.Message!, + Item: routingConfigReason.Item, + $metadata: err.$metadata, + }), + lockNumber + ); + } + + // Find which template updates failed - likely due to lock number mismatch + const failedTemplateIds: string[] = []; + + for (const [index, reason] of templateReasons.entries()) { + if (reason.Code === 'ConditionalCheckFailed') { + const templateId = templateIds[index]; + failedTemplateIds.push(templateId); + } + } + + if (failedTemplateIds.length > 0) { + return failure( + ErrorCase.CONFLICT, + 'Some templates have been modified since they were retrieved', + err, + { templateIds: failedTemplateIds.join(',') } + ); + } + + return this.handleUpdateError(err, lockNumber); + } } diff --git a/lambdas/backend-client/src/__tests__/schemas/routing-config.test.ts b/lambdas/backend-client/src/__tests__/schemas/routing-config.test.ts index 575ae856b..1da4f5288 100644 --- a/lambdas/backend-client/src/__tests__/schemas/routing-config.test.ts +++ b/lambdas/backend-client/src/__tests__/schemas/routing-config.test.ts @@ -3,6 +3,8 @@ import { $ListRoutingConfigFilters, $RoutingConfig, $UpdateRoutingConfig, + $SubmittableCascade, + $SubmittableCascadeItem, } from '../../schemas/routing-config'; const cascadeItemDefault = { @@ -90,33 +92,6 @@ describe.each([ expect(res.success).toBe(true); }); - test('valid with conditional template ids set to null', () => { - const res = $Schema.safeParse({ - ...baseInput, - cascade: [ - { - ...cascadeCondAcc, - conditionalTemplates: [ - { - accessibleFormat: 'x1', - templateId: null, - }, - ], - }, - { - ...cascadeCondLang, - conditionalTemplates: [ - { - language: 'ar', - templateId: null, - }, - ], - }, - ], - }); - expect(res.success).toBe(true); - }); - test('snapshot full error', () => { const res = $Schema.safeParse({}); @@ -416,3 +391,148 @@ describe('UpdateRoutingConfig', () => { expect(res.error).toMatchSnapshot(); }); }); + +describe('$SubmittableCascadeItem', () => { + test('valid with defaultTemplateId', () => { + const res = $SubmittableCascadeItem.safeParse({ + cascadeGroups: ['standard'], + channel: 'LETTER', + channelType: 'primary', + defaultTemplateId: '90e46ece-4a3b-47bd-b781-f986b42a5a09', + }); + expect(res.success).toBe(true); + }); + + test('valid with defaultTemplateId and conditionalTemplates', () => { + const res = $SubmittableCascadeItem.safeParse({ + cascadeGroups: ['standard', 'translations'], + channel: 'LETTER', + channelType: 'primary', + defaultTemplateId: '90e46ece-4a3b-47bd-b781-f986b42a5a09', + conditionalTemplates: [ + { language: 'ar', templateId: '90e46ece-4a3b-47bd-b781-f986b42a5a09' }, + ], + }); + expect(res.success).toBe(true); + }); + + test('valid with conditionalTemplates only (non-empty)', () => { + const res = $SubmittableCascadeItem.safeParse({ + cascadeGroups: ['translations'], + channel: 'LETTER', + channelType: 'secondary', + conditionalTemplates: [ + { language: 'ar', templateId: '90e46ece-4a3b-47bd-b781-f986b42a5a09' }, + ], + }); + expect(res.success).toBe(true); + }); + + test('valid with accessible conditional templates only', () => { + const res = $SubmittableCascadeItem.safeParse({ + cascadeGroups: ['accessible'], + channel: 'LETTER', + channelType: 'secondary', + conditionalTemplates: [ + { + accessibleFormat: 'x1', + templateId: '90e46ece-4a3b-47bd-b781-f986b42a5a09', + }, + ], + }); + expect(res.success).toBe(true); + }); + + test('invalid without defaultTemplateId and without conditionalTemplates', () => { + const res = $SubmittableCascadeItem.safeParse({ + cascadeGroups: ['standard'], + channel: 'LETTER', + channelType: 'primary', + }); + expect(res.success).toBe(false); + }); + + test('invalid with empty conditionalTemplates without defaultTemplateId', () => { + const res = $SubmittableCascadeItem.safeParse({ + cascadeGroups: ['standard'], + channel: 'LETTER', + channelType: 'primary', + conditionalTemplates: [], + }); + expect(res.success).toBe(false); + }); + + test('invalid with empty defaultTemplateId', () => { + const res = $SubmittableCascadeItem.safeParse({ + cascadeGroups: ['standard'], + channel: 'LETTER', + channelType: 'primary', + defaultTemplateId: '', + conditionalTemplates: [ + { language: 'ar', templateId: '90e46ece-4a3b-47bd-b781-f986b42a5a09' }, + ], + }); + expect(res.success).toBe(false); + }); + + test('invalid with null defaultTemplateId', () => { + const res = $SubmittableCascadeItem.safeParse({ + cascadeGroups: ['standard'], + channel: 'LETTER', + channelType: 'primary', + defaultTemplateId: null, + conditionalTemplates: [ + { language: 'ar', templateId: '90e46ece-4a3b-47bd-b781-f986b42a5a09' }, + ], + }); + expect(res.success).toBe(false); + }); +}); + +describe('$SubmittableCascade', () => { + const validCascadeItem = { + cascadeGroups: ['standard'], + channel: 'LETTER', + channelType: 'primary', + defaultTemplateId: '90e46ece-4a3b-47bd-b781-f986b42a5a09', + }; + + test('valid with single item', () => { + const res = $SubmittableCascade.safeParse([validCascadeItem]); + expect(res.success).toBe(true); + }); + + test('valid with multiple items', () => { + const res = $SubmittableCascade.safeParse([ + validCascadeItem, + { + cascadeGroups: ['translations'], + channel: 'EMAIL', + channelType: 'secondary', + conditionalTemplates: [ + { + language: 'ar', + templateId: '90e46ece-4a3b-47bd-b781-f986b42a5a09', + }, + ], + }, + ]); + expect(res.success).toBe(true); + }); + + test('invalid with empty array', () => { + const res = $SubmittableCascade.safeParse([]); + expect(res.success).toBe(false); + }); + + test('invalid with invalid cascade item in array', () => { + const res = $SubmittableCascade.safeParse([ + { + cascadeGroups: ['standard'], + channel: 'LETTER', + channelType: 'primary', + }, + ]); + expect(res.success).toBe(false); + }); +}); diff --git a/lambdas/backend-client/src/schemas/routing-config.ts b/lambdas/backend-client/src/schemas/routing-config.ts index 450418e99..a40826e7d 100644 --- a/lambdas/backend-client/src/schemas/routing-config.ts +++ b/lambdas/backend-client/src/schemas/routing-config.ts @@ -58,14 +58,14 @@ const $CascadeGroup = schemaFor()( ]) ); -export const $Channel = schemaFor()(z.enum(CHANNEL_LIST)); +const $Channel = schemaFor()(z.enum(CHANNEL_LIST)); const $ChannelType = schemaFor()(z.enum(CHANNEL_TYPE_LIST)); const $ConditionalTemplateLanguage = schemaFor()( z.object({ language: $Language, - templateId: z.string().nonempty().nullable(), + templateId: z.string().nonempty(), supplierReferences: z.record(z.string(), z.string()).optional(), }) ); @@ -74,7 +74,7 @@ const $ConditionalTemplateAccessible = schemaFor()( z.object({ accessibleFormat: $LetterType, - templateId: z.string().nonempty().nullable(), + templateId: z.string().nonempty(), supplierReferences: z.record(z.string(), z.string()).optional(), }) ); @@ -115,6 +115,35 @@ const $CascadeItem = schemaFor()( ) ); +export const $SubmittableCascadeItem = $CascadeItemBase.and( + z.union([ + z.object({ + defaultTemplateId: z.string().nonempty(), + conditionalTemplates: z + .array( + z.union([ + $ConditionalTemplateAccessible, + $ConditionalTemplateLanguage, + ]) + ) + .optional(), + }), + z.object({ + defaultTemplateId: z.string().nonempty().optional(), + conditionalTemplates: z + .array( + z.union([ + $ConditionalTemplateAccessible, + $ConditionalTemplateLanguage, + ]) + ) + .nonempty(), + }), + ]) +); + +export const $SubmittableCascade = z.array($SubmittableCascadeItem).nonempty(); + export const $CreateRoutingConfig = schemaFor()( z.object({ campaignId: z.string(), diff --git a/lambdas/backend-client/src/schemas/template.ts b/lambdas/backend-client/src/schemas/template.ts index e58f4d378..dee1ac4bb 100644 --- a/lambdas/backend-client/src/schemas/template.ts +++ b/lambdas/backend-client/src/schemas/template.ts @@ -84,7 +84,7 @@ export const $SmsProperties = schemaFor()( }) ); -export const $BaseLetterTemplateProperties = z.object({ +const $BaseLetterTemplateProperties = z.object({ templateType: z.literal('LETTER'), letterType: z.enum(LETTER_TYPE_LIST), language: z.enum(LANGUAGE_LIST), @@ -134,7 +134,7 @@ export const $LetterProperties = z.discriminatedUnion('letterVersion', [ $AuthoringLetterProperties, ]); -export const $BaseTemplateSchema = schemaFor()( +const $BaseTemplateSchema = schemaFor()( z.object({ name: z.string().trim().min(1), templateType: z.enum(TEMPLATE_TYPE_LIST), diff --git a/lambdas/backend-client/src/types/generated/types.gen.ts b/lambdas/backend-client/src/types/generated/types.gen.ts index 1006e804e..ebf392677 100644 --- a/lambdas/backend-client/src/types/generated/types.gen.ts +++ b/lambdas/backend-client/src/types/generated/types.gen.ts @@ -107,7 +107,7 @@ export type ConditionalTemplateAccessible = { supplierReferences?: { [key: string]: string; }; - templateId: string | null; + templateId: string; }; export type ConditionalTemplateLanguage = { @@ -115,7 +115,7 @@ export type ConditionalTemplateLanguage = { supplierReferences?: { [key: string]: string; }; - templateId: string | null; + templateId: string; }; export type CountSuccess = { diff --git a/package-lock.json b/package-lock.json index a47557758..b426ccbc7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7746,9 +7746,9 @@ } }, "node_modules/@isaacs/brace-expansion": { - "version": "5.0.0", - "resolved": "https://registry.npmjs.org/@isaacs/brace-expansion/-/brace-expansion-5.0.0.tgz", - "integrity": "sha512-ZT55BDLV0yv0RBm2czMiZ+SqCGO7AvmOM3G/w2xhVPH+te0aKgFjmBvGlL1dH+ql2tgGO3MVrbb3jCKyvpgnxA==", + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/@isaacs/brace-expansion/-/brace-expansion-5.0.1.tgz", + "integrity": "sha512-WMz71T1JS624nWj2n2fnYAuPovhv7EUhk69R6i9dsVyzxt5eM3bjwvgk9L+APE1TRscGysAVMANkB0jh0LQZrQ==", "license": "MIT", "dependencies": { "@isaacs/balanced-match": "^4.0.1" @@ -16164,7 +16164,7 @@ "dev": true, "license": "BlueOak-1.0.0", "dependencies": { - "@isaacs/brace-expansion": "^5.0.0" + "@isaacs/brace-expansion": "^5.0.1" }, "engines": { "node": "20 || >=22" @@ -17198,12 +17198,12 @@ } }, "node_modules/glob/node_modules/minimatch": { - "version": "10.1.1", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.1.1.tgz", - "integrity": "sha512-enIvLvRAFZYXJzkCYG5RKmPfrFArdLv+R+lbQ53BmIMLIry74bjKzX6iHAm8WYamJkhSSEabrWN5D97XnKObjQ==", + "version": "10.1.2", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.1.2.tgz", + "integrity": "sha512-fu656aJ0n2kcXwsnwnv9g24tkU5uSmOlTjd6WyyaKm2Z+h1qmY6bAjrcaIxF/BslFqbZ8UBtbJi7KgQOZD2PTw==", "license": "BlueOak-1.0.0", "dependencies": { - "@isaacs/brace-expansion": "^5.0.0" + "@isaacs/brace-expansion": "^5.0.1" }, "engines": { "node": "20 || >=22" @@ -23289,13 +23289,6 @@ "url": "https://github.com/chalk/ansi-styles?sponsor=1" } }, - "node_modules/pretty-format/node_modules/react-is": { - "version": "17.0.2", - "resolved": "https://registry.npmjs.org/react-is/-/react-is-17.0.2.tgz", - "integrity": "sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w==", - "dev": true, - "license": "MIT" - }, "node_modules/process": { "version": "0.11.10", "resolved": "https://registry.npmjs.org/process/-/process-0.11.10.tgz", diff --git a/tests/test-team/template-mgmt-api-tests/delete-routing-config.api.spec.ts b/tests/test-team/template-mgmt-api-tests/delete-routing-config.api.spec.ts index a1cdd7022..3d59f3214 100644 --- a/tests/test-team/template-mgmt-api-tests/delete-routing-config.api.spec.ts +++ b/tests/test-team/template-mgmt-api-tests/delete-routing-config.api.spec.ts @@ -254,7 +254,7 @@ test.describe('DELETE /v1/routing-configuration/:routingConfigId', () => { expect(await response.json()).toEqual({ statusCode: 409, technicalMessage: - 'Lock number mismatch - Message Plan has been modified since last read', + 'Lock number mismatch - Routing configuration has been modified since last read', }); }); }); diff --git a/tests/test-team/template-mgmt-api-tests/get-routing-config.api.spec.ts b/tests/test-team/template-mgmt-api-tests/get-routing-config.api.spec.ts index 3c8224ec3..3c2e0e2cc 100644 --- a/tests/test-team/template-mgmt-api-tests/get-routing-config.api.spec.ts +++ b/tests/test-team/template-mgmt-api-tests/get-routing-config.api.spec.ts @@ -83,7 +83,7 @@ test.describe('GET /v1/routing-configuration/:routingConfigId', () => { expect(response.status()).toBe(404); expect(await response.json()).toEqual({ statusCode: 404, - technicalMessage: 'Routing Config not found', + technicalMessage: 'Routing configuration not found', }); }); @@ -104,7 +104,7 @@ test.describe('GET /v1/routing-configuration/:routingConfigId', () => { expect(response.status()).toBe(404); expect(await response.json()).toEqual({ statusCode: 404, - technicalMessage: 'Routing Config not found', + technicalMessage: 'Routing configuration not found', }); }); @@ -125,7 +125,7 @@ test.describe('GET /v1/routing-configuration/:routingConfigId', () => { expect(response.status()).toBe(404); expect(await response.json()).toEqual({ statusCode: 404, - technicalMessage: 'Routing Config not found', + technicalMessage: 'Routing configuration not found', }); }); diff --git a/tests/test-team/template-mgmt-api-tests/submit-routing-config.api.spec.ts b/tests/test-team/template-mgmt-api-tests/submit-routing-config.api.spec.ts index ae68c2c73..fd788d423 100644 --- a/tests/test-team/template-mgmt-api-tests/submit-routing-config.api.spec.ts +++ b/tests/test-team/template-mgmt-api-tests/submit-routing-config.api.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from '@playwright/test'; +import { randomUUID } from 'node:crypto'; import { createAuthHelper, type TestUser, @@ -6,12 +7,15 @@ import { } from '../helpers/auth/cognito-auth-helper'; import { isoDateRegExp } from 'nhs-notify-web-template-management-test-helper-utils'; import { RoutingConfigStorageHelper } from '../helpers/db/routing-config-storage-helper'; +import { TemplateStorageHelper } from '../helpers/db/template-storage-helper'; import { RoutingConfigFactory } from '../helpers/factories/routing-config-factory'; +import { TemplateFactory } from '../helpers/factories/template-factory'; import { RoutingConfigStatus } from 'nhs-notify-backend-client'; test.describe('PATCH /v1/routing-configuration/:routingConfigId/submit', () => { const authHelper = createAuthHelper(); const storageHelper = new RoutingConfigStorageHelper(); + const templateStorageHelper = new TemplateStorageHelper(); let user1: TestUser; let userDifferentClient: TestUser; let userSharedClient: TestUser; @@ -28,6 +32,7 @@ test.describe('PATCH /v1/routing-configuration/:routingConfigId/submit', () => { test.afterAll(async () => { await storageHelper.deleteSeeded(); + await templateStorageHelper.deleteSeededTemplates(); }); test('returns 401 if no auth token', async ({ request }) => { @@ -97,7 +102,27 @@ test.describe('PATCH /v1/routing-configuration/:routingConfigId/submit', () => { test('returns 200 and the updated routing config data', async ({ request, }) => { - const { dbEntry, apiResponse } = RoutingConfigFactory.create(user1); + const templateId = randomUUID(); + + // Create a template for the routing config to reference + const template = TemplateFactory.createNhsAppTemplate( + templateId, + user1, + 'Test Template for Submit' + ); + + await templateStorageHelper.seedTemplateData([template]); + + const { dbEntry, apiResponse } = RoutingConfigFactory.create(user1, { + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'NHSAPP', + channelType: 'primary', + defaultTemplateId: templateId, + }, + ], + }); await storageHelper.seed([dbEntry]); @@ -193,7 +218,27 @@ test.describe('PATCH /v1/routing-configuration/:routingConfigId/submit', () => { test('user belonging to the same client as the creator can submit', async ({ request, }) => { - const { dbEntry, apiResponse } = RoutingConfigFactory.create(user1); + const templateId = randomUUID(); + + // Create a template for the routing config to reference + const template = TemplateFactory.createNhsAppTemplate( + templateId, + user1, + 'Test Template for Shared Client Submit' + ); + + await templateStorageHelper.seedTemplateData([template]); + + const { dbEntry, apiResponse } = RoutingConfigFactory.create(user1, { + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'NHSAPP', + channelType: 'primary', + defaultTemplateId: templateId, + }, + ], + }); await storageHelper.seed([dbEntry]); @@ -295,7 +340,409 @@ test.describe('PATCH /v1/routing-configuration/:routingConfigId/submit', () => { expect(await response.json()).toEqual({ statusCode: 409, technicalMessage: - 'Lock number mismatch - Message Plan has been modified since last read', + 'Lock number mismatch - Routing configuration has been modified since last read', + }); + }); + + test.describe('cascade validation', () => { + test('returns 400 if cascade item has no defaultTemplateId and no conditionalTemplates', async ({ + request, + }) => { + const { dbEntry } = RoutingConfigFactory.create(user1, { + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'NHSAPP', + channelType: 'primary', + defaultTemplateId: null, + }, + ], + }); + + await storageHelper.seed([dbEntry]); + + const response = await request.patch( + `${process.env.API_BASE_URL}/v1/routing-configuration/${dbEntry.id}/submit`, + { + headers: { + Authorization: await user1.getAccessToken(), + 'X-Lock-Number': String(dbEntry.lockNumber), + }, + } + ); + + expect(response.status()).toBe(400); + + expect(await response.json()).toEqual({ + statusCode: 400, + technicalMessage: + 'Routing config is not ready for submission: all cascade items must have templates assigned', + }); + }); + + test('returns 400 if cascade item has empty conditionalTemplates array', async ({ + request, + }) => { + const { dbEntry } = RoutingConfigFactory.create(user1, { + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'NHSAPP', + channelType: 'primary', + conditionalTemplates: [], + }, + ], + }); + + await storageHelper.seed([dbEntry]); + + const response = await request.patch( + `${process.env.API_BASE_URL}/v1/routing-configuration/${dbEntry.id}/submit`, + { + headers: { + Authorization: await user1.getAccessToken(), + 'X-Lock-Number': String(dbEntry.lockNumber), + }, + } + ); + + expect(response.status()).toBe(400); + + expect(await response.json()).toEqual({ + statusCode: 400, + technicalMessage: + 'Routing config is not ready for submission: all cascade items must have templates assigned', + }); + }); + + test('returns 200 if cascade item has conditionalTemplates instead of defaultTemplateId', async ({ + request, + }) => { + const frenchTemplateId = randomUUID(); + const arabicTemplateId = randomUUID(); + + // Create templates for conditional templates + const frenchTemplate = TemplateFactory.uploadLetterTemplate( + frenchTemplateId, + user1, + 'French Letter Template', + 'PROOF_APPROVED', + 'PASSED', + { language: 'fr' } + ); + + const arabicTemplate = TemplateFactory.uploadLetterTemplate( + arabicTemplateId, + user1, + 'Arabic Letter Template', + 'PROOF_APPROVED', + 'PASSED', + { language: 'ar' } + ); + + await templateStorageHelper.seedTemplateData([ + frenchTemplate, + arabicTemplate, + ]); + + const { dbEntry } = RoutingConfigFactory.create(user1, { + cascade: [ + { + cascadeGroups: ['translations'], + channel: 'LETTER', + channelType: 'primary', + // No defaultTemplateId - using conditionalTemplates instead + conditionalTemplates: [ + { language: 'fr', templateId: frenchTemplateId }, + { language: 'ar', templateId: arabicTemplateId }, + ], + }, + ], + cascadeGroupOverrides: [ + { name: 'translations', language: ['fr', 'ar'] }, + ], + }); + + await storageHelper.seed([dbEntry]); + + const response = await request.patch( + `${process.env.API_BASE_URL}/v1/routing-configuration/${dbEntry.id}/submit`, + { + headers: { + Authorization: await user1.getAccessToken(), + 'X-Lock-Number': String(dbEntry.lockNumber), + }, + } + ); + + expect(response.status()).toBe(200); + + const responseBody = await response.json(); + expect(responseBody.data.status).toBe('COMPLETED'); + }); + }); + + test('returns 400 if referenced template does not exist', async ({ + request, + }) => { + const nonExistentTemplateId = 'non-existent-template-id'; + + const { dbEntry } = RoutingConfigFactory.create(user1, { + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'NHSAPP', + channelType: 'primary', + defaultTemplateId: nonExistentTemplateId, + }, + ], + }); + + await storageHelper.seed([dbEntry]); + + const response = await request.patch( + `${process.env.API_BASE_URL}/v1/routing-configuration/${dbEntry.id}/submit`, + { + headers: { + Authorization: await user1.getAccessToken(), + 'X-Lock-Number': String(dbEntry.lockNumber), + }, + } + ); + + expect(response.status()).toBe(400); + + expect(await response.json()).toEqual({ + statusCode: 400, + technicalMessage: 'Some templates not found', + details: { + templateIds: nonExistentTemplateId, + }, }); }); + + test('returns 400 if referenced template has DELETED status', async ({ + request, + }) => { + const deletedTemplateId = randomUUID(); + + // Create a template with DELETED status + const deletedTemplate = TemplateFactory.createNhsAppTemplate( + deletedTemplateId, + user1, + 'Deleted Template' + ); + deletedTemplate.templateStatus = 'DELETED'; + + await templateStorageHelper.seedTemplateData([deletedTemplate]); + + const { dbEntry } = RoutingConfigFactory.create(user1, { + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'NHSAPP', + channelType: 'primary', + defaultTemplateId: deletedTemplateId, + }, + ], + }); + + await storageHelper.seed([dbEntry]); + + const response = await request.patch( + `${process.env.API_BASE_URL}/v1/routing-configuration/${dbEntry.id}/submit`, + { + headers: { + Authorization: await user1.getAccessToken(), + 'X-Lock-Number': String(dbEntry.lockNumber), + }, + } + ); + + expect(response.status()).toBe(400); + + expect(await response.json()).toEqual({ + statusCode: 400, + technicalMessage: 'Some templates not found', + details: { + templateIds: deletedTemplateId, + }, + }); + }); + + test('returns 400 if LETTER template has status NOT_YET_SUBMITTED', async ({ + request, + }) => { + const letterTemplateId = randomUUID(); + + // Create a LETTER template with NOT_YET_SUBMITTED status + const letterTemplate = TemplateFactory.uploadLetterTemplate( + letterTemplateId, + user1, + 'Test Letter Template', + 'NOT_YET_SUBMITTED' + ); + + await templateStorageHelper.seedTemplateData([letterTemplate]); + + const { dbEntry } = RoutingConfigFactory.create(user1, { + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'LETTER', + channelType: 'primary', + defaultTemplateId: letterTemplateId, + }, + ], + }); + + await storageHelper.seed([dbEntry]); + + const response = await request.patch( + `${process.env.API_BASE_URL}/v1/routing-configuration/${dbEntry.id}/submit`, + { + headers: { + Authorization: await user1.getAccessToken(), + 'X-Lock-Number': String(dbEntry.lockNumber), + }, + } + ); + + expect(response.status()).toBe(400); + + expect(await response.json()).toEqual({ + statusCode: 400, + technicalMessage: + 'Letter templates must have status PROOF_APPROVED or SUBMITTED', + details: { + templateIds: letterTemplateId, + }, + }); + }); + + test('Template statuses are updated on routing config submit', async ({ + request, + }) => { + const appTemplateId = randomUUID(); + const emailTemplateId = randomUUID(); + const englishTemplateId = randomUUID(); + const frenchTemplateId = randomUUID(); + + const nhsAppTemplate = TemplateFactory.createNhsAppTemplate( + appTemplateId, + user1, + 'Nhs app template' + ); + nhsAppTemplate.templateStatus = 'NOT_YET_SUBMITTED'; + nhsAppTemplate.lockNumber = 1; + + const emailTemplate = TemplateFactory.createEmailTemplate( + emailTemplateId, + user1, + 'Email Template' + ); + emailTemplate.templateStatus = 'SUBMITTED'; + emailTemplate.lockNumber = 2; + + const englishTemplate = TemplateFactory.createAuthoringLetterTemplate( + englishTemplateId, + user1, + 'English Template' + ); + englishTemplate.templateStatus = 'PROOF_APPROVED'; + englishTemplate.lockNumber = 3; + + const frenchTemplate = TemplateFactory.createAuthoringLetterTemplate( + frenchTemplateId, + user1, + 'French Template' + ); + frenchTemplate.templateStatus = 'PROOF_APPROVED'; + frenchTemplate.lockNumber = 4; + + await templateStorageHelper.seedTemplateData([ + nhsAppTemplate, + emailTemplate, + englishTemplate, + frenchTemplate, + ]); + + const startingEmailTemplate = await templateStorageHelper.getTemplate({ + clientId: user1.clientId, + templateId: emailTemplateId, + }); + + const { dbEntry } = RoutingConfigFactory.create(user1, { + cascade: [ + { + cascadeGroups: ['standard'], + channel: 'NHSAPP', + channelType: 'primary', + defaultTemplateId: appTemplateId, + }, + { + cascadeGroups: ['standard'], + channel: 'EMAIL', + channelType: 'primary', + defaultTemplateId: emailTemplateId, + }, + { + cascadeGroups: ['translations'], + channel: 'LETTER', + channelType: 'primary', + conditionalTemplates: [ + { language: 'en', templateId: englishTemplateId }, + { language: 'fr', templateId: frenchTemplateId }, + ], + }, + ], + }); + + await storageHelper.seed([dbEntry]); + + const response = await request.patch( + `${process.env.API_BASE_URL}/v1/routing-configuration/${dbEntry.id}/submit`, + { + headers: { + Authorization: await user1.getAccessToken(), + 'X-Lock-Number': String(dbEntry.lockNumber), + }, + } + ); + + expect(response.status()).toBe(200); + const responseBody = await response.json(); + expect(responseBody.data.status).toBe('COMPLETED'); + + const dbAppTemplate = await templateStorageHelper.getTemplate({ + clientId: user1.clientId, + templateId: appTemplateId, + }); + const dbEmailTemplate = await templateStorageHelper.getTemplate({ + clientId: user1.clientId, + templateId: emailTemplateId, + }); + const dbEnglishTemplate = await templateStorageHelper.getTemplate({ + clientId: user1.clientId, + templateId: englishTemplateId, + }); + const dbFrenchTemplate = await templateStorageHelper.getTemplate({ + clientId: user1.clientId, + templateId: frenchTemplateId, + }); + + expect(dbAppTemplate.templateStatus).toBe('SUBMITTED'); + expect(dbAppTemplate.lockNumber).toBe(2); + + // email was already submitted so should not have changed + expect(dbEmailTemplate.templateStatus).toBe('SUBMITTED'); + expect(dbEmailTemplate.lockNumber).toBe(2); + expect(dbEmailTemplate.updatedAt).toEqual(startingEmailTemplate.updatedAt); + + expect(dbEnglishTemplate.templateStatus).toBe('SUBMITTED'); + expect(dbEnglishTemplate.lockNumber).toBe(4); + + expect(dbFrenchTemplate.templateStatus).toBe('SUBMITTED'); + expect(dbFrenchTemplate.lockNumber).toBe(5); + }); }); diff --git a/tests/test-team/template-mgmt-api-tests/update-routing-config.api.spec.ts b/tests/test-team/template-mgmt-api-tests/update-routing-config.api.spec.ts index 0fbd7553e..59bb1c974 100644 --- a/tests/test-team/template-mgmt-api-tests/update-routing-config.api.spec.ts +++ b/tests/test-team/template-mgmt-api-tests/update-routing-config.api.spec.ts @@ -720,7 +720,7 @@ test.describe('PATCH /v1/routing-configuration/:routingConfigId', () => { expect(await response.json()).toEqual({ statusCode: 409, technicalMessage: - 'Lock number mismatch - Message Plan has been modified since last read', + 'Lock number mismatch - Routing configuration has been modified since last read', }); }); }); diff --git a/tests/test-team/template-mgmt-event-tests/routing-config.event.spec.ts b/tests/test-team/template-mgmt-event-tests/routing-config.event.spec.ts index 8bbbffaef..75c13f23e 100644 --- a/tests/test-team/template-mgmt-event-tests/routing-config.event.spec.ts +++ b/tests/test-team/template-mgmt-event-tests/routing-config.event.spec.ts @@ -1,3 +1,4 @@ +import { randomUUID } from 'node:crypto'; import { test, expect } from '@playwright/test'; import { createAuthHelper, @@ -7,10 +8,13 @@ import { import { EventCacheHelper } from '../helpers/events/event-cache-helper'; import { RoutingConfigStorageHelper } from 'helpers/db/routing-config-storage-helper'; import { RoutingConfigFactory } from 'helpers/factories/routing-config-factory'; +import { TemplateFactory } from 'helpers/factories/template-factory'; +import { TemplateStorageHelper } from 'helpers/db/template-storage-helper'; test.describe('Event publishing - Routing Config', () => { const authHelper = createAuthHelper(); const storageHelper = new RoutingConfigStorageHelper(); + const templateStorageHelper = new TemplateStorageHelper(); const eventCacheHelper = new EventCacheHelper(); let user: TestUser; @@ -21,6 +25,7 @@ test.describe('Event publishing - Routing Config', () => { test.afterAll(async () => { await storageHelper.deleteSeeded(); + await templateStorageHelper.deleteSeededTemplates(); }); test('Expect a draft event and a deleted event when some template IDs are null', async ({ @@ -170,13 +175,23 @@ test.describe('Event publishing - Routing Config', () => { }); test('Expect a draft event and a completed event', async ({ request }) => { + const templateId = randomUUID(); + + const template = TemplateFactory.createNhsAppTemplate( + templateId, + user, + 'Test Template for Submit' + ); + + await templateStorageHelper.seedTemplateData([template]); + const payload = RoutingConfigFactory.create(user, { cascade: [ { cascadeGroups: ['standard'], channel: 'NHSAPP', channelType: 'primary', - defaultTemplateId: 'b1854a33-fc1b-4e7d-99d0-6f7b92b8c530', + defaultTemplateId: templateId, }, ], }).apiPayload;