openmrs · Feb 14, 2014 · Feb 15, 2014 · Feb 16, 2014
diff --git a/api/src/main/java/org/openmrs/DrugOrder.java b/api/src/main/java/org/openmrs/DrugOrder.java
@@ -438,6 +438,41 @@ public Order cloneForDiscontinuing() {
 		return newOrder;
 	}
 
+	/**
+	 * Creates a DrugOrder for revision from this order, sets the previousOrder, action field and other drug order fields.
+	 *
+	 * @return the newly created order
+	 * @since 1.10
+	 * @should set all the relevant fields
+	 */
+	@Override
+	public Order cloneForRevision() {
+		DrugOrder newOrder = new DrugOrder();
+		newOrder.setCareSetting(this.getCareSetting());
+		newOrder.setConcept(this.getConcept());
+		newOrder.setAction(Action.REVISE);
+		newOrder.setPreviousOrder(this);
+		newOrder.setPatient(this.getPatient());
+		newOrder.setInstructions(this.getInstructions());
+		newOrder.setCommentToFulfiller(this.getCommentToFulfiller());
+		newOrder.setOrderReason(this.getOrderReason());
+		newOrder.setOrderReasonNonCoded(this.getOrderReasonNonCoded());
+		newOrder.setDose(this.getDose());
+		newOrder.setDoseUnits(this.getDoseUnits());
+		newOrder.setFrequency(this.getFrequency());
+		newOrder.setAsNeeded(this.getAsNeeded());
+		newOrder.setAsNeededCondition(this.getAsNeededCondition());
+		newOrder.setQuantity(this.getQuantity());
+		newOrder.setQuantityUnits(this.getQuantityUnits());
+		newOrder.setDrug(this.getDrug());
+		newOrder.setDosingType(this.getDosingType());
+		newOrder.setDosingInstructions(this.getDosingInstructions());
+		newOrder.setDuration(this.getDuration());
+		newOrder.setDurationUnits(this.getDurationUnits());
+		newOrder.setRoute(this.getRoute());
+		return newOrder;
+	}
+
 	public String toString() {
 		return "DrugOrder(" + getDose() + getDoseUnits() + " of " + (getDrug() != null ? getDrug().getName() : "[no drug]")
 		        + " from " + getStartDate() + " to " + (isDiscontinuedRightNow() ? getDateStopped() : getAutoExpireDate())

diff --git a/api/src/main/java/org/openmrs/Order.java b/api/src/main/java/org/openmrs/Order.java
@@ -155,13 +155,6 @@ protected Order copyHelper(Order target) {
 		return target;
 	}
 
-	/**
-	 * true/false whether or not this is a drug order overridden in extending class drugOrders.
-	 */
-	public boolean isDrugOrder() {
-		return false;
-	}
-
 	// Property accessors
 
 	/**
@@ -555,4 +548,25 @@ public Order cloneForDiscontinuing() {
 
 		return newOrder;
 	}
+
+	/**
+	 * Creates an order for revision from this order, sets the previousOrder and action field.
+	 *
+	 * @return the newly created order
+	 * @since 1.10
+	 * @should set all the relevant fields
+	 */
+	public Order cloneForRevision() {
+		Order newOrder = new Order();
+		newOrder.setCareSetting(this.getCareSetting());
+		newOrder.setConcept(this.getConcept());
+		newOrder.setAction(Action.REVISE);
+		newOrder.setPreviousOrder(this);
+		newOrder.setPatient(this.getPatient());
+
+		newOrder.setInstructions(this.getInstructions());
+		newOrder.setUrgency(this.getUrgency());
+		newOrder.setCommentToFulfiller(this.getCommentToFulfiller());
+		return newOrder;
+	}
 }
diff --git a/api/src/main/java/org/openmrs/TestOrder.java b/api/src/main/java/org/openmrs/TestOrder.java
@@ -149,4 +149,29 @@ public Integer getNumberOfRepeats() {
 	public void setNumberOfRepeats(Integer numberOfRepeats) {
 		this.numberOfRepeats = numberOfRepeats;
 	}
+
+	/**
+	 * Creates a TestOrder for revision from this order, sets the previousOrder, action field and other test order fields.
+	 *
+	 * @return the newly created order
+	 * @since 1.10
+	 * @should set all the relevant fields
+	 */
+	@Override
+	public Order cloneForRevision() {
+		TestOrder newOrder = new TestOrder();
+		newOrder.setCareSetting(this.getCareSetting());
+		newOrder.setConcept(this.getConcept());
+		newOrder.setAction(Action.REVISE);
+		newOrder.setPreviousOrder(this);
+		newOrder.setPatient(this.getPatient());
+
+		newOrder.setSpecimenSource(getSpecimenSource());
+		newOrder.setLaterality(getLaterality());
+		newOrder.setClinicalHistory(getClinicalHistory());
+		newOrder.setFrequency(getFrequency());
+		newOrder.setNumberOfRepeats(getNumberOfRepeats());
+		return newOrder;
+
+	}
 }
diff --git a/api/src/main/java/org/openmrs/api/OrderService.java b/api/src/main/java/org/openmrs/api/OrderService.java
@@ -47,13 +47,17 @@ public interface OrderService extends OpenmrsService {
 	 * @return the Order that was saved
 	 * @throws APIException
 	 * @should not save order if order doesnt validate
-	 * @should save discontinued reason non coded
 	 * @should discontinue existing active order if new order being saved with action to discontinue
 	 * @should pass if the existing drug order matches the concept and drug of the DC order
 	 * @should fail if the existing drug order matches the concept and not drug of the DC order
 	 * @should discontinue previousOrder if it is not already discontinued
 	 * @should fail if concept in previous order does not match this concept
 	 * @should not allow editing an existing order
+	 * @should not allow revising a voided order
+	 * @should not allow revising a stopped order
+	 * @should not allow revising an expired order
+	 * @should not allow revising an order with no previous order
+	 * @should save a revised order
 	 */
 	@Authorized( { PrivilegeConstants.EDIT_ORDERS, PrivilegeConstants.ADD_ORDERS })
 	public Order saveOrder(Order order) throws APIException;

diff --git a/api/src/main/java/org/openmrs/api/impl/OrderServiceImpl.java b/api/src/main/java/org/openmrs/api/impl/OrderServiceImpl.java
@@ -34,7 +34,6 @@
 import org.openmrs.api.OrderService;
 import org.openmrs.api.context.Context;
 import org.openmrs.api.db.OrderDAO;
-import org.openmrs.order.OrderUtil;
 import org.openmrs.util.OpenmrsUtil;
 import org.springframework.transaction.annotation.Propagation;
 import org.springframework.transaction.annotation.Transactional;
@@ -72,10 +71,18 @@ public void setOrderDAO(OrderDAO dao) {
 	 */
 	public Order saveOrder(Order order) throws APIException {
 		if (order.getOrderId() != null) {
-			throw new APIException("Cannot edit an existing order, see OrderService.reviseOrder");
+			throw new APIException("Cannot edit an existing order, you need to revise it instead");
 		}
 
-		discontinueExistingOrdersIfNecessary(order);
+		if (Order.Action.REVISE.equals(order.getAction())) {
+			Order previousOrder = order.getPreviousOrder();
+			if (previousOrder == null) {
+				throw new APIException("Previous Order is required for a revised order");
+			}
+			stopOrder(previousOrder, null);
+		} else if (Order.Action.DISCONTINUE.equals(order.getAction())) {
+			discontinueExistingOrdersIfNecessary(order);
+		}
 
 		return saveOrderInternal(order);
 	}
@@ -117,16 +124,14 @@ private void discontinueExistingOrdersIfNecessary(Order order) {
 			if (!previousOrder.getConcept().equals(order.getConcept())) {
 				throw new APIException("Concept of previous order and this order should be the same");
 			}
-			if (OrderUtil.isOrderActive(previousOrder, null)) {
-				markAsDiscontinued(previousOrder, order.getStartDate());
-				saveOrderInternal(previousOrder);
-			}
+			stopOrder(previousOrder, order.getStartDate());
 			return;
 		}
 
 		//Mark first order found corresponding to this DC order as discontinued.
 		List<? extends Order> orders = getActiveOrders(order.getPatient(), order.getClass(), order.getCareSetting(), null);
-		boolean isDrugOrderAndHasADrug = order.isDrugOrder() && ((DrugOrder) order).getDrug() != null;
+		boolean isDrugOrderAndHasADrug = DrugOrder.class.isAssignableFrom(order.getClass())
+		        && ((DrugOrder) order).getDrug() != null;
 		for (Order activeOrder : orders) {
 			boolean shouldMarkAsDiscontinued = false;
 			//For drug orders, the drug must match if the order has a drug
@@ -142,8 +147,7 @@ private void discontinueExistingOrdersIfNecessary(Order order) {
 
 			if (shouldMarkAsDiscontinued) {
 				order.setPreviousOrder(activeOrder);
-				markAsDiscontinued(activeOrder, order.getStartDate());
-				saveOrderInternal(activeOrder);
+				stopOrder(activeOrder, order.getStartDate());
 				return;
 			}
 		}
@@ -397,8 +401,7 @@ public List<OrderFrequency> getOrderFrequencies(boolean includeRetired) {
 	 */
 	@Override
 	public Order discontinueOrder(Order orderToDiscontinue, Concept reasonCoded, Date discontinueDate) throws Exception {
-		markAsDiscontinued(orderToDiscontinue, discontinueDate);
-
+		stopOrder(orderToDiscontinue, discontinueDate);
 		Order newOrder = orderToDiscontinue.cloneForDiscontinuing();
 		newOrder.setOrderReason(reasonCoded);
 
@@ -410,8 +413,7 @@ public Order discontinueOrder(Order orderToDiscontinue, Concept reasonCoded, Dat
 	 */
 	@Override
 	public Order discontinueOrder(Order orderToDiscontinue, String reasonNonCoded, Date discontinueDate) throws Exception {
-		markAsDiscontinued(orderToDiscontinue, discontinueDate);
-
+		stopOrder(orderToDiscontinue, discontinueDate);
 		Order newOrder = orderToDiscontinue.cloneForDiscontinuing();
 		newOrder.setOrderReasonNonCoded(reasonNonCoded);
 
@@ -422,23 +424,24 @@ public Order discontinueOrder(Order orderToDiscontinue, String reasonNonCoded, D
 	 * Make necessary checks, set necessary fields for discontinuing <code>orderToDiscontinue</code>
 	 * and save.
 	 * 
-	 * @param orderToDiscontinue
+	 * @param orderToStop
 	 * @param discontinueDate
 	 */
-	private void markAsDiscontinued(Order orderToDiscontinue, Date discontinueDate) {
+	private void stopOrder(Order orderToStop, Date discontinueDate) {
 		if (discontinueDate == null) {
 			discontinueDate = new Date();
 		}
 		if (discontinueDate.after(new Date())) {
 			throw new IllegalArgumentException("Discontinue date cannot be in the future");
 		}
-		if (!orderToDiscontinue.isCurrent()) {
+		if (!orderToStop.isCurrent()) {
 			throw new APIException("Cannot discontinue an order that is already stopped, expired or voided");
 		}
-		if (orderToDiscontinue.getAction().equals(Order.Action.DISCONTINUE)) {
+		if (orderToStop.getAction().equals(Order.Action.DISCONTINUE)) {
 			throw new APIException("An order with action " + Order.Action.DISCONTINUE + " cannot be discontinued.");
 		}
 
-		orderToDiscontinue.setDateStopped(discontinueDate);
+		orderToStop.setDateStopped(discontinueDate);
+		saveOrderInternal(orderToStop);
 	}
 }
diff --git a/api/src/test/java/org/openmrs/api/OrderServiceTest.java b/api/src/test/java/org/openmrs/api/OrderServiceTest.java
@@ -15,6 +15,7 @@
 
 import static org.hamcrest.Matchers.hasItems;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
@@ -30,7 +31,9 @@
 
 import org.junit.Assert;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.openmrs.CareSetting;
 import org.openmrs.Concept;
 import org.openmrs.Drug;
@@ -52,10 +55,15 @@
  */
 public class OrderServiceTest extends BaseContextSensitiveTest {
 
+	private ConceptService conceptService;
+
 	private OrderService orderService;
 
 	private PatientService patientService;
 
+	@Rule
+	public ExpectedException expectedException = ExpectedException.none();
+
 	@Before
 	public void setup() {
 		if (orderService == null) {
@@ -64,6 +72,10 @@ public void setup() {
 		if (patientService == null) {
 			patientService = Context.getPatientService();
 		}
+
+		if (conceptService == null) {
+			conceptService = Context.getConceptService();
+		}
 	}
 
 	/**
@@ -264,8 +276,7 @@ public void getOrderHistoryByOrderNumber_shouldReturnAllOrderHistoryForGivenOrde
 	 */
 	@Test
 	public void getOrderFrequency_shouldReturnTheOrderFrequencyThatMatchesTheSpecifiedId() throws Exception {
-		assertEquals("28090760-7c38-11e3-baa7-0800200c9a66", Context.getOrderService().getOrderFrequency(1)
-		                .getUuid());
+		assertEquals("28090760-7c38-11e3-baa7-0800200c9a66", Context.getOrderService().getOrderFrequency(1).getUuid());
 	}
 
 	/**
@@ -773,4 +784,89 @@ public void getCareSettings_shouldReturnRetiredCareSettingsIfIncludeRetiredIsSet
 		assertEquals(3, careSettings.size());
 		assertTrue(containsId(careSettings, retiredCareSetting.getCareSettingId()));
 	}
+
+	/**
+	 * @verifies not allow revising a stopped order
+	 * @see OrderService#saveOrder(org.openmrs.Order)
+	 */
+	@Test
+	public void saveOrder_shouldNotAllowRevisingAStoppedOrder() throws Exception {
+		Order originalOrder = orderService.getOrder(1);
+		assertNotNull(originalOrder.getDateStopped());
+		Order revisedOrder = originalOrder.cloneForRevision();
+		revisedOrder.setInstructions("Take after a meal");
+		expectedException.expect(APIException.class);
+		expectedException.expectMessage("Cannot discontinue an order that is already stopped, expired or voided");
+		orderService.saveOrder(revisedOrder);
+	}
+
+	/**
+	 * @verifies not allow revising a voided order
+	 * @see OrderService#saveOrder(org.openmrs.Order)
+	 */
+	@Test
+	public void saveOrder_shouldNotAllowRevisingAVoidedOrder() throws Exception {
+		Order originalOrder = orderService.getOrder(8);
+		assertTrue(originalOrder.isVoided());
+		Order revisedOrder = originalOrder.cloneForRevision();
+		revisedOrder.setInstructions("Take after a meal");
+		expectedException.expect(APIException.class);
+		expectedException.expectMessage("Cannot discontinue an order that is already stopped, expired or voided");
+		orderService.saveOrder(revisedOrder);
+	}
+
+	/**
+	 * @verifies not allow revising an expired order
+	 * @see OrderService#saveOrder(org.openmrs.Order)
+	 */
+	@Test
+	public void saveOrder_shouldNotAllowRevisingAnExpiredOrder() throws Exception {
+		Order originalOrder = orderService.getOrder(6);
+		assertNotNull(originalOrder.getAutoExpireDate());
+		assertTrue(originalOrder.getAutoExpireDate().before(new Date()));
+		Order revisedOrder = originalOrder.cloneForRevision();
+		revisedOrder.setInstructions("Take after a meal");
+		expectedException.expect(APIException.class);
+		expectedException.expectMessage("Cannot discontinue an order that is already stopped, expired or voided");
+		orderService.saveOrder(revisedOrder);
+	}
+
+	/**
+	 * @verifies not allow revising an order with no previous order
+	 * @see OrderService#saveOrder(org.openmrs.Order)
+	 */
+	@Test
+	public void saveOrder_shouldNotAllowRevisingAnOrderWithNoPreviousOrder() throws Exception {
+		Order originalOrder = orderService.getOrder(111);
+		assertTrue(originalOrder.isCurrent());
+		Order revisedOrder = originalOrder.cloneForRevision();
+		revisedOrder.setInstructions("Take after a meal");
+		revisedOrder.setPreviousOrder(null);
+
+		expectedException.expect(APIException.class);
+		expectedException.expectMessage("Previous Order is required for a revised order");
+		orderService.saveOrder(revisedOrder);
+	}
+
+	/**
+	 * @verifies save a revised order
+	 * @see OrderService#saveOrder(org.openmrs.Order)
+	 */
+	@Test
+	public void saveOrder_shouldSaveARevisedOrder() throws Exception {
+		Order originalOrder = orderService.getOrder(111);
+		assertTrue(originalOrder.isCurrent());
+		final Patient patient = originalOrder.getPatient();
+		List<Order> originalActiveOrders = orderService.getActiveOrders(patient, null, null, null);
+		final int originalOrderCount = originalActiveOrders.size();
+		assertTrue(originalActiveOrders.contains(originalOrder));
+		Order revisedOrder = originalOrder.cloneForRevision();
+		revisedOrder.setInstructions("Take after a meal");
+		revisedOrder.setStartDate(new Date());
+		orderService.saveOrder(revisedOrder);
+
+		List<Order> activeOrders = orderService.getActiveOrders(patient, null, null, null);
+		assertEquals(originalOrderCount, activeOrders.size());
+		assertFalse(originalOrder.isCurrent());
+	}
 }
diff --git a/api/src/test/resources/org/openmrs/api/include/OrderServiceTest-reviseDiscontinuedOrders.xml b/api/src/test/resources/org/openmrs/api/include/OrderServiceTest-reviseDiscontinuedOrders.xml
@@ -0,0 +1,8 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<dataset>
+    <orders order_id="16" order_number="6" urgency="ROUTINE" order_action="DISCONTINUE" concept_id="88" orderer="1" instructions="" 
+          creator="1" date_created="2012-01-19 12:24:10.0"
+          voided="false" patient_id="2"
+          uuid="0c96f25c-4949-4f72-9931-d808fbcdb611" care_setting="1" />
+  <drug_order order_id="16" drug_inventory_id="3" dose="125.0" dose_units="50" frequency="1" as_needed="false" dosing_type="SIMPLE" duration="10"/>
+</dataset>