feat: 입금 확인 요청, 승인, 거절 api 추가 및 입금 요청 리스트 조회 api 추가#32
Merged
Conversation
Walkthrough입금 확인 요청 기능을 신규 추가: 도메인 엔티티·상태·예외, 리포지토리 쿼리, 생성/조회/처리/검증 서비스, 명령·쿼리 레이어, 컨트롤러·응답 DTO, 문서와 단위 및 컨트롤러 테스트가 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as PaymentRequestController
participant QuerySettlement as QuerySettlementService
participant Command as CommandPaymentRequest
participant Creator as PaymentRequestCreator
participant Validator as PaymentRequestValidator
participant Repo as PaymentRequestRepository
participant MemberUpd as MemberUpdater
Client->>Controller: POST /groups/{code}/payments (로그인)
Controller->>QuerySettlement: resolveSettlementId(code)
Controller->>Command: createPaymentRequest(settlementId, userId)
Command->>Creator: createPaymentRequest(settlementId, userId)
Creator->>MemberReader: findBySettlementIdAndUserId(settlementId, userId)
Creator->>Validator: validateCreateRequest(settlementId, requestMember)
Creator->>Repo: save(PaymentRequest)
Repo-->>Creator: saved PaymentRequest
Creator-->>Command: PaymentRequest
Command-->>Controller: PaymentRequestResponse
Controller-->>Client: 201 Created (body)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
시
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 8
🧹 Nitpick comments (8)
src/main/java/com/dnd/moddo/event/application/impl/MemberExpenseReader.java (1)
23-24: 메서드 네이밍을 신규 용어(MemberIds)로 통일하면 좋겠습니다.
findAllByAppointMemberIds는 현재 컨텍스트에서 의미가 모호해,findAllByMemberIds로 점진 통합하는 편이 읽기 쉽습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/dnd/moddo/event/application/impl/MemberExpenseReader.java` around lines 23 - 24, Rename the ambiguous method findAllByAppointMemberIds to the preferred consistent name findAllByMemberIds in MemberExpenseReader and update its implementation to call memberExpenseRepository.findAllByAppointmentMemberIds(memberIds) (or, if repository already exposes findAllByMemberIds, call that instead); also update any callers to use MemberExpenseReader.findAllByMemberIds so the naming is uniform across the codebase.src/main/java/com/dnd/moddo/event/infrastructure/MemberExpenseRepository.java (1)
21-25: 동일 의미 조회 메서드가 중복되어 API 표면이 커졌습니다.
findAllByAppointmentMemberIds와findAllByMemberIds가 같은 조건을 수행하고 있어, 하나로 수렴(또는 기존 메서드 deprecate)하면 혼선을 줄일 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/dnd/moddo/event/infrastructure/MemberExpenseRepository.java` around lines 21 - 25, The repository exposes duplicate query methods causing API bloat: in MemberExpenseRepository consolidate the equivalent methods findAllByMemberIds and findAllByAppointmentMemberIds into a single canonical method (choose one name, e.g., findAllByMemberIds) or mark the redundant one as `@Deprecated` and delegate to the canonical method; update the query annotation if needed on the retained method (the existing JPQL "select me from MemberExpense me where me.member.id in :memberIds"), remove the duplicate declaration of the same JPQL, and search & replace callers to use the retained method name (or keep the deprecated wrapper that calls the retained method to preserve backward compatibility).src/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/DuplicatePendingPaymentRequestException.java (1)
8-11: HTTP 상태코드를 409(CONFLICT)로 변경하세요.중복 대기 요청은 잘못된 입력이 아니라 기존 리소스의 상태와 충돌하는 상황입니다.
HttpStatus.CONFLICT가 API 의미를 더 정확히 전달하며, 프로젝트 내 유사한 경우인MemberDuplicateNameException에서도 409를 사용하고 있습니다.수정안
public class DuplicatePendingPaymentRequestException extends ModdoException { public DuplicatePendingPaymentRequestException(Long settlementId, Long requestMemberId) { super( - HttpStatus.BAD_REQUEST, + HttpStatus.CONFLICT, "이미 처리 대기 중인 입금 확인 요청이 있습니다. (Settlement ID: " + settlementId + ", Member ID: " + requestMemberId + ")" ); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/DuplicatePendingPaymentRequestException.java` around lines 8 - 11, Change the HTTP status used by the DuplicatePendingPaymentRequestException constructor from HttpStatus.BAD_REQUEST to HttpStatus.CONFLICT so the exception correctly signals a resource-state conflict; update the constructor in DuplicatePendingPaymentRequestException(Long settlementId, Long requestMemberId) to pass HttpStatus.CONFLICT to the superclass while leaving the existing message and parameters unchanged.src/test/java/com/dnd/moddo/domain/paymentRequest/controller/PaymentRequestControllerTest.java (1)
17-17: 미사용 import
HandlerMethodArgumentResolver가 import 되었지만 이 파일에서 직접 사용되지 않습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/dnd/moddo/domain/paymentRequest/controller/PaymentRequestControllerTest.java` at line 17, Remove the unused import of HandlerMethodArgumentResolver from PaymentRequestControllerTest; locate the import statement "org.springframework.web.method.support.HandlerMethodArgumentResolver" at the top of the PaymentRequestControllerTest class and delete it so there are no unused imports remaining.src/main/java/com/dnd/moddo/event/application/impl/PaymentRequestReader.java (1)
29-31: DB 레벨에서 PENDING 상태 필터링 고려현재 모든 PaymentRequest를 조회한 후 애플리케이션 레벨에서 PENDING 상태를 필터링하고 있습니다. 데이터 양이 많아지면 성능에 영향을 줄 수 있으므로,
PaymentRequestRepository에 상태 필터링이 포함된 쿼리 메서드 추가를 권장합니다.♻️ 제안된 개선
-List<PaymentRequest> paymentRequests = paymentRequestRepository.findByTargetUserId(targetUserId) - .stream().filter(paymentRequest -> paymentRequest.getStatus() == PaymentRequestStatus.PENDING) - .toList(); +List<PaymentRequest> paymentRequests = paymentRequestRepository.findByTargetUserIdAndStatus(targetUserId, PaymentRequestStatus.PENDING);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/dnd/moddo/event/application/impl/PaymentRequestReader.java` around lines 29 - 31, Currently the code fetches all PaymentRequest entities via paymentRequestRepository.findByTargetUserId(...) and then filters for PaymentRequestStatus.PENDING in the stream which can be inefficient; add a repository query method (e.g., findByTargetUserIdAndStatus or findAllByTargetUserIdAndStatus) to PaymentRequestRepository and replace the call in PaymentRequestReader so it directly retrieves only PENDING requests (use PaymentRequestStatus.PENDING as the status parameter) and remove the downstream stream filter to improve DB-level filtering and performance.src/main/java/com/dnd/moddo/event/application/impl/PaymentRequestCreator.java (1)
14-16:@Transactional어노테이션 추가 권장
createPaymentRequest메서드가 여러 읽기 작업과 쓰기 작업을 수행합니다. 명시적인 트랜잭션 관리를 위해@Transactional어노테이션 추가를 고려해 주세요.♻️ 제안
`@Service` +@Transactional `@RequiredArgsConstructor` public class PaymentRequestCreator {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/dnd/moddo/event/application/impl/PaymentRequestCreator.java` around lines 14 - 16, The PaymentRequestCreator service performs multiple reads and writes; annotate it or its createPaymentRequest method with `@Transactional` to ensure atomicity and proper transaction management (e.g., add `@Transactional` on the PaymentRequestCreator class or immediately above the createPaymentRequest method), and import org.springframework.transaction.annotation.Transactional; if you prefer optimized semantics, mark read-only operations with a separate `@Transactional`(readOnly = true) where appropriate.src/main/java/com/dnd/moddo/event/infrastructure/PaymentRequestRepository.java (1)
33-34: 상태 필터를 DB 쿼리로 내려 조회 비용을 줄이세요.현재는 대상 유저의 전체 요청을 가져온 뒤 애플리케이션에서 PENDING만 필터링합니다. 데이터가 커지면 불필요한 조회/매핑 비용이 커집니다.
예시 수정안
- `@Query`("select pr from PaymentRequest pr where pr.targetUser.id = :targetUserId") - List<PaymentRequest> findByTargetUserId(`@Param`("targetUserId") Long targetUserId); + `@Query`(""" + select pr + from PaymentRequest pr + where pr.targetUser.id = :targetUserId + and pr.status = :status + """) + List<PaymentRequest> findByTargetUserIdAndStatus( + `@Param`("targetUserId") Long targetUserId, + `@Param`("status") PaymentRequestStatus status + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/dnd/moddo/event/infrastructure/PaymentRequestRepository.java` around lines 33 - 34, The query in PaymentRequestRepository currently selects all PaymentRequest rows for a target user and leaves PENDING filtering to application code; change the repository query to filter by status in the DB to avoid extra fetch/mapping. Update the repository method (e.g., replace or add findByTargetUserId to findByTargetUserIdAndStatus or modify the `@Query` on findByTargetUserId) to include "and pr.status = :status" (or hardcode Status.PENDING in the JPQL) and add a `@Param`("status") PaymentRequestStatus status parameter so only PENDING requests are returned from the database.src/test/java/com/dnd/moddo/domain/paymentRequest/service/implementation/PaymentRequestReaderTest.java (1)
84-84: 멤버 ID 리스트 순서 고정 스텁은 테스트를 불안정하게 만듭니다.동일한 ID 집합이어도 순서가 달라지면 Line 84 스텁이 깨질 수 있습니다. 순서 비의존 매처로 바꾸는 편이 안전합니다.
예시 수정안
- when(memberExpenseReader.findAllByMemberIds(List.of(11L, 12L))).thenReturn(memberExpenses); + when(memberExpenseReader.findAllByMemberIds(argThat(ids -> + ids.size() == 2 && ids.containsAll(List.of(11L, 12L)) + ))).thenReturn(memberExpenses);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/dnd/moddo/domain/paymentRequest/service/implementation/PaymentRequestReaderTest.java` at line 84, The stub on memberExpenseReader is using an order-sensitive List matcher (when(memberExpenseReader.findAllByMemberIds(List.of(11L, 12L))).thenReturn(...)) which makes the test brittle; change it to an order-independent matcher such as ArgumentMatchers.argThat that compares sets (e.g. argThat(ids -> new HashSet<>(ids).equals(Set.of(11L, 12L)))) so the stub for memberExpenseReader.findAllByMemberIds(...) returns memberExpenses regardless of the input list order.
🤖 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/dnd/moddo/event/application/impl/MemberExpenseReader.java`:
- Around line 35-37: The method MemberExpenseReader.findAllByMemberIds currently
forwards null/empty memberIds to memberExpenseRepository.findAllByMemberIds; add
an early boundary check at the start of MemberExpenseReader.findAllByMemberIds
to return an empty List when memberIds is null or empty to avoid unnecessary
repository calls and clarify behavior. Locate the findAllByMemberIds method in
MemberExpenseReader and implement a null/empty check (e.g., if memberIds == null
|| memberIds.isEmpty()) that returns Collections.emptyList() (or List.of())
before calling memberExpenseRepository.findAllByMemberIds.
In `@src/main/java/com/dnd/moddo/event/application/impl/MemberReader.java`:
- Around line 39-42: The current findBySettlementIdAndUserId in MemberReader
throws MemberNotFoundException(userId) which mislabels the missing identifier;
update the error handling so the thrown exception clearly represents the search
by settlement and user — either create and throw a new exception like
MemberNotFoundBySettlementAndUserException(settlementId, userId) or modify
MemberNotFoundException to accept and format both settlementId and userId;
ensure the change is applied where memberRepository.findBySettlementIdAndUserId
is used so logs and stack traces show the correct identifiers.
In
`@src/main/java/com/dnd/moddo/event/application/impl/PaymentRequestReader.java`:
- Around line 38-42: The stream aggregation in PaymentRequestReader uses
Collectors.summingLong(MemberExpense::getAmount) which will throw NPE if
MemberExpense.getAmount() is null; update the collector used in
memberExpenseReader.findAllByMemberIds(...).stream().collect(...) to defensively
convert nullable amounts to 0L before summing (e.g., replace
MemberExpense::getAmount with a lambda that returns
Optional.ofNullable(e.getAmount()).orElse(0L) or e -> e.getAmount() == null ? 0L
: e.getAmount()), ensuring the Map<Long,Long> amountByMemberId sums safely
without NPE.
In
`@src/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/ManagerPaymentRequestNotAllowedException.java`:
- Around line 8-9: Change the HTTP status in
ManagerPaymentRequestNotAllowedException's constructor from
HttpStatus.BAD_REQUEST to HttpStatus.FORBIDDEN to align with the domain's
authorization errors (see PaymentRequestUnauthorizedException). Also consider
removing the requestMemberId from the exception message to avoid exposing member
IDs (or make that change only after consistency review with other authorization
exceptions); update the message string in the constructor accordingly.
In
`@src/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/PaymentRequestUnauthorizedException.java`:
- Around line 8-12: The exception currently exposes internal IDs in
PaymentRequestUnauthorizedException's constructor message; remove the
PaymentRequest ID and User ID from the message and replace with a generic,
non-sensitive message (e.g., "해당 입금 확인 요청을 처리할 권한이 없습니다.") inside the
PaymentRequestUnauthorizedException(Long paymentRequestId, Long userId)
constructor so GlobalExceptionHandler.handleDefineException() will not leak
internal identifiers to clients; keep the constructor signature but only use IDs
for internal logs if needed (use a logger inside the constructor or where
thrown) rather than returning them in the exception message.
In `@src/main/java/com/dnd/moddo/event/domain/paymentRequest/PaymentRequest.java`:
- Around line 62-70: 현재 PaymentRequest.approve()와 PaymentRequest.reject()에 상태 전이
가드가 없어 여러 번 호출 시 status와 processedAt가 덮어써집니다; approve()와 reject()에서 처리 전 상태가
PaymentRequestStatus.PENDING인지 확인하고 그렇지 않으면 IllegalStateException(또는 도메인 예외)를
던지도록 변경하여 재처리를 방지하세요; 구체적으로 PaymentRequest 클래스의 approve()와 reject()에 "if
(this.status != PaymentRequestStatus.PENDING) throw new
IllegalStateException(...)" 검사를 추가하고, 검사 통과 시에만 status와 processedAt를 갱신하도록
구현하세요.
- Around line 35-57: The constructor PaymentRequest(Settlement settlement,
Member requestMember, User targetUser) lacks null checks causing potential NPEs
when downstream code (and the ID getters that dereference these associations)
access their IDs; add defensive null validation in that constructor (throw
IllegalArgumentException or NullPointerException with clear messages if any arg
is null) and annotate the association mappings (settlement, requestMember,
targetUser) with non-nullability (e.g., `@JoinColumn`(nullable = false) or
equivalent) so DB schema and runtime both enforce the required relationships;
also consider making the ID getters null-safe (explicit checks or Optional) to
avoid direct dereference when associations are absent.
In
`@src/test/java/com/dnd/moddo/domain/paymentRequest/service/CommandPaymentRequestTest.java`:
- Around line 45-77: The tests approvePaymentRequest and rejectPaymentRequest
currently assert PaymentRequestStatus.PENDING for both scenarios; update each
test to stub the PaymentRequest's status appropriately and assert the expected
transition: in approvePaymentRequest, ensure the PaymentRequest mock (used by
when(paymentRequestUpdater.approvePaymentRequest(1L, 2L))) returns
PaymentRequestStatus.APPROVED for getStatus() and assert response.status() ==
PaymentRequestStatus.APPROVED; in rejectPaymentRequest, make the mock return
PaymentRequestStatus.REJECTED and assert response.status() ==
PaymentRequestStatus.REJECTED (you can keep stubPaymentRequest(PaymentRequest)
for common fields and override when(paymentRequest.getStatus()).thenReturn(...)
per test before invoking
commandPaymentRequest.approvePaymentRequest/rejectPaymentRequest).
---
Nitpick comments:
In `@src/main/java/com/dnd/moddo/event/application/impl/MemberExpenseReader.java`:
- Around line 23-24: Rename the ambiguous method findAllByAppointMemberIds to
the preferred consistent name findAllByMemberIds in MemberExpenseReader and
update its implementation to call
memberExpenseRepository.findAllByAppointmentMemberIds(memberIds) (or, if
repository already exposes findAllByMemberIds, call that instead); also update
any callers to use MemberExpenseReader.findAllByMemberIds so the naming is
uniform across the codebase.
In
`@src/main/java/com/dnd/moddo/event/application/impl/PaymentRequestCreator.java`:
- Around line 14-16: The PaymentRequestCreator service performs multiple reads
and writes; annotate it or its createPaymentRequest method with `@Transactional`
to ensure atomicity and proper transaction management (e.g., add `@Transactional`
on the PaymentRequestCreator class or immediately above the createPaymentRequest
method), and import org.springframework.transaction.annotation.Transactional; if
you prefer optimized semantics, mark read-only operations with a separate
`@Transactional`(readOnly = true) where appropriate.
In
`@src/main/java/com/dnd/moddo/event/application/impl/PaymentRequestReader.java`:
- Around line 29-31: Currently the code fetches all PaymentRequest entities via
paymentRequestRepository.findByTargetUserId(...) and then filters for
PaymentRequestStatus.PENDING in the stream which can be inefficient; add a
repository query method (e.g., findByTargetUserIdAndStatus or
findAllByTargetUserIdAndStatus) to PaymentRequestRepository and replace the call
in PaymentRequestReader so it directly retrieves only PENDING requests (use
PaymentRequestStatus.PENDING as the status parameter) and remove the downstream
stream filter to improve DB-level filtering and performance.
In
`@src/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/DuplicatePendingPaymentRequestException.java`:
- Around line 8-11: Change the HTTP status used by the
DuplicatePendingPaymentRequestException constructor from HttpStatus.BAD_REQUEST
to HttpStatus.CONFLICT so the exception correctly signals a resource-state
conflict; update the constructor in DuplicatePendingPaymentRequestException(Long
settlementId, Long requestMemberId) to pass HttpStatus.CONFLICT to the
superclass while leaving the existing message and parameters unchanged.
In
`@src/main/java/com/dnd/moddo/event/infrastructure/MemberExpenseRepository.java`:
- Around line 21-25: The repository exposes duplicate query methods causing API
bloat: in MemberExpenseRepository consolidate the equivalent methods
findAllByMemberIds and findAllByAppointmentMemberIds into a single canonical
method (choose one name, e.g., findAllByMemberIds) or mark the redundant one as
`@Deprecated` and delegate to the canonical method; update the query annotation if
needed on the retained method (the existing JPQL "select me from MemberExpense
me where me.member.id in :memberIds"), remove the duplicate declaration of the
same JPQL, and search & replace callers to use the retained method name (or keep
the deprecated wrapper that calls the retained method to preserve backward
compatibility).
In
`@src/main/java/com/dnd/moddo/event/infrastructure/PaymentRequestRepository.java`:
- Around line 33-34: The query in PaymentRequestRepository currently selects all
PaymentRequest rows for a target user and leaves PENDING filtering to
application code; change the repository query to filter by status in the DB to
avoid extra fetch/mapping. Update the repository method (e.g., replace or add
findByTargetUserId to findByTargetUserIdAndStatus or modify the `@Query` on
findByTargetUserId) to include "and pr.status = :status" (or hardcode
Status.PENDING in the JPQL) and add a `@Param`("status") PaymentRequestStatus
status parameter so only PENDING requests are returned from the database.
In
`@src/test/java/com/dnd/moddo/domain/paymentRequest/controller/PaymentRequestControllerTest.java`:
- Line 17: Remove the unused import of HandlerMethodArgumentResolver from
PaymentRequestControllerTest; locate the import statement
"org.springframework.web.method.support.HandlerMethodArgumentResolver" at the
top of the PaymentRequestControllerTest class and delete it so there are no
unused imports remaining.
In
`@src/test/java/com/dnd/moddo/domain/paymentRequest/service/implementation/PaymentRequestReaderTest.java`:
- Line 84: The stub on memberExpenseReader is using an order-sensitive List
matcher (when(memberExpenseReader.findAllByMemberIds(List.of(11L,
12L))).thenReturn(...)) which makes the test brittle; change it to an
order-independent matcher such as ArgumentMatchers.argThat that compares sets
(e.g. argThat(ids -> new HashSet<>(ids).equals(Set.of(11L, 12L)))) so the stub
for memberExpenseReader.findAllByMemberIds(...) returns memberExpenses
regardless of the input list order.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 12181142-b644-45d8-920c-4bdfa3ebf487
📒 Files selected for processing (36)
src/docs/asciidoc/index.adocsrc/docs/asciidoc/payment.adocsrc/main/java/com/dnd/moddo/event/application/command/CommandPaymentRequest.javasrc/main/java/com/dnd/moddo/event/application/impl/MemberExpenseReader.javasrc/main/java/com/dnd/moddo/event/application/impl/MemberReader.javasrc/main/java/com/dnd/moddo/event/application/impl/MemberUpdater.javasrc/main/java/com/dnd/moddo/event/application/impl/PaymentRequestCreator.javasrc/main/java/com/dnd/moddo/event/application/impl/PaymentRequestReader.javasrc/main/java/com/dnd/moddo/event/application/impl/PaymentRequestUpdater.javasrc/main/java/com/dnd/moddo/event/application/impl/PaymentRequestValidator.javasrc/main/java/com/dnd/moddo/event/application/query/QueryPaymentRequestService.javasrc/main/java/com/dnd/moddo/event/domain/memberExpense/MemberExpense.javasrc/main/java/com/dnd/moddo/event/domain/paymentRequest/PaymentRequest.javasrc/main/java/com/dnd/moddo/event/domain/paymentRequest/PaymentRequestStatus.javasrc/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/DuplicatePendingPaymentRequestException.javasrc/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/ManagerPaymentRequestNotAllowedException.javasrc/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/PaymentRequestAlreadyApprovedException.javasrc/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/PaymentRequestNotFoundException.javasrc/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/PaymentRequestNotPendingException.javasrc/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/PaymentRequestUnauthorizedException.javasrc/main/java/com/dnd/moddo/event/infrastructure/MemberExpenseRepository.javasrc/main/java/com/dnd/moddo/event/infrastructure/MemberRepository.javasrc/main/java/com/dnd/moddo/event/infrastructure/PaymentRequestRepository.javasrc/main/java/com/dnd/moddo/event/presentation/PaymentRequestController.javasrc/main/java/com/dnd/moddo/event/presentation/response/PaymentRequestItemResponse.javasrc/main/java/com/dnd/moddo/event/presentation/response/PaymentRequestResponse.javasrc/main/java/com/dnd/moddo/event/presentation/response/PaymentRequestsResponse.javasrc/main/java/com/dnd/moddo/user/application/impl/UserReader.javasrc/test/java/com/dnd/moddo/domain/paymentRequest/controller/PaymentRequestControllerTest.javasrc/test/java/com/dnd/moddo/domain/paymentRequest/service/CommandPaymentRequestTest.javasrc/test/java/com/dnd/moddo/domain/paymentRequest/service/QueryPaymentRequestServiceTest.javasrc/test/java/com/dnd/moddo/domain/paymentRequest/service/implementation/PaymentRequestCreatorTest.javasrc/test/java/com/dnd/moddo/domain/paymentRequest/service/implementation/PaymentRequestReaderTest.javasrc/test/java/com/dnd/moddo/domain/paymentRequest/service/implementation/PaymentRequestUpdaterTest.javasrc/test/java/com/dnd/moddo/domain/paymentRequest/service/implementation/PaymentRequestValidatorTest.javasrc/test/java/com/dnd/moddo/global/util/ControllerTest.java
src/main/java/com/dnd/moddo/event/application/impl/MemberExpenseReader.java
Show resolved
Hide resolved
src/main/java/com/dnd/moddo/event/application/impl/PaymentRequestReader.java
Show resolved
Hide resolved
...nd/moddo/event/domain/paymentRequest/exception/ManagerPaymentRequestNotAllowedException.java
Outdated
Show resolved
Hide resolved
...com/dnd/moddo/event/domain/paymentRequest/exception/PaymentRequestUnauthorizedException.java
Show resolved
Hide resolved
src/main/java/com/dnd/moddo/event/domain/paymentRequest/PaymentRequest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/dnd/moddo/event/domain/paymentRequest/PaymentRequest.java
Show resolved
Hide resolved
src/test/java/com/dnd/moddo/domain/paymentRequest/service/CommandPaymentRequestTest.java
Show resolved
Hide resolved
📝 테스트 커버리지 리포트입니다!
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/test/java/com/dnd/moddo/domain/paymentRequest/service/CommandPaymentRequestTest.java (1)
1-1: 테스트 패키지 위치가 테스트 대상 클래스의 패키지와 불일치합니다.테스트 클래스가
com.dnd.moddo.domain.paymentRequest.service패키지에 있지만, 테스트 대상인CommandPaymentRequest는com.dnd.moddo.event.application.command패키지에 위치합니다. 일반적으로 테스트 클래스는 대상 클래스와 동일한 패키지 구조를 따르는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/dnd/moddo/domain/paymentRequest/service/CommandPaymentRequestTest.java` at line 1, 테스트 클래스 CommandPaymentRequestTest의 패키지 선언이 대상 클래스 CommandPaymentRequest의 패키지(com.dnd.moddo.event.application.command)와 불일치합니다; CommandPaymentRequestTest의 package 선언을 대상 클래스와 동일한 패키(com.dnd.moddo.event.application.command)로 변경하고 필요하면 import 문을 정리하여 테스트가 같은 패키 구조에서 실행되도록 수정하세요.src/test/java/com/dnd/moddo/domain/paymentRequest/entity/PaymentRequestTest.java (2)
63-83: 재처리 예외 케이스에 교차 전이 시나리오도 추가해 주세요.현재는
approve→approve,reject→reject만 검증합니다. “이미 처리됨” 규칙을 더 견고하게 하려면approve→reject,reject→approve도 포함하는 게 좋습니다.테스트 보강 예시
+ `@Test` + `@DisplayName`("이미 승인된 입금 확인 요청은 거절할 수 없다.") + void rejectFailAfterApprove() { + PaymentRequest paymentRequest = createPaymentRequestWithIds(); + paymentRequest.approve(); + + assertThatThrownBy(paymentRequest::reject) + .isInstanceOf(IllegalStateException.class) + .hasMessage("이미 처리된 입금 요청입니다."); + } + + `@Test` + `@DisplayName`("이미 거절된 입금 확인 요청은 승인할 수 없다.") + void approveFailAfterReject() { + PaymentRequest paymentRequest = createPaymentRequestWithIds(); + paymentRequest.reject(); + + assertThatThrownBy(paymentRequest::approve) + .isInstanceOf(IllegalStateException.class) + .hasMessage("이미 처리된 입금 요청입니다."); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/dnd/moddo/domain/paymentRequest/entity/PaymentRequestTest.java` around lines 63 - 83, Add tests to cover cross-transition reprocessing: in PaymentRequestTest add one test that calls createPaymentRequestWithIds(), invokes paymentRequest.approve(), then asserts that calling paymentRequest.reject() throws IllegalStateException with message "이미 처리된 입금 요청입니다.", and another test that first calls paymentRequest.reject() then asserts that paymentRequest.approve() throws the same IllegalStateException and message; keep naming and style consistent with existing tests (e.g., DisplayName and assertThatThrownBy) and reuse createPaymentRequestWithIds() to construct the PaymentRequest.
22-35: 테스트 픽스처 생성 로직 중복을 줄이면 유지보수가 쉬워집니다.
createPaymentRequest()와createPaymentRequestWithIds()의 데이터 세팅이 거의 동일해서, 한쪽을 재사용하도록 정리하면 변경 포인트를 줄일 수 있습니다.중복 제거 예시
`@Test` `@DisplayName`("입금 확인 요청을 생성하면 초기 상태는 PENDING이다.") void createPaymentRequest() { - Settlement settlement = GroupTestFactory.createDefault(); - Member requestMember = createMember(settlement); - User targetUser = UserTestFactory.createWithEmail("[email protected]"); - - setField(settlement, "id", 1L); - setField(requestMember, "id", 2L); - setField(targetUser, "id", 3L); - - PaymentRequest paymentRequest = PaymentRequest.builder() - .settlement(settlement) - .requestMember(requestMember) - .targetUser(targetUser) - .build(); + PaymentRequest paymentRequest = createPaymentRequestWithIds(); assertThat(paymentRequest.getStatus()).isEqualTo(PaymentRequestStatus.PENDING); assertThat(paymentRequest.getRequestedAt()).isNotNull(); assertThat(paymentRequest.getProcessedAt()).isNull(); }Also applies to: 95-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/dnd/moddo/domain/paymentRequest/entity/PaymentRequestTest.java` around lines 22 - 35, Tests duplicate fixture setup between createPaymentRequest() and createPaymentRequestWithIds(); refactor by having one helper call the other: implement a single createPaymentRequest(...) that builds the PaymentRequest (using GroupTestFactory.createDefault(), createMember(), UserTestFactory.createWithEmail() and the builder in PaymentRequest) and add an optional overload or parameters for ids that, when provided, sets id fields via setField(settlement, "id", ...), setField(requestMember, "id", ...), setField(targetUser, "id", ...); update createPaymentRequestWithIds() to delegate to the new createPaymentRequest(...) with ids so both tests reuse the same construction logic (references: createPaymentRequest, createPaymentRequestWithIds, PaymentRequest.builder()).src/main/java/com/dnd/moddo/event/application/impl/PaymentRequestReader.java (1)
29-31: 상태 필터링은 DB 레벨로 내리는 것을 권장합니다.현재는 대상 사용자 요청을 모두 가져온 뒤 애플리케이션에서
PENDING만 거르므로, 데이터가 많아질수록 불필요한 로딩 비용이 커질 수 있습니다. repository에서 상태 조건까지 함께 조회하면 더 효율적입니다.예시 변경안
- List<PaymentRequest> paymentRequests = paymentRequestRepository.findByTargetUserId(targetUserId) - .stream().filter(paymentRequest -> paymentRequest.getStatus() == PaymentRequestStatus.PENDING) - .toList(); + List<PaymentRequest> paymentRequests = paymentRequestRepository + .findByTargetUserIdAndStatus(targetUserId, PaymentRequestStatus.PENDING);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/dnd/moddo/event/application/impl/PaymentRequestReader.java` around lines 29 - 31, The code in PaymentRequestReader loads all requests via paymentRequestRepository.findByTargetUserId and then filters for PaymentRequestStatus.PENDING in-memory; change the repository call to perform the status filter at DB level (e.g., add/use a method like findByTargetUserIdAndStatus or findByTargetUserIdAndStatusIn) and replace the stream filter so PaymentRequestReader directly queries only PENDING PaymentRequest entries (refer to paymentRequestRepository.findByTargetUserId usage and PaymentRequestStatus.PENDING).src/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/ManagerPaymentRequestNotAllowedException.java (1)
8-9: 사용하지 않는 생성자 인자는 정리해 두는 편이 좋겠습니다.메시지에서 ID를 더 이상 쓰지 않으니 이 시그니처는 호출자만 헷갈리게 합니다.
src/main/java/com/dnd/moddo/event/application/impl/PaymentRequestValidator.java:57-61도 함께 무인자 생성자로 맞추면 예외 API 의미가 더 분명해집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/ManagerPaymentRequestNotAllowedException.java` around lines 8 - 9, The constructor ManagerPaymentRequestNotAllowedException(Long requestMemberId) still takes an unused parameter; change its signature to a no-argument constructor that calls super(HttpStatus.FORBIDDEN, "정산 담당자는 입금 확인 요청을 보낼 수 없습니다."); then update any callers (e.g., in PaymentRequestValidator) to instantiate new ManagerPaymentRequestNotAllowedException() instead of passing an ID so the exception API and usage are consistent and the unused parameter is removed.
🤖 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/dnd/moddo/event/application/impl/MemberUpdater.java`:
- Around line 93-94: The current auto-unassign line in MemberUpdater
(memberRepository.findBySettlementIdAndUserId(...).ifPresent(member ->
member.unassignUser(userId));) can null out the manager link because manager
members are also stored in members.user_id; change the logic to skip unassigning
if the found member is the manager (e.g., add a filter or an if-check using
member.isManager() or member.getRole() == Role.MANAGER before calling
unassignUser), so only non-manager participation rows are auto-unassigned; add a
regression test (e.g., MemberUpdaterTest) that creates a settlement with a
manager member (as set up by MemberCreator) and verifies that invoking the
auto-unassign path for another user does NOT clear the manager's user_id.
In `@src/main/java/com/dnd/moddo/event/domain/member/Member.java`:
- Around line 29-34: The new unique constraint declared in the Member entity
(`@Table` uniqueConstraints name= "uk_members_settlement_user" on columns
settlement_id,user_id) can cause schema-update failures if duplicate rows exist
in production; instead of relying solely on Hibernate ddl-auto, add a
pre-deployment migration that finds and resolves duplicates for (settlement_id,
user_id) and then applies the constraint—implement this as a DB
migration/backfill script (or add Flyway/Liquibase) that (1) queries the members
table for duplicates, (2) consolidates or deletes duplicates according to
business rules, and (3) adds the uk_members_settlement_user constraint in a
separate safe migration step before deploying the updated Member class.
---
Nitpick comments:
In
`@src/main/java/com/dnd/moddo/event/application/impl/PaymentRequestReader.java`:
- Around line 29-31: The code in PaymentRequestReader loads all requests via
paymentRequestRepository.findByTargetUserId and then filters for
PaymentRequestStatus.PENDING in-memory; change the repository call to perform
the status filter at DB level (e.g., add/use a method like
findByTargetUserIdAndStatus or findByTargetUserIdAndStatusIn) and replace the
stream filter so PaymentRequestReader directly queries only PENDING
PaymentRequest entries (refer to paymentRequestRepository.findByTargetUserId
usage and PaymentRequestStatus.PENDING).
In
`@src/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/ManagerPaymentRequestNotAllowedException.java`:
- Around line 8-9: The constructor ManagerPaymentRequestNotAllowedException(Long
requestMemberId) still takes an unused parameter; change its signature to a
no-argument constructor that calls super(HttpStatus.FORBIDDEN, "정산 담당자는 입금 확인
요청을 보낼 수 없습니다."); then update any callers (e.g., in PaymentRequestValidator) to
instantiate new ManagerPaymentRequestNotAllowedException() instead of passing an
ID so the exception API and usage are consistent and the unused parameter is
removed.
In
`@src/test/java/com/dnd/moddo/domain/paymentRequest/entity/PaymentRequestTest.java`:
- Around line 63-83: Add tests to cover cross-transition reprocessing: in
PaymentRequestTest add one test that calls createPaymentRequestWithIds(),
invokes paymentRequest.approve(), then asserts that calling
paymentRequest.reject() throws IllegalStateException with message "이미 처리된 입금
요청입니다.", and another test that first calls paymentRequest.reject() then asserts
that paymentRequest.approve() throws the same IllegalStateException and message;
keep naming and style consistent with existing tests (e.g., DisplayName and
assertThatThrownBy) and reuse createPaymentRequestWithIds() to construct the
PaymentRequest.
- Around line 22-35: Tests duplicate fixture setup between
createPaymentRequest() and createPaymentRequestWithIds(); refactor by having one
helper call the other: implement a single createPaymentRequest(...) that builds
the PaymentRequest (using GroupTestFactory.createDefault(), createMember(),
UserTestFactory.createWithEmail() and the builder in PaymentRequest) and add an
optional overload or parameters for ids that, when provided, sets id fields via
setField(settlement, "id", ...), setField(requestMember, "id", ...),
setField(targetUser, "id", ...); update createPaymentRequestWithIds() to
delegate to the new createPaymentRequest(...) with ids so both tests reuse the
same construction logic (references: createPaymentRequest,
createPaymentRequestWithIds, PaymentRequest.builder()).
In
`@src/test/java/com/dnd/moddo/domain/paymentRequest/service/CommandPaymentRequestTest.java`:
- Line 1: 테스트 클래스 CommandPaymentRequestTest의 패키지 선언이 대상 클래스
CommandPaymentRequest의 패키지(com.dnd.moddo.event.application.command)와 불일치합니다;
CommandPaymentRequestTest의 package 선언을 대상 클래스와 동일한
패키(com.dnd.moddo.event.application.command)로 변경하고 필요하면 import 문을 정리하여 테스트가 같은 패키
구조에서 실행되도록 수정하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0a70436f-37a0-474c-93ae-0bba892e621b
📒 Files selected for processing (18)
src/main/java/com/dnd/moddo/event/application/impl/MemberExpenseReader.javasrc/main/java/com/dnd/moddo/event/application/impl/MemberUpdater.javasrc/main/java/com/dnd/moddo/event/application/impl/PaymentRequestReader.javasrc/main/java/com/dnd/moddo/event/domain/member/Member.javasrc/main/java/com/dnd/moddo/event/domain/member/exception/MemberAlreadyAssignedException.javasrc/main/java/com/dnd/moddo/event/domain/member/exception/MemberNotFoundException.javasrc/main/java/com/dnd/moddo/event/domain/paymentRequest/PaymentRequest.javasrc/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/ManagerPaymentRequestNotAllowedException.javasrc/main/java/com/dnd/moddo/event/domain/paymentRequest/exception/PaymentRequestUnauthorizedException.javasrc/test/java/com/dnd/moddo/domain/Member/service/implementation/MemberDeleterTest.javasrc/test/java/com/dnd/moddo/domain/Member/service/implementation/MemberReaderTest.javasrc/test/java/com/dnd/moddo/domain/Member/service/implementation/MemberUpdaterTest.javasrc/test/java/com/dnd/moddo/domain/memberExpense/service/implementation/MemberExpenseReaderTest.javasrc/test/java/com/dnd/moddo/domain/paymentRequest/controller/PaymentRequestControllerTest.javasrc/test/java/com/dnd/moddo/domain/paymentRequest/entity/PaymentRequestTest.javasrc/test/java/com/dnd/moddo/domain/paymentRequest/service/CommandPaymentRequestTest.javasrc/test/java/com/dnd/moddo/domain/paymentRequest/service/QueryPaymentRequestServiceTest.javasrc/test/java/com/dnd/moddo/domain/paymentRequest/service/implementation/PaymentRequestReaderTest.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/test/java/com/dnd/moddo/domain/paymentRequest/service/implementation/PaymentRequestReaderTest.java
- src/test/java/com/dnd/moddo/domain/paymentRequest/service/QueryPaymentRequestServiceTest.java
- src/test/java/com/dnd/moddo/domain/paymentRequest/controller/PaymentRequestControllerTest.java
- src/main/java/com/dnd/moddo/event/domain/paymentRequest/PaymentRequest.java
src/main/java/com/dnd/moddo/event/application/impl/MemberUpdater.java
Outdated
Show resolved
Hide resolved
- 참여자가 정산담당자일때는 수정안되도록 처리
📝 테스트 커버리지 리포트입니다!
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/com/dnd/moddo/domain/Member/service/implementation/MemberUpdaterTest.java (1)
51-52: 사용되지 않는 Mock 필드를 제거하세요.
S3Bucket은MemberUpdater의 의존성이 아니므로 이 mock 필드는 불필요합니다.🧹 제안하는 수정
`@Mock` - private S3Bucket s3Bucket; - `@Mock` private UserRepository userRepository;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/dnd/moddo/domain/Member/service/implementation/MemberUpdaterTest.java` around lines 51 - 52, MemberUpdaterTest contains an unused mock field S3Bucket that is not a dependency of MemberUpdater; remove the private S3Bucket s3Bucket; `@Mock` declaration from MemberUpdaterTest and any unused import for S3Bucket so the test class only mocks actual dependencies of MemberUpdater (e.g., remove references to S3Bucket from the test class and imports).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/test/java/com/dnd/moddo/domain/Member/service/implementation/MemberUpdaterTest.java`:
- Around line 51-52: MemberUpdaterTest contains an unused mock field S3Bucket
that is not a dependency of MemberUpdater; remove the private S3Bucket s3Bucket;
`@Mock` declaration from MemberUpdaterTest and any unused import for S3Bucket so
the test class only mocks actual dependencies of MemberUpdater (e.g., remove
references to S3Bucket from the test class and imports).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 71737b67-d3fd-42aa-b28f-84c9d862853b
📒 Files selected for processing (2)
src/main/java/com/dnd/moddo/event/application/impl/MemberUpdater.javasrc/test/java/com/dnd/moddo/domain/Member/service/implementation/MemberUpdaterTest.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
🔀반영 브랜치
feature/payment-request -> develop
🔧변경 사항
💬리뷰 요구사항(선택)
Summary by CodeRabbit
릴리스 노트