feat: 연차 사용 내역 삭제 API · 잔여가 총 연차를 넘지 못하게 한다 - #268
Conversation
- DELETE /api/v1/leaves/me/usages/{usageId} 를 열고, 응답으로 갱신된 내 연차 전체를
내려준다. 화면이 삭제 후 목록·잔여를 한 번의 왕복으로 다시 그린다
- 삭제가 없어 프론트가 음수 days 를 등록해 상쇄하고 있었는데, 같은 취소가 두 번 들어오면
사용 합이 음수로 내려가 잔여가 총 연차를 넘었다(15일 중 2일 쓰고 취소 2회 → 잔여 17)
- 그 불변식을 LeaveSummary 가 갖게 했다. 서비스에 if 를 두면 그 서비스를 안 거치는 경로가
생기는 순간 다시 새므로, 사용 합을 0 아래로 못 내려가게 하는 팩토리로 값객체가 보장한다.
자른 사실은 isLedgerNegative 로 드러내 호출자가 warn 을 남긴다(조용한 실패 금지)
- 음수 등록 자체를 막는다(LEAVE-013). 삭제가 생기면 상쇄 등록은 필요 없고, 그 등록은
재시도·중복 탭에 상한이 없어 장부가 그만큼 틀어진다. 0.5 단위 위반(LEAVE-010)과
코드를 가른 이유는 화면이 "삭제로 취소하세요" 를 안내해야 하기 때문
- 이미 쌓인 음수 행은 마이그레이션으로 건드리지 않는다. 남의 장부를 서버가 지우는 셈이고,
이제 사용자가 그 행을 직접 지울 수 있다. 그때까지는 위 clamp 가 화면을 정상으로 지킨다
- 코스 확정 내역(courseId 있음)은 삭제를 409(LEAVE-014)로 막는다. 그 행은 차감량이자 확정
표식이라 지우면 "코스는 확정인데 연차는 안 깎인" 상태가 남고, 코스 삭제·날짜 변경도 그
행을 전제로 돈다. 404 로 감추지 않는 이유는 자기 내역이 화면에 보이기 때문 — 이유를
알려줘야 코스 화면으로 갈 수 있다
- 없는 내역과 남의 내역은 같은 404(LEAVE-012). 나눠 답하면 번호를 넣어보며 존재 여부를
알아낼 수 있어 코스 조회와 같은 규칙을 따른다. 조회 자체도 소유자를 쿼리 조건에 함께 건다
- 수정(PATCH)은 이번에 넣지 않는다. 삭제 + 재등록으로 같은 결과를 낼 수 있고 프론트도 급하지
않다고 했다. days 수정은 코스 차감량과 어긋날 수 있어 계약을 따로 정해야 한다 (#267)
- "취소 2회 → 잔여 17" 을 두 층에서 잠근다. 단위로는 원장 합이 음수여도 잔여가 총을 넘지 않는 것, 통합으로는 같은 내역을 두 번 지우면 두 번째가 404 이고 잔여가 총 그대로인 것 - LeaveSummary 는 경계값을 표로 망라했다 — 잔여 = 총(사용 0), 잔여 0(딱 맞게 씀), 초과 사용(음수 유지), 음수 원장(-0.5 ~ -99). 위쪽만 막고 아래쪽은 여는 비대칭이 핵심이라 두 방향을 함께 둔다 - 음수 등록 400 은 code 뿐 아니라 detail 까지 단언한다. 이 문구가 "삭제로 취소하세요" 를 사용자에게 전하는 유일한 통로라, 바뀌면 화면 안내가 조용히 사라진다 - 코스 확정 내역 삭제 409 는 코스 통합 테스트에 둔다. 차감 내역을 실제로 만들어야 하는데 그 준비가 이미 그쪽에 있고, 거절 뒤에도 차감이 남아 있는지까지 확인한다 - 기존 "취소하면 되돌아온다" 테스트는 음수 등록으로 검증하고 있어 삭제 기반으로 옮겼다
|
Warning Review limit reached
Next review available in: 110 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
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 |
- 현황을 만드는 경로가 둘인데(내 연차 조회·홈 배지) warn 이 한쪽에만 있었다. remainingDaysOrNull 이 LeaveSummary.of 를 직접 불러, 원장 합이 음수인 소유자가 홈만 열면 clamp 가 조용히 일어났다. 더 자주 불리는 쪽이 그쪽이라 방향이 반대였다. - 두 경로를 private summaryOf(guestId, totalDays) 하나로 모아 자르는 곳과 알리는 곳을 일치시켰다. 홈 배지는 총 연차를 이미 손에 들고 있어 조회를 한 번 아끼는데, 그 최적화가 로그를 건너뛰는 이유가 되지 않게 총 연차를 인자로 받는 오버로드를 뒀다.
- 새 삭제 엔드포인트에만 401 이 붙어 있어, 나머지 다섯 개가 공개로 읽혔다. SecurityConfig 는 anyRequest().authenticated() 라 여섯 개 모두 401 도달 가능하다. 한 곳만 적으면 없는 쪽이 permitAll 이라는 뜻이 되므로 전부에 붙이고, 인터페이스 주석에 근거(#122 인증 게이트)를 남겼다. - LeaveUsage 의 usedOn·days 필드 주석이 "증감 · 취소는 음수" 로 남아 있었다. 클래스 주석만 고치고 필드를 놓친 자리다. 하이드레이션이 생성자를 거치지 않아 옛 음수 행은 그대로 읽힌다는 사실도 함께 적었다. - MyLeaveResponse.usedDays 를 "내역 합" 이라 설명하던 것을 고쳤다. 이제 0 으로 잘리므로 옛 음수 행이 있으면 usages 를 더한 값과 다르다 — 클라이언트가 목록으로 검산하면 서버와 어긋난다.
- "마이그레이션으로 지우지 않고 사용자가 직접 지운다" 는 이 PR 의 결정이 테스트로 잠기지 않아, 그 행들이 실제로 어떻게 보이는지 아무도 확인하지 않은 상태였다. - API 로는 더 이상 음수를 넣을 수 없으므로 JdbcTemplate 으로 도메인을 우회해 심는다. 옛 데이터의 실제 모습이 그렇다 — 하이드레이션은 생성자를 거치지 않는다. - 한 시나리오로 셋을 잠근다. 원장 -2 여도 잔여가 17 이 아니라 15 인 것(clamp 가 단위 테스트가 아니라 실데이터에서 도는 것), 그래도 음수 행이 목록에 그대로 나가는 것(감추지 않기로 한 사양), 지우고 나면 usedDays 2·잔여 13 으로 장부가 맞아떨어지는 것. - clamp 를 제거해 돌려 이 테스트가 실제로 깨지는 것을 확인했다.
리뷰 — PR #268 (CodeRabbit 레이트리밋 대체)본문의 주장을 코드로 짚어가며 봤다. 결론: 설계는 맞다. 다만 머지 전에 사람이 정해야 할 것이 하나 있고(배포 순서), 자잘한 지적 넷은 이 브랜치에 직접 고쳐 올렸다. 검증한 것
좋았던 것 (근거 있는 것만)
지적🔴 [High · 머지 전 사람이 결정할 것] ① 음수 등록 400 은 배포 순서에 창(window)을 만든다 — 본문의 대응이 성립하지 않았다본문 Result 에 이렇게 적혀 있었다.
이 순서는 불가능하다. 프론트가 상쇄 등록을 삭제로 바꾸려면
이 PR 에서 가장 큰 실무 위험이고, 코드로는 해결되지 않는다. 선택지는 둘이다.
기술적으로는 ⓑ 가 위험/이득 비가 낫다고 본다. 다만 이건 릴리스 조율 판단이라 리뷰어가 대신 정하지 않았다. 본문 해당 bullet 만 표적 수정해 순서가 백엔드→프론트로 고정이라는 사실과 두 선택지를 적어뒀다(다른 서술은 건드리지 않았다). 🟡 [Medium · 고쳤다 →
|
대응 커밋 — 위 리뷰의 지적을 이 브랜치에 직접 고쳐 올렸다
본문 표적 수정 1건 — Result 의 "프론트가 … 바꾼 뒤 배포해야 한다" bullet 만 고쳤다. 그 순서가 불가능하다는 사실(백엔드→프론트 고정)과 선택지 ⓐ/ⓑ 를 적었다. 나머지 서술은 그대로 뒀다. 고치지 않은 것 — ① 배포 순서(사람이 결정할 릴리스 조율 사안), 재검증
|
- 거절을 이 PR 에 함께 넣으면 배포 순서에 창(window)이 생긴다. 프론트는 삭제 API 가 배포된 뒤에야 갈아탈 수 있어 순서가 백엔드 → 프론트로 고정인데, 그 사이 앱의 "취소" 는 400 LEAVE-013 을 받는다. 지금 앱은 그 코드를 모르고 삭제도 안 붙였으므로 그 구간 동안 사용자가 취소를 아예 못 한다. 리뷰에서 드러나 거절만 #276 으로 떼어냈다. - 미뤄도 이 PR 의 본질은 남는다. 사용자 눈에 보이는 "잔여가 총을 넘는" 증상은 LeaveSummary 의 clamp 가 막고, 틀어진 장부는 새 삭제 API 로 정리된다. 남는 것은 원장에 음수 행이 더 쌓이는 것뿐인데 그건 clamp 로 가려지고 삭제로 정리된다. - 되돌린 범위: isValidUsage 를 음수 허용으로 복구, isReversal 제거, 요청 DTO·도메인의 throw 두 곳 제거, 관련 문서·테스트. 삭제 API·clamp·409·404·warn 일원화는 그대로 둔다. - LEAVE-013 엔트리와 LeaveException 팩토리는 지우지 않았다. 에러코드 번호가 append-only 라 재사용·재배치가 금지이고, #276 이 그 코드를 그대로 쓴다. 지금 아무도 던지지 않아 죽은 코드로 보이므로 양쪽 javadoc 에 "#276 이 호출부를 만든다" 를 박았다. - 거절 테스트는 지우지 않고 뒤집었다. 음수 등록이 다시 201 로 통과하는 것을 단언한다 — 되돌리다 반쯤 남기는 것을 막는 자리다. 재현 시나리오 테스트도 JdbcTemplate 우회 대신 실제 앱 경로(상쇄 등록 두 번)로 바꿔, 원장 -2 에서 잔여가 17 이 아니라 15 인 것을 본다.
ⓑ 채택 — 음수 등록 거절을 이 PR 에서 걷어내 #276 으로 분리했다앞선 리뷰의 🔴 ① 에 대한 결정이다. 음수 왜거절을 여기 두면 배포 순서에 창(window)이 생긴다. 프론트는 이 PR 이 배포된 뒤에야 삭제 API 로 갈아탈 수 있어 순서가 백엔드 → 프론트로 고정인데, 그 사이 앱의 "취소" 가 미뤄도 이 PR 의 본질은 그대로다.
걷어낸 범위 —
|
| 되돌린 것 | 그대로 둔 것 |
|---|---|
LeaveDays.isValidUsage 를 양수 전용으로 좁힌 것 → 음수 허용 복구 |
DELETE /me/usages/{id} 삭제 API 전체 |
LeaveDays.isReversal 도입 |
LeaveSummary clamp + 불변식 + ledgerDays |
AddLeaveUsageRequest.toCommand 의 throw |
코스 확정 내역 삭제 차단 409 LEAVE-014 |
LeaveUsage.requireDays 의 throw |
없는/남의 내역 404 LEAVE-012 |
거절 관련 문서(@ApiResponse 400 설명, schema, javadoc) |
리뷰로 고친 warn 일원화·401 전수 문서화·주석 정정 |
LEAVE-013 엔트리와 LeaveException 팩토리는 지우지 않았다. 에러코드 번호가 append-only 라 재사용·재배치가 금지고, #276 이 그 코드를 그대로 쓴다. 지금 아무도 던지지 않아 죽은 코드로 보이므로 양쪽 javadoc 에 "#276 이 호출부를 만든다" 를 박아 의도를 남겼다.
반쯤 남지 않았는지 어떻게 확인했나
되돌리다 절반만 남는 것이 제일 나쁘므로 세 겹으로 봤다.
- 거절 테스트를 지우지 않고 뒤집었다 —
음수_등록은_아직_받는다()가-1등록이 201 로 통과하고 응답에days: -1.0이 실리는 것을 단언한다. 거절이 어딘가에 남아 있으면 이 테스트가 깨진다. dev와 코드 레벨로 대조했다 —LeaveDays·AddLeaveUsageRequest.toCommand·LeaveUsage.requireDays의 실행 코드가dev와 동일함을 확인했다(LeaveDays는 리터럴0이NONE상수로 바뀐 것만 차이 — 의미 동일).- 전수 grep —
isReversal/leaveUsageReversalNotAllowed호출부가src전체에 0건.
재현 시나리오 테스트도 JdbcTemplate 우회 대신 실제 앱 경로(상쇄 등록 두 번)로 바꿨다. 원장 +2, -2, -2 에서 잔여가 17 이 아니라 15 이고, 음수 행 2건이 목록에 그대로 보이며, 지우면 usedDays 2 / 잔여 13 으로 맞아떨어지는 것을 한 시나리오로 잠근다.
재검증
./gradlew cleanTest test→ 1347 tests / 0 failures / 21 skipped, BUILD SUCCESSFUL- 1357 → 1347 은
isReversal파라미터 테스트 10건이 빠진 수와 정확히 일치한다
- 1357 → 1347 은
- 컨벤션 훅 변경 파일 전수 → 차단 0
후속
- 이슈 음수 days 등록을 400 LEAVE-013 으로 거절 (앱이 삭제 API 로 갈아탄 뒤) #276 — 음수
days등록을 400LEAVE-013으로 거절 - 머지 조건: 앱이 상쇄 등록을 삭제 API 로 갈아탄 뒤에 머지·배포한다
본문도 바뀐 범위에 맞춰 해당 bullet 들만 표적 수정했다. 머지는 하지 않았다.
|
1 participant
Situation
POST /api/v1/leaves/me/usages로 쌓기만 할 수 있고DELETE는 404 였다.days를 새로 등록해 상쇄하는 방식으로 취소를 흉내내고 있었다. 그 우회가 실제로 데이터를 망가뜨렸다.Task
Action
불변식을 값객체로 내렸다
되돌리기를 등록에서 삭제로 옮겼다
DELETE /api/v1/leaves/me/usages/{usageId}— 200 과 함께 갱신된 내 연차 전체(총·쓴·남은 + 내역 목록)를 준다. 화면이 한 번의 왕복으로 다시 그린다.LEAVE-013), 리뷰에서 배포 순서에 창(window)이 생긴다는 것이 드러났다. 프론트는 이 PR 이 배포된 뒤에야 삭제로 갈아탈 수 있어 순서가 백엔드 → 프론트로 고정인데, 그 사이 앱의 "취소" 가 400 을 받아 사용자가 취소를 아예 못 한다.LEAVE-013엔트리는 지우지 않고 남겼다. 에러코드 번호가 append-only 라 재사용·재배치가 금지이고 음수 days 등록을 400 LEAVE-013 으로 거절 (앱이 삭제 API 로 갈아탄 뒤) #276 이 그 코드를 그대로 쓴다. 지금 아무도 던지지 않아 죽은 코드로 보이므로 javadoc 에 그 사정을 박아뒀다.이미 쌓인 음수 데이터
지울 수 있는 것과 없는 것
LEAVE-012LEAVE-012LEAVE-014이번에 넣지 않은 것 — 수정(PATCH)
days수정은 코스가 들고 있는 차감량과 어긋날 수 있어 분기가 는다Result
GET /me를 다시 부르지 않아도 된다.days등록은 지금처럼 201 로 계속 받는다 — 앱이 삭제로 갈아탈 때까지 그 경로가 열려 있어야 한다. 거절은 음수 days 등록을 400 LEAVE-013 으로 거절 (앱이 삭제 API 로 갈아탄 뒤) #276 이 앱 전환 뒤에 닫는다. 통합 테스트가 음수 등록이 다시 201 로 통과하는 것을 단언해, 되돌리다 반쯤 남는 것을 막는다.usages[].days는 상쇄 등록이면 음수다(옛 행도, 음수 days 등록을 400 LEAVE-013 으로 거절 (앱이 삭제 API 로 갈아탄 뒤) #276 전까지 새로 들어오는 것도). 그 행도 삭제 대상이다 — 사용자가 목록에서 지워 장부를 정리한다.작업을 마치기 전 자문 셋
연관 이슈