Skip to content

Commit c94892f

Browse files
francisco-videira-nhsfrancisco-videira-nhs
committed
Refactor contracts and mappers
1 parent 6e5bf07 commit c94892f

File tree

13 files changed

+276
-201
lines changed

13 files changed

+276
-201
lines changed

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

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { Letter } from '../types';
44
import { Logger } from 'pino';
55
import { createTestLogger, LogStream } from './logs';
66
import { PutCommand } from '@aws-sdk/lib-dynamodb';
7+
import { LetterDto } from '../../../../lambdas/api-handler/src/contracts/letters';
78

89
function createLetter(supplierId: string, letterId: string, status: Letter['status'] = 'PENDING'): Omit<Letter, 'ttl' | 'supplierStatus' | 'supplierStatusSk'> {
910
return {
@@ -106,7 +107,14 @@ describe('LetterRepository', () => {
106107
await letterRepository.putLetter(letter);
107108
await checkLetterStatus('supplier1', 'letter1', 'PENDING');
108109

109-
await letterRepository.updateLetterStatus('supplier1', 'letter1', 'REJECTED', 1, "Reason text");
110+
const letterDto: LetterDto = {
111+
id: 'letter1',
112+
supplierId: 'supplier1',
113+
status: 'REJECTED',
114+
reasonCode: 1,
115+
reasonText: 'Reason text'
116+
};
117+
await letterRepository.updateLetterStatus(letterDto);
110118

111119
const updatedLetter = await letterRepository.getLetterById('supplier1', 'letter1');
112120
expect(updatedLetter.status).toBe('REJECTED');
@@ -124,13 +132,25 @@ describe('LetterRepository', () => {
124132
// Month is zero-indexed in JavaScript Date
125133
// Day is one-indexed
126134
jest.setSystemTime(new Date(2020, 1, 2));
127-
await letterRepository.updateLetterStatus('supplier1', 'letter1', 'DELIVERED', undefined, undefined);
135+
const letterDto: LetterDto = {
136+
id: 'letter1',
137+
supplierId: 'supplier1',
138+
status: 'DELIVERED'
139+
};
140+
141+
await letterRepository.updateLetterStatus(letterDto);
128142
const updatedLetter = await letterRepository.getLetterById('supplier1', 'letter1');
143+
129144
expect(updatedLetter.updatedAt).toBe('2020-02-02T00:00:00.000Z');
130145
});
131146

132147
test('can\'t update a letter that does not exist', async () => {
133-
await expect(letterRepository.updateLetterStatus('supplier1', 'letter1', 'DELIVERED', undefined, undefined))
148+
const letterDto: LetterDto = {
149+
id: 'letter1',
150+
supplierId: 'supplier1',
151+
status: 'DELIVERED'
152+
};
153+
await expect(letterRepository.updateLetterStatus(letterDto))
134154
.rejects.toThrow('Letter with id letter1 not found for supplier supplier1');
135155
});
136156

@@ -139,7 +159,13 @@ describe('LetterRepository', () => {
139159
...db.config,
140160
lettersTableName: 'nonexistent-table'
141161
});
142-
await expect(misconfiguredRepository.updateLetterStatus('supplier1', 'letter1', 'DELIVERED', undefined, undefined))
162+
163+
const letterDto: LetterDto = {
164+
id: 'letter1',
165+
supplierId: 'supplier1',
166+
status: 'DELIVERED'
167+
};
168+
await expect(misconfiguredRepository.updateLetterStatus(letterDto))
143169
.rejects.toThrow('Cannot do operations on a non-existent table');
144170
});
145171

@@ -164,7 +190,12 @@ describe('LetterRepository', () => {
164190
const pendingLetters = await letterRepository.getLettersByStatus('supplier1', 'PENDING');
165191
expect(pendingLetters.letters).toHaveLength(2);
166192

167-
await letterRepository.updateLetterStatus('supplier1', 'letter1', 'DELIVERED', undefined, undefined);
193+
const letterDto: LetterDto = {
194+
id: 'letter1',
195+
supplierId: 'supplier1',
196+
status: 'DELIVERED'
197+
};
198+
await letterRepository.updateLetterStatus(letterDto);
168199
const remainingLetters = await letterRepository.getLettersByStatus('supplier1', 'PENDING');
169200
expect(remainingLetters.letters).toHaveLength(1);
170201
expect(remainingLetters.letters[0].id).toBe('letter2');

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

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
import { Letter, LetterBase, LetterSchema, LetterSchemaBase } from './types';
1111
import { Logger } from 'pino';
1212
import { z } from 'zod';
13+
import { LetterDto } from '../../../lambdas/api-handler/src/contracts/letters';
1314

1415
export type PagingOptions = Partial<{
1516
exclusiveStartKey: Record<string, any>,
@@ -133,37 +134,37 @@ export class LetterRepository {
133134
}
134135
}
135136

136-
async updateLetterStatus(supplierId: string, letterId: string, status: Letter['status'], reasonCode: number | undefined, reasonText: string | undefined): Promise<Letter> {
137-
this.log.debug(`Updating letter ${letterId} to status ${status}`);
137+
async updateLetterStatus(letterToUpdate: LetterDto): Promise<Letter> {
138+
this.log.debug(`Updating letter ${letterToUpdate.id} to status ${letterToUpdate.status}`);
138139
let result: UpdateCommandOutput;
139140
try {
140141
let updateExpression = 'set #status = :status, updatedAt = :updatedAt, supplierStatus = :supplierStatus, #ttl = :ttl';
141142
let expressionAttributeValues = {
142-
':status': status,
143+
':status': letterToUpdate.status,
143144
':updatedAt': new Date().toISOString(),
144-
':supplierStatus': `${supplierId}#${status}`,
145+
':supplierStatus': `${letterToUpdate.supplierId}#${letterToUpdate.status}`,
145146
':ttl': Math.floor(Date.now() / 1000 + 60 * 60 * this.config.ttlHours),
146-
...(!reasonCode && {':reasonCode': reasonCode}),
147-
...(!reasonText && {':reasonText': reasonText})
147+
...(!letterToUpdate.reasonCode && {':reasonCode': letterToUpdate.reasonCode}),
148+
...(!letterToUpdate.reasonText && {':reasonText': letterToUpdate.reasonText})
148149
};
149150

150-
if (reasonCode)
151+
if (letterToUpdate.reasonCode)
151152
{
152153
updateExpression += ', reasonCode = :reasonCode';
153-
expressionAttributeValues[':reasonCode'] = reasonCode;
154+
expressionAttributeValues[':reasonCode'] = letterToUpdate.reasonCode;
154155
}
155156

156-
if (reasonText)
157+
if (letterToUpdate.reasonText)
157158
{
158159
updateExpression += ', reasonText = :reasonText';
159-
expressionAttributeValues[':reasonText'] = reasonText;
160+
expressionAttributeValues[':reasonText'] = letterToUpdate.reasonText;
160161
}
161162

162163
result = await this.ddbClient.send(new UpdateCommand({
163164
TableName: this.config.lettersTableName,
164165
Key: {
165-
supplierId: supplierId,
166-
id: letterId
166+
supplierId: letterToUpdate.supplierId,
167+
id: letterToUpdate.id
167168
},
168169
UpdateExpression: updateExpression,
169170
ConditionExpression: 'attribute_exists(id)', // Ensure letter exists
@@ -176,12 +177,12 @@ export class LetterRepository {
176177
}));
177178
} catch (error) {
178179
if (error instanceof Error && error.name === 'ConditionalCheckFailedException') {
179-
throw new Error(`Letter with id ${letterId} not found for supplier ${supplierId}`);
180+
throw new Error(`Letter with id ${letterToUpdate.id} not found for supplier ${letterToUpdate.supplierId}`);
180181
}
181182
throw error;
182183
}
183184

184-
this.log.debug(`Updated letter ${letterId} to status ${status}`);
185+
this.log.debug(`Updated letter ${letterToUpdate.id} to status ${letterToUpdate.status}`);
185186
return LetterSchema.parse(result.Attributes);
186187
}
187188

‎lambdas/api-handler/src/contracts/json-api.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { z } from 'zod';
2+
3+
// Single document wrapper
4+
export const makeDocumentSchema = <T extends z.ZodTypeAny>(resourceSchema: T) =>
5+
z.object({ data: resourceSchema }).strict();
6+
7+
// Collection document wrapper
8+
export const makeCollectionSchema = <T extends z.ZodTypeAny>(resourceSchema: T) =>
9+
z.object({ data: z.array(resourceSchema) }).strict();

‎lambdas/api-handler/src/contracts/letter-api.ts‎

Lines changed: 0 additions & 48 deletions
This file was deleted.

‎lambdas/api-handler/src/contracts/letters.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { z } from 'zod';
2+
import { makeCollectionSchema, makeDocumentSchema } from './json-api';
3+
4+
export type LetterDto = {
5+
id: string,
6+
status: LetterStatus,
7+
supplierId: string,
8+
specificationId?: string,
9+
groupId?: string,
10+
reasonCode?: number,
11+
reasonText?: string
12+
};
13+
14+
export const LetterStatusSchema = z.enum([
15+
'PENDING',
16+
'ACCEPTED',
17+
'REJECTED',
18+
'PRINTED',
19+
'ENCLOSED',
20+
'CANCELLED',
21+
'DISPATCHED',
22+
'FAILED',
23+
'RETURNED',
24+
'DESTROYED',
25+
'FORWARDED',
26+
'DELIVERED'
27+
]);
28+
29+
export const PatchLetterRequestResourceSchema = z.object({
30+
id: z.string(),
31+
type: z.literal('Letter'),
32+
attributes: z.object({
33+
status: LetterStatusSchema,
34+
reasonCode: z.number().optional(),
35+
reasonText: z.string().optional(),
36+
}).strict()
37+
}).strict();
38+
39+
export const PatchLetterResponseResourceSchema = z.object({
40+
id: z.string(),
41+
type: z.literal('Letter'),
42+
attributes: z.object({
43+
status: LetterStatusSchema,
44+
specificationId: z.string(),
45+
groupId: z.string().optional(),
46+
reasonCode: z.number().optional(),
47+
reasonText: z.string().optional(),
48+
}).strict()
49+
}).strict();
50+
51+
export const GetLettersResponseResourceSchema = z.object({
52+
id: z.string(),
53+
type: z.literal('Letter'),
54+
attributes: z.object({
55+
status: LetterStatusSchema,
56+
specificationId: z.string(),
57+
groupId: z.string().optional(),
58+
}).strict()
59+
}).strict();
60+
61+
export type LetterStatus = z.infer<typeof LetterStatusSchema>;
62+
63+
export const PatchLetterRequestSchema = makeDocumentSchema(PatchLetterRequestResourceSchema);
64+
export const PatchLetterResponseSchema = makeDocumentSchema(PatchLetterResponseResourceSchema);
65+
export const GetLettersResponseSchema = makeCollectionSchema(GetLettersResponseResourceSchema);
66+
67+
export type PatchLetterRequest = z.infer<typeof PatchLetterRequestSchema>;
68+
export type PatchLetterResponse = z.infer<typeof PatchLetterResponseSchema>;
69+
export type GetLettersResponse = z.infer<typeof GetLettersResponseSchema>;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ describe('API Lambda handler', () => {
6565
specificationId: "s1",
6666
groupId: 'g1',
6767
status: "PENDING",
68-
reasonCode: 123,
69-
reasonText: "Reason text"
68+
reasonCode: 123, // shouldn't be returned if present
69+
reasonText: "Reason text" // shouldn't be returned if present
7070
},
7171
]);
7272

0 commit comments

Comments
 (0)