OBPIH-3759 Make order adjustment description required #2336
Conversation
| @@ -286,7 +286,7 @@ class OrderController { | |||
| orderAdjustment.orderItem.removeFromOrderAdjustments(orderAdjustment) | |||
| } | |||
| orderAdjustment.properties = params | |||
| if (!orderAdjustment.hasErrors() && orderAdjustment.save(flush: true)) { | |||
| if (orderAdjustment.validate() && !orderAdjustment.hasErrors() && orderAdjustment.save(flush: true)) { | |||
There was a problem hiding this comment.
You shouldn't need validate and hasErrors. You can remove the && !orderAdjustment.hasErrors() as validate and save should return false if there are validation errors. In fact you can probably remove all of this except orderAdjustment.save() as that should be enough to trigger the error handling.
There was a problem hiding this comment.
@awalkowiak This is actually probably a case where the change doesn't justify holding up testing since the additional code is basically a no-op. But this should definitely be addressed in our coding conventions. In this case, we'd want to move persistence logic to a service and find a consistent pattern for validation and saving domain objects.
The binding in line 288 should cause a validate() to occur, so if that's not happening then something weird is happening here. I'm ok if you merge this but can you investigate this further and let me know why the validate was necessary?
No description provided.