Refactor vacation scheduling logic in WorkdayService to streamline input validation and improve time resolution#46
Conversation
…put validation and improve time resolution
Test Results47 tests 47 ✅ 0s ⏱️ Results for commit bae0caf. |
There was a problem hiding this comment.
Pull request overview
This PR refactors vacation schedule time resolution in WorkdayService by removing the dedicated validateVacationInput method and folding parts of that logic into resolveVacationTimes, with the goal of simplifying the vacation upsert flow.
Changes:
- Removed
validateVacationInputand updated the VACATION upsert path to call the newresolveVacationTimes(date, req, existingSchedule, policy). - Updated
resolveVacationTimesto optionally derive clock-in/clock-out from an existing saved schedule before falling back to policy defaults.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val schedule = resolveSchedule(savedSchedule, policy, date) | ||
| if (schedule.clockIn != null && schedule.clockOut != null) { | ||
| return schedule.clockIn to schedule.clockOut | ||
| } | ||
|
|
||
| return policy.clockInTime to policy.clockOutTime |
There was a problem hiding this comment.
resolveVacationTimes no longer enforces the validation that previously existed in validateVacationInput. With the current logic, if the request omits times, the method will (a) use savedSchedule times even when the saved schedule is a non-default/custom WORK schedule, and (b) fall back to policy times even when resolveSchedule(...) resolves to NONE (e.g., a non-workday). This is a concrete behavior change from the removed guard that threw BadRequestException(ErrorCode.INVALID_WORKDAY_INPUT) when the date was not the default work schedule. If that constraint is still required, reintroduce the equivalent check here (e.g., only allow implicit vacation times when the resolved schedule is default WORK with policy times, or when the existing saved schedule is already VACATION).
| val schedule = resolveSchedule(savedSchedule, policy, date) | |
| if (schedule.clockIn != null && schedule.clockOut != null) { | |
| return schedule.clockIn to schedule.clockOut | |
| } | |
| return policy.clockInTime to policy.clockOutTime | |
| if ( | |
| savedSchedule?.type == DailyWorkScheduleType.VACATION && | |
| savedSchedule.clockInTime != null && | |
| savedSchedule.clockOutTime != null | |
| ) { | |
| return savedSchedule.clockInTime to savedSchedule.clockOutTime | |
| } | |
| val schedule = resolveSchedule(savedSchedule, policy, date) | |
| if ( | |
| savedSchedule == null && | |
| schedule.type == DailyWorkScheduleType.WORK && | |
| schedule.clockIn != null && | |
| schedule.clockOut != null | |
| ) { | |
| return schedule.clockIn to schedule.clockOut | |
| } | |
| throw BadRequestException(ErrorCode.INVALID_WORKDAY_INPUT) |
There was a problem hiding this comment.
This behavior change is intentional.
We explicitly removed validateVacationInput so VACATION can be upserted even when the date is not a default workday or when a custom saved schedule already exists.
Time resolution now follows this order: request values, existing/resolved schedule values, then policy default times.
This pull request refactors how vacation schedule times are resolved in the
WorkdayService. The main change is the removal of thevalidateVacationInputmethod and integrating its logic directly into theresolveVacationTimesfunction, streamlining the handling of vacation workdays and reducing redundant validation.Vacation schedule handling:
validateVacationInputmethod, eliminating a separate validation step for vacation inputs and simplifying the code path for vacation scheduling.resolveVacationTimesmethod to takedate,req, andsavedScheduleas parameters, and incorporated logic to resolve the correct clock-in and clock-out times for vacation days, including using existing schedules if available. [1] [2] [3]These changes make the vacation scheduling logic more concise and easier to maintain.