(fix) : 입금 진행 후 지출 수정 방지#67
Merged
Merged
Conversation
Walkthrough입금 확인 요청 또는 결제 완료 참가자가 있는 정산의 지출 변경을 차단하고, multipart 요청 예외에 대한 경고 로깅 및 400 응답 처리를 추가했습니다. 관련 서비스, 저장소, 예외 처리기와 테스트가 함께 변경되었습니다. Changes지출 수정 잠금
Multipart 예외 처리
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CommandExpenseService
participant PaymentRequestReader
participant MemberReader
Client->>CommandExpenseService: 지출 변경 요청
CommandExpenseService->>PaymentRequestReader: 정산 잠금 여부 확인
PaymentRequestReader->>MemberReader: 결제 완료 참가자 확인
MemberReader-->>PaymentRequestReader: 존재 여부 반환
PaymentRequestReader-->>CommandExpenseService: 입금 요청 또는 완료 참가자 여부
CommandExpenseService-->>Client: 잠금 예외 또는 변경 결과
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
🧹 Nitpick comments (2)
src/main/java/com/dnd/moddo/event/domain/expense/exception/ExpenseModificationLockedException.java (1)
8-10: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win예외 메시지가 모든 동작(생성/수정/삭제/이미지변경)에 대해 "수정할 수 없습니다"로 고정됨.
이 예외는
CommandExpenseService의createExpenses,update,updateImgUrl,delete4곳에서 모두 던져지지만 메시지는 항상 "수정"으로 고정되어 있어, 실제로는 생성이나 삭제를 시도했는데도 "수정할 수 없다"는 메시지가 노출됩니다. 클라이언트/사용자가 오류 원인을 파악하기 어려울 수 있습니다.♻️ 동작명을 파라미터로 받는 개선 예시
public class ExpenseModificationLockedException extends ModdoException { - public ExpenseModificationLockedException(Long settlementId) { - super(HttpStatus.FORBIDDEN, "입금 확인 요청 또는 입금 완료 상태가 있어 지출내역을 수정할 수 없습니다. (Settlement ID: " + settlementId + ")"); - } + public ExpenseModificationLockedException(Long settlementId, String action) { + super(HttpStatus.FORBIDDEN, + "입금 확인 요청 또는 입금 완료 상태가 있어 지출내역을 " + action + "할 수 없습니다. (Settlement ID: " + settlementId + ")"); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/dnd/moddo/event/domain/expense/exception/ExpenseModificationLockedException.java` around lines 8 - 10, Update ExpenseModificationLockedException to accept the attempted expense operation as a parameter and build its message with that operation instead of hardcoding “수정”. Modify CommandExpenseService call sites in createExpenses, update, updateImgUrl, and delete to pass the corresponding operation name, preserving the existing settlementId and forbidden status.src/test/java/com/dnd/moddo/domain/expense/service/CommandExpenseServiceTest.java (1)
301-321: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
createExpenses/updateImgUrl경로의 잠금 실패 테스트가 없음.
update(141-164)와delete(214-236)에는existsBySettlementId=true일 때ExpenseModificationLockedException이 발생하는 테스트가 추가되었지만, 동일하게validateExpenseModifiable이 적용된createExpenses와updateImgUrl에는 잠금 실패 시나리오 테스트가 보이지 않습니다. 이 PR의 핵심 변경사항(4개 진입점 모두 잠금)에 대한 테스트 커버리지가 일관되지 않습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/dnd/moddo/domain/expense/service/CommandExpenseServiceTest.java` around lines 301 - 321, Extend CommandExpenseServiceTest to cover locked-settlement failures for both createExpenses and updateImgUrl. Stub paymentRequestReader.existsBySettlementId(groupId) to return true, assert ExpenseModificationLockedException is thrown, and verify expense creation/image update is not invoked; retain the existing successful-path tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@src/main/java/com/dnd/moddo/event/domain/expense/exception/ExpenseModificationLockedException.java`:
- Around line 8-10: Update ExpenseModificationLockedException to accept the
attempted expense operation as a parameter and build its message with that
operation instead of hardcoding “수정”. Modify CommandExpenseService call sites in
createExpenses, update, updateImgUrl, and delete to pass the corresponding
operation name, preserving the existing settlementId and forbidden status.
In
`@src/test/java/com/dnd/moddo/domain/expense/service/CommandExpenseServiceTest.java`:
- Around line 301-321: Extend CommandExpenseServiceTest to cover
locked-settlement failures for both createExpenses and updateImgUrl. Stub
paymentRequestReader.existsBySettlementId(groupId) to return true, assert
ExpenseModificationLockedException is thrown, and verify expense creation/image
update is not invoked; retain the existing successful-path tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c3323cd6-0198-41e4-b51d-eba8da1006df
📒 Files selected for processing (11)
src/main/java/com/dnd/moddo/common/exception/GlobalExceptionHandler.javasrc/main/java/com/dnd/moddo/common/logging/LoggingUtils.javasrc/main/java/com/dnd/moddo/event/application/command/CommandExpenseService.javasrc/main/java/com/dnd/moddo/event/application/impl/MemberReader.javasrc/main/java/com/dnd/moddo/event/application/impl/PaymentRequestReader.javasrc/main/java/com/dnd/moddo/event/domain/expense/exception/ExpenseModificationLockedException.javasrc/main/java/com/dnd/moddo/event/infrastructure/MemberRepository.javasrc/test/java/com/dnd/moddo/common/exception/GlobalExceptionHandlerTest.javasrc/test/java/com/dnd/moddo/domain/expense/service/CommandExpenseServiceTest.javasrc/test/java/com/dnd/moddo/domain/paymentRequest/service/QueryPaymentRequestServiceTest.javasrc/test/java/com/dnd/moddo/domain/paymentRequest/service/implementation/PaymentRequestReaderTest.java
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.
#️⃣연관된 이슈
X
🔀반영 브랜치
fix/expense-lock-payment-progress -> develop
🔧변경 사항
💬리뷰 요구사항(선택)
X
체크:
Summary by CodeRabbit
버그 수정
400 Bad Request응답을 반환합니다.테스트