TRUNK-5952:Order validation is bypassed if certain fields are set from the OrderContext #3757
Conversation
HerbertYiga
changed the title
|
|
||
| OrderContext orderContext = new OrderContext(); | ||
| orderContext.setCareSetting(orderService.getCareSetting(1)); | ||
| order.setQuantity(42.0); |
There was a problem hiding this comment.
hi @mseaton so on introducing ValidateUtil.validate(order), quantity becomes required !
HerbertYiga
force-pushed
the
TRUNK-5952
branch
4 times, most recently
from
3c1ad26
to
57ad0ab
Compare
|
Hi @mseaton a gentle request to checkout this, thanks |
|
@HerbertYiga my impression is this PR as is seems fine and it likely solves some of the issues present in the ticket, and is a good addition. It does not solve the entire ticket however. It solves scenario 2 that was listed (which is that an invalid order can get saved), but it does not solve scenario 1 (which is that a validation error is thrown when it should not be). I'm fine if we keep this ticket just about adding in the extra validation to deal with scenario 2, and then creating a separate ticket for scenario 1, if that makes this easier to manage. @dkayiwa ? |
thnaks @mseaton well let me go on to look through scenario one again and i make a fix on this same pr |
Makes perfect sense! |
thanks @dkayiwa does this mean i can go on to make a separate ticket to solve scenario one? |
|
@HerbertYiga how about raising a second pull request for the other scenario but using the same ticket? |
@dkayiwa thanks let me do this! |
| @@ -333,9 +334,9 @@ private Order saveOrderInternal(Order order, OrderContext orderContext) { | |||
| cal.set(Calendar.MILLISECOND, 0); | |||
| order.setAutoExpireDate(cal.getTime()); | |||
| } | |||
| ValidateUtil.validate(order); | |||
There was a problem hiding this comment.
Can you explain why you put this validate method here (inside an if-then block), rather that directly before the dao.saveOrder(order) method, that will get invoked in every case?
There was a problem hiding this comment.
Can you explain why you put this validate method here (inside an if-then block), rather that directly before the dao.saveOrder(order) method, that will get invoked in every case?
thanks @mseaton ,just a quick reply to this,i first added ValidateUtil.validate(order) before the dao.saveOrder(order) method, and it was breaking things,
There was a problem hiding this comment.
That sounds like something worth investigating. How was it breaking things?
There was a problem hiding this comment.
That sounds like something worth investigating. How was it breaking things?
i am going to re run the change and drop here the logs
There was a problem hiding this comment.
That sounds like something worth investigating. How was it breaking things?
hi @mseaton here are the full logs on putting the validate method directly before the dao.saveOrder(order),
https://pastebin.com/a8LCW0LK
There was a problem hiding this comment.
Thanks @HerbertYiga - I see the errors. Have you investigated the cause?
There was a problem hiding this comment.
I see the errors. Have you investigated the cause?
@mseaton well i see the cause of the error is this line -> failed to validate with reason: dateActivated: Date activated cannot be before that of the associated encounter and let me work on the fix
…m the OrderContext
|
moving this conversation to #3812 |
Ticket Id
https://issues.openmrs.org/browse/TRUNK-5952
Description
Order validation should not be bypassed if certain fields are set from the OrderContext (STILL IN PROGRESS)