Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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`] = `
<LetterTemplateForm
campaignIds={
[
"campaign-id",
]
}
initialState={
{
"campaignId": "campaign-id",
"language": "en",
"letterType": "x0",
"name": "",
"templateType": "LETTER",
}
}
/>
`;

exports[`UploadLetterTemplatePage should render UploadLetterTemplatePage with campaignIds field when available 1`] = `
<LetterTemplateForm
campaignIds={
Expand Down
28 changes: 3 additions & 25 deletions frontend/src/__tests__/app/upload-letter-template/page.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe('UploadLetterTemplatePage', () => {
});
mockFetchClient.mockResolvedValueOnce({
data: {
campaignId: 'campaign2',
campaignIds: ['campaign2'],
features: {},
},
});
Expand All @@ -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: {},
},
});
Expand Down Expand Up @@ -107,7 +87,7 @@ describe('UploadLetterTemplatePage', () => {
});
mockFetchClient.mockResolvedValueOnce({
data: {
campaignId: 'campaign2',
campaignIds: ['campaign2'],
features: {},
},
});
Expand All @@ -130,7 +110,6 @@ describe('UploadLetterTemplatePage', () => {
mockFetchClient.mockResolvedValueOnce({
data: {
campaignIds: [],
campaignId: 'campaign-id',
features: {},
},
});
Expand All @@ -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({
Expand All @@ -153,7 +132,6 @@ describe('UploadLetterTemplatePage', () => {
mockFetchClient.mockResolvedValueOnce({
data: {
campaignIds: undefined,
campaignId: undefined,
features: {},
},
});
Expand Down
10 changes: 2 additions & 8 deletions frontend/src/app/upload-letter-template/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
4 changes: 0 additions & 4 deletions infrastructure/terraform/modules/backend-api/spec.tmpl.json
Original file line number Diff line number Diff line change
Expand Up @@ -296,10 +296,6 @@
},
"ClientConfiguration": {
"properties": {
"campaignId": {
"deprecated": true,
"type": "string"
},
"campaignIds": {
"items": {
"type": "string"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ describe('Template API - get client configuration', () => {

const clientConfiguration: ClientConfiguration = {
features: { proofing: false },
campaignId: 'campaign',
campaignIds: ['campaign'],
};

mocks.templateClient.getClientConfiguration.mockResolvedValueOnce({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
)
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -247,8 +204,7 @@ describe('templateClient', () => {
expect(mocks.templateRepository.create).toHaveBeenCalledWith(
data,
user,
'NOT_YET_SUBMITTED',
undefined
'NOT_YET_SUBMITTED'
);

expect(result).toEqual({
Expand Down Expand Up @@ -298,8 +254,7 @@ describe('templateClient', () => {
expect(mocks.templateRepository.create).toHaveBeenCalledWith(
data,
user,
'NOT_YET_SUBMITTED',
undefined
'NOT_YET_SUBMITTED'
);

expect(result).toEqual({
Expand Down Expand Up @@ -354,8 +309,7 @@ describe('templateClient', () => {
expect(mocks.templateRepository.create).toHaveBeenCalledWith(
data,
user,
'NOT_YET_SUBMITTED',
undefined
'NOT_YET_SUBMITTED'
);

expect(result).toEqual({
Expand Down Expand Up @@ -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');
Expand All @@ -1779,7 +1733,7 @@ describe('templateClient', () => {

mocks.clientConfigRepository.get.mockResolvedValueOnce({
data: {
campaignId: 'campaignId',
campaignIds: ['campaignId'],
features: {
proofing: true,
},
Expand Down Expand Up @@ -1840,7 +1794,7 @@ describe('templateClient', () => {

mocks.clientConfigRepository.get.mockResolvedValueOnce({
data: {
campaignId: 'campaignId',
campaignIds: ['campaignId'],
features: {
proofing: true,
},
Expand Down Expand Up @@ -1891,7 +1845,7 @@ describe('templateClient', () => {

mocks.clientConfigRepository.get.mockResolvedValueOnce({
data: {
campaignId: 'campaignId',
campaignIds: ['campaignId'],
features: {
proofing: true,
},
Expand Down Expand Up @@ -1959,7 +1913,7 @@ describe('templateClient', () => {

mocks.clientConfigRepository.get.mockResolvedValueOnce({
data: {
campaignId: 'campaign-id-from-ssm',
campaignIds: ['campaign-id-from-ssm'],
features: {
proofing: true,
},
Expand Down Expand Up @@ -2027,7 +1981,7 @@ describe('templateClient', () => {

mocks.clientConfigRepository.get.mockResolvedValueOnce({
data: {
campaignId: 'campaign-from-ssm',
campaignIds: ['campaign-from-ssm'],
features: {
proofing: true,
},
Expand Down Expand Up @@ -2179,7 +2133,7 @@ describe('templateClient', () => {

const client: ClientConfiguration = {
features: { proofing: true },
campaignId: 'campaign',
campaignIds: ['campaign'],
};

mocks.clientConfigRepository.get.mockResolvedValueOnce({ data: client });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
26 changes: 3 additions & 23 deletions lambdas/backend-api/src/templates/app/template-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 0 additions & 4 deletions lambdas/backend-client/src/types/generated/types.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,6 @@ export type Channel = 'EMAIL' | 'LETTER' | 'NHSAPP' | 'SMS';
export type ChannelType = 'primary' | 'secondary';

export type ClientConfiguration = {
/**
* @deprecated
*/
campaignId?: string;
campaignIds?: Array<string>;
features: ClientFeatures;
};
Expand Down
2 changes: 1 addition & 1 deletion tests/accessibility/test-user-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
8 changes: 1 addition & 7 deletions tests/test-team/helpers/auth/cognito-auth-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,6 @@ export const testUsers: Record<string, TestUserStaticDetails> = {
clientKey: 'ClientWithMultipleCampaigns',
},

UserWithFallbackCampaignId: {
userId: 'UserWithFallbackCampaignId',
clientKey: 'ClientWithFallbackCampaignId',
},

/**
* UserRoutingEnabled belongs to an alternate client with routing enabled
*/
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -331,7 +326,6 @@ export class CognitoAuthHelper {
{
email,
userId: sub,
campaignId,
campaignIds,
clientId,
clientKey: userDetails.clientKey,
Expand Down
Loading
Loading