비즈니스 로직 가독성을 위한 도메인 전용 레이어 추가-transfer#227
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the travel and transfer modules, adopting a Command/Reader/UseCase architecture and removing soft-delete logic from travel itineraries. It introduces a comprehensive transfer management system for handling travel-related expenses, including features for creation, updates, and member settlement. The review feedback highlights a potential data consistency issue where the persistence context should be flushed before querying the database for status synchronization in TransferCommand, and suggests strengthening error handling when resolving user identities from encrypted IDs in TravelUseCase.
| private void syncTransferStatus(final Transfer transfer) { | ||
| boolean hasUnsettledMembers = transferUserJpaRepository.existsByTransferIdAndSettledAtIsNull(transfer.getId()); | ||
| transfer.syncStatus(hasUnsettledMembers); | ||
| } |
There was a problem hiding this comment.
syncTransferStatus 메서드에서 existsByTransferIdAndSettledAtIsNull을 호출하여 DB 상태를 조회하고 있습니다. settleTransferUser와 같이 엔티티의 상태만 변경하고 아직 DB에 flush되지 않은 시점에 이 메서드가 호출되면, DB에는 여전히 정산되지 않은 상태로 남아있어 TransferStatus가 올바르게 업데이트되지 않을 수 있습니다.
조회 전 flush()를 호출하여 영속성 컨텍스트의 변경 사항을 DB에 반영해야 정확한 상태 동기화가 가능합니다.
private void syncTransferStatus(final Transfer transfer) {
transferUserJpaRepository.flush();
boolean hasUnsettledMembers = transferUserJpaRepository.existsByTransferIdAndSettledAtIsNull(transfer.getId());
transfer.syncStatus(hasUnsettledMembers);
}|
|
||
| groupReader.findJoinedUserGroup(req.groupId(), userId); | ||
|
|
||
| List<Long> memberIds = req.memberUuids().stream().map(userIdentityResolver::resolve).toList(); |
There was a problem hiding this comment.
saveTravelItinerary 메서드에서 memberUuids를 resolve할 때, 유효하지 않은 UUID가 포함되어 있을 경우 null이 반환되지만 이에 대한 예외 처리가 없습니다. 이로 인해 존재하지 않는 사용자가 멤버 목록에서 소리 없이 제외될 수 있습니다.
클래스 내부에 정의된 resolveMemberUserIdOrThrow 메서드를 활용하여 모든 UUID가 올바르게 해석되는지 검증하는 것이 좋습니다.
| List<Long> memberIds = req.memberUuids().stream().map(userIdentityResolver::resolve).toList(); | |
| List<Long> memberIds = req.memberUuids().stream().map(this::resolveMemberUserIdOrThrow).toList(); |
No description provided.