Skip to content

Commit 23c18a2

Browse files
alexnuttallalexnuttall
committed
CCM-10387: refactor
1 parent 85c6e2e commit 23c18a2

File tree

40 files changed

+531
-299
lines changed

40 files changed

+531
-299
lines changed

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,9 @@ test.each([
4141
'calls dependencies as expected for a %s virus scan',
4242
async (scanResultStatus, virusScanStatus, s3Expectation) => {
4343
const templateRepository = mockDeep<TemplateRepository>({
44-
getOwner: jest.fn().mockReturnValue('CLIENT#template-owner'),
44+
getOwner: jest
45+
.fn()
46+
.mockReturnValue({ owner: 'template-owner', clientOwned: true }),
4547
});
4648
const letterFileRepository = mockDeep<LetterFileRepository>();
4749

@@ -68,8 +70,7 @@ test.each([
6870
expect(
6971
templateRepository.setLetterFileVirusScanStatusForProof
7072
).toHaveBeenCalledWith(
71-
'CLIENT#template-owner',
72-
'template-id',
73+
{ owner: 'template-owner', id: 'template-id', clientOwned: true },
7374
'proof.pdf',
7475
virusScanStatus,
7576
'supplier'
@@ -79,7 +80,9 @@ test.each([
7980

8081
test('sets virus scan status for a user-owned template', async () => {
8182
const templateRepository = mockDeep<TemplateRepository>({
82-
getOwner: jest.fn().mockReturnValue('user-template-owner'),
83+
getOwner: jest
84+
.fn()
85+
.mockReturnValue({ owner: 'user-template-owner', clientOwned: false }),
8386
});
8487
const letterFileRepository = mockDeep<LetterFileRepository>();
8588

@@ -112,8 +115,7 @@ test('sets virus scan status for a user-owned template', async () => {
112115
expect(
113116
templateRepository.setLetterFileVirusScanStatusForProof
114117
).toHaveBeenCalledWith(
115-
'user-template-owner',
116-
'template-id',
118+
{ owner: 'user-template-owner', id: 'template-id', clientOwned: false },
117119
'proof.pdf',
118120
'PASSED',
119121
'supplier'

‎lambdas/backend-api/src/__tests__/templates/api/set-letter-upload-virus-scan-status.test.ts‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,17 @@ it('sets the virus scan status on pdf uploads identified by file metadata', asyn
2929
},
3030
});
3131

32-
mocks.templateRepository.getOwner.mockResolvedValueOnce('template-owner');
32+
mocks.templateRepository.getOwner.mockResolvedValueOnce({
33+
owner: 'template-owner',
34+
clientOwned: true,
35+
});
3336

3437
await handler(event);
3538

3639
expect(
3740
mocks.templateRepository.setLetterFileVirusScanStatusForUpload
3841
).toHaveBeenCalledWith(
39-
{ id: 'template-id', owner: 'template-owner' },
42+
{ id: 'template-id', owner: 'template-owner', clientOwned: true },
4043
'pdf-template',
4144
'template-version',
4245
'PASSED'
@@ -57,14 +60,17 @@ it('sets the virus scan status on csv files identified by file metadata', async
5760
},
5861
});
5962

60-
mocks.templateRepository.getOwner.mockResolvedValueOnce('template-owner');
63+
mocks.templateRepository.getOwner.mockResolvedValueOnce({
64+
owner: 'template-owner',
65+
clientOwned: true,
66+
});
6167

6268
await handler(event);
6369

6470
expect(
6571
mocks.templateRepository.setLetterFileVirusScanStatusForUpload
6672
).toHaveBeenCalledWith(
67-
{ id: 'template-id', owner: 'template-owner' },
73+
{ id: 'template-id', owner: 'template-owner', clientOwned: true },
6874
'test-data',
6975
'template-version',
7076
'PASSED'
@@ -88,14 +94,17 @@ it.each($GuardDutyMalwareScanStatusFailed.options)(
8894
},
8995
});
9096

91-
mocks.templateRepository.getOwner.mockResolvedValueOnce('template-owner');
97+
mocks.templateRepository.getOwner.mockResolvedValueOnce({
98+
owner: 'template-owner',
99+
clientOwned: true,
100+
});
92101

93102
await handler(event);
94103

95104
expect(
96105
mocks.templateRepository.setLetterFileVirusScanStatusForUpload
97106
).toHaveBeenCalledWith(
98-
{ id: 'template-id', owner: 'template-owner' },
107+
{ id: 'template-id', owner: 'template-owner', clientOwned: true },
99108
'pdf-template',
100109
'template-version',
101110
'FAILED'
@@ -178,7 +187,10 @@ it('errors if status update fails', async () => {
178187
},
179188
});
180189

181-
mocks.templateRepository.getOwner.mockResolvedValueOnce('template-owner');
190+
mocks.templateRepository.getOwner.mockResolvedValueOnce({
191+
owner: 'template-owner',
192+
clientOwned: true,
193+
});
182194

183195
const error = new Error('Status Update error');
184196

@@ -203,9 +215,10 @@ it('errors if owner from database does not match s3 path', async () => {
203215
},
204216
});
205217

206-
mocks.templateRepository.getOwner.mockResolvedValueOnce(
207-
'CLIENT#someone-else'
208-
);
218+
mocks.templateRepository.getOwner.mockResolvedValueOnce({
219+
owner: 'someone-else',
220+
clientOwned: true,
221+
});
209222

210223
await expect(handler(event)).rejects.toThrow(
211224
'Database owner and s3 owner mismatch'

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

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,10 @@ describe('guard duty handler', () => {
122122
versionId
123123
);
124124

125-
expect(mocks.TemplatePdf).toHaveBeenCalledWith(templateId, userId, pdfData);
125+
expect(mocks.TemplatePdf).toHaveBeenCalledWith(
126+
{ id: templateId, owner: userId, clientOwned: false },
127+
pdfData
128+
);
126129
expect(mocks.TestDataCsv).toHaveBeenCalledWith(csvData);
127130

128131
expect(pdf.parse).toHaveBeenCalled();
@@ -133,7 +136,7 @@ describe('guard duty handler', () => {
133136
expect(
134137
mocks.templateRepository.setLetterValidationResult
135138
).toHaveBeenCalledWith(
136-
{ id: templateId, owner: userId },
139+
{ id: templateId, owner: userId, clientOwned: false },
137140
versionId,
138141
true,
139142
pdf.personalisationParameters,
@@ -204,8 +207,7 @@ describe('guard duty handler', () => {
204207
);
205208

206209
expect(mocks.TemplatePdf).toHaveBeenCalledWith(
207-
templateId,
208-
clientId,
210+
{ id: templateId, owner: clientId, clientOwned: true },
209211
pdfData
210212
);
211213

@@ -219,7 +221,7 @@ describe('guard duty handler', () => {
219221
expect(
220222
mocks.templateRepository.setLetterValidationResult
221223
).toHaveBeenCalledWith(
222-
{ id: templateId, owner: `CLIENT#${clientId}` },
224+
{ id: templateId, owner: `CLIENT#${clientId}`, clientOwned: true },
223225
versionId,
224226
true,
225227
pdf.personalisationParameters,
@@ -293,15 +295,18 @@ describe('guard duty handler', () => {
293295
await handler(event);
294296

295297
// assert
296-
expect(mocks.TemplatePdf).toHaveBeenCalledWith(templateId, userId, pdfData);
298+
expect(mocks.TemplatePdf).toHaveBeenCalledWith(
299+
{ id: templateId, owner: userId, clientOwned: false },
300+
pdfData
301+
);
297302
expect(mocks.TestDataCsv).toHaveBeenCalledWith(csvData);
298303
expect(pdf.parse).toHaveBeenCalled();
299304
expect(csv.parse).toHaveBeenCalled();
300305
expect(mocks.validateLetterTemplateFiles).not.toHaveBeenCalled();
301306
expect(
302307
mocks.templateRepository.setLetterValidationResult
303308
).toHaveBeenCalledWith(
304-
{ id: templateId, owner: userId },
309+
{ id: templateId, owner: userId, clientOwned: false },
305310
versionId,
306311
true,
307312
['firstName', 'parameter_1', 'unknown_parameter'],
@@ -381,7 +386,7 @@ describe('guard duty handler', () => {
381386
expect(
382387
mocks.templateRepository.setLetterValidationResult
383388
).toHaveBeenCalledWith(
384-
{ id: templateId, owner: userId },
389+
{ id: templateId, owner: userId, clientOwned: false },
385390
versionId,
386391
false,
387392
['firstName', 'parameter_1', 'unknown_parameter'],
@@ -448,7 +453,10 @@ describe('guard duty handler', () => {
448453
versionId
449454
);
450455

451-
expect(mocks.TemplatePdf).toHaveBeenCalledWith(templateId, userId, pdfData);
456+
expect(mocks.TemplatePdf).toHaveBeenCalledWith(
457+
{ id: templateId, owner: userId, clientOwned: false },
458+
pdfData
459+
);
452460
expect(mocks.TestDataCsv).not.toHaveBeenCalled();
453461

454462
expect(pdf.parse).toHaveBeenCalled();
@@ -461,7 +469,7 @@ describe('guard duty handler', () => {
461469
expect(
462470
mocks.templateRepository.setLetterValidationResult
463471
).toHaveBeenCalledWith(
464-
{ id: templateId, owner: userId },
472+
{ id: templateId, owner: userId, clientOwned: false },
465473
versionId,
466474
true,
467475
pdf.personalisationParameters,
@@ -990,6 +998,7 @@ describe('guard duty handler', () => {
990998
},
991999
templateStatus: 'PENDING_VALIDATION',
9921000
language: 'en',
1001+
owner: 'CLIENT#client-id',
9931002
}),
9941003
});
9951004

@@ -1033,6 +1042,7 @@ describe('guard duty handler', () => {
10331042
},
10341043
templateStatus: 'PENDING_VALIDATION',
10351044
language: 'en',
1045+
owner: 'CLIENT#client-id',
10361046
}),
10371047
});
10381048

@@ -1080,6 +1090,7 @@ describe('guard duty handler', () => {
10801090
},
10811091
templateStatus: 'PENDING_VALIDATION',
10821092
language: 'en',
1093+
owner: 'CLIENT#client-id',
10831094
}),
10841095
});
10851096

@@ -1147,7 +1158,7 @@ describe('guard duty handler', () => {
11471158
expect(
11481159
mocks.templateRepository.setLetterValidationResult
11491160
).toHaveBeenCalledWith(
1150-
{ id: templateId, owner: userId },
1161+
{ id: templateId, owner: userId, clientOwned: false },
11511162
versionId,
11521163
false,
11531164
[],
@@ -1211,7 +1222,7 @@ describe('guard duty handler', () => {
12111222
expect(
12121223
mocks.templateRepository.setLetterValidationResult
12131224
).toHaveBeenCalledWith(
1214-
{ id: templateId, owner: userId },
1225+
{ id: templateId, owner: userId, clientOwned: false },
12151226
versionId,
12161227
false,
12171228
[],
@@ -1275,7 +1286,7 @@ describe('guard duty handler', () => {
12751286
expect(
12761287
mocks.templateRepository.setLetterValidationResult
12771288
).toHaveBeenCalledWith(
1278-
{ id: templateId, owner: userId },
1289+
{ id: templateId, owner: userId, clientOwned: false },
12791290
versionId,
12801291
false,
12811292
pdf.personalisationParameters,

‎lambdas/backend-api/src/__tests__/templates/domain/template-pdf.test.ts‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,23 @@ import path from 'node:path';
55
import { TemplatePdf } from '@backend-api/templates/domain/template-pdf';
66

77
test('has the given key attributes', () => {
8-
const pdf = new TemplatePdf('template-id', 'template-owner', Buffer.from(''));
8+
const pdf = new TemplatePdf(
9+
{ id: 'template-id', owner: 'template-owner', clientOwned: true },
10+
Buffer.from('')
11+
);
912

1013
expect(pdf.templateId).toBe('template-id');
11-
expect(pdf.userOrClientId).toBe('template-owner');
14+
expect(pdf.owner).toBe('template-owner');
1215
});
1316

1417
test('parse with no custom personalisation', async () => {
1518
const file = fs.readFileSync(
1619
path.resolve(__dirname, '../fixtures/no-custom-personalisation.pdf')
1720
);
18-
const pdf = new TemplatePdf('template-id', 'template-owner', file);
21+
const pdf = new TemplatePdf(
22+
{ id: 'template-id', owner: 'template-owner', clientOwned: true },
23+
file
24+
);
1925
await pdf.parse();
2026

2127
expect(pdf.personalisationParameters).toEqual([
@@ -59,7 +65,10 @@ test('parse with custom personalisation', async () => {
5965
const file = fs.readFileSync(
6066
path.resolve(__dirname, '../fixtures/custom-personalisation.pdf')
6167
);
62-
const pdf = new TemplatePdf('template-id', 'template-owner', file);
68+
const pdf = new TemplatePdf(
69+
{ id: 'template-id', owner: 'template-owner', clientOwned: true },
70+
file
71+
);
6372
await pdf.parse();
6473

6574
expect(pdf.personalisationParameters).toEqual([
@@ -114,7 +123,10 @@ test('errors if parse is not called before reading personalisation', () => {
114123
const file = fs.readFileSync(
115124
path.resolve(__dirname, '../fixtures/no-custom-personalisation.pdf')
116125
);
117-
const pdf = new TemplatePdf('template-id', 'template-owner', file);
126+
const pdf = new TemplatePdf(
127+
{ id: 'template-id', owner: 'template-owner', clientOwned: true },
128+
file
129+
);
118130

119131
expect(() => pdf.personalisationParameters).toThrow(
120132
'PDF has not been parsed'
@@ -135,7 +147,10 @@ test('errors if file cannot be parsed', async () => {
135147
path.resolve(__dirname, '../fixtures/test-data.csv')
136148
);
137149

138-
const pdf = new TemplatePdf('template-id', 'template-owner', file);
150+
const pdf = new TemplatePdf(
151+
{ id: 'template-id', owner: 'template-owner', clientOwned: true },
152+
file
153+
);
139154

140155
await expect(pdf.parse()).rejects.toThrow();
141156
});
@@ -145,7 +160,10 @@ test('errors if file cannot be opened', async () => {
145160
path.resolve(__dirname, '../fixtures/password.pdf')
146161
);
147162

148-
const pdf = new TemplatePdf('template-id', 'template-owner', file);
163+
const pdf = new TemplatePdf(
164+
{ id: 'template-id', owner: 'template-owner', clientOwned: true },
165+
file
166+
);
149167

150168
await expect(pdf.parse()).rejects.toThrow();
151169
});

0 commit comments

Comments
 (0)