Feature/ccm 12952 publish mi events #275
Conversation
nhsd-david-wass
force-pushed
the
feature/CCM-12952-Publish-MI-Events
branch
7 times, most recently
from
6f221b9 to
c9b1df0
Compare
nhsd-david-wass
force-pushed
the
feature/CCM-12952-Publish-MI-Events
branch
from
6d0cb00 to
15c58d6
Compare
nhsd-david-wass
force-pushed
the
feature/CCM-12952-Publish-MI-Events
branch
2 times, most recently
from
9fcbc8e to
598d07a
Compare
|
I seem to remember you mentioning moving both the existing letter transformer and your new MI transformer into a |
|
|
||
| function extractMIData(record: DynamoDBRecord): MI { | ||
| const newImage = record.dynamodb?.NewImage!; | ||
| return MISchema.parse(unmarshall(newImage as any)); |
There was a problem hiding this comment.
The reason for doing an MISchema.parse here is as an extra validation check, so it's probably worth having a test case where it's not valid (see it("does not publish invalid letter data"... in letter=updates-transformer-tests.ts
| } | ||
|
|
||
| function extractMIData(record: DynamoDBRecord): MI { | ||
| const newImage = record.dynamodb?.NewImage!; |
There was a problem hiding this comment.
I suspect NewImage would be undefined if a record was deleted from the MI table, causing this code to blow up. But perhaps we don't anticipate that ever happening?
There was a problem hiding this comment.
We don't support that operation - only POST for creation of new data
It's maybe worth adding an INSERT filter for the kinesis event with a log out for unexpected event types for a bit of defense?
| ); | ||
| }); | ||
|
|
||
| it("does not publish invalid letter data", async () => { |
There was a problem hiding this comment.
| it("does not publish invalid letter data", async () => { | |
| it("does not publish invalid MI data", async () => { |
nhsd-david-wass
force-pushed
the
feature/CCM-12952-Publish-MI-Events
branch
3 times, most recently
from
9e44d73 to
52e64d9
Compare
nhsd-david-wass
force-pushed
the
feature/CCM-12952-Publish-MI-Events
branch
from
52e64d9 to
cefd277
Compare
masl2
left a comment
masl2
left a comment
There was a problem hiding this comment.
Think we need a guard on the kinesis event type
| function generateInsertRecord(newMI: MI): DynamoDBRecord { | ||
| const newImage = Object.fromEntries( | ||
| Object.entries(newMI).map(([key, value]) => [key, toAttr(value)]), | ||
| ); | ||
| return { | ||
| eventName: "INSERT", | ||
| dynamodb: { NewImage: newImage }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Thinking further we probably should verify the behaviour for MODIFY/DELETE events.
While the API doesn't generate them, if we have to do any form of data correction, we could produce errors.
Probably just needs to drop them with a log
| const cloudEvents: MISubmittedEvent[] = streamEvent.Records.map((record) => | ||
| extractPayload(record, deps), | ||
| ) | ||
| .map((element) => extractMIData(element)) | ||
| .map((payload) => mapMIToCloudEvent(payload, deps)); |
There was a problem hiding this comment.
I think this does need a guard so it only maps those INSERT events
While standard operation shouldn't produce anything else, a DR scenario or data fix might and the downstream code makes the assumption that it will receive INSERT data (if my read is correct?)
4 participants
Description
Context
Type of changes
Checklist
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.