Skip to content

Commit 3d8593f

Browse files
harrim91harrim91
committed
CCM-12327: locking on submit
1 parent 56c79ca commit 3d8593f

File tree

6 files changed

+176
-31
lines changed

6 files changed

+176
-31
lines changed

‎lambdas/backend-api/src/__tests__/templates/api/submit.test.ts‎

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ describe('Template API - Submit', () => {
2929
const event = mock<APIGatewayProxyEvent>({
3030
requestContext: { authorizer: ctx },
3131
pathParameters: { templateId: 'id' },
32+
headers: { 'X-Lock-Number': '0' },
3233
});
3334

3435
const result = await handler(event, mock<Context>(), jest.fn());
@@ -54,6 +55,7 @@ describe('Template API - Submit', () => {
5455
},
5556
body: JSON.stringify({ name: 'test' }),
5657
pathParameters: { templateId: undefined },
58+
headers: { 'X-Lock-Number': '0' },
5759
});
5860

5961
const result = await handler(event, mock<Context>(), jest.fn());
@@ -86,6 +88,7 @@ describe('Template API - Submit', () => {
8688
authorizer: { user: 'sub', clientId: 'nhs-notify-client-id' },
8789
},
8890
pathParameters: { templateId: '1-2-3' },
91+
headers: { 'X-Lock-Number': '0' },
8992
});
9093

9194
const result = await handler(event, mock<Context>(), jest.fn());
@@ -98,10 +101,53 @@ describe('Template API - Submit', () => {
98101
}),
99102
});
100103

101-
expect(mocks.templateClient.submitTemplate).toHaveBeenCalledWith('1-2-3', {
102-
userId: 'sub',
103-
clientId: 'nhs-notify-client-id',
104+
expect(mocks.templateClient.submitTemplate).toHaveBeenCalledWith(
105+
'1-2-3',
106+
{
107+
userId: 'sub',
108+
clientId: 'nhs-notify-client-id',
109+
},
110+
'0'
111+
);
112+
});
113+
114+
test('should coerce missing lock number header to empty string', async () => {
115+
const { handler, mocks } = setup();
116+
117+
mocks.templateClient.submitTemplate.mockResolvedValueOnce({
118+
error: {
119+
errorMeta: {
120+
code: 409,
121+
description: 'Invalid lock number',
122+
},
123+
},
124+
});
125+
126+
const event = mock<APIGatewayProxyEvent>({
127+
requestContext: {
128+
authorizer: { user: 'sub', clientId: 'nhs-notify-client-id' },
129+
},
130+
pathParameters: { templateId: '1-2-3' },
131+
});
132+
133+
const result = await handler(event, mock<Context>(), jest.fn());
134+
135+
expect(result).toEqual({
136+
statusCode: 409,
137+
body: JSON.stringify({
138+
statusCode: 409,
139+
technicalMessage: 'Invalid lock number',
140+
}),
104141
});
142+
143+
expect(mocks.templateClient.submitTemplate).toHaveBeenCalledWith(
144+
'1-2-3',
145+
{
146+
userId: 'sub',
147+
clientId: 'nhs-notify-client-id',
148+
},
149+
''
150+
);
105151
});
106152

107153
test('should return template', async () => {
@@ -127,6 +173,7 @@ describe('Template API - Submit', () => {
127173
authorizer: { user: 'sub', clientId: 'nhs-notify-client-id' },
128174
},
129175
pathParameters: { templateId: '1-2-3' },
176+
headers: { 'X-Lock-Number': '0' },
130177
});
131178

132179
const result = await handler(event, mock<Context>(), jest.fn());
@@ -136,9 +183,13 @@ describe('Template API - Submit', () => {
136183
body: JSON.stringify({ statusCode: 200, data: response }),
137184
});
138185

139-
expect(mocks.templateClient.submitTemplate).toHaveBeenCalledWith('1-2-3', {
140-
userId: 'sub',
141-
clientId: 'nhs-notify-client-id',
142-
});
186+
expect(mocks.templateClient.submitTemplate).toHaveBeenCalledWith(
187+
'1-2-3',
188+
{
189+
userId: 'sub',
190+
clientId: 'nhs-notify-client-id',
191+
},
192+
'0'
193+
);
143194
});
144195
});

‎lambdas/backend-api/src/__tests__/templates/app/template-client.test.ts‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1664,6 +1664,23 @@ describe('templateClient', () => {
16641664
});
16651665

16661666
describe('submitTemplate', () => {
1667+
test('returns failure result when lock number is invalid', async () => {
1668+
const { templateClient, mocks } = setup();
1669+
1670+
const result = await templateClient.submitTemplate(templateId, user, '');
1671+
1672+
expect(mocks.templateRepository.submit).not.toHaveBeenCalled();
1673+
1674+
expect(result).toEqual({
1675+
error: {
1676+
errorMeta: {
1677+
code: 409,
1678+
description: 'Invalid lock number',
1679+
},
1680+
},
1681+
});
1682+
});
1683+
16671684
test('submitTemplate should return a failure result, when saving to the database unexpectedly fails', async () => {
16681685
const { templateClient, mocks } = setup();
16691686

@@ -1676,11 +1693,12 @@ describe('templateClient', () => {
16761693
},
16771694
});
16781695

1679-
const result = await templateClient.submitTemplate(templateId, user);
1696+
const result = await templateClient.submitTemplate(templateId, user, 0);
16801697

16811698
expect(mocks.templateRepository.submit).toHaveBeenCalledWith(
16821699
templateId,
1683-
user
1700+
user,
1701+
0
16841702
);
16851703

16861704
expect(result).toEqual({
@@ -1717,11 +1735,12 @@ describe('templateClient', () => {
17171735
data: template,
17181736
});
17191737

1720-
const result = await templateClient.submitTemplate(templateId, user);
1738+
const result = await templateClient.submitTemplate(templateId, user, 0);
17211739

17221740
expect(mocks.templateRepository.submit).toHaveBeenCalledWith(
17231741
templateId,
1724-
user
1742+
user,
1743+
0
17251744
);
17261745

17271746
expect(result).toEqual({
@@ -1752,11 +1771,12 @@ describe('templateClient', () => {
17521771
data: { ...template, owner: user.userId, version: 1 },
17531772
});
17541773

1755-
const result = await templateClient.submitTemplate(templateId, user);
1774+
const result = await templateClient.submitTemplate(templateId, user, 0);
17561775

17571776
expect(mocks.templateRepository.submit).toHaveBeenCalledWith(
17581777
templateId,
1759-
user
1778+
user,
1779+
0
17601780
);
17611781

17621782
expect(result).toEqual({

‎lambdas/backend-api/src/__tests__/templates/infra/template-repository.test.ts‎

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,7 @@ describe('templateRepository', () => {
697697
describe('submit', () => {
698698
test.each([
699699
{
700+
testName: 'When template does not exist',
700701
Item: undefined,
701702
code: 404,
702703
message: 'Template not found',
@@ -707,6 +708,7 @@ describe('templateRepository', () => {
707708
Item: marshall({
708709
templateType: 'EMAIL',
709710
templateStatus: 'SUBMITTED',
711+
lockNumber: 0,
710712
}),
711713
code: 400,
712714
message: 'Template with status SUBMITTED cannot be updated',
@@ -717,6 +719,7 @@ describe('templateRepository', () => {
717719
Item: marshall({
718720
templateType: 'LETTER',
719721
templateStatus: 'PENDING_UPLOAD',
722+
lockNumber: 0,
720723
}),
721724
code: 400,
722725
message: 'Template cannot be submitted',
@@ -727,6 +730,7 @@ describe('templateRepository', () => {
727730
Item: marshall({
728731
templateType: 'LETTER',
729732
templateStatus: 'PENDING_VALIDATION',
733+
lockNumber: 0,
730734
}),
731735
code: 400,
732736
message: 'Template cannot be submitted',
@@ -737,6 +741,7 @@ describe('templateRepository', () => {
737741
Item: marshall({
738742
templateType: 'EMAIL',
739743
templateStatus: 'DELETED',
744+
lockNumber: 0,
740745
}),
741746
code: 404,
742747
message: 'Template not found',
@@ -747,6 +752,7 @@ describe('templateRepository', () => {
747752
Item: marshall({
748753
templateType: 'LETTER',
749754
templateStatus: 'NOT_YET_SUBMITTED',
755+
lockNumber: 0,
750756
files: {
751757
pdfTemplate: {
752758
virusScanStatus: 'PASSED',
@@ -763,6 +769,16 @@ describe('templateRepository', () => {
763769
code: 400,
764770
message: 'Template cannot be submitted',
765771
},
772+
{
773+
testName: 'Fails when stored lock number differs from input',
774+
Item: marshall({
775+
templateType: 'SMS',
776+
templateStatus: 'NOT_YET_SUBMITTED',
777+
lockNumber: 1,
778+
}),
779+
code: 409,
780+
message: 'Invalid lock number',
781+
},
766782
])('submit: $testName', async ({ Item, code, message }) => {
767783
const { templateRepository, mocks } = setup();
768784

@@ -776,7 +792,8 @@ describe('templateRepository', () => {
776792

777793
const response = await templateRepository.submit(
778794
'abc-def-ghi-jkl-123',
779-
user
795+
user,
796+
0
780797
);
781798

782799
expect(response).toEqual({
@@ -799,7 +816,8 @@ describe('templateRepository', () => {
799816

800817
const response = await templateRepository.submit(
801818
'abc-def-ghi-jkl-123',
802-
user
819+
user,
820+
0
803821
);
804822

805823
expect(response).toEqual({
@@ -827,6 +845,7 @@ describe('templateRepository', () => {
827845
templateType: 'NHS_APP',
828846
updatedAt: 'now',
829847
createdAt: 'yesterday',
848+
lockNumber: 1,
830849
};
831850

832851
mocks.ddbDocClient
@@ -836,11 +855,41 @@ describe('templateRepository', () => {
836855
})
837856
.resolves({ Attributes: databaseTemplate });
838857

839-
const response = await templateRepository.submit(id, user);
858+
const response = await templateRepository.submit(id, user, 0);
840859

841860
expect(response).toEqual({
842861
data: databaseTemplate,
843862
});
863+
864+
expect(mocks.ddbDocClient).toHaveReceivedCommandWith(UpdateCommand, {
865+
TableName: 'templates',
866+
Key: { id: 'abc-def-ghi-jkl-123', owner: 'CLIENT#client-id' },
867+
ReturnValues: 'ALL_NEW',
868+
ReturnValuesOnConditionCheckFailure: 'ALL_OLD',
869+
ExpressionAttributeNames: {
870+
'#lockNumber': 'lockNumber',
871+
'#templateStatus': 'templateStatus',
872+
'#updatedAt': 'updatedAt',
873+
'#updatedBy': 'updatedBy',
874+
},
875+
ExpressionAttributeValues: {
876+
':deleted': 'DELETED',
877+
':expectedLetterStatus': 'PROOF_AVAILABLE',
878+
':expectedLockNumber': 0,
879+
':expectedStatus': 'NOT_YET_SUBMITTED',
880+
':lockNumberIncrement': 1,
881+
':newStatus': 'SUBMITTED',
882+
':passed': 'PASSED',
883+
':submitted': 'SUBMITTED',
884+
':updatedAt': '2024-12-27T00:00:00.000Z',
885+
':updatedBy': 'user-id',
886+
},
887+
888+
UpdateExpression:
889+
'SET #templateStatus = :newStatus, #updatedAt = :updatedAt, #updatedBy = :updatedBy ADD #lockNumber :lockNumberIncrement',
890+
ConditionExpression:
891+
'attribute_exists(id) AND NOT #templateStatus IN (:deleted, :submitted) AND (attribute_not_exists(files.pdfTemplate) OR files.pdfTemplate.virusScanStatus = :passed) AND (attribute_not_exists(files.testDataCsv) OR files.testDataCsv.virusScanStatus = :passed) AND (#templateStatus = :expectedStatus OR #templateStatus = :expectedLetterStatus) AND (attribute_not_exists(#lockNumber) OR #lockNumber = :expectedLockNumber)',
892+
});
844893
});
845894
});
846895

‎lambdas/backend-api/src/templates/api/submit.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,14 @@ export function createHandler({
1919
return apiFailure(400, 'Invalid request');
2020
}
2121

22-
const { data, error } = await templateClient.submitTemplate(templateId, {
23-
userId,
24-
clientId,
25-
});
22+
const { data, error } = await templateClient.submitTemplate(
23+
templateId,
24+
{
25+
userId,
26+
clientId,
27+
},
28+
event.headers['X-Lock-Number'] ?? ''
29+
);
2630

2731
if (error) {
2832
return apiFailure(

‎lambdas/backend-api/src/templates/app/template-client.ts‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -303,11 +303,27 @@ export class TemplateClient {
303303

304304
async submitTemplate(
305305
templateId: string,
306-
user: User
306+
user: User,
307+
lockNumber: number | string
307308
): Promise<Result<TemplateDto>> {
308309
const log = this.logger.child({ templateId, user });
309310

310-
const submitResult = await this.templateRepository.submit(templateId, user);
311+
const lockNumberValidation = $LockNumber.safeParse(lockNumber);
312+
313+
if (!lockNumberValidation.success) {
314+
log.error(
315+
'Lock number failed validation',
316+
z.treeifyError(lockNumberValidation.error)
317+
);
318+
319+
return failure(ErrorCase.CONFLICT, 'Invalid lock number');
320+
}
321+
322+
const submitResult = await this.templateRepository.submit(
323+
templateId,
324+
user,
325+
lockNumber
326+
);
311327

312328
if (submitResult.error) {
313329
log

0 commit comments

Comments
 (0)