Skip to content

Commit 214ef5f

Browse files
simonlabareresimonlabarere
committed
CCM-11602: Get enpoint tests
1 parent 6b2aa91 commit 214ef5f

File tree

11 files changed

+183
-153
lines changed

11 files changed

+183
-153
lines changed

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ resource "aws_dynamodb_table" "letters" {
1313
global_secondary_index {
1414
name = "supplierStatus-index"
1515
hash_key = "supplierStatus"
16-
range_key = "id"
16+
range_key = "supplierStatusSk"
1717
projection_type = "ALL"
1818
}
1919

@@ -32,6 +32,11 @@ resource "aws_dynamodb_table" "letters" {
3232
type = "S"
3333
}
3434

35+
attribute {
36+
name = "supplierStatusSk"
37+
type = "S"
38+
}
39+
3540
point_in_time_recovery {
3641
enabled = true
3742
}

‎internal/datastore/src/__test__/db.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export async function createTables(context: DBContext) {
5959
IndexName: 'supplierStatus-index',
6060
KeySchema: [
6161
{ AttributeName: 'supplierStatus', KeyType: 'HASH' }, // Partition key for GSI
62-
{ AttributeName: 'id', KeyType: 'RANGE' } // Sort key for GSI
62+
{ AttributeName: 'supplierStatusSk', KeyType: 'RANGE' } // Sort key for GSI
6363
],
6464
Projection: {
6565
ProjectionType: 'ALL'
@@ -69,7 +69,8 @@ export async function createTables(context: DBContext) {
6969
AttributeDefinitions: [
7070
{ AttributeName: 'supplierId', AttributeType: 'S' },
7171
{ AttributeName: 'id', AttributeType: 'S' },
72-
{ AttributeName: 'supplierStatus', AttributeType: 'S' }
72+
{ AttributeName: 'supplierStatus', AttributeType: 'S' },
73+
{ AttributeName: 'supplierStatusSk', AttributeType: 'S' },
7374
]
7475
}));
7576

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

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { Logger } from 'pino';
55
import { createTestLogger, LogStream } from './logs';
66
import { PutCommand } from '@aws-sdk/lib-dynamodb';
77

8-
function createLetter(supplierId: string, letterId: string, status: Letter['status'] = 'PENDING'): Omit<Letter, 'ttl' | 'supplierStatus'> {
8+
function createLetter(supplierId: string, letterId: string, status: Letter['status'] = 'PENDING'): Omit<Letter, 'ttl' | 'supplierStatus' | 'supplierStatusSk'> {
99
return {
1010
id: letterId,
1111
supplierId: supplierId,
@@ -79,7 +79,7 @@ describe('LetterRepository', () => {
7979
}));
8080
});
8181

82-
test('throws an error when fetching a letter that does not exist', async () => {
82+
test('throws an error when fetching a letter that does not e ist', async () => {
8383
await expect(letterRepository.getLetterById('supplier1', 'letter1'))
8484
.rejects.toThrow('Letter with id letter1 not found for supplier supplier1');
8585
});
@@ -205,6 +205,7 @@ describe('LetterRepository', () => {
205205
url: 's3://bucket/invalid-letter.pdf',
206206
status: 'PENDING',
207207
supplierStatus: 'supplier1#PENDING',
208+
supplierStatusSk: 'supplier1',
208209
createdAt: new Date().toISOString(),
209210
updatedAt: new Date().toISOString()
210211
}
@@ -218,30 +219,45 @@ describe('LetterRepository', () => {
218219
expect(logStream.logs).toContainEqual(expect.stringMatching(/.*specificationId.*Invalid input: expected string.*/));
219220
});
220221

221-
test('should return all letter ids for a supplier', async () => {
222-
await letterRepository.putLetter(createLetter('supplier1', 'letter1'));
223-
await letterRepository.putLetter(createLetter('supplier1', 'letter2'));
224-
await letterRepository.putLetter(createLetter('supplier1', 'letter3'));
225-
await letterRepository.putLetter(createLetter('supplier2', 'letter4'));
226-
await letterRepository.putLetter(createLetter('supplier2', 'letter5'));
227-
228-
const letters = await letterRepository.getLetterIdsBySupplier('supplier1');
229-
expect(letters).toEqual(['letter1', 'letter2', 'letter3']);
230-
});
231-
232-
test('should return empty if no letters exist for a supplier', async () => {
233-
await letterRepository.putLetter(createLetter('supplier1', 'letter1'));
234-
await letterRepository.putLetter(createLetter('supplier1', 'letter2'));
222+
test("should return all letters for a supplier a status", async () => {
223+
await letterRepository.putLetter(createLetter("supplier1", "letter1"));
224+
await letterRepository.putLetter(createLetter("supplier1", "letter2"));
225+
await letterRepository.putLetter(createLetter("supplier1", "letter3"));
226+
await letterRepository.putLetter(
227+
createLetter("supplier1", "letter4", "REJECTED"),
228+
);
229+
await letterRepository.putLetter(createLetter("supplier2", "letter1"));
230+
await letterRepository.putLetter(createLetter("supplier2", "letter2"));
235231

236-
const letters = await letterRepository.getLetterIdsBySupplier('supplier2');
237-
expect(letters).toEqual([]);
232+
const letters = await letterRepository.getLettersBySupplier(
233+
"supplier1",
234+
"PENDING",
235+
10,
236+
);
237+
expect(letters).toEqual([
238+
{
239+
id: "letter1",
240+
specificationId: "specification1",
241+
status: "PENDING",
242+
},
243+
{
244+
id: "letter2",
245+
specificationId: "specification1",
246+
status: "PENDING",
247+
},
248+
{
249+
id: "letter3",
250+
specificationId: "specification1",
251+
status: "PENDING",
252+
},
253+
]);
238254
});
239255

240256
test('should return empty if no letters exist for a supplier', async () => {
241257
await letterRepository.putLetter(createLetter('supplier1', 'letter1'));
242258
await letterRepository.putLetter(createLetter('supplier1', 'letter2'));
243259

244-
const letters = await letterRepository.getLetterIdsBySupplier('supplier2');
260+
const letters = await letterRepository.getLettersBySupplier('supplier2', 'PENDING', 10);
245261
expect(letters).toEqual([]);
246262
});
247263

@@ -252,7 +268,7 @@ describe('LetterRepository', () => {
252268
const mockDdbClient = { send: mockSend } as any;
253269
const repo = new LetterRepository(mockDdbClient, { debug: jest.fn() } as any, { lettersTableName: 'letters', ttlHours: 1 });
254270

255-
const result = await repo.getLetterIdsBySupplier('supplier1');
256-
expect(result).toEqual([]);
271+
const letters = await repo.getLettersBySupplier('supplier1', 'PENDING', 10);
272+
expect(letters).toEqual([]);
257273
});
258274
});

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

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
UpdateCommand,
77
UpdateCommandOutput
88
} from '@aws-sdk/lib-dynamodb';
9-
import { Letter, LetterSchema, LettersSchema } from './types';
9+
import { Letter, LetterBase, LetterSchema, LetterSchemaBase } from './types';
1010
import { Logger } from 'pino';
1111
import { z } from 'zod';
1212

@@ -30,10 +30,11 @@ export class LetterRepository {
3030
readonly config: LetterRepositoryConfig) {
3131
}
3232

33-
async putLetter(letter: Omit<Letter, 'ttl' | 'supplierStatus'>): Promise<Letter> {
33+
async putLetter(letter: Omit<Letter, 'ttl' | 'supplierStatus' | 'supplierStatusSk'>): Promise<Letter> {
3434
const letterDb: Letter = {
3535
...letter,
3636
supplierStatus: `${letter.supplierId}#${letter.status}`,
37+
supplierStatusSk: letter.id,
3738
ttl: Math.floor(Date.now() / 1000 + 60 * 60 * this.config.ttlHours)
3839
};
3940
try {
@@ -131,37 +132,21 @@ export class LetterRepository {
131132
return LetterSchema.parse(result.Attributes);
132133
}
133134

134-
async getLettersBySupplier(supplierId: string, status: string, size: number, cursor?: string): Promise<{
135-
nextCursor: string;
136-
letters: Letter[]}> {
135+
async getLettersBySupplier(supplierId: string, status: string, size: number): Promise<LetterBase[]> {
137136
const supplierStatus = `${supplierId}#${status}`;
138137
const result = await this.ddbClient.send(new QueryCommand({
139138
TableName: this.config.lettersTableName,
140139
IndexName: 'supplierStatus-index',
141140
KeyConditionExpression: 'supplierStatus = :supplierStatus',
142141
Limit: size,
142+
ExpressionAttributeNames: {
143+
'#status': 'status' // reserved keyword
144+
},
143145
ExpressionAttributeValues: {
144146
':supplierStatus': supplierStatus
145147
},
146-
...(cursor != undefined ? {
147-
ExclusiveStartKey: {
148-
id: cursor,
149-
supplierStatus,
150-
supplierId,
151-
}
152-
} : {})
148+
ProjectionExpression: 'id, #status, specificationId, reasonCode, reasonText'
153149
}));
154-
this.log.info({
155-
description: 'items',
156-
supplierId,
157-
status,
158-
size,
159-
cursor,
160-
result,
161-
})
162-
return {
163-
nextCursor: result.LastEvaluatedKey?.id,
164-
letters: z.array(LetterSchema).parse(result.Items)
165-
};
150+
return z.array(LetterSchemaBase).parse(result.Items ?? []);
166151
}
167152
}

‎internal/datastore/src/types.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,23 @@ export const LetterStatus = z.enum([
1717
'ENCLOSED', 'CANCELLED', 'DISPATCHED', 'FAILED',
1818
'RETURNED', 'DESTROYED', 'FORWARDED', 'DELIVERED']);
1919

20-
export const LetterSchema = z.object({
20+
export const LetterSchemaBase = z.object({
2121
id: z.string(),
22-
supplierId: z.string(),
22+
status: LetterStatus,
2323
specificationId: z.string(),
24+
reasonCode: z.number().optional(),
25+
reasonText: z.string().optional()
26+
});
27+
28+
export const LetterSchema = LetterSchemaBase.extend({
29+
supplierId: z.string(),
2430
groupId: z.string(),
2531
url: z.url(),
26-
status: LetterStatus,
2732
createdAt: z.string(),
2833
updatedAt: z.string(),
2934
supplierStatus: z.string().describe('Secondary index PK'),
30-
ttl: z.int()
35+
supplierStatusSk: z.string().describe('Secondary index SK'),
36+
ttl: z.int(),
3137
}).describe('Letter');
3238

3339
/**
@@ -36,6 +42,7 @@ export const LetterSchema = z.object({
3642
* The ttl is used for automatic deletion of old letters.
3743
*/
3844
export type Letter = z.infer<typeof LetterSchema>;
45+
export type LetterBase = z.infer<typeof LetterSchemaBase>;
3946

4047
export const MISchema = z.object({
4148
id: z.string(),

‎lambdas/api-handler/src/handlers/__tests__/get-letters.test.ts‎

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,28 +14,49 @@ describe('API Lambda handler', () => {
1414

1515
it('returns 200 OK with basic paginated resources', async () => {
1616

17-
const mockedGetLetterIds = letterService.getLetterIdsForSupplier as jest.Mock;
18-
mockedGetLetterIds.mockResolvedValue(['l1', 'l2', 'l3']);
17+
const mockedGetLetters = letterService.getLettersForSupplier as jest.Mock;
18+
mockedGetLetters.mockResolvedValue([
19+
{
20+
id: "l1",
21+
specificationId: "s1",
22+
status: "PENDING",
23+
},
24+
{
25+
id: "l2",
26+
specificationId: "s1",
27+
status: "PENDING",
28+
},
29+
{
30+
id: "l3",
31+
specificationId: "s1",
32+
status: "PENDING",
33+
},
34+
]);
1935

2036
const event = makeApiGwEvent({path: '/letters'});
2137
const context = mockDeep<Context>();
2238
const callback = jest.fn();
2339
const result = await getLetters(event, context, callback);
2440

2541
const expected = {
26-
"links": {
27-
"self": "/letters?page=1",
28-
"first": "/letters?page=1",
29-
"last": "/letters?page=1",
30-
"next": "/letters?page=1",
31-
"prev": "/letters?page=1"
32-
},
33-
"data": [
34-
{ "type": "letter", "id": "l1" },
35-
{ "type": "letter", "id": "l2" },
36-
{ "type": "letter", "id": "l3" }
37-
]
38-
}
42+
data: [
43+
{
44+
id: "l1",
45+
type: "Letter",
46+
attributes: { specificationId: "s1", status: "PENDING" },
47+
},
48+
{
49+
id: "l2",
50+
type: "Letter",
51+
attributes: { specificationId: "s1", status: "PENDING" },
52+
},
53+
{
54+
id: "l3",
55+
type: "Letter",
56+
attributes: { specificationId: "s1", status: "PENDING" },
57+
},
58+
],
59+
};
3960

4061
expect(result).toEqual({
4162
statusCode: 200,
@@ -54,4 +75,16 @@ describe('API Lambda handler', () => {
5475
body: 'Not Found',
5576
});
5677
});
78+
79+
it('returns 400 for missing supplier ID', async () => {
80+
const event = makeApiGwEvent({ path: "/letters", headers: {} });
81+
const context = mockDeep<Context>();
82+
const callback = jest.fn();
83+
const result = await getLetters(event, context, callback);
84+
85+
expect(result).toEqual({
86+
statusCode: 400,
87+
body: 'Bad Request: Missing supplier ID',
88+
});
89+
});
5790
});

‎lambdas/api-handler/src/handlers/__tests__/utils/test-utils.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ export function makeApiGwEvent(
77
resource: '/{proxy+}',
88
path: '/',
99
httpMethod: 'GET',
10-
headers: {},
10+
headers: {
11+
'nhsd-supplier-id': 'supplier1'
12+
},
1113
multiValueHeaders: {},
1214
queryStringParameters: null,
1315
multiValueQueryStringParameters: null,

0 commit comments

Comments
 (0)