CCM-11602: Update Get Letters endpoint

.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(),