Feature/bu 206: 급여 로직 수정#127
Conversation
- PayrollCalculationInput: 급여 계산에 필요한 모든 입력 정보 통합 - OvertimeConditions: 가산수당 발생/지급 조건 판정 정보 - InsuranceEligibility: 4대보험 가입 여부 정보 - InsuranceRates: 4대보험 요율 정보 (2025년 기준 기본값 포함) v1.1 급여 계산 로직 구현을 위한 기반 DTO 구조 확립
- PayrollCalculationResult: 급여 계산 결과를 담는 통합 DTO
* 지급 항목: 기본급, 연장수당, 야간수당, 휴일수당, 주휴수당, 총지급액
* 공제 항목: 소득세, 지방소득세, 고용보험, 건강보험, 장기요양보험, 국민연금, 총공제액
* 최종 금액: 실지급액
* 계산 헬퍼 메서드: calculateTotalPay(), calculateTotalDeduction(), calculateNetPay()
* 디버깅용 계산 상세 정보 필드 포함
v1.1 급여 계산 로직의 출력 구조 확립
PayrollCalculator에 v1.1 급여 계산 통합 메서드 추가
- calculate(PayrollCalculationInput): 메인 계산 메서드
* 기본급 계산: 시급 × (1일 소정시간 × 근로일수)
* 가산수당 지급 가능 여부 판정 (5인 이상/미만 분기)
* 조건 미충족 시 가산시간 무효화
* 중복가산 규칙 반영 (연장 1.5배, 야간 +0.5배, 휴일 1.5배)
* 주휴수당 (일용직 조건 충족 시)
* 4대보험 (일용직/상용직 분기 처리)
- 일용직: 월 근로일수 8일 이상, 월소득 220만원 이상 조건
- 상용직: 항상 적용
* 세금 (일용직/상용직 분기 처리)
- 일용직: 일별 계산, 15만원 공제, 6% 세율, 55% 세액공제, 1천원 미만 면제
- 상용직: 간이세액표 (현재 3.3% 적용)
* 실지급액 = 총지급액 - 보험 - 세금
- v1.1 급여 계산 로직 통합 (PayrollCalculationInput/Result 사용) - 가산수당 발생 조건 판단 로직 적용 - 4대보험 및 세금 계산을 v1.1 메서드로 통합 - 기본값 적용: totalEmployees=5, autoPayOvertime/Night/Holiday=true - 기존 로직 유지: 비과세 소득, 산재보험 계산 - Controller/Response API는 변경 없음 (DTO 기반 내부 로직만 변경) Refs: BU-XXX
- 일용직 4대보험 가입 조건 수정: 월 8일 미만 시 건강보험/국민연금 미가입 처리 - 일용직 4대보험 계산 기준을 총지급액 기준으로 변경 - v1.1 통합 급여 계산 테스트 케이스 추가 - 일용직 일급 기본 케이스 - 일용직 일급 세금 발생 케이스 - 가산수당 포함 케이스 - 5인 미만 사업장 가산수당 지급/미지급 케이스 - 일용직 월급 4대보험 가입/미가입 케이스 - 상용직 월급 4대보험 전체 적용 케이스 - 휴일근로 + 야간근로 중복가산 케이스
Walkthrough급여 계산용 DTO 5개 추가(입력/결과 포함), PayrollCalculator에 통합된 calculate(PayrollCalculationInput) 메서드 추가, SalaryGenerationService가 새 계산 흐름으로 리팩토링되었으며 관련 단위 테스트와 PDF 템플릿 소폭 조정이 포함됨. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SalaryService as SalaryGenerationService
participant Calculator as PayrollCalculator
participant CalcLogic as CalculationLogic
participant ResultDTO as PayrollCalculationResult
participant Storage as PDF/S3
Client->>SalaryService: 급여 생성 요청
SalaryService->>SalaryService: PayrollCalculationInput 생성
SalaryService->>Calculator: calculate(input)
Calculator->>CalcLogic: 기본급 및 수당 계산
Calculator->>CalcLogic: 수당 적격성 판단 (OT/야간/휴일)
alt 일급
Calculator->>CalcLogic: 일급용 보험/세금 계산
else 정직원
Calculator->>CalcLogic: 정직원용 보험/세금 계산
end
Calculator->>CalcLogic: 총공제 및 순급여 계산
Calculator-->>ResultDTO: PayrollCalculationResult 반환
SalaryService->>SalaryService: 결과에서 항목 추출 및 Payslip 생성
SalaryService->>Storage: PDF 생성 및 S3 업로드
SalaryService-->>Client: 처리 완료 응답
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분
Possibly related PRs
Suggested reviewers
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
src/main/java/com/concrete/buildup/domain/payroll/dto/PayrollCalculationResult.java (1)
86-94:totalPay필드와calculateTotalPay()메서드 간의 잠재적 불일치
totalPay필드가 Builder를 통해 설정된 후,calculateTotalPay()메서드가 개별 지급 항목들을 재계산하여 다른 값을 반환할 수 있습니다. 이로 인해getTotalPay()와calculateTotalPay()가 서로 다른 값을 반환하는 혼란이 발생할 수 있습니다.일관성을 위해 다음 중 하나를 고려해 보세요:
totalPay필드를 제거하고 항상calculateTotalPay()로 계산- 또는
calculateTotalPay()를 제거하고totalPay필드만 사용src/main/java/com/concrete/buildup/domain/payroll/dto/InsuranceEligibility.java (1)
23-34: 장기요양보험 가입 여부 필드 누락 확인
InsuranceRates에는longTermCare요율이 포함되어 있지만,InsuranceEligibility에는 장기요양보험 가입 여부 필드가 없습니다. 장기요양보험은 건강보험 가입자에게 자동 적용되므로 별도 필드가 불필요할 수 있지만, 명시적으로 문서화하거나 필드를 추가하는 것을 고려해 보세요.src/main/java/com/concrete/buildup/domain/payroll/service/SalaryGenerationService.java (2)
368-369:dailyWorkHours가 8시간으로 하드코딩됨계약서에 소정근로시간이 다르게 설정되어 있을 수 있습니다.
contractDetail에서 실제 일일 근무시간을 가져오는 것이 정확합니다.- .dailyWorkHours(new BigDecimal("8")) // 기본 일 근무시간 (계약상 기준) + .dailyWorkHours(contractDetail.getDailyWorkHours() != null + ? contractDetail.getDailyWorkHours() + : new BigDecimal("8"))
382-390:totalEmployees가 5로 하드코딩되어 항상 가산수당 의무 지급 사업장으로 처리됨5인 미만 사업장은 가산수당 의무가 없으므로, 실제 사업장의 상시근로자 수를 사용해야 정확한 계산이 가능합니다.
향후
Corporation엔티티에서 상시근로자 수를 조회하여 적용하는 것을 권장합니다.src/main/java/com/concrete/buildup/domain/payroll/dto/PayrollCalculationInput.java (1)
37-44: 필수 필드에 대한 Bean Validation 어노테이션 고려
hourlyRate,dailyWorkHours,workDays는 필수 필드로required = true가 명시되어 있으나, 실제 Bean Validation 어노테이션이 없습니다. 런타임 안정성을 위해@NotNull등 추가를 고려할 수 있습니다.+import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Positive; - @Schema(description = "시급", example = "10000", required = true) + @NotNull + @Positive + @Schema(description = "시급", example = "10000", required = true) private BigDecimal hourlyRate;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/com/concrete/buildup/domain/payroll/dto/InsuranceEligibility.java(1 hunks)src/main/java/com/concrete/buildup/domain/payroll/dto/InsuranceRates.java(1 hunks)src/main/java/com/concrete/buildup/domain/payroll/dto/OvertimeConditions.java(1 hunks)src/main/java/com/concrete/buildup/domain/payroll/dto/PayrollCalculationInput.java(1 hunks)src/main/java/com/concrete/buildup/domain/payroll/dto/PayrollCalculationResult.java(1 hunks)src/main/java/com/concrete/buildup/domain/payroll/service/SalaryGenerationService.java(2 hunks)src/main/java/com/concrete/buildup/domain/payroll/util/PayrollCalculator.java(2 hunks)src/main/resources/templates/contract/contract-pdf.html(1 hunks)src/test/java/com/concrete/buildup/domain/payroll/util/PayrollCalculatorTest.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/main/java/com/concrete/buildup/domain/payroll/dto/InsuranceRates.java (2)
src/main/java/com/concrete/buildup/domain/payroll/dto/InsuranceEligibility.java (1)
Getter(18-35)src/main/java/com/concrete/buildup/domain/payroll/dto/PayrollCalculationInput.java (1)
Getter(22-89)
src/main/java/com/concrete/buildup/domain/payroll/dto/PayrollCalculationInput.java (4)
src/main/java/com/concrete/buildup/domain/payroll/dto/InsuranceEligibility.java (1)
Getter(18-35)src/main/java/com/concrete/buildup/domain/payroll/dto/InsuranceRates.java (1)
Getter(20-54)src/main/java/com/concrete/buildup/domain/payroll/dto/OvertimeConditions.java (1)
Getter(18-56)src/main/java/com/concrete/buildup/global/util/MaskingUtil.java (1)
NoArgsConstructor(15-154)
src/main/java/com/concrete/buildup/domain/payroll/dto/InsuranceEligibility.java (2)
src/main/java/com/concrete/buildup/domain/payroll/dto/InsuranceRates.java (1)
Getter(20-54)src/main/java/com/concrete/buildup/domain/payroll/dto/PayrollCalculationInput.java (1)
Getter(22-89)
src/main/java/com/concrete/buildup/domain/payroll/dto/OvertimeConditions.java (4)
src/main/java/com/concrete/buildup/domain/payroll/dto/InsuranceEligibility.java (1)
Getter(18-35)src/main/java/com/concrete/buildup/domain/payroll/dto/InsuranceRates.java (1)
Getter(20-54)src/main/java/com/concrete/buildup/domain/payroll/dto/PayrollCalculationInput.java (1)
Getter(22-89)src/main/java/com/concrete/buildup/global/util/MaskingUtil.java (1)
NoArgsConstructor(15-154)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
🔇 Additional comments (16)
src/main/resources/templates/contract/contract-pdf.html (1)
55-58: LGTM!서명 영역의 상단 여백 조정은 적절해 보입니다. PDF 레이아웃 개선을 위한 변경으로 이해됩니다.
src/main/java/com/concrete/buildup/domain/payroll/dto/InsuranceRates.java (1)
11-25: LGTM!DTO 구조와 문서화가 잘 되어 있습니다. Lombok과 Swagger 어노테이션 사용이 프로젝트 컨벤션과 일관됩니다.
src/main/java/com/concrete/buildup/domain/payroll/dto/PayrollCalculationResult.java (1)
117-121: LGTM - null 안전 처리가 적절합니다.
calculateNetPay()메서드에서 필드 값 우선, null인 경우 재계산하는 로직이 유연성을 제공합니다. 다만, 위에서 언급한 필드와 계산 메서드 간의 일관성 문제를 함께 고려해 주세요.src/test/java/com/concrete/buildup/domain/payroll/util/PayrollCalculatorTest.java (5)
405-407: LGTM - v1.1 통합 테스트 구조가 잘 구성되었습니다.
@Nested클래스를 사용하여 v1.1 테스트를 논리적으로 그룹화한 것이 좋습니다. PR 목표에 맞는 다양한 시나리오(일용직 일급/월급, 상용직, 소규모 사업장, 가산수당 중복 등)를 포함하고 있습니다.
733-776: 핵심 버그 수정 검증 테스트 - LGTM!PR 목표에 명시된 "월 8일 미만 일용직 4대보험 미가입" 조건을 정확히 검증하고 있습니다.
insurance필드에서 가입 여부가true로 설정되어 있더라도, 월 근로일수가 8일 미만이면 실제 공제액이 0원이 되는 것을 확인하는 중요한 테스트입니다.
568-572: 야간수당 계산 방식 확인 필요v1.1 통합 계산에서 야간수당이
시급 × 시간 × 0.5(가산분만)로 계산되고 있습니다. 그러나 기존calculateNightWorkPay메서드 테스트(60-71라인)에서는시급 × 시간 × 1.5로 계산됩니다.이 차이가 의도된 것인지 확인해 주세요:
- 야간근로가 기본급에 포함된 시간이면 0.5 가산만 적용 (현재 v1.1 방식)
- 야간근로가 별도 추가 시간이면 1.5 적용 (기존 방식)
853-903: LGTM - 휴일·야간 중복가산 테스트근로기준법상 휴일근로와 야간근로가 중복되는 경우 각 가산율이 합산 적용되는 것을 정확히 검증하고 있습니다. 이는 중요한 엣지 케이스로, 테스트 커버리지 확장에 기여합니다.
778-851: LGTM - 상용직 월급 4대보험 전체 적용 테스트상용직 시나리오에서 기본급, 주휴수당, 세금, 4대보험이 모두 정확히 계산되는지 포괄적으로 검증하고 있습니다. 실수령액까지 수동 계산하여 검증하는 방식이 신뢰성을 높여줍니다.
src/main/java/com/concrete/buildup/domain/payroll/dto/InsuranceEligibility.java (1)
9-22: LGTM!DTO 구조가 명확하고 문서화가 잘 되어 있습니다. 산재보험 제외 사유가 주석으로 잘 설명되어 있습니다.
src/main/java/com/concrete/buildup/domain/payroll/dto/OvertimeConditions.java (1)
1-56: LGTM - DTO 구조가 명확하게 설계됨가산수당 지급 조건을 캡슐화하는 DTO가 잘 설계되었습니다. Javadoc과 Swagger 어노테이션이 적절히 문서화되어 있습니다.
src/main/java/com/concrete/buildup/domain/payroll/service/SalaryGenerationService.java (1)
350-426: v1.1 급여 계산 통합 구현 확인기존 개별 계산 로직에서
PayrollCalculationInput과PayrollCalculationResult를 활용한 통합 계산 흐름으로 잘 전환되었습니다. 계산 결과 추출 및 로깅이 명확합니다.src/main/java/com/concrete/buildup/domain/payroll/util/PayrollCalculator.java (4)
446-464: 일용직 4대보험 버그 수정 확인 - 월 8일 미만 조건 적용PR 목적대로 월 8일 미만 일용직에 대해 건강보험과 국민연금을 미가입 처리하는 로직이 올바르게 구현되었습니다. 또한 월소득 기준에서 총지급액 기준으로 변경된 점도 확인됩니다.
318-323: 야간수당 0.5배 가산은 정상 - 기본급에 이미 1.0이 포함됨야간근로의 경우 기본 시급(1.0)은
basePay에서 계산되므로, 가산분 0.5만 별도 계산하는 것이 맞습니다. 다만, 이 로직이 기존calculateNightWorkPay()(1.5배)와 다르므로 의도된 것인지 확인이 필요합니다.v1.1에서는 야간시간의 기본급이
basePay에 포함되지 않는다면 1.5배가 맞고, 포함된다면 0.5배가 맞습니다. 현재basePay계산에 야간시간이 포함되는지 확인해주세요.
550-572: 내부 DTO 클래스 구현 적절함
InsuranceResult와TaxResult내부 클래스가 계산 결과를 캡슐화하는 용도로 적절하게 사용되었습니다.
457-458: Based on my verification:계산 로직은 정확합니다
웹 검색 결과 및 코드 분석을 통해 확인한 결과, 장기요양보험료 계산이 올바르게 구현되어 있습니다:
- 공식: 건강보험료 × 12.81% ÷ 10 (내림) × 10 = 10원 단위 절사
- 테스트 검증: 70,900 × 0.1281 = 9,082.29 → 9,080원 (정확함)
현재 로직
healthIns.multiply(rates.getLongTermCare()).divide(new BigDecimal("10"), 0, RoundingMode.FLOOR).multiply(new BigDecimal("10"))은 의도된 계산 방식대로 동작합니다.src/main/java/com/concrete/buildup/domain/payroll/dto/PayrollCalculationInput.java (1)
1-89: LGTM - DTO 구조가 명확하고 문서화가 잘 되어 있음v1.1 급여 계산에 필요한 모든 입력 정보를 포함하는 DTO가 잘 설계되었습니다. Swagger 어노테이션과 Javadoc이 적절히 작성되어 API 문서화 및 유지보수에 도움이 됩니다.
| @Schema(description = "상시근로자 수 (5인 이상이면 가산수당 의무 지급)", example = "10", required = true) | ||
| private Integer totalEmployees; |
There was a problem hiding this comment.
totalEmployees가 null일 경우 NPE 발생 가능
PayrollCalculator.canPayOvertimeAllowance()에서 conditions.getTotalEmployees() >= 5 비교 시 totalEmployees가 null이면 NullPointerException이 발생합니다.
int 기본 타입 사용 또는 null-safe 처리를 권장합니다:
- @Schema(description = "상시근로자 수 (5인 이상이면 가산수당 의무 지급)", example = "10", required = true)
- private Integer totalEmployees;
+ @Schema(description = "상시근로자 수 (5인 이상이면 가산수당 의무 지급)", example = "10", required = true)
+ @Builder.Default
+ private Integer totalEmployees = 0;또는 PayrollCalculator에서 null 체크를 추가해야 합니다.
🤖 Prompt for AI Agents
In src/main/java/com/concrete/buildup/domain/payroll/dto/OvertimeConditions.java
around lines 27-28, totalEmployees is declared as Integer which can be null and
causes an NPE when compared in PayrollCalculator; change totalEmployees to the
primitive int (or initialize it to a non-null default and add a validation
annotation like @NotNull/@Min(0)) so callers never receive null, or
alternatively update PayrollCalculator.canPayOvertimeAllowance() to null-check
conditions.getTotalEmployees() (treat null as 0) before comparing to 5; pick one
approach and apply consistently with input validation.
| BigDecimal totalWorkHours = input.getDailyWorkHours().multiply(new BigDecimal(input.getWorkDays())); | ||
| BigDecimal basePay = input.getHourlyRate().multiply(totalWorkHours).setScale(0, RoundingMode.FLOOR); |
There was a problem hiding this comment.
dailyWorkHours 또는 workDays가 null일 경우 NPE 발생 가능
calculate() 메서드 진입 시 필수 입력값에 대한 null 검증이 없습니다.
메서드 시작 부분에 입력값 검증을 추가하는 것을 권장합니다:
public PayrollCalculationResult calculate(PayrollCalculationInput input) {
log.info("v1.1 급여 계산 시작 - empType: {}, payPeriod: {}", input.getEmpType(), input.getPayPeriod());
+
+ if (input.getHourlyRate() == null || input.getDailyWorkHours() == null || input.getWorkDays() == null) {
+ throw new IllegalArgumentException("필수 입력값(hourlyRate, dailyWorkHours, workDays)이 누락되었습니다.");
+ }
Map<String, Object> details = new HashMap<>();Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/concrete/buildup/domain/payroll/util/PayrollCalculator.java
around lines 298-299, add null checks at the start of calculate(): validate that
input, input.getDailyWorkHours(), input.getWorkDays(), and input.getHourlyRate()
are not null (use Objects.requireNonNull or explicit if-checks) and throw a
clear IllegalArgumentException or NullPointerException with descriptive
messages; this prevents NPE when computing totalWorkHours and basePay and
ensures callers get an explicit validation error before any BigDecimal
operations.
| private boolean canPayOvertimeAllowance(OvertimeConditions conditions) { | ||
| boolean isMandatory = conditions.getTotalEmployees() >= 5; | ||
| boolean meetsCondition = Boolean.TRUE.equals(conditions.getExceedsDailyLimit()) | ||
| || Boolean.TRUE.equals(conditions.getExceedsWeeklyLimit()); | ||
|
|
||
| if (isMandatory) { | ||
| return meetsCondition; | ||
| } else { | ||
| return Boolean.TRUE.equals(conditions.getAutoPayOvertime()) && meetsCondition; | ||
| } | ||
| } |
There was a problem hiding this comment.
getTotalEmployees() null 체크 누락
conditions.getTotalEmployees()가 null일 경우 >= 5 비교에서 NPE가 발생합니다.
private boolean canPayOvertimeAllowance(OvertimeConditions conditions) {
- boolean isMandatory = conditions.getTotalEmployees() >= 5;
+ int employees = conditions.getTotalEmployees() != null ? conditions.getTotalEmployees() : 0;
+ boolean isMandatory = employees >= 5;
boolean meetsCondition = Boolean.TRUE.equals(conditions.getExceedsDailyLimit())
|| Boolean.TRUE.equals(conditions.getExceedsWeeklyLimit());canPayNightAllowance와 canPayHolidayAllowance에도 동일하게 적용 필요합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private boolean canPayOvertimeAllowance(OvertimeConditions conditions) { | |
| boolean isMandatory = conditions.getTotalEmployees() >= 5; | |
| boolean meetsCondition = Boolean.TRUE.equals(conditions.getExceedsDailyLimit()) | |
| || Boolean.TRUE.equals(conditions.getExceedsWeeklyLimit()); | |
| if (isMandatory) { | |
| return meetsCondition; | |
| } else { | |
| return Boolean.TRUE.equals(conditions.getAutoPayOvertime()) && meetsCondition; | |
| } | |
| } | |
| private boolean canPayOvertimeAllowance(OvertimeConditions conditions) { | |
| int employees = conditions.getTotalEmployees() != null ? conditions.getTotalEmployees() : 0; | |
| boolean isMandatory = employees >= 5; | |
| boolean meetsCondition = Boolean.TRUE.equals(conditions.getExceedsDailyLimit()) | |
| || Boolean.TRUE.equals(conditions.getExceedsWeeklyLimit()); | |
| if (isMandatory) { | |
| return meetsCondition; | |
| } else { | |
| return Boolean.TRUE.equals(conditions.getAutoPayOvertime()) && meetsCondition; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/concrete/buildup/domain/payroll/util/PayrollCalculator.java
around lines 384 to 394, the expression conditions.getTotalEmployees() >= 5 can
throw an NPE when getTotalEmployees() returns null; make the check null-safe
(e.g. retrieve into a local Integer, test for non-null and compare, or default
to 0) so isMandatory is computed without risking NPE, keep the existing
Boolean.TRUE.equals(...) patterns for other flags, and apply the same null-safe
total-employees check to canPayNightAllowance and canPayHolidayAllowance as
well.
- 테스트 코드: 일용직 일급 테스트에서 monthlyWorkDaysAccumulated를 0에서 workDays 값(1)로 수정 - 서비스 로직: SalaryGenerationService에서 일용직의 경우 workDays 값을 monthlyWorkDaysAccumulated에 사용하도록 수정 - 상용직의 경우 monthlyWorkDaysAccumulated는 사용하지 않으므로 0으로 유지 이제 일용직 일급/월급 모두 실제 근로일수가 4대보험 가입 조건 판단에 올바르게 반영됩니다.
- calculateDailyTax 메서드에서 workDays가 null이거나 0일 때 조기 반환하여 예외 방지 - 사용하지 않는 필드/메서드에 @SuppressWarnings 추가하여 린터 경고 해결 - NON_TAXABLE_LIMIT: 향후 사용 예정 - nvl 메서드: calculate 메서드에서 사용 중
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/com/concrete/buildup/domain/payroll/util/PayrollCalculator.java (2)
293-302: 필수 입력값 null 체크 누락 (이전 리뷰 미반영)
input.getDailyWorkHours()또는input.getWorkDays()가 null일 경우 Line 299에서NullPointerException이 발생합니다.메서드 시작 부분에 입력값 검증을 추가하세요:
public PayrollCalculationResult calculate(PayrollCalculationInput input) { log.info("v1.1 급여 계산 시작 - empType: {}, payPeriod: {}", input.getEmpType(), input.getPayPeriod()); + + if (input.getHourlyRate() == null || input.getDailyWorkHours() == null || input.getWorkDays() == null) { + throw new IllegalArgumentException("필수 입력값(hourlyRate, dailyWorkHours, workDays)이 누락되었습니다."); + } Map<String, Object> details = new HashMap<>();
385-395:getTotalEmployees()null 체크 누락 (이전 리뷰 미반영)
conditions.getTotalEmployees()가 null일 경우>= 5비교에서NullPointerException이 발생합니다.null-safe 처리를 추가하세요:
private boolean canPayOvertimeAllowance(OvertimeConditions conditions) { - boolean isMandatory = conditions.getTotalEmployees() >= 5; + int employees = conditions.getTotalEmployees() != null ? conditions.getTotalEmployees() : 0; + boolean isMandatory = employees >= 5; boolean meetsCondition = Boolean.TRUE.equals(conditions.getExceedsDailyLimit()) || Boolean.TRUE.equals(conditions.getExceedsWeeklyLimit());
canPayNightAllowance(Line 401)와canPayHolidayAllowance(Line 415)에도 동일하게 적용이 필요합니다.
🧹 Nitpick comments (1)
src/main/java/com/concrete/buildup/domain/payroll/service/SalaryGenerationService.java (1)
388-392: 하드코딩된 기본값 검토 필요
totalEmployees가 5로 하드코딩되어 있어 모든 사업장이 5인 이상으로 처리됩니다. 실제 사업장 규모에 따라 가산수당 지급 의무가 달라지므로, 향후Corporation또는 관련 엔티티에서 실제 직원 수를 가져오는 것이 바람직합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/concrete/buildup/domain/payroll/service/SalaryGenerationService.java(2 hunks)src/main/java/com/concrete/buildup/domain/payroll/util/PayrollCalculator.java(3 hunks)src/test/java/com/concrete/buildup/domain/payroll/util/PayrollCalculatorTest.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
🔇 Additional comments (7)
src/test/java/com/concrete/buildup/domain/payroll/util/PayrollCalculatorTest.java (3)
405-467: 테스트 커버리지 양호!일용직 기본 케이스에 대한 테스트가 잘 구성되어 있습니다. 일급 15만원 미만 비과세 처리와 4대보험 미가입 케이스를 정확히 검증하고 있습니다.
675-731: 테스트 입력과 계산 기준 불일치 가능성
monthlyEstimatedIncome이 250만원으로 설정되어 있지만,PayrollCalculator.calculateDailyInsurance()에서 실제 보험 계산은taxableIncome(=totalPay= 200만원)을 기준으로 합니다. 테스트 주석과 실제 계산 기준이 다를 수 있습니다.현재
PayrollCalculator구현상monthlyEstimatedIncome은 사용되지 않고taxableIncome이 사용되므로, 테스트 주석의 "월소득 220만원 이상 조건 충족" 설명이 혼란을 줄 수 있습니다.
853-903: 휴일+야간 중복가산 테스트 LGTM휴일근로와 야간근로가 중복되는 시나리오를 잘 테스트하고 있습니다. 각 수당이 별도로 계산되어 합산되는 로직을 명확히 검증합니다.
src/main/java/com/concrete/buildup/domain/payroll/service/SalaryGenerationService.java (1)
363-400: 이전 리뷰 이슈 해결 확인 -monthlyWorkDaysAccumulated수정됨이전 리뷰에서 지적된
monthlyWorkDaysAccumulated가 0으로 하드코딩된 문제가 수정되었습니다. 이제 일용직의 경우 실제 근로일수(attendances.size())를 사용합니다.다만,
monthlyEstimatedIncome이 여전히BigDecimal.ZERO로 설정되어 있습니다.PayrollCalculator.calculateDailyInsurance()에서monthlyEstimatedIncome은 현재 사용되지 않고taxableIncome이 사용되므로 동작에는 문제가 없지만, 향후 사용 예정이라면 실제 값을 계산하는 것이 좋습니다.src/main/java/com/concrete/buildup/domain/payroll/util/PayrollCalculator.java (3)
487-491: workDays 0 체크 수정 완료이전 리뷰에서 지적된
workDays가 0일 때ArithmeticException발생 문제가 수정되었습니다. null 또는 0일 경우 세금 0원을 반환하도록 처리되었습니다.
326-338: 주휴수당 계산 로직 확인 필요일용직 주휴수당이 "하루 일당"으로 계산되고, 상용직은 "주 4회 × 일당"으로 계산됩니다. 그러나
SalaryGenerationService에서 주차별로calculateWeeklyHolidayPay()를 호출하여 합산한 값과 여기서 계산된 값이 다를 수 있습니다.v1.1
calculate()메서드 사용 시 Line 411에서weeklyHolidayPay가 덮어써지므로, 기존 주차별 합산 로직(Lines 318-322 in SalaryGenerationService)이 무시됩니다. 의도된 동작인지 확인이 필요합니다.
557-579: 내부 DTO 클래스 구현 LGTM
InsuranceResult와TaxResult내부 클래스가 명확하게 구현되어 있습니다.getTotal()메서드를 통해 합계 계산이 편리합니다.
| private InsuranceResult calculatePermanentInsurance(PayrollCalculationInput input, BigDecimal taxableIncome) { | ||
| InsuranceRates rates = input.getRates(); | ||
|
|
||
| BigDecimal employmentIns = taxableIncome.multiply(rates.getEmploymentInsurance()).setScale(0, RoundingMode.FLOOR); | ||
| BigDecimal healthIns = taxableIncome.multiply(rates.getHealthInsurance()).setScale(0, RoundingMode.FLOOR); | ||
| BigDecimal longTermCareIns = healthIns.multiply(rates.getLongTermCare()).divide(new BigDecimal("10"), 0, RoundingMode.FLOOR).multiply(new BigDecimal("10")); | ||
| BigDecimal nationalPension = taxableIncome.multiply(rates.getNationalPension()).setScale(0, RoundingMode.FLOOR); | ||
|
|
||
| return new InsuranceResult(employmentIns, healthIns, longTermCareIns, nationalPension); | ||
| } |
There was a problem hiding this comment.
상용직 4대보험: InsuranceEligibility 미사용
calculatePermanentInsurance()에서 InsuranceEligibility 가입 여부를 체크하지 않고 무조건 4대보험을 계산합니다. 계약에서 특정 보험을 미가입으로 설정한 경우에도 공제가 발생할 수 있습니다.
private InsuranceResult calculatePermanentInsurance(PayrollCalculationInput input, BigDecimal taxableIncome) {
+ InsuranceEligibility eligibility = input.getInsurance();
InsuranceRates rates = input.getRates();
- BigDecimal employmentIns = taxableIncome.multiply(rates.getEmploymentInsurance()).setScale(0, RoundingMode.FLOOR);
- BigDecimal healthIns = taxableIncome.multiply(rates.getHealthInsurance()).setScale(0, RoundingMode.FLOOR);
+ BigDecimal employmentIns = Boolean.TRUE.equals(eligibility.getEmploymentInsurance())
+ ? taxableIncome.multiply(rates.getEmploymentInsurance()).setScale(0, RoundingMode.FLOOR)
+ : BigDecimal.ZERO;
+ BigDecimal healthIns = Boolean.TRUE.equals(eligibility.getHealthInsurance())
+ ? taxableIncome.multiply(rates.getHealthInsurance()).setScale(0, RoundingMode.FLOOR)
+ : BigDecimal.ZERO;
BigDecimal longTermCareIns = healthIns.multiply(rates.getLongTermCare()).divide(new BigDecimal("10"), 0, RoundingMode.FLOOR).multiply(new BigDecimal("10"));
- BigDecimal nationalPension = taxableIncome.multiply(rates.getNationalPension()).setScale(0, RoundingMode.FLOOR);
+ BigDecimal nationalPension = Boolean.TRUE.equals(eligibility.getNationalPension())
+ ? taxableIncome.multiply(rates.getNationalPension()).setScale(0, RoundingMode.FLOOR)
+ : BigDecimal.ZERO;
return new InsuranceResult(employmentIns, healthIns, longTermCareIns, nationalPension);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private InsuranceResult calculatePermanentInsurance(PayrollCalculationInput input, BigDecimal taxableIncome) { | |
| InsuranceRates rates = input.getRates(); | |
| BigDecimal employmentIns = taxableIncome.multiply(rates.getEmploymentInsurance()).setScale(0, RoundingMode.FLOOR); | |
| BigDecimal healthIns = taxableIncome.multiply(rates.getHealthInsurance()).setScale(0, RoundingMode.FLOOR); | |
| BigDecimal longTermCareIns = healthIns.multiply(rates.getLongTermCare()).divide(new BigDecimal("10"), 0, RoundingMode.FLOOR).multiply(new BigDecimal("10")); | |
| BigDecimal nationalPension = taxableIncome.multiply(rates.getNationalPension()).setScale(0, RoundingMode.FLOOR); | |
| return new InsuranceResult(employmentIns, healthIns, longTermCareIns, nationalPension); | |
| } | |
| private InsuranceResult calculatePermanentInsurance(PayrollCalculationInput input, BigDecimal taxableIncome) { | |
| InsuranceEligibility eligibility = input.getInsurance(); | |
| InsuranceRates rates = input.getRates(); | |
| BigDecimal employmentIns = Boolean.TRUE.equals(eligibility.getEmploymentInsurance()) | |
| ? taxableIncome.multiply(rates.getEmploymentInsurance()).setScale(0, RoundingMode.FLOOR) | |
| : BigDecimal.ZERO; | |
| BigDecimal healthIns = Boolean.TRUE.equals(eligibility.getHealthInsurance()) | |
| ? taxableIncome.multiply(rates.getHealthInsurance()).setScale(0, RoundingMode.FLOOR) | |
| : BigDecimal.ZERO; | |
| BigDecimal longTermCareIns = healthIns.multiply(rates.getLongTermCare()).divide(new BigDecimal("10"), 0, RoundingMode.FLOOR).multiply(new BigDecimal("10")); | |
| BigDecimal nationalPension = Boolean.TRUE.equals(eligibility.getNationalPension()) | |
| ? taxableIncome.multiply(rates.getNationalPension()).setScale(0, RoundingMode.FLOOR) | |
| : BigDecimal.ZERO; | |
| return new InsuranceResult(employmentIns, healthIns, longTermCareIns, nationalPension); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/concrete/buildup/domain/payroll/util/PayrollCalculator.java
around lines 473 to 482, the method calculatePermanentInsurance unconditionally
computes all four insurance amounts; change it to read the InsuranceEligibility
from the input and compute each insurance only if the corresponding eligibility
flag is true, otherwise set that insurance amount to BigDecimal.ZERO (preserving
the same scale/rounding behavior for calculated values); for longTermCare,
compute it only when health insurance is eligible (and eligibility for long-term
care itself is true) and keep the existing rounding logic when calculating
non-zero values; return an InsuranceResult containing either the computed
amounts or BigDecimal.ZERO for ineligible insurances.
1 participant
Pull Request
📋 Jira Issue
📝 변경 사항 요약
v1.1 급여 계산 로직의 버그를 수정하고 테스트 코드를 추가했습니다. 일용직 4대보험 가입 조건 및 계산 기준을 올바르게 수정하여 테스트 실패 문제를 해결했습니다.
주요 변경사항
🎯 변경 목적
v1.1 급여 계산 로직 구현 중 테스트 실패가 발생하여 다음 문제들을 해결했습니다:
🔍 변경 내역 상세
버그 수정 (Bug Fixes)
버그 1: 일용직 4대보험 가입 조건 수정
PayrollCalculator.java(446-449줄)버그 2: 일용직 4대보험 계산 기준 수정
PayrollCalculator.java(451-464줄)추가된 기능 (Features)
PayrollCalculatorTest.java(405-904줄)🧪 테스트 방법
단위 테스트
테스트 실행 결과:
./gradlew test --tests PayrollCalculatorTest결과:
수동 테스트
단계 1: 일용직 월 5일 근무 케이스
단계 2: 일용직 월 10일 근무 케이스
단계 3: 가산수당 조건 판정 테스트
🔗 의존성 변경
🗄️ 데이터베이스 변경
✅ 체크리스트
코드 품질
테스트
./gradlew test)./gradlew clean build)보안
성능
문서
Git
test:타입 사용)Refs: BU-206)📌 리뷰어에게
특별히 확인해주시면 좋을 부분:
일용직 4대보험 가입 조건 로직 (446-449줄)
일용직 4대보험 계산 기준 (453줄)
테스트 케이스 커버리지
참고사항:
calculateDailyInsurance메서드에만 국한됩니다.🔗 관련 PR/Issue
📚 참고 자료
Summary by CodeRabbit
새로운 기능
문서/템플릿
테스트
✏️ Tip: You can customize this high-level summary in your review settings.