CCM-22090: Recipient response message retrieval API - #1090
Conversation
|
This branch is work on a ticket in the NHS Digital APM JIRA Project. Here's a handy link to the ticket: CCM-22090 |
|
This branch is work on a ticket in the NHS Digital APM JIRA Project. Here's a handy link to the ticket: CCM-22090 |
|
This branch is work on a ticket in the NHS Digital APM JIRA Project. Here's a handy link to the ticket: CCM-22090 |
|
This branch is work on a ticket in the NHS Digital APM JIRA Project. Here's a handy link to the ticket: CCM-22090 |
|
This branch is work on a ticket in the NHS Digital APM JIRA Project. Here's a handy link to the ticket: CCM-22090 |
mjewildnhs
left a comment
mjewildnhs
left a comment
There was a problem hiding this comment.
I've paused reviewing until we get an answer on whether we should be following the JSON:API convention as per my review comment.
|
After a team discussion we decided that
|
|
One revision to the above - since JSON API mandates specific fields for the data wrapper, we can't extract |
|
This branch is work on a ticket in the NHS Digital APM JIRA Project. Here's a handy link to the ticket: CCM-22090 |
|
Migrated to JSON:API |
mjewildnhs
left a comment
mjewildnhs
left a comment
There was a problem hiding this comment.
Not finished yet but posting feedback I have.
|
This branch is work on a ticket in the NHS Digital APM JIRA Project. Here's a handy link to the ticket: CCM-22090 |
Requested changes have been addressed
| const tooManyResponsesMessageId = '00000000-0000-4000-8000-000000000500'; | ||
| const supportedContentTypes = ['application/json', 'application/vnd.api+json']; | ||
|
|
||
| export async function messageResponses(req, res, next) { |
There was a problem hiding this comment.
Follow on from one of my previous comments: #1090 (comment)
Looking at this again I've got this wrong.
I think the sandbox will use the proper proxy config so which will transform error responses based on the rules.
So we should return error responses in line with what our app actually does.
If we can get the deployment working and APIM access we can properly test this. I was testing against the sandbox directly and there isn't a way to locally deploy the full sandbox API consumers will see.
Let's leave this unresolved for now until we are happy with the error responses app-response returns and we can test the properly deployed sandbox.
There was a problem hiding this comment.
Addressed feedback - reverted to simple responses for the responses that are unconditionally overwritten, and preserved the ones that aren't.
| from lib.fixtures import * # NOSONAR | ||
|
|
||
|
|
||
| @pytest.mark.devtest |
There was a problem hiding this comment.
Its a bit of a shame we can't test this in other environments.
e.g. get_message runs its tests in all envs
However it can create messages to then get.
We don't have a way to create a response so I guess we are stuck with dev only test which I think is against the sandbox. This does risk things breaking in prod on APIM releases.
Might be worth discussion with @cgitim
|
This branch is work on a ticket in the NHS Digital APM JIRA Project. Here's a handy link to the ticket: CCM-22090 |
|
This branch is work on a ticket in the NHS Digital APM JIRA Project. Here's a handy link to the ticket: CCM-22090 |
52e5a23 to
2c4a7d5
Compare
|
This branch is work on a ticket in the NHS Digital APM JIRA Project. Here's a handy link to the ticket: CCM-22090 |
|
This branch is work on a ticket in the NHS Digital APM JIRA Project. Here's a handy link to the ticket: CCM-22090 |
|
This branch is work on a ticket in the NHS Digital APM JIRA Project. Here's a handy link to the ticket: CCM-22090 |
|
This branch is work on a ticket in the NHS Digital APM JIRA Project. Here's a handy link to the ticket: CCM-22090 |
|
This branch is work on a ticket in the NHS Digital APM JIRA Project. Here's a handy link to the ticket: CCM-22090 |
|
Happy with the changes. |
mark-r-bjss
left a comment
mark-r-bjss
left a comment
There was a problem hiding this comment.
Approved, in that I can't see any issues but it's very hard to follow what's going on with these XML files and we will be relying heavily on the testing to find any issues. Also worth ensuring that somebody with experience in this area reviews this.
|
This branch is work on a ticket in the NHS Digital APM JIRA Project. Here's a handy link to the ticket: CCM-22090 |
4 participants
Summary
Adds proxy configuration for new recipient response message retrieval API.
Reviews Required
Checklist