Fixes

Author: stevebux

infrastructure/terraform/components/api/locals.tf

@@ -22,10 +22,10 @@ locals {
   common_lambda_env_vars = {
     LETTERS_TABLE_NAME              = aws_dynamodb_table.letters.name,
     LETTER_QUEUE_TABLE_NAME         = aws_dynamodb_table.letter_queue.name,
-    LETTER_QUEUE_VISIBILITY_TIMEOUT = 600, # 1O minutes * 60 seconds
+    LETTER_QUEUE_VISIBILITY_TIMEOUT = 600, # 10 minutes * 60 seconds
     MI_TABLE_NAME                   = aws_dynamodb_table.mi.name,
     LETTER_TTL_HOURS                = 12960, # 18 months * 30 days * 24 hours
-    LETTER_QUEUE_TTL_HOURS          = 168    # 7 days * 24 days
+    LETTER_QUEUE_TTL_HOURS          = 168    # 7 days * 24 hours
     MI_TTL_HOURS                    = 2160   # 90 days * 24 hours
     SUPPLIER_ID_HEADER              = "nhsd-supplier-id",
     APIM_CORRELATION_HEADER         = "nhsd-correlation-id",

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

@@ -159,44 +159,28 @@ describe("LetterQueueRepository", () => {
 
   describe("updateLetterTimestamp", () => {
     it("updates the queueTimestamp on an existing letter", async () => {
-      jest.useFakeTimers().setSystemTime(new Date("2026-03-04T13:15:45.000Z"));
-      await letterQueueRepository.putLetter(createLetter());
+      const pendingLetter =
+        await letterQueueRepository.putLetter(createLetter());
 
       await letterQueueRepository.updateLetterTimestamp(
-        "supplier1",
-        "letter1",
-        600,
+        pendingLetter,
+        new Date("2026-03-04T13:15:45.000Z"),
       );
 
       const letter = await getLetter(db, "supplier1", "letter1");
-      expect(letter?.queueTimestamp).toBe("2026-03-04T13:25:45.000Z");
-    });
-
-    it("throws LetterDoesNotExistError when the letter does not exist", async () => {
-      await expect(
-        letterQueueRepository.updateLetterTimestamp("supplier1", "letter1", 60),
-      ).rejects.toThrow(LetterDoesNotExistError);
+      expect(letter?.queueTimestamp).toBe("2026-03-04T13:15:45.000Z");
     });
 
-    it("does nothing when the letter is deleted before it can be updated", async () => {
-      jest.spyOn(db.docClient, "send").mockImplementationOnce((_) => ({
-        // Fake the existence of the letter for the GetCommand
-        Item: {
-          ...createLetter(),
-          queueTimestamp: "2026-03-04T13:15:45.000Z",
-        },
-      }));
-
+    it("does nothing when the letter does not exist", async () => {
       await letterQueueRepository.updateLetterTimestamp(
-        "supplier1",
-        "letter1",
-        60,
+        createLetter(),
+        new Date(),
       );
 
       expect(await letterExists(db, "supplier1", "letter1")).toBe(false);
     });
 
-    it("rethrows errors from DynamoDB when getting the letter", async () => {
+    it("rethrows errors from DynamoDB when updating the letter", async () => {
       const misconfiguredRepository = new LetterQueueRepository(
         db.docClient,
         logger,
@@ -207,27 +191,11 @@ describe("LetterQueueRepository", () => {
       );
       await expect(
         misconfiguredRepository.updateLetterTimestamp(
-          "supplier1",
-          "letter1",
-          60,
+          createLetter(),
+          new Date(),
         ),
       ).rejects.toThrow("Cannot do operations on a non-existent table");
     });
-
-    it("rethrows errors from DynamoDB when updating the error", async () => {
-      await letterQueueRepository.putLetter(createLetter());
-      const originalSend = db.docClient.send.bind(db.docClient);
-      jest
-        .spyOn(db.docClient, "send")
-        .mockImplementationOnce(originalSend)
-        .mockImplementationOnce((_) => {
-          throw new Error("error");
-        });
-
-      await expect(
-        letterQueueRepository.updateLetterTimestamp("supplier1", "letter1", 60),
-      ).rejects.toThrow("error");
-    });
   });
 });
 

internal/datastore/src/letter-queue-repository.ts

@@ -1,7 +1,6 @@
 import {
   DeleteCommand,
   DynamoDBDocumentClient,
-  GetCommand,
   PutCommand,
   QueryCommand,
   UpdateCommand,
@@ -98,35 +97,21 @@ export default class LetterQueueRepository {
   }
 
   async updateLetterTimestamp(
-    supplierId: string,
-    letterId: string,
-    seconds: number,
+    pendingLetter: PendingLetterBase,
+    timestamp: Date,
   ): Promise<void> {
-    const item = await this.ddbClient.send(
-      new GetCommand({
-        TableName: this.config.letterQueueTableName,
-        Key: { supplierId, letterId },
-      }),
-    );
-
-    if (!item.Item) {
-      throw new LetterDoesNotExistError(supplierId, letterId);
-    }
-
-    const currentTimestamp = new Date(item.Item.queueTimestamp).getTime();
-    const updatedTimestamp = new Date(
-      currentTimestamp + seconds * 1000,
-    ).toISOString();
-
     try {
       await this.ddbClient.send(
         new UpdateCommand({
           TableName: this.config.letterQueueTableName,
-          Key: { supplierId, letterId },
+          Key: {
+            supplierId: pendingLetter.supplierId,
+            letterId: pendingLetter.letterId,
+          },
           UpdateExpression: "SET queueTimestamp = :ts",
           ConditionExpression: "attribute_exists(letterId)",
           ExpressionAttributeValues: {
-            ":ts": updatedTimestamp,
+            ":ts": timestamp.toISOString(),
           },
         }),
       );

lambdas/api-handler/src/config/__tests__/env.test.ts

@@ -21,8 +21,6 @@ describe("lambdaEnv", () => {
     process.env.LETTERS_TABLE_NAME = "letters-table";
     process.env.LETTER_QUEUE_TABLE_NAME = "letter-queue-table";
     process.env.MI_TABLE_NAME = "mi-table";
-    process.env.LETTER_QUEUE_TABLE_NAME = "letter-queue-table";
-    process.env.MI_TABLE_NAME = "mi-table";
     process.env.LETTER_TTL_HOURS = "12960";
     process.env.LETTER_QUEUE_TTL_HOURS = "240";
     process.env.MI_TTL_HOURS = "2160";

lambdas/api-handler/src/services/__tests__/letter-operations.test.ts

@@ -44,12 +44,17 @@ function makeLetter(id: string, status: Letter["status"]): Letter {
   };
 }
 
+afterEach(async () => {
+  jest.useRealTimers();
+});
+
 describe("getPendingLetters", () => {
   beforeEach(() => {
     jest.clearAllMocks();
   });
 
   it("returns letters from the letter queue repository", async () => {
+    jest.useFakeTimers().setSystemTime(new Date("2026-03-04T13:15:45.000Z"));
     const mockRepo = {
       getLetters: jest.fn().mockResolvedValue([
         {
@@ -78,15 +83,13 @@ describe("getPendingLetters", () => {
     expect(mockRepo.getLetters).toHaveBeenCalledWith("supplier1", 10);
     expect(mockRepo.updateLetterTimestamp).toHaveBeenNthCalledWith(
       1,
-      "supplier1",
-      "id1",
-      600,
+      expect.objectContaining({ supplierId: "supplier1", letterId: "id1" }),
+      new Date("2026-03-04T13:25:45.000Z"),
     );
     expect(mockRepo.updateLetterTimestamp).toHaveBeenNthCalledWith(
       2,
-      "supplier1",
-      "id2",
-      600,
+      expect.objectContaining({ supplierId: "supplier1", letterId: "id2" }),
+      new Date("2026-03-04T13:25:45.000Z"),
     );
     expect(result).toEqual([
       { id: "id1", status: "PENDING", specificationId: "s1", groupId: "g1" },

lambdas/api-handler/src/services/letter-operations.ts

@@ -52,12 +52,9 @@ export const getPendingLetters = async (
   visibilityTimeout: number,
 ): Promise<LetterBase[]> => {
   const pendingLetters = await letterQueueRepo.getLetters(supplierId, limit);
+  const timestamp = new Date(Date.now() + visibilityTimeout * 1000);
   for (const letter of pendingLetters) {
-    await letterQueueRepo.updateLetterTimestamp(
-      letter.supplierId,
-      letter.letterId,
-      visibilityTimeout,
-    );
+    await letterQueueRepo.updateLetterTimestamp(letter, timestamp);
   }
   return pendingLetters.map((letter) => mapPendingLetterToLetterBase(letter));
 };