Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,21 @@ public void stringToDocument_shouldNotAccessFilesystemResources() throws java.io

}

@Test
@Verifies(value = "shouldSetValueTextToProviderIdGivenAProvider", method = "createObs(Concept concept, Object value, Date datetime, String accessionNumber)")
public void createObs_shouldSetValueTextToProviderIdGivenAProvider() {
Provider provider = new Provider();
provider.setId(42);

Concept c = new Concept();
ConceptDatatype cd = new ConceptDatatype();
cd.setUuid("8d4a4ab4-c2cc-11de-8d13-0010c6dffd0f");
c.setDatatype(cd);

Obs o = HtmlFormEntryUtil.createObs(c, provider, null, null);
Assert.assertEquals("42", o.getValueText());
}

@Test
@Verifies(value = "shouldSetTheValueComplexOfObsIfConceptIsComplex", method = "createObs(Concept concept, Object value, Date datetime, String accessionNumber)")
public void createObs_shouldSetTheValueComplexOfObsIfConceptIsComplex() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.openmrs.ConceptNumeric;
import org.openmrs.api.ConceptNameType;
import org.openmrs.api.context.Context;
import org.openmrs.module.htmlformentry.BadFormDesignException;
import org.openmrs.module.htmlformentry.FormEntryContext;
import org.openmrs.module.htmlformentry.HtmlFormEntryUtil;
import org.openmrs.module.htmlformentry.TestUtil;
Expand Down Expand Up @@ -84,7 +85,7 @@ public void tearDown() {
}

@Test
public void testShowUnitsUsingTrue() {
public void testShowUnitsUsingTrue() throws Exception {
ConceptDatatype numeric = new ConceptDatatype();
numeric.setUuid(ConceptDatatype.NUMERIC_UUID);

Expand All @@ -106,7 +107,7 @@ public void testShowUnitsUsingTrue() {

@Test
@Ignore
public void testShowUnitsUsingCode() {
public void testShowUnitsUsingCode() throws Exception {
ConceptDatatype numeric = new ConceptDatatype();
numeric.setUuid(ConceptDatatype.NUMERIC_UUID);

Expand Down Expand Up @@ -211,31 +212,31 @@ public void testForcingLocaleOnRadioLabels() throws Exception {
}

@Test(expected = RuntimeException.class)
public void testNoConceptId_shouldThrowException() {
public void testNoConceptId_shouldThrowException() throws Exception {
new ObsSubmissionElement(context, params);
}

@Test(expected = RuntimeException.class)
public void testDuplicateConceptIds_shouldThrowException() {
public void testDuplicateConceptIds_shouldThrowException() throws Exception {
params.put("conceptId", "1");
params.put("conceptIds", "2");
new ObsSubmissionElement(context, params);
}

@Test(expected = RuntimeException.class)
public void testInvalidConceptIds_shouldThrowException() {
public void testInvalidConceptIds_shouldThrowException() throws Exception{
params.put("conceptIds", "adas,asda");
new ObsSubmissionElement(context, params);
}

@Test(expected = RuntimeException.class)
public void testEmptyConceptIds_shouldThrowException() {
public void testEmptyConceptIds_shouldThrowException() throws Exception {
params.put("conceptIds", "");
new ObsSubmissionElement(context, params);
}

@Test(expected = RuntimeException.class)
public void testInvalidAnswerConceptIds_shouldThrowException() {
public void testInvalidAnswerConceptIds_shouldThrowException() throws Exception {
params.put("answerConceptIds", "adas,asda");
new ObsSubmissionElement(context, params);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,8 @@ public static Obs createObs(Concept concept, Object value, Date datetime, String
} else if (value instanceof Person) {
Person person = (Person) value;
obs.setValueText(person.getId().toString() + " - " + person.getPersonName().toString());
} else if (value instanceof Provider) {
obs.setValueText(((Provider) value).getId().toString());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love setting the primary key id to the value - it would be much better if this were the uuid - but if this is the pre-existing convention, I guess it is fine. Can you confirm?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we have "<id> - <name>" format for Location and Person when saving them as obs values, but reading / parsing the value is more relaxed and can accept that, the id or the uuid.

However, Provider is stored as just <id>. We've been doing that for <obs style="provider_dropdown"> and <obs style="provider_radio"> already.

None of them are stored as uuid.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sounds like legacy behavior--if it's straightforward to change all of them to use uuids (while being backwards compatible) it would likely make sense to fix, though I my gut would be to do it in a quick follow-on ticket and commit so we can track it separately in case there are any issues.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

} else {
obs.setValueText(value.toString());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.openmrs.Obs;
import org.openmrs.Person;
import org.openmrs.api.context.Context;
import org.openmrs.module.htmlformentry.BadFormDesignException;
import org.openmrs.module.htmlformentry.FormEntryContext;
import org.openmrs.module.htmlformentry.HtmlFormEntryConstants;
import org.openmrs.module.htmlformentry.HtmlFormEntryUtil;
Expand All @@ -37,7 +38,7 @@ public class ObsReferenceSubmissionElement extends ObsSubmissionElement<FormEntr

private String tooltipTemplate = "({{encounterType}} on {{encounterDate}})";

public ObsReferenceSubmissionElement(FormEntryContext context, Map<String, String> parameters) {
public ObsReferenceSubmissionElement(FormEntryContext context, Map<String, String> parameters) throws BadFormDesignException {

super(context, parameters);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.openmrs.Role;
import org.openmrs.Visit;
import org.openmrs.api.context.Context;
import org.openmrs.module.htmlformentry.BadFormDesignException;
import org.openmrs.module.htmlformentry.FormEntryContext;
import org.openmrs.module.htmlformentry.FormEntryContext.Mode;
import org.openmrs.module.htmlformentry.FormEntrySession;
Expand All @@ -30,8 +31,10 @@
import org.openmrs.module.htmlformentry.widget.ConceptSearchAutocompleteWidget;
import org.openmrs.module.htmlformentry.widget.DateTimeWidget;
import org.openmrs.module.htmlformentry.widget.DateWidget;
import org.openmrs.module.htmlformentry.util.MatchMode;
import org.openmrs.module.htmlformentry.widget.DropdownWidget;
import org.openmrs.module.htmlformentry.widget.DynamicAutocompleteWidget;
import org.openmrs.module.htmlformentry.widget.ProviderAjaxAutoCompleteWidget;
import org.openmrs.module.htmlformentry.widget.ErrorWidget;
import org.openmrs.module.htmlformentry.widget.NumberFieldWidget;
import org.openmrs.module.htmlformentry.widget.Option;
Expand Down Expand Up @@ -163,7 +166,7 @@ public class ObsSubmissionElement<T extends FormEntryContext> implements HtmlGen

private String tagControlId;

public ObsSubmissionElement(T context, Map<String, String> parameters) {
public ObsSubmissionElement(T context, Map<String, String> parameters) throws BadFormDesignException {
if (parameters.get("locale") != null) {
this.locale = LocaleUtility.fromSpecification(parameters.get("locale"));
}
Expand Down Expand Up @@ -241,7 +244,7 @@ else if (conceptId == null && conceptIds == null)
isLocationObs = "location".equals(parameters.get("style")) || "location_radio".equals(parameters.get("style"))
|| "location_dropdown".equals(parameters.get("style"));
isProviderObs = "provider".equals(parameters.get("style")) || "provider_radio".equals(parameters.get("style"))
|| "provider_dropdown".equals(parameters.get("style"));
|| "provider_dropdown".equals(parameters.get("style")) || "provider_autocomplete".equals(parameters.get("style"));
isRadioSet = "radio".equals(parameters.get("style")) || "location_radio".equals(parameters.get("style"))
|| "provider_radio".equals(parameters.get("style"));

Expand Down Expand Up @@ -273,7 +276,7 @@ private Widget buildDropdownWidget(Integer size) {
return dropdownWidget;
}

private void prepareWidgets(T context, Map<String, String> parameters) {
private void prepareWidgets(T context, Map<String, String> parameters) throws BadFormDesignException {
String userLocaleStr = locale.toString();
try {
if (answerConcept == null)
Expand Down Expand Up @@ -565,40 +568,55 @@ private void prepareWidgets(T context, Map<String, String> parameters) {
}
// configure the special obs type that allows selection of a provider (the provider_id PK is stored as the valueText)
else if (isProviderObs) {
if (isRadioSet) {
valueWidget = new RadioButtonsWidget();
if (answerSeparator != null) {
((RadioButtonsWidget) valueWidget).setAnswerSeparator(answerSeparator);
}
} else { // dropdown
valueWidget = new DropdownWidget();
// if initialValueIsSet=false, no initial/default location, hence this shows the 'select input' field as first option
boolean initialValueIsSet = !(initialValue == null);
((SingleOptionWidget) valueWidget).addOption(
new Option(Context.getMessageSourceService().getMessage("htmlformentry.chooseAProvider"), "",
!initialValueIsSet));
}
List<String> roleIds = new ArrayList<>();
String roleParam = parameters.get("providerRoles");
if (StringUtils.isNotBlank(roleParam)) {
for (String roleId : roleParam.split(",")) {
roleIds.add(roleId.trim());
}
}
List<Provider> providers = HtmlFormEntryUtil.getProviders(roleIds, true);

List<Option> providerOptions = new ArrayList<>();
for (Provider provider : providers) {
String providerId = provider.getId().toString();
String label = HtmlFormEntryUtil.format(provider);
Option option = new Option(label, providerId, providerId.equals(initialValue));
providerOptions.add(option);
}
Collections.sort(providerOptions, new OptionComparator());

if (!providerOptions.isEmpty()) {
for (Option option : providerOptions) {
((SingleOptionWidget) valueWidget).addOption(option);
if ("provider_autocomplete".equals(parameters.get("style"))) {
MatchMode matchMode = MatchMode.ANYWHERE;
if (StringUtils.isNotBlank(parameters.get("providerMatchMode"))) {
try {
matchMode = MatchMode.valueOf(parameters.get("providerMatchMode").toUpperCase());
}
catch (Exception e) {
throw new BadFormDesignException(
"Invalid providerMatchMode '" + parameters.get("providerMatchMode")
+ "'. Valid values are: " + Arrays.toString(MatchMode.values()), e);
}
}
valueWidget = new ProviderAjaxAutoCompleteWidget(matchMode, roleIds);
} else {
if (isRadioSet) {
valueWidget = new RadioButtonsWidget();
if (answerSeparator != null) {
((RadioButtonsWidget) valueWidget).setAnswerSeparator(answerSeparator);
}
} else { // dropdown
valueWidget = new DropdownWidget();
// if initialValueIsSet=false, no initial/default location, hence this shows the 'select input' field as first option
boolean initialValueIsSet = !(initialValue == null);
((SingleOptionWidget) valueWidget).addOption(
new Option(Context.getMessageSourceService().getMessage("htmlformentry.chooseAProvider"), "",
!initialValueIsSet));
}
List<Provider> providers = HtmlFormEntryUtil.getProviders(roleIds, true);

List<Option> providerOptions = new ArrayList<>();
for (Provider provider : providers) {
String providerId = provider.getId().toString();
String label = HtmlFormEntryUtil.format(provider);
Option option = new Option(label, providerId, providerId.equals(initialValue));
providerOptions.add(option);
}
Collections.sort(providerOptions, new OptionComparator());

if (!providerOptions.isEmpty()) {
for (Option option : providerOptions) {
((SingleOptionWidget) valueWidget).addOption(option);
}
}
}
} else if ("person".equals(parameters.get("style"))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ public Object getProviders(@RequestParam(value = "searchParam", required = false

List<Provider> providerList = HtmlFormEntryUtil.getProviders(providerRoleIds, true);

/*
* A frontend bug makes it possible for the matchMode to be null, when the element is considered rendered with <controls>;
* see HTML-887. In that case, we default to MatchMode.ANYWHERE.
*/
if (matchMode == null) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default, the <obs style="provider_autocomplete" /> element is rendered with <input type='hidden' id='*_matchMode_hid' value='ANYWHERE' /> to indicate that the matchMode should be ANYWHERE. However, if the <obs> is conditionally rendered with <controls><when></controls>, the value of matchMode is reset to "". This gets around it by always defaulting matchMode to ANYWHERE if it's not specified.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I'm understanding it correctly, you discovered that match mode doesn't work when hidden within a controls/when?

Now, if you specific another match mode rather than "ANYWHERE" but within a controls/when, does it revert to "ANYWHERE", or is it only a problem in the "default" case?

If you've confirmed there's no easy way to fix this, I'm okay with this workaround, but let's add some of the notes above as a comment in the code (and add a quick note to the text on the limitations--ie don't use match mode within a when/then).

@chibongho chibongho Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. If I put <encounterProviderAndRole autocompleteProvider="true" providerMatchMode="START" /> inside a controls/when, the providerMatchMode value is erased. Without the providerMatchMode value, the server does not do provider filtering at all, and just returns the entire list of providers regardless of what's typed in the text box. So, the providerMatchMode value erasure aside, I think we should add this default value anyway.

I haven't actually implemented providerMatchMode for <obs style="provider_autocomplete">, so it always defaults to ANYWHERE.

I will:

  • document the providerMatchMode defaulting to ANYWHERE for provider_autocomplete
  • Add a comment explaining for why we need to default the matchMode value if it's not set
  • file a ticket for fixing the _matchMode_hid value being erased when rendering inside a controls/when. (this might be a bigger issue for <input type='hidden'> in general)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • documented the new style="provider_autocomplete" feature, and its behavior being equivalent to providerMatchMode=ANYWHERE, in here.
  • Updated with comment regarding the <input> value clearing bug
  • Filed ticket for <input> value clearing bug here.

matchMode = MatchMode.ANYWHERE;
}

List<ProviderStub> stubs;
if (searchParam == null) {
stubs = HtmlFormEntryUtil.getProviderStubs(providerList);
Expand Down