Skip to content

Commit a458730

Browse files
harrim91harrim91
committed
CCM-12744: feedback
1 parent 2bb21b2 commit a458730

File tree

6 files changed

+49
-53
lines changed

6 files changed

+49
-53
lines changed

‎frontend/src/__tests__/components/forms/ChooseChannelTemplate/server-action.test.ts‎

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ test('submit form - validation error', async () => {
2121
messagePlan: ROUTING_CONFIG,
2222
pageHeading: 'Choose an NHS App template',
2323
cascadeIndex: 0,
24+
templateList: [NHS_APP_TEMPLATE],
2425
},
2526
getMockFormData({})
2627
);
@@ -38,9 +39,6 @@ test('submit form - validation error', async () => {
3839
});
3940

4041
test('submit form - success updates config and redirects to choose templates', async () => {
41-
const mockRedirect = jest.mocked(redirect);
42-
const mockUpdateRoutingConfig = jest.mocked(updateRoutingConfig);
43-
4442
await chooseChannelTemplateAction(
4543
{
4644
messagePlan: {
@@ -68,14 +66,15 @@ test('submit form - success updates config and redirects to choose templates', a
6866
},
6967
pageHeading: 'Choose an email template',
7068
cascadeIndex: 1,
69+
templateList: [EMAIL_TEMPLATE],
7170
},
7271
getMockFormData({
7372
channelTemplate: EMAIL_TEMPLATE.id,
7473
lockNumber: String(EMAIL_TEMPLATE.lockNumber),
7574
})
7675
);
7776

78-
expect(mockUpdateRoutingConfig).toHaveBeenCalledWith(
77+
expect(updateRoutingConfig).toHaveBeenCalledWith(
7978
ROUTING_CONFIG.id,
8079
{
8180
cascade: [
@@ -103,16 +102,13 @@ test('submit form - success updates config and redirects to choose templates', a
103102
EMAIL_TEMPLATE.lockNumber
104103
);
105104

106-
expect(mockRedirect).toHaveBeenCalledWith(
105+
expect(redirect).toHaveBeenCalledWith(
107106
`/message-plans/choose-templates/${ROUTING_CONFIG.id}`,
108107
RedirectType.push
109108
);
110109
});
111110

112111
test('submit form - success updates config and redirects to choose templates for letter template with supplier references', async () => {
113-
const mockRedirect = jest.mocked(redirect);
114-
const mockUpdateRoutingConfig = jest.mocked(updateRoutingConfig);
115-
116112
await chooseChannelTemplateAction(
117113
{
118114
messagePlan: {
@@ -139,23 +135,28 @@ test('submit form - success updates config and redirects to choose templates for
139135
},
140136
getMockFormData({
141137
channelTemplate: LETTER_TEMPLATE.id,
138+
lockNumber: String(LETTER_TEMPLATE.lockNumber),
142139
})
143140
);
144141

145-
expect(mockUpdateRoutingConfig).toHaveBeenCalledWith(ROUTING_CONFIG.id, {
146-
cascade: [
147-
{
148-
cascadeGroups: ['standard'],
149-
channel: 'LETTER',
150-
channelType: 'primary',
151-
defaultTemplateId: LETTER_TEMPLATE.id,
152-
supplierReferences: { MBA: 'mba-supplier-reference' },
153-
},
154-
],
155-
cascadeGroupOverrides: [],
156-
});
142+
expect(updateRoutingConfig).toHaveBeenCalledWith(
143+
ROUTING_CONFIG.id,
144+
{
145+
cascade: [
146+
{
147+
cascadeGroups: ['standard'],
148+
channel: 'LETTER',
149+
channelType: 'primary',
150+
defaultTemplateId: LETTER_TEMPLATE.id,
151+
supplierReferences: { MBA: 'mba-supplier-reference' },
152+
},
153+
],
154+
cascadeGroupOverrides: [],
155+
},
156+
LETTER_TEMPLATE.lockNumber
157+
);
157158

158-
expect(mockRedirect).toHaveBeenCalledWith(
159+
expect(redirect).toHaveBeenCalledWith(
159160
`/message-plans/choose-templates/${ROUTING_CONFIG.id}`,
160161
RedirectType.push
161162
);

‎frontend/src/components/forms/ChooseChannelTemplate/server-action.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { $LockNumber } from 'nhs-notify-backend-client';
88
export type ChooseChannelTemplateFormState = FormState &
99
Pick<
1010
ChooseChannelTemplateProps,
11-
'cascadeIndex' | 'messagePlan' | 'pageHeading'
11+
'cascadeIndex' | 'messagePlan' | 'pageHeading' | 'templateList'
1212
>;
1313

1414
export const $ChooseChannelTemplate = (errorMessage: string) =>

‎lambdas/backend-api/src/__tests__/infra/routing-config-repository/repository.test.ts‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ describe('RoutingConfigRepository', () => {
247247

248248
expect(mocks.dynamo).toHaveReceivedCommandWith(UpdateCommand, {
249249
ConditionExpression:
250-
'#status = :condition_1_status AND (#lockNumber = :condition_2_1_lockNumber OR attribute_not_exists (#lockNumber))',
250+
'#status = :condition_1_status AND #lockNumber = :condition_2_lockNumber',
251251
ExpressionAttributeNames: {
252252
'#status': 'status',
253253
'#updatedAt': 'updatedAt',
@@ -256,7 +256,7 @@ describe('RoutingConfigRepository', () => {
256256
},
257257
ExpressionAttributeValues: {
258258
':condition_1_status': 'DRAFT',
259-
':condition_2_1_lockNumber': 2,
259+
':condition_2_lockNumber': 2,
260260
':lockNumber': 1,
261261
':status': 'COMPLETED',
262262
':updatedAt': date.toISOString(),
@@ -441,7 +441,7 @@ describe('RoutingConfigRepository', () => {
441441

442442
expect(mocks.dynamo).toHaveReceivedCommandWith(UpdateCommand, {
443443
ConditionExpression:
444-
'#status = :condition_1_status AND (#lockNumber = :condition_2_1_lockNumber OR attribute_not_exists (#lockNumber))',
444+
'#status = :condition_1_status AND #lockNumber = :condition_2_lockNumber',
445445
ExpressionAttributeNames: {
446446
'#lockNumber': 'lockNumber',
447447
'#status': 'status',
@@ -451,7 +451,7 @@ describe('RoutingConfigRepository', () => {
451451
},
452452
ExpressionAttributeValues: {
453453
':condition_1_status': 'DRAFT',
454-
':condition_2_1_lockNumber': 2,
454+
':condition_2_lockNumber': 2,
455455
':lockNumber': 1,
456456
':status': 'DELETED',
457457
':updatedAt': date.toISOString(),
@@ -644,7 +644,7 @@ describe('RoutingConfigRepository', () => {
644644

645645
expect(mocks.dynamo).toHaveReceivedCommandWith(UpdateCommand, {
646646
ConditionExpression:
647-
'#status = :condition_1_status AND (#lockNumber = :condition_2_1_lockNumber OR attribute_not_exists (#lockNumber))',
647+
'#status = :condition_1_status AND #lockNumber = :condition_2_lockNumber',
648648
ExpressionAttributeNames: {
649649
'#campaignId': 'campaignId',
650650
'#cascade': 'cascade',
@@ -660,7 +660,7 @@ describe('RoutingConfigRepository', () => {
660660
':cascade': update.cascade,
661661
':cascadeGroupOverrides': update.cascadeGroupOverrides,
662662
':condition_1_status': 'DRAFT',
663-
':condition_2_1_lockNumber': 2,
663+
':condition_2_lockNumber': 2,
664664
':lockNumber': 1,
665665
':name': update.name,
666666
':updatedAt': date.toISOString(),
@@ -697,7 +697,7 @@ describe('RoutingConfigRepository', () => {
697697

698698
expect(mocks.dynamo).toHaveReceivedCommandWith(UpdateCommand, {
699699
ConditionExpression:
700-
'#status = :condition_1_status AND (#lockNumber = :condition_2_1_lockNumber OR attribute_not_exists (#lockNumber))',
700+
'#status = :condition_1_status AND #lockNumber = :condition_2_lockNumber',
701701
ExpressionAttributeNames: {
702702
'#lockNumber': 'lockNumber',
703703
'#name': 'name',
@@ -707,7 +707,7 @@ describe('RoutingConfigRepository', () => {
707707
},
708708
ExpressionAttributeValues: {
709709
':condition_1_status': 'DRAFT',
710-
':condition_2_1_lockNumber': 2,
710+
':condition_2_lockNumber': 2,
711711
':lockNumber': 1,
712712
':name': update.name,
713713
':updatedAt': date.toISOString(),
@@ -744,7 +744,7 @@ describe('RoutingConfigRepository', () => {
744744

745745
expect(mocks.dynamo).toHaveReceivedCommandWith(UpdateCommand, {
746746
ConditionExpression:
747-
'#status = :condition_1_status AND (#lockNumber = :condition_2_1_lockNumber OR attribute_not_exists (#lockNumber))',
747+
'#status = :condition_1_status AND #lockNumber = :condition_2_lockNumber',
748748
ExpressionAttributeNames: {
749749
'#lockNumber': 'lockNumber',
750750
'#campaignId': 'campaignId',
@@ -754,7 +754,7 @@ describe('RoutingConfigRepository', () => {
754754
},
755755
ExpressionAttributeValues: {
756756
':condition_1_status': 'DRAFT',
757-
':condition_2_1_lockNumber': 2,
757+
':condition_2_lockNumber': 2,
758758
':lockNumber': 1,
759759
':campaignId': update.campaignId,
760760
':updatedAt': date.toISOString(),
@@ -799,7 +799,7 @@ describe('RoutingConfigRepository', () => {
799799

800800
expect(mocks.dynamo).toHaveReceivedCommandWith(UpdateCommand, {
801801
ConditionExpression:
802-
'#status = :condition_1_status AND (#lockNumber = :condition_2_1_lockNumber OR attribute_not_exists (#lockNumber))',
802+
'#status = :condition_1_status AND #lockNumber = :condition_2_lockNumber',
803803
ExpressionAttributeNames: {
804804
'#cascade': 'cascade',
805805
'#cascadeGroupOverrides': 'cascadeGroupOverrides',
@@ -810,7 +810,7 @@ describe('RoutingConfigRepository', () => {
810810
},
811811
ExpressionAttributeValues: {
812812
':condition_1_status': 'DRAFT',
813-
':condition_2_1_lockNumber': 2,
813+
':condition_2_lockNumber': 2,
814814
':lockNumber': 1,
815815
':cascade': update.cascade,
816816
':cascadeGroupOverrides': update.cascadeGroupOverrides,
@@ -856,7 +856,7 @@ describe('RoutingConfigRepository', () => {
856856

857857
expect(mocks.dynamo).toHaveReceivedCommandWith(UpdateCommand, {
858858
ConditionExpression:
859-
'#status = :condition_1_status AND (#lockNumber = :condition_2_1_lockNumber OR attribute_not_exists (#lockNumber))',
859+
'#status = :condition_1_status AND #lockNumber = :condition_2_lockNumber',
860860
ExpressionAttributeNames: {
861861
'#lockNumber': 'lockNumber',
862862
'#name': 'name',
@@ -866,7 +866,7 @@ describe('RoutingConfigRepository', () => {
866866
},
867867
ExpressionAttributeValues: {
868868
':condition_1_status': 'DRAFT',
869-
':condition_2_1_lockNumber': 2,
869+
':condition_2_lockNumber': 2,
870870
':lockNumber': 1,
871871
':name': update.name,
872872
':updatedAt': date.toISOString(),
@@ -904,7 +904,7 @@ describe('RoutingConfigRepository', () => {
904904

905905
expect(mocks.dynamo).toHaveReceivedCommandWith(UpdateCommand, {
906906
ConditionExpression:
907-
'#status = :condition_1_status AND (#lockNumber = :condition_2_1_lockNumber OR attribute_not_exists (#lockNumber))',
907+
'#status = :condition_1_status AND #lockNumber = :condition_2_lockNumber',
908908
ExpressionAttributeNames: {
909909
'#lockNumber': 'lockNumber',
910910
'#name': 'name',
@@ -914,7 +914,7 @@ describe('RoutingConfigRepository', () => {
914914
},
915915
ExpressionAttributeValues: {
916916
':condition_1_status': 'DRAFT',
917-
':condition_2_1_lockNumber': 2,
917+
':condition_2_lockNumber': 2,
918918
':lockNumber': 1,
919919
':name': update.name,
920920
':updatedAt': date.toISOString(),
@@ -950,7 +950,7 @@ describe('RoutingConfigRepository', () => {
950950

951951
expect(mocks.dynamo).toHaveReceivedCommandWith(UpdateCommand, {
952952
ConditionExpression:
953-
'#status = :condition_1_status AND (#lockNumber = :condition_2_1_lockNumber OR attribute_not_exists (#lockNumber))',
953+
'#status = :condition_1_status AND #lockNumber = :condition_2_lockNumber',
954954
ExpressionAttributeNames: {
955955
'#campaignId': 'campaignId',
956956
'#cascade': 'cascade',
@@ -966,7 +966,7 @@ describe('RoutingConfigRepository', () => {
966966
':cascade': update.cascade,
967967
':cascadeGroupOverrides': update.cascadeGroupOverrides,
968968
':condition_1_status': 'DRAFT',
969-
':condition_2_1_lockNumber': 2,
969+
':condition_2_lockNumber': 2,
970970
':lockNumber': 1,
971971
':name': update.name,
972972
':updatedAt': date.toISOString(),

‎tests/test-team/template-mgmt-api-tests/update-routing-config.api.spec.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ test.describe('PATCH /v1/routing-configuration/:routingConfigId', () => {
411411
const start = new Date();
412412

413413
const updateResponse = await request.put(
414-
`${process.env.API_BASE_URL}/v1/routing-configuration/${routingConfig.dbEntry.id}`,
414+
`${process.env.API_BASE_URL}/v1/routing-configuration/${dbEntry.id}`,
415415
{
416416
headers: {
417417
Authorization: await user1.getAccessToken(),
@@ -427,7 +427,7 @@ test.describe('PATCH /v1/routing-configuration/:routingConfigId', () => {
427427
expect(updated).toEqual({
428428
statusCode: 200,
429429
data: {
430-
...routingConfig.apiResponse,
430+
...apiResponse,
431431
...update,
432432
updatedAt: expect.stringMatching(isoDateRegExp),
433433
},
@@ -437,13 +437,13 @@ test.describe('PATCH /v1/routing-configuration/:routingConfigId', () => {
437437
start,
438438
new Date(),
439439
]);
440-
expect(updated.data.createdAt).toEqual(routingConfig.dbEntry.createdAt);
440+
expect(updated.data.createdAt).toEqual(dbEntry.createdAt);
441441
});
442442

443443
test('cascade and cascadeGroupOverrides with supplierReferences - returns 200 and the updated routing config data', async ({
444444
request,
445445
}) => {
446-
const routingConfig = RoutingConfigFactory.create(user1, {
446+
const { dbEntry, apiResponse } = RoutingConfigFactory.create(user1, {
447447
cascade: [
448448
{
449449
cascadeGroups: ['standard'],
@@ -457,7 +457,7 @@ test.describe('PATCH /v1/routing-configuration/:routingConfigId', () => {
457457
],
458458
});
459459

460-
await storageHelper.seed([routingConfig.dbEntry]);
460+
await storageHelper.seed([dbEntry]);
461461

462462
const update = {
463463
cascade: [

‎utils/entity-update-command-builder/src/__tests__/routing-config-update-builder.test.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,11 +349,10 @@ describe('RoutingConfigUpdateBuilder', () => {
349349
'#status': 'status',
350350
},
351351
ExpressionAttributeValues: {
352-
':condition_1_1_lockNumber': 5,
352+
':condition_1_lockNumber': 5,
353353
':status': 'COMPLETED',
354354
},
355-
ConditionExpression:
356-
'(#lockNumber = :condition_1_1_lockNumber OR attribute_not_exists (#lockNumber))',
355+
ConditionExpression: '#lockNumber = :condition_1_lockNumber',
357356
Key: {
358357
id: mockId,
359358
owner: mockOwnerKey,

‎utils/entity-update-command-builder/src/routing-config-update-builder.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,7 @@ export class RoutingConfigUpdateBuilder extends EntityUpdateBuilder<
8080
}
8181

8282
expectLockNumber(lockNumber: number) {
83-
this.updateBuilder.conditions.andGroup((group) => {
84-
group
85-
.and('lockNumber', '=', lockNumber)
86-
.orFn('attribute_not_exists', 'lockNumber');
87-
});
83+
this.updateBuilder.conditions.and('lockNumber', '=', lockNumber);
8884

8985
return this;
9086
}

0 commit comments

Comments
 (0)