Fix for Entity Controller setup causing build fails. #1509
Conversation
…PR1507) - Fixed EntityControllerTestIT to incorporate unique data per test using test utility helpers. - Removed hard coded static values - Fixed teardown to ensure all test entities cleaned up after testsing. - Added check in EntityController to fix unused documented query param errors
|
@rma-bryson and/or @adamkorynta this PR is ready for a review |
|
|
||
| if (!officeId.equalsIgnoreCase(entity.getId().getOfficeId())) { | ||
| ctx.status(HttpServletResponse.SC_BAD_REQUEST); | ||
| ctx.result("Office ID in query parameter must match the Office ID in the request body."); |
There was a problem hiding this comment.
don't we have "global" entities under the cwms office?
There was a problem hiding this comment.
After discussion with Bryson, we decided the office query parameter is not needed in update method and will be validated in the body. I removed unnecessary office id checks in the update method.
|
|
||
| if (!entityId.equalsIgnoreCase(entity.getId().getName())) { | ||
| ctx.status(HttpServletResponse.SC_NOT_FOUND); | ||
| ctx.result("Entity not found for the given entity-id."); |
There was a problem hiding this comment.
shouldn't this be a similar message and error response code to the mismatched office-id? something like:
""Entity ID in path parameter must match the Entity ID in the request body.""
There was a problem hiding this comment.
updated message to be more consistent.
There was a problem hiding this comment.
It doesn't look like the change Bryson asked for made it to github.
- Removed office-id query param and redundant checks - Added missing entity-id validation test - Added invalid office authorization test - Improved variable naming - Fixed failing .replace() logic
|
Refactored entity update controller and tests.
|
There was a problem hiding this comment.
I like the naming scheme you've chosen here. It's very explicit as to what's going on.
3 participants
Refactored and cleaned Entity Controller to address clean runner build fail PR #1507.