Merged
Conversation
…nused clock out resolution logic
…arningsService for earnings calculations and improve code clarity
…yService to utilize it
Test Results52 tests 52 ✅ 0s ⏱️ Results for commit 2446af4. |
There was a problem hiding this comment.
Pull request overview
This PR refactors earnings/workday calculation responsibilities by introducing MemberEarningsService (member-level payroll orchestration) and consolidating schedule/date/policy resolution helpers in WorkdayService, while replacing legacy calculator/service dependencies.
Changes:
- Introduced
MemberEarningsServiceto centralize payroll version lookup and daily/monthly earnings/minutes computation, delegating pure math toCompensationCalculator. - Refactored
WorkdayServiceto use unified month range + schedule resolution helpers and to separate displayed daily pay vs settlement aggregation logic. - Removed legacy
EarningsCalculatorand replaced/renamedSalaryCalculatorwithCompensationCalculator, updating/adding tests accordingly.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/kotlin/com/moa/service/MemberEarningsService.kt | New orchestrator service for payroll version selection + earnings/minutes calculations. |
| src/main/kotlin/com/moa/service/WorkdayService.kt | Replaces legacy calculators with MemberEarningsService and adds schedule/month/policy helper methods + display/settlement separation. |
| src/main/kotlin/com/moa/service/calculator/CompensationCalculator.kt | Renamed/refined pure calculation utility (daily rate, minutes, earnings, workdays in period). |
| src/main/kotlin/com/moa/service/calculator/EarningsCalculator.kt | Removed legacy service in favor of MemberEarningsService. |
| src/main/kotlin/com/moa/service/notification/NotificationMessageBuilder.kt | Switched earnings computation dependency to MemberEarningsService for clock-out notifications. |
| src/test/kotlin/com/moa/service/MemberEarningsServiceTest.kt | Updated/renamed tests to cover MemberEarningsService behaviors (0 handling, minutes helpers). |
| src/test/kotlin/com/moa/service/calculator/CompensationCalculatorTest.kt | Added tests for CompensationCalculator replacing the removed salary calculator tests. |
| src/test/kotlin/com/moa/service/calculator/SalaryCalculatorTest.kt | Removed legacy tests along with legacy calculator. |
Comments suppressed due to low confidence (1)
src/main/kotlin/com/moa/service/notification/NotificationMessageBuilder.kt:40
calculateTodayEarningsreturnsBigDecimal?and can returnnullwhen no policy exists, butbuildClockOutBodyno longer checks fornullbefore formatting. This can lead to formattingnull(and/or skipping the fallback body) when policy lookup fails. Either makecalculateTodayEarningsreturnBigDecimal(e.g.,BigDecimal.ZEROon missing policy) or restore thenullguard inbuildClockOutBody.
private fun buildClockOutBody(notification: NotificationLog): String {
val earnings = calculateTodayEarnings(notification.memberId, notification.scheduledDate)
if (earnings == BigDecimal.ZERO) {
return CLOCK_OUT_FALLBACK_BODY
}
val formatted = NumberFormat.getNumberInstance(Locale.KOREA).format(earnings)
return notification.notificationType.getBody(formatted)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…ulatorTest for improved clarity and consistency
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request refactors the earnings calculation logic in the
WorkdayServiceby introducing a newMemberEarningsServiceand consolidating date and schedule resolution helpers. The changes remove legacy calculator dependencies, unify schedule interpretation rules, and clarify the separation between display logic and settlement logic for daily earnings. The overall result is a cleaner, more maintainable codebase with improved modularity for payroll and workday calculations.Earnings Calculation Refactor
MemberEarningsServiceas a dedicated service for member-level payroll and earnings calculations, replacing direct usage ofEarningsCalculatorandSalaryCalculatorinWorkdayService. This new service centralizes monthly salary, daily earnings, and work minute calculations, delegating computation toCompensationCalculatorand managing which payroll data to use. (src/main/kotlin/com/moa/service/MemberEarningsService.kt)WorkdayServicewith a single dependency onMemberEarningsService, updating all monthly and daily earnings logic to use the new service. (src/main/kotlin/com/moa/service/WorkdayService.kt) [1] [2] [3]Schedule and Policy Resolution Helpers
src/main/kotlin/com/moa/service/WorkdayService.kt) [1] [2] [3] [4] [5] [6] [7] [8]Daily Earnings and Display Logic Separation
resolveDisplayedDailyPayandresolveCompletedWorkForSettlementhelpers, ensuring that UI and accounting logic use the correct calculation for each context. (src/main/kotlin/com/moa/service/WorkdayService.kt) [1] [2]Vacation Time Handling
resolveVacationTimes), simplifying and centralizing the logic for determining which times to use when saving vacation days. (src/main/kotlin/com/moa/service/WorkdayService.kt)Cleanup and Removal of Legacy Logic
resolveClockOutForEarnings, now handled by the new helpers and service, further simplifying the code and improving maintainability. (src/main/kotlin/com/moa/service/WorkdayService.kt)