Skip to content

Commit 1d9a7ff

Browse files
committed
CCM-10547: add client level check for toggling PENDING_PROOF_REQUEST
1 parent c1f5765 commit 1d9a7ff

File tree

7 files changed

+129
-57
lines changed

7 files changed

+129
-57
lines changed

lambdas/backend-api/src/__tests__/templates/api/validate-letter-template-files.test.ts

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@ import {
1717
$GuardDutyMalwareScanStatusFailed,
1818
DatabaseTemplate,
1919
} from 'nhs-notify-web-template-management-utils';
20+
import { ClientConfigRepository } from '@backend-api/templates/infra/client-config-repository';
2021

2122
jest.mock('@backend-api/templates/domain/template-pdf');
2223
jest.mock('@backend-api/templates/domain/test-data-csv');
2324
jest.mock('@backend-api/templates/domain/validate-letter-template-files');
2425
jest.mock('nhs-notify-web-template-management-utils/logger');
26+
jest.mock('@backend-api/templates/infra/client-config-repository');
2527
jest.mocked(logger).child.mockReturnThis();
2628

2729
const versionId = 'template-version-id';
@@ -35,6 +37,7 @@ function setup() {
3537
TemplatePdf: jest.mocked(TemplatePdf),
3638
TestDataCsv: jest.mocked(TestDataCsv),
3739
validateLetterTemplateFiles: jest.mocked(validateLetterTemplateFiles),
40+
clientConfigRepository: mock<ClientConfigRepository>(),
3841
};
3942

4043
const handler = new ValidateLetterTemplateFilesLambda(mocks).guardDutyHandler;
@@ -135,14 +138,19 @@ describe('guard duty handler', () => {
135138
versionId,
136139
true,
137140
pdf.personalisationParameters,
138-
csv.parameters
141+
csv.parameters,
142+
false
139143
);
140144
});
141145

142146
test('skips personalisation field validation for RTL languages', async () => {
143147
// arrange
144148
const { handler, mocks } = setup();
145149

150+
mocks.clientConfigRepository.get.mockResolvedValueOnce({
151+
features: { proofing: true },
152+
});
153+
146154
const event = makeGuardDutyMalwareScanResultNotificationEvent({
147155
detail: {
148156
s3ObjectDetails: {
@@ -171,6 +179,7 @@ describe('guard duty handler', () => {
171179
},
172180
templateStatus: 'PENDING_VALIDATION',
173181
language: 'fa',
182+
clientId: 'clientId',
174183
}),
175184
});
176185

@@ -209,14 +218,16 @@ describe('guard duty handler', () => {
209218
expect(pdf.parse).toHaveBeenCalled();
210219
expect(csv.parse).toHaveBeenCalled();
211220
expect(mocks.validateLetterTemplateFiles).not.toHaveBeenCalled();
221+
expect(mocks.clientConfigRepository.get).toHaveBeenCalledWith('clientId');
212222
expect(
213223
mocks.templateRepository.setLetterValidationResult
214224
).toHaveBeenCalledWith(
215225
{ id: templateId, owner },
216226
versionId,
217227
true,
218228
['firstName', 'parameter_1', 'unknown_parameter'],
219-
['parameter_1', 'missing_parameter']
229+
['parameter_1', 'missing_parameter'],
230+
true
220231
);
221232
});
222233

@@ -293,7 +304,8 @@ describe('guard duty handler', () => {
293304
versionId,
294305
false,
295306
['firstName', 'parameter_1', 'unknown_parameter'],
296-
['parameter_1', 'missing_parameter']
307+
['parameter_1', 'missing_parameter'],
308+
false
297309
);
298310
});
299311

@@ -372,7 +384,8 @@ describe('guard duty handler', () => {
372384
versionId,
373385
true,
374386
pdf.personalisationParameters,
375-
[]
387+
[],
388+
false
376389
);
377390
});
378391

@@ -1048,7 +1061,14 @@ describe('guard duty handler', () => {
10481061

10491062
expect(
10501063
mocks.templateRepository.setLetterValidationResult
1051-
).toHaveBeenCalledWith({ id: templateId, owner }, versionId, false, [], []);
1064+
).toHaveBeenCalledWith(
1065+
{ id: templateId, owner },
1066+
versionId,
1067+
false,
1068+
[],
1069+
[],
1070+
false
1071+
);
10521072
});
10531073

10541074
test('sets the template to failed if unable to parse the csv file', async () => {
@@ -1103,7 +1123,14 @@ describe('guard duty handler', () => {
11031123

11041124
expect(
11051125
mocks.templateRepository.setLetterValidationResult
1106-
).toHaveBeenCalledWith({ id: templateId, owner }, versionId, false, [], []);
1126+
).toHaveBeenCalledWith(
1127+
{ id: templateId, owner },
1128+
versionId,
1129+
false,
1130+
[],
1131+
[],
1132+
false
1133+
);
11071134
});
11081135

11091136
test('sets the template to failed if the validation fails', async () => {
@@ -1163,7 +1190,8 @@ describe('guard duty handler', () => {
11631190
versionId,
11641191
false,
11651192
pdf.personalisationParameters,
1166-
csv.parameters
1193+
csv.parameters,
1194+
false
11671195
);
11681196
});
11691197
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1562,7 +1562,7 @@ describe('templateClient', () => {
15621562
const result = await templateClient.requestProof(templateId, user);
15631563

15641564
expect(mocks.clientConfigRepository.get).toHaveBeenCalledWith(
1565-
user.userId
1565+
user.clientId
15661566
);
15671567

15681568
expect(

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

Lines changed: 70 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1259,15 +1259,18 @@ describe('templateRepository', () => {
12591259

12601260
describe('setLetterValidationResult', () => {
12611261
describe('when proofing flag is enabled', () => {
1262-
const { templateRepository, mocks } = setup(true);
1262+
const proofingEnabled = true;
1263+
1264+
const { templateRepository, mocks } = setup(proofingEnabled);
12631265

12641266
it('should update the templateStatus to PENDING_PROOF_REQUEST, personalisationParameters and csvHeader when template is valid', async () => {
12651267
await templateRepository.setLetterValidationResult(
12661268
{ owner: 'template-owner', id: 'template-id' },
12671269
'file-version-id',
12681270
true,
12691271
['personalisation', 'parameters'],
1270-
['csv', 'headers']
1272+
['csv', 'headers'],
1273+
proofingEnabled
12711274
);
12721275

12731276
expect(mocks.ddbDocClient).toHaveReceivedCommandWith(UpdateCommand, {
@@ -1304,7 +1307,8 @@ describe('templateRepository', () => {
13041307
'file-version-id',
13051308
false,
13061309
[],
1307-
[]
1310+
[],
1311+
proofingEnabled
13081312
);
13091313

13101314
expect(mocks.ddbDocClient).toHaveReceivedCommandWith(UpdateCommand, {
@@ -1333,52 +1337,72 @@ describe('templateRepository', () => {
13331337
});
13341338

13351339
describe('when proofing flag is disabled', () => {
1336-
const { templateRepository, mocks } = setup(false);
1337-
1338-
it('updates the templateStatus to NOT_YET_SUBMITTED, personalisationParameters and csvHeaders if valid', async () => {
1339-
await templateRepository.setLetterValidationResult(
1340-
{ owner: 'template-owner', id: 'template-id' },
1341-
'file-version-id',
1342-
true,
1343-
['personalisation', 'parameters'],
1344-
['csv', 'headers']
1345-
);
1346-
1347-
expect(mocks.ddbDocClient).toHaveReceivedCommandWith(UpdateCommand, {
1348-
TableName: 'templates',
1349-
Key: { id: 'template-id', owner: 'template-owner' },
1350-
UpdateExpression:
1351-
'SET #templateStatus = :templateStatus , #updatedAt = :updatedAt , #personalisationParameters = :personalisationParameters , #testDataCsvHeaders = :testDataCsvHeaders',
1352-
ConditionExpression:
1353-
'#files.#file.#version = :version and not #templateStatus in (:templateStatusDeleted, :templateStatusSubmitted)',
1354-
ExpressionAttributeNames: {
1355-
'#testDataCsvHeaders': 'testDataCsvHeaders',
1356-
'#file': 'pdfTemplate',
1357-
'#files': 'files',
1358-
'#personalisationParameters': 'personalisationParameters',
1359-
'#templateStatus': 'templateStatus',
1360-
'#updatedAt': 'updatedAt',
1361-
'#version': 'currentVersion',
1362-
},
1363-
ExpressionAttributeValues: {
1364-
':testDataCsvHeaders': ['csv', 'headers'],
1365-
':personalisationParameters': ['personalisation', 'parameters'],
1366-
':templateStatus': 'NOT_YET_SUBMITTED',
1367-
':templateStatusDeleted': 'DELETED',
1368-
':templateStatusSubmitted': 'SUBMITTED',
1369-
':updatedAt': '2024-12-27T00:00:00.000Z',
1370-
':version': 'file-version-id',
1371-
},
1372-
});
1373-
});
1340+
test.each([
1341+
{
1342+
globalProofing: false,
1343+
clientProofing: false,
1344+
},
1345+
{
1346+
globalProofing: true,
1347+
clientProofing: false,
1348+
},
1349+
{
1350+
globalProofing: false,
1351+
clientProofing: true,
1352+
},
1353+
])(
1354+
'updates the templateStatus to NOT_YET_SUBMITTED when global proofing is $globalProofing and client proofing is $clientProofing',
1355+
async ({ clientProofing, globalProofing }) => {
1356+
const { templateRepository, mocks } = setup(globalProofing);
1357+
1358+
await templateRepository.setLetterValidationResult(
1359+
{ owner: 'template-owner', id: 'template-id' },
1360+
'file-version-id',
1361+
true,
1362+
['personalisation', 'parameters'],
1363+
['csv', 'headers'],
1364+
clientProofing
1365+
);
1366+
1367+
expect(mocks.ddbDocClient).toHaveReceivedCommandWith(UpdateCommand, {
1368+
TableName: 'templates',
1369+
Key: { id: 'template-id', owner: 'template-owner' },
1370+
UpdateExpression:
1371+
'SET #templateStatus = :templateStatus , #updatedAt = :updatedAt , #personalisationParameters = :personalisationParameters , #testDataCsvHeaders = :testDataCsvHeaders',
1372+
ConditionExpression:
1373+
'#files.#file.#version = :version and not #templateStatus in (:templateStatusDeleted, :templateStatusSubmitted)',
1374+
ExpressionAttributeNames: {
1375+
'#testDataCsvHeaders': 'testDataCsvHeaders',
1376+
'#file': 'pdfTemplate',
1377+
'#files': 'files',
1378+
'#personalisationParameters': 'personalisationParameters',
1379+
'#templateStatus': 'templateStatus',
1380+
'#updatedAt': 'updatedAt',
1381+
'#version': 'currentVersion',
1382+
},
1383+
ExpressionAttributeValues: {
1384+
':testDataCsvHeaders': ['csv', 'headers'],
1385+
':personalisationParameters': ['personalisation', 'parameters'],
1386+
':templateStatus': 'NOT_YET_SUBMITTED',
1387+
':templateStatusDeleted': 'DELETED',
1388+
':templateStatusSubmitted': 'SUBMITTED',
1389+
':updatedAt': '2024-12-27T00:00:00.000Z',
1390+
':version': 'file-version-id',
1391+
},
1392+
});
1393+
}
1394+
);
13741395

13751396
it('updates the templateStatus to VALIDATION_FAILED if not valid', async () => {
1397+
const { templateRepository, mocks } = setup(false);
1398+
13761399
await templateRepository.setLetterValidationResult(
13771400
{ owner: 'template-owner', id: 'template-id' },
13781401
'file-version-id',
13791402
false,
13801403
[],
1381-
[]
1404+
[],
1405+
false
13821406
);
13831407

13841408
expect(mocks.ddbDocClient).toHaveReceivedCommandWith(UpdateCommand, {
@@ -1422,7 +1446,8 @@ describe('templateRepository', () => {
14221446
'file-version-id',
14231447
false,
14241448
[],
1425-
[]
1449+
[],
1450+
false
14261451
)
14271452
).resolves.not.toThrow();
14281453
});
@@ -1438,7 +1463,8 @@ describe('templateRepository', () => {
14381463
'file-version-id',
14391464
false,
14401465
[],
1441-
[]
1466+
[],
1467+
false
14421468
)
14431469
).rejects.toThrow('Something went wrong');
14441470
});

lambdas/backend-api/src/templates/api/validate-letter-template-files.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,27 @@ import { TemplatePdf } from '../domain/template-pdf';
88
import { TestDataCsv } from '../domain/test-data-csv';
99
import { validateLetterTemplateFiles } from '../domain/validate-letter-template-files';
1010
import { SQSBatchItemFailure, SQSBatchResponse, SQSEvent } from 'aws-lambda';
11+
import { ClientConfigRepository } from '../infra/client-config-repository';
1112

1213
export class ValidateLetterTemplateFilesLambda {
1314
private letterUploadRepository: LetterUploadRepository;
1415

1516
private templateRepository: TemplateRepository;
1617

18+
private clientConfigRepository: ClientConfigRepository;
19+
1720
constructor({
1821
letterUploadRepository,
1922
templateRepository,
23+
clientConfigRepository,
2024
}: {
2125
letterUploadRepository: LetterUploadRepository;
2226
templateRepository: TemplateRepository;
27+
clientConfigRepository: ClientConfigRepository;
2328
}) {
2429
this.letterUploadRepository = letterUploadRepository;
2530
this.templateRepository = templateRepository;
31+
this.clientConfigRepository = clientConfigRepository;
2632
}
2733

2834
sqsHandler = async (event: SQSEvent): Promise<SQSBatchResponse> => {
@@ -148,6 +154,12 @@ export class ValidateLetterTemplateFilesLambda {
148154
const pdf = new TemplatePdf({ id: templateId, owner }, pdfBuff);
149155
let csv;
150156

157+
const client = await this.clientConfigRepository.get(
158+
String(getTemplateResult.data.clientId)
159+
);
160+
161+
const clientProofingEnabled = client?.features?.proofing || false;
162+
151163
try {
152164
await pdf.parse();
153165

@@ -164,7 +176,8 @@ export class ValidateLetterTemplateFilesLambda {
164176
versionId,
165177
false,
166178
[],
167-
[]
179+
[],
180+
clientProofingEnabled
168181
);
169182

170183
return;
@@ -178,7 +191,8 @@ export class ValidateLetterTemplateFilesLambda {
178191
versionId,
179192
valid,
180193
pdf.personalisationParameters,
181-
csv?.parameters || []
194+
csv?.parameters || [],
195+
clientProofingEnabled
182196
);
183197
};
184198
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ export class TemplateClient {
330330
): Promise<Result<TemplateDto>> {
331331
const log = this.logger.child({ templateId, user });
332332

333-
const client = await this.clientConfigRepository.get(String(user.userId));
333+
const client = await this.clientConfigRepository.get(String(user.clientId));
334334

335335
if (!client?.features.proofing) {
336336
log.error({

lambdas/backend-api/src/templates/container.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ export function createContainer() {
6363
templateClient,
6464
templateRepository,
6565
letterUploadRepository,
66+
clientConfigRepository,
6667
};
6768
}
6869

0 commit comments

Comments
 (0)