Skip to content

CCM-15185 List Letters uses new queue #508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
stevebux wants to merge 8 commits into
base: main
Choose a base branch
from
Open

CCM-15185 List Letters uses new queue #508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a9d2fae
CCM-15185 List Letters uses new queue
stevebux Mar 31, 2026
5c492dd
Don't swallow asdf install terraform failure
stevebux Mar 31, 2026
5ac3630
Prettier fixes
stevebux Mar 31, 2026
b0e7b43
Type fix
stevebux Mar 31, 2026
b346e01
type fix
stevebux Mar 31, 2026
de56784
type cleanup
stevebux Mar 31, 2026
2350e78
Address co-pilot review comments
stevebux Apr 1, 2026
b2a3207
Correct visibility timeout to match ticket
stevebux Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions infrastructure/terraform/components/api/locals.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,19 @@ locals {
destination_arn = "arn:aws:logs:${var.region}:${var.shared_infra_account_id}:destination:nhs-main-obs-firehose-logs"

common_lambda_env_vars = {
APIM_CORRELATION_HEADER = "nhsd-correlation-id",
DOWNLOAD_URL_TTL_SECONDS = 60
EVENT_SOURCE = "/data-plane/supplier-api/${var.group}/${var.environment}/letters"
LETTER_TTL_HOURS = 12960, # 18 months * 30 days * 24 hours
LETTERS_TABLE_NAME = aws_dynamodb_table.letters.name,
MI_TABLE_NAME = aws_dynamodb_table.mi.name,
MI_TTL_HOURS = 2160 # 90 days * 24 hours
SNS_TOPIC_ARN = "${module.eventsub.sns_topic.arn}",
SUPPLIER_CONFIG_TABLE_NAME = aws_dynamodb_table.supplier-configuration.name
SUPPLIER_ID_HEADER = "nhsd-supplier-id",
APIM_CORRELATION_HEADER = "nhsd-correlation-id",
DOWNLOAD_URL_TTL_SECONDS = 60
EVENT_SOURCE = "/data-plane/supplier-api/${var.group}/${var.environment}/letters"
LETTER_TTL_HOURS = 12960, # 18 months * 30 days * 24 hours
LETTER_QUEUE_TABLE_NAME = aws_dynamodb_table.letter_queue.name,
LETTER_QUEUE_TTL_HOURS = 168 # 7 days * 24 hours
LETTER_QUEUE_VISIBILITY_TIMEOUT = 300, # 5 minutes * 60 seconds
LETTERS_TABLE_NAME = aws_dynamodb_table.letters.name,
MI_TABLE_NAME = aws_dynamodb_table.mi.name,
MI_TTL_HOURS = 2160 # 90 days * 24 hours
SNS_TOPIC_ARN = "${module.eventsub.sns_topic.arn}",
SUPPLIER_CONFIG_TABLE_NAME = aws_dynamodb_table.supplier-configuration.name
SUPPLIER_ID_HEADER = "nhsd-supplier-id",
}

core_pdf_bucket_arn = "arn:aws:s3:::comms-${var.core_account_id}-eu-west-2-${var.core_environment}-api-stg-pdf-pipeline"
Expand Down
5 changes: 3 additions & 2 deletions infrastructure/terraform/components/api/module_lambda_get_letters.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,12 @@ data "aws_iam_policy_document" "get_letters_lambda" {
"dynamodb:GetItem",
"dynamodb:Query",
"dynamodb:Scan",
"dynamodb:UpdateItem",
]

resources = [
aws_dynamodb_table.letters.arn,
"${aws_dynamodb_table.letters.arn}/index/supplierStatus-index"
aws_dynamodb_table.letter_queue.arn,
"${aws_dynamodb_table.letter_queue.arn}/index/queueSortOrder-index"
]
}
}
191 changes: 191 additions & 0 deletions internal/datastore/src/__test__/letter-queue-repository.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ describe("LetterQueueRepository", () => {
afterEach(async () => {
await deleteTables(db);
jest.useRealTimers();
jest.restoreAllMocks();
});

afterAll(async () => {
Expand Down Expand Up @@ -149,6 +150,196 @@ describe("LetterQueueRepository", () => {
).rejects.toThrow("Cannot do operations on a non-existent table");
});
});

describe("getLetters", () => {
Copy link
Copy Markdown
Contributor

@francisco-videira-nhs francisco-videira-nhs Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think it's an older issue, but on my end the editor can't find some of these jest globals.
We can either import them
import { describe, test, expect, jest } from "@jest/globals"; in each file
or
add them to tsconfig.json

  "compilerOptions": {
    "types": [
      "jest"
    ]
  },

Copy link
Copy Markdown
Contributor Author

@stevebux stevebux Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, interesting, it's fine in my editor. I find closing and re-opening vscode fixes most issues like this. Unless I did else something to fix this at some point and I've just forgotten?

Adding the import to each file would make the issue go away, I'd expect, but it shouldn't be necessary. I don't pretend to understand how tsconfig.json works, but I think your suggestion would restrict the use of globals to jest, that is prevent them from being used from other packages.

it("filters by supplierId", async () => {
await letterQueueRepository.putLetter(createLetter());

const letters = await letterQueueRepository.getLetters("supplier2", 1);

expect(letters).toHaveLength(0);
});

it("filters by visibilityTimestamp", async () => {
const pendingLetter = createLetter();
await letterQueueRepository.putLetter(createLetter());
await letterQueueRepository.updateVisibilityTimestamp(
pendingLetter,
new Date(Date.now() + 600_000),
);

const letters = await letterQueueRepository.getLetters("supplier1", 1);

expect(letters).toHaveLength(0);
});

it("returns letters in timestamp order", async () => {
jest.useFakeTimers().setSystemTime(new Date());
await letterQueueRepository.putLetter(
createLetter({ letterId: "first-letter" }),
);
jest.advanceTimersByTime(1);
await letterQueueRepository.putLetter(
createLetter({ letterId: "second-letter" }),
);
jest.advanceTimersByTime(1);
await letterQueueRepository.putLetter(
createLetter({ letterId: "third-letter" }),
);
jest.advanceTimersByTime(1);
await letterQueueRepository.putLetter(
createLetter({ letterId: "fourth-letter" }),
);
jest.advanceTimersByTime(1);
await letterQueueRepository.putLetter(
createLetter({ letterId: "fifth-letter" }),
);
jest.advanceTimersByTime(1);

const letters = await letterQueueRepository.getLetters("supplier1", 5);

expect(letters[0].letterId).toBe("first-letter");
expect(letters[1].letterId).toBe("second-letter");
expect(letters[2].letterId).toBe("third-letter");
expect(letters[3].letterId).toBe("fourth-letter");
expect(letters[4].letterId).toBe("fifth-letter");
});

it("limits results to the supplied number", async () => {
await letterQueueRepository.putLetter(
createLetter({ letterId: "first-letter" }),
);
await letterQueueRepository.putLetter(
createLetter({ letterId: "second-letter" }),
);
await letterQueueRepository.putLetter(
createLetter({ letterId: "third-letter" }),
);
await letterQueueRepository.putLetter(
createLetter({ letterId: "fourth-letter" }),
);

const letters = await letterQueueRepository.getLetters("supplier1", 3);

expect(letters).toHaveLength(3);
expect(letters[2].letterId).toBe("third-letter");
});

it("applies the limit after filtering on supplier", async () => {
await letterQueueRepository.putLetter(
createLetter({ letterId: "first-letter" }),
);
await letterQueueRepository.putLetter(
createLetter({ letterId: "second-letter", supplierId: "supplier2" }),
);
await letterQueueRepository.putLetter(
createLetter({ letterId: "third-letter" }),
);
await letterQueueRepository.putLetter(
createLetter({ letterId: "fourth-letter" }),
);

const letters = await letterQueueRepository.getLetters("supplier1", 3);

expect(letters).toHaveLength(3);
expect(letters[2].letterId).toBe("fourth-letter");
});

it("applies the limit after filtering on visibilityTimestamp", async () => {
await letterQueueRepository.putLetter(
createLetter({ letterId: "first-letter" }),
);
await letterQueueRepository.putLetter(
createLetter({ letterId: "second-letter" }),
);
await letterQueueRepository.putLetter(
createLetter({ letterId: "third-letter" }),
);
await letterQueueRepository.putLetter(
createLetter({ letterId: "fourth-letter" }),
);
await letterQueueRepository.updateVisibilityTimestamp(
createLetter({ letterId: "second-letter" }),
new Date(Date.now() + 600_000),
);

const letters = await letterQueueRepository.getLetters("supplier1", 3);

expect(letters).toHaveLength(3);
expect(letters[2].letterId).toBe("fourth-letter");
});

it("paginates through multiple DynamoDB pages to reach the limit", async () => {
await letterQueueRepository.putLetter(
createLetter({ letterId: "first-letter" }),
);
await letterQueueRepository.putLetter(
createLetter({ letterId: "second-letter" }),
);
await letterQueueRepository.putLetter(
createLetter({ letterId: "third-letter" }),
);

const pagedRepository = new LetterQueueRepository(db.docClient, logger, {
...db.config,
queryPageSize: 1,
});

const letters = await pagedRepository.getLetters("supplier1", 3);

expect(letters).toHaveLength(3);
expect(letters[0].letterId).toBe("first-letter");
expect(letters[1].letterId).toBe("second-letter");
expect(letters[2].letterId).toBe("third-letter");
});

it("returns an empty array if no items found", async () => {
const letters = await letterQueueRepository.getLetters("supplier1", 3);

expect(letters).toHaveLength(0);
});
});

describe("updateVisibilityTimestamp", () => {
it("updates the visibilityTimestamp on an existing letter", async () => {
const pendingLetter =
await letterQueueRepository.putLetter(createLetter());

await letterQueueRepository.updateVisibilityTimestamp(
pendingLetter,
new Date("2026-03-04T13:15:45.000Z"),
);

const letter = await getLetter(db, "supplier1", "letter1");
expect(letter?.visibilityTimestamp).toBe("2026-03-04T13:15:45.000Z");
});

it("does nothing when the letter does not exist", async () => {
await letterQueueRepository.updateVisibilityTimestamp(
createLetter(),
new Date(),
);

expect(await letterExists(db, "supplier1", "letter1")).toBe(false);
});

it("rethrows errors from DynamoDB when updating the letter", async () => {
const misconfiguredRepository = new LetterQueueRepository(
db.docClient,
logger,
{
...db.config,
letterQueueTableName: "nonexistent-table",
},
);
await expect(
misconfiguredRepository.updateVisibilityTimestamp(
createLetter(),
new Date(),
),
).rejects.toThrow("Cannot do operations on a non-existent table");
});
});
});

async function getLetter(db: DBContext, supplierId: string, letterId: string) {
Expand Down
Loading
Loading