Location hierarchy should not allow loops, description should not be …

…required - TRUNK-2707
openmrs · May 6, 2013 · aec1701 · aec1701
1 parent a354ea1
commit aec1701
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 0 deletions.
diff --git a/src/api/org/openmrs/validator/LocationValidator.java b/src/api/org/openmrs/validator/LocationValidator.java
@@ -53,6 +53,7 @@ public boolean supports(Class c) {
 	 * @should set retired to false if retireReason is null or empty
 	 * @should pass validation if all fields are correct
 	 * @should pass validation if retired location is given retired reason
+     * @should fail validation if parent location creates a loop
 	 */
 	public void validate(Object obj, Errors errors) {
 		Location location = (Location) obj;
@@ -66,6 +67,17 @@ public void validate(Object obj, Errors errors) {
 					errors.rejectValue("retireReason", "error.null");
 				}
 			}
+
+		    // Traverse all the way up (down?) to the root and check if it
+            // equals the root.
+            Location root = location;
+            while (root.getParentLocation() != null) {
+                root = root.getParentLocation();
+                if (root.equals(location)) { // Have gone in a circle
+                    errors.rejectValue("parentLocation", "Location.parentLocation.error");
+                    break;
+                }
+            }
 		}
 	}
 

diff --git a/test/api/org/openmrs/validator/LocationValidatorTest.java b/test/api/org/openmrs/validator/LocationValidatorTest.java
@@ -107,4 +107,27 @@ public void validate_shouldPassValidationIfRetiredLocationIsGivenRetiredReason()
 
 		Assert.assertFalse(errors.hasErrors());
 	}
+
+
+    /**
+     * @see {@link LocationValidator#validate(Object,Errors)}
+     */
+    @Test
+    @Verifies(value = "should fail validation if parent location creates a loop", method = "validate(Object,Errors)")
+    public void validate_shouldFailValidationIfParentLocationCreatesALoop() throws Exception {
+        Location location1 = new Location();
+        Location location2 = new Location();
+        Location location3 = new Location();
+        location1.setName("County General");
+        location2.setName("Radiology");
+        location3.setName("Radiology Lab");
+        location3.setParentLocation(location2);
+        location2.setParentLocation(location1);
+        location1.setParentLocation(location3);
+
+        Errors errors = new BindException(location1, "location");
+        new LocationValidator().validate(location1, errors);
+
+        Assert.assertTrue(errors.hasFieldErrors("parentLocation"));
+    }
 }
diff --git a/web/WEB-INF/messages.properties b/web/WEB-INF/messages.properties
@@ -894,6 +894,7 @@ Location.tags=Tags
 Location.search.error=Error while attempting to find locations
 Location.noLocationsFound=No locations found. Please search again.
 Location.get.error=Error while attempting to get locations
+Location.parentLocation.error=Can't set parent to a child! As this would create a loop in the hierarchy.
 
 Location.hierarchy=Location Hierarchy
 Location.hierarchy.view=View Location Hierarchy