[feat] 근로자 퇴사 시 예정된 근무 기록 삭제 처리#118
Merged
geunyong16 merged 3 commits intoFeb 22, 2026
Merged
Conversation
…tion - WorkRecordCommandService에 deleteFutureWorkRecords 메서드 추가 - ContractService.terminateContract에서 미래 근무 기록 삭제 호출 - 삭제 시 관련 CorrectionRequest 함께 정리 - buildActionData에서 null 반환 대신 RuntimeException throw로 변경 - Clock 주입으로 테스트 용이성 개선 - WorkRecordCalculationService에 일괄 계산 메서드 추가
Walkthrough계약 종료 시 미래 예정된 SCHEDULED 근무 기록 및 관련 CorrectionRequest를 정리하도록 삭제 호출을 추가했고, WorkRecord 계산에 배치 처리 메서드를 도입했으며, Clock 빈 주입과 SLF4J 로깅·오류 로그 추가로 시간 결정론화 및 로깅을 강화했습니다. Changes
Sequence DiagramsequenceDiagram
participant User as 사용자
participant ContractService as ContractService
participant WorkRecordCommandService as WorkRecordCommandService
participant WorkRecordDB as WorkRecordDB
participant CorrectionDB as CorrectionRequestDB
rect rgba(200,200,255,0.5)
User->>ContractService: terminateContract(contractId)
end
rect rgba(200,255,200,0.5)
ContractService->>WorkRecordCommandService: deleteFutureWorkRecords(contractId)
end
rect rgba(255,230,200,0.5)
WorkRecordCommandService->>WorkRecordDB: find SCHEDULED records after (today-1)
WorkRecordDB-->>WorkRecordCommandService: future WorkRecords
WorkRecordCommandService->>CorrectionDB: delete CorrectionRequest by workRecord ids / contractId
CorrectionDB-->>WorkRecordCommandService: deletion confirmation
WorkRecordCommandService->>WorkRecordDB: mark WorkRecords as DELETED (soft-delete)
WorkRecordDB-->>WorkRecordCommandService: update confirmation
end
ContractService-->>User: termination complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 4
🧹 Nitpick comments (3)
src/main/java/com/example/paycheck/global/config/ClockConfig.java (1)
12-13: 타임존을 명시적으로 설정하는 것을 권장합니다.
Clock.systemDefaultZone()은 JVM 기본 타임존을 사용합니다. Docker/클라우드 환경에서는 기본 타임존이 UTC가 되는 경우가 많아 날짜 경계(자정 전후)가LocalDate.now(clock)결과에 영향을 줄 수 있습니다. 한국 서비스이므로Asia/Seoul을 명시하는 것이 안전합니다.♻️ 명시적 타임존 설정 제안
+import java.time.ZoneId; `@Bean` public Clock clock() { - return Clock.systemDefaultZone(); + return Clock.system(ZoneId.of("Asia/Seoul")); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/example/paycheck/global/config/ClockConfig.java` around lines 12 - 13, The ClockConfig.clock() currently returns Clock.systemDefaultZone(), which relies on the JVM default TZ; change it to return a Clock explicitly using the Seoul zone (e.g., Clock.system(ZoneId.of("Asia/Seoul"))) so all consumers (like LocalDate.now(clock)) get a consistent Korea timezone regardless of container/JVM defaults; update imports/usages to include ZoneId and ensure the bean method name ClockConfig.clock() is returning the new zone-specific Clock.src/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.java (1)
432-441:buildActionData에서 매 호출마다new ObjectMapper()를 생성하고 있습니다.
ObjectMapper는 스레드 세이프하며 무거운 객체이므로 싱글턴으로 재사용해야 합니다.ContractService는ObjectMapper를 DI로 주입받아 사용합니다.WorkRecordCommandService도 동일하게 주입받아 사용해야 합니다.♻️ ObjectMapper 주입 방식으로 수정
+import com.fasterxml.jackson.databind.ObjectMapper; // 필드 선언부에 추가 +private final ObjectMapper objectMapper; private String buildActionData(Long workRecordId) { try { - ObjectMapper mapper = new ObjectMapper(); Map<String, Object> data = new HashMap<>(); data.put("workRecordId", workRecordId); - return mapper.writeValueAsString(data); + return objectMapper.writeValueAsString(data); } catch (Exception e) { throw new RuntimeException("알림 액션 데이터 생성 실패: workRecordId=" + workRecordId, e); } }🤖 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 432 - 441, buildActionData currently creates a new ObjectMapper on every call which is heavy; change WorkRecordCommandService to accept and reuse a singleton ObjectMapper (via constructor or field injection like ContractService does) and replace the local "new ObjectMapper()" usage in buildActionData with the injected ObjectMapper instance so the service reuses the thread-safe mapper across calls and avoid per-call allocations.src/main/java/com/example/paycheck/domain/contract/service/ContractService.java (1)
203-213:buildActionData오류 처리 방식이WorkRecordCommandService와 일치하지 않습니다.
ContractService.buildActionData는JsonProcessingException발생 시null을 반환(알림이actionData=null로 발행됨)하지만,WorkRecordCommandService.buildActionData는RuntimeException을 throw(트랜잭션 롤백 가능)합니다. 두 방식 중 하나로 통일하거나, 각각의 선택 이유를 주석으로 명시하는 것이 좋습니다.🤖 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/contract/service/ContractService.java` around lines 203 - 213, The error handling in ContractService.buildActionData is inconsistent with WorkRecordCommandService.buildActionData; instead of returning null on JsonProcessingException, change ContractService.buildActionData to rethrow a RuntimeException (including a clear message and the caught exception as the cause) so callers get consistent failure behavior and transactions can roll back, or alternatively add a brief comment in buildActionData documenting why this method returns null if you choose to keep it—refer to buildActionData in ContractService and WorkRecordCommandService.buildActionData to apply the same approach.
🤖 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/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.java`:
- Around line 391-399: Capture LocalDate once to avoid midnight-race
inconsistencies and make the deletion semantics consistent with
regenerateFutureWorkRecords: in deleteFutureWorkRecords(), assign LocalDate
today = LocalDate.now(clock) once and use that variable for both repository
calls
(correctionRequestRepository.deleteByWorkRecordContractAndDateAfterAndStatus and
workRecordRepository.deleteByContractIdAndWorkDateAfterAndStatus); remove the
minusDays(1) so the "after" comparisons are today-exclusive, and update the
comment to reflect "오늘 이후 (오늘 제외)".
- Around line 386-399: The current deleteFutureWorkRecords method physically
deletes rows; change it to perform a soft delete by updating WorkRecord.status
to WorkRecordStatus.DELETED instead of deleting rows: add/implement
workRecordRepository.softDeleteByContractIdAndWorkDateAfterAndStatus(...) (an
`@Modifying` UPDATE JPQL that sets w.status = :deletedStatus where contract.id =
:contractId AND workDate > :date AND status = :scheduledStatus) and call it from
deleteFutureWorkRecords; keep
correctionRequestRepository.deleteByWorkRecordContractAndDateAfterAndStatus(...)
as-is for removing associated requests (or adjust its query to target WorkRecord
IDs if needed). Also extract LocalDate today = LocalDate.now(clock) once and use
that variable in both repository calls.
In
`@src/test/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandServiceTest.java`:
- Around line 67-68: The test suite is missing unit tests for
deleteFutureWorkRecords(Long) and the `@Spy` Clock isn't used to fix "today"; add
a test (e.g., deleteFutureWorkRecords_Success) that stubs clock.instant() and
clock.getZone() with doReturn(...) so "today" is deterministic, call
workRecordCommandService.deleteFutureWorkRecords(contractId), and verify
correctionRequestRepository.deleteByWorkRecordContractAndDateAfterAndStatus(...)
and workRecordRepository.deleteByContractIdAndWorkDateAfterAndStatus(...) are
invoked with the expected contractId, date boundary (today.minusDays(1)) and
WorkRecordStatus.SCHEDULED to assert correct parameters and date range handling.
- Line 27: Remove the unused import of ZoneId from the test class; in
WorkRecordCommandServiceTest delete the line importing java.time.ZoneId since
ZoneId is not referenced anywhere in that class.
---
Nitpick comments:
In
`@src/main/java/com/example/paycheck/domain/contract/service/ContractService.java`:
- Around line 203-213: The error handling in ContractService.buildActionData is
inconsistent with WorkRecordCommandService.buildActionData; instead of returning
null on JsonProcessingException, change ContractService.buildActionData to
rethrow a RuntimeException (including a clear message and the caught exception
as the cause) so callers get consistent failure behavior and transactions can
roll back, or alternatively add a brief comment in buildActionData documenting
why this method returns null if you choose to keep it—refer to buildActionData
in ContractService and WorkRecordCommandService.buildActionData to apply the
same approach.
In
`@src/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.java`:
- Around line 432-441: buildActionData currently creates a new ObjectMapper on
every call which is heavy; change WorkRecordCommandService to accept and reuse a
singleton ObjectMapper (via constructor or field injection like ContractService
does) and replace the local "new ObjectMapper()" usage in buildActionData with
the injected ObjectMapper instance so the service reuses the thread-safe mapper
across calls and avoid per-call allocations.
In `@src/main/java/com/example/paycheck/global/config/ClockConfig.java`:
- Around line 12-13: The ClockConfig.clock() currently returns
Clock.systemDefaultZone(), which relies on the JVM default TZ; change it to
return a Clock explicitly using the Seoul zone (e.g.,
Clock.system(ZoneId.of("Asia/Seoul"))) so all consumers (like
LocalDate.now(clock)) get a consistent Korea timezone regardless of
container/JVM defaults; update imports/usages to include ZoneId and ensure the
bean method name ClockConfig.clock() is returning the new zone-specific Clock.
Comment on lines
+391
to
+399
| public void deleteFutureWorkRecords(Long contractId) { | ||
| // 삭제할 WorkRecord를 참조하는 CorrectionRequest 삭제 | ||
| correctionRequestRepository.deleteByWorkRecordContractAndDateAfterAndStatus( | ||
| contractId, LocalDate.now(clock).minusDays(1), WorkRecordStatus.SCHEDULED); | ||
|
|
||
| // 오늘 이후의 SCHEDULED 상태 WorkRecord 삭제 | ||
| workRecordRepository.deleteByContractIdAndWorkDateAfterAndStatus( | ||
| contractId, LocalDate.now(clock).minusDays(1), WorkRecordStatus.SCHEDULED); | ||
| } |
There was a problem hiding this comment.
LocalDate.now(clock)을 두 번 호출하여 자정 경계에서 불일치가 발생할 수 있으며, 주석과 구현의 날짜 범위가 다릅니다.
LocalDate.now(clock)이 lines 394, 398에서 각각 호출되고 있어 자정 바로 직전에 메서드가 시작되면 두 호출이 서로 다른 날짜를 반환할 수 있습니다. 로컬 변수로 한 번만 캡처해야 합니다.
또한, 주석은 "오늘 이후" (today exclusive)라고 되어 있지만, minusDays(1)을 사용하므로 실제로는 오늘 포함 이후(today inclusive)로 삭제됩니다. 반면 regenerateFutureWorkRecords는 LocalDate.now(clock) (today exclusive)를 사용해 날짜 기준이 다릅니다. 의도적 차이라면 주석에 명확히 표기해야 합니다.
🛠️ 수정 제안
public void deleteFutureWorkRecords(Long contractId) {
+ LocalDate today = LocalDate.now(clock);
+
// 삭제할 WorkRecord를 참조하는 CorrectionRequest 삭제
correctionRequestRepository.deleteByWorkRecordContractAndDateAfterAndStatus(
- contractId, LocalDate.now(clock).minusDays(1), WorkRecordStatus.SCHEDULED);
+ contractId, today.minusDays(1), WorkRecordStatus.SCHEDULED);
- // 오늘 이후의 SCHEDULED 상태 WorkRecord 삭제
+ // 오늘 포함 이후(today inclusive)의 SCHEDULED 상태 WorkRecord 삭제
workRecordRepository.deleteByContractIdAndWorkDateAfterAndStatus(
- contractId, LocalDate.now(clock).minusDays(1), WorkRecordStatus.SCHEDULED);
+ contractId, today.minusDays(1), WorkRecordStatus.SCHEDULED);
}🤖 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 391 - 399, Capture LocalDate once to avoid midnight-race
inconsistencies and make the deletion semantics consistent with
regenerateFutureWorkRecords: in deleteFutureWorkRecords(), assign LocalDate
today = LocalDate.now(clock) once and use that variable for both repository
calls
(correctionRequestRepository.deleteByWorkRecordContractAndDateAfterAndStatus and
workRecordRepository.deleteByContractIdAndWorkDateAfterAndStatus); remove the
minusDays(1) so the "after" comparisons are today-exclusive, and update the
comment to reflect "오늘 이후 (오늘 제외)".
- LocalDate.now(clock) 중복 호출 제거 → 로컬 변수 today로 캡처 - 주석 '오늘 이후' → '오늘 포함 이후'로 수정 (today inclusive 명확화) - @SPY Clock을 고정 날짜(2026-02-21)로 변경하여 날짜 경계 테스트 안정화 - deleteFutureWorkRecords 단위 테스트 추가 (정상 삭제, 삭제 순서 보장)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.java (1)
343-353:⚠️ Potential issue | 🟠 Major배치와 단건 메서드의 공휴일 판정 기준 불일치 → 시급 계산 편차 가능성
calculateWorkRecordDetailsBatch는getHolidayDates(year)(전체 공휴일 테이블 조회)를 사용하고,calculateWorkRecordDetails는isPublicHoliday()(isPublicHoliday=true인 항목만 필터)를 사용합니다. 그 결과 기념일(예: 식목일,isPublicHoliday=false)처럼isPublicHoliday플래그가 거짓인 항목에 대해 배치는 휴일 수당을 적용하고 단건은 적용하지 않을 수 있습니다.두 메서드가 동일한 로직으로 공휴일을 판정하도록 통일하세요. 예를 들어 배치도
isPublicHoliday()메서드를 사용하거나,getHolidayDates()조회 시isPublicHoliday=true필터를 추가하세요.🤖 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 343 - 353, The batch vs single calculation use different holiday criteria causing pay discrepancies: update calculateWorkRecordDetailsBatch and/or the holiday retrieval so both use the same public-holiday predicate as calculateWorkRecordDetails; specifically either make calculateWorkRecordDetailsBatch call the same isPublicHoliday() filtering logic (reuse the same helper) when deciding holiday pay, or change getHolidayDates(year) usage in the batch path to return only entries with isPublicHoliday=true so its results match calculateWorkRecordDetails; ensure both methods reference the same helper/function to determine holiday status (isPublicHoliday) to guarantee identical holiday-pay decisions.
🧹 Nitpick comments (1)
src/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.java (1)
413-421:regenerateFutureWorkRecords에서LocalDate.now(clock)세 번 중복 호출Lines 414, 418, 421에서 각각
LocalDate.now(clock)을 호출하고 있어 자정 경계 직전 실행 시 일관성 없는 날짜가 사용될 수 있습니다.deleteFutureWorkRecords에서today로컬 변수로 한 번만 캡처한 패턴과 통일성이 없습니다.♻️ 제안 수정
public void regenerateFutureWorkRecords(Long contractId) { WorkerContract contract = workerContractRepository.findById(contractId) .orElseThrow(() -> new NotFoundException(ErrorCode.CONTRACT_NOT_FOUND, "계약을 찾을 수 없습니다.")); + LocalDate today = LocalDate.now(clock); + // 삭제할 WorkRecord를 참조하는 CorrectionRequest 삭제 correctionRequestRepository.deleteByWorkRecordContractAndDateAfterAndStatus( - contractId, LocalDate.now(clock), WorkRecordStatus.SCHEDULED); + contractId, today, WorkRecordStatus.SCHEDULED); // 오늘 이후의 SCHEDULED 상태 WorkRecord 삭제 workRecordRepository.deleteByContractIdAndWorkDateAfterAndStatus( - contractId, LocalDate.now(clock), WorkRecordStatus.SCHEDULED); + contractId, today, WorkRecordStatus.SCHEDULED); // 새로운 WorkRecord 생성 (오늘+1 ~ 2개월 뒤) - LocalDate startDate = LocalDate.now(clock).plusDays(1); + LocalDate startDate = today.plusDays(1); LocalDate endDate = startDate.plusMonths(2); workRecordGenerationService.generateWorkRecordsForPeriod(contract, startDate, endDate); }🤖 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 413 - 421, The method calls LocalDate.now(clock) three times causing potential inconsistent dates; capture LocalDate.now(clock) once into a local variable (e.g., today) at the start of regenerateFutureWorkRecords and reuse that variable in the three locations: correctionRequestRepository.deleteByWorkRecordContractAndDateAfterAndStatus(contractId, today, ...), workRecordRepository.deleteByContractIdAndWorkDateAfterAndStatus(contractId, today, ...), and compute startDate as today.plusDays(1) so all operations use the same consistent date snapshot.
🤖 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/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.java`:
- Around line 441-442: buildActionData currently throws a RuntimeException which
will bubble up and cause the surrounding `@Transactional` methods
(createWorkRecordByEmployer, updateWorkRecord in WorkRecordCommandService) to
rollback; change buildActionData to catch serialization/processing exceptions,
log the error with context (include workRecordId and exception), and return null
(or an Optional) instead of rethrowing so the business transaction isn't rolled
back, or alternatively extract the notification creation/publishing into a
separate method annotated with `@Transactional`(propagation = REQUIRES_NEW) (e.g.,
publishNotification or createNotificationInNewTx) and call that from the main
flow so notification failures don't affect the main work record transaction.
---
Outside diff comments:
In
`@src/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.java`:
- Around line 343-353: The batch vs single calculation use different holiday
criteria causing pay discrepancies: update calculateWorkRecordDetailsBatch
and/or the holiday retrieval so both use the same public-holiday predicate as
calculateWorkRecordDetails; specifically either make
calculateWorkRecordDetailsBatch call the same isPublicHoliday() filtering logic
(reuse the same helper) when deciding holiday pay, or change
getHolidayDates(year) usage in the batch path to return only entries with
isPublicHoliday=true so its results match calculateWorkRecordDetails; ensure
both methods reference the same helper/function to determine holiday status
(isPublicHoliday) to guarantee identical holiday-pay decisions.
---
Duplicate comments:
In
`@src/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.java`:
- Around line 386-401: deleteFutureWorkRecords currently performs a physical
delete via workRecordRepository.deleteByContractIdAndWorkDateAfterAndStatus
which violates the soft-delete policy; change the repository call to perform a
status update to WorkRecordStatus.DELETED (e.g., add a `@Modifying` JPQL method
like setStatusDeletedByContractIdAndWorkDateAfterAndStatus or
updateStatusByContractIdAndWorkDateAfterAndStatus and call that from
deleteFutureWorkRecords), keep or confirm
correctionRequestRepository.deleteByWorkRecordContractAndDateAfterAndStatus
behavior as needed, ensure the update is executed within the existing
transaction context, and update the method JavaDoc to reflect soft-delete
semantics (set status to DELETED) instead of “물리 삭제”.
---
Nitpick comments:
In
`@src/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.java`:
- Around line 413-421: The method calls LocalDate.now(clock) three times causing
potential inconsistent dates; capture LocalDate.now(clock) once into a local
variable (e.g., today) at the start of regenerateFutureWorkRecords and reuse
that variable in the three locations:
correctionRequestRepository.deleteByWorkRecordContractAndDateAfterAndStatus(contractId,
today, ...),
workRecordRepository.deleteByContractIdAndWorkDateAfterAndStatus(contractId,
today, ...), and compute startDate as today.plusDays(1) so all operations use
the same consistent date snapshot.
…failure RuntimeException throw 시 @transactional 롤백으로 근무 기록 저장까지 실패하는 문제 수정. 알림 데이터 생성 실패는 비즈니스 트랜잭션과 분리하여 예외 로깅 후 빈 JSON 객체 반환으로 처리.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.java (2)
413-421:LocalDate.now(clock)을 세 번 호출하여 자정 경계에서 불일치가 발생할 수 있습니다.
deleteFutureWorkRecords에서today변수를 추출하여 해결한 것과 달리,regenerateFutureWorkRecords에서는LocalDate.now(clock)을 lines 414, 418, 421에서 각각 호출하고 있습니다. 자정 직전에 메서드가 시작되면 세 호출이 서로 다른 날짜를 반환할 수 있어 삭제 범위와 생성 시작일이 어긋날 수 있습니다.♻️ 수정 제안
public void regenerateFutureWorkRecords(Long contractId) { WorkerContract contract = workerContractRepository.findById(contractId) .orElseThrow(() -> new NotFoundException(ErrorCode.CONTRACT_NOT_FOUND, "계약을 찾을 수 없습니다.")); + LocalDate today = LocalDate.now(clock); + // 삭제할 WorkRecord를 참조하는 CorrectionRequest 삭제 correctionRequestRepository.deleteByWorkRecordContractAndDateAfterAndStatus( - contractId, LocalDate.now(clock), WorkRecordStatus.SCHEDULED); + contractId, today, WorkRecordStatus.SCHEDULED); // 오늘 이후의 SCHEDULED 상태 WorkRecord 삭제 workRecordRepository.deleteByContractIdAndWorkDateAfterAndStatus( - contractId, LocalDate.now(clock), WorkRecordStatus.SCHEDULED); + contractId, today, WorkRecordStatus.SCHEDULED); // 새로운 WorkRecord 생성 (오늘+1 ~ 2개월 뒤) - LocalDate startDate = LocalDate.now(clock).plusDays(1); + LocalDate startDate = today.plusDays(1); LocalDate endDate = startDate.plusMonths(2); workRecordGenerationService.generateWorkRecordsForPeriod(contract, startDate, endDate); }🤖 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 413 - 421, In regenerateFutureWorkRecords, avoid calling LocalDate.now(clock) multiple times; capture a single LocalDate today = LocalDate.now(clock) at the start of the method and use that variable for all subsequent date calculations (replace the calls used in deleteByWorkRecordContractAndDateAfterAndStatus, workRecordRepository.deleteByContractIdAndWorkDateAfterAndStatus, and when computing startDate = today.plusDays(1)), ensuring contractId and any other repository calls in this method reference the same today value so deletion and new WorkRecord creation use a consistent boundary.
343-353:calculateWorkRecordDetailsBatch호출에 빈 리스트 가드가 없어saveAll호출과 비대칭입니다.
completedRecords가 비어 있으면calculateWorkRecordDetailsBatch(line 348)는 불필요하게 호출되는 반면,workRecordRepository.saveAll(line 351)은 빈 리스트 검사 후 호출됩니다. 두 호출 모두 동일한 조건 블록 안에 묶는 것이 일관성 면에서 낫습니다.♻️ 수정 제안
- calculationService.calculateWorkRecordDetailsBatch(completedRecords); - - // 7. 계산된 WorkRecord 일괄 업데이트 - if (!completedRecords.isEmpty()) { + // 6-7. COMPLETED 레코드 상세 계산 및 일괄 업데이트 + if (!completedRecords.isEmpty()) { + calculationService.calculateWorkRecordDetailsBatch(completedRecords); workRecordRepository.saveAll(completedRecords); }🤖 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 343 - 353, completedRecords is checked before calling workRecordRepository.saveAll but not before calling calculationService.calculateWorkRecordDetailsBatch, causing an unnecessary call when the list is empty; wrap the calculateWorkRecordDetailsBatch and workRecordRepository.saveAll calls in the same non-empty guard (e.g., if (!completedRecords.isEmpty()) { calculationService.calculateWorkRecordDetailsBatch(completedRecords); workRecordRepository.saveAll(completedRecords); }) so both operations execute only when completedRecords has elements, referencing calculateWorkRecordDetailsBatch and workRecordRepository.saveAll in WorkRecordCommandService.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@src/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.java`:
- Around line 386-401: The method deleteFutureWorkRecords currently calls
workRecordRepository.deleteByContractIdAndWorkDateAfterAndStatus which
physically removes rows; change this to perform a soft delete by adding a
`@Modifying` update method softDeleteByContractIdAndWorkDateAfterAndStatus on
WorkRecordRepository (update w.status = :deletedStatus where w.contract.id =
:contractId and w.workDate > :date and w.status = :scheduledStatus) and call
that from deleteFutureWorkRecords with WorkRecordStatus.DELETED (and also update
the JavaDoc/comment that currently says "물리 삭제" to reflect soft delete); keep
the existing
correctionRequestRepository.deleteByWorkRecordContractAndDateAfterAndStatus call
as-is.
---
Nitpick comments:
In
`@src/main/java/com/example/paycheck/domain/workrecord/service/WorkRecordCommandService.java`:
- Around line 413-421: In regenerateFutureWorkRecords, avoid calling
LocalDate.now(clock) multiple times; capture a single LocalDate today =
LocalDate.now(clock) at the start of the method and use that variable for all
subsequent date calculations (replace the calls used in
deleteByWorkRecordContractAndDateAfterAndStatus,
workRecordRepository.deleteByContractIdAndWorkDateAfterAndStatus, and when
computing startDate = today.plusDays(1)), ensuring contractId and any other
repository calls in this method reference the same today value so deletion and
new WorkRecord creation use a consistent boundary.
- Around line 343-353: completedRecords is checked before calling
workRecordRepository.saveAll but not before calling
calculationService.calculateWorkRecordDetailsBatch, causing an unnecessary call
when the list is empty; wrap the calculateWorkRecordDetailsBatch and
workRecordRepository.saveAll calls in the same non-empty guard (e.g., if
(!completedRecords.isEmpty()) {
calculationService.calculateWorkRecordDetailsBatch(completedRecords);
workRecordRepository.saveAll(completedRecords); }) so both operations execute
only when completedRecords has elements, referencing
calculateWorkRecordDetailsBatch and workRecordRepository.saveAll in
WorkRecordCommandService.
geunyong16
deleted the
feature/112-delete-future-work-records-on-termination
branch
This was referenced May 2, 2026
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
📝 변경 사항
주요 변경 내용
WorkRecordCommandService에deleteFutureWorkRecords메서드 추가ContractService.terminateContract에서 미래 근무 기록 삭제 호출CorrectionRequest함께 정리buildActionData에서null반환 대신RuntimeExceptionthrow로 변경 (리뷰 반영)Clock주입으로 테스트 용이성 개선WorkRecordCalculationService에 일괄 계산 메서드 추가상세 설명
근로 계약 해지(퇴사) 시, 해당 근로자의 미래 예정된 SCHEDULED 상태 근무 기록을 일괄 삭제 처리합니다.
ContractService.terminateContract()실행 시WorkRecordCommandService.deleteFutureWorkRecords()를 호출하여 미래 근무 기록을 soft-delete합니다.CorrectionRequest도 함께 정리하여 데이터 정합성을 유지합니다.LocalDate.now()대신Clock을 주입받아 테스트에서 시간을 제어할 수 있도록 개선했습니다.buildActionData에서 JSON 직렬화 실패 시null반환 대신RuntimeException을 throw하여 데이터 무결성을 보장합니다.✅ 체크리스트
Summary by CodeRabbit
릴리스 노트
New Features
Bug Fixes
Chores
Tests