TRUNK-3949: Do not commit with validation errors

openmrs · Apr 2, 2013 · 6629e8f · 6629e8f
1 parent 35ff9d9
commit 6629e8f
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 6 deletions.
diff --git a/api/src/main/java/org/openmrs/api/UserService.java b/api/src/main/java/org/openmrs/api/UserService.java
@@ -22,7 +22,6 @@
 import org.openmrs.User;
 import org.openmrs.annotation.Authorized;
 import org.openmrs.annotation.Logging;
-import org.openmrs.api.context.Context;
 import org.openmrs.api.context.UserContext;
 import org.openmrs.util.PersonByNameComparator;
 import org.openmrs.util.PrivilegeConstants;
@@ -642,5 +641,6 @@ public List<User> getUsers(String name, List<Role> roles, boolean includeRetired
 	 * @param hasPrivilege <code>true</code> if the authenticated user has the required privilege or if it is a proxy privilege
 	 * @since 1.8.4, 1.9.1, 1.10
 	 */
+	@Transactional(readOnly = true)
 	public void notifyPrivilegeListeners(User user, String privilege, boolean hasPrivilege);
 }
diff --git a/web/src/main/java/org/openmrs/web/controller/ConceptFormController.java b/web/src/main/java/org/openmrs/web/controller/ConceptFormController.java
@@ -56,6 +56,7 @@
 import org.openmrs.util.OpenmrsConstants;
 import org.openmrs.util.OpenmrsUtil;
 import org.openmrs.validator.ConceptValidator;
+import org.openmrs.validator.ValidateUtil;
 import org.openmrs.web.WebConstants;
 import org.springframework.beans.propertyeditors.CustomDateEditor;
 import org.springframework.beans.propertyeditors.CustomNumberEditor;
@@ -148,6 +149,7 @@ else if ("next".equals(jumpAction))
 	 * @should return a concept with a null id if no match is found
 	 * @should void a synonym marked as preferred when it is removed
 	 * @should set the local preferred name
+	 * @should not save changes if there are validation errors
 	 */
 	@Override
 	protected ModelAndView onSubmit(HttpServletRequest request, HttpServletResponse response, Object obj,

diff --git a/web/src/test/java/org/openmrs/web/controller/ConceptFormControllerTest.java b/web/src/test/java/org/openmrs/web/controller/ConceptFormControllerTest.java
@@ -36,9 +36,13 @@
 import org.openmrs.test.Verifies;
 import org.openmrs.web.controller.ConceptFormController.ConceptFormBackingObject;
 import org.openmrs.web.test.BaseWebContextSensitiveTest;
+import org.openmrs.web.test.WebTestHelper;
+import org.openmrs.web.test.WebTestHelper.Response;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.mock.web.MockHttpSession;
+import org.springframework.test.annotation.NotTransactional;
 import org.springframework.validation.BindException;
 import org.springframework.web.servlet.ModelAndView;
 
@@ -47,6 +51,12 @@
  */
 public class ConceptFormControllerTest extends BaseWebContextSensitiveTest {
 
+	@Autowired
+	WebTestHelper webTestHelper;
+
+	@Autowired
+	ConceptService conceptService;
+
 	/**
 	 * Checks that the conceptId query param gets a concept from the database
 	 * 
@@ -748,4 +758,28 @@ public void onSubmit_shouldVoidASynonymMarkedAsPreferredWhenItIsRemoved() throws
 		Assert.assertEquals(true, preferredName.isVoided());
 	}
 
+	/**
+     * @see ConceptFormController#onSubmit(HttpServletRequest,HttpServletResponse,Object,BindException)
+     * @verifies not save changes if there are validation errors
+     */
+    @Test
+    @NotTransactional
+    public void onSubmit_shouldNotSaveChangesIfThereAreValidationErrors() throws Exception {
+    	Integer conceptId = 792;
+
+	    MockHttpServletRequest request = new MockHttpServletRequest("POST", "/dictionary/concept.form");
+		request.setParameter("conceptId", conceptId.toString());
+		request.setParameter("namesByLocale[en].name", "should not change");
+		request.setParameter("preferredNamesByLocale[en]", "should not change");
+		request.setParameter("synonymsByLocale[en][1].name", ""); //empty name is invalid
+		request.setParameter("synonymsByLocale[en][1].voided", "false");
+
+		Response response = webTestHelper.handle(request);
+		assertEquals(true, response.getErrors().hasFieldErrors("synonymsByLocale[en][1].name"));
+
+		Context.clearSession();
+
+		Concept concept = conceptService.getConcept(conceptId);
+		assertEquals("STAVUDINE LAMIVUDINE AND NEVIRAPINE", concept.getPreferredName(Locale.ENGLISH).getName());
+    }
 }
diff --git a/web/src/test/java/org/openmrs/web/controller/user/RoleFormControllerTest.java b/web/src/test/java/org/openmrs/web/controller/user/RoleFormControllerTest.java
@@ -25,6 +25,7 @@
 import org.openmrs.web.test.WebTestHelper.Response;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.test.annotation.NotTransactional;
 
 /**`
  * Tests {@link RoleFormController}.
@@ -39,6 +40,7 @@ public UserService getUS() {
 	}
 
 	@Test
+	@NotTransactional
 	public void shouldUpdateRoleWithParent() throws Exception {
 		Role child = new Role("child", "child");
 		getUS().saveRole(child);
@@ -59,6 +61,8 @@ public void shouldUpdateRoleWithParent() throws Exception {
 		wth.handle(requestPOST);
 
 		Assert.assertEquals("updated child", getUS().getRole("child").getDescription());
+
+		deleteAllData();
 	}
 
 }
diff --git a/web/src/test/java/org/openmrs/web/test/WebTestHelper.java b/web/src/test/java/org/openmrs/web/test/WebTestHelper.java
@@ -24,6 +24,8 @@
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.stereotype.Component;
+import org.springframework.validation.BindException;
+import org.springframework.validation.Errors;
 import org.springframework.web.servlet.HandlerAdapter;
 import org.springframework.web.servlet.HandlerExecutionChain;
 import org.springframework.web.servlet.HandlerMapping;
@@ -132,11 +134,7 @@ public Response handle(final HttpServletRequest request) throws Exception {
 		}
 
 		Assert.assertTrue("The requested URI has no handlers: " + request.getRequestURI(), supported);
-
-		//Complete the request by flushing and clearing the Hibernate session
-		Context.flushSession();
-		Context.clearSession();
-
+
 		return new Response(response, request.getSession(), modelAndView);
 	}
 
@@ -153,5 +151,13 @@ public Response(MockHttpServletResponse http, HttpSession session, ModelAndView
 			this.session = session;
 			this.modelAndView = modelAndView;
 		}
+
+		public Errors getErrors(String model) {
+			return (Errors) modelAndView.getModel().get(BindException.MODEL_KEY_PREFIX + model);
+		}
+
+		public Errors getErrors() {
+			return getErrors("command");
+		}
 	}
 }