Skip to content

Commit 23be770

Browse files
nhsd-david-wassnhsd-david-wass
committed
pre review tidy
1 parent 7d035cc commit 23be770

6 files changed

Lines changed: 11 additions & 52 deletions

File tree

‎infrastructure/terraform/components/api/README.md‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ No requirements.
3737
| <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 |
3838
| <a name="input_letter_event_source"></a> [letter\_event\_source](#input\_letter\_event\_source) | Source value to use for the letter status event updates | `string` | `"/data-plane/supplier-api/nhs-supplier-api-prod/main/update-status"` | no |
3939
| <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 |
40-
| <a name="input_letter_variant_map"></a> [letter\_variant\_map](#input\_letter\_variant\_map) | n/a | `map(object({ supplierId = string, specId = string, priority = number, billingId = string }))` | <pre>{<br/> "client1-aspiring": {<br/> "billingId": "client1-aspiring-billing",<br/> "priority": "0",<br/> "specId": "client1-aspiring",<br/> "supplierId": "supplier1"<br/> },<br/> "client1-dmapp": {<br/> "billingId": "notify-admail-billing",<br/> "priority": "1",<br/> "specId": "notify-admail",<br/> "supplierId": "supplier1"<br/> },<br/> "client1-globalminds": {<br/> "billingId": "client1-globalminds-billing",<br/> "priority": "2",<br/> "specId": "client1-globalminds",<br/> "supplierId": "supplier1"<br/> },<br/> "client1-mymelanoma": {<br/> "billingId": "client1-mymelanoma-billing",<br/> "priority": "3",<br/> "specId": "client1-mymelanoma",<br/> "supplierId": "supplier1"<br/> },<br/> "client1-ofh": {<br/> "billingId": "client1-ofh-billing",<br/> "priority": "4",<br/> "specId": "client1-ofh",<br/> "supplierId": "supplier1"<br/> },<br/> "client1-prostateprogress": {<br/> "billingId": "client1-prostateprogress-billing",<br/> "priority": "5",<br/> "specId": "client1-prostateprogress",<br/> "supplierId": "supplier1"<br/> },<br/> "client1-protectc": {<br/> "billingId": "notify-c5-colour-billing",<br/> "priority": "6",<br/> "specId": "notify-c5-colour",<br/> "supplierId": "supplier1"<br/> },<br/> "client1-restore": {<br/> "billingId": "client1-restore-billing",<br/> "priority": "7",<br/> "specId": "client1-restore",<br/> "supplierId": "supplier1"<br/> },<br/> "gpreg-admail": {<br/> "billingId": "notify-admail-billing",<br/> "priority": "8",<br/> "specId": "notify-admail",<br/> "supplierId": "supplier1"<br/> },<br/> "nces-abnormal-results": {<br/> "billingId": "nces-abnormal-results-billing",<br/> "priority": "9",<br/> "specId": "nces-abnormal-results",<br/> "supplierId": "supplier1"<br/> },<br/> "nces-abnormal-results-braille": {<br/> "billingId": "nces-abnormal-results-braille-billing",<br/> "priority": "10",<br/> "specId": "nces-abnormal-results-braille",<br/> "supplierId": "supplier1"<br/> },<br/> "nces-invites": {<br/> "billingId": "nces-invites-billing",<br/> "priority": "10",<br/> "specId": "nces-invites",<br/> "supplierId": "supplier1"<br/> },<br/> "nces-invites-braille": {<br/> "billingId": "nces-invites-braille-billing",<br/> "priority": "10",<br/> "specId": "nces-invites-braille",<br/> "supplierId": "supplier1"<br/> },<br/> "nces-standard": {<br/> "billingId": "notify-c5-whitemail-billing",<br/> "priority": "11",<br/> "specId": "notify-c5-whitemail",<br/> "supplierId": "supplier1"<br/> },<br/> "nces-standard-braille": {<br/> "billingId": "notify-braille-whitemail-billing",<br/> "priority": "12",<br/> "specId": "notify-braille-whitemail",<br/> "supplierId": "supplier1"<br/> },<br/> "notify-braille": {<br/> "billingId": "notify-braille-billing",<br/> "priority": "13",<br/> "specId": "notify-braille",<br/> "supplierId": "supplier1"<br/> },<br/> "notify-digital-letters-standard": {<br/> "billingId": "notify-c5-billing",<br/> "priority": "97",<br/> "specId": "notify-c5",<br/> "supplierId": "supplier1"<br/> },<br/> "notify-standard": {<br/> "billingId": "notify-c5-billing",<br/> "priority": "98",<br/> "specId": "notify-c5",<br/> "supplierId": "supplier1"<br/> },<br/> "notify-standard-colour": {<br/> "billingId": "notify-c5-colour-billing",<br/> "priority": "99",<br/> "specId": "notify-c5-colour",<br/> "supplierId": "supplier1"<br/> }<br/>}</pre> | no |
4140
| <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 |
4241
| <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 |
4342
| <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 |

‎infrastructure/terraform/components/api/ddb_table_supplier_quotas.tf‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,21 +20,6 @@ resource "aws_dynamodb_table" "supplier-quotas" {
2020
type = "S"
2121
}
2222

23-
attribute {
24-
name = "entityType"
25-
type = "S"
26-
}
27-
28-
29-
30-
// The type-index GSI allows us to query for all supplier quotas of a given type (e.g. all supplier daily quotas)
31-
global_secondary_index {
32-
name = "EntityTypeIndex"
33-
hash_key = "entityType"
34-
range_key = "sk"
35-
projection_type = "ALL"
36-
}
37-
3823
point_in_time_recovery {
3924
enabled = true
4025
}

‎infrastructure/terraform/components/api/variables.tf‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -135,31 +135,6 @@ variable "eventpub_control_plane_bus_arn" {
135135
default = ""
136136
}
137137

138-
variable "letter_variant_map" {
139-
type = map(object({ supplierId = string, specId = string, priority = number, billingId = string }))
140-
default = {
141-
"client1-aspiring" = { supplierId = "supplier1", specId = "client1-aspiring", priority = "0", billingId = "client1-aspiring-billing" },
142-
"client1-dmapp" = { supplierId = "supplier1", specId = "notify-admail", priority = "1", billingId = "notify-admail-billing" },
143-
"client1-globalminds" = { supplierId = "supplier1", specId = "client1-globalminds", priority = "2", billingId = "client1-globalminds-billing" },
144-
"client1-mymelanoma" = { supplierId = "supplier1", specId = "client1-mymelanoma", priority = "3", billingId = "client1-mymelanoma-billing" },
145-
"client1-ofh" = { supplierId = "supplier1", specId = "client1-ofh", priority = "4", billingId = "client1-ofh-billing" },
146-
"client1-prostateprogress" = { supplierId = "supplier1", specId = "client1-prostateprogress", priority = "5", billingId = "client1-prostateprogress-billing" },
147-
"client1-protectc" = { supplierId = "supplier1", specId = "notify-c5-colour", priority = "6", billingId = "notify-c5-colour-billing" },
148-
"client1-restore" = { supplierId = "supplier1", specId = "client1-restore", priority = "7", billingId = "client1-restore-billing" },
149-
"gpreg-admail" = { supplierId = "supplier1", specId = "notify-admail", priority = "8", billingId = "notify-admail-billing" },
150-
"nces-abnormal-results" = { supplierId = "supplier1", specId = "nces-abnormal-results", priority = "9", billingId = "nces-abnormal-results-billing" },
151-
"nces-abnormal-results-braille" = { supplierId = "supplier1", specId = "nces-abnormal-results-braille", priority = "10", billingId = "nces-abnormal-results-braille-billing" },
152-
"nces-invites" = { supplierId = "supplier1", specId = "nces-invites", priority = "10", billingId = "nces-invites-billing" },
153-
"nces-invites-braille" = { supplierId = "supplier1", specId = "nces-invites-braille", priority = "10", billingId = "nces-invites-braille-billing" },
154-
"nces-standard" = { supplierId = "supplier1", specId = "notify-c5-whitemail", priority = "11", billingId = "notify-c5-whitemail-billing" },
155-
"nces-standard-braille" = { supplierId = "supplier1", specId = "notify-braille-whitemail", priority = "12", billingId = "notify-braille-whitemail-billing" },
156-
"notify-braille" = { supplierId = "supplier1", specId = "notify-braille", priority = "13", billingId = "notify-braille-billing" },
157-
"notify-digital-letters-standard" = { supplierId = "supplier1", specId = "notify-c5", priority = "97", billingId = "notify-c5-billing" },
158-
"notify-standard" = { supplierId = "supplier1", specId = "notify-c5", priority = "98", billingId = "notify-c5-billing" },
159-
"notify-standard-colour" = { supplierId = "supplier1", specId = "notify-c5-colour", priority = "99", billingId = "notify-c5-colour-billing" }
160-
}
161-
}
162-
163138
variable "disable_gateway_execute_endpoint" {
164139
type = bool
165140
description = "Disable the execution endpoint for the API Gateway"

‎internal/datastore/src/__test__/supplier-quotas-repository.test.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,8 @@ describe("SupplierQuotasRepository", () => {
150150
);
151151

152152
const result = await repository.getOverallAllocation(volumeGroupId);
153-
expect(result?.allocations[supplierId]).toBe(150);
153+
const resultMap = new Map(Object.entries(result?.allocations ?? {}));
154+
expect(resultMap.get(supplierId)).toBe(150);
154155
});
155156

156157
test("getDailyAllocation returns correct allocation for existing group and date", async () => {
@@ -225,6 +226,7 @@ describe("SupplierQuotasRepository", () => {
225226
await repository.updateDailyAllocation(date, supplierId, newAllocation);
226227

227228
const result = await repository.getDailyAllocation(date);
228-
expect(result?.allocations[supplierId]).toBe(75);
229+
const resultMap = new Map(Object.entries(result?.allocations ?? {}));
230+
expect(resultMap.get(supplierId)).toBe(75);
229231
});
230232
});

‎internal/datastore/src/supplier-quotas-repository.ts‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,9 @@ export class SupplierQuotasRepository {
7474
): Promise<void> {
7575
const overallAllocation = await this.getOverallAllocation(groupId);
7676
const allocations = overallAllocation?.allocations ?? {};
77-
const currentAllocation = allocations[supplierId] ?? 0;
77+
const allocationsMap = new Map(Object.entries(allocations));
78+
const currentAllocation = allocationsMap.get(supplierId) ?? 0;
79+
7880
const updatedAllocation = currentAllocation + newAllocation;
7981

8082
if (overallAllocation) {
@@ -107,7 +109,6 @@ export class SupplierQuotasRepository {
107109
}
108110

109111
async getDailyAllocation(date: string): Promise<DailyAllocation | undefined> {
110-
console.log("Getting daily allocation for date:", date);
111112
const result = await this.ddbClient.send(
112113
new GetCommand({
113114
TableName: this.config.supplierQuotasTableName,
@@ -118,7 +119,6 @@ export class SupplierQuotasRepository {
118119
}),
119120
);
120121
if (!result.Item) {
121-
console.log("No daily allocation found for date:", date);
122122
return undefined;
123123
}
124124
// Strip DynamoDB keys before parsing
@@ -129,8 +129,7 @@ export class SupplierQuotasRepository {
129129

130130
async putDailyAllocation(allocation: DailyAllocation): Promise<void> {
131131
const parsedAllocation = $DailyAllocation.parse(allocation);
132-
console.log("Putting daily allocation:", parsedAllocation);
133-
const output = await this.ddbClient.send(
132+
await this.ddbClient.send(
134133
new PutCommand({
135134
TableName: this.config.supplierQuotasTableName,
136135
Item: ItemForRecord(
@@ -140,7 +139,6 @@ export class SupplierQuotasRepository {
140139
),
141140
}),
142141
);
143-
console.log("PutDailyAllocation output:", output);
144142
}
145143

146144
async updateDailyAllocation(
@@ -150,7 +148,8 @@ export class SupplierQuotasRepository {
150148
): Promise<void> {
151149
const dailyAllocation = await this.getDailyAllocation(date);
152150
const allocations = dailyAllocation?.allocations ?? {};
153-
const currentAllocation = allocations[supplierId] ?? 0;
151+
const allocationsMap = new Map(Object.entries(allocations));
152+
const currentAllocation = allocationsMap.get(supplierId) ?? 0;
154153
const updatedAllocation = currentAllocation + newAllocation;
155154

156155
if (dailyAllocation) {

‎lambdas/supplier-allocator/src/handler/allocate-handler.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,7 @@ export default function createSupplierAllocatorHandler(deps: Deps): SQSHandler {
196196
const batchItemFailures: SQSBatchItemFailure[] = [];
197197
const perAllocationSuccess: AllocationMetrics = new Map();
198198
const perAllocationFailure: AllocationMetrics = new Map();
199-
const volumeGroupAllocations: VolumeGroupAllocation = new Map(); // Map of volume group id to supplier allocations for that group, used to track the allocations calculated in this batch for emitting metrics and updating the quotas after processing the batch
200-
// Initialise the supplier quotas.
199+
const volumeGroupAllocations: VolumeGroupAllocation = new Map();
201200

202201
const tasks = event.Records.map(async (record) => {
203202
let supplier = "unknown";

0 commit comments

Comments
 (0)