Skip to content

Commit 1cf53d3

Browse files
CCM-11537: Code review comments
1 parent a490566 commit 1cf53d3

File tree

17 files changed

+62
-62
lines changed

17 files changed

+62
-62
lines changed

frontend/src/__tests__/app/message-plans/choose-standard-english-letter-template/preview-template/__snapshots__/page.test.tsx.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3-
exports[`PreviewStandardEnglishLetterTemplateFromMessagePlan page renders Email template preview 1`] = `
3+
exports[`PreviewStandardEnglishLetterTemplateFromMessagePlan page renders letter template preview 1`] = `
44
<DocumentFragment>
55
<a
66
class="nhsuk-back-link"

frontend/src/__tests__/app/message-plans/choose-standard-english-letter-template/preview-template/page.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ describe('PreviewStandardEnglishLetterTemplateFromMessagePlan page', () => {
2828
expect(redirectMock).toHaveBeenCalledWith('/invalid-template', 'replace');
2929
});
3030

31-
it('renders Email template preview', async () => {
31+
it('renders letter template preview', async () => {
3232
getTemplateMock.mockResolvedValueOnce({
3333
...LETTER_TEMPLATE,
3434
templateStatus: 'SUBMITTED',

frontend/src/__tests__/components/forms/ChooseChannelTemplate/__snapshots__/ChooseChannelTemplate.test.tsx.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ exports[`ChooseChannelTemplate renders correctly when no templates are available
330330
</a>
331331
</p>
332332
<a
333-
class="nhsuk-u-font-size-19 false"
333+
class="nhsuk-u-font-size-19"
334334
href="/message-plans/choose-templates/fbb81055-79b9-4759-ac07-d191ae57be34"
335335
>
336336
Go back

frontend/src/components/forms/ChooseChannelTemplate/ChooseChannelTemplate.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
} from './server-action';
1818
import { validate } from '@utils/client-validate-form';
1919
import { NHSNotifyFormWrapper } from '@molecules/NHSNotifyFormWrapper/NHSNotifyFormWrapper';
20+
import classNames from 'classnames';
2021

2122
const content = baseContent.components.chooseChannelTemplate;
2223

@@ -106,7 +107,11 @@ export function ChooseChannelTemplate(props: ChooseChannelTemplateProps) {
106107
href={interpolate(content.actions.backLink.href, {
107108
routingConfigId: messagePlan.id,
108109
})}
109-
className={`nhsuk-u-font-size-19 ${templateList.length > 0 && 'inline-block nhsuk-u-margin-left-3 nhsuk-u-padding-top-3'}`}
110+
className={classNames(
111+
'nhsuk-u-font-size-19',
112+
templateList.length > 0 &&
113+
'inline-block nhsuk-u-margin-left-3 nhsuk-u-padding-top-3'
114+
)}
110115
>
111116
{content.actions.backLink.text}
112117
</Link>

frontend/src/utils/interpolate.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const interpolationPattern = /{{([^|}]+)(?:\|([^|}]+)\|([^}]+))?}}/g;
1717

1818
export function interpolate(
1919
template: string,
20-
variables: Record<string, string | number | undefined> = {}
20+
variables: Record<string, string | number> = {}
2121
): string {
2222
// eslint-disable-next-line unicorn/prefer-string-replace-all
2323
return template.replace(interpolationPattern, (_, key, singular, plural) => {

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1637,11 +1637,11 @@ describe('templateClient', () => {
16371637
const result = await templateClient.listTemplates(user, null);
16381638

16391639
expect(templateRepository.query).toHaveBeenCalledWith(user.clientId);
1640-
expect(queryMock.excludeTemplateStatus).toHaveBeenCalledWith(['DELETED']);
1641-
expect(queryMock.templateStatus).toHaveBeenCalledWith([]);
1642-
expect(queryMock.templateType).toHaveBeenCalledWith([]);
1643-
expect(queryMock.language).toHaveBeenCalledWith([]);
1644-
expect(queryMock.letterType).toHaveBeenCalledWith([]);
1640+
expect(queryMock.excludeTemplateStatus).toHaveBeenCalledWith('DELETED');
1641+
expect(queryMock.templateStatus).not.toHaveBeenCalled();
1642+
expect(queryMock.templateType).not.toHaveBeenCalled();
1643+
expect(queryMock.language).not.toHaveBeenCalled();
1644+
expect(queryMock.letterType).not.toHaveBeenCalled();
16451645

16461646
expect(result).toEqual({
16471647
data: [template],
@@ -1705,11 +1705,11 @@ describe('templateClient', () => {
17051705
const result = await templateClient.listTemplates(user, filter);
17061706

17071707
expect(templateRepository.query).toHaveBeenCalledWith(user.clientId);
1708-
expect(queryMock.excludeTemplateStatus).toHaveBeenCalledWith(['DELETED']);
1709-
expect(queryMock.templateStatus).toHaveBeenCalledWith(['SUBMITTED']);
1710-
expect(queryMock.templateType).toHaveBeenCalledWith(['NHS_APP']);
1711-
expect(queryMock.language).toHaveBeenCalledWith(['en']);
1712-
expect(queryMock.letterType).toHaveBeenCalledWith(['x0']);
1708+
expect(queryMock.excludeTemplateStatus).toHaveBeenCalledWith('DELETED');
1709+
expect(queryMock.templateStatus).toHaveBeenCalledWith('SUBMITTED');
1710+
expect(queryMock.templateType).toHaveBeenCalledWith('NHS_APP');
1711+
expect(queryMock.language).toHaveBeenCalledWith('en');
1712+
expect(queryMock.letterType).toHaveBeenCalledWith('x0');
17131713

17141714
expect(result).toEqual({
17151715
data: [template],

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

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ describe('TemplateRepo#query', () => {
110110

111111
await repo
112112
.query(clientId)
113-
.templateStatus(['SUBMITTED', 'DELETED'])
114-
.templateStatus(['NOT_YET_SUBMITTED'])
113+
.templateStatus('SUBMITTED', 'DELETED')
114+
.templateStatus('NOT_YET_SUBMITTED')
115115
.list();
116116

117117
expect(mocks.dynamo).toHaveReceivedCommandTimes(QueryCommand, 1);
@@ -142,8 +142,8 @@ describe('TemplateRepo#query', () => {
142142

143143
await repo
144144
.query(clientId)
145-
.excludeTemplateStatus(['SUBMITTED', 'DELETED'])
146-
.excludeTemplateStatus(['NOT_YET_SUBMITTED'])
145+
.excludeTemplateStatus('SUBMITTED', 'DELETED')
146+
.excludeTemplateStatus('NOT_YET_SUBMITTED')
147147
.list();
148148

149149
expect(mocks.dynamo).toHaveReceivedCommandTimes(QueryCommand, 1);
@@ -174,8 +174,8 @@ describe('TemplateRepo#query', () => {
174174

175175
await repo
176176
.query(clientId)
177-
.templateType(['SMS', 'NHS_APP'])
178-
.templateType(['EMAIL'])
177+
.templateType('SMS', 'NHS_APP')
178+
.templateType('EMAIL')
179179
.list();
180180

181181
expect(mocks.dynamo).toHaveReceivedCommandTimes(QueryCommand, 1);
@@ -204,7 +204,7 @@ describe('TemplateRepo#query', () => {
204204
Items: [],
205205
});
206206

207-
await repo.query(clientId).language(['en', 'fr']).language(['es']).list();
207+
await repo.query(clientId).language('en', 'fr').language('es').list();
208208

209209
expect(mocks.dynamo).toHaveReceivedCommandTimes(QueryCommand, 1);
210210
expect(mocks.dynamo).toHaveReceivedCommandWith(QueryCommand, {
@@ -231,11 +231,7 @@ describe('TemplateRepo#query', () => {
231231
Items: [],
232232
});
233233

234-
await repo
235-
.query(clientId)
236-
.letterType(['x0', 'x1'])
237-
.letterType(['q4'])
238-
.list();
234+
await repo.query(clientId).letterType('x0', 'x1').letterType('q4').list();
239235

240236
expect(mocks.dynamo).toHaveReceivedCommandTimes(QueryCommand, 1);
241237
expect(mocks.dynamo).toHaveReceivedCommandWith(QueryCommand, {
@@ -265,11 +261,11 @@ describe('TemplateRepo#query', () => {
265261

266262
await repo
267263
.query(clientId)
268-
.templateStatus(['SUBMITTED'])
269-
.excludeTemplateStatus(['DELETED'])
270-
.templateType(['LETTER'])
271-
.language(['en'])
272-
.letterType(['x0'])
264+
.templateStatus('SUBMITTED')
265+
.excludeTemplateStatus('DELETED')
266+
.templateType('LETTER')
267+
.language('en')
268+
.letterType('x0')
273269
.list();
274270

275271
expect(mocks.dynamo).toHaveReceivedCommandTimes(QueryCommand, 1);
@@ -305,16 +301,16 @@ describe('TemplateRepo#query', () => {
305301

306302
await repo
307303
.query(clientId)
308-
.templateStatus(['SUBMITTED'])
309-
.templateStatus(['SUBMITTED'])
310-
.excludeTemplateStatus(['DELETED'])
311-
.excludeTemplateStatus(['DELETED'])
312-
.templateType(['LETTER'])
313-
.templateType(['LETTER'])
314-
.language(['en'])
315-
.language(['en'])
316-
.letterType(['x0'])
317-
.letterType(['x0'])
304+
.templateStatus('SUBMITTED')
305+
.templateStatus('SUBMITTED')
306+
.excludeTemplateStatus('DELETED')
307+
.excludeTemplateStatus('DELETED')
308+
.templateType('LETTER')
309+
.templateType('LETTER')
310+
.language('en')
311+
.language('en')
312+
.letterType('x0')
313+
.letterType('x0')
318314
.list();
319315

320316
expect(mocks.dynamo).toHaveReceivedCommandTimes(QueryCommand, 1);

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -435,13 +435,12 @@ export class TemplateClient {
435435

436436
const { templateStatus, templateType, language, letterType } =
437437
parsedFilters;
438-
const query = this.templateRepository
439-
.query(user.clientId)
440-
.excludeTemplateStatus(['DELETED'])
441-
.templateStatus(templateStatus ? [templateStatus] : [])
442-
.templateType(templateType ? [templateType] : [])
443-
.language(language ? [language] : [])
444-
.letterType(letterType ? [letterType] : []);
438+
const query = this.templateRepository.query(user.clientId);
439+
query.excludeTemplateStatus('DELETED');
440+
if (templateStatus) query.templateStatus(templateStatus);
441+
if (templateType) query.templateType(templateType);
442+
if (language) query.language(language);
443+
if (letterType) query.letterType(letterType);
445444

446445
return query.list();
447446
}

lambdas/backend-api/src/infra/template-repository/query.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,30 +25,30 @@ export class TemplateQuery extends AbstractQuery<TemplateDto> {
2525
}
2626

2727
/** Include items with any of the given template statuses. */
28-
templateStatus(statuses: TemplateStatus[]) {
28+
templateStatus(...statuses: TemplateStatus[]) {
2929
this.includeStatuses.push(...statuses);
3030
return this;
3131
}
3232

3333
/** Exclude items with any of the given template statuses. */
34-
excludeTemplateStatus(statuses: TemplateStatus[]) {
34+
excludeTemplateStatus(...statuses: TemplateStatus[]) {
3535
this.excludeStatuses.push(...statuses);
3636
return this;
3737
}
3838

3939
/** Include items with any of the given template types. */
40-
templateType(templateTypes: TemplateType[]) {
40+
templateType(...templateTypes: TemplateType[]) {
4141
this.includeTemplateTypes.push(...templateTypes);
4242
return this;
4343
}
4444

4545
/** Include items with any of the given languages. */
46-
language(languages: Language[]) {
46+
language(...languages: Language[]) {
4747
this.includeLanguages.push(...languages);
4848
return this;
4949
}
5050

51-
letterType(letterTypes: LetterType[]) {
51+
letterType(...letterTypes: LetterType[]) {
5252
this.includeLetterTypes.push(...letterTypes);
5353
return this;
5454
}

tests/test-team/template-mgmt-routing-component-tests/email/choose-email-template.routing-component.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const routingConfigStorageHelper = new RoutingConfigStorageHelper();
2222
const templateStorageHelper = new TemplateStorageHelper();
2323

2424
const invalidMessagePlanId = 'invalid-id';
25-
const notFoundMessagePlanId = randomUUID();
25+
const notFoundMessagePlanId = '48f86a39-b43c-4859-ae0b-4be4826f3a0f';
2626

2727
function createMessagePlans(user: TestUser) {
2828
return {

0 commit comments

Comments
 (0)