Skip to content

fix: 물품 승인 처리 및 소유권 승인 관련 문제 #197

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jmj732 merged 1 commit into from
Feb 24, 2026
Merged

fix: 물품 승인 처리 및 소유권 승인 관련 문제 #197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/main/java/com/eod/eod/domain/item/application/ItemClaimService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,12 @@ public void approveClaim(Long claimId, User currentUser) {
// 승인 처리
claim.approve();

// 물품 승인 처리 (소유권 승인 시 물품도 함께 승인)
Item item = claim.getItem();
item.processApproval(Item.ApprovalStatus.APPROVED, currentUser);

// 같은 물품에 대한 다른 PENDING 상태의 주장들을 모두 거절
Comment on lines +57 to 61
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

소유권 주장을 승인할 때 item.processApproval()을 호출하는 것은 좋은 변경이지만, processApproval()에서 status를 GIVEN으로 설정하는 것(Item.java line 147)은 문제를 일으킵니다.

소유권 승인(claim approval)과 물품 승인(item approval)은 다른 개념일 수 있으므로, 이 호출이 적절한지 재검토가 필요합니다.

시스템에 다음 두 가지 승인 플로우가 있습니다:

  1. 직접 물품 승인 (ItemApprovalService.processApproval)
  2. 소유권 주장 승인 시 물품 승인 (여기)

두 플로우가 일관되게 동작하는지 확인이 필요합니다.

Suggested change
// 물품 승인 처리 (소유권 승인 시 물품도 함께 승인)
Item item = claim.getItem();
item.processApproval(Item.ApprovalStatus.APPROVED, currentUser);
// 같은 물품에 대한 다른 PENDING 상태의 주장들을 모두 거절
// 같은 물품에 대한 다른 PENDING 상태의 주장들을 모두 거절
Item item = claim.getItem();

Copilot uses AI. Check for mistakes.
Long itemId = claim.getItem().getId();
Long itemId = item.getId();
List<ItemClaim> otherPendingClaims = itemClaimRepository
.findByItemIdAndStatus(itemId, ItemClaim.ClaimStatus.PENDING);

Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/eod/eod/domain/item/model/Item.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ public void processApproval(ApprovalStatus approvalStatus, User approver) {

if (approvalStatus == ApprovalStatus.APPROVED) {
this.approvalStatus = ApprovalStatus.APPROVED;
this.status = ItemStatus.GIVEN;
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

승인 시 status를 GIVEN으로 설정하면 이후 giveToStudent() 메서드(line 133-139)에서 물품을 실제로 지급하려 할 때 "이미 지급 처리되었습니다" 오류가 발생합니다.

ItemStatus.GIVEN은 물리적인 물품 인도를 의미해야 하는데, 승인(approval) 단계에서 이미 GIVEN으로 설정하면 두 가지 다른 개념이 충돌합니다:

  1. 승인 완료 (approval) - 소유권이 승인됨
  2. 물품 지급 완료 (given) - 물품이 실제로 전달됨

상점 지급을 위해 승인 여부를 확인해야 한다면, RewardRecord의 validateItemGiven() 대신 item.getApprovalStatus()를 확인하는 것이 더 적절합니다.

Suggested change
this.status = ItemStatus.GIVEN;

Copilot uses AI. Check for mistakes.
} else if (approvalStatus == ApprovalStatus.REJECTED) {
this.approvalStatus = ApprovalStatus.REJECTED;
} else {
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/com/eod/eod/domain/reward/application/RewardGiveService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ public void giveRewardToStudent(Long studentId, Long itemId, User currentUser) {
Item item = itemRepository.findById(itemId)
.orElseThrow(() -> new IllegalArgumentException("물품을 찾을 수 없습니다."));

// 중복 상점 지급 방지
if (rewardRecordRepository.existsByItemId(itemId)) {
throw new IllegalStateException("이미 상점이 지급된 물품입니다.");
}

Comment on lines +32 to +36
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

existsByItemId() 메서드만으로는 중복 검사가 불완전할 수 있습니다.

현재 로직:

  1. RewardGiveService에서 existsByItemId() 확인 (line 33)
  2. RewardRecord 생성자에서 validateItemGiven() 확인 (line 41)

두 검사 사이에 race condition이 발생할 수 있습니다. 데이터베이스 레벨에서 item_id에 대한 unique constraint를 추가하거나, 트랜잭션 격리 레벨을 조정하는 것을 고려해야 합니다.

Suggested change
// 중복 상점 지급 방지
if (rewardRecordRepository.existsByItemId(itemId)) {
throw new IllegalStateException("이미 상점이 지급된 물품입니다.");
}

Copilot uses AI. Check for mistakes.
// 상점 지급 기록 생성 (RewardRecord 도메인에서 권한 및 검증 처리)
RewardRecord rewardRecord = RewardRecord.builder()
.student(student)
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/com/eod/eod/domain/reward/infrastructure/RewardRecordRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ public interface RewardRecordRepository extends JpaRepository<RewardRecord, Long
"WHERE r.student.id = :studentId")
List<RewardRecord> findByStudentId(@Param("studentId") Long studentId);

// 물품 ID로 상점 지급 중복 여부 확인
boolean existsByItemId(Long itemId);

// 날짜, 학년, 반으로 상점 지급 이력 조회 (N+1 방지: item, teacher fetch join)
@Query("SELECT r FROM RewardRecord r " +
"JOIN FETCH r.student s " +
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/com/eod/eod/domain/reward/model/RewardRecord.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class RewardRecord {
@Builder
public RewardRecord(User student, Item item, User teacher) {
validateTeacherRole(teacher);
validateItemNotGiven(item);
validateItemGiven(item);
this.student = student;
this.item = item;
this.teacher = teacher;
Expand All @@ -52,10 +52,10 @@ private void validateTeacherRole(User user) {
}
}

// 물품이 이미 지급되지 않았는지 검증
private void validateItemNotGiven(Item item) {
if (item.isGiven()) {
throw new IllegalStateException("이미 지급된 물품입니다.");
// 물품이 지급 완료 상태인지 검증
private void validateItemGiven(Item item) {
if (!item.isGiven()) {
throw new IllegalStateException("지급 처리되지 않은 물품입니다.");
Comment on lines +56 to +58
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

물품이 지급된 상태인지 확인하는 이 검증 로직은 Item.processApproval()에서 status를 GIVEN으로 설정하는 변경사항(Item.java line 147)과 맞물려 문제가 됩니다.

상점 지급의 전제 조건은 "물품이 실제로 학생에게 전달되었는지"가 아니라 "물품의 소유권이 승인되었는지"여야 합니다. 따라서 다음과 같이 수정하는 것이 더 적절합니다:

  • item.isGiven() 대신 item.getApprovalStatus() == ApprovalStatus.APPROVED 확인
  • 또는 별도의 메서드 (예: item.isApproved()) 사용

이렇게 하면 물리적 지급 여부(ItemStatus)와 승인 여부(ApprovalStatus)를 명확히 구분할 수 있습니다.

Copilot uses AI. Check for mistakes.
}
}
}