Skip to content

HTML-882: drop support for all-day appointments in ScheduleAppointment tag - #344

Merged
chibongho merged 3 commits into
masterfrom
HTML-882
Jul 23, 2026
Merged

chibongho merged 3 commits into
masterfrom
HTML-882

Conversation

@chibongho

Copy link
Copy Markdown
Contributor

"All-day Appointment" is not well-supported by the appointments module, despite that being an option in the O3 appointments app (via config). This PR removes the ability to schedule all-day appointments in the <scheduleAppointments> element, since we don't need it anyway.

@chibongho
chibongho requested review from cioan, dkayiwa, mogoodrich and mseaton and removed request for mseaton July 22, 2026 20:38
// All-day / Specific-time toggle — shown BEFORE the date/time row
String allDayFieldName = context.getFieldName(allDayWidget);
sb.append("<p class=\"schedule-appointment-timing\">");
sb.append("<label>").append(msg("htmlformentry.scheduleAppointment.timing"))

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.

With the toggle gone, the htmlformentry.scheduleAppointment.timing and htmlformentry.scheduleAppointment.specificTime keys aren't referenced from anywhere anymore, but they're still in messages.properties and the es/fr/ht bundles. Worth dropping them in this PR so the dead strings don't stick around in the translation files. (allDay is still used by the view-mode rendering, so that one stays.)

msg("htmlformentry.scheduleAppointment.error.timeRequired")));
}
Date time = (Date) startDateTimeWidget.getTimeWidget().getValue(context, submission);
if (time == null) {

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.

Could we add a validation test that omits the time selects (and one omitting duration), along the lines of shouldRequireServiceInlineNotAsGlobalError? Making time and duration unconditionally required is the one behavior this PR changes, and every submission test in the suite supplies both, so nothing would fail if the requirement quietly regressed. Not a blocker.

@chibongho

Copy link
Copy Markdown
Contributor Author

Added a couple tests on the time and duration fields, and removed the unused translation strings.

@chibongho
chibongho requested a review from dkayiwa July 23, 2026 13:01
@chibongho
chibongho merged commit 01d74ec into master Jul 23, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants