Details
-
Story
-
Status: Closed
-
TBD
-
Resolution: Fixed
-
None
-
None
-
None
-
Low
Description
When the Order Frequency object was added to core, in TRUNK-4188, there was validation logic intentionally included to prohibit editing an existing Order Frequency that had any orders associated with it.
See comments in this ticket: https://issues.openmrs.org/browse/TRUNK-4188?focusedCommentId=203343&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-203343
This was implemented in the OrderServiceImpl as such (and this is the current implementation from the 2.3.x branch of OpenMRS core):
/** * @see org.openmrs.api.OrderService#saveOrderFrequency(org.openmrs.OrderFrequency) */ @Override public OrderFrequency saveOrderFrequency(OrderFrequency orderFrequency) throws APIException { if (orderFrequency.getOrderFrequencyId() != null && dao.isOrderFrequencyInUse(orderFrequency)) { throw new CannotUpdateObjectInUseException("Order.frequency.cannot.edit"); } return dao.saveOrderFrequency(orderFrequency); }
This has the ultimate effect of not allowing any kind of saving of Order Frequencies at all - whether they are dirty or not (any properties have actually changed) - and regardless of the nature of these changes.
Although the intent of this validation seem clear - to ensure that modifications at a later date are unable to change the meaning of historical data (see comments from burke on aforementioned ticket) - this is inconsistent with pretty much all other API-level behavior in OpenMRS, and most downstream tooling is unable to cope with this.
Specifically, tools like Initializer and MetadataDeploy will strive to save one's configuration that is loaded in, and trust the API to know whether data has been modified incorrectly or not.
For no other domain do we do this. One can change concepts at will, even though Obs refer to them. One can change drugs at will, even though Orders refer to them. One can change encounter types, programs, workflows, states, and pretty much any other metadata in the system as suits their needs, without this level of validation.
We should remove this validation check, or at the very least provide a mechanism that can be used to bypass it where required.
burke can you please weigh in on this, given you were opinionated on the original design?
mogoodrich and mksd FYI, as this relates to the bug I filed with Initializer earlier today, and my hope is that this would resolve it without any changes needed to Initializer.