Skip to content

Commit 595bb27

Browse files
mogoodrichmogoodrich
authored
HTML-880: Required fields should not be required if they are hidden via a "controls/when/then" functionality (#332)
1 parent ec5d7a2 commit 595bb27

4 files changed

Lines changed: 192 additions & 2 deletions

File tree

‎api-tests/src/test/java/org/openmrs/module/htmlformentry/ControlsWhenRequiredTest.java‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
package org.openmrs.module.htmlformentry;
2+
3+
import org.junit.Before;
4+
import org.junit.Test;
5+
import org.openmrs.api.context.Context;
6+
import org.springframework.mock.web.MockHttpServletRequest;
7+
8+
import java.util.Date;
9+
import java.util.Map;
10+
11+
/**
12+
* Tests that fields hidden by <controls>/<when thenDisplay> are exempt from required
13+
* validation, and that required validation still fires when the field is visible.
14+
*/
15+
public class ControlsWhenRequiredTest extends BaseHtmlFormEntryTest {
16+
17+
@Before
18+
public void loadData() throws Exception {
19+
executeVersionedDataSet("org/openmrs/module/htmlformentry/data/RegressionTest-data-openmrs-2.8.xml");
20+
}
21+
22+
/**
23+
* When the controlling obs value does not match the thenDisplay condition the section is hidden
24+
* client-side and the JS populates hfeHiddenFields with the widget name. The server should skip
25+
* required validation for those fields.
26+
*/
27+
@Test
28+
public void requiredFieldHiddenByControls_shouldNotProduceValidationError() throws Exception {
29+
final Date date = new Date();
30+
new RegressionTestHelper() {
31+
32+
@Override
33+
public String getFormName() {
34+
return "obsWithControlsAndRequired";
35+
}
36+
37+
@Override
38+
public String[] widgetLabels() {
39+
return new String[] { "Date:", "Location:", "Provider:", "Allergy:", "Details:" };
40+
}
41+
42+
@Override
43+
public void setupRequest(MockHttpServletRequest request, Map<String, String> widgets) {
44+
request.addParameter(widgets.get("Date:"), dateAsString(date));
45+
request.addParameter(widgets.get("Location:"), "2");
46+
request.addParameter(widgets.get("Provider:"), "502");
47+
// Allergy is left blank — section #allergy-followup stays hidden.
48+
// Simulate what the JS sends: the hidden field's widget name in hfeHiddenFields.
49+
request.addParameter("hfeHiddenFields", widgets.get("Details:"));
50+
}
51+
52+
@Override
53+
public void testResults(SubmissionResults results) {
54+
results.assertNoErrors();
55+
results.assertEncounterCreated();
56+
}
57+
}.run();
58+
}
59+
60+
/**
61+
* When the controlling obs value matches the thenDisplay condition the section is shown. The
62+
* required field inside it must be filled; leaving it empty should produce a validation error.
63+
*/
64+
@Test
65+
public void requiredFieldVisibleByControls_withNoValue_shouldProduceValidationError() throws Exception {
66+
final Date date = new Date();
67+
new RegressionTestHelper() {
68+
69+
@Override
70+
public String getFormName() {
71+
return "obsWithControlsAndRequired";
72+
}
73+
74+
@Override
75+
public String[] widgetLabels() {
76+
return new String[] { "Date:", "Location:", "Provider:", "Allergy:", "Details:" };
77+
}
78+
79+
@Override
80+
public void setupRequest(MockHttpServletRequest request, Map<String, String> widgets) {
81+
request.addParameter(widgets.get("Date:"), dateAsString(date));
82+
request.addParameter(widgets.get("Location:"), "2");
83+
request.addParameter(widgets.get("Provider:"), "502");
84+
// CATS (1002) triggers the thenDisplay — section is visible.
85+
request.addParameter(widgets.get("Allergy:"), "1002");
86+
// Details field is visible but intentionally left empty.
87+
// hfeHiddenFields is not set for the Details widget.
88+
}
89+
90+
@Override
91+
public void testResults(SubmissionResults results) {
92+
results.assertErrors(1);
93+
results.assertNoEncounterCreated();
94+
}
95+
}.run();
96+
}
97+
98+
/**
99+
* When the controlling obs value matches the thenDisplay condition and the required field is
100+
* filled, the form should submit successfully and save both obs.
101+
*/
102+
@Test
103+
public void requiredFieldVisibleByControls_withValue_shouldSucceed() throws Exception {
104+
final Date date = new Date();
105+
new RegressionTestHelper() {
106+
107+
@Override
108+
public String getFormName() {
109+
return "obsWithControlsAndRequired";
110+
}
111+
112+
@Override
113+
public String[] widgetLabels() {
114+
return new String[] { "Date:", "Location:", "Provider:", "Allergy:", "Details:" };
115+
}
116+
117+
@Override
118+
public void setupRequest(MockHttpServletRequest request, Map<String, String> widgets) {
119+
request.addParameter(widgets.get("Date:"), dateAsString(date));
120+
request.addParameter(widgets.get("Location:"), "2");
121+
request.addParameter(widgets.get("Provider:"), "502");
122+
// CATS (1002) triggers the thenDisplay — section is visible.
123+
request.addParameter(widgets.get("Allergy:"), "1002");
124+
// Required Details field is filled.
125+
request.addParameter(widgets.get("Details:"), "Follow-up details here");
126+
}
127+
128+
@Override
129+
public void testResults(SubmissionResults results) {
130+
results.assertNoErrors();
131+
results.assertEncounterCreated();
132+
results.assertObsCreated(1000, Context.getConceptService().getConcept(1002));
133+
results.assertObsCreated(60000, "Follow-up details here");
134+
}
135+
}.run();
136+
}
137+
}

‎api/src/main/java/org/openmrs/module/htmlformentry/element/ObsSubmissionElement.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1444,7 +1444,7 @@ public Collection<FormSubmissionError> validateSubmission(FormEntryContext conte
14441444
}
14451445
}
14461446

1447-
if (required) {
1447+
if (required && !isFieldHiddenByControls(context, submission)) {
14481448
if (value == null) {
14491449
ret.add(new FormSubmissionError(valueWidget,
14501450
Context.getMessageSourceService().getMessage("htmlformentry.error.required")));
@@ -1459,7 +1459,16 @@ public Collection<FormSubmissionError> validateSubmission(FormEntryContext conte
14591459

14601460
return ret;
14611461
}
1462-
1462+
1463+
private boolean isFieldHiddenByControls(FormEntryContext context, HttpServletRequest submission) {
1464+
String hiddenFields = submission.getParameter("hfeHiddenFields");
1465+
if (StringUtils.isEmpty(hiddenFields)) {
1466+
return false;
1467+
}
1468+
String fieldName = context.getFieldNameIfRegistered(valueWidget);
1469+
return fieldName != null && Arrays.asList(hiddenFields.split(",")).contains(fieldName);
1470+
}
1471+
14631472
@Override
14641473
public void handleSubmission(FormEntrySession session, HttpServletRequest submission) {
14651474
Object value = valueWidget.getValue(session.getContext(), submission);

‎api/src/test/resources/org/openmrs/module/htmlformentry/include/obsWithControlsAndRequired.xml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<htmlform>
2+
Date: <encounterDate default="today"/>
3+
Location: <encounterLocation default="2"/>
4+
Provider: <encounterProvider role="Provider"/>
5+
Allergy: <obs conceptId="1000" id="allergy" answerConceptIds="1001,1002">
6+
<controls>
7+
<when value="1002" thenDisplay="#allergy-followup"/>
8+
</controls>
9+
</obs>
10+
<div id="allergy-followup">
11+
Details: <obs conceptId="60000" required="true"/>
12+
</div>
13+
<submit/>
14+
</htmlform>

‎omod/src/main/webapp/resources/htmlForm.js‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,31 @@
11
//Uses the namespace pattern from http://stackoverflow.com/a/5947280
22
(function( htmlForm, $, undefined) {
33

4+
var getHiddenFieldsTracker = function() {
5+
return $('form input[name="hfeHiddenFields"]');
6+
};
7+
8+
// Adds or removes field names within a section from the hidden-fields tracker.
9+
// The tracker is a comma-separated hidden input read server-side to skip required
10+
// validation on fields that were hidden by when/then controls at submission time.
11+
var updateHiddenFieldsTracker = function(section, isHiding) {
12+
var $tracker = getHiddenFieldsTracker();
13+
if (!$tracker.length) return;
14+
15+
var hidden = $tracker.val() ? $tracker.val().split(',') : [];
16+
$(section).find(':input[name]').each(function() {
17+
var name = $(this).attr('name');
18+
if (!name) return;
19+
var idx = hidden.indexOf(name);
20+
if (isHiding) {
21+
if (idx === -1) hidden.push(name);
22+
} else {
23+
if (idx > -1) hidden.splice(idx, 1);
24+
}
25+
});
26+
$tracker.val(hidden.join(','));
27+
};
28+
429
var onObsChangedCheck = function() {
530
var whenValueThenDisplaySection = $(this).data('whenValueThenDisplaySection');
631

@@ -30,6 +55,7 @@
3055
if (val === ifValue) {
3156
$(thenSection).show();
3257
displayedSection = thenSection;
58+
updateHiddenFieldsTracker(thenSection, false);
3359
}
3460
});
3561

@@ -38,6 +64,7 @@
3864
$.each(whenValueThenDisplaySection, function(ifValue, thenSection) {
3965
if (val !== ifValue && thenSection !== displayedSection) {
4066
$(thenSection).hide();
67+
updateHiddenFieldsTracker(thenSection, true);
4168
$(thenSection).find('input[type="hidden"], input:text, input:password, input:file, select, textarea').val('');
4269
$(thenSection).find('input:checkbox, input:radio').removeAttr('checked').removeAttr('selected');
4370
}
@@ -68,6 +95,9 @@
6895
}
6996

7097
htmlForm.setupWhenThen = function(obsId, valueToSection, valueToThenJs, valueToElseJs) {
98+
if (getHiddenFieldsTracker().length === 0) {
99+
$('form').append('<input type="hidden" name="hfeHiddenFields" value="">');
100+
}
71101
var field = getField(obsId + '.value');
72102
field.data('whenValueThenDisplaySection', valueToSection);
73103
field.data('whenValueThenJs', valueToThenJs);

0 commit comments

Comments
 (0)