[fix] 주간 캘린더 예상 근무비에 SCHEDULED 근무 포함#177
Merged
Merged
Conversation
SCHEDULED 상태의 근무 기록도 예상 급여를 계산하여 저장하도록 수정. 이전에는 COMPLETED 상태만 급여를 계산하여 주간 캘린더의 '이번 주 예상 근무비'가 완료된 근무만 반영되는 버그가 있었음. - createWorkRecordByEmployer: SCHEDULED 생성 시에도 예상 급여 계산 - createWorkRecordsBatch: 전체 레코드 예상 급여 일괄 계산 후 저장 - updateWorkRecord: SCHEDULED 수정 시에도 예상 급여 재계산 - WorkRecordGenerationService: 자동 생성 레코드에도 예상 급여 계산 적용
Walkthrough근무 기록 생성 및 수정 시, 계산 단계가 모든 근무 기록(SCHEDULED 포함)에 적용되도록 변경되었습니다. 일관성 검증은 COMPLETED 상태에만 유지되며, 배치 생성도 동일한 패턴으로 업데이트되었습니다. Changes근무 기록 계산 범위 확대
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.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: 1
🧹 Nitpick comments (2)
src/test/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandServiceTest.java (1)
119-119: ⚡ Quick win
save횟수 고정 검증은 테스트를 구현 디테일에 과도하게 결합시킵니다.핵심 요구사항은 “미래(SCHEDULED) 생성 시 계산이 수행되는지”이므로, 저장 횟수보다
calculateWorkRecordDetails호출과validateWorkRecordConsistency미호출을 검증하는 쪽이 회귀 방지에 더 유리합니다.As per coding guidelines,
src/test/**/*.java의 "Mock 사용이 적절한지 확인" 및 "Given-When-Then 패턴 준수 확인" 취지에 맞게 핵심 행위 검증 중심으로 테스트를 두는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandServiceTest.java` at line 119, The test in WorkRecordCommandServiceTest currently asserts a fixed number of saves via verify(workRecordRepository, times(2)).save(any(WorkRecord.class)); change this to assert the behavioral requirements instead: remove the times(2) save-count assertion and instead verify that calculateWorkRecordDetails(...) on the service/utility used during SCHEDULED creation is invoked (e.g., verify(...).calculateWorkRecordDetails(...)) and that validateWorkRecordConsistency(...) is NOT invoked (e.g., verify(..., never()).validateWorkRecordConsistency(...)); keep the Given-When-Then structure and only mock/verify these core interactions rather than implementation details like exact save call counts.src/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.java (1)
98-103: @transactional 컨텍스트에서 이중save/saveAll호출을 단일 호출로 통합하세요.세 경로(create/update/batch) 모두에서 계산 전후로 저장을 2번 수행하고 있습니다.
calculateWorkRecordDetails메서드가 엔티티를 in-place로 갱신하며, 클래스 레벨@Transactional선언으로 인해 JPA 영속성 컨텍스트가 변경사항을 자동 추적하므로, 트랜잭션 커밋 시점의 자동 flush로 충분합니다. 두 번째 save/saveAll 호출을 제거하면 불필요한 DB 쓰기를 줄일 수 있습니다.각 메서드마다 save/saveAll 호출을 1회로 정리하는 리팩터링을 검토해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.java` around lines 98 - 103, Methods handling create/update/batch are performing two saves around calculationService.calculateWorkRecordDetails(...) (and optional calculationService.validateWorkRecordConsistency(...)); because the class is annotated with `@Transactional` and calculateWorkRecordDetails mutates the entity in-place, remove the redundant second workRecordRepository.save(...) / saveAll(...) call and ensure each method calls the repository exactly once (either before calculations if initial persist is required or only once after calculations) so that JPA's persistence context and automatic flush at commit handle DB writes; update create/update/batch paths to rely on the single save/saveAll and drop the extra one.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/test/java/com/example/paycheck/domain/workrecord/service/WorkRecordGenerationServiceTest.java`:
- Around line 35-37: Tests in WorkRecordGenerationServiceTest added a Mock for
calculationService but never verify its key interaction, so the suite would pass
even if calculateWorkRecordDetailsBatch(...) is not called; update at least one
existing test to call Mockito.verify(calculationService) to assert
calculateWorkRecordDetailsBatch(...) was invoked with the expected argument list
(or use argument matchers like eq(...) / argThat(...) to validate contents)
after the service under test runs, ensuring you reference the mock field
calculationService and the method calculateWorkRecordDetailsBatch(...) when
adding the assertion.
---
Nitpick comments:
In
`@src/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.java`:
- Around line 98-103: Methods handling create/update/batch are performing two
saves around calculationService.calculateWorkRecordDetails(...) (and optional
calculationService.validateWorkRecordConsistency(...)); because the class is
annotated with `@Transactional` and calculateWorkRecordDetails mutates the entity
in-place, remove the redundant second workRecordRepository.save(...) /
saveAll(...) call and ensure each method calls the repository exactly once
(either before calculations if initial persist is required or only once after
calculations) so that JPA's persistence context and automatic flush at commit
handle DB writes; update create/update/batch paths to rely on the single
save/saveAll and drop the extra one.
In
`@src/test/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandServiceTest.java`:
- Line 119: The test in WorkRecordCommandServiceTest currently asserts a fixed
number of saves via verify(workRecordRepository,
times(2)).save(any(WorkRecord.class)); change this to assert the behavioral
requirements instead: remove the times(2) save-count assertion and instead
verify that calculateWorkRecordDetails(...) on the service/utility used during
SCHEDULED creation is invoked (e.g.,
verify(...).calculateWorkRecordDetails(...)) and that
validateWorkRecordConsistency(...) is NOT invoked (e.g., verify(...,
never()).validateWorkRecordConsistency(...)); keep the Given-When-Then structure
and only mock/verify these core interactions rather than implementation details
like exact save call counts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: be837e31-fcec-4e43-8756-f9732894f1f9
📒 Files selected for processing (4)
src/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.javasrc/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordGenerationService.javasrc/test/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandServiceTest.javasrc/test/java/com/example/paycheck/domain/workrecord/service/WorkRecordGenerationServiceTest.java
Comment on lines
+35
to
+37
| @Mock | ||
| private WorkRecordCalculationService calculationService; | ||
|
|
There was a problem hiding this comment.
추가한 calculationService Mock의 핵심 호출 검증이 빠져 있습니다.
현재 테스트들은 saveAll만 검증하고 있어, 생성 경로에서 calculateWorkRecordDetailsBatch(...) 호출이 빠져도 통과할 수 있습니다. 최소 1개 케이스에서 해당 호출(및 인자 리스트)을 검증해 주세요.
예시 보강 (한 테스트에 추가)
// then
verify(workRecordRepository).saveAll(workRecordsCaptor.capture());
+verify(calculationService).calculateWorkRecordDetailsBatch(anyList());As per coding guidelines, src/test/**/*.java의 "Mock 사용이 적절한지 확인" 규칙에 따라 새로 추가된 Mock의 핵심 상호작용을 검증해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/example/paycheck/domain/workrecord/service/WorkRecordGenerationServiceTest.java`
around lines 35 - 37, Tests in WorkRecordGenerationServiceTest added a Mock for
calculationService but never verify its key interaction, so the suite would pass
even if calculateWorkRecordDetailsBatch(...) is not called; update at least one
existing test to call Mockito.verify(calculationService) to assert
calculateWorkRecordDetailsBatch(...) was invoked with the expected argument list
(or use argument matchers like eq(...) / argThat(...) to validate contents)
after the service under test runs, ensuring you reference the mock field
calculationService and the method calculateWorkRecordDetailsBatch(...) when
adding the assertion.
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
1 participant
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.
🔖 관련 GitHub Issue
📝 변경 사항
주요 변경 내용
상세 설명
문제
WorkRecordDto.DetailedResponse의totalSalary필드가 SCHEDULED 상태 근무에서 항상0으로 반환되어, 주간 캘린더의 '이번 주 예상 근무비' 계산 시 완료된 근무만 반영되는 버그.원인
기존 코드는
WorkRecordCommandService와WorkRecordGenerationService에서COMPLETED상태일 때만calculationService.calculateWorkRecordDetails()를 호출하고,SCHEDULED상태는 급여 계산을 건너뜀.수정 내용
createWorkRecordByEmployer():COMPLETED조건 제거, SCHEDULED 생성 시에도 예상 급여 계산createWorkRecordsBatch(): 전체 레코드에 예상 급여 일괄 계산 후 저장 (완료 레코드만 일괄 저장하던 버그도 함께 수정)updateWorkRecord():COMPLETED조건 제거, SCHEDULED 수정 시에도 예상 급여 재계산WorkRecordGenerationService:WorkRecordCalculationService주입 추가, 자동 생성 레코드에도 예상 급여 계산 적용SCHEDULED 근무가 COMPLETED로 전환되면 기존처럼 정확한 계산이 다시 수행됨.
📸 스크린샷 (선택사항)
스크린샷 보기
Before
After
✅ 체크리스트
🔗 관련 PR
Summary by CodeRabbit
릴리스 노트
버그 수정
리팩토링