Uploaded image for project: 'OpenMRS Core'
  1. OpenMRS Core
  2. TRUNK-6006

OrderService prevents saving Order Frequencies

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Code Review (Post-Commit)
    • Priority: TBD
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: Core 2.4.1, Core 2.3.4, Core 2.5.0
    • Component/s: None
    • Labels:
      None
    • Complexity:
      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 Mamlin 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 Mamlin can you please weigh in on this, given you were opinionated on the original design?

      Mark Goodrich and Dimitri R 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.

       

        Gliffy Diagrams

          Attachments

            Activity

              People

              Assignee:
              saurabh Saurabh Kumar
              Reporter:
              mseaton Mike Seaton
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated: