diff --git a/frontend/src/__tests__/app/upload-letter-template/__snapshots__/page.test.tsx.snap b/frontend/src/__tests__/app/upload-letter-template/__snapshots__/page.test.tsx.snap index 1a221b56e..7a6cd692d 100644 --- a/frontend/src/__tests__/app/upload-letter-template/__snapshots__/page.test.tsx.snap +++ b/frontend/src/__tests__/app/upload-letter-template/__snapshots__/page.test.tsx.snap @@ -1,24 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`UploadLetterTemplatePage should render UploadLetterTemplatePage with campaignId field when campaignIds is not available 1`] = ` - -`; - exports[`UploadLetterTemplatePage should render UploadLetterTemplatePage with campaignIds field when available 1`] = ` { }); mockFetchClient.mockResolvedValueOnce({ data: { - campaignId: 'campaign2', + campaignIds: ['campaign2'], features: {}, }, }); @@ -49,26 +49,6 @@ describe('UploadLetterTemplatePage', () => { mockFetchClient.mockResolvedValueOnce({ data: { campaignIds: ['campaign-id', 'other-campaign-id'], - campaignId: 'campaign-id', - features: {}, - }, - }); - - const page = await UploadLetterTemplatePage(); - - expect(await generateMetadata()).toEqual({ title: pageTitle }); - expect(page).toMatchSnapshot(); - }); - - it('should render UploadLetterTemplatePage with campaignId field when campaignIds is not available', async () => { - mockGetSessionServer.mockResolvedValueOnce({ - accessToken: 'mocktoken', - clientId: 'client1', - }); - mockFetchClient.mockResolvedValueOnce({ - data: { - campaignIds: undefined, - campaignId: 'campaign-id', features: {}, }, }); @@ -107,7 +87,7 @@ describe('UploadLetterTemplatePage', () => { }); mockFetchClient.mockResolvedValueOnce({ data: { - campaignId: 'campaign2', + campaignIds: ['campaign2'], features: {}, }, }); @@ -130,7 +110,6 @@ describe('UploadLetterTemplatePage', () => { mockFetchClient.mockResolvedValueOnce({ data: { campaignIds: [], - campaignId: 'campaign-id', features: {}, }, }); @@ -143,7 +122,7 @@ describe('UploadLetterTemplatePage', () => { ); }); - it('should redirect to error page when neither campaignIds nor campaignId is present', async () => { + it('should redirect to error page when neither campaignIds are not present', async () => { const mockRedirect = jest.mocked(redirect); mockGetSessionServer.mockResolvedValueOnce({ @@ -153,7 +132,6 @@ describe('UploadLetterTemplatePage', () => { mockFetchClient.mockResolvedValueOnce({ data: { campaignIds: undefined, - campaignId: undefined, features: {}, }, }); diff --git a/frontend/src/app/upload-letter-template/page.tsx b/frontend/src/app/upload-letter-template/page.tsx index 42c84733a..6b96e4fe3 100644 --- a/frontend/src/app/upload-letter-template/page.tsx +++ b/frontend/src/app/upload-letter-template/page.tsx @@ -22,15 +22,9 @@ const getSortedCampaignIds = ( return; } - const { campaignIds, campaignId } = clientConfiguration; + const { campaignIds = [] } = clientConfiguration; - if (campaignIds) { - return campaignIds.sort(); - } - - if (campaignId) { - return [campaignId]; - } + return campaignIds.sort(); }; const UploadLetterTemplatePage = async () => { diff --git a/infrastructure/terraform/modules/backend-api/spec.tmpl.json b/infrastructure/terraform/modules/backend-api/spec.tmpl.json index f2c6a1cc6..5a9f03711 100644 --- a/infrastructure/terraform/modules/backend-api/spec.tmpl.json +++ b/infrastructure/terraform/modules/backend-api/spec.tmpl.json @@ -296,10 +296,6 @@ }, "ClientConfiguration": { "properties": { - "campaignId": { - "deprecated": true, - "type": "string" - }, "campaignIds": { "items": { "type": "string" diff --git a/lambdas/backend-api/src/__tests__/templates/api/get-client-configuration.test.ts b/lambdas/backend-api/src/__tests__/templates/api/get-client-configuration.test.ts index 103a3a601..f0fc09078 100644 --- a/lambdas/backend-api/src/__tests__/templates/api/get-client-configuration.test.ts +++ b/lambdas/backend-api/src/__tests__/templates/api/get-client-configuration.test.ts @@ -86,7 +86,7 @@ describe('Template API - get client configuration', () => { const clientConfiguration: ClientConfiguration = { features: { proofing: false }, - campaignId: 'campaign', + campaignIds: ['campaign'], }; mocks.templateClient.getClientConfiguration.mockResolvedValueOnce({ diff --git a/lambdas/backend-api/src/__tests__/templates/app/template-client.test.ts b/lambdas/backend-api/src/__tests__/templates/app/template-client.test.ts index 63b2fbe0f..9befbb676 100644 --- a/lambdas/backend-api/src/__tests__/templates/app/template-client.test.ts +++ b/lambdas/backend-api/src/__tests__/templates/app/template-client.test.ts @@ -98,33 +98,18 @@ describe('templateClient', () => { { features: {}, campaignIds: ['pea-campaign'], - campaignId: 'bean-campaign', }, 'bean-campaign' ) ).toEqual(false); }); - test('campaignIds list not present and campaign ID matches fallback campaign ID', () => { + test('campaignIds not present', () => { const { templateClient } = setup(); expect( templateClient.isCampaignIdValid( { features: {}, - campaignId: 'bean-campaign', - }, - 'bean-campaign' - ) - ).toEqual(true); - }); - - test('campaign ID does not match config', () => { - const { templateClient } = setup(); - expect( - templateClient.isCampaignIdValid( - { - features: {}, - campaignId: 'pea-campaign', }, 'bean-campaign' ) @@ -191,34 +176,6 @@ describe('templateClient', () => { }); }); - test('should return a failure result when client configuration unexpectedly cant be fetched', async () => { - const { templateClient, mocks } = setup(); - - const data: CreateUpdateTemplate = { - templateType: 'EMAIL', - name: 'name', - message: 'message', - subject: 'subject', - }; - - mocks.clientConfigRepository.get.mockResolvedValueOnce({ - error: { errorMeta: { code: 500, description: 'err' } }, - }); - - const result = await templateClient.createTemplate(data, user); - - expect(mocks.templateRepository.create).not.toHaveBeenCalled(); - - expect(result).toEqual({ - error: { - errorMeta: { - code: 500, - description: 'err', - }, - }, - }); - }); - test('should return a failure result, when saving to the database unexpectedly fails', async () => { const { templateClient, mocks } = setup(); @@ -247,8 +204,7 @@ describe('templateClient', () => { expect(mocks.templateRepository.create).toHaveBeenCalledWith( data, user, - 'NOT_YET_SUBMITTED', - undefined + 'NOT_YET_SUBMITTED' ); expect(result).toEqual({ @@ -298,8 +254,7 @@ describe('templateClient', () => { expect(mocks.templateRepository.create).toHaveBeenCalledWith( data, user, - 'NOT_YET_SUBMITTED', - undefined + 'NOT_YET_SUBMITTED' ); expect(result).toEqual({ @@ -354,8 +309,7 @@ describe('templateClient', () => { expect(mocks.templateRepository.create).toHaveBeenCalledWith( data, user, - 'NOT_YET_SUBMITTED', - undefined + 'NOT_YET_SUBMITTED' ); expect(result).toEqual({ @@ -1762,7 +1716,7 @@ describe('templateClient', () => { const { templateClient, mocks, logMessages } = setup(); mocks.clientConfigRepository.get.mockResolvedValueOnce({ - data: { features: { proofing: true }, campaignId: 'campaignId' }, + data: { features: { proofing: true }, campaignIds: ['campaignId'] }, }); const actualError = new Error('from db'); @@ -1779,7 +1733,7 @@ describe('templateClient', () => { mocks.clientConfigRepository.get.mockResolvedValueOnce({ data: { - campaignId: 'campaignId', + campaignIds: ['campaignId'], features: { proofing: true, }, @@ -1840,7 +1794,7 @@ describe('templateClient', () => { mocks.clientConfigRepository.get.mockResolvedValueOnce({ data: { - campaignId: 'campaignId', + campaignIds: ['campaignId'], features: { proofing: true, }, @@ -1891,7 +1845,7 @@ describe('templateClient', () => { mocks.clientConfigRepository.get.mockResolvedValueOnce({ data: { - campaignId: 'campaignId', + campaignIds: ['campaignId'], features: { proofing: true, }, @@ -1959,7 +1913,7 @@ describe('templateClient', () => { mocks.clientConfigRepository.get.mockResolvedValueOnce({ data: { - campaignId: 'campaign-id-from-ssm', + campaignIds: ['campaign-id-from-ssm'], features: { proofing: true, }, @@ -2027,7 +1981,7 @@ describe('templateClient', () => { mocks.clientConfigRepository.get.mockResolvedValueOnce({ data: { - campaignId: 'campaign-from-ssm', + campaignIds: ['campaign-from-ssm'], features: { proofing: true, }, @@ -2179,7 +2133,7 @@ describe('templateClient', () => { const client: ClientConfiguration = { features: { proofing: true }, - campaignId: 'campaign', + campaignIds: ['campaign'], }; mocks.clientConfigRepository.get.mockResolvedValueOnce({ data: client }); diff --git a/lambdas/backend-api/src/__tests__/templates/infra/client-config-repository.test.ts b/lambdas/backend-api/src/__tests__/templates/infra/client-config-repository.test.ts index 38fb623f9..e66c2c93f 100644 --- a/lambdas/backend-api/src/__tests__/templates/infra/client-config-repository.test.ts +++ b/lambdas/backend-api/src/__tests__/templates/infra/client-config-repository.test.ts @@ -30,7 +30,7 @@ const mockClientId = 'test-client-123'; const mockKey = `${mockSSMKeyPrefix}/${mockClientId}`; const validClient: ClientConfiguration = { - campaignId: 'campaign-123', + campaignIds: ['campaign-123'], features: { proofing: true, }, diff --git a/lambdas/backend-api/src/templates/app/template-client.ts b/lambdas/backend-api/src/templates/app/template-client.ts index f8d4ff698..ade76707c 100644 --- a/lambdas/backend-api/src/templates/app/template-client.ts +++ b/lambdas/backend-api/src/templates/app/template-client.ts @@ -62,26 +62,10 @@ export class TemplateClient { return validationResult; } - const clientConfigurationResult = await this.clientConfigRepository.get( - user.clientId - ); - - if (clientConfigurationResult.error) { - log - .child(clientConfigurationResult.error.errorMeta) - .error( - 'Failed to fetch client configuration', - clientConfigurationResult.error.actualError - ); - - return clientConfigurationResult; - } - const createResult = await this.templateRepository.create( validationResult.data, user, - 'NOT_YET_SUBMITTED', - clientConfigurationResult.data?.campaignId + 'NOT_YET_SUBMITTED' ); if (createResult.error) { @@ -539,13 +523,9 @@ export class TemplateClient { return false; } - const { campaignIds, campaignId } = clientConfiguration; - - const validCampaignId = campaignIds?.includes(campaignIdFromRequest); - const validFallbackCampaignId = - !campaignIds && campaignIdFromRequest === campaignId; + const { campaignIds = [] } = clientConfiguration; - return validCampaignId || validFallbackCampaignId; + return campaignIds.includes(campaignIdFromRequest); } async getClientConfiguration( diff --git a/lambdas/backend-client/src/types/generated/types.gen.ts b/lambdas/backend-client/src/types/generated/types.gen.ts index 21c423fc2..9723d8df8 100644 --- a/lambdas/backend-client/src/types/generated/types.gen.ts +++ b/lambdas/backend-client/src/types/generated/types.gen.ts @@ -70,10 +70,6 @@ export type Channel = 'EMAIL' | 'LETTER' | 'NHSAPP' | 'SMS'; export type ChannelType = 'primary' | 'secondary'; export type ClientConfiguration = { - /** - * @deprecated - */ - campaignId?: string; campaignIds?: Array; features: ClientFeatures; }; diff --git a/tests/accessibility/test-user-client.ts b/tests/accessibility/test-user-client.ts index c127ee633..62c2ba602 100644 --- a/tests/accessibility/test-user-client.ts +++ b/tests/accessibility/test-user-client.ts @@ -39,7 +39,7 @@ export class TestUserClient { Name: `${this.clientSsmPathPrefix}/${clientId}`, Value: JSON.stringify({ features: { proofing: true }, - campaignId: 'accessibility-test-campaign', + campaignIds: ['accessibility-test-campaign'], } satisfies ClientConfiguration), Overwrite: true, Type: 'String', diff --git a/tests/test-team/helpers/auth/cognito-auth-helper.ts b/tests/test-team/helpers/auth/cognito-auth-helper.ts index edb73b477..e64c6f819 100644 --- a/tests/test-team/helpers/auth/cognito-auth-helper.ts +++ b/tests/test-team/helpers/auth/cognito-auth-helper.ts @@ -118,11 +118,6 @@ export const testUsers: Record = { clientKey: 'ClientWithMultipleCampaigns', }, - UserWithFallbackCampaignId: { - userId: 'UserWithFallbackCampaignId', - clientKey: 'ClientWithFallbackCampaignId', - }, - /** * UserRoutingEnabled belongs to an alternate client with routing enabled */ @@ -271,7 +266,7 @@ export class CognitoAuthHelper { const clientConfig: ClientConfiguration | undefined = testClients[userDetails.clientKey]; - const { name: clientName, campaignId, campaignIds } = clientConfig ?? {}; + const { name: clientName, campaignIds } = clientConfig ?? {}; const clientAttributes = [ { Name: 'custom:sbx_client_id', Value: clientId }, @@ -331,7 +326,6 @@ export class CognitoAuthHelper { { email, userId: sub, - campaignId, campaignIds, clientId, clientKey: userDetails.clientKey, diff --git a/tests/test-team/helpers/client/client-helper.ts b/tests/test-team/helpers/client/client-helper.ts index 2fd9a76e3..3331f27c2 100644 --- a/tests/test-team/helpers/client/client-helper.ts +++ b/tests/test-team/helpers/client/client-helper.ts @@ -5,7 +5,6 @@ import { } from '@aws-sdk/client-ssm'; export type ClientConfiguration = { - campaignId?: string; campaignIds?: string[]; features: { proofing: boolean; @@ -15,7 +14,7 @@ export type ClientConfiguration = { }; export type ClientKey = - `Client${1 | 2 | 3 | 4 | 5 | 6 | 'WithMultipleCampaigns' | 'WithFallbackCampaignId' | 'RoutingEnabled'}`; + `Client${1 | 2 | 3 | 4 | 5 | 6 | 'WithMultipleCampaigns' | 'RoutingEnabled'}`; type TestClients = Record; @@ -51,7 +50,6 @@ export const testClients: TestClients = { * Client 4 has configuration but no campaignId set */ Client4: { - campaignId: undefined, name: 'NHS Test Client 4', features: { proofing: true, @@ -89,14 +87,6 @@ export const testClients: TestClients = { }, }, - ClientWithFallbackCampaignId: { - campaignId: 'campaign-id', - features: { - proofing: true, - routing: false, - }, - }, - /** * ClientRoutingEnabled is an alternative client with routing enabled */ diff --git a/tests/test-team/template-mgmt-api-tests/upload-letter-template.api.spec.ts b/tests/test-team/template-mgmt-api-tests/upload-letter-template.api.spec.ts index 38d128fcf..c87432678 100644 --- a/tests/test-team/template-mgmt-api-tests/upload-letter-template.api.spec.ts +++ b/tests/test-team/template-mgmt-api-tests/upload-letter-template.api.spec.ts @@ -16,13 +16,9 @@ test.describe('POST /v1/letter-template', () => { const authHelper = createAuthHelper(); const templateStorageHelper = new TemplateStorageHelper(); let user1: TestUser; - let userWithFallbackCampaignId: TestUser; test.beforeAll(async () => { user1 = await authHelper.getTestUser(testUsers.User1.userId); - userWithFallbackCampaignId = await authHelper.getTestUser( - testUsers.UserWithFallbackCampaignId.userId - ); }); test.afterAll(async () => { @@ -119,98 +115,6 @@ test.describe('POST /v1/letter-template', () => { expect(result.data.createdAt).not.toEqual(result.data.updatedAt); }); - test('returns 201 if input is valid, using fallback campaign id', async ({ - request, - }) => { - const { templateData, multipart, contentType } = - TemplateAPIPayloadFactory.getUploadLetterTemplatePayload( - { - templateType: 'LETTER', - campaignId: 'campaign-id', - }, - [ - { - _type: 'json', - partName: 'template', - }, - { - _type: 'file', - partName: 'letterPdf', - fileName: 'template.pdf', - fileType: 'application/pdf', - file: pdfUploadFixtures.withPersonalisation.pdf.open(), - }, - { - _type: 'file', - partName: 'testCsv', - fileName: 'test-data.csv', - fileType: 'text/csv', - file: pdfUploadFixtures.withPersonalisation.csv.open(), - }, - ] - ); - - const start = new Date(); - - const response = await request.post( - `${process.env.API_BASE_URL}/v1/letter-template`, - { - data: multipart, - headers: { - Authorization: await userWithFallbackCampaignId.getAccessToken(), - 'Content-Type': contentType, - }, - } - ); - - const result = await response.json(); - const debug = JSON.stringify(result, null, 2); - - expect(response.status(), debug).toBe(201); - - templateStorageHelper.addAdHocTemplateKey({ - templateId: result.data.id, - clientId: userWithFallbackCampaignId.clientId, - }); - - expect(result).toEqual({ - statusCode: 201, - data: { - campaignId: userWithFallbackCampaignId.campaignId, - createdAt: expect.stringMatching(isoDateRegExp), - files: { - pdfTemplate: { - currentVersion: expect.stringMatching(uuidRegExp), - fileName: 'template.pdf', - virusScanStatus: 'PENDING', - }, - testDataCsv: { - currentVersion: expect.stringMatching(uuidRegExp), - fileName: 'test-data.csv', - virusScanStatus: 'PENDING', - }, - proofs: {}, - }, - id: expect.stringMatching(uuidRegExp), - language: 'en', - letterType: 'x0', - name: templateData.name, - proofingEnabled: true, - templateStatus: 'PENDING_VALIDATION', - templateType: templateData.templateType, - updatedAt: expect.stringMatching(isoDateRegExp), - clientId: userWithFallbackCampaignId.clientId, - }, - }); - - expect(result.data.files.pdfTemplate.currentVersion).toBe( - result.data.files.testDataCsv.currentVersion - ); - - expect(result.data.createdAt).toBeDateRoughlyBetween([start, new Date()]); - expect(result.data.createdAt).not.toEqual(result.data.updatedAt); - }); - test('returns 201 if input is valid, test data is optional', async ({ request, }) => { diff --git a/tests/test-team/template-mgmt-component-tests/letter/template-mgmt-create-letter-template-page.component.spec.ts b/tests/test-team/template-mgmt-component-tests/letter/template-mgmt-create-letter-template-page.component.spec.ts index bca984210..6ee091484 100644 --- a/tests/test-team/template-mgmt-component-tests/letter/template-mgmt-create-letter-template-page.component.spec.ts +++ b/tests/test-team/template-mgmt-component-tests/letter/template-mgmt-create-letter-template-page.component.spec.ts @@ -22,7 +22,6 @@ test.describe('Upload letter Template Page', () => { let user: TestUser; let userWithoutCampaignId: TestUser; let userWithMultipleCampaigns: TestUser; - let userWithFallbackCampaignId: TestUser; test.beforeAll(async () => { const authHelper = createAuthHelper(); @@ -33,9 +32,6 @@ test.describe('Upload letter Template Page', () => { userWithMultipleCampaigns = await authHelper.getTestUser( testUsers.UserWithMultipleCampaigns.userId ); - userWithFallbackCampaignId = await authHelper.getTestUser( - testUsers.UserWithFallbackCampaignId.userId - ); }); test.afterAll(async () => { @@ -134,40 +130,6 @@ test.describe('Upload letter Template Page', () => { ]); }); - test('when user with a fallback campaign ID submits form with valid data, then the next page is displayed', async ({ - page, - }) => { - await loginAsUser(userWithFallbackCampaignId, page); - const createTemplatePage = new TemplateMgmtUploadLetterPage(page); - - await createTemplatePage.loadPage(); - await page - .locator('[id="letterTemplateName"]') - .fill('This is an NHS App template name'); - - await page.locator('input[name="letterTemplatePdf"]').click(); - await page - .locator('input[name="letterTemplatePdf"]') - .setInputFiles( - './fixtures/pdf-upload/with-personalisation/template.pdf' - ); - - await createTemplatePage.clickSaveAndPreviewButton(); - - const previewPageRegex = - /\/templates\/preview-letter-template\/([\dA-Fa-f-]+)(?:\?from=edit)?$/; - - // eslint-disable-next-line security/detect-non-literal-regexp - await expect(page).toHaveURL(new RegExp(previewPageRegex)); - - const previewPageParts = page.url().match(previewPageRegex); - expect(previewPageParts?.length).toEqual(2); - templateStorageHelper.addAdHocTemplateKey({ - templateId: previewPageParts![1], - clientId: user.clientId, - }); - }); - test('when user with a multiple campaign IDs submits form with valid data, then the next page is displayed', async ({ page, }) => {