diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 000000000..c0aed7b0f --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,50 @@ +{ + "configurations": [ + { + "args": [ + "${relativeFile}", + "--runInBand" + ], + "console": "integratedTerminal", + "cwd": "${workspaceFolder}/${input:workspace}", + "internalConsoleOptions": "neverOpen", + "name": "Debug current test file (pick workspace)", + "program": "${workspaceFolder}/node_modules/.bin/jest", + "request": "launch", + "type": "node" + }, + { + "args": [ + "${relativeFile}", + "--runInBand", + "--testNamePattern", + "${input:testName}" + ], + "console": "integratedTerminal", + "cwd": "${workspaceFolder}/${input:workspace}", + "internalConsoleOptions": "neverOpen", + "name": "Debug single test by name (pick workspace)", + "program": "${workspaceFolder}/node_modules/.bin/jest", + "request": "launch", + "type": "node" + } + ], + "inputs": [ + { + "description": "Select package workspace to run tests in", + "id": "workspace", + "options": [ + "lambdas/api-handler", + "lambdas/authorizer", + "internal/datastore" + ], + "type": "pickString" + }, + { + "description": "Enter test name regex for --testNamePattern", + "id": "testName", + "type": "promptString" + } + ], + "version": "0.0.1" +} diff --git a/infrastructure/terraform/components/api/README.md b/infrastructure/terraform/components/api/README.md index ee77c6c38..d48845d84 100644 --- a/infrastructure/terraform/components/api/README.md +++ b/infrastructure/terraform/components/api/README.md @@ -32,12 +32,12 @@ No requirements. | Name | Source | Version | |------|--------|---------| | [authorizer\_lambda](#module\_authorizer\_lambda) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-lambda.zip | n/a | -| [domain\_truststore](#module\_domain\_truststore) | git::https://github.com/NHSDigital/nhs-notify-shared-modules.git//infrastructure/modules/s3bucket | v2.0.17 | +| [domain\_truststore](#module\_domain\_truststore) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-s3bucket.zip | n/a | | [get\_letters](#module\_get\_letters) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-lambda.zip | n/a | | [kms](#module\_kms) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-kms.zip | n/a | -| [logging\_bucket](#module\_logging\_bucket) | git::https://github.com/NHSDigital/nhs-notify-shared-modules.git//infrastructure/modules/s3bucket | v2.0.17 | +| [logging\_bucket](#module\_logging\_bucket) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-s3bucket.zip | n/a | | [patch\_letters](#module\_patch\_letters) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-lambda.zip | n/a | -| [supplier\_ssl](#module\_supplier\_ssl) | git::https://github.com/NHSDigital/nhs-notify-shared-modules.git//infrastructure/modules/ssl | v2.0.17 | +| [supplier\_ssl](#module\_supplier\_ssl) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-ssl.zip | n/a | ## Outputs | Name | Description | diff --git a/infrastructure/terraform/components/api/locals.tf b/infrastructure/terraform/components/api/locals.tf index 832e819cc..1ee9232f0 100644 --- a/infrastructure/terraform/components/api/locals.tf +++ b/infrastructure/terraform/components/api/locals.tf @@ -13,4 +13,11 @@ locals { }) destination_arn = "arn:aws:logs:${var.region}:${var.shared_infra_account_id}:destination:nhs-main-obs-firehose-logs" + + common_db_access_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" + } } diff --git a/infrastructure/terraform/components/api/module_domain_truststore.tf b/infrastructure/terraform/components/api/module_domain_truststore.tf index 24de9b2a9..75ba5d445 100644 --- a/infrastructure/terraform/components/api/module_domain_truststore.tf +++ b/infrastructure/terraform/components/api/module_domain_truststore.tf @@ -1,5 +1,5 @@ module "domain_truststore" { - source = "git::https://github.com/NHSDigital/nhs-notify-shared-modules.git//infrastructure/modules/s3bucket?ref=v2.0.17" + source = "https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-s3bucket.zip" name = "truststore" aws_account_id = var.aws_account_id diff --git a/infrastructure/terraform/components/api/module_lambda_get_letters.tf b/infrastructure/terraform/components/api/module_lambda_get_letters.tf index 1b53a86b6..d654fa174 100644 --- a/infrastructure/terraform/components/api/module_lambda_get_letters.tf +++ b/infrastructure/terraform/components/api/module_lambda_get_letters.tf @@ -35,11 +35,9 @@ module "get_letters" { log_destination_arn = local.destination_arn log_subscription_role_arn = local.acct.log_subscription_role_arn - lambda_env_vars = { - LETTERS_TABLE_NAME = aws_dynamodb_table.letters.name, - LETTER_TTL_HOURS = var.letter_table_ttl_hours, - MAX_LIMIT = var.max_get_limit, - } + lambda_env_vars = merge(local.common_db_access_lambda_env_vars, { + MAX_LIMIT = var.max_get_limit + }) } data "aws_iam_policy_document" "get_letters_lambda" { diff --git a/infrastructure/terraform/components/api/module_lambda_patch_letters.tf b/infrastructure/terraform/components/api/module_lambda_patch_letters.tf index 225b88d46..c99388572 100644 --- a/infrastructure/terraform/components/api/module_lambda_patch_letters.tf +++ b/infrastructure/terraform/components/api/module_lambda_patch_letters.tf @@ -35,10 +35,7 @@ module "patch_letters" { log_destination_arn = local.destination_arn log_subscription_role_arn = local.acct.log_subscription_role_arn - lambda_env_vars = { - LETTERS_TABLE_NAME = aws_dynamodb_table.letters.name, - LETTER_TTL_HOURS = 24 - } + lambda_env_vars = merge(local.common_db_access_lambda_env_vars, {}) } data "aws_iam_policy_document" "patch_letters_lambda" { diff --git a/infrastructure/terraform/components/api/module_logging_bucket.tf b/infrastructure/terraform/components/api/module_logging_bucket.tf index 1b7820fdb..da4bfba11 100644 --- a/infrastructure/terraform/components/api/module_logging_bucket.tf +++ b/infrastructure/terraform/components/api/module_logging_bucket.tf @@ -1,5 +1,5 @@ module "logging_bucket" { - source = "git::https://github.com/NHSDigital/nhs-notify-shared-modules.git//infrastructure/modules/s3bucket?ref=v2.0.17" + source = "https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-s3bucket.zip" name = "bucket-logs" aws_account_id = var.aws_account_id diff --git a/infrastructure/terraform/components/api/module_supplier_ssl.tf b/infrastructure/terraform/components/api/module_supplier_ssl.tf index a7d4c8cf8..63535da03 100644 --- a/infrastructure/terraform/components/api/module_supplier_ssl.tf +++ b/infrastructure/terraform/components/api/module_supplier_ssl.tf @@ -1,7 +1,7 @@ module "supplier_ssl" { count = var.manually_configure_mtls_truststore ? 0 : 1 - source = "git::https://github.com/NHSDigital/nhs-notify-shared-modules.git//infrastructure/modules/ssl?ref=v2.0.17" + source = "https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-ssl.zip" name = "sapi_trust" aws_account_id = var.aws_account_id diff --git a/infrastructure/terraform/components/api/resources/spec.tmpl.json b/infrastructure/terraform/components/api/resources/spec.tmpl.json index 070c3cfea..193d08d18 100644 --- a/infrastructure/terraform/components/api/resources/spec.tmpl.json +++ b/infrastructure/terraform/components/api/resources/spec.tmpl.json @@ -67,146 +67,22 @@ ], "patch": { "description": "Update the status of a letter by providing the new status in the request body.", + "operationId": "patchLetters", "requestBody": { - "content": { - "application/vnd.api+json": { - "schema": { - "properties": { - "data": { - "properties": { - "attributes": { - "properties": { - "reasonCode": { - "description": "Reason code for the given status", - "type": "number" - }, - "reasonText": { - "description": "Reason text for the given status", - "type": "string" - }, - "requestedProductionStatus": { - "description": "The requested production status for this letter.\nMay only be set by NHS Notify.", - "enum": [ - "ACTIVE", - "HOLD", - "CANCEL" - ], - "title": "ProductionStatus", - "type": "string" - }, - "status": { - "default": "PENDING", - "description": "The supplier status of an individual letter", - "enum": [ - "PENDING", - "ACCEPTED", - "REJECTED", - "PRINTED", - "ENCLOSED", - "CANCELLED", - "DISPATCHED", - "FAILED", - "RETURNED", - "DESTROYED", - "FORWARDED" - ], - "type": "string" - } - }, - "type": "object" - }, - "id": { - "type": "string" - }, - "type": { - "const": "Letter", - "type": "string" - } - }, - "type": "object" - } - }, - "type": "object" - } - } - }, "required": true }, "responses": { "200": { - "content": { - "application/vnd.api+json": { - "schema": { - "properties": { - "data": { - "properties": { - "attributes": { - "properties": { - "reasonCode": { - "description": "Reason code for the given status", - "type": "number" - }, - "reasonText": { - "description": "Reason text for the given status", - "type": "string" - }, - "requestedProductionStatus": { - "description": "A requested status for the production of a letter", - "enum": [ - "ACTIVE", - "HOLD", - "CANCEL" - ], - "title": "ProductionStatus", - "type": "string" - }, - "status": { - "default": "PENDING", - "description": "The supplier status of an individual letter", - "enum": [ - "PENDING", - "ACCEPTED", - "REJECTED", - "PRINTED", - "ENCLOSED", - "CANCELLED", - "DISPATCHED", - "FAILED", - "RETURNED", - "DESTROYED", - "FORWARDED" - ], - "type": "string" - } - }, - "required": [ - "status", - "requestedProductionStatus" - ], - "type": "object" - }, - "id": { - "type": "string" - }, - "type": { - "const": "Letter", - "type": "string" - } - }, - "type": "object" - } - }, - "type": "object" - } - } - }, - "description": "Letter resource updated successfully" + "description": "List of letters to process" }, "400": { - "description": "Bad request" + "description": "Bad request, invalid input data" }, "404": { "description": "Resource not found" + }, + "500": { + "description": "Server error" } }, "security": [ @@ -214,7 +90,6 @@ "LambdaAuthorizer": [] } ], - "summary": "Update the status of a letter", "x-amazon-apigateway-integration": { "contentHandling": "CONVERT_TO_TEXT", "credentials": "${APIG_EXECUTION_ROLE_ARN}", diff --git a/internal/datastore/src/__test__/letter-repository.test.ts b/internal/datastore/src/__test__/letter-repository.test.ts index 8f4caff24..34712b397 100644 --- a/internal/datastore/src/__test__/letter-repository.test.ts +++ b/internal/datastore/src/__test__/letter-repository.test.ts @@ -50,21 +50,23 @@ describe('LetterRepository', () => { await db.container.stop(); }); - async function checkLetterExists(supplierId: string, letterId: string) { - const letter = await letterRepository.getLetterById(supplierId, letterId); - expect(letter).toBeDefined(); - expect(letter.id).toBe(letterId); - expect(letter.supplierId).toBe(supplierId); - } - async function checkLetterStatus(supplierId: string, letterId: string, status: Letter['status']) { const letter = await letterRepository.getLetterById(supplierId, letterId); expect(letter.status).toBe(status); } test('adds a letter to the database', async () => { - await letterRepository.putLetter(createLetter('supplier1', 'letter1')); - await checkLetterExists('supplier1', 'letter1'); + const supplierId = 'supplier1'; + const letterId = 'letter1'; + + await letterRepository.putLetter(createLetter(supplierId, letterId)); + + const letter = await letterRepository.getLetterById(supplierId, letterId); + expect(letter).toBeDefined(); + expect(letter.id).toBe(letterId); + expect(letter.supplierId).toBe(supplierId); + expect(letter.reasonCode).toBeUndefined(); + expect(letter.reasonText).toBeUndefined(); }); test('fetches a letter by id', async () => { @@ -100,11 +102,16 @@ describe('LetterRepository', () => { }); test('updates a letter\'s status in the database', async () => { - await letterRepository.putLetter(createLetter('supplier1', 'letter1', 'PENDING')); + const letter = createLetter('supplier1', 'letter1', 'PENDING'); + await letterRepository.putLetter(letter); await checkLetterStatus('supplier1', 'letter1', 'PENDING'); - await letterRepository.updateLetterStatus('supplier1', 'letter1', 'DELIVERED'); - await checkLetterStatus('supplier1', 'letter1', 'DELIVERED'); + await letterRepository.updateLetterStatus('supplier1', 'letter1', 'REJECTED', 1, "Reason text"); + + const updatedLetter = await letterRepository.getLetterById('supplier1', 'letter1'); + expect(updatedLetter.status).toBe('REJECTED'); + expect(updatedLetter.reasonCode).toBe(1); + expect(updatedLetter.reasonText).toBe('Reason text'); }); test('updates a letter\'s updatedAt date', async () => { @@ -117,13 +124,13 @@ 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'); + await letterRepository.updateLetterStatus('supplier1', 'letter1', 'DELIVERED', undefined, undefined); 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')) + await expect(letterRepository.updateLetterStatus('supplier1', 'letter1', 'DELIVERED', undefined, undefined)) .rejects.toThrow('Letter with id letter1 not found for supplier supplier1'); }); @@ -132,7 +139,7 @@ describe('LetterRepository', () => { ...db.config, lettersTableName: 'nonexistent-table' }); - await expect(misconfiguredRepository.updateLetterStatus('supplier1', 'letter1', 'DELIVERED')) + await expect(misconfiguredRepository.updateLetterStatus('supplier1', 'letter1', 'DELIVERED', undefined, undefined)) .rejects.toThrow('Cannot do operations on a non-existent table'); }); @@ -157,7 +164,7 @@ describe('LetterRepository', () => { const pendingLetters = await letterRepository.getLettersByStatus('supplier1', 'PENDING'); expect(pendingLetters.letters).toHaveLength(2); - await letterRepository.updateLetterStatus('supplier1', 'letter1', 'DELIVERED'); + await letterRepository.updateLetterStatus('supplier1', 'letter1', 'DELIVERED', undefined, undefined); const remainingLetters = await letterRepository.getLettersByStatus('supplier1', 'PENDING'); expect(remainingLetters.letters).toHaveLength(1); expect(remainingLetters.letters[0].id).toBe('letter2'); diff --git a/internal/datastore/src/letter-repository.ts b/internal/datastore/src/letter-repository.ts index 3c9acdf3a..c6c3f729f 100644 --- a/internal/datastore/src/letter-repository.ts +++ b/internal/datastore/src/letter-repository.ts @@ -34,7 +34,7 @@ export class LetterRepository { const letterDb: Letter = { ...letter, supplierStatus: `${letter.supplierId}#${letter.status}`, - supplierStatusSk: Date.now().toString(), + supplierStatusSk: new Date().toISOString(), ttl: Math.floor(Date.now() / 1000 + 60 * 60 * this.config.ttlHours) }; try { @@ -97,28 +97,45 @@ export class LetterRepository { } } - async updateLetterStatus(supplierId: string, letterId: string, status: Letter['status']): Promise { + async updateLetterStatus(supplierId: string, letterId: string, status: Letter['status'], reasonCode: number | undefined, reasonText: string | undefined): Promise { this.log.debug(`Updating letter ${letterId} to status ${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) + { + updateExpression += ', reasonCode = :reasonCode'; + expressionAttributeValues[':reasonCode'] = reasonCode; + } + + if (reasonText) + { + updateExpression += ', reasonText = :reasonText'; + expressionAttributeValues[':reasonText'] = reasonText; + } + result = await this.ddbClient.send(new UpdateCommand({ TableName: this.config.lettersTableName, Key: { supplierId: supplierId, id: letterId }, - UpdateExpression: 'set #status = :status, updatedAt = :updatedAt, supplierStatus = :supplierStatus, #ttl = :ttl', + UpdateExpression: updateExpression, ConditionExpression: 'attribute_exists(id)', // Ensure letter exists ExpressionAttributeNames: { '#status': 'status', '#ttl': 'ttl' }, - ExpressionAttributeValues: { - ':status': status, - ':updatedAt': new Date().toISOString(), - ':supplierStatus': `${supplierId}#${status}`, - ':ttl': Math.floor(Date.now() / 1000 + 60 * 60 * this.config.ttlHours) - }, + ExpressionAttributeValues: expressionAttributeValues, ReturnValues: 'ALL_NEW' })); } catch (error) { diff --git a/lambdas/api-handler/src/config/lambda-config.ts b/lambdas/api-handler/src/config/lambda-config.ts new file mode 100644 index 000000000..7ff98a0f7 --- /dev/null +++ b/lambdas/api-handler/src/config/lambda-config.ts @@ -0,0 +1,17 @@ +interface LambdaConfig { + SUPPLIER_ID_HEADER: string; + APIM_CORRELATION_HEADER: string; +} + +export const lambdaConfig: LambdaConfig = { + SUPPLIER_ID_HEADER: getEnv("SUPPLIER_ID_HEADER")!, + APIM_CORRELATION_HEADER: getEnv("APIM_CORRELATION_HEADER")! +}; + +function getEnv(name: string, required = true): string | undefined { + const value = process.env[name]; + if (!value && required) { + throw new Error(`Missing required env var: ${name}`); + } + return value; +} diff --git a/lambdas/api-handler/src/contracts/errors.ts b/lambdas/api-handler/src/contracts/errors.ts new file mode 100644 index 000000000..9dc98f607 --- /dev/null +++ b/lambdas/api-handler/src/contracts/errors.ts @@ -0,0 +1,56 @@ +export interface ApiError { + id: string; + code: ApiErrorCode; + links: {about: 'https://digital.nhs.uk/developer/api-catalogue/nhs-notify-supplier'}; + status: ApiErrorStatus; + title: ApiErrorTitle; + detail: ApiErrorDetail | string; +} + +export enum ApiErrorCode { + InternalServerError = 'NOTIFY_INTERNAL_SERVER_ERROR', + InvalidRequest = 'NOTIFY_INVALID_REQUEST', + NotFound = 'NOTIFY_LETTER_NOT_FOUND' +} + +export enum ApiErrorTitle { + InternalServerError = 'Internal server error', + InvalidRequest = 'Invalid request', + NotFound = 'Not found' +} + +export enum ApiErrorStatus { + InternalServerError = "500", + InvalidRequest = "400", + NotFound = "404" +} + +export enum ApiErrorDetail { + NotFoundLetterId = 'No resource found with that ID', + InvalidRequestMissingSupplierId = 'The supplier ID is missing from the request', + InvalidRequestMissingBody = 'The request is missing the body', + InvalidRequestMissingLetterIdPathParameter = 'The request is missing the letter id path parameter', + InvalidRequestLetterIdsMismatch = 'The letter ID in the request body does not match the letter ID path parameter', + InvalidRequestBody = 'The request body is invalid', + InvalidRequestLimitNotANumber = 'The limit parameter is not a number', + InvalidRequestLimitNotInRange = 'The limit parameter must be a positive number not greater than %s', + InvalidRequestLimitOnly = "Only 'limit' query parameter is supported", + InvalidRequestNoRequestId = 'The request does not contain a request id' +} + +export function buildApiError(params: { + id: string + code: ApiErrorCode; + status: ApiErrorStatus; + title: ApiErrorTitle; + detail: ApiErrorDetail | string; +}): ApiError { + return { + id: params.id, + code: params.code, + links: { about: 'https://digital.nhs.uk/developer/api-catalogue/nhs-notify-supplier' }, + status: params.status, + title: params.title, + detail: params.detail, + }; +} diff --git a/lambdas/api-handler/src/contracts/letter-api.ts b/lambdas/api-handler/src/contracts/letter-api.ts index 39522b4ba..ec3c8647f 100644 --- a/lambdas/api-handler/src/contracts/letter-api.ts +++ b/lambdas/api-handler/src/contracts/letter-api.ts @@ -1,18 +1,48 @@ -export interface LetterApiAttributes { - reasonCode: number; - reasonText: string; - requestedProductionStatus: 'ACTIVE' | 'HOLD' | 'CANCEL'; - status: LetterApiStatus; -} - -export interface LetterApiResource { - id: string; - type: 'Letter'; - attributes: LetterApiAttributes; -} - -export interface LetterApiDocument { - data: LetterApiResource; -} - -export type LetterApiStatus = 'PENDING' | 'ACCEPTED' | 'REJECTED' | 'PRINTED' | 'ENCLOSED' | 'CANCELLED' | 'DISPATCHED' | 'FAILED' | 'RETURNED' | 'DESTROYED' | 'FORWARDED' | 'DELIVERED'; +import { z } from "zod"; + +export const LetterApiStatusSchema = z.enum([ + "PENDING", + "ACCEPTED", + "REJECTED", + "PRINTED", + "ENCLOSED", + "CANCELLED", + "DISPATCHED", + "FAILED", + "RETURNED", + "DESTROYED", + "FORWARDED", + "DELIVERED", +]); + +export type LetterApiStatus = z.infer; + +export const LetterApiAttributesSchema = z.object({ + status: LetterApiStatusSchema, + specificationId: z.string(), + groupId: z.string().optional(), + reasonCode: z.number().optional(), + reasonText: z.string().optional(), +}); + +export type LetterApiAttributes = z.infer; + +export const LetterApiResourceSchema = z.object({ + id: z.string(), + type: z.literal("Letter"), + attributes: LetterApiAttributesSchema +}); + +export type LetterApiResource = z.infer; + +export const LetterApiDocumentSchema = z.object({ + data: LetterApiResourceSchema +}); + +export const LettersApiDocumentSchema = z.object({ + data: z.array(LetterApiResourceSchema) +}); + +export type LetterApiDocument = z.infer; + +export type LettersApiDocument = z.infer; diff --git a/lambdas/api-handler/src/errors/index.ts b/lambdas/api-handler/src/errors/index.ts index 7237b956a..eeeb179f8 100644 --- a/lambdas/api-handler/src/errors/index.ts +++ b/lambdas/api-handler/src/errors/index.ts @@ -1,3 +1,15 @@ -export class NotFoundError extends Error {} -export class ValidationError extends Error {} -export class ConflictError extends Error {} +import util from "util"; + +class ApiError extends Error { + readonly detail: string; + + constructor(detail: string, opts: { args?: unknown[]; cause?: unknown } = {}) { + const formatted = opts.args?.length ? util.format(detail, ...(opts.args)) : detail; + super(formatted, opts.cause ? { cause: opts.cause } : undefined); + this.detail = formatted; + } +} + +export class NotFoundError extends ApiError {} + +export class ValidationError extends ApiError {} diff --git a/lambdas/api-handler/src/handlers/__tests__/get-letters.test.ts b/lambdas/api-handler/src/handlers/__tests__/get-letters.test.ts index a60ac6470..7a0d1baa5 100644 --- a/lambdas/api-handler/src/handlers/__tests__/get-letters.test.ts +++ b/lambdas/api-handler/src/handlers/__tests__/get-letters.test.ts @@ -1,18 +1,36 @@ import { getLetters } from '../../index'; -import type { Context } from 'aws-lambda'; +import type { APIGatewayProxyResult, Context } from 'aws-lambda'; import { mockDeep } from 'jest-mock-extended'; import { makeApiGwEvent } from './utils/test-utils'; import * as letterService from '../../services/letter-operations'; +import { mapErrorToResponse } from '../../mappers/error-mapper'; import { getEnvars } from '../get-letters'; +import { ValidationError } from '../../errors'; +import * as errors from '../../contracts/errors'; + +jest.mock('../../mappers/error-mapper'); +const mockedMapErrorToResponse = jest.mocked(mapErrorToResponse); +const expectedErrorResponse: APIGatewayProxyResult = { + statusCode: 400, + body: 'Error' +}; +mockedMapErrorToResponse.mockReturnValue(expectedErrorResponse); jest.mock('../../services/letter-operations'); +jest.mock('../../config/lambda-config', () => ({ + lambdaConfig: { + SUPPLIER_ID_HEADER: 'nhsd-supplier-id', + APIM_CORRELATION_HEADER: 'nhsd-correlation-id' + } +})); + describe('API Lambda handler', () => { const originalEnv = process.env; beforeEach(() => { - jest.resetAllMocks(); + jest.clearAllMocks(); jest.resetModules(); process.env = { ...originalEnv }; process.env.MAX_LIMIT = '2500'; @@ -34,7 +52,7 @@ describe('API Lambda handler', () => { id: "l1", specificationId: "s1", groupId: 'g1', - status: "PENDING", + status: "PENDING" }, { id: "l2", @@ -47,10 +65,13 @@ describe('API Lambda handler', () => { specificationId: "s1", groupId: 'g1', status: "PENDING", + reasonCode: 123, + reasonText: "Reason text" }, ]); - const event = makeApiGwEvent({path: '/letters'}); + const event = makeApiGwEvent({path: '/letters', + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'}}); const context = mockDeep(); const callback = jest.fn(); const result = await getLetters(event, context, callback); @@ -60,18 +81,18 @@ describe('API Lambda handler', () => { { id: "l1", type: "Letter", - attributes: { specificationId: "s1", groupId: 'g1', status: "PENDING" }, + attributes: { status: "PENDING", specificationId: "s1", groupId: 'g1' }, }, { id: "l2", type: "Letter", - attributes: { specificationId: "s1", groupId: 'g1', status: "PENDING" }, + attributes: { status: "PENDING", specificationId: "s1", groupId: 'g1' }, }, { id: "l3", type: "Letter", - attributes: { specificationId: "s1", groupId: 'g1', status: "PENDING" }, - }, + attributes: { status: "PENDING", specificationId: "s1", groupId: 'g1' } + } ], }; @@ -81,91 +102,79 @@ describe('API Lambda handler', () => { }); }); - it('returns 404 Not Found for an unknown path', async () => { - const event = makeApiGwEvent({ path: '/unknown' }); - const context = mockDeep(); - const callback = jest.fn(); - const result = await getLetters(event, context, callback); - - expect(result).toEqual({ - statusCode: 404, - body: 'Not Found', - }); - }); - it("returns 400 if the limit parameter is not a number", async () => { const event = makeApiGwEvent({ path: "/letters", queryStringParameters: { limit: "1%" }, + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'} }); + const context = mockDeep(); const callback = jest.fn(); const result = await getLetters(event, context, callback); - expect(result).toEqual({ - statusCode: 400, - body: "Invalid Request: limit parameter must be a positive number not greater than 2500", - }); + expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestLimitNotANumber), 'correlationId'); + expect(result).toEqual(expectedErrorResponse); }); it("returns 400 if the limit parameter is negative", async () => { const event = makeApiGwEvent({ path: "/letters", queryStringParameters: { limit: "-1" }, + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'} }); + const context = mockDeep(); const callback = jest.fn(); const result = await getLetters(event, context, callback); - expect(result).toEqual({ - statusCode: 400, - body: "Invalid Request: limit parameter must be a positive number not greater than 2500", - }); + expect(mockedMapErrorToResponse).toHaveBeenCalledWith( + new ValidationError(errors.ApiErrorDetail.InvalidRequestLimitNotInRange, { args: [getEnvars().maxLimit] }), 'correlationId'); + expect(result).toEqual(expectedErrorResponse); }); it("returns 400 if the limit parameter is zero", async () => { const event = makeApiGwEvent({ path: "/letters", queryStringParameters: { limit: "0" }, + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'} }); const context = mockDeep(); const callback = jest.fn(); const result = await getLetters(event, context, callback); - expect(result).toEqual({ - statusCode: 400, - body: "Invalid Request: limit parameter must be a positive number not greater than 2500", - }); + expect(mockedMapErrorToResponse).toHaveBeenCalledWith( + new ValidationError(errors.ApiErrorDetail.InvalidRequestLimitNotInRange, { args: [getEnvars().maxLimit] }), 'correlationId'); + expect(result).toEqual(expectedErrorResponse); }); - it("returns 400 if the limit parameter is out of range", async () => { + it("returns 400 if the limit parameter is higher than max limit", async () => { const event = makeApiGwEvent({ path: "/letters", queryStringParameters: { limit: "2501" }, + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'} }); const context = mockDeep(); const callback = jest.fn(); const result = await getLetters(event, context, callback); - expect(result).toEqual({ - statusCode: 400, - body: "Invalid Request: limit parameter must be a positive number not greater than 2500", - }); + expect(mockedMapErrorToResponse).toHaveBeenCalledWith( + new ValidationError(errors.ApiErrorDetail.InvalidRequestLimitNotInRange, { args: [getEnvars().maxLimit] }), 'correlationId'); + expect(result).toEqual(expectedErrorResponse); }); it("returns 400 if unknown parameters are present", async () => { const event = makeApiGwEvent({ path: "/letters", queryStringParameters: { max: "2000" }, + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'} }); const context = mockDeep(); const callback = jest.fn(); const result = await getLetters(event, context, callback); - expect(result).toEqual({ - statusCode: 400, - body: "Invalid Request: Only 'limit' query parameter is supported", - }); + expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestLimitOnly), 'correlationId'); + expect(result).toEqual(expectedErrorResponse); }); it('returns 400 for missing supplier ID (empty headers)', async () => { @@ -174,21 +183,21 @@ describe('API Lambda handler', () => { const callback = jest.fn(); const result = await getLetters(event, context, callback); - expect(result).toEqual({ - statusCode: 400, - body: 'Invalid Request: Missing supplier ID', - }); + expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new Error('The request headers are empty'), undefined); + expect(result).toEqual(expectedErrorResponse); }); - it('returns 400 for missing supplier ID (undefined headers)', async () => { - const event = makeApiGwEvent({ path: "/letters", headers: undefined }); + it("returns 500 if correlation id not provided in request", async () => { + const event = makeApiGwEvent({ + path: "/letters", + queryStringParameters: { limit: "2000" }, + headers: {'nhsd-supplier-id': 'supplier1'} + }); const context = mockDeep(); const callback = jest.fn(); const result = await getLetters(event, context, callback); - expect(result).toEqual({ - statusCode: 400, - body: 'Invalid Request: Missing supplier ID', - }); + expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new Error("The request headers don't contain the APIM correlation id"), undefined); + expect(result).toEqual(expectedErrorResponse); }); }); diff --git a/lambdas/api-handler/src/handlers/__tests__/patch-letters.test.ts b/lambdas/api-handler/src/handlers/__tests__/patch-letters.test.ts index 63ef9a069..c649d6faf 100644 --- a/lambdas/api-handler/src/handlers/__tests__/patch-letters.test.ts +++ b/lambdas/api-handler/src/handlers/__tests__/patch-letters.test.ts @@ -1,12 +1,34 @@ import { patchLetters } from '../../index'; -import type { Context } from 'aws-lambda'; +import { APIGatewayProxyResult, Context } from 'aws-lambda'; import { mockDeep } from 'jest-mock-extended'; import { makeApiGwEvent } from './utils/test-utils'; import * as letterService from '../../services/letter-operations'; -import { NotFoundError, ValidationError } from '../../errors'; import { LetterApiDocument, LetterApiStatus } from '../../contracts/letter-api'; +import { mapErrorToResponse } from '../../mappers/error-mapper'; +import { ValidationError } from '../../errors'; +import * as errors from '../../contracts/errors'; jest.mock('../../services/letter-operations'); +jest.mock('../../mappers/error-mapper'); + +jest.mock("../../config/lambda-config", () => ({ + lambdaConfig: { + SUPPLIER_ID_HEADER: "nhsd-supplier-id", + APIM_CORRELATION_HEADER: 'nhsd-correlation-id' + } +})); + +const mockedMapErrorToResponse = jest.mocked(mapErrorToResponse); +const expectedErrorResponse: APIGatewayProxyResult = { + statusCode: 400, + body: "Error" +}; +mockedMapErrorToResponse.mockReturnValue(expectedErrorResponse); + +const mockedPatchLetterStatus = jest.mocked(letterService.patchLetterStatus); + +const letterApiDocument = makeLetterApiDocument("id1", "REJECTED"); +const requestBody = JSON.stringify(letterApiDocument, null, 2); function makeLetterApiDocument(id: string, status: LetterApiStatus) : LetterApiDocument { return { @@ -14,7 +36,7 @@ function makeLetterApiDocument(id: string, status: LetterApiStatus) : LetterApiD attributes: { reasonCode: 123, reasonText: "Reason text", - requestedProductionStatus: "ACTIVE", + specificationId: "spec1", status }, id, @@ -23,21 +45,22 @@ function makeLetterApiDocument(id: string, status: LetterApiStatus) : LetterApiD }; } -const letterApiDocument = makeLetterApiDocument("id1", "REJECTED"); - -const requestBody = JSON.stringify(letterApiDocument, null, 2); +beforeEach(() => { + jest.clearAllMocks(); +}); describe('patchLetters API Handler', () => { - it('returns 200 OK with updated resource', async () => { + it('returns 200 OK with updated resource', async () => { const event = makeApiGwEvent({ path: '/letters/id1', body: requestBody, - pathParameters: {id: "id1"}}); + pathParameters: {id: "id1"}, + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'} + }); const context = mockDeep(); const callback = jest.fn(); - const mockedPatchLetterStatus = letterService.patchLetterStatus as jest.Mock; mockedPatchLetterStatus.mockResolvedValue(letterApiDocument); const result = await patchLetters(event, context, callback); @@ -48,101 +71,154 @@ describe('patchLetters API Handler', () => { }); }); - it('returns 400 Bad Request as there is no body', async () => { + it('returns error response when there is no body', async () => { const event = makeApiGwEvent({ path: '/letters/id1', - pathParameters: {id: "id1"}}); + pathParameters: {id: "id1"}, + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'} + }); const context = mockDeep(); const callback = jest.fn(); + const result = await patchLetters(event, context, callback); - expect(result).toEqual({ - statusCode: 400, - body: 'Bad Request: Missing request body', - }); + expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestMissingBody), 'correlationId'); + expect(result).toEqual(expectedErrorResponse); }); - it('returns 404 Not Found as path is unknown', async () => { + it('returns error response when path parameter letterId is not found', async () => { const event = makeApiGwEvent({ - path: '/unknown', + path: '/letters/', body: requestBody, - pathParameters: {id: "id1"}}); + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'} + }); const context = mockDeep(); const callback = jest.fn(); const result = await patchLetters(event, context, callback); - expect(result).toEqual({ - statusCode: 404, - body: 'Not Found: The requested resource does not exist', - }); + expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestMissingLetterIdPathParameter), 'correlationId'); + expect(result).toEqual(expectedErrorResponse); }); - it('returns 404 Not Found as path parameter is not found', async () => { + it('returns error response when error is thrown by service', async () => { + const error = new Error('Service error'); + mockedPatchLetterStatus.mockRejectedValue(error); + const event = makeApiGwEvent({ - path: '/letters', - body: requestBody}); + path: '/letters/id1', + body: requestBody, + pathParameters: {id: "id1"}, + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'} + }); const context = mockDeep(); const callback = jest.fn(); + const result = await patchLetters(event, context, callback); - expect(result).toEqual({ - statusCode: 404, - body: 'Not Found: The requested resource does not exist', - }); + expect(mockedMapErrorToResponse).toHaveBeenCalledWith(error, 'correlationId'); + expect(result).toEqual(expectedErrorResponse); }); - it('returns 400 Bad Request when ValidationError is thrown by service', async () => { - const mockedPatchLetterStatus = letterService.patchLetterStatus as jest.Mock; - mockedPatchLetterStatus.mockRejectedValue(new ValidationError('Validation failed')); - + it('returns error when supplier id is missing', async () => { const event = makeApiGwEvent({ path: '/letters/id1', body: requestBody, - pathParameters: { id: "id1" } + pathParameters: {id: "id1"}, + headers: {'nhsd-correlation-id': 'correlationId'} }); const context = mockDeep(); const callback = jest.fn(); const result = await patchLetters(event, context, callback); - expect(result).toEqual({ - statusCode: 400, - body: 'Validation failed' - }); + expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestMissingSupplierId), 'correlationId'); + expect(result).toEqual(expectedErrorResponse); }); - it('returns 404 Not Found when NotFoundError is thrown by service', async () => { - const mockedPatchLetterStatus = letterService.patchLetterStatus as jest.Mock; - mockedPatchLetterStatus.mockRejectedValue(new NotFoundError('Letter not found')); + it('returns error when request body does not have correct shape', async () => { + const event = makeApiGwEvent({ + path: '/letters/id1', + body: '{test: "test"}', + pathParameters: {id: "id1"}, + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'} + }); + const context = mockDeep(); + const callback = jest.fn(); + + const result = await patchLetters(event, context, callback); + expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestBody), 'correlationId'); + expect(result).toEqual(expectedErrorResponse); + }); + + it('returns error when request body is not json', async () => { const event = makeApiGwEvent({ path: '/letters/id1', - body: requestBody, - pathParameters: { id: "id1" } + body: '{#invalidJSON', + pathParameters: {id: "id1"}, + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'} }); const context = mockDeep(); const callback = jest.fn(); const result = await patchLetters(event, context, callback); - expect(result).toEqual({ - statusCode: 404, - body: 'Letter not found' + expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new ValidationError(errors.ApiErrorDetail.InvalidRequestBody), 'correlationId'); + expect(result).toEqual(expectedErrorResponse); + }); + + it('returns error if unexpected error is thrown', async () => { + const event = makeApiGwEvent({ + path: '/letters/id1', + body: 'somebody', + pathParameters: {id: "id1"}, + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'} + }); + const context = mockDeep(); + const callback = jest.fn(); + + const error = "Unexpected error"; + const spy = jest.spyOn(JSON, "parse").mockImplementation(() => { + throw error; }); + + const result = await patchLetters(event, context, callback); + + expect(mockedMapErrorToResponse).toHaveBeenCalledWith(error, 'correlationId'); + expect(result).toEqual(expectedErrorResponse); + + spy.mockRestore(); }); - it('throws unexpected errors from service', async () => { - const mockedPatchLetterStatus = letterService.patchLetterStatus as jest.Mock; - mockedPatchLetterStatus.mockRejectedValue(new Error('Unexpected error')); + it("returns error if correlation id not provided in request", async () => { + const event = makeApiGwEvent({ + path: '/letters/id1', + body: requestBody, + pathParameters: {id: "id1"}, + headers: {'nhsd-supplier-id': 'supplier1'} + }); + const context = mockDeep(); + const callback = jest.fn(); + + const result = await patchLetters(event, context, callback); + + expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new Error("The request headers don't contain the APIM correlation id"), undefined); + expect(result).toEqual(expectedErrorResponse); + }); + it('returns 400 for missing supplier ID (empty headers)', async () => { const event = makeApiGwEvent({ path: '/letters/id1', body: requestBody, - pathParameters: { id: "id1" } + pathParameters: {id: "id1"}, + headers: {} }); const context = mockDeep(); const callback = jest.fn(); - await expect(patchLetters(event, context, callback)).rejects.toThrow('Unexpected error'); + const result = await patchLetters(event, context, callback); + + expect(mockedMapErrorToResponse).toHaveBeenCalledWith(new Error('The request headers are empty'), undefined); + expect(result).toEqual(expectedErrorResponse); }); }); diff --git a/lambdas/api-handler/src/handlers/get-letters.ts b/lambdas/api-handler/src/handlers/get-letters.ts index 1a576b2c7..0c24ca4b1 100644 --- a/lambdas/api-handler/src/handlers/get-letters.ts +++ b/lambdas/api-handler/src/handlers/get-letters.ts @@ -1,82 +1,36 @@ -import { APIGatewayProxyHandler } from "aws-lambda"; +import { APIGatewayProxyEventQueryStringParameters, APIGatewayProxyHandler } from "aws-lambda"; import { getLettersForSupplier } from "../services/letter-operations"; import { createLetterRepository } from "../infrastructure/letter-repo-factory"; import { LetterBase } from "../../../../internal/datastore/src"; +import { assertNotEmpty } from "../utils/validation"; +import { ApiErrorDetail } from '../contracts/errors'; +import { lambdaConfig } from "../config/lambda-config"; import pino from 'pino'; +import { mapErrorToResponse } from "../mappers/error-mapper"; +import { ValidationError } from "../errors"; +import { mapLetterBaseToApiResource } from "../mappers/letter-mapper"; const letterRepo = createLetterRepository(); + const log = pino(); export const getEnvars = (): { maxLimit: number } => ({ maxLimit: parseInt(process.env.MAX_LIMIT!) }); +// The endpoint should only return pending letters for now +const status = "PENDING"; + export const getLetters: APIGatewayProxyHandler = async (event) => { const { maxLimit } = getEnvars(); + let correlationId; - if (event.path === "/letters") { - const supplierId = event.headers ? event.headers["NHSD-Supplier-ID"] : undefined; - - if (!supplierId) { - log.info({ - description: 'Supplier ID not provided' - }); - return { - statusCode: 400, - body: "Invalid Request: Missing supplier ID", - }; - } - - // The endpoint should only return pending letters for now - const status = "PENDING"; - - if ( - event.queryStringParameters && - Object.keys(event.queryStringParameters).some( - (key) => key !== "limit" - ) - ) { - log.info({ - description: "Unexpected query parameter(s) present", - queryStringParameters: event.queryStringParameters, - }); - - return { - statusCode: 400, - body: "Invalid Request: Only 'limit' query parameter is supported", - }; - } - - let limitNumber; - - if (event.queryStringParameters?.limit) { - let limitParam = event.queryStringParameters?.limit; - limitNumber = Number(limitParam); - if (isNaN(limitNumber)) { - log.info({ - description: "limit parameter is not a number", - limitParam, - }); - return { - statusCode: 400, - body: "Invalid Request: limit parameter must be a positive number not greater than 2500", - }; - } - } else { - limitNumber = maxLimit; - } - - if (limitNumber <= 0 || limitNumber > maxLimit) { - log.info({ - description: "Limit value is invalid", - limitNumber, - }); - return { - statusCode: 400, - body: `Invalid Request: limit parameter must be a positive number not greater than ${maxLimit}`, - }; - } + try { + assertNotEmpty(event.headers, new Error("The request headers are empty")); + correlationId = assertNotEmpty(event.headers[lambdaConfig.APIM_CORRELATION_HEADER], new Error("The request headers don't contain the APIM correlation id")); + const supplierId = assertNotEmpty(event.headers[lambdaConfig.SUPPLIER_ID_HEADER], new ValidationError(ApiErrorDetail.InvalidRequestMissingSupplierId)); + const limitNumber = getLimitOrDefault(event.queryStringParameters, maxLimit); const letters = await getLettersForSupplier( supplierId, @@ -85,7 +39,9 @@ export const getLetters: APIGatewayProxyHandler = async (event) => { letterRepo, ); - const response = createGetLettersResponse(letters); + const response = { + data: letters.map((letter: LetterBase) => (mapLetterBaseToApiResource(letter, { excludeOptional: true }))) + }; log.info({ description: 'Pending letters successfully fetched', @@ -100,44 +56,61 @@ export const getLetters: APIGatewayProxyHandler = async (event) => { body: JSON.stringify(response, null, 2), }; } + catch (error) { + return mapErrorToResponse(error, correlationId); + } +}; - log.warn({ - description: 'Unsupported event path', - path: event.path - }); +function getLimitOrDefault(queryStringParameters: APIGatewayProxyEventQueryStringParameters | null, maxLimit: number) : number { - return { - statusCode: 404, - body: "Not Found", - }; -}; + validateLimitParamOnly(queryStringParameters); + return getLimit(queryStringParameters?.limit, maxLimit); +} -interface GetLettersResponse { - data: Array<{ - type: "Letter"; - id: string; - attributes: { - specificationId: string; - groupId: string; - status: string; - reasonCode?: number; - reasonText?: string; - }; - }>; +function assertIsNumber(limitNumber: number) { + if (isNaN(limitNumber)) { + log.info({ + description: "limit parameter is not a number", + limitNumber, + }); + throw new ValidationError(ApiErrorDetail.InvalidRequestLimitNotANumber); + } } -function createGetLettersResponse(letters: LetterBase[]): GetLettersResponse { - return { - data: letters.map((letter) => ({ - id: letter.id, - type: "Letter", - attributes: { - specificationId: letter.specificationId, - groupId: letter.groupId, - status: letter.status, - reasonCode: letter.reasonCode, - reasonText: letter.reasonText, - }, - })), - }; +function assertLimitInRange(limitNumber: number, maxLimit: number) { + if (limitNumber <= 0 || limitNumber > maxLimit) { + log.info({ + description: "Limit value is invalid", + limitNumber, + }); + throw new ValidationError(ApiErrorDetail.InvalidRequestLimitNotInRange, { args: [maxLimit]}); + } +} + +function validateLimitParamOnly(queryStringParameters: APIGatewayProxyEventQueryStringParameters | null) { + if ( + queryStringParameters && + Object.keys(queryStringParameters).some( + (key) => key !== "limit" + ) + ) { + log.info({ + description: "Unexpected query parameter(s) present", + queryStringParameters: queryStringParameters, + }); + throw new ValidationError(ApiErrorDetail.InvalidRequestLimitOnly); + } +} + +function getLimit(limit: string | undefined, maxLimit: number) { + let result; + if (limit) { + let limitParam = limit; + result = Number(limitParam); + assertIsNumber(result); + assertLimitInRange(result, maxLimit); + } else { + result = maxLimit; + } + return result; } diff --git a/lambdas/api-handler/src/handlers/patch-letters.ts b/lambdas/api-handler/src/handlers/patch-letters.ts index af75e674b..558ac7ec4 100644 --- a/lambdas/api-handler/src/handlers/patch-letters.ts +++ b/lambdas/api-handler/src/handlers/patch-letters.ts @@ -1,59 +1,44 @@ import { APIGatewayProxyHandler } from 'aws-lambda'; import { createLetterRepository } from '../infrastructure/letter-repo-factory'; import { patchLetterStatus } from '../services/letter-operations'; -import { LetterApiDocument } from '../contracts/letter-api'; -import { NotFoundError, ValidationError } from '../errors'; +import { LetterApiDocument, LetterApiDocumentSchema } from '../contracts/letter-api'; +import { ApiErrorDetail } from '../contracts/errors'; +import { ValidationError } from '../errors'; +import { mapErrorToResponse } from '../mappers/error-mapper'; +import { lambdaConfig } from "../config/lambda-config"; +import { assertNotEmpty } from '../utils/validation'; const letterRepo = createLetterRepository(); export const patchLetters: APIGatewayProxyHandler = async (event) => { - // TODO CCM-11188: default to supplier1 for now - const supplierId = event.headers['nhsd-apim-apikey'] ?? "supplier1"; + let correlationId; - const pathParameters = event.pathParameters || {}; - const letterId = pathParameters["id"]; + try { + assertNotEmpty(event.headers, new Error('The request headers are empty')); + correlationId = assertNotEmpty(event.headers[lambdaConfig.APIM_CORRELATION_HEADER], new Error("The request headers don't contain the APIM correlation id")); + const supplierId = assertNotEmpty(event.headers[lambdaConfig.SUPPLIER_ID_HEADER], new ValidationError(ApiErrorDetail.InvalidRequestMissingSupplierId)); + const letterId = assertNotEmpty( event.pathParameters?.id, new ValidationError(ApiErrorDetail.InvalidRequestMissingLetterIdPathParameter)); + const body = assertNotEmpty(event.body, new ValidationError(ApiErrorDetail.InvalidRequestMissingBody)); - if (event.path.includes('/letters/') && letterId) { + let patchLetterRequest: LetterApiDocument; - if (!event.body) - { - return { - statusCode: 400, - body: "Bad Request: Missing request body" + try { + patchLetterRequest = LetterApiDocumentSchema.parse(JSON.parse(body)); + } catch (error) { + if (error instanceof Error) { + throw new ValidationError(ApiErrorDetail.InvalidRequestBody, { cause: error}); } + else throw error; } - const patchLetterRequest: LetterApiDocument = JSON.parse(event.body); - - try { - - // TODO CCM-11188: Is it worth retrieving the letter first to check if the status is different? + const result = await patchLetterStatus(patchLetterRequest.data, letterId!, supplierId!, letterRepo); - const result = await patchLetterStatus(patchLetterRequest.data, letterId, supplierId, letterRepo); + return { + statusCode: 200, + body: JSON.stringify(result, null, 2) + }; - return { - statusCode: 200, - body: JSON.stringify(result, null, 2) - }; - } catch (error) { - if (error instanceof ValidationError) { - return { - statusCode: 400, - body: error.message - }; - } else if (error instanceof NotFoundError) { - return { - statusCode: 404, - body: error.message - }; - } - throw error; - } + } catch (error) { + return mapErrorToResponse(error, correlationId); } - - // TODO CCM-11188: Is this reachable with the API GW? - return { - statusCode: 404, - body: 'Not Found: The requested resource does not exist', - }; }; diff --git a/lambdas/api-handler/src/mappers/__tests__/error-mapper.test.ts b/lambdas/api-handler/src/mappers/__tests__/error-mapper.test.ts new file mode 100644 index 000000000..35e5aba99 --- /dev/null +++ b/lambdas/api-handler/src/mappers/__tests__/error-mapper.test.ts @@ -0,0 +1,93 @@ +import { mapErrorToResponse } from "../error-mapper"; +import { ValidationError, NotFoundError } from "../../errors"; +import { ApiErrorDetail } from "../../contracts/errors"; + +describe("mapErrorToResponse", () => { + it("should map ValidationError to InvalidRequest response", () => { + const err = new ValidationError(ApiErrorDetail.InvalidRequestLetterIdsMismatch); + + const res = mapErrorToResponse(err, 'correlationId'); + + expect(res.statusCode).toEqual(400); + expect(JSON.parse(res.body)).toEqual({ + "errors": [ + { + "code": "NOTIFY_INVALID_REQUEST", + "detail": "The letter ID in the request body does not match the letter ID path parameter", + "id": "correlationId", + "links": { + "about": "https://digital.nhs.uk/developer/api-catalogue/nhs-notify-supplier" + }, + "status": "400", + "title": "Invalid request" + } + ] + }); + }); + + it("should map NotFoundError to NotFound response", () => { + const err = new NotFoundError(ApiErrorDetail.NotFoundLetterId); + + const res = mapErrorToResponse(err, undefined); + + expect(res.statusCode).toEqual(404); + expect(JSON.parse(res.body)).toEqual({ + "errors": [ + { + "code": "NOTIFY_LETTER_NOT_FOUND", + "detail": "No resource found with that ID", + "id": expect.any(String), + "links": { + "about": "https://digital.nhs.uk/developer/api-catalogue/nhs-notify-supplier" + }, + "status": "404", + "title": "Not found" + } + ] + }); + }); + + it("should map generic Error to InternalServerError response", () => { + const err = new Error("Something broke"); + + const res = mapErrorToResponse(err, 'correlationId'); + + expect(res.statusCode).toEqual(500); + expect(JSON.parse(res.body)).toEqual({ + "errors": [ + { + "code": "NOTIFY_INTERNAL_SERVER_ERROR", + "detail": "Something broke", + "id": "correlationId", + "links": { + "about": "https://digital.nhs.uk/developer/api-catalogue/nhs-notify-supplier" + }, + "status": "500", + "title": "Internal server error" + } + ] + }); + }); + + it("should map unexpected non-error to InternalServerError response", () => { + const err = 12345; // not an Error + + const res = mapErrorToResponse(err, 'correlationId'); + + expect(res.statusCode).toEqual(500); + expect(JSON.parse(res.body)).toEqual({ + "errors": [ + { + "code": "NOTIFY_INTERNAL_SERVER_ERROR", + "detail": "Unexpected error", + "id": "correlationId", + "links": { + "about": "https://digital.nhs.uk/developer/api-catalogue/nhs-notify-supplier" + }, + "status": "500", + "title": "Internal server error" + } + ] + }); + }); +}); diff --git a/lambdas/api-handler/src/mappers/__tests__/letter-mapper.test.ts b/lambdas/api-handler/src/mappers/__tests__/letter-mapper.test.ts index ee0be34c7..82adabd28 100644 --- a/lambdas/api-handler/src/mappers/__tests__/letter-mapper.test.ts +++ b/lambdas/api-handler/src/mappers/__tests__/letter-mapper.test.ts @@ -1,8 +1,8 @@ -import { toApiLetter } from '../letter-mapper'; +import { mapLetterBaseToApiDocument, mapLetterBaseToApiResource } from '../letter-mapper'; import { Letter } from '../../../../../internal/datastore'; -import { LetterApiDocument } from '../../contracts/letter-api'; +import { LetterApiDocument, LetterApiResource } from '../../contracts/letter-api'; -describe('toApiLetter', () => { +describe('letter-mapper', () => { it('maps a Letter to LetterApiDocument', () => { const letter: Letter = { id: 'abc123', @@ -18,19 +18,117 @@ describe('toApiLetter', () => { ttl: 123 }; - const result: LetterApiDocument = toApiLetter(letter); + const result: LetterApiDocument = mapLetterBaseToApiDocument(letter); expect(result).toEqual({ data: { id: 'abc123', type: 'Letter', attributes: { + specificationId: 'spec123', + status: 'PENDING', + groupId: 'group123' + } + } + }); + }); + + it('maps a Letter to LetterApiDocument with reasonCode and reasonText when present', () => { + const letter: Letter = { + id: 'abc123', + status: 'PENDING', + supplierId: 'supplier1', + specificationId: 'spec123', + groupId: 'group123', + url: 'https://example.com/letter/abc123', + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + supplierStatus: 'supplier1#PENDING', + supplierStatusSk: Date.now().toString(), + ttl: 123, + reasonCode: 123, + reasonText: 'Reason text' + }; + + const result: LetterApiDocument = mapLetterBaseToApiDocument(letter, {excludeOptional:false}); + + expect(result).toEqual({ + data: { + id: 'abc123', + type: 'Letter', + attributes: { + specificationId: 'spec123', + status: 'PENDING', + groupId: 'group123', reasonCode: 123, reasonText: 'Reason text', - requestedProductionStatus: 'ACTIVE', - status: 'PENDING' } } }); }); + + it('maps a Letter to LetterApiDocument without reasonCode and reasonText when present', () => { + const letter: Letter = { + id: 'abc123', + status: 'PENDING', + supplierId: 'supplier1', + specificationId: 'spec123', + groupId: 'group123', + url: 'https://example.com/letter/abc123', + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + supplierStatus: 'supplier1#PENDING', + supplierStatusSk: Date.now().toString(), + ttl: 123, + reasonCode: 123, + reasonText: 'Reason text' + }; + + const result: LetterApiDocument = mapLetterBaseToApiDocument(letter, {excludeOptional: true}); + + expect(result).toEqual({ + data: { + id: 'abc123', + type: 'Letter', + attributes: { + specificationId: 'spec123', + status: 'PENDING', + groupId: 'group123' + } + } + }); + }); + + + it('maps a Letter to LetterApiResource with reasonCode and reasonText when present', () => { + const letter: Letter = { + id: 'abc123', + status: 'PENDING', + supplierId: 'supplier1', + specificationId: 'spec123', + groupId: 'group123', + url: 'https://example.com/letter/abc123', + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + supplierStatus: 'supplier1#PENDING', + supplierStatusSk: Date.now().toString(), + ttl: 123, + reasonCode: 123, + reasonText: 'Reason text' + }; + + const result: LetterApiResource = mapLetterBaseToApiResource(letter); + + expect(result).toEqual({ + id: 'abc123', + type: 'Letter', + attributes: { + specificationId: 'spec123', + status: 'PENDING', + groupId: 'group123', + reasonCode: 123, + reasonText: 'Reason text' + } + }); + }); }); diff --git a/lambdas/api-handler/src/mappers/error-mapper.ts b/lambdas/api-handler/src/mappers/error-mapper.ts new file mode 100644 index 000000000..5a9f09d7a --- /dev/null +++ b/lambdas/api-handler/src/mappers/error-mapper.ts @@ -0,0 +1,61 @@ +import { APIGatewayProxyResult } from "aws-lambda"; +import { NotFoundError, ValidationError } from "../errors"; +import { buildApiError, ApiErrorCode, ApiErrorDetail, ApiErrorTitle, ApiError, ApiErrorStatus } from "../contracts/errors"; +import pino from "pino"; +import { v4 as uuid } from 'uuid'; + +const logger = pino(); +export interface ErrorResponse { + errors: ApiError[]; +} + +export function mapErrorToResponse(error: unknown, correlationId: string | undefined): APIGatewayProxyResult { + if (error instanceof ValidationError) { + logger.info({ err: error }, `Validation error correlationId=${correlationId}`); + return buildResponseFromErrorCode(ApiErrorCode.InvalidRequest, error.detail, correlationId); + } else if (error instanceof NotFoundError) { + logger.info({ err: error }, `Not found error correlationId=${correlationId}`); + return buildResponseFromErrorCode(ApiErrorCode.NotFound, error.detail, correlationId); + } else if (error instanceof Error) { + logger.error({ err: error }, `Internal server error correlationId=${correlationId}`); + return buildResponseFromErrorCode(ApiErrorCode.InternalServerError, error.message, correlationId); + } else { + logger.error({ err: error }, `Internal server error (non-Error thrown) correlationId=${correlationId}`); + return buildResponseFromErrorCode(ApiErrorCode.InternalServerError, "Unexpected error", correlationId); + } +} + +function buildResponseFromErrorCode(code: ApiErrorCode, detail: string, correlationId: string | undefined): APIGatewayProxyResult { + const id = correlationId ? correlationId : uuid(); + const responseError = buildApiError({ + id, + code, + status: codeToStatus(code), + title: codeToTitle(code), + detail + }); + return { + statusCode: +responseError.status, + body: JSON.stringify(buildErrorResponseFromError(responseError), null, 2) + }; +} + +function codeToStatus(code: ApiErrorCode): ApiErrorStatus { + switch(code) { + case ApiErrorCode.InvalidRequest: return ApiErrorStatus.InvalidRequest; + case ApiErrorCode.NotFound: return ApiErrorStatus.NotFound; + default: return ApiErrorStatus.InternalServerError; + } +} + +function codeToTitle(code: ApiErrorCode): ApiErrorTitle { + switch(code) { + case ApiErrorCode.InvalidRequest: return ApiErrorTitle.InvalidRequest; + case ApiErrorCode.NotFound: return ApiErrorTitle.NotFound; + default: return ApiErrorTitle.InternalServerError; + } +} + +function buildErrorResponseFromError(error: ApiError): ErrorResponse { + return { errors: [error] }; +} diff --git a/lambdas/api-handler/src/mappers/letter-mapper.ts b/lambdas/api-handler/src/mappers/letter-mapper.ts index a72c31158..c9c55ab21 100644 --- a/lambdas/api-handler/src/mappers/letter-mapper.ts +++ b/lambdas/api-handler/src/mappers/letter-mapper.ts @@ -1,17 +1,22 @@ -import { Letter } from "../../../../internal/datastore"; -import { LetterApiDocument } from '../contracts/letter-api'; +import { LetterBase } from "../../../../internal/datastore"; +import { LetterApiDocument, LetterApiDocumentSchema, LetterApiResource, LetterApiResourceSchema } from '../contracts/letter-api'; -export function toApiLetter(letter: Letter): LetterApiDocument { - return { - data: { - id: letter.id, - type: 'Letter', - attributes: { - reasonCode: 123, // TODO CCM-11188: map from DB if stored - reasonText: 'Reason text', // TODO CCM-11188: map from DB if stored - requestedProductionStatus: 'ACTIVE', // TODO CCM-11188: map from DB if stored - status: letter.status - } +export function mapLetterBaseToApiDocument(letterBase: LetterBase, opts: { excludeOptional: boolean } = { excludeOptional: false }): LetterApiDocument { + return LetterApiDocumentSchema.parse({ + data: mapLetterBaseToApiResource(letterBase, opts) + }); +} + +export function mapLetterBaseToApiResource(letterBase: LetterBase, opts: { excludeOptional: boolean } = { excludeOptional: false }): LetterApiResource { + return LetterApiResourceSchema.parse({ + id: letterBase.id, + type: 'Letter', + attributes: { + status: letterBase.status, + specificationId: letterBase.specificationId, + groupId: letterBase.groupId, + ...(letterBase.reasonCode && !opts.excludeOptional && { reasonCode: letterBase.reasonCode }), + ...(letterBase.reasonText && !opts.excludeOptional && { reasonText: letterBase.reasonText }) } - }; + }); } diff --git a/lambdas/api-handler/src/services/__tests__/letter-operations.test.ts b/lambdas/api-handler/src/services/__tests__/letter-operations.test.ts index e2eba224e..1a6f82f45 100644 --- a/lambdas/api-handler/src/services/__tests__/letter-operations.test.ts +++ b/lambdas/api-handler/src/services/__tests__/letter-operations.test.ts @@ -5,10 +5,11 @@ import { getLettersForSupplier, patchLetterStatus } from '../letter-operations'; function makeLetterApiResource(id: string, status: LetterApiStatus) : LetterApiResource { return { attributes: { + specificationId: "spec123", + status, + groupId: 'group123', reasonCode: 123, - reasonText: "Reason text", - requestedProductionStatus: "ACTIVE", - status + reasonText: "Reason text" }, id, type: "Letter" @@ -27,7 +28,9 @@ function makeLetter(id: string, status: Letter['status']) : Letter { updatedAt: new Date().toISOString(), supplierStatus: `supplier1#${status}`, supplierStatusSk: Date.now().toString(), - ttl: 123 + ttl: 123, + reasonCode: 123, + reasonText: "Reason text" }; } @@ -61,9 +64,9 @@ describe("getLetterIdsForSupplier", () => { describe('patchLetterStatus function', () => { - const letterResource = makeLetterApiResource("letter1", "ACCEPTED"); + const letterResource = makeLetterApiResource("letter1", "REJECTED"); - const updatedLetter = makeLetter("letter1", "ACCEPTED"); + const updatedLetter = makeLetter("letter1", "REJECTED"); it('should update the letter status successfully', async () => { const mockRepo = { @@ -76,15 +79,15 @@ describe('patchLetterStatus function', () => { }); it('should throw validationError when letterIds differ', async () => { - await expect(patchLetterStatus(letterResource, 'letter2', "supplier1", {} as any)).rejects.toThrow("Bad Request: Letter ID in body does not match path parameter"); + await expect(patchLetterStatus(letterResource, 'letter2', "supplier1", {} as any)).rejects.toThrow("The letter ID in the request body does not match the letter ID path parameter"); }); it('should throw notFoundError when letter does not exist', async () => { const mockRepo = { - updateLetterStatus: jest.fn().mockRejectedValue(new Error('not found')) + updateLetterStatus: jest.fn().mockRejectedValue(new Error('Letter with id l1 not found for supplier s1')) }; - await expect(patchLetterStatus(letterResource, 'letter1', 'supplier1', mockRepo as any)).rejects.toThrow("Not Found: Letter with ID letter1 does not exist"); + await expect(patchLetterStatus(letterResource, 'letter1', 'supplier1', mockRepo as any)).rejects.toThrow("No resource found with that ID"); }); it('should throw unexpected error', async () => { @@ -95,5 +98,4 @@ describe('patchLetterStatus function', () => { await expect(patchLetterStatus(letterResource, 'letter1', 'supplier1', mockRepo as any)).rejects.toThrow("unexpected error"); }); - }); diff --git a/lambdas/api-handler/src/services/letter-operations.ts b/lambdas/api-handler/src/services/letter-operations.ts index d16fd1a2a..c0104b946 100644 --- a/lambdas/api-handler/src/services/letter-operations.ts +++ b/lambdas/api-handler/src/services/letter-operations.ts @@ -1,7 +1,8 @@ import { LetterBase, LetterRepository } from '../../../../internal/datastore/src' import { NotFoundError, ValidationError } from '../errors'; import { LetterApiResource, LetterApiDocument } from '../contracts/letter-api'; -import { toApiLetter } from '../mappers/letter-mapper'; +import { mapLetterBaseToApiDocument } from '../mappers/letter-mapper'; +import { ApiErrorDetail } from '../contracts/errors'; export const getLettersForSupplier = async (supplierId: string, status: string, limit: number, letterRepo: LetterRepository): Promise => { @@ -12,18 +13,20 @@ export const getLettersForSupplier = async (supplierId: string, status: string, export const patchLetterStatus = async (letterToUpdate: LetterApiResource, letterId: string, supplierId: string, letterRepo: LetterRepository): Promise => { if (letterToUpdate.id !== letterId) { - throw new ValidationError("Bad Request: Letter ID in body does not match path parameter"); + throw new ValidationError(ApiErrorDetail.InvalidRequestLetterIdsMismatch); } - try { - const updatedLetter = await letterRepo.updateLetterStatus(supplierId, letterId, letterToUpdate.attributes.status); - - return toApiLetter(updatedLetter); + let updatedLetter; + try { + updatedLetter = await letterRepo.updateLetterStatus(supplierId, letterId, letterToUpdate.attributes.status, + letterToUpdate.attributes.reasonCode, letterToUpdate.attributes.reasonText); } catch (error) { - if (error instanceof Error && error.message.includes('not found')) { - throw new NotFoundError(`Not Found: Letter with ID ${letterId} does not exist`); + if (error instanceof Error && /^Letter with id \w+ not found for supplier \w+$/.test(error.message)) { + throw new NotFoundError(ApiErrorDetail.NotFoundLetterId); } throw error; } + + return mapLetterBaseToApiDocument(updatedLetter); } diff --git a/lambdas/api-handler/src/utils/__tests__/validation.test.ts b/lambdas/api-handler/src/utils/__tests__/validation.test.ts new file mode 100644 index 000000000..c35bc332f --- /dev/null +++ b/lambdas/api-handler/src/utils/__tests__/validation.test.ts @@ -0,0 +1,54 @@ +import { assertNotEmpty } from "../validation"; + +describe("assertNotEmpty", () => { + const error = new Error(); + + it("throws for null", () => { + expect(() => assertNotEmpty(null, error)).toThrow(Error); + }); + + it("throws for undefined", () => { + expect(() => assertNotEmpty(undefined, error)).toThrow(Error); + }); + + it("throws for empty string", () => { + expect(() => assertNotEmpty("", error)).toThrow(Error); + }); + + it("throws for whitespace string", () => { + expect(() => assertNotEmpty(" ", error)).toThrow(Error); + }); + + it("does not throw for empty array", () => { + const arr: any[] = []; + expect(() => assertNotEmpty(arr, error)).toThrow(Error); + + }); + + it("does not throw for empty object", () => { + const obj = {}; + expect(() => assertNotEmpty(obj, error)).toThrow(Error); + }); + + it("returns non-empty string", () => { + const result = assertNotEmpty("hello", error); + expect(result).toBe("hello"); + }); + + it("returns number", () => { + const result = assertNotEmpty(42, error); + expect(result).toBe(42); + }); + + it("returns object", () => { + const obj = { a: 1 }; + const result = assertNotEmpty(obj, error); + expect(result).toBe(obj); + }); + + it("returns array", () => { + const arr = [1, 2, 3]; + const result = assertNotEmpty(arr, error); + expect(result).toBe(arr); + }); +}); diff --git a/lambdas/api-handler/src/utils/validation.ts b/lambdas/api-handler/src/utils/validation.ts new file mode 100644 index 000000000..f29b7ae0b --- /dev/null +++ b/lambdas/api-handler/src/utils/validation.ts @@ -0,0 +1,18 @@ +export function assertNotEmpty( + value: T | null | undefined, + error: Error +): T { + if (value == null) { + throw error; + } + + if (typeof value === "string" && value.trim() === "") { + throw error; + } + + if (typeof value === "object" && Object.keys(value).length === 0) { + throw error; + } + + return value; +} diff --git a/project.code-workspace b/project.code-workspace index 60417cd4c..ab1a80f67 100644 --- a/project.code-workspace +++ b/project.code-workspace @@ -4,6 +4,14 @@ "name": "NHS Notify Supplier API", "path": "." }, + { + "name": "Infrastructure", + "path": "./infrastructure" + }, + { + "name": "Lambdas Api Handler", + "path": "./lambdas/api-handler" + }, { "name": "Application Source Code", "path": "./src" @@ -16,10 +24,6 @@ "name": "Tests", "path": "./tests" }, - { - "name": "Infrastructure", - "path": "./infrastructure" - }, { "name": "Scripts", "path": "./scripts" @@ -40,8 +44,7 @@ "settings": { "autoOpenWorkspace.enableAutoOpenIfSingleWorkspace": true, "githubCodeOwners.format.enabled": true, - "workspace-terminals.switchTerminal": "always", - "workspace-terminals.auto": "always", + "workspace-terminals.auto": "never", "markdownlint.config": { "MD013": false, "MD024": { "siblings_only": true }, diff --git a/specification/api/components/endpoints/patchLetter.yml b/specification/api/components/endpoints/patchLetter.yml index f523ba418..6deaca9d6 100644 --- a/specification/api/components/endpoints/patchLetter.yml +++ b/specification/api/components/endpoints/patchLetter.yml @@ -7,7 +7,7 @@ responses: "200": $ref: "../responses/patchLetter200.yml" "400": - $ref: "../responses/errors/badRequest.yml" + $ref: "../responses/errors/patchLetter/patchLetterBadRequest.yml" "404": $ref: "../responses/errors/resourceNotFound.yml" "429": diff --git a/specification/api/components/examples/errors/responses/listLetters/limitInvalidValue.json b/specification/api/components/examples/errors/responses/listLetters/limitInvalidValue.json index 58277a5a8..dfb7d3b2a 100644 --- a/specification/api/components/examples/errors/responses/listLetters/limitInvalidValue.json +++ b/specification/api/components/examples/errors/responses/listLetters/limitInvalidValue.json @@ -2,7 +2,7 @@ "errors": [ { "code": "NOTIFY_INVALID_REQUEST", - "detail": "Invalid Request: limit parameter must be a positive number not greater than 2500", + "detail": "The limit parameter must be a positive number not greater than 2500", "id": "rrt-1931948104716186917-c-geu2-10664-3111479-3.0", "links": { "about": "https://digital.nhs.uk/developer/api-catalogue/nhs-notify-supplier" diff --git a/specification/api/components/examples/errors/responses/listLetters/unknownParameter.json b/specification/api/components/examples/errors/responses/listLetters/unknownParameter.json index 036b8f56c..ba1562288 100644 --- a/specification/api/components/examples/errors/responses/listLetters/unknownParameter.json +++ b/specification/api/components/examples/errors/responses/listLetters/unknownParameter.json @@ -2,7 +2,7 @@ "errors": [ { "code": "NOTIFY_INVALID_REQUEST", - "detail": "Invalid Request: Only 'limit' query parameter is supported", + "detail": "Only 'limit' query parameter is supported", "id": "rrt-1931948104716186917-c-geu2-10664-3111479-3.0", "links": { "about": "https://digital.nhs.uk/developer/api-catalogue/nhs-notify-supplier" diff --git a/specification/api/components/examples/errors/responses/patchLetter/invalidBody.json b/specification/api/components/examples/errors/responses/patchLetter/invalidBody.json new file mode 100644 index 000000000..dcafceb9f --- /dev/null +++ b/specification/api/components/examples/errors/responses/patchLetter/invalidBody.json @@ -0,0 +1,14 @@ +{ + "errors": [ + { + "code": "NOTIFY_INVALID_REQUEST", + "detail": "The request body is invalid", + "id": "rrt-1931948104716186917-c-geu2-10664-3111479-3.0", + "links": { + "about": "https://digital.nhs.uk/developer/api-catalogue/nhs-notify-supplier" + }, + "status": "400", + "title": "Invalid request" + } + ] +} diff --git a/specification/api/components/examples/errors/responses/serverError.json b/specification/api/components/examples/errors/responses/serverError.json index a667a0934..57588e9c6 100644 --- a/specification/api/components/examples/errors/responses/serverError.json +++ b/specification/api/components/examples/errors/responses/serverError.json @@ -8,7 +8,7 @@ "about": "https://digital.nhs.uk/developer/api-catalogue/nhs-notify-supplier" }, "status": "500", - "title": "Internal Server Error" + "title": "Internal server error" } ] } diff --git a/specification/api/components/responses/errors/patchLetter/patchLetterBadRequest.yml b/specification/api/components/responses/errors/patchLetter/patchLetterBadRequest.yml new file mode 100644 index 000000000..09afe4bc0 --- /dev/null +++ b/specification/api/components/responses/errors/patchLetter/patchLetterBadRequest.yml @@ -0,0 +1,9 @@ +description: "Bad request, invalid input data" +content: + application/vnd.api+json: + schema: + $ref: "../../../schemas/errorResponse.yml" + examples: + error-bad-request-invalid-request-body: + value: + $ref: ../../../examples/errors/responses/patchLetter/invalidBody.json diff --git a/specification/api/components/schemas/errorData.yml b/specification/api/components/schemas/errorData.yml deleted file mode 100644 index d20c6117f..000000000 --- a/specification/api/components/schemas/errorData.yml +++ /dev/null @@ -1,6 +0,0 @@ -type: object -properties: - errors: - type: array - items: - $ref: "./errorItem.yml" diff --git a/specification/api/components/schemas/errorResponse.yml b/specification/api/components/schemas/errorResponse.yml index 0866e6bce..d20c6117f 100644 --- a/specification/api/components/schemas/errorResponse.yml +++ b/specification/api/components/schemas/errorResponse.yml @@ -1,4 +1,6 @@ type: object properties: - data: - $ref: "./errorData.yml" + errors: + type: array + items: + $ref: "./errorItem.yml"