CCM-11602: Get Letters endpoint (#135)

* CCM-11602: Test

* CCM-11602: letters schema

* CCM-11602: letters schema

* CCM-11602: Give get letters access to GSI

* CCM-11602: GET Letters endpoint

* CCM-11602: GET Letters endpoint

* CCM-11602: GET Letters endpoint

* CCM-11602: GET Letters endpoint

* CCM-11602: GET Letters endpoint

* CCM-11602: GET Endpoint sandbox updates

* CCM-11602: Get enpoint tests

* CCM-11602: Get enpoint safe header check

* CCM-11602: Update sandbox

* CCM-11602: Update sandbox

* CCM-11602: Add logging

* CCM-11602: Update sandbox

* CCM-11602: Rename size parameter to limit

* CCM-11602: Add validation for limit parameter

* CCM-11602: increase Trivy scan timeout

* CCM-11602: Add groupId

* Correct target URL for main (#134)

* CCM-11942 Fixing cross repo workflows (#137)

* CCM-11942 Fixing cross repo workflows

* CCM-11942 Fixing cross repo workflows

* CCM-11942 Fixing cross repo workflows

* CCM-11751: Use github release assests for shared modules call (#142)

* CCM-11751: Use github release assests for shared modules call

* CCM-11751: Use github release assests for shared modules call

* CCM-11751: Use github release assests for shared modules call

* CCM-11751: Use github release assests for shared modules call

* CCM-11751: Fixing workflow trigger (#146)

* CCM-11580: Updates for OAS Spec V1 (#139)

* CCM-11580: OAS syntax fixes

Remove 'requestedProductionStatus' from letter-related JSON schemas and update examples to singular form

CCM-11580: Update serve-oas command

CCM-11580: Update serve-oas command

CCM-11580: Remove unneeded endpoints and resource types

+ Move getData endpoint under letter

CCM-11580: Remove hello world endpoint

CCM-11580: Remove hello world endpoint

CCM-11580: Add sandbox handler metadata to endpoints

CCM-11580: More fixes to examples syntax

CCM-11580: Remove remaining reference to hello-world lambda

CCM-11580: Remove remaining reference to hello-world lambda

CCM-11580: add limit parameter to list letters endpoint

CCM-11580: Update sandbox endpoints to match OAS

CCM-11580: Update letter responses in Sandbox to match spec changes

CCM-11580: implement get letter status endpoint in sandbox

Add groupId to schema and examples

* CCM-11580: remove x-eov extension from non sandox spec

---------

Co-authored-by: mark.slowey1 <[email protected]>

* Update README.md (#140)

Small change to fix Sonar cloud badges

* Add timestamp value to supplierStatusSk

* CCM-11602: param validation, max range, and OAS 400 examples

* CCM-11602: unit test expectation wording

* CCM-11602: MAX LIMIT envar and added into test context

* CCM-11602: limit can't be 0, return maxLimit value

---------

Co-authored-by: David Wass <[email protected]>
Co-authored-by: Aiden Vaines <[email protected]>
Co-authored-by: sidnhs <[email protected]>
Co-authored-by: Mike Houston <[email protected]>
Co-authored-by: mark.slowey1 <[email protected]>
Co-authored-by: Tim Ireland <[email protected]>
Co-authored-by: Mark Slowey <[email protected]>
Co-authored-by: Francisco Videira <[email protected]>

Author: 8 people

.github/workflows/stage-1-commit.yaml

@@ -149,7 +149,7 @@ jobs:
   trivy:
     name: "Trivy Scan"
     runs-on: ubuntu-latest
-    timeout-minutes: 5
+    timeout-minutes: 10
     needs: detect-terraform-changes
     if: needs.detect-terraform-changes.outputs.terraform_changed == 'true'
     steps:

infrastructure/terraform/components/api/README.md

@@ -18,9 +18,11 @@ No requirements.
 | <a name="input_force_lambda_code_deploy"></a> [force\_lambda\_code\_deploy](#input\_force\_lambda\_code\_deploy) | If the lambda package in s3 has the same commit id tag as the terraform build branch, the lambda will not update automatically. Set to True if making changes to Lambda code from on the same commit for example during development | `bool` | `false` | no |
 | <a name="input_group"></a> [group](#input\_group) | The group variables are being inherited from (often synonmous with account short-name) | `string` | n/a | yes |
 | <a name="input_kms_deletion_window"></a> [kms\_deletion\_window](#input\_kms\_deletion\_window) | When a kms key is deleted, how long should it wait in the pending deletion state? | `string` | `"30"` | no |
+| <a name="input_letter_table_ttl_hours"></a> [letter\_table\_ttl\_hours](#input\_letter\_table\_ttl\_hours) | Number of hours to set as TTL on letters table | `number` | `24` | no |
 | <a name="input_log_level"></a> [log\_level](#input\_log\_level) | The log level to be used in lambda functions within the component. Any log with a lower severity than the configured value will not be logged: https://docs.python.org/3/library/logging.html#levels | `string` | `"INFO"` | no |
 | <a name="input_log_retention_in_days"></a> [log\_retention\_in\_days](#input\_log\_retention\_in\_days) | The retention period in days for the Cloudwatch Logs events to be retained, default of 0 is indefinite | `number` | `0` | no |
 | <a name="input_manually_configure_mtls_truststore"></a> [manually\_configure\_mtls\_truststore](#input\_manually\_configure\_mtls\_truststore) | Manually manage the truststore used for API Gateway mTLS (e.g. for prod environment) | `bool` | `false` | no |
+| <a name="input_max_get_limit"></a> [max\_get\_limit](#input\_max\_get\_limit) | Default limit to apply to GET requests that support pagination | `number` | `2500` | no |
 | <a name="input_parent_acct_environment"></a> [parent\_acct\_environment](#input\_parent\_acct\_environment) | Name of the environment responsible for the acct resources used, affects things like DNS zone. Useful for named dev environments | `string` | `"main"` | no |
 | <a name="input_project"></a> [project](#input\_project) | The name of the tfscaffold project | `string` | n/a | yes |
 | <a name="input_region"></a> [region](#input\_region) | The AWS Region | `string` | n/a | yes |
@@ -31,8 +33,6 @@ No requirements.
 |------|--------|---------|
 | <a name="module_authorizer_lambda"></a> [authorizer\_lambda](#module\_authorizer\_lambda) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-lambda.zip | n/a |
 | <a name="module_domain_truststore"></a> [domain\_truststore](#module\_domain\_truststore) | git::https://github.com/NHSDigital/nhs-notify-shared-modules.git//infrastructure/modules/s3bucket | v2.0.17 |
-| <a name="module_get_letters"></a> [get\_letters](#module\_get\_letters) | git::https://github.com/NHSDigital/nhs-notify-shared-modules.git//infrastructure/modules/lambda | v2.0.10 |
-| <a name="module_kms"></a> [kms](#module\_kms) | git::https://github.com/NHSDigital/nhs-notify-shared-modules.git//infrastructure/modules/kms | v2.0.10 |
 | <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) | git::https://github.com/NHSDigital/nhs-notify-shared-modules.git//infrastructure/modules/s3bucket | v2.0.17 |

infrastructure/terraform/components/api/ddb_table_letters.tf

@@ -13,7 +13,7 @@ resource "aws_dynamodb_table" "letters" {
   global_secondary_index {
     name            = "supplierStatus-index"
     hash_key        = "supplierStatus"
-    range_key       = "id"
+    range_key       = "supplierStatusSk"
     projection_type = "ALL"
   }
 
@@ -32,6 +32,11 @@ resource "aws_dynamodb_table" "letters" {
     type = "S"
   }
 
+  attribute {
+    name = "supplierStatusSk"
+    type = "S"
+  }
+
   point_in_time_recovery {
     enabled = true
   }

infrastructure/terraform/components/api/module_lambda_get_letters.tf

@@ -37,7 +37,8 @@ module "get_letters" {
 
   lambda_env_vars = {
     LETTERS_TABLE_NAME = aws_dynamodb_table.letters.name,
-    LETTER_TTL_HOURS   = 24
+    LETTER_TTL_HOURS   = var.letter_table_ttl_hours,
+    MAX_LIMIT          = var.max_get_limit,
   }
 }
 
@@ -69,6 +70,7 @@ data "aws_iam_policy_document" "get_letters_lambda" {
 
     resources = [
       aws_dynamodb_table.letters.arn,
+      "${aws_dynamodb_table.letters.arn}/index/supplierStatus-index"
     ]
   }
 }

infrastructure/terraform/components/api/variables.tf

@@ -99,9 +99,20 @@ variable "enable_backups" {
   default     = false
 }
 
-
 variable "ca_pem_filename" {
   type        = string
   description = "Filename for the CA truststore file within the s3 bucket"
   default     = null
 }
+
+variable "letter_table_ttl_hours" {
+  type        = number
+  description = "Number of hours to set as TTL on letters table"
+  default     = 24
+}
+
+variable "max_get_limit" {
+  type        = number
+  description = "Default limit to apply to GET requests that support pagination"
+  default     = 2500
+}

internal/datastore/src/__test__/db.ts

@@ -59,7 +59,7 @@ export async function createTables(context: DBContext) {
         IndexName: 'supplierStatus-index',
         KeySchema: [
           { AttributeName: 'supplierStatus', KeyType: 'HASH' }, // Partition key for GSI
-          { AttributeName: 'id', KeyType: 'RANGE' }              // Sort key for GSI
+          { AttributeName: 'supplierStatusSk', KeyType: 'RANGE' }              // Sort key for GSI
         ],
         Projection: {
           ProjectionType: 'ALL'
@@ -69,7 +69,8 @@ export async function createTables(context: DBContext) {
     AttributeDefinitions: [
       { AttributeName: 'supplierId', AttributeType: 'S' },
       { AttributeName: 'id', AttributeType: 'S' },
-      { AttributeName: 'supplierStatus', AttributeType: 'S' }
+      { AttributeName: 'supplierStatus', AttributeType: 'S' },
+      { AttributeName: 'supplierStatusSk', AttributeType: 'S' },
     ]
   }));
 

internal/datastore/src/__test__/letter-repository.test.ts

@@ -5,7 +5,7 @@ import { Logger } from 'pino';
 import { createTestLogger, LogStream } from './logs';
 import { PutCommand } from '@aws-sdk/lib-dynamodb';
 
-function createLetter(supplierId: string, letterId: string, status: Letter['status'] = 'PENDING'): Omit<Letter, 'ttl' | 'supplierStatus'> {
+function createLetter(supplierId: string, letterId: string, status: Letter['status'] = 'PENDING'): Omit<Letter, 'ttl' | 'supplierStatus' | 'supplierStatusSk'> {
   return {
     id: letterId,
     supplierId: supplierId,
@@ -205,6 +205,7 @@ describe('LetterRepository', () => {
         url: 's3://bucket/invalid-letter.pdf',
         status: 'PENDING',
         supplierStatus: 'supplier1#PENDING',
+        supplierStatusSk: Date.now().toString(),
         createdAt: new Date().toISOString(),
         updatedAt: new Date().toISOString()
       }
@@ -218,30 +219,48 @@ describe('LetterRepository', () => {
     expect(logStream.logs).toContainEqual(expect.stringMatching(/.*specificationId.*Invalid input: expected string.*/));
   });
 
-  test('should return all letter ids for a supplier', async () => {
-    await letterRepository.putLetter(createLetter('supplier1', 'letter1'));
-    await letterRepository.putLetter(createLetter('supplier1', 'letter2'));
-    await letterRepository.putLetter(createLetter('supplier1', 'letter3'));
-    await letterRepository.putLetter(createLetter('supplier2', 'letter4'));
-    await letterRepository.putLetter(createLetter('supplier2', 'letter5'));
-
-    const letters = await letterRepository.getLetterIdsBySupplier('supplier1');
-    expect(letters).toEqual(['letter1', 'letter2', 'letter3']);
-  });
-
-  test('should return empty if no letters exist for a supplier', async () => {
-    await letterRepository.putLetter(createLetter('supplier1', 'letter1'));
-    await letterRepository.putLetter(createLetter('supplier1', 'letter2'));
+  test("should return all letters for a supplier status", async () => {
+    await letterRepository.putLetter(createLetter("supplier1", "letter1"));
+    await letterRepository.putLetter(createLetter("supplier1", "letter2"));
+    await letterRepository.putLetter(createLetter("supplier1", "letter3"));
+    await letterRepository.putLetter(
+      createLetter("supplier1", "letter4", "REJECTED"),
+    );
+    await letterRepository.putLetter(createLetter("supplier2", "letter1"));
+    await letterRepository.putLetter(createLetter("supplier2", "letter2"));
 
-    const letters = await letterRepository.getLetterIdsBySupplier('supplier2');
-    expect(letters).toEqual([]);
+    const letters = await letterRepository.getLettersBySupplier(
+      "supplier1",
+      "PENDING",
+      10,
+    );
+    expect(letters).toEqual([
+      {
+        id: "letter1",
+        specificationId: "specification1",
+        groupId: 'group1',
+        status: "PENDING",
+      },
+      {
+        id: "letter2",
+        specificationId: "specification1",
+        groupId: 'group1',
+        status: "PENDING",
+      },
+      {
+        id: "letter3",
+        specificationId: "specification1",
+        groupId: 'group1',
+        status: "PENDING",
+      },
+    ]);
   });
 
   test('should return empty if no letters exist for a supplier', async () => {
     await letterRepository.putLetter(createLetter('supplier1', 'letter1'));
     await letterRepository.putLetter(createLetter('supplier1', 'letter2'));
 
-    const letters = await letterRepository.getLetterIdsBySupplier('supplier2');
+    const letters = await letterRepository.getLettersBySupplier('supplier2', 'PENDING', 10);
     expect(letters).toEqual([]);
   });
 
@@ -252,7 +271,7 @@ describe('LetterRepository', () => {
     const mockDdbClient = { send: mockSend } as any;
     const repo = new LetterRepository(mockDdbClient, { debug: jest.fn() } as any, { lettersTableName: 'letters', ttlHours: 1 });
 
-    const result = await repo.getLetterIdsBySupplier('supplier1');
-    expect(result).toEqual([]);
+    const letters = await repo.getLettersBySupplier('supplier1', 'PENDING', 10);
+    expect(letters).toEqual([]);
   });
 });

internal/datastore/src/letter-repository.ts

@@ -6,8 +6,9 @@ import {
   UpdateCommand,
   UpdateCommandOutput
 } from '@aws-sdk/lib-dynamodb';
-import { Letter, LetterSchema } from './types';
+import { Letter, LetterBase, LetterSchema, LetterSchemaBase } from './types';
 import { Logger } from 'pino';
+import { z } from 'zod';
 
 export type PagingOptions = Partial<{
   exclusiveStartKey: Record<string, any>,
@@ -29,10 +30,11 @@ export class LetterRepository {
               readonly config: LetterRepositoryConfig) {
   }
 
-  async putLetter(letter: Omit<Letter, 'ttl' | 'supplierStatus'>): Promise<Letter> {
+  async putLetter(letter: Omit<Letter, 'ttl' | 'supplierStatus' | 'supplierStatusSk'>): Promise<Letter> {
     const letterDb: Letter = {
       ...letter,
       supplierStatus: `${letter.supplierId}#${letter.status}`,
+      supplierStatusSk: Date.now().toString(),
       ttl: Math.floor(Date.now() / 1000 + 60 * 60 * this.config.ttlHours)
     };
     try {
@@ -130,15 +132,21 @@ export class LetterRepository {
     return LetterSchema.parse(result.Attributes);
   }
 
-  async getLetterIdsBySupplier(supplierId: string): Promise<string[]> {
+  async getLettersBySupplier(supplierId: string, status: string, limit: number): Promise<LetterBase[]> {
+    const supplierStatus = `${supplierId}#${status}`;
     const result = await this.ddbClient.send(new QueryCommand({
       TableName: this.config.lettersTableName,
-      KeyConditionExpression: 'supplierId = :supplierId',
+      IndexName: 'supplierStatus-index',
+      KeyConditionExpression: 'supplierStatus = :supplierStatus',
+      Limit: limit,
+      ExpressionAttributeNames: {
+        '#status': 'status' // reserved keyword
+      },
       ExpressionAttributeValues: {
-        ':supplierId': supplierId
+        ':supplierStatus': supplierStatus
       },
-      ProjectionExpression: 'id'
+      ProjectionExpression: 'id, #status, specificationId, groupId, reasonCode, reasonText'
     }));
-    return (result.Items ?? []).map(item => item.id);
+    return z.array(LetterSchemaBase).parse(result.Items ?? []);
   }
 }

internal/datastore/src/types.md

@@ -10,19 +10,19 @@ The schemas are generated from Zod definitions and provide a visual representati
 erDiagram
     Letter {
         string id
-        string supplierId "ref: Supplier"
+        string status "enum: PENDING, ACCEPTED, REJECTED, PRINTED, ENCLOSED, CANCELLED, DISPATCHED, FAILED, RETURNED, DESTROYED, FORWARDED, DELIVERED"
         string specificationId
         string groupId
+        number reasonCode
+        string reasonText
+        string supplierId
         string url "url"
-        string status "enum: PENDING, ACCEPTED, DISPATCHED, FAILED, REJECTED, DELIVERED, CANCELLED"
         string createdAt
         string updatedAt
         string supplierStatus
+        string supplierStatusSk
         number ttl "min: -9007199254740991, max: 9007199254740991"
     }
-    Supplier {
-    }
-    Letter }o--|| Supplier : "supplierId"
 ```
 
 ## MI schema

internal/datastore/src/types.ts

@@ -17,17 +17,23 @@ export const LetterStatus = z.enum([
   'ENCLOSED', 'CANCELLED', 'DISPATCHED', 'FAILED',
   'RETURNED', 'DESTROYED', 'FORWARDED', 'DELIVERED']);
 
-export const LetterSchema = z.object({
+export const LetterSchemaBase = z.object({
   id: z.string(),
-  supplierId: idRef(SupplierSchema),
+  status: LetterStatus,
   specificationId: z.string(),
   groupId: z.string(),
+  reasonCode: z.number().optional(),
+  reasonText: z.string().optional()
+});
+
+export const LetterSchema = LetterSchemaBase.extend({
+  supplierId: z.string(),
   url: z.url(),
-  status: LetterStatus,
   createdAt: z.string(),
   updatedAt: z.string(),
   supplierStatus: z.string().describe('Secondary index PK'),
-  ttl: z.int()
+  supplierStatusSk: z.string().describe('Secondary index SK'),
+  ttl: z.int(),
 }).describe('Letter');
 
 /**
@@ -36,6 +42,7 @@ export const LetterSchema = z.object({
  * The ttl is used for automatic deletion of old letters.
  */
 export type Letter = z.infer<typeof LetterSchema>;
+export type LetterBase = z.infer<typeof LetterSchemaBase>;
 
 export const MISchema = z.object({
   id: z.string(),