Skip to content

Commit 56c79ca

Browse files
harrim91harrim91
committed
CCM-12327: locking in request proof endpoint
1 parent eb6eefd commit 56c79ca

File tree

7 files changed

+168
-32
lines changed

7 files changed

+168
-32
lines changed

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

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ describe('Template API - request proof', () => {
2727
const event = mock<APIGatewayProxyEvent>({
2828
requestContext: { authorizer: ctx },
2929
pathParameters: { templateId: 'id' },
30+
headers: { 'X-Lock-Number': '0' },
3031
});
3132

3233
const result = await handler(event, mock<Context>(), jest.fn());
@@ -51,6 +52,7 @@ describe('Template API - request proof', () => {
5152
authorizer: { user: 'sub', clientId: 'nhs-notify-client-id' },
5253
},
5354
pathParameters: { templateId: undefined },
55+
headers: { 'X-Lock-Number': '0' },
5456
});
5557

5658
const result = await handler(event, mock<Context>(), jest.fn());
@@ -83,6 +85,7 @@ describe('Template API - request proof', () => {
8385
authorizer: { user: 'sub', clientId: 'nhs-notify-client-id' },
8486
},
8587
pathParameters: { templateId: 'template-id' },
88+
headers: { 'X-Lock-Number': '0' },
8689
});
8790

8891
const result = await handler(event, mock<Context>(), jest.fn());
@@ -97,7 +100,44 @@ describe('Template API - request proof', () => {
97100

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

@@ -137,6 +177,7 @@ describe('Template API - request proof', () => {
137177
authorizer: { user: 'sub', clientId: 'notify-client-id' },
138178
},
139179
pathParameters: { templateId: 'id' },
180+
headers: { 'X-Lock-Number': '0' },
140181
});
141182

142183
const result = await handler(event, mock<Context>(), jest.fn());
@@ -146,9 +187,13 @@ describe('Template API - request proof', () => {
146187
body: JSON.stringify({ statusCode: 200, data: response }),
147188
});
148189

149-
expect(mocks.templateClient.requestProof).toHaveBeenCalledWith('id', {
150-
userId: 'sub',
151-
clientId: 'notify-client-id',
152-
});
190+
expect(mocks.templateClient.requestProof).toHaveBeenCalledWith(
191+
'id',
192+
{
193+
userId: 'sub',
194+
clientId: 'notify-client-id',
195+
},
196+
'0'
197+
);
153198
});
154199
});

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

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1766,14 +1766,33 @@ describe('templateClient', () => {
17661766
});
17671767

17681768
describe('requestProof', () => {
1769+
test('returns failure result when lock number is invalid', async () => {
1770+
const { templateClient, mocks } = setup();
1771+
1772+
const result = await templateClient.requestProof(templateId, user, '');
1773+
1774+
expect(
1775+
mocks.templateRepository.proofRequestUpdate
1776+
).not.toHaveBeenCalled();
1777+
1778+
expect(result).toEqual({
1779+
error: {
1780+
errorMeta: {
1781+
code: 409,
1782+
description: 'Invalid lock number',
1783+
},
1784+
},
1785+
});
1786+
});
1787+
17691788
test('should return a failure result, when proofing is disabled', async () => {
17701789
const { templateClient, mocks } = setup();
17711790

17721791
mocks.clientConfigRepository.get.mockResolvedValueOnce({
17731792
data: { features: { proofing: false } },
17741793
});
17751794

1776-
const result = await templateClient.requestProof(templateId, user);
1795+
const result = await templateClient.requestProof(templateId, user, 1);
17771796

17781797
expect(
17791798
mocks.templateRepository.proofRequestUpdate
@@ -1796,7 +1815,7 @@ describe('templateClient', () => {
17961815
error: { errorMeta: { description: 'err', code: 500 } },
17971816
});
17981817

1799-
const result = await templateClient.requestProof(templateId, user);
1818+
const result = await templateClient.requestProof(templateId, user, 1);
18001819

18011820
expect(
18021821
mocks.templateRepository.proofRequestUpdate
@@ -1840,11 +1859,12 @@ describe('templateClient', () => {
18401859
},
18411860
});
18421861

1843-
const result = await templateClient.requestProof(templateId, user);
1862+
const result = await templateClient.requestProof(templateId, user, 1);
18441863

18451864
expect(mocks.templateRepository.proofRequestUpdate).toHaveBeenCalledWith(
18461865
templateId,
1847-
user
1866+
user,
1867+
1
18481868
);
18491869

18501870
expect(result).toEqual({
@@ -1902,11 +1922,12 @@ describe('templateClient', () => {
19021922
},
19031923
});
19041924

1905-
const result = await templateClient.requestProof(templateId, user);
1925+
const result = await templateClient.requestProof(templateId, user, 1);
19061926

19071927
expect(mocks.templateRepository.proofRequestUpdate).toHaveBeenCalledWith(
19081928
templateId,
1909-
user
1929+
user,
1930+
1
19101931
);
19111932

19121933
expect(result).toEqual({
@@ -1954,11 +1975,12 @@ describe('templateClient', () => {
19541975
},
19551976
});
19561977

1957-
const result = await templateClient.requestProof(templateId, user);
1978+
const result = await templateClient.requestProof(templateId, user, 1);
19581979

19591980
expect(mocks.templateRepository.proofRequestUpdate).toHaveBeenCalledWith(
19601981
templateId,
1961-
user
1982+
user,
1983+
1
19621984
);
19631985

19641986
expect(result).toEqual({
@@ -2023,11 +2045,12 @@ describe('templateClient', () => {
20232045
},
20242046
});
20252047

2026-
const result = await templateClient.requestProof(templateId, user);
2048+
const result = await templateClient.requestProof(templateId, user, 1);
20272049

20282050
expect(mocks.templateRepository.proofRequestUpdate).toHaveBeenCalledWith(
20292051
templateId,
2030-
user
2052+
user,
2053+
1
20312054
);
20322055

20332056
expect(mocks.queueMock.send).toHaveBeenCalledTimes(1);
@@ -2098,11 +2121,12 @@ describe('templateClient', () => {
20982121

20992122
mocks.queueMock.send.mockResolvedValueOnce({ data: { $metadata: {} } });
21002123

2101-
const result = await templateClient.requestProof(templateId, user);
2124+
const result = await templateClient.requestProof(templateId, user, 1);
21022125

21032126
expect(mocks.templateRepository.proofRequestUpdate).toHaveBeenCalledWith(
21042127
templateId,
2105-
user
2128+
user,
2129+
1
21062130
);
21072131

21082132
expect(mocks.queueMock.send).toHaveBeenCalledTimes(1);

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

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1734,14 +1734,15 @@ describe('templateRepository', () => {
17341734

17351735
const result = await templateRepository.proofRequestUpdate(
17361736
'template-id',
1737-
user
1737+
user,
1738+
0
17381739
);
17391740

17401741
expect(result).toEqual({ data: { id: 'template-id' } });
17411742

17421743
expect(mocks.ddbDocClient).toHaveReceivedCommandWith(UpdateCommand, {
17431744
ConditionExpression:
1744-
'#templateStatus = :condition_1_templateStatus AND #templateType = :condition_2_templateType AND #clientId = :condition_3_clientId AND #proofingEnabled = :condition_4_proofingEnabled',
1745+
'#templateStatus = :condition_1_templateStatus AND #templateType = :condition_2_templateType AND #clientId = :condition_3_clientId AND #proofingEnabled = :condition_4_proofingEnabled AND (#lockNumber = :condition_5_1_lockNumber OR attribute_not_exists (#lockNumber))',
17451746
ExpressionAttributeNames: {
17461747
'#clientId': 'clientId',
17471748
'#templateStatus': 'templateStatus',
@@ -1750,12 +1751,15 @@ describe('templateRepository', () => {
17501751
'#updatedBy': 'updatedBy',
17511752
'#proofingEnabled': 'proofingEnabled',
17521753
'#supplierReferences': 'supplierReferences',
1754+
'#lockNumber': 'lockNumber',
17531755
},
17541756
ExpressionAttributeValues: {
17551757
':condition_1_templateStatus': 'PENDING_PROOF_REQUEST',
17561758
':condition_2_templateType': 'LETTER',
17571759
':condition_3_clientId': clientId,
17581760
':condition_4_proofingEnabled': true,
1761+
':condition_5_1_lockNumber': 0,
1762+
':lockNumber': 1,
17591763
':templateStatus': 'WAITING_FOR_PROOF',
17601764
':updatedAt': '2024-12-27T00:00:00.000Z',
17611765
':updatedBy': userId,
@@ -1766,7 +1770,7 @@ describe('templateRepository', () => {
17661770
ReturnValuesOnConditionCheckFailure: 'ALL_OLD',
17671771
TableName: 'templates',
17681772
UpdateExpression:
1769-
'SET #templateStatus = :templateStatus, #updatedAt = :updatedAt, #updatedBy = :updatedBy, #supplierReferences = if_not_exists(#supplierReferences, :supplierReferences)',
1773+
'SET #templateStatus = :templateStatus, #updatedAt = :updatedAt, #updatedBy = :updatedBy, #supplierReferences = if_not_exists(#supplierReferences, :supplierReferences) ADD #lockNumber :lockNumber',
17701774
});
17711775
});
17721776

@@ -1782,7 +1786,8 @@ describe('templateRepository', () => {
17821786

17831787
const result = await templateRepository.proofRequestUpdate(
17841788
'template-id',
1785-
user
1789+
user,
1790+
0
17861791
);
17871792

17881793
expect(result).toEqual({
@@ -1796,6 +1801,37 @@ describe('templateRepository', () => {
17961801
});
17971802
});
17981803

1804+
it('returns 409 error response when conditional check fails when lock numbers are different', async () => {
1805+
const { templateRepository, mocks } = setup();
1806+
1807+
const err = new ConditionalCheckFailedException({
1808+
message: 'condition check failed',
1809+
$metadata: {},
1810+
Item: {
1811+
templateStatus: { S: 'PENDING_UPLOAD' },
1812+
lockNumber: { N: '1' },
1813+
},
1814+
});
1815+
1816+
mocks.ddbDocClient.on(UpdateCommand).rejectsOnce(err);
1817+
1818+
const result = await templateRepository.proofRequestUpdate(
1819+
'template-id',
1820+
user,
1821+
0
1822+
);
1823+
1824+
expect(result).toEqual({
1825+
error: {
1826+
actualError: err,
1827+
errorMeta: {
1828+
code: 409,
1829+
description: 'Invalid lock number',
1830+
},
1831+
},
1832+
});
1833+
});
1834+
17991835
it('returns 400 error response when conditional check fails, but item exists, with a status other than DELETED or PENDING_PROOF_REQUEST', async () => {
18001836
const { templateRepository, mocks } = setup();
18011837

@@ -1804,14 +1840,16 @@ describe('templateRepository', () => {
18041840
$metadata: {},
18051841
Item: {
18061842
templateStatus: { S: 'PENDING_UPLOAD' },
1843+
lockNumber: { N: '0' },
18071844
},
18081845
});
18091846

18101847
mocks.ddbDocClient.on(UpdateCommand).rejectsOnce(err);
18111848

18121849
const result = await templateRepository.proofRequestUpdate(
18131850
'template-id',
1814-
user
1851+
user,
1852+
0
18151853
);
18161854

18171855
expect(result).toEqual({
@@ -1834,7 +1872,8 @@ describe('templateRepository', () => {
18341872

18351873
const result = await templateRepository.proofRequestUpdate(
18361874
'template-id',
1837-
user
1875+
user,
1876+
0
18381877
);
18391878

18401879
expect(result).toEqual({

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

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

19-
const { data, error } = await templateClient.requestProof(templateId, {
20-
userId,
21-
clientId,
22-
});
19+
const { data, error } = await templateClient.requestProof(
20+
templateId,
21+
{
22+
userId,
23+
clientId,
24+
},
25+
event.headers['X-Lock-Number'] ?? ''
26+
);
2327

2428
if (error) {
2529
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
@@ -419,10 +419,22 @@ export class TemplateClient {
419419

420420
async requestProof(
421421
templateId: string,
422-
user: User
422+
user: User,
423+
lockNumber: number | string
423424
): Promise<Result<TemplateDto>> {
424425
const log = this.logger.child({ templateId, user });
425426

427+
const lockNumberValidation = $LockNumber.safeParse(lockNumber);
428+
429+
if (!lockNumberValidation.success) {
430+
log.error(
431+
'Lock number failed validation',
432+
z.treeifyError(lockNumberValidation.error)
433+
);
434+
435+
return failure(ErrorCase.CONFLICT, 'Invalid lock number');
436+
}
437+
426438
const clientConfigurationResult = await this.clientConfigRepository.get(
427439
user.clientId
428440
);
@@ -451,7 +463,11 @@ export class TemplateClient {
451463
}
452464

453465
const proofRequestUpdateResult =
454-
await this.templateRepository.proofRequestUpdate(templateId, user);
466+
await this.templateRepository.proofRequestUpdate(
467+
templateId,
468+
user,
469+
lockNumberValidation.data
470+
);
455471

456472
if (proofRequestUpdateResult.error) {
457473
log

0 commit comments

Comments
 (0)