Responding to review comments for: Add missing CRUD methods for Order

Frequency - TRUNK-4188
openmrs · Feb 27, 2014 · 4144b0d · 4144b0d
1 parent 7c8cc31
commit 4144b0d
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 5 deletions.
diff --git a/api/src/main/java/org/openmrs/api/OrderService.java b/api/src/main/java/org/openmrs/api/OrderService.java
@@ -407,7 +407,7 @@ public List<OrderFrequency> getOrderFrequencies(String searchPhrase, Locale loca
 	public OrderFrequency retireOrderFrequency(OrderFrequency orderFrequency, String reason);
 
 	/**
-	 * Restores an order frequency that was previous retired in the database
+	 * Restores an order frequency that was previously retired in the database
 	 * 
 	 * @param orderFrequency the order frequency to unretire
 	 * @return the unretired order frequency
@@ -423,7 +423,8 @@ public List<OrderFrequency> getOrderFrequencies(String searchPhrase, Locale loca
 	 * @param orderFrequency the order frequency to purge
 	 * @since 1.10
 	 * @should delete given order frequency
+	 * @should not allow deleting an order frequency that is in use
 	 */
 	@Authorized(PrivilegeConstants.PURGE_ORDER_FREQUENCIES)
-	public void purgeOrderFrequency(OrderFrequency orderFrequency);
+	public void purgeOrderFrequency(OrderFrequency orderFrequency) 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
@@ -36,7 +36,6 @@
 import org.openmrs.api.context.Context;
 import org.openmrs.api.db.OrderDAO;
 import org.openmrs.util.OpenmrsUtil;
-import org.openmrs.validator.ValidateUtil;
 import org.springframework.transaction.annotation.Propagation;
 import org.springframework.transaction.annotation.Transactional;
 import org.springframework.util.StringUtils;
@@ -470,7 +469,6 @@ public OrderFrequency saveOrderFrequency(OrderFrequency orderFrequency) throws A
 			}
 		}
 
-		ValidateUtil.validate(orderFrequency);
 		return dao.saveOrderFrequency(orderFrequency);
 	}
 
@@ -495,6 +493,11 @@ public OrderFrequency unretireOrderFrequency(OrderFrequency orderFrequency) {
 	 */
 	@Override
 	public void purgeOrderFrequency(OrderFrequency orderFrequency) {
+
+		if (dao.isOrderFrequencyInUse(orderFrequency)) {
+			throw new APIException("This order frequency cannot be deleted because it is already in use");
+		}
+
 		dao.purgeOrderFrequency(orderFrequency);
 	}
 }
diff --git a/api/src/test/java/org/openmrs/api/OrderServiceTest.java b/api/src/test/java/org/openmrs/api/OrderServiceTest.java
@@ -987,13 +987,15 @@ public void retireOrderFrequency_shouldRetireGivenOrderFrequency() throws Except
 		assertNotNull(orderFrequency);
 		Assert.assertFalse(orderFrequency.isRetired());
 		Assert.assertNull(orderFrequency.getRetireReason());
+		Assert.assertNull(orderFrequency.getDateRetired());
 
 		Context.getOrderService().retireOrderFrequency(orderFrequency, "retire reason");
 
 		orderFrequency = Context.getOrderService().getOrderFrequency(1);
 		assertNotNull(orderFrequency);
 		assertTrue(orderFrequency.isRetired());
 		assertEquals("retire reason", orderFrequency.getRetireReason());
+		assertNotNull(orderFrequency.getDateRetired());
 
 		//Should not change the number of order frequencies.
 		assertEquals(3, Context.getOrderService().getOrderFrequencies(true).size());
@@ -1006,13 +1008,15 @@ public void unretireOrderFrequency_shouldUnretireGivenOrderFrequency() throws Ex
 		assertNotNull(orderFrequency);
 		assertTrue(orderFrequency.isRetired());
 		assertEquals("Some Retire Reason", orderFrequency.getRetireReason());
+		assertNotNull(orderFrequency.getDateRetired());
 
 		Context.getOrderService().unretireOrderFrequency(orderFrequency);
 
 		orderFrequency = Context.getOrderService().getOrderFrequency(3);
 		assertNotNull(orderFrequency);
 		assertFalse(orderFrequency.isRetired());
 		assertNull(orderFrequency.getRetireReason());
+		assertNull(orderFrequency.getDateRetired());
 
 		//Should not change the number of order frequencies.
 		assertEquals(3, Context.getOrderService().getOrderFrequencies(true).size());
@@ -1070,13 +1074,29 @@ public void saveOrderFrequency_shouldEditAnExistingOrderFrequencyThatIsNotInUse(
 	/**
 	 * @see {@link OrderService#saveOrderFrequency(OrderFrequency)}
 	 */
-	@Test(expected = APIException.class)
+	@Test
 	@Verifies(value = "should not allow editing an existing order frequency that is in use", method = "saveOrderFrequency(OrderFrequency)")
 	public void saveOrderFrequency_shouldNotAllowEditingAnExistingOrderFrequencyThatIsInUse() throws Exception {
 		OrderFrequency orderFrequency = Context.getOrderService().getOrderFrequency(1);
 		assertNotNull(orderFrequency);
 
 		orderFrequency.setFrequencyPerDay(4d);
+		expectedException.expect(APIException.class);
+		expectedException.expectMessage("This order frequency cannot be edited because it is already in use");
 		Context.getOrderService().saveOrderFrequency(orderFrequency);
 	}
+
+	/**
+	 * @see {@link OrderService#purgeOrderFrequency(OrderFrequency)}
+	 */
+	@Test
+	@Verifies(value = "should not allow deleting an order frequency that is in use", method = "purgeOrderFrequency(OrderFrequency)")
+	public void purgeOrderFrequency_shouldNotAllowDeletingAnOrderFrequencyThatIsInUse() throws Exception {
+		OrderFrequency orderFrequency = Context.getOrderService().getOrderFrequency(1);
+		assertNotNull(orderFrequency);
+
+		expectedException.expect(APIException.class);
+		expectedException.expectMessage("This order frequency cannot be deleted because it is already in use");
+		Context.getOrderService().purgeOrderFrequency(orderFrequency);
+	}
 }