Skip to content

CCM-12392: Allocation idempotency#574

Open
nhsd-david-wass wants to merge 2 commits into
mainfrom
feature/CCM-12392-Letter-Event-Idempotency
Open

CCM-12392: Allocation idempotency#574
nhsd-david-wass wants to merge 2 commits into
mainfrom
feature/CCM-12392-Letter-Event-Idempotency

Conversation

@nhsd-david-wass
Copy link
Copy Markdown
Contributor

Description

Context

Type of changes

  • Refactoring (non-breaking change)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would change existing functionality)
  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I am familiar with the contributing guidelines
  • I have followed the code style of the project
  • I have added tests to cover my changes
  • I have updated the documentation accordingly
  • This PR is a result of pair or mob programming

DT3-Specific Checklist

  • If I have added a new resource (SQS, Lambda, Gateway, DDB table, etc), I have created the appropriate alarms

Sensitive Information Declaration

To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.

  • I confirm that neither PII/PID nor sensitive data are included in this PR and the codebase changes.

@nhsd-david-wass nhsd-david-wass requested review from a team as code owners May 12, 2026 10:23
@nhsd-david-wass nhsd-david-wass force-pushed the feature/CCM-12392-Letter-Event-Idempotency branch 11 times, most recently from dfbf624 to e1cfaab Compare May 19, 2026 09:47
@nhsd-david-wass nhsd-david-wass force-pushed the feature/CCM-12392-Letter-Event-Idempotency branch 2 times, most recently from 54ba53d to 2a6c9b6 Compare May 20, 2026 08:35
Comment thread tests/helpers/send-sqs-event.ts Fixed
@nhsd-david-wass nhsd-david-wass force-pushed the feature/CCM-12392-Letter-Event-Idempotency branch 4 times, most recently from 7a90a03 to d938b0a Compare May 21, 2026 07:34
@nhsd-david-wass nhsd-david-wass force-pushed the feature/CCM-12392-Letter-Event-Idempotency branch 2 times, most recently from 47a2ce5 to 40b7347 Compare June 1, 2026 14:22
expect(status).toBe("REJECTED");
});

test("Verify that supplier allocations are correctly updated for a volume group", async () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you please update this reflect allocation happens only once for multiple copies of same request

@nhsd-david-wass nhsd-david-wass force-pushed the feature/CCM-12392-Letter-Event-Idempotency branch from 40b7347 to 35d8a5e Compare June 3, 2026 08:27
Copy link
Copy Markdown
Contributor

@stevebux stevebux left a comment

Choose a reason for hiding this comment

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

Looks good to me, a few suggested changes


export default function createSupplierAllocatorHandler(deps: Deps): SQSHandler {
return async (event: SQSEvent) => {
const getSupplierIdempotently = (
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I got a little confused by the naming here - this function is creating another function that gets the supplier idempotently, rather than doing it directly. Maybe call it createGetSupplierIdempotently and rename boundGetSupplierIdempotently below to just getSupplierIdempotently?

const perAllocationFailure: AllocationMetrics = new Map();
const volumeGroupAllocations: VolumeGroupAllocation = new Map();

// create an idempotent function bound to this handler's batchItemFailures
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment mentions batchItemFailures, but they're not bound to the function being created

@nhsd-david-wass nhsd-david-wass force-pushed the feature/CCM-12392-Letter-Event-Idempotency branch from 7486aec to d08117d Compare June 4, 2026 12:34
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 4, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants