I created a ticket at TRUNK-4901, feel free to edit the description
I disagree, because the current behavior of isActive is very different than what the behavior of isActivated would be. (Hence the verbose names I have proposed above.)
@teleivo, since @burke is the design lead for Order Entry, I’d like to get his approval on the method renaming. (He’s on vacation now, and is back on Tuesday, I think. Do you have a pressing need to do this earlier?)
You can proceed with fixing the “cannot discontinue” bug that @wyclif created a ticket for.
No not pressing, I can wait for Burke’s return
The bug doesn’t exist in older releases, the behavior was correct in 1.10 and 1.11 but got broken in 2.0.x so we should wait to release 2.0 and get this fix in.
In 1.11.x, isActive() checked if isActivated && not expired && not stopped as of a given date, so for backwards compatibility for code that depended on this logic and minimizing more refactoring in our code base I think we need to restore that behavior and rename the method to something like isActivatedAndStillActive(), then we add a new method named something like isStartedAndStillActive() that checks if started && not expired && not stopped
From a clinical standpoint, “Active Orders” is a very common concept. Active Orders represent all orders that have been placed on a patient’s chart that have not been discontinued or expired. This would include any orders that are scheduled in the future. For example, all these would be considered active orders:
- aspirin 81 mg - one tab by mouth daily.
- vitamin D - take 50,000 units by mouth each Sunday.
- chest x-ray - for shortness of breath tomorrow morning.
- hematology referral - for unresponsive anemia. patient scheduled for 12-Aug-2016 at 9am.
“Activated” means that the order could be carried out. It distinguishes from a “pending” or “draft” order (which we don’t currently support; with the current design, all orders are activated when saved).
“Active” represents any activated order that has not been discontinued or expired.
I believe we have used “Started” to distinguish between active orders scheduled for the future versus already underway.
We will not always be guaranteed a clean signal of when orders are being applied to the patient or when they are completed. We will have these details in the happy path, where every aspect of the order’s life cycle are performed within the same system; however, many orders are carried out across multiple systems & workflows. For example, an order for a lab test may be printed and carried out through a separate system without a reliable signal when/if it is completed, there may be no signal when/if a referral is completed, etc. In these cases (where we don’t have reliable closed loops), we use business rules to decide when orders should be auto-expired. A crude approach that covers 80% or more cases is prescriptions without an computer-understandable duration don’t auto-expire (i.e., remain active until discontinued) and tests auto-expire after 24 hours.
@teleivo, we discussed
Background (feel free to skip if you like)
In the transition to Platform 2.0, the
isFuture method, which was considering the order’s activation date, was removed and we mistakenly switched from using it to using
isStarted inside the
isFuture was a confusing name for a method determining whether or not an order has been activated. An “activated” order represents a request that may have been sent to a receiving system. Currently, all orders stored in the API are considered activated (their
dateActivated defaults to now). In future version of the Platform, this may change. For example, we may allow for draft & pending orders or orders needing cosignature that could be stored via the API before being activated.
isFuture was removed, the
isActive method mistakenly replaced it with checking
isStarted. Any activated orders (currently, all orders written to the API) should be considered active unless they have been discontinued or have expired. The
isStarted method checks the start date for scheduled orders and does not apply to an order’s active status. To clarify, “active” does not mean that the order is being actively carried out; rather, it means that the order has been activated and has not yet been countermanded or run its course (expired). This means scheduled orders written to start in the future are still considered “active” before their start date, since these “active” order can be discontinued or revised prior to the start date. Thus,
isStarted has no bearing on whether or not an order is active.
isActive should revert to prior behavior:
Any activated order is active unless it has expired or been discontinued. This means future orders (orders scheduled to start in the future) should return
isActive == true. For example, the last line will look something like:
return isActivated(checkDate) && !isDiscontinued(checkDate) && !isExpired(checkDate);
2. The logic for the new public method
isActivated(checkDate) should be:
Returns true for an order that has been activated (not in a draft or pending state). Currently, all orders stored in the API are considered activated (draft or pending orders are not yet implemented).
return dateActivated != null && (dateActivated.before(checkDate) || dateActivated.equals(checkDate));
(note that the new public method
isActivated will effectively be the inverse of the previous
isFuture method, which was functioning as a poorly named
Btw, you should use the OpenmrsUtil.compare(Date, Date) method instead of doing this specific code, because it is written to handle the case where hibernate gives you back a Timestamp, which isn’t properly comparable with a Date.
this confusing me a bit.
the behavior: scheduled order can be discontinued or revised prior to the start date makes sense to me but
a scheduled order is still considered “active” even after it is started as of your definition above in isActive since this does not take into account a scheduled orders start date. makes sense to me that an order is active even if started.
so what I am confused about is
“active” before their start date
should it be not active after their start date (I assume not unless discontinued or expired.)
The reason why this is important is that currently even if I make above change you can discontinue an order if its not discontinued, not expired but started (regardless of if its a scheduled order or not). So if you also want to prevent discontinuation of scheduled orders that are started, this would need changes in the OrderService.
I have implemented your proposed changes which are at
I still have some questions (additional ones to my above post), thats why I didnt create a pull request yet.
- Should we prevent scheduled orders from being stopped if they are started at the discontinueDate? I added a clause in OrderServiceImpl.stoppedOrder() which prevents that, not sure if I should. This is related to
This means scheduled orders written to start in the future are still considered “active” before their start date, since these “active” order can be discontinued or revised prior to the start date.
2 . I added tests in the OrderService to ensure we can discontinue orders scheduled for the future and also added on for revising an order scheduled for the future. Here I encountered that
When revising an order scheduled for the future the original order now under revisedOrder.previousOrder is still considered active. This is due to the fact that
revisedOrder.previousOrder.isDiscontinued(new Date()) does return false when the order is scheduled for the future and true only in case I revise a non
See tests ** saveOrder_revisingNonScheduledOrderMakesPreviousOrderNotActiveSinceDiscontinued ** saveOrder_reviseScheduledOrderDoesNotAffectWhetherThePreviousOrderIsActive which exemplify the behavior I mean.
Please have a look at my code (ignore that I made two methods public and part of the OrderService interface, that was just for testing purposes).
I created a pull request, please review my changes.
Found the reason for the issue in above post. I showed you that if you revise an order scheduled for the future its original order is not considered discontinued as far as
isDiscontinued(new Date()) is concerned.
The reason is that the commit
which introduced this bug in isActive also affects isDiscontinued and isExpired, since isFuture was simply replaced by isStarted/!isStarted
In above case the expression
for an order scheduled for the future this expression is true and thus
isDiscontinued(new Date()) == false
Solution I replaced the former isFuture(checkDate) by the new isActivated(checkDate)
and added some more tests.
Now when revising an order scheduled for the future isActive() returns false like it does for non scheduled orders.
This matches the behavior of
public List<Order> getActiveOrders(Patient patient, List<OrderType> orderTypes, CareSetting careSetting, Date asOfDate)
which doesn’t return the original order in both cases (scheduled, non-scheduled)
Remaining Question Should it be possible to discontinue/revise scheduled orders when they are started? Because currently it is.
If not I could create another PR which prevents this (just a clause in the private stopOrder method)
Just let me know.
I haven’t had time to get into all the details of your last few posts, but I’ll try to answer just this:
By “scheduled orders when they are started” do you mean, for example, an order that was written 2 days ago with “scheduled start date” of yesterday?
You should be able to discontinue any active order (i.e. one that was activated, and has not already been discontinued or expired). Therefore yes, you should be able to discontinue an order either before or after its scheduled start date (as long as it’s active).
I think the same holds true for revising. You can revise any active order (whether it’s scheduled for tomorrow, or has been carried out since yesterday).
@teleivo you might be over thinking this, the proposed solution didn’t mention anything to do with scheduled date, isActivated() doesn’t take into consideration scheduled date so it shouldn’t matter.
The only changes you should be making is to add the new isActivated() methods and updating stopOrder() to use isActivated(Date) and not isStarted(Date), and of course adding/updating tests
already done, thats why there is a PR