difference voided and discontinued order

wyclif · July 1, 2016, 2:30pm

I created a ticket at TRUNK-4901, feel free to edit the description

darius · July 1, 2016, 5:17pm

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.

teleivo · July 1, 2016, 5:50pm

No not pressing, I can wait for Burke’s return

wyclif · July 1, 2016, 5:52pm

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

burke · July 6, 2016, 6:44pm

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.

burke · July 11, 2016, 5:35pm

@teleivo, we discussed

in today’s Design Forum (notes here). Here’s the outcome of that discussion:

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 isActive method.

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.

When the 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.

Recommendation

1. `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 isNotYetActivated method)

darius · July 11, 2016, 9:19pm

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.

teleivo · July 13, 2016, 8:01am

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.

teleivo · July 13, 2016, 3:20pm

I have implemented your proposed changes which are at

https://github.com/teleivo/openmrs-core/tree/TRUNK-4901

I still have some questions (additional ones to my above post), thats why I didnt create a pull request yet.

Questions

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 scheduled order.

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).

teleivo · July 14, 2016, 1:09pm

dear @burke and @darius,

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 !isStarted(checkDate)

github.com

openmrs/openmrs-core/blob/2.0.x/api/src/main/java/org/openmrs/Order.java#L465


	 * @should return false if the order is not yet activated as of the check date
	 * @should return false if the order was scheduled to start after the check date
	 * @should return true if the order was scheduled to start on the check date
	 * @should return true if the order was scheduled to start before the check date
	 * @should return true if the order is started and not scheduled
	 */
	public boolean isStarted(Date checkDate) {
		if (isVoided()) {
			return false;
		}
		if (checkDate == null) {
			checkDate = new Date();
		}
		if (getEffectiveStartDate() == null) {
			return false;
		}
		return !checkDate.before(getEffectiveStartDate());
	}
	
	/**
	 * Convenience method to determine if the order is discontinued as of the specified date

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.

darius · July 14, 2016, 1:51pm

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).

wyclif · July 14, 2016, 2:20pm

@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.

teleivo · July 14, 2016, 2:25pm

@darius and @wyclif thanks, just wanted to make sure! than please check out my pull request which implemented @burke’s recommended solution.

wyclif · July 14, 2016, 3:22pm

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

teleivo · July 14, 2016, 3:24pm

already done, thats why there is a PR

difference voided and discontinued order

Background (feel free to skip if you like)

Recommendation

1. isActive should revert to prior behavior:

2. The logic for the new public method isActivated(checkDate) should be:

1. `isActive` should revert to prior behavior:

2. The logic for the new public method `isActivated(checkDate)` should be: