수기 수정 상시 허용: MANUAL 새 버전 축적 + 상태 409 계약 폐기 - #847
Conversation
- ItemSnapshot.recover(in-place FAILED→READY)를 폐기하고 ItemSnapshot.manual 팩토리로 대체 — 수정은 기존 행을 고치지 않고 출처 MANUAL·편집자(edited_by)가 박힌 새 READY 버전으로 쌓인다(#825 결정 4). 기계 버전 불변이 이력 보존의 바닥이고, 카드가 "마지막 SERVER* READY" 를 믿는 구조에서 수기값이 접히려면 별도 행이어야 한다 - 상태 제한 해제: READY·PENDING·PROCESSING 어느 상태든 수정 가능. ALREADY_READY(ITEM-001)·STILL_PROCESSING(ITEM-002) 409 를 폐기하고 결번 처리 — 진행 중이던 파싱은 자기 행에서 계속돼 완료 시 이력으로 남으므로 충돌 자체가 없다 - 위시: manualEdit 이 wish 행 락으로 refresh 와 직렬화(둘 다 활성 포인터 스왑 경로 — 옛 FAILED-상태 분리 방어의 대체) 후 활성 버전을 base 로 병합·스왑. 병합 후 필수값 부재는 400(ITEM-003~005) 유지 - 토너먼트: 같은 규칙 + pin 이동(repinSnapshot) — "출전 시점 고정"의 의도는 남의 변경으로부터의 격리라, adder 본인의 의도적 수정은 격리를 깨지 않는다 - WishlistApi·TournamentItemApi 의 409 선언·Examples 제거, 수기 수정 계약으로 설명 재작성(응답 전수 문서화 규칙) - 테스트: 409 계약 테스트를 "READY 수정 200 + MANUAL 교체"·"PROCESSING 은 병합 필수값 부재 400"으로 재작성, 기존 행 불변·pin 이동·editedBy 를 통합으로 고정, 에러 카탈로그에서 ITEM-001·002 부재를 단언(결번 박제)
|
Discord 스레드 연동용 메타데이터입니다. discord-pr-bot 워크플로가 자동 생성하며, 수정·삭제하면 PR 과 Discord 알림 연동이 끊깁니다. |
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
WalkthroughFAILED 스냅샷 복구를 제거하고, 모든 상태의 아이템 수정을 새 MANUAL·READY 스냅샷 생성과 활성 포인터 교체로 변경했습니다. 상태 충돌 오류 코드와 관련 API 예시를 제거하고 도메인·토너먼트·위시 흐름의 테스트를 갱신했습니다. Changes수기 수정 스냅샷 흐름
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant WishlistService
participant WishPersistenceService
participant ItemSnapshot
Client->>WishlistService: 수기 수정 요청
WishlistService->>ItemSnapshot: 병합 결과 필수값 검증
WishlistService->>WishPersistenceService: manualEdit 호출
WishPersistenceService->>ItemSnapshot: 새 MANUAL READY 스냅샷 생성
WishPersistenceService->>Client: 새 활성 스냅샷 결과 반환
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/kotlin/com/depromeet/piki/tournament/controller/TournamentItemApi.kt (1)
528-598: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSwagger 설명이 새 정책을 절반만 반영 — 여전히 "FAILED 전용/READY 등 수정 불가(409)" 로 안내
409 응답 설명(Line 598)은
ITEM-001/ITEM-002를 제거해 새 계약에 맞췄지만, 바로 위@Operation의 description(Line 530-536)과 200 응답 설명(Line 543)은 옛 정책 그대로입니다.
- "파싱 실패(FAILED) 상태인 토너먼트 아이템을 유저가 직접 보정한다"
- "수정 성공 시 아이템 상태가 FAILED → READY 로 전환된다"
- "READY·PENDING·PROCESSING 아이템은 수정 불가(409)"
하지만 실제로는 READY/PENDING/PROCESSING 모두 수정 가능하고(
#825결정 4,TournamentIntegrationTest.ktLine 1591~ 에서 검증), 결과는 MANUAL 새 버전으로 pin 이동입니다. 이 상태로 두면 프론트엔드가 문서만 보고 "READY 는 수정 불가"로 오해할 수 있어요.WishlistApi.ktLine 306-321 의 동일 API 설명은 이미 올바르게 갱신됐으니 같은 패턴으로 맞춰주시면 됩니다.📝 제안 수정
`@Operation`( summary = "토너먼트 아이템 수정", description = """ - 파싱 실패(FAILED) 상태인 토너먼트 아이템을 유저가 직접 보정한다. - 수정 성공 시 아이템 상태가 FAILED → READY 로 전환된다. + 토너먼트 아이템의 상품 정보를 유저가 직접 수정한다. 상태 제한이 없다 — + READY·PENDING·PROCESSING·FAILED 모든 상태에서 수정할 수 있다. 수정 가능 필드: 이름, 가격, 가격 단위, 이미지(multipart/form-data 의 image 파트) — null 이면 기존 값 유지. 이미지는 파일로 업로드하며 서버가 S3 에 저장한 URL 로 item.imageUrl 을 갱신한다. - READY·PENDING·PROCESSING 아이템은 수정 불가(409). 아이템을 등록한 본인만 수정 가능. + 수정은 기존 버전을 고치지 않고 MANUAL 새 버전으로 쌓이며 pin 이 그 버전으로 옮겨진다. 아이템을 등록한 본인만 수정 가능. 이름은 수정 후에도 반드시 존재해야 한다 — 기존 이름이 없고 name 도 미입력이면 400. """, ) ... ApiResponse( responseCode = "200", - description = "수정 성공 (FAILED → READY 전환)", + description = "수정 성공 (MANUAL 새 버전으로 교체, 이후 이 버전이 표시됨)",🤖 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/kotlin/com/depromeet/piki/tournament/controller/TournamentItemApi.kt` around lines 528 - 598, Update the Swagger documentation in the tournament item modification `@Operation` description and 200 ApiResponse: describe editing as available for READY, PENDING, and PROCESSING items, and document that success creates or pins a MANUAL version instead of transitioning FAILED to READY. Remove the outdated FAILED-only and “READY·PENDING·PROCESSING 수정 불가(409)” wording, matching the corresponding description in WishlistApi.
🧹 Nitpick comments (2)
src/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.kt (1)
208-249: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value함수명
recoverWishItem이 새 계약과 어울리지 않습니다.이 PR의 핵심은 "복구(recover, 409 충돌 있음)"에서 "수기 수정(manual, 상태 무관 허용)"으로의 전환인데, 내부에서 호출하는
wishPersistenceService.manualEdit은 이름을 잘 바꿨지만 이 함수 자체는 여전히recoverWishItem으로 남아 있어요. 나중에 합류하는 팀원이 "얘는 아직 FAILED 전용 복구인가?"하고 혼동할 수 있습니다.manualEditWishItem같은 이름으로 맞추는 걸 제안합니다(컨트롤러 호출부도 함께 바꿔야 합니다).🤖 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/kotlin/com/depromeet/piki/wishlist/service/WishlistService.kt` around lines 208 - 249, Rename the service method recoverWishItem to manualEditWishItem to reflect the state-independent manual-edit contract, and update all controller and other call sites to use the new name while preserving the existing behavior and parameters.src/test/kotlin/com/depromeet/piki/wishlist/controller/WishlistRefreshIntegrationTest.kt (1)
363-409: 🩺 Stability & Availability | 🔵 TrivialLGTM!
FAILED base + 최신 PROCESSING 공존 상황에서 "활성 포인터가 결정하는 base"와 "기존 행 불변"을 정확히 잡아낸 좋은 회귀 테스트네요.
한 가지 운영 관점 제안:
manualEdit과refresh가 같은 wish 행 락으로 직렬화된다는 것이 이 기능의 핵심 동시성 보장인데(주석에도 명시됨), 현재는 순차 시나리오만 검증되고 있습니다. 두 요청이 동시에 들어왔을 때 한쪽 스왑이 유실되지 않는지 확인하는 동시성 테스트를 추가하면 이 보장을 코드로 고정할 수 있을 것 같습니다.🤖 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/kotlin/com/depromeet/piki/wishlist/controller/WishlistRefreshIntegrationTest.kt` around lines 363 - 409, Extend the wishlist integration tests around manualEdit and refresh to execute both operations concurrently for the same wish and verify they are serialized by the wish-row lock. Assert that both operations complete without losing either snapshot swap, and preserve the existing assertions that the final state and prior snapshot rows reflect the expected concurrent-update behavior.Source: Path instructions
🤖 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.
Inline comments:
In
`@src/main/kotlin/com/depromeet/piki/tournament/service/TournamentItemPersistenceService.kt`:
- Around line 122-126: Prevent concurrent manual edits from overwriting the
pinned snapshot by adding a PESSIMISTIC_WRITE-locked findByIdForUpdate method to
TournamentItemRepository, matching the existing WishRepository pattern. Update
manualEdit to use this method for the tournament item lookup so validation,
ItemSnapshot.manual creation, saving, and repinSnapshot execute against the
locked row.
In
`@src/main/kotlin/com/depromeet/piki/tournament/service/TournamentItemService.kt`:
- Around line 106-108: Invert the productImage condition around
ItemSnapshot.manual validation so the merged required fields are checked before
imageStorage.upload when productImage is present. Keep manualEdit as the final
persistence and authorization step, while ensuring invalid requests cannot
upload an orphaned S3 object.
In `@src/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.kt`:
- Around line 221-248: Fix the pre-upload validation in the manual-edit flow
around ItemSnapshot.manual and productImage: validate the merged name/price
state before uploading any image, not only when productImage is null. Preserve
the existing activeSnapshot lookup and ensure validation failures prevent S3
upload; if validation or manualEdit can still fail after upload, apply the
existing registerFromImages cleanup pattern using the available image deletion
API to remove uploaded objects.
---
Outside diff comments:
In
`@src/main/kotlin/com/depromeet/piki/tournament/controller/TournamentItemApi.kt`:
- Around line 528-598: Update the Swagger documentation in the tournament item
modification `@Operation` description and 200 ApiResponse: describe editing as
available for READY, PENDING, and PROCESSING items, and document that success
creates or pins a MANUAL version instead of transitioning FAILED to READY.
Remove the outdated FAILED-only and “READY·PENDING·PROCESSING 수정 불가(409)”
wording, matching the corresponding description in WishlistApi.
---
Nitpick comments:
In `@src/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.kt`:
- Around line 208-249: Rename the service method recoverWishItem to
manualEditWishItem to reflect the state-independent manual-edit contract, and
update all controller and other call sites to use the new name while preserving
the existing behavior and parameters.
In
`@src/test/kotlin/com/depromeet/piki/wishlist/controller/WishlistRefreshIntegrationTest.kt`:
- Around line 363-409: Extend the wishlist integration tests around manualEdit
and refresh to execute both operations concurrently for the same wish and verify
they are serialized by the wish-row lock. Assert that both operations complete
without losing either snapshot swap, and preserve the existing assertions that
the final state and prior snapshot rows reflect the expected concurrent-update
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 542fd8a3-f1a1-468b-aa1a-bfdfcf0c7344
📒 Files selected for processing (17)
src/main/kotlin/com/depromeet/piki/item/domain/ItemErrorCode.ktsrc/main/kotlin/com/depromeet/piki/item/domain/ItemException.ktsrc/main/kotlin/com/depromeet/piki/item/domain/ItemSnapshot.ktsrc/main/kotlin/com/depromeet/piki/tournament/controller/TournamentItemApi.ktsrc/main/kotlin/com/depromeet/piki/tournament/controller/TournamentItemApiExamples.ktsrc/main/kotlin/com/depromeet/piki/tournament/domain/TournamentItem.ktsrc/main/kotlin/com/depromeet/piki/tournament/service/TournamentItemPersistenceService.ktsrc/main/kotlin/com/depromeet/piki/tournament/service/TournamentItemService.ktsrc/main/kotlin/com/depromeet/piki/wishlist/controller/WishlistApi.ktsrc/main/kotlin/com/depromeet/piki/wishlist/controller/WishlistApiExamples.ktsrc/main/kotlin/com/depromeet/piki/wishlist/service/WishPersistenceService.ktsrc/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.ktsrc/test/kotlin/com/depromeet/piki/common/openapi/ErrorCodeCatalogTest.ktsrc/test/kotlin/com/depromeet/piki/item/domain/ItemSnapshotTest.ktsrc/test/kotlin/com/depromeet/piki/tournament/controller/TournamentIntegrationTest.ktsrc/test/kotlin/com/depromeet/piki/wishlist/controller/WishlistCrudIntegrationTest.ktsrc/test/kotlin/com/depromeet/piki/wishlist/controller/WishlistRefreshIntegrationTest.kt
- 사전 검증(orphan 방지)이 이미지 없는 경로에서만 돌던 반전 버그 수정 — 이미지가 있으면 업로드가 imageUrl 을 채우므로 자리표시 URL(dry-run 전용, 저장 안 됨)로 그 자리만 메워 이름·가격 병합을 항상 검증한다. S3 실패 502 테스트는 검증 통과용 필수값을 채워 시나리오를 유지 - TournamentItemRepository 에 findByIdForUpdate(PESSIMISTIC_WRITE) 추가, manualEdit 이 pin 이동 전 행 락 — 동시 수기 수정 두 건이 같은 pin 을 읽고 나중 커밋이 먼저 만든 MANUAL 버전을 덮는(유령 버전) 경합을 직렬화 (wish 행 락과 같은 결)
1 participant
Situation
Task
Action
도메인
ItemSnapshot.recover폐기 →ItemSnapshot.manual팩토리. base(현재 보던 버전) 값 위에 입력을 병합해 출처 MANUAL·편집자(edited_by)가 박힌 새 READY 버전을 만든다. 기존 행(기계·수기 불문)은 불변.경로별 반영
문서·테스트
Result
연관 이슈
Summary by CodeRabbit
새 기능
변경 사항