Skip to content

Commit b91e208

Browse files
francisco-videira-nhsfrancisco-videira-nhs
committed
Last peer review actions
1 parent 1d0a6a5 commit b91e208

File tree

11 files changed

+292
-152
lines changed

11 files changed

+292
-152
lines changed

‎infrastructure/terraform/components/api/locals.tf‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ locals {
1717

1818
common_lambda_env_vars = {
1919
LETTERS_TABLE_NAME = aws_dynamodb_table.letters.name,
20-
LETTER_TTL_HOURS = 24,
20+
LETTER_TTL_HOURS = 12960, # 18 months * 30 days * 24 hours
2121
SUPPLIER_ID_HEADER = "nhsd-supplier-id",
2222
APIM_CORRELATION_HEADER = "nhsd-correlation-id",
23-
DOWNLOAD_URL_TTL_SECONDS = 3600
23+
DOWNLOAD_URL_TTL_SECONDS = 60
2424
}
2525
}

‎lambdas/api-handler/src/config/__tests__/deps.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ describe('createDependenciesContainer', () => {
3030
jest.mock('../env', () => ({
3131
envVars: {
3232
LETTERS_TABLE_NAME: 'LettersTable',
33-
LETTER_TTL_HOURS: 24,
33+
LETTER_TTL_HOURS: 12960,
3434
SUPPLIER_ID_HEADER: 'nhsd-supplier-id',
3535
APIM_CORRELATION_HEADER: 'nhsd-correlation-id',
36-
DOWNLOAD_URL_TTL_SECONDS: 3600
36+
DOWNLOAD_URL_TTL_SECONDS: 60
3737
},
3838
}));
3939
});
@@ -54,15 +54,15 @@ describe('createDependenciesContainer', () => {
5454
const repoCtorArgs = (LetterRepository as jest.Mock).mock.calls[0];
5555
expect(repoCtorArgs[2]).toEqual({
5656
lettersTableName: 'LettersTable',
57-
ttlHours: 24
57+
ttlHours: 12960
5858
});
5959

6060
expect(deps.env).toEqual({
6161
LETTERS_TABLE_NAME: 'LettersTable',
62-
LETTER_TTL_HOURS: 24,
62+
LETTER_TTL_HOURS: 12960,
6363
SUPPLIER_ID_HEADER: 'nhsd-supplier-id',
6464
APIM_CORRELATION_HEADER: 'nhsd-correlation-id',
65-
DOWNLOAD_URL_TTL_SECONDS: 3600
65+
DOWNLOAD_URL_TTL_SECONDS: 60
6666
});
6767
});
6868
});

‎lambdas/api-handler/src/config/__tests__/env.test.ts‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,30 +13,51 @@ describe('lambdaEnv', () => {
1313
});
1414

1515
it('should load all environment variables successfully', () => {
16-
process.env.SUPPLIER_ID_HEADER = 'x-supplier-id';
17-
process.env.APIM_CORRELATION_HEADER = 'x-correlation-id';
16+
process.env.SUPPLIER_ID_HEADER = 'nhsd-supplier-id';
17+
process.env.APIM_CORRELATION_HEADER = 'nhsd-correlation-id';
1818
process.env.LETTERS_TABLE_NAME = 'letters-table';
19-
process.env.LETTER_TTL_HOURS = '24';
20-
process.env.DOWNLOAD_URL_TTL_SECONDS = '3600';
19+
process.env.LETTER_TTL_HOURS = '12960';
20+
process.env.DOWNLOAD_URL_TTL_SECONDS = '60';
21+
process.env.MAX_LIMIT = '2500';
2122

2223
const { envVars } = require('../env');
2324

2425
expect(envVars).toEqual({
25-
SUPPLIER_ID_HEADER: 'x-supplier-id',
26-
APIM_CORRELATION_HEADER: 'x-correlation-id',
26+
SUPPLIER_ID_HEADER: 'nhsd-supplier-id',
27+
APIM_CORRELATION_HEADER: 'nhsd-correlation-id',
2728
LETTERS_TABLE_NAME: 'letters-table',
28-
LETTER_TTL_HOURS: 24,
29-
DOWNLOAD_URL_TTL_SECONDS: 3600
29+
LETTER_TTL_HOURS: 12960,
30+
DOWNLOAD_URL_TTL_SECONDS: 60,
31+
MAX_LIMIT: 2500,
3032
});
3133
});
3234

3335
it('should throw if a required env var is missing', () => {
34-
process.env.SUPPLIER_ID_HEADER = 'x-supplier-id';
35-
process.env.APIM_CORRELATION_HEADER = 'x-correlation-id';
36+
process.env.SUPPLIER_ID_HEADER = 'nhsd-supplier-id';
37+
process.env.APIM_CORRELATION_HEADER = 'nhsd-correlation-id';
3638
process.env.LETTERS_TABLE_NAME = undefined; // simulate missing var
37-
process.env.LETTER_TTL_HOURS = '24';
38-
process.env.DOWNLOAD_URL_TTL_SECONDS = '3600';
39+
process.env.LETTER_TTL_HOURS = '12960';
40+
process.env.DOWNLOAD_URL_TTL_SECONDS = '60';
3941

4042
expect(() => require('../env')).toThrow(ZodError);
4143
});
44+
45+
it('should not throw if optional are not set', () => {
46+
process.env.SUPPLIER_ID_HEADER = 'nhsd-supplier-id';
47+
process.env.APIM_CORRELATION_HEADER = 'nhsd-correlation-id';
48+
process.env.LETTERS_TABLE_NAME = 'letters-table';
49+
process.env.LETTER_TTL_HOURS = '12960';
50+
process.env.DOWNLOAD_URL_TTL_SECONDS = '60';
51+
52+
const { envVars } = require('../env');
53+
54+
expect(envVars).toEqual({
55+
SUPPLIER_ID_HEADER: 'nhsd-supplier-id',
56+
APIM_CORRELATION_HEADER: 'nhsd-correlation-id',
57+
LETTERS_TABLE_NAME: 'letters-table',
58+
LETTER_TTL_HOURS: 12960,
59+
DOWNLOAD_URL_TTL_SECONDS: 60,
60+
MAX_LIMIT: undefined
61+
});
62+
});
4263
});

‎lambdas/api-handler/src/config/env.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ const EnvVarsSchema = z.object({
55
APIM_CORRELATION_HEADER: z.string(),
66
LETTERS_TABLE_NAME: z.string(),
77
LETTER_TTL_HOURS: z.coerce.number().int(),
8-
DOWNLOAD_URL_TTL_SECONDS: z.coerce.number().int()
8+
DOWNLOAD_URL_TTL_SECONDS: z.coerce.number().int(),
9+
MAX_LIMIT: z.coerce.number().int().optional()
910
});
1011

1112
export type EnvVars = z.infer<typeof EnvVarsSchema>;

‎lambdas/api-handler/src/contracts/errors.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ export enum ApiErrorStatus {
2727

2828
export enum ApiErrorDetail {
2929
NotFoundLetterId = 'No resource found with that ID',
30-
InvalidRequestMissingSupplierId = 'The supplier ID is missing from the request',
3130
InvalidRequestMissingBody = 'The request is missing the body',
3231
InvalidRequestMissingLetterIdPathParameter = 'The request is missing the letter id path parameter',
3332
InvalidRequestLetterIdsMismatch = 'The letter ID in the request body does not match the letter ID path parameter',

‎lambdas/api-handler/src/handlers/__tests__/get-letter-data.test.ts‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ describe('API Lambda handler', () => {
3434
SUPPLIER_ID_HEADER: 'nhsd-supplier-id',
3535
APIM_CORRELATION_HEADER: 'nhsd-correlation-id',
3636
LETTERS_TABLE_NAME: 'LETTERS_TABLE_NAME',
37-
LETTER_TTL_HOURS: 1,
38-
DOWNLOAD_URL_TTL_SECONDS: 1
37+
LETTER_TTL_HOURS: 12960,
38+
DOWNLOAD_URL_TTL_SECONDS: 60
3939
} as unknown as EnvVars
4040
}
4141

@@ -48,8 +48,13 @@ describe('API Lambda handler', () => {
4848
const mockedGetLetterDataUrlService = letterService.getLetterDataUrl as jest.Mock;
4949
mockedGetLetterDataUrlService.mockResolvedValue('https://somePreSignedUrl.com');
5050

51-
const event = makeApiGwEvent({path: '/letters/letter1/data',
52-
headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'},
51+
const event = makeApiGwEvent({
52+
path: '/letters/letter1/data',
53+
headers: {
54+
'nhsd-supplier-id': 'supplier1',
55+
'nhsd-correlation-id': 'correlationId',
56+
'x-request-id': 'requestId'
57+
},
5358
pathParameters: {id: 'id1'}
5459
});
5560
const context = mockDeep<Context>();
@@ -67,7 +72,7 @@ describe('API Lambda handler', () => {
6772
});
6873
});
6974

70-
it('returns 400 for missing supplier ID (empty headers)', async () => {
75+
it('returns error if headers are empty', async () => {
7176
const event = makeApiGwEvent({ path: '/letters/letter1/data', headers: {},
7277
pathParameters: {id: 'id1'}
7378
});
@@ -81,11 +86,14 @@ describe('API Lambda handler', () => {
8186
expect(result).toEqual(expectedErrorResponse);
8287
});
8388

84-
it('returns 500 if correlation id not provided in request', async () => {
89+
it('returns error if correlation id not provided in request', async () => {
8590
const event = makeApiGwEvent({
8691
path: '/letters/letter1/data',
8792
queryStringParameters: { limit: '2000' },
88-
headers: {'nhsd-supplier-id': 'supplier1'},
93+
headers: {
94+
'nhsd-supplier-id': 'supplier1',
95+
'x-request-id': 'requestId'
96+
},
8997
pathParameters: {id: 'id1'}
9098
});
9199
const context = mockDeep<Context>();
@@ -101,7 +109,11 @@ describe('API Lambda handler', () => {
101109
it('returns error response when path parameter letterId is not found', async () => {
102110
const event = makeApiGwEvent({
103111
path: '/letters/',
104-
headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'}
112+
headers: {
113+
'nhsd-supplier-id': 'supplier1',
114+
'nhsd-correlation-id': 'correlationId',
115+
'x-request-id': 'requestId'
116+
},
105117
});
106118
const context = mockDeep<Context>();
107119
const callback = jest.fn();

0 commit comments

Comments
 (0)