From 5d1c5ae983aac12b0a1da329ba488f0f0a5352a3 Mon Sep 17 00:00:00 2001 From: Chi Bong Ho Date: Tue, 21 Jul 2026 18:37:54 -0400 Subject: [PATCH 1/2] drop support for all-day appointments in ScheduleAppointment tag --- .../ScheduleAppointmentTagTest.java | 149 ++++-------------- .../element/ScheduleAppointmentElement.java | 124 +++++---------- .../ScheduleAppointmentElementTest.java | 23 ++- 3 files changed, 86 insertions(+), 210 deletions(-) diff --git a/api-tests/src/test/java/org/openmrs/module/htmlformentry/ScheduleAppointmentTagTest.java b/api-tests/src/test/java/org/openmrs/module/htmlformentry/ScheduleAppointmentTagTest.java index 2e4edf6da..892658bf7 100644 --- a/api-tests/src/test/java/org/openmrs/module/htmlformentry/ScheduleAppointmentTagTest.java +++ b/api-tests/src/test/java/org/openmrs/module/htmlformentry/ScheduleAppointmentTagTest.java @@ -35,12 +35,14 @@ public class ScheduleAppointmentTagTest extends BaseHtmlFormEntryTest { // Text concept added in scheduleAppointmentTest.xml for storing appointment UUID obs private static final String APPT_UUID_CONCEPT = "c36006e5-9fbb-4f20-866b-0ece245615a7"; - // Widget-label note: date and time widgets are found via the "Timing" section - // label as a stable anchor. "Timing!!2" = allday hidden input (3rd name="w" after - // "Timing"), "Timing!!3" = appointment date hidden input (4th), "Timing!!4" = - // time-hours select (5th). Using "Date##N" labels is fragile because the shared - // html variable in getLabeledWidgets is mutated by ## processing, making - // consecutive "Date##N" label lookups interact unexpectedly. + // Widget-label note: the date/time widgets have no text label of their own that's safe to + // anchor on ("Date" also matches the earlier "Encounter Date:" widget, and the wrapper's CSS + // class name is also a substring of the "); @@ -332,28 +322,7 @@ public String generateHtml(FormEntryContext context) { .append("\" value=\"").append(typeWidget.getOptions().get(0).getValue()).append("\"/>"); } - // All-day / Specific-time toggle — shown BEFORE the date/time row - String allDayFieldName = context.getFieldName(allDayWidget); - sb.append("

"); - sb.append(""); - sb.append(""); - sb.append(""); - sb.append(""); - sb.append(""); - sb.append(""); - sb.append("

"); - - // Date + Time on one line; time portion shown/hidden by the toggle above + // Date + Time sb.append("

"); sb.append(""); @@ -367,13 +336,11 @@ public String generateHtml(FormEntryContext context) { sb.append(""); sb.append("

"); - // Duration (shown only for specific-time) - sb.append("
"); + // Duration sb.append(fieldRow("schedule-appointment-duration", msg("htmlformentry.scheduleAppointment.duration"), durationWidget.generateHtml(context) + durationErrorWidget.generateHtml(context), true)); - sb.append("
"); // Provider sb.append(fieldRow("schedule-appointment-provider", @@ -500,27 +467,22 @@ public Collection validateSubmission(FormEntryContext conte msg("htmlformentry.scheduleAppointment.error.dateRequired"))); } - // Time and duration required only for specific-time appointments - String allDayValue = (String) allDayWidget.getValue(context, submission); - boolean isAllDay = "allday".equals(allDayValue); - if (!isAllDay) { - Date time = (Date) startDateTimeWidget.getTimeWidget().getValue(context, submission); - if (time == null) { - errors.add(new FormSubmissionError(context.getFieldName(timeErrorWidget), - msg("htmlformentry.scheduleAppointment.error.timeRequired"))); - } + Date time = (Date) startDateTimeWidget.getTimeWidget().getValue(context, submission); + if (time == null) { + errors.add(new FormSubmissionError(context.getFieldName(timeErrorWidget), + msg("htmlformentry.scheduleAppointment.error.timeRequired"))); + } - Number durNum = null; - try { - durNum = (Number) durationWidget.getValue(context, submission); - } - catch (IllegalArgumentException e) { - // non-numeric input — falls through to error - } - if (durNum == null || durNum.intValue() < 1) { - errors.add(new FormSubmissionError(context.getFieldName(durationErrorWidget), - msg("htmlformentry.scheduleAppointment.error.durationRequired"))); - } + Number durNum = null; + try { + durNum = (Number) durationWidget.getValue(context, submission); + } + catch (IllegalArgumentException e) { + // non-numeric input — falls through to error + } + if (durNum == null || durNum.intValue() < 1) { + errors.add(new FormSubmissionError(context.getFieldName(durationErrorWidget), + msg("htmlformentry.scheduleAppointment.error.durationRequired"))); } Provider provider = (Provider) providerWidget.getValue(context, submission); @@ -549,8 +511,6 @@ public void handleSubmission(FormEntrySession session, HttpServletRequest submis String serviceUuid = (String) serviceWidget.getValue(context, submission); String typeValue = (String) typeWidget.getValue(context, submission); Date date = (Date) startDateTimeWidget.getDateWidget().getValue(context, submission); - String allDayValue = (String) allDayWidget.getValue(context, submission); - boolean isAllDay = "allday".equals(allDayValue); Provider selectedProvider = (Provider) providerWidget.getValue(context, submission); String note = (String) noteWidget.getValue(context, submission); @@ -560,44 +520,34 @@ public void handleSubmission(FormEntrySession session, HttpServletRequest submis // Build start datetime Calendar cal = Calendar.getInstance(); cal.setTime(date); - if (!isAllDay) { - Date time = (Date) startDateTimeWidget.getTimeWidget().getValue(context, submission); - if (time != null) { - Calendar timeCal = Calendar.getInstance(); - timeCal.setTime(time); - cal.set(Calendar.HOUR_OF_DAY, timeCal.get(Calendar.HOUR_OF_DAY)); - cal.set(Calendar.MINUTE, timeCal.get(Calendar.MINUTE)); - } - } else { - cal.set(Calendar.HOUR_OF_DAY, 0); - cal.set(Calendar.MINUTE, 0); + Date time = (Date) startDateTimeWidget.getTimeWidget().getValue(context, submission); + if (time != null) { + Calendar timeCal = Calendar.getInstance(); + timeCal.setTime(time); + cal.set(Calendar.HOUR_OF_DAY, timeCal.get(Calendar.HOUR_OF_DAY)); + cal.set(Calendar.MINUTE, timeCal.get(Calendar.MINUTE)); } cal.set(Calendar.SECOND, 0); cal.set(Calendar.MILLISECOND, 0); Date startDateTime = cal.getTime(); // Build end datetime - Date endDateTime; - if (isAllDay) { - endDateTime = OpenmrsUtil.getLastMomentOfDay(date); - } else { - Integer durationMins = null; - try { - Number durNum = (Number) durationWidget.getValue(context, submission); - if (durNum != null) { - durationMins = durNum.intValue(); - } - } - catch (IllegalArgumentException e) { - // non-numeric input — validation should have caught this - } - if (durationMins == null && service != null) { - durationMins = service.getDurationMins(); + Integer durationMins = null; + try { + Number durNum = (Number) durationWidget.getValue(context, submission); + if (durNum != null) { + durationMins = durNum.intValue(); } - endDateTime = durationMins != null - ? new Date(startDateTime.getTime() + (long) durationMins * 60 * 1000) - : null; } + catch (IllegalArgumentException e) { + // non-numeric input — validation should have caught this + } + if (durationMins == null && service != null) { + durationMins = service.getDurationMins(); + } + Date endDateTime = durationMins != null + ? new Date(startDateTime.getTime() + (long) durationMins * 60 * 1000) + : null; Appointment appointment = new Appointment(); appointment.setPatient(session.getPatient()); diff --git a/api/src/test/java/org/openmrs/module/htmlformentry/element/ScheduleAppointmentElementTest.java b/api/src/test/java/org/openmrs/module/htmlformentry/element/ScheduleAppointmentElementTest.java index be62604d3..ce8375a2d 100644 --- a/api/src/test/java/org/openmrs/module/htmlformentry/element/ScheduleAppointmentElementTest.java +++ b/api/src/test/java/org/openmrs/module/htmlformentry/element/ScheduleAppointmentElementTest.java @@ -14,7 +14,9 @@ import static org.mockito.Mockito.when; import javax.servlet.http.HttpServletRequest; +import java.text.SimpleDateFormat; import java.util.Collections; +import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -104,10 +106,28 @@ public void setup() throws Exception { mockedHtmlFormEntryUtil.when(() -> HtmlFormEntryUtil.getConcept(CONCEPT_ID)).thenReturn(concept); mockedHtmlFormEntryUtil.when(() -> HtmlFormEntryUtil.createObs(any(Concept.class), any(), any(), any())) .thenCallRealMethod(); + // real conversion logic per requested type, since different widgets request different types + // (DateWidget wants a Date, TimeWidget wants Integers) and a blanket canned value breaks whichever + // widget didn't ask for that type mockedHtmlFormEntryUtil .when(() -> HtmlFormEntryUtil.getParameterAsType(any(HttpServletRequest.class), anyString(), any(Class.class))) - .thenReturn(new java.util.Date()); + .thenAnswer(invocation -> { + HttpServletRequest req = invocation.getArgument(0); + String name = invocation.getArgument(1); + Class clazz = invocation.getArgument(2); + String val = req.getParameter(name); + if (val == null) { + return null; + } + if (Date.class.isAssignableFrom(clazz)) { + return new SimpleDateFormat("yyyy-MM-dd").parse(val); + } + if (Integer.class.isAssignableFrom(clazz)) { + return Integer.valueOf(val); + } + return val; + }); mockedContext.when(Context::getAdministrationService).thenReturn(administrationService); mockedContext.when(Context::getMessageSourceService).thenReturn(messageSourceService); @@ -240,7 +260,6 @@ private MockHttpServletRequest buildRequest() { MockHttpServletRequest req = new MockHttpServletRequest(); req.addParameter("dd", AppointmentKind.Scheduled.name()); // all DropdownWidgets share "dd"; value must be a valid AppointmentKind for the type widget - req.addParameter("tf", "allday"); // allDayWidget → all-day, no time widget needed req.addParameter("dt", "2025-01-15"); // date (parsed as yyyy-MM-dd by HtmlFormEntryUtil) // prov_hid not set → provider = null (appointment is created without provider) return req; From 3964a921c9f45d30d880dcb30b37fdbc59a3a6bb Mon Sep 17 00:00:00 2001 From: Chi Bong Ho Date: Thu, 23 Jul 2026 08:47:27 -0400 Subject: [PATCH 2/2] address feedback --- .../ScheduleAppointmentTagTest.java | 80 +++++++++++++++++++ api/src/main/resources/messages.properties | 2 - api/src/main/resources/messages_es.properties | 2 - api/src/main/resources/messages_fr.properties | 2 - api/src/main/resources/messages_ht.properties | 2 - 5 files changed, 80 insertions(+), 8 deletions(-) diff --git a/api-tests/src/test/java/org/openmrs/module/htmlformentry/ScheduleAppointmentTagTest.java b/api-tests/src/test/java/org/openmrs/module/htmlformentry/ScheduleAppointmentTagTest.java index 892658bf7..2ab42cd1a 100644 --- a/api-tests/src/test/java/org/openmrs/module/htmlformentry/ScheduleAppointmentTagTest.java +++ b/api-tests/src/test/java/org/openmrs/module/htmlformentry/ScheduleAppointmentTagTest.java @@ -543,6 +543,86 @@ public void testResults(SubmissionResults results) { }.run(); } + @Test + public void scheduleAppointmentTag_shouldRequireTimeInline() throws Exception { + new RegressionTestHelper() { + + @Override + public String getFormName() { + return "scheduleAppointmentForm"; + } + + @Override + public String[] widgetLabels() { + return new String[] { "Encounter Date:", "Location", "Service", "Appointment Type", + DATE_TIME_ANCHOR, DATE_TIME_ANCHOR + "!!1", "Duration (minutes)", "Provider" }; + } + + @Override + public void setupRequest(MockHttpServletRequest request, Map widgets) { + request.setParameter(widgets.get("Encounter Date:"), "2025-01-15"); + request.setParameter(widgets.get("Location"), KIGALI_UUID); + request.setParameter(widgets.get("Service"), CONSULTATION_UUID); + request.setParameter(widgets.get("Appointment Type"), AppointmentKind.Scheduled.name()); + request.setParameter(widgets.get(DATE_TIME_ANCHOR), "2025-01-15"); + // Time intentionally omitted + + request.setParameter(widgets.get("Duration (minutes)"), "30"); + request.setParameter(widgets.get("Provider") + "_hid", PROVIDER_ID); + } + + @Override + public void testResults(SubmissionResults results) { + results.assertErrors(); + Assert.assertTrue("Expected inline 'Appointment time is required' error", + results.getValidationErrors().stream() + .anyMatch(e -> e.getError().contains("Appointment time is required"))); + } + }.run(); + } + + @Test + public void scheduleAppointmentTag_shouldRequireDurationInline() throws Exception { + new RegressionTestHelper() { + + @Override + public String getFormName() { + return "scheduleAppointmentForm"; + } + + @Override + public String[] widgetLabels() { + return new String[] { "Encounter Date:", "Location", "Service", "Appointment Type", + DATE_TIME_ANCHOR, DATE_TIME_ANCHOR + "!!1", "Duration (minutes)", "Provider" }; + } + + @Override + public void setupRequest(MockHttpServletRequest request, Map widgets) { + request.setParameter(widgets.get("Encounter Date:"), "2025-01-15"); + request.setParameter(widgets.get("Location"), KIGALI_UUID); + request.setParameter(widgets.get("Service"), CONSULTATION_UUID); + request.setParameter(widgets.get("Appointment Type"), AppointmentKind.Scheduled.name()); + request.setParameter(widgets.get(DATE_TIME_ANCHOR), "2025-01-15"); + + String timeBase = widgets.get(DATE_TIME_ANCHOR + "!!1").replace("hours", ""); + request.setParameter(timeBase + "hours", "10"); + request.setParameter(timeBase + "minutes", "30"); + request.setParameter(timeBase + "seconds", "0"); + + // Duration intentionally omitted + request.setParameter(widgets.get("Provider") + "_hid", PROVIDER_ID); + } + + @Override + public void testResults(SubmissionResults results) { + results.assertErrors(); + Assert.assertTrue("Expected inline 'Duration is required' error", + results.getValidationErrors().stream() + .anyMatch(e -> e.getError().contains("Duration is required"))); + } + }.run(); + } + // --------------------------------------------------------------- // Optional tag tests // --------------------------------------------------------------- diff --git a/api/src/main/resources/messages.properties b/api/src/main/resources/messages.properties index 454adb61c..b04543b63 100644 --- a/api/src/main/resources/messages.properties +++ b/api/src/main/resources/messages.properties @@ -195,9 +195,7 @@ htmlformentry.scheduleAppointment.service=Service htmlformentry.scheduleAppointment.appointmentType=Appointment Type htmlformentry.scheduleAppointment.date=Date htmlformentry.scheduleAppointment.appointmentTime=Time -htmlformentry.scheduleAppointment.timing=Timing htmlformentry.scheduleAppointment.allDay=All day -htmlformentry.scheduleAppointment.specificTime=Specific time htmlformentry.scheduleAppointment.duration=Duration (minutes) htmlformentry.scheduleAppointment.provider=Provider htmlformentry.scheduleAppointment.note=Note diff --git a/api/src/main/resources/messages_es.properties b/api/src/main/resources/messages_es.properties index 06408fd42..a0ad25efd 100644 --- a/api/src/main/resources/messages_es.properties +++ b/api/src/main/resources/messages_es.properties @@ -186,9 +186,7 @@ htmlformentry.scheduleAppointment.service=Servicio htmlformentry.scheduleAppointment.appointmentType=Tipo de cita htmlformentry.scheduleAppointment.date=Fecha htmlformentry.scheduleAppointment.appointmentTime=Hora -htmlformentry.scheduleAppointment.timing=Horario htmlformentry.scheduleAppointment.allDay=Todo el día -htmlformentry.scheduleAppointment.specificTime=Hora específica htmlformentry.scheduleAppointment.duration=Duración (minutos) htmlformentry.scheduleAppointment.provider=Proveedor htmlformentry.scheduleAppointment.note=Nota diff --git a/api/src/main/resources/messages_fr.properties b/api/src/main/resources/messages_fr.properties index 8a3c93cc1..b4c1a6a52 100644 --- a/api/src/main/resources/messages_fr.properties +++ b/api/src/main/resources/messages_fr.properties @@ -186,9 +186,7 @@ htmlformentry.scheduleAppointment.service=Service htmlformentry.scheduleAppointment.appointmentType=Type de rendez-vous htmlformentry.scheduleAppointment.date=Date htmlformentry.scheduleAppointment.appointmentTime=Heure -htmlformentry.scheduleAppointment.timing=Horaire htmlformentry.scheduleAppointment.allDay=Toute la journée -htmlformentry.scheduleAppointment.specificTime=Heure précise htmlformentry.scheduleAppointment.duration=Durée (minutes) htmlformentry.scheduleAppointment.provider=Prestataire de soins htmlformentry.scheduleAppointment.note=Note diff --git a/api/src/main/resources/messages_ht.properties b/api/src/main/resources/messages_ht.properties index 333b2b88c..60f3d1177 100644 --- a/api/src/main/resources/messages_ht.properties +++ b/api/src/main/resources/messages_ht.properties @@ -186,9 +186,7 @@ htmlformentry.scheduleAppointment.service=Sèvis htmlformentry.scheduleAppointment.appointmentType=Kalite randevou htmlformentry.scheduleAppointment.date=Dat htmlformentry.scheduleAppointment.appointmentTime=Lè -htmlformentry.scheduleAppointment.timing=Orè htmlformentry.scheduleAppointment.allDay=Tout jounen an -htmlformentry.scheduleAppointment.specificTime=Lè presi htmlformentry.scheduleAppointment.duration=Kantite tan (minit) htmlformentry.scheduleAppointment.provider=Moun ki bay swen htmlformentry.scheduleAppointment.note=Nòt