CCM-22090: Recipient response message retrieval API - #1090
rhyscoxnhs wants to merge 20 commits into
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
|
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 |
|
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 |
c71aca7 to
718b5eb
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 |
| app: app.locals.app_name, | ||
| msg: "startup", | ||
| server_port: server.address().port, | ||
| server_port: address ? address.port : null, |
There was a problem hiding this comment.
This fixes some flakes with the unit tests, without this they sporadically fail in CI
| ) | ||
|
|
||
|
|
||
| @pytest.mark.devtestonly |
There was a problem hiding this comment.
Seems a shame to only run these new tests against internal-dev. Any reason they can't?
There was a problem hiding this comment.
Just noticed Mike's comment above.
There was a problem hiding this comment.
In order for these tests to work we need to seed the environment with long lived test data. My feeling is that dev is the only environment where this is suitable.
| {% if ENVIRONMENT_TYPE == 'sandbox' %} | ||
| <Template>/api/v1/message-responses/{data.messageId}</Template> | ||
| {% else %} | ||
| <Template>/api/response/{data.messageId}</Template> | ||
| {% endif %} |
There was a problem hiding this comment.
I think it would be nice for the sandbox to be as similar to the real backend as possible. Is there anything stopping us from changing sandbox so it responds to the same path?
5 participants
Summary
Adds proxy configuration for new recipient response message retrieval API.
Reviews Required
Checklist