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
2 changes: 1 addition & 1 deletion infrastructure/terraform/components/api/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ No requirements.
| <a name="module_get_letters"></a> [get\_letters](#module\_get\_letters) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-lambda.zip | n/a |
| <a name="module_kms"></a> [kms](#module\_kms) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-kms.zip | n/a |
| <a name="module_logging_bucket"></a> [logging\_bucket](#module\_logging\_bucket) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-s3bucket.zip | n/a |
| <a name="module_patch_letters"></a> [patch\_letters](#module\_patch\_letters) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-lambda.zip | n/a |
| <a name="module_patch_letter"></a> [patch\_letter](#module\_patch\_letter) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-lambda.zip | n/a |
| <a name="module_s3bucket_test_letters"></a> [s3bucket\_test\_letters](#module\_s3bucket\_test\_letters) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-s3bucket.zip | n/a |
| <a name="module_supplier_ssl"></a> [supplier\_ssl](#module\_supplier\_ssl) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-ssl.zip | n/a |
## Outputs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ data "aws_iam_policy_document" "api_gateway_execution_policy" {
resources = [
module.authorizer_lambda.function_arn,
module.get_letters.function_arn,
module.patch_letters.function_arn
module.patch_letter.function_arn
]
}
}
6 changes: 3 additions & 3 deletions infrastructure/terraform/components/api/locals.tf
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ locals {
AWS_REGION = var.region
AUTHORIZER_LAMBDA_ARN = module.authorizer_lambda.function_arn
GET_LETTERS_LAMBDA_ARN = module.get_letters.function_arn
PATCH_LETTERS_LAMBDA_ARN = module.patch_letters.function_arn
PATCH_LETTER_LAMBDA_ARN = module.patch_letter.function_arn
})

destination_arn = "arn:aws:logs:${var.region}:${var.shared_infra_account_id}:destination:nhs-main-obs-firehose-logs"

common_db_access_lambda_env_vars = {
common_lambda_env_vars = {
LETTERS_TABLE_NAME = aws_dynamodb_table.letters.name,
LETTER_TTL_HOURS = 24,
SUPPLIER_ID_HEADER = "nhsd-supplier-id"
SUPPLIER_ID_HEADER = "nhsd-correlation-id"
APIM_CORRELATION_HEADER = "nhsd-correlation-id"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ module "get_letters" {
log_destination_arn = local.destination_arn
log_subscription_role_arn = local.acct.log_subscription_role_arn

lambda_env_vars = merge(local.common_db_access_lambda_env_vars, {
lambda_env_vars = merge(local.common_lambda_env_vars, {
MAX_LIMIT = var.max_get_limit
})
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module "patch_letters" {
module "patch_letter" {
source = "https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-lambda.zip"

function_name = "patch_letters"
function_name = "patch_letter"
description = "Update the status of a letter"

aws_account_id = var.aws_account_id
Expand All @@ -15,14 +15,14 @@ module "patch_letters" {
kms_key_arn = module.kms.key_arn

iam_policy_document = {
body = data.aws_iam_policy_document.patch_letters_lambda.json
body = data.aws_iam_policy_document.patch_letter_lambda.json
}

function_s3_bucket = local.acct.s3_buckets["lambda_function_artefacts"]["id"]
function_code_base_path = local.aws_lambda_functions_dir_path
function_code_dir = "api-handler/dist"
function_include_common = true
handler_function_name = "patchLetters"
handler_function_name = "patchLetter"
runtime = "nodejs22.x"
memory = 128
timeout = 5
Expand All @@ -35,10 +35,10 @@ module "patch_letters" {
log_destination_arn = local.destination_arn
log_subscription_role_arn = local.acct.log_subscription_role_arn

lambda_env_vars = merge(local.common_db_access_lambda_env_vars, {})
lambda_env_vars = merge(local.common_lambda_env_vars, {})
}

data "aws_iam_policy_document" "patch_letters_lambda" {
data "aws_iam_policy_document" "patch_letter_lambda" {
statement {
sid = "KMSPermissions"
effect = "Allow"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
],
"patch": {
"description": "Update the status of a letter by providing the new status in the request body.",
"operationId": "patchLetters",
"operationId": "patchLetter",
"requestBody": {
"required": true
},
Expand Down Expand Up @@ -102,7 +102,7 @@
},
"timeoutInMillis": 29000,
"type": "AWS_PROXY",
"uri": "arn:aws:apigateway:${AWS_REGION}:lambda:path/2015-03-31/functions/${PATCH_LETTERS_LAMBDA_ARN}/invocations"
"uri": "arn:aws:apigateway:${AWS_REGION}:lambda:path/2015-03-31/functions/${PATCH_LETTER_LAMBDA_ARN}/invocations"
}
}
}
Expand Down
41 changes: 36 additions & 5 deletions internal/datastore/src/__test__/letter-repository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Letter } from '../types';
import { Logger } from 'pino';
import { createTestLogger, LogStream } from './logs';
import { PutCommand } from '@aws-sdk/lib-dynamodb';
import { LetterDto } from '../../../../lambdas/api-handler/src/contracts/letters';

function createLetter(supplierId: string, letterId: string, status: Letter['status'] = 'PENDING'): Omit<Letter, 'ttl' | 'supplierStatus' | 'supplierStatusSk'> {
return {
Expand Down Expand Up @@ -106,7 +107,14 @@ describe('LetterRepository', () => {
await letterRepository.putLetter(letter);
await checkLetterStatus('supplier1', 'letter1', 'PENDING');

await letterRepository.updateLetterStatus('supplier1', 'letter1', 'REJECTED', 1, "Reason text");
const letterDto: LetterDto = {
id: 'letter1',
supplierId: 'supplier1',
status: 'REJECTED',
reasonCode: 1,
reasonText: 'Reason text'
};
await letterRepository.updateLetterStatus(letterDto);

const updatedLetter = await letterRepository.getLetterById('supplier1', 'letter1');
expect(updatedLetter.status).toBe('REJECTED');
Expand All @@ -124,13 +132,25 @@ describe('LetterRepository', () => {
// Month is zero-indexed in JavaScript Date
// Day is one-indexed
jest.setSystemTime(new Date(2020, 1, 2));
await letterRepository.updateLetterStatus('supplier1', 'letter1', 'DELIVERED', undefined, undefined);
const letterDto: LetterDto = {
id: 'letter1',
supplierId: 'supplier1',
status: 'DELIVERED'
};

await letterRepository.updateLetterStatus(letterDto);
const updatedLetter = await letterRepository.getLetterById('supplier1', 'letter1');

expect(updatedLetter.updatedAt).toBe('2020-02-02T00:00:00.000Z');
});

test('can\'t update a letter that does not exist', async () => {
await expect(letterRepository.updateLetterStatus('supplier1', 'letter1', 'DELIVERED', undefined, undefined))
const letterDto: LetterDto = {
id: 'letter1',
supplierId: 'supplier1',
status: 'DELIVERED'
};
await expect(letterRepository.updateLetterStatus(letterDto))
.rejects.toThrow('Letter with id letter1 not found for supplier supplier1');
});

Expand All @@ -139,7 +159,13 @@ describe('LetterRepository', () => {
...db.config,
lettersTableName: 'nonexistent-table'
});
await expect(misconfiguredRepository.updateLetterStatus('supplier1', 'letter1', 'DELIVERED', undefined, undefined))

const letterDto: LetterDto = {
id: 'letter1',
supplierId: 'supplier1',
status: 'DELIVERED'
};
await expect(misconfiguredRepository.updateLetterStatus(letterDto))
.rejects.toThrow('Cannot do operations on a non-existent table');
});

Expand All @@ -164,7 +190,12 @@ describe('LetterRepository', () => {
const pendingLetters = await letterRepository.getLettersByStatus('supplier1', 'PENDING');
expect(pendingLetters.letters).toHaveLength(2);

await letterRepository.updateLetterStatus('supplier1', 'letter1', 'DELIVERED', undefined, undefined);
const letterDto: LetterDto = {
id: 'letter1',
supplierId: 'supplier1',
status: 'DELIVERED'
};
await letterRepository.updateLetterStatus(letterDto);
const remainingLetters = await letterRepository.getLettersByStatus('supplier1', 'PENDING');
expect(remainingLetters.letters).toHaveLength(1);
expect(remainingLetters.letters[0].id).toBe('letter2');
Expand Down
37 changes: 18 additions & 19 deletions internal/datastore/src/letter-repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
import { Letter, LetterBase, LetterSchema, LetterSchemaBase } from './types';
import { Logger } from 'pino';
import { z } from 'zod';
import { LetterDto } from '../../../lambdas/api-handler/src/contracts/letters';

export type PagingOptions = Partial<{
exclusiveStartKey: Record<string, any>,
Expand Down Expand Up @@ -133,37 +134,35 @@ export class LetterRepository {
}
}

async updateLetterStatus(supplierId: string, letterId: string, status: Letter['status'], reasonCode: number | undefined, reasonText: string | undefined): Promise<Letter> {
this.log.debug(`Updating letter ${letterId} to status ${status}`);
async updateLetterStatus(letterToUpdate: LetterDto): Promise<Letter> {
this.log.debug(`Updating letter ${letterToUpdate.id} to status ${letterToUpdate.status}`);
let result: UpdateCommandOutput;
try {
let updateExpression = 'set #status = :status, updatedAt = :updatedAt, supplierStatus = :supplierStatus, #ttl = :ttl';
let expressionAttributeValues = {
':status': status,
':updatedAt': new Date().toISOString(),
':supplierStatus': `${supplierId}#${status}`,
':ttl': Math.floor(Date.now() / 1000 + 60 * 60 * this.config.ttlHours),
...(!reasonCode && {':reasonCode': reasonCode}),
...(!reasonText && {':reasonText': reasonText})
};

if (reasonCode)
let expressionAttributeValues : Record<string, any> = {
':status': letterToUpdate.status,
':updatedAt': new Date().toISOString(),
':supplierStatus': `${letterToUpdate.supplierId}#${letterToUpdate.status}`,
':ttl': Math.floor(Date.now() / 1000 + 60 * 60 * this.config.ttlHours)
};

if (letterToUpdate.reasonCode)
{
updateExpression += ', reasonCode = :reasonCode';
expressionAttributeValues[':reasonCode'] = reasonCode;
expressionAttributeValues[':reasonCode'] = letterToUpdate.reasonCode;
}

if (reasonText)
if (letterToUpdate.reasonText)
{
updateExpression += ', reasonText = :reasonText';
expressionAttributeValues[':reasonText'] = reasonText;
expressionAttributeValues[':reasonText'] = letterToUpdate.reasonText;
}

result = await this.ddbClient.send(new UpdateCommand({
TableName: this.config.lettersTableName,
Key: {
supplierId: supplierId,
id: letterId
supplierId: letterToUpdate.supplierId,
id: letterToUpdate.id
},
UpdateExpression: updateExpression,
ConditionExpression: 'attribute_exists(id)', // Ensure letter exists
Expand All @@ -176,12 +175,12 @@ export class LetterRepository {
}));
} catch (error) {
if (error instanceof Error && error.name === 'ConditionalCheckFailedException') {
throw new Error(`Letter with id ${letterId} not found for supplier ${supplierId}`);
throw new Error(`Letter with id ${letterToUpdate.id} not found for supplier ${letterToUpdate.supplierId}`);
}
throw error;
}

this.log.debug(`Updated letter ${letterId} to status ${status}`);
this.log.debug(`Updated letter ${letterToUpdate.id} to status ${letterToUpdate.status}`);
return LetterSchema.parse(result.Attributes);
}

Expand Down
9 changes: 9 additions & 0 deletions lambdas/api-handler/src/contracts/json-api.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { z } from 'zod';

// Single document wrapper
export const makeDocumentSchema = <T extends z.ZodTypeAny>(resourceSchema: T) =>
z.object({ data: resourceSchema }).strict();

// Collection document wrapper
export const makeCollectionSchema = <T extends z.ZodTypeAny>(resourceSchema: T) =>
z.object({ data: z.array(resourceSchema) }).strict();
48 changes: 0 additions & 48 deletions lambdas/api-handler/src/contracts/letter-api.ts

This file was deleted.

69 changes: 69 additions & 0 deletions lambdas/api-handler/src/contracts/letters.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { z } from 'zod';
import { makeCollectionSchema, makeDocumentSchema } from './json-api';

export type LetterDto = {
id: string,
status: LetterStatus,
supplierId: string,
specificationId?: string,
groupId?: string,
reasonCode?: number,
reasonText?: string
};

export const LetterStatusSchema = z.enum([
'PENDING',
'ACCEPTED',
'REJECTED',
'PRINTED',
'ENCLOSED',
'CANCELLED',
'DISPATCHED',
'FAILED',
'RETURNED',
'DESTROYED',
'FORWARDED',
'DELIVERED'
]);

export const PatchLetterRequestResourceSchema = z.object({
id: z.string(),
type: z.literal('Letter'),
attributes: z.object({
status: LetterStatusSchema,
reasonCode: z.number().optional(),
reasonText: z.string().optional(),
}).strict()
}).strict();

export const PatchLetterResponseResourceSchema = z.object({
id: z.string(),
type: z.literal('Letter'),
attributes: z.object({
status: LetterStatusSchema,
specificationId: z.string(),
groupId: z.string().optional(),
reasonCode: z.number().optional(),
reasonText: z.string().optional(),
}).strict()
}).strict();

export const GetLettersResponseResourceSchema = z.object({
id: z.string(),
type: z.literal('Letter'),
attributes: z.object({
status: LetterStatusSchema,
specificationId: z.string(),
groupId: z.string().optional(),
}).strict()
}).strict();

export type LetterStatus = z.infer<typeof LetterStatusSchema>;

export const PatchLetterRequestSchema = makeDocumentSchema(PatchLetterRequestResourceSchema);
export const PatchLetterResponseSchema = makeDocumentSchema(PatchLetterResponseResourceSchema);
export const GetLettersResponseSchema = makeCollectionSchema(GetLettersResponseResourceSchema);

export type PatchLetterRequest = z.infer<typeof PatchLetterRequestSchema>;
export type PatchLetterResponse = z.infer<typeof PatchLetterResponseSchema>;
export type GetLettersResponse = z.infer<typeof GetLettersResponseSchema>;
Loading
Loading