Adds support for managing and handling public holidays#47
Conversation
Test Results53 tests 53 ✅ 0s ⏱️ Results for commit 0359e8a. |
There was a problem hiding this comment.
Pull request overview
Adds first-class “public holiday” support across scheduling, earnings calculation, daily events, and notifications, including an admin API to manage holiday dates.
Changes:
- Introduces
PublicHolidaydomain (entity/repository/service) and admin controller endpoints to create/list/delete holidays. - Integrates public holidays into workday resolution and compensation calculations (standard minutes, workday counts, daily rate).
- Adds
PUBLIC_HOLIDAYto daily events and notification flows, including batch generation and dispatch-time earnings message calculation.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/kotlin/com/moa/entity/PublicHoliday.kt | New entity for persisted public holiday dates/names. |
| src/main/kotlin/com/moa/repository/PublicHolidayRepository.kt | Repository queries for holiday existence and date ranges. |
| src/main/kotlin/com/moa/service/PublicHolidayService.kt | CRUD + month/year holiday date retrieval helpers. |
| src/main/kotlin/com/moa/controller/PublicHolidayController.kt | Admin API for managing public holidays. |
| src/main/kotlin/com/moa/service/WorkdayService.kt | Excludes holidays from inferred schedules and includes holiday events/earnings adjustments. |
| src/main/kotlin/com/moa/service/calculator/CompensationCalculator.kt | Adds publicHolidays parameter to exclude holidays from workday counts and derived rates/minutes. |
| src/main/kotlin/com/moa/entity/DailyEventType.kt | Adds PUBLIC_HOLIDAY event and resolves it from provided holiday set. |
| src/main/kotlin/com/moa/entity/notification/NotificationType.kt | Adds PUBLIC_HOLIDAY notification type. |
| src/main/kotlin/com/moa/service/notification/NotificationBatchService.kt | Generates PUBLIC_HOLIDAY notifications on holiday dates; avoids work notifications. |
| src/main/kotlin/com/moa/service/notification/NotificationDispatchService.kt | Fetches per-month holiday sets and passes them into message building. |
| src/main/kotlin/com/moa/service/notification/NotificationMessageBuilder.kt | Requires holiday set to compute CLOCK_OUT earnings correctly with holiday-adjusted rates. |
| src/main/kotlin/com/moa/service/notification/NotificationEarningsService.kt | Threads holiday set into earnings calculation used by notifications. |
| src/main/kotlin/com/moa/service/notification/NotificationSyncService.kt | Excludes PUBLIC_HOLIDAY from “work notification” sync logic. |
| src/main/kotlin/com/moa/service/MemberEarningsService.kt | Adds optional publicHolidays parameter to keep calculations consistent where used. |
| src/main/kotlin/com/moa/service/dto/PublicHolidayCreateRequest.kt | Request DTO for creating holidays. |
| src/main/kotlin/com/moa/service/dto/PublicHolidayResponse.kt | Response DTO for returning holiday records. |
| src/test/kotlin/com/moa/service/calculator/CompensationCalculatorTest.kt | Adds tests for holiday-adjusted workday counts and daily rate changes. |
| src/test/kotlin/com/moa/entity/DailyEventTypeTest.kt | Updates resolve signature and adds tests for PUBLIC_HOLIDAY event behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -45,6 +91,8 @@ class NotificationBatchService( | |||
| private fun isAlreadyGenerated(date: LocalDate): Boolean { | |||
| val exists = notificationLogRepository | |||
| .existsByScheduledDateAndNotificationType(date, NotificationType.CLOCK_IN) | |||
| || notificationLogRepository | |||
| .existsByScheduledDateAndNotificationType(date, NotificationType.PUBLIC_HOLIDAY) | |||
| if (exists) log.info("Notifications already generated for date: {}", date) | |||
| return exists | |||
| } | |||
There was a problem hiding this comment.
isAlreadyGenerated returns true if either CLOCK_IN or PUBLIC_HOLIDAY logs exist for the date, which can incorrectly block generating PUBLIC_HOLIDAY logs on a holiday if CLOCK_IN logs already exist (e.g., created before the holiday was registered), and vice versa. Consider branching the existence check based on publicHolidayService.isHoliday(date) (or checking only the target types you’re about to generate) so holiday/work generation don’t block each other unintentionally.
There was a problem hiding this comment.
공휴일 정보는 미리 넣어둘 것이고
임의로 삭제하는 일을 없을 것 같아서 넘어 감
| fun delete(@PathVariable id: Long) = | ||
| ApiResponse.success(publicHolidayService.delete(id)) |
There was a problem hiding this comment.
delete currently wraps a Unit return value in ApiResponse.success(publicHolidayService.delete(id)), which will set content to a Unit instance rather than returning an empty success response. Prefer calling publicHolidayService.delete(id) and then returning ApiResponse.success() so the response body is consistent with other void endpoints.
| fun delete(@PathVariable id: Long) = | |
| ApiResponse.success(publicHolidayService.delete(id)) | |
| fun delete(@PathVariable id: Long) { | |
| publicHolidayService.delete(id) | |
| return ApiResponse.success() | |
| } |
There was a problem hiding this comment.
ㅜ 어드민이라 ㄱㅊ을 거 같은데 파일럿 너가 싫다면 고민좀 해볼게
| @Entity | ||
| @Table(name = "public_holiday") | ||
| class PublicHoliday( | ||
| @Column(nullable = false) | ||
| val date: LocalDate, | ||
|
|
||
| @Column(nullable = false, length = 50) | ||
| val name: String, | ||
| ) : BaseEntity() { |
There was a problem hiding this comment.
PublicHoliday appears to represent a single holiday per calendar date, but the table definition doesn’t enforce uniqueness on date. Without a unique constraint (and/or a pre-save check), duplicate rows for the same date can be created, making management and deletions ambiguous (one delete won’t necessarily remove the holiday). Consider adding a unique constraint on date (and handling duplicate creates gracefully).
There was a problem hiding this comment.
의도적으로 제거한거임
개천절 + 추석 등 하루에 겹칠 수 있어서
그래서 무급 계산할 때 중복 계산 방지하기 위해서 Set<LocalDate> 중복 제거 필수
공휴일 무급 처리 기능공휴일을 무급(NONE) 기반으로 추가합니다. 무급 처리란?공휴일 당일은 비근무일(NONE)로 처리되어 예시: 월급 300만원, 5월 변경 전 (공휴일 없음): 근무일 22일, 일급 = 300만 ÷ 22 = 136,364원 어린이날 dailyPay = 0원 공휴일에 출근하면? — 추가 소득 발생공휴일은 기본적으로 NONE(비근무)이지만, 사용자가 직접 WORK로 변경할 수 있습니다. 이 경우 근무일수 계산에서 이미 공휴일이 차감되어 일급이 높아진 상태에서 해당 날에 추가로 소득이 발생하므로, 월 합계가 기준 월급을 초과합니다. 예시: 월급 300만원, 5월 어린이날을 WORK로 변경 근무일수 계산: 22일 - 1일(공휴일) = 21일 일반 근무 21일 × 142,857원 = 2,999,997원
공휴일 출근은 추가 근무에 대한 보상이라는 개념이며, 이것은 의도된 정책. (나중에 변경될 수 있음) 스케줄 결정 우선순위
Admin API — 공휴일 CRUD
알림 처리
|
jeyongsong
changed the title
This pull request adds support for managing and handling public holidays throughout the system. It introduces a new public holiday entity, controller, and service, integrates public holiday logic into workday and earnings calculations, and updates event and notification types to reflect public holidays. The main changes are grouped below.
Public Holiday Management:
PublicHolidayentity and corresponding JPA repository for storing public holidays. [1] [2]PublicHolidayServicefor CRUD operations and holiday date queries, andPublicHolidayControllerfor admin API endpoints to manage public holidays. [1] [2]Integration with Workday and Earnings Logic:
WorkdayServiceandCompensationCalculatorto fetch public holidays and exclude them from work schedules and standard workday/minute calculations. Public holidays are now considered when resolving schedules, calculating daily earnings, and generating workday responses. [1] [2] [3] [4] [5] [6] [7] [8] [9]Event and Notification Enhancements:
PUBLIC_HOLIDAYto bothDailyEventTypeandNotificationType, allowing the system to display and notify users about public holidays. [1] [2]These changes ensure that public holidays are centrally managed and consistently respected across scheduling, payroll, and user notifications.