ARCPOC-1407#489
Conversation
| type: array | ||
| description: Array of entry IDs whose officials should be replaced. All must belong to the list in the path. | ||
| minItems: 1 | ||
| uniqueItems: true |
There was a problem hiding this comment.
Removing this means the OpenAPI schema no longer declares that entryIds must be unique. Did this need to be removed to avoid duplicates being hidden from the code?
There was a problem hiding this comment.
yes having it in means that duplicates are silently ignored, so can't then check for duplicates. Catch-22 do we silently ignore them, or flag it?
There was a problem hiding this comment.
I think flag so that consumers know. Maybe document that duplicates are rejected in the schema description as well.
|
Please could you add a test to BulkUpdateOfficialsValidatorTest for the duplicate entry case? |
There was a problem hiding this comment.
I see a new error has been added in the middle next to another error which begins with 'ENTRY'. I appreciate that it's nice to group them but this means other errors have been renumbered. I think it may be best to move the error to the end so that anyone using the old numbers isn't affected. Don't think anyone is but I believe best practice here is to append. We've been casual in the past but maybe better to be strict.
mustafaahmeddev
left a comment
mustafaahmeddev
left a comment
There was a problem hiding this comment.
Thanks for picking this up, I've suggested some changes.
2 participants
Jira link
https://tools.hmcts.net/jira/browse/ARCPOC-1407
Change description
Produce error when submitting duplicate EntryId's to POST: application-lists/{app list id}/entries/officials
Testing done
Unit Tests
Integration Tests
Security Vulnerability Assessment
CVE Suppression: Are there any CVEs present in the codebase (either newly introduced or pre-existing) that are being intentionally suppressed or ignored by this commit?
Checklist