Conversation
nhsd-rebecca-flynn
force-pushed
the
feature/CDAPI-111
branch
from
f68ef5f to
9c6959f
Compare
…narios to PDM endpoint
nhsd-rebecca-flynn
force-pushed
the
feature/CDAPI-111
branch
2 times, most recently
from
b70406c to
dbe777c
Compare
nhsd-rebecca-flynn
force-pushed
the
feature/CDAPI-111
branch
from
87ee044 to
b5aafa2
Compare
nhsd-jack-wainwright
left a comment
nhsd-jack-wainwright
left a comment
There was a problem hiding this comment.
LGTM 👍 , just a couple minor suggestions around the get event endpoint.
mocks/src/mns_mock/handler.py
Outdated
| def _get_event_from_table(subject: str) -> EventItem: | ||
| expression = Attr("subject").eq(subject) | ||
| item = storage_helper.find_items(expression)[0] | ||
| return cast("EventItem", item) |
There was a problem hiding this comment.
Rather than just casting here, I wonder if its worth checking the type and throwing an error if the type is incorrect
| return cast("EventItem", item) | |
| if not isinstance(item, EventItem): | |
| raise ValueError(f"Event found but was not correct type. {item}") |
There was a problem hiding this comment.
Can't include this instance off check here due to EventItem being a TypedDict, meaning its type cannot be checked at runtime.
mocks/src/mns_mock/handler.py
Outdated
|
|
||
|
|
||
| @mns_routes.get("/mns/mock/event/<subject>") | ||
| def get_event(subject: str) -> Response[str]: |
There was a problem hiding this comment.
Rather than being a get endpoint, I wonder if this endpoint would make more sense as a search endpoint that accepted a query param. That way multiple events could be returned as its possible that multiple events have been created for the same subject.
Perhaps the endpoint could look something like /mns/mock/event?subject=<subject>
There was a problem hiding this comment.
Updated to instead be a find_events endpoint as suggested.
|
|
Deployment Complete
|
3 participants
Description
Adds MNS Mock Implementation with relevant unit tests
Context
Similar changes to the PDM Mock Implementation.
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.