Made notifications email address required unless notifications are made only internally - TRUNK-2498

wluyima · wluyima · commit 94b684115865 · 2013-06-10T10:50:32.000-04:00
diff --git a/web/src/main/java/org/openmrs/web/controller/OptionsFormController.java b/web/src/main/java/org/openmrs/web/controller/OptionsFormController.java
@@ -201,10 +201,10 @@ protected ModelAndView onSubmit(HttpServletRequest request, HttpServletResponse
 			String notifyType = opts.getNotification();
 			if (notifyType != null) {
 				if (notifyType.equals("internal") || notifyType.equals("internalProtected") || notifyType.equals("email")) {
-					if (StringUtils.isNotEmpty(opts.getNotificationAddress())) {
-						if (!EmailValidator.getInstance().isValid(opts.getNotificationAddress())) {
-							errors.reject("error.options.notificationAddress.invalid");
-						}
+					if (opts.getNotificationAddress().isEmpty()) {
+						errors.reject("error.options.notificationAddress.empty");
+					} else if (!EmailValidator.getInstance().isValid(opts.getNotificationAddress())) {
+						errors.reject("error.options.notificationAddress.invalid");
 					}
 				}
 			}
diff --git a/web/src/test/java/org/openmrs/web/controller/OptionsFormControllerTest.java b/web/src/test/java/org/openmrs/web/controller/OptionsFormControllerTest.java
@@ -221,4 +221,18 @@ public void shouldAcceptValidNotificationAddress() throws Exception {
 			assertEquals(correctAddress, optionsForm.getNotificationAddress());
 		}
 	}
+	
+	@Test
+	public void shouldRejectEmptyNotificationAddress() throws Exception {
+		MockHttpServletRequest request = new MockHttpServletRequest("POST", "");
+		request.setParameter("notification", "email");
+		request.setParameter("notificationAddress", "");
+		
+		HttpServletResponse response = new MockHttpServletResponse();
+		ModelAndView modelAndView = controller.handleRequest(request, response);
+		
+		BeanPropertyBindingResult bindingResult = (BeanPropertyBindingResult) modelAndView.getModel().get(
+		    "org.springframework.validation.BindingResult.opts");
+		Assert.assertTrue(bindingResult.hasErrors());
+	}
 }
diff --git a/webapp/src/main/webapp/WEB-INF/messages.properties b/webapp/src/main/webapp/WEB-INF/messages.properties
@@ -373,6 +373,8 @@ error.username.invalid=Invalid username.  Username must be alphanumeric and cann
 error.username.email=Invalid username.  Username must be a valid e-mail.
 error.retired.requireMetadata=Who retired this and why?
 error.options.notificationAddress.invalid=Invalid notifications email address
+error.options.notificationAddress.empty=No notifications email address specified
+
 
 changePassword.hint.password.length=Password should have at least {0} characters
 changePassword.hint.password.bothCasesRequired=both upper and lower case characters

Original file line number	Diff line number	Diff line change
`@@ -201,10 +201,10 @@ protected ModelAndView onSubmit(HttpServletRequest request, HttpServletResponse`
`201`	`201`	`String notifyType = opts.getNotification();`
`202`	`202`	`if (notifyType != null) {`
`203`	`203`	`if (notifyType.equals("internal") \|\| notifyType.equals("internalProtected") \|\| notifyType.equals("email")) {`
`204`		`- if (StringUtils.isNotEmpty(opts.getNotificationAddress())) {`
`205`		`- if (!EmailValidator.getInstance().isValid(opts.getNotificationAddress())) {`
`206`		`- errors.reject("error.options.notificationAddress.invalid");`
`207`		`- }`
	`204`	`+ if (opts.getNotificationAddress().isEmpty()) {`
	`205`	`+ errors.reject("error.options.notificationAddress.empty");`
	`206`	`+ } else if (!EmailValidator.getInstance().isValid(opts.getNotificationAddress())) {`
	`207`	`+ errors.reject("error.options.notificationAddress.invalid");`
`208`	`208`	`}`
`209`	`209`	`}`
`210`	`210`	`}`
Original file line number	Diff line number	Diff line change
`@@ -221,4 +221,18 @@ public void shouldAcceptValidNotificationAddress() throws Exception {`
`221`	`221`	`assertEquals(correctAddress, optionsForm.getNotificationAddress());`
`222`	`222`	`}`
`223`	`223`	`}`
	`224`	`+`
	`225`	`+ @Test`
	`226`	`+ public void shouldRejectEmptyNotificationAddress() throws Exception {`
	`227`	`+ MockHttpServletRequest request = new MockHttpServletRequest("POST", "");`
	`228`	`+ request.setParameter("notification", "email");`
	`229`	`+ request.setParameter("notificationAddress", "");`
	`230`	`+`
	`231`	`+ HttpServletResponse response = new MockHttpServletResponse();`
	`232`	`+ ModelAndView modelAndView = controller.handleRequest(request, response);`
	`233`	`+`
	`234`	`+ BeanPropertyBindingResult bindingResult = (BeanPropertyBindingResult) modelAndView.getModel().get(`
	`235`	`+ "org.springframework.validation.BindingResult.opts");`
	`236`	`+ Assert.assertTrue(bindingResult.hasErrors());`
	`237`	`+ }`
`224`	`238`	`}`