fix: 음수 days 등록을 400 LEAVE-013 으로 거절한다 - #277
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 는 코스 통합 테스트에 둔다. 차감 내역을 실제로 만들어야 하는데 그 준비가 이미 그쪽에 있고, 거절 뒤에도 차감이 남아 있는지까지 확인한다 - 기존 "취소하면 되돌아온다" 테스트는 음수 등록으로 검증하고 있어 삭제 기반으로 옮겼다
- 현황을 만드는 경로가 둘인데(내 연차 조회·홈 배지) 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 에 함께 넣으면 배포 순서에 창(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 인 것을 본다.
- 상쇄 등록은 취소가 아니라 새 기록이라, 같은 취소가 두 번 들어오면 그만큼 더 상쇄돼 원장이 틀어진다. 취소는 #265 가 연 DELETE /me/usages/{id} 로 한다. - 0.5 단위 위반(LEAVE-010)과 코드를 가른다. 같은 400 으로 뭉뚱그리면 화면이 "삭제로 취소하세요" 를 안내할 수 없고 사용자는 자기가 숫자를 잘못 넣은 줄 안다. 그 detail 이 안내를 전하는 유일한 통로라 통합 테스트가 문구까지 단언한다. - 거절을 요청 DTO 와 도메인 양쪽에 둔다. 코스 확정 차감이 LeaveUsage.forCourse 를 서비스에서 직접 부르는 경로가 있어 DTO 만으로는 최후의 보루가 되지 않는다. 코스 차감은 애초에 음수가 들어올 길이 없어 사유를 갈라도 소득이 없으므로 수동 내역에만 적용한다. - LEAVE-013 엔트리는 #265 가 자리만 잡아둔 것이다(번호가 append-only 라 비워둘 수 없다). 여기서 던지는 자리를 만들어 그 코드를 쓴다. - 이제 API 로 음수를 넣을 수 없으므로, 옛 상쇄 등록 행이 목록에 보이고 지워지는 시나리오는 JdbcTemplate 으로 도메인을 우회해 심는다 — 하이드레이션은 생성자를 거치지 않아 그게 옛 데이터의 실제 모습이다.
|
Warning Review limit reached
Next review available in: 111 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 |
🚧 Draft 로 둔다 — 머지 조건이 코드 밖에 있다이 PR 은 완성돼 있고 테스트도 전부 통과한다. 그런데 지금 머지하면 이 PR 이 막으려던 것보다 큰 것이 깨진다.
실수로 머지되는 것을 막으려고 draft 로 둔다. 아래 둘이 끝나면 ready 로 바꾼다.
base 가
|
|
|
@coderabbitai review |
|
1 participant
Situation
days등록을 400 으로 거절하는 것까지 함께 넣었다.days등록 (아직 삭제 API 를 안 붙였다)LEAVE-013— 앱이 모르는 코드DELETE /me/usages/{id}063f4c5) 이 PR 로 분리했다.Task
Action
거절과 그 사유
POST /api/v1/leaves/me/usages가 음수days를 400LEAVE-013으로 거절한다.LEAVE-010)과 코드를 가른다. 같은 400 으로 뭉뚱그리면 화면이 "삭제로 취소하세요" 를 안내할 수 없고, 사용자는 자기가 숫자를 잘못 넣은 줄 안다.detail문구가 안내를 사용자에게 전하는 유일한 통로라, 통합 테스트가 code 뿐 아니라 문구까지 단언한다. 바뀌면 화면 안내가 조용히 사라진다.days: 1.5days: 0.3LEAVE-010days: 0LEAVE-010days: -1LEAVE-013어디서 막는가
LeaveDays.isValidUsage를 양수 전용으로 좁히고, 사유를 가르는LeaveDays.isReversal을 둔다. 던지는 자리는AddLeaveUsageRequest.toCommand와LeaveUsage.requireDays두 곳이다.에러코드가 이미 있는 이유
LEAVE-013(LEAVE_USAGE_REVERSAL_NOT_ALLOWED) 엔트리는 feat: 연차 사용 내역 삭제 API · 잔여가 총 연차를 넘지 못하게 한다 #268 이 이미 들고 있다. 거절을 걷어낼 때 그 번호를 지우지 않았다 — 에러코드 번호는 append-only 라 재사용·재배치가 금지다.언제 켜는가 — 이 PR 이 따로 있는 이유
LeaveSummary의 clamp 가 막고, 틀어진 장부는 삭제 API 로 사용자가 정리한다. 남는 것은 원장에 음수 행이 더 쌓이는 것뿐인데, 그것도 clamp 로 가려지고 삭제로 정리된다.테스트가 시대를 넘어가는 자리
Result
DELETE /api/v1/leaves/me/usages/{id}로 전환해 배포dev가 아니라feat/265-leave-usage-delete다.LEAVE-013엔트리를 feat: 연차 사용 내역 삭제 API · 잔여가 총 연차를 넘지 못하게 한다 #268 이 들고 있어,dev기준으로 두면 이 PR 이 같은 엔트리를 다시 추가해 머지 시 충돌한다. 스택으로 두면 diff 가 거절 델타만 남고, feat: 연차 사용 내역 삭제 API · 잔여가 총 연차를 넘지 못하게 한다 #268 이 머지되면 GitHub 가 base 를dev로 자동 재지정한다.LEAVE-010과 코드가 갈리는지,detail문구가 그대로인지, 코스 차감은 여전히 영향받지 않는지를 함께 단언한다 — 이 셋 중 하나만 어긋나도 화면 안내가 조용히 사라진다.연관 이슈