Skip to content

Commit 4699df3

Browse files
francisco-videira-nhsfrancisco-videira-nhs
authored
CCM-12915: Async Patch (#251)
* Add post letters endpoint * Fix names * Fix event mapper * Fix sqs * Fix arn * Fix sqs name * Fix param * Fix attempt * Fix attempt sqs * Fix attempt await * Clean up tests * Naming and minor refactor * Add duplicate validation * Change handler name * Fix unit tests * Change patch to return 202 and use sqs * Add await * Change mapper name
1 parent cd7f36b commit 4699df3

File tree

7 files changed

+94
-190
lines changed

7 files changed

+94
-190
lines changed

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

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ module "patch_letter" {
3535
log_destination_arn = local.destination_arn
3636
log_subscription_role_arn = local.acct.log_subscription_role_arn
3737

38-
lambda_env_vars = merge(local.common_lambda_env_vars, {})
38+
lambda_env_vars = merge(local.common_lambda_env_vars, {
39+
QUEUE_URL = module.letter_status_updates_queue.sqs_queue_url
40+
})
3941
}
4042

4143
data "aws_iam_policy_document" "patch_letter_lambda" {
@@ -54,21 +56,16 @@ data "aws_iam_policy_document" "patch_letter_lambda" {
5456
}
5557

5658
statement {
57-
sid = "AllowDynamoDBAccess"
59+
sid = "AllowQueueAccess"
5860
effect = "Allow"
5961

6062
actions = [
61-
"dynamodb:BatchGetItem",
62-
"dynamodb:BatchWriteItem",
63-
"dynamodb:GetItem",
64-
"dynamodb:PutItem",
65-
"dynamodb:Query",
66-
"dynamodb:Scan",
67-
"dynamodb:UpdateItem",
63+
"sqs:SendMessage",
64+
"sqs:GetQueueAttributes",
6865
]
6966

7067
resources = [
71-
aws_dynamodb_table.letters.arn,
68+
module.letter_status_updates_queue.sqs_queue_arn
7269
]
7370
}
7471
}

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

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// mock service
22
jest.mock('../../services/letter-operations');
33
import * as letterService from '../../services/letter-operations';
4-
const mockedPatchLetterStatus = jest.mocked(letterService.patchLetterStatus);
4+
const mockedBatchUpdateStatus = jest.mocked(letterService.enqueueLetterUpdateRequests);
55

66
// mock mapper
77
jest.mock('../../mappers/error-mapper');
@@ -59,7 +59,7 @@ describe('patchLetter API Handler', () => {
5959
} as unknown as EnvVars
6060
} as Deps;
6161

62-
it('returns 200 OK with updated resource', async () => {
62+
it('returns 202 Accepted', async () => {
6363
const event = makeApiGwEvent({
6464
path: '/letters/id1',
6565
body: requestBody,
@@ -86,14 +86,14 @@ describe('patchLetter API Handler', () => {
8686
}
8787
}
8888
};
89-
mockedPatchLetterStatus.mockResolvedValue(updateLetterServiceResponse);
89+
mockedBatchUpdateStatus.mockResolvedValue();
9090

9191
const patchLetterHandler = createPatchLetterHandler(mockedDeps);
9292
const result = await patchLetterHandler(event, context, callback);
9393

9494
expect(result).toEqual({
95-
statusCode: 200,
96-
body: JSON.stringify(updateLetterServiceResponse, null, 2)
95+
statusCode: 202,
96+
body: ''
9797
});
9898
});
9999

@@ -137,16 +137,12 @@ describe('patchLetter API Handler', () => {
137137
expect(result).toEqual(expectedErrorResponse);
138138
});
139139

140-
it('returns error response when error is thrown by service', async () => {
141-
const error = new Error('Service error');
142-
mockedPatchLetterStatus.mockRejectedValue(error);
143-
140+
it('returns error when supplier id is missing', async () => {
144141
const event = makeApiGwEvent({
145142
path: '/letters/id1',
146143
body: requestBody,
147144
pathParameters: {id: 'id1'},
148145
headers: {
149-
'nhsd-supplier-id': 'supplier1',
150146
'nhsd-correlation-id': 'correlationId',
151147
'x-request-id': 'requestId'
152148
}
@@ -157,16 +153,17 @@ describe('patchLetter API Handler', () => {
157153
const patchLetterHandler = createPatchLetterHandler(mockedDeps);
158154
const result = await patchLetterHandler(event, context, callback);
159155

160-
expect(mockedProcessError).toHaveBeenCalledWith(error, 'correlationId', mockedDeps.logger);
156+
expect(mockedProcessError).toHaveBeenCalledWith(new Error('The supplier ID is missing from the request'), 'correlationId', mockedDeps.logger);
161157
expect(result).toEqual(expectedErrorResponse);
162158
});
163159

164-
it('returns error when supplier id is missing', async () => {
160+
it('returns error when request body does not have correct shape', async () => {
165161
const event = makeApiGwEvent({
166162
path: '/letters/id1',
167-
body: requestBody,
163+
body: "{test: 'test'}",
168164
pathParameters: {id: 'id1'},
169165
headers: {
166+
'nhsd-supplier-id': 'supplier1',
170167
'nhsd-correlation-id': 'correlationId',
171168
'x-request-id': 'requestId'
172169
}
@@ -177,14 +174,14 @@ describe('patchLetter API Handler', () => {
177174
const patchLetterHandler = createPatchLetterHandler(mockedDeps);
178175
const result = await patchLetterHandler(event, context, callback);
179176

180-
expect(mockedProcessError).toHaveBeenCalledWith(new Error('The supplier ID is missing from the request'), 'correlationId', mockedDeps.logger);
177+
expect(mockedProcessError).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestBody), 'correlationId', mockedDeps.logger);
181178
expect(result).toEqual(expectedErrorResponse);
182179
});
183180

184-
it('returns error when request body does not have correct shape', async () => {
181+
it('returns error when request body is not json', async () => {
185182
const event = makeApiGwEvent({
186183
path: '/letters/id1',
187-
body: "{test: 'test'}",
184+
body: '{#invalidJSON',
188185
pathParameters: {id: 'id1'},
189186
headers: {
190187
'nhsd-supplier-id': 'supplier1',
@@ -202,11 +199,11 @@ describe('patchLetter API Handler', () => {
202199
expect(result).toEqual(expectedErrorResponse);
203200
});
204201

205-
it('returns error when request body is not json', async () => {
202+
it('returns error if path letterId and body letterId do not match', async () => {
206203
const event = makeApiGwEvent({
207-
path: '/letters/id1',
208-
body: '{#invalidJSON',
209-
pathParameters: {id: 'id1'},
204+
path: '/letters/id2',
205+
body: requestBody,
206+
pathParameters: {id: 'id2'},
210207
headers: {
211208
'nhsd-supplier-id': 'supplier1',
212209
'nhsd-correlation-id': 'correlationId',
@@ -219,7 +216,7 @@ describe('patchLetter API Handler', () => {
219216
const patchLetterHandler = createPatchLetterHandler(mockedDeps);
220217
const result = await patchLetterHandler(event, context, callback);
221218

222-
expect(mockedProcessError).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestBody), 'correlationId', mockedDeps.logger);
219+
expect(mockedProcessError).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestLetterIdsMismatch), 'correlationId', mockedDeps.logger);
223220
expect(result).toEqual(expectedErrorResponse);
224221
});
225222

‎lambdas/api-handler/src/handlers/patch-letter.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { APIGatewayProxyHandler } from 'aws-lambda';
2-
import { patchLetterStatus } from '../services/letter-operations';
3-
import { PatchLetterRequest, PatchLetterRequestSchema } from '../contracts/letters';
2+
import { enqueueLetterUpdateRequests } from '../services/letter-operations';
3+
import { LetterDto, PatchLetterRequest, PatchLetterRequestSchema } from '../contracts/letters';
44
import { ApiErrorDetail } from '../contracts/errors';
55
import { ValidationError } from '../errors';
66
import { processError } from '../mappers/error-mapper';
@@ -36,11 +36,17 @@ export function createPatchLetterHandler(deps: Deps): APIGatewayProxyHandler {
3636
else throw error;
3737
}
3838

39-
const updatedLetter = await patchLetterStatus(mapPatchLetterToDto(patchLetterRequest, commonIds.value.supplierId), letterId, deps.letterRepo);
39+
const letterToUpdate: LetterDto = mapPatchLetterToDto(patchLetterRequest, commonIds.value.supplierId);
40+
41+
if (letterToUpdate.id !== letterId) {
42+
throw new ValidationError(ApiErrorDetail.InvalidRequestLetterIdsMismatch);
43+
}
44+
45+
await enqueueLetterUpdateRequests([letterToUpdate], commonIds.value.correlationId, deps);
4046

4147
return {
42-
statusCode: 200,
43-
body: JSON.stringify(updatedLetter, null, 2)
48+
statusCode: 202,
49+
body: ''
4450
};
4551

4652
} catch (error) {

‎lambdas/api-handler/src/handlers/post-letters.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { ApiErrorDetail } from '../contracts/errors';
44
import { PostLettersRequest, PostLettersRequestSchema } from '../contracts/letters';
55
import { ValidationError } from '../errors';
66
import { processError } from '../mappers/error-mapper';
7+
import { mapPostLettersToDtoArray } from '../mappers/letter-mapper';
78
import { enqueueLetterUpdateRequests } from '../services/letter-operations';
89
import { extractCommonIds } from '../utils/commonIds';
910
import { assertNotEmpty, requireEnvVar } from '../utils/validation';
@@ -43,7 +44,11 @@ export function createPostLettersHandler(deps: Deps): APIGatewayProxyHandler {
4344
throw new ValidationError(ApiErrorDetail.InvalidRequestDuplicateLetterId);
4445
}
4546

46-
await enqueueLetterUpdateRequests(postLettersRequest, commonIds.value.supplierId, commonIds.value.correlationId, deps);
47+
await enqueueLetterUpdateRequests(
48+
mapPostLettersToDtoArray(postLettersRequest, commonIds.value.supplierId),
49+
commonIds.value.correlationId,
50+
deps
51+
);
4752

4853
return {
4954
statusCode: 202,

‎lambdas/api-handler/src/mappers/letter-mapper.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@ export function mapPatchLetterToDto(request: PatchLetterRequest, supplierId: str
1111
};
1212
}
1313

14-
export function mapPostLetterResourceToDto(request: PostLettersRequestResource, supplierId: string): LetterDto {
15-
return {
16-
id: request.id,
14+
export function mapPostLettersToDtoArray(request: PostLettersRequest, supplierId: string): LetterDto[] {
15+
return request.data.map( (letterToUpdate: PostLettersRequestResource) => ({
16+
id: letterToUpdate.id,
1717
supplierId,
18-
status: LetterStatus.parse(request.attributes.status),
19-
reasonCode: request.attributes.reasonCode,
20-
reasonText: request.attributes.reasonText,
21-
};
18+
status: LetterStatus.parse(letterToUpdate.attributes.status),
19+
reasonCode: letterToUpdate.attributes.reasonCode,
20+
reasonText: letterToUpdate.attributes.reasonText,
21+
}));
2222
}
2323

2424
export function mapToPatchLetterResponse(letter: LetterBase): PatchLetterResponse {

0 commit comments

Comments
 (0)