Conversation
Walkthrough모임 이미지 관리 기능을 재설계하여 단순 문자열 목록에서 정렬 순서와 유효성 검사를 포함한 복합 객체 구조로 변경했습니다. 새로운 에러 코드 4개를 추가하고 서비스 로직을 확장하여 이미지 검증, 중복 방지, 자동 정렬을 지원합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Validator
participant Service
participant DB
Client->>Controller: PATCH /api/v2/groups/{id}<br/>UpdateGroupV2Request<br/>(images: CreateGroupImageV2Request[])
Controller->>Validator: `@Valid` images[]
rect rgba(200, 220, 255, 0.3)
Note over Validator,Service: Phase 1: Input Validation
Validator->>Validator: Check null items, blank keys<br/>Validate sortOrder range (0-2)
Validator->>Validator: Detect duplicate sortOrders
alt Validation Fails
Validator-->>Client: BAD_REQUEST<br/>(INVALID_*, DUPLICATED_*)
end
end
Validator->>Service: Validated items
rect rgba(200, 255, 220, 0.3)
Note over Service: Phase 2: Normalization & Deduplication
Service->>Service: Trim imageKeys<br/>Enforce MAX_IMAGES=3<br/>Remove duplicate keys (keep first)
end
rect rgba(255, 240, 200, 0.3)
Note over Service: Phase 3: Final Order Determination
alt All sortOrders are null
Service->>Service: Use request order as final
else Some sortOrders provided
Service->>Service: Validate no gaps<br/>Auto-assign missing sortOrders<br/>Sort by sortOrder then index
end
Service->>Service: Produce desiredKeys (0..n-1)
end
rect rgba(255, 220, 220, 0.3)
Note over Service,DB: Phase 4: Apply to Database
Service->>DB: Shift existing images to<br/>negative sortOrder
Service->>DB: Flush changes
Service->>DB: Map current state by key
Service->>DB: Create new images<br/>from pre-uploaded assets
Service->>DB: Assign final sortOrder 0..n-1
end
Service-->>Client: 200 OK<br/>Updated Group<br/>(images with sortOrder 0..2)
Estimated code review effort🎯 4 (복잡) | ⏱️ ~45분 Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Pull request overview
This PR updates the group API documentation and refactors the image field structure to use a consistent format across creation and update operations. The main change is migrating from a simple imageKeys string list to a structured images array containing sortOrder and imageKey properties.
- Unified the image field structure by replacing
imageKeyswithimages(containing sortOrder and imageKey) in both create and update operations - Enhanced validation logic to handle explicit sort ordering with automatic gap-filling for null sortOrder values
- Updated HTTP test files to reflect the new API structure and fixed file paths
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| v2-group-update.http | Updated test cases to use the new images array structure with sortOrder and imageKey fields instead of simple imageKeys array |
| v2-group-kick-targets.http | Changed HTTP method from POST to GET for kick-targets endpoint |
| v2-group-create.http | Fixed relative file paths for test image resources (added one more level: ../ → ../../) |
| GroupV2UpdateService.java | Refactored image update logic to handle the new structured CreateGroupImageV2Request format with comprehensive sortOrder validation and automatic assignment |
| UpdateGroupV2Request.java | Changed images field type from List<String> to List<CreateGroupImageV2Request> with @Valid annotation |
| CreateGroupImageV2Request.java | Updated validation annotation from @min to @max for sortOrder field |
| GroupErrorCode.java | Added new error codes for image validation scenarios (invalid items, keys, duplicate sort orders, and invalid ranges) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| String imageKey, | ||
|
|
||
| @Min(value = 0, message = "모임 이미지: 모임 이미지 순서는 0 이상 숫자만 가능합니다.") | ||
| @Max(value = 3, message = "모임 이미지: 모임 이미지 순서는 2 이하만 가능합니다.") |
There was a problem hiding this comment.
The validation annotation and error message are inconsistent. The annotation uses @max(value = 3), but the error message states "모임 이미지 순서는 2 이하만 가능합니다" (order must be 2 or less). Since the code allows a maximum of 3 images with indices 0-2, the annotation should be @max(value = 2) to match the error message, or the error message should be updated to say "3 이하" (3 or less).
| @Max(value = 3, message = "모임 이미지: 모임 이미지 순서는 2 이하만 가능합니다.") | |
| @Max(value = 2, message = "모임 이미지: 모임 이미지 순서는 2 이하만 가능합니다.") |
| import jakarta.validation.constraints.Max; | ||
|
|
||
| public record CreateGroupImageV2Request( | ||
| String imageKey, | ||
|
|
There was a problem hiding this comment.
The @min validation annotation was removed but there's no replacement validation. The code at lines 229-232 in GroupV2UpdateService.java checks if sortOrder is less than 0, but this validation should ideally be enforced at the DTO level as well for consistency. Consider adding @min(value = 0, message = "...") back to ensure negative values are rejected early in the request validation phase.
| import jakarta.validation.constraints.Max; | |
| public record CreateGroupImageV2Request( | |
| String imageKey, | |
| import jakarta.validation.constraints.Max; | |
| import jakarta.validation.constraints.Min; | |
| public record CreateGroupImageV2Request( | |
| String imageKey, | |
| @Min(value = 0, message = "모임 이미지: 모임 이미지 순서는 0 이상이어야 합니다.") |
|
|
||
| @Size(max = 3) | ||
| List<String> images | ||
| @Valid |
There was a problem hiding this comment.
The @SiZe(max = 3) validation was removed from the images field. However, the service layer still enforces the maximum of 3 images at line 247 of GroupV2UpdateService.java. While the validation exists in the service layer, removing the DTO-level validation means invalid requests will be processed further before being rejected, which is less efficient. Consider keeping @SiZe(max = 3) on the List for early validation.
| @Valid | |
| @Valid | |
| @Size(max = 3) |
| * 이미지 업데이트: 요청 imageKeys를 "최종 상태(순서)"로 해석한다. - 요청 순서대로 sortOrder=0.. 부여 (0번이 대표) - 요청에 없는 기존 | ||
| * 이미지는 삭제(orphanRemoval) - 요청에 있고 기존에 없으면 preupload(REDIS) consume 후 새로 생성(insert) - (group_id, | ||
| * sort_order) 유니크를 유지하기 위해 2-phase 임시 sortOrder 사용 | ||
| * applyImagesWithSafeReorder()가 “DTO → 최종 desiredKeys”를 만들어주는 역할 이미지 업데이트: 요청 imageKeys를 "최종 |
There was a problem hiding this comment.
The comment contains a typo. "applyImagesWithSafeReorder()가 'DTO → 최종 desiredKeys'를 만들어주는 역할 이미지 업데이트" appears to be missing proper punctuation. It should be "applyImagesWithSafeReorder()가 'DTO → 최종 desiredKeys'를 만들어주는 역할. 이미지 업데이트:" (adding a period after 역할 and a colon after 업데이트).
| * applyImagesWithSafeReorder()가 “DTO → 최종 desiredKeys”를 만들어주는 역할 이미지 업데이트: 요청 imageKeys를 "최종 | |
| * applyImagesWithSafeReorder()가 “DTO → 최종 desiredKeys”를 만들어주는 역할. 이미지 업데이트: 요청 imageKeys를 "최종 |
| import jakarta.validation.constraints.Max; | ||
| import jakarta.validation.constraints.Min; |
There was a problem hiding this comment.
The imports for @max and @min are unused in this file. These validation annotations are not applied to any fields in UpdateGroupV2Request. Consider removing these unused imports to keep the code clean.
| import jakarta.validation.constraints.Max; | |
| import jakarta.validation.constraints.Min; |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2UpdateService.java (1)
106-109: imageItems가 비어 있을 때 기본 이미지를 추가하지 않습니다.주석에서는 "이미지가 0개면 기본 이미지(variants 2개) 1장 내려주기"라고 명시하고 있지만, 실제 코드는 빈
ArrayList만 생성하고 있습니다. HTTP 테스트(v2-group-update.http)의 기대값과도 불일치합니다:
- 테스트는
imageKey="DEFAULT"인 기본 이미지 1개와 2개의 variants를 기대하지만, 현재 구현은 빈 배열을 반환합니다GroupImageItem.defaultLogo()메서드가 존재하지만 사용되지 않으며, variants 정보가 없는 불완전한 상태입니다빈 경우 기본 이미지를 추가하도록 수정이 필요합니다.
🧹 Nitpick comments (2)
src/main/java/team/wego/wegobackend/group/v2/application/dto/request/UpdateGroupV2Request.java (1)
3-6: 사용하지 않는 import 제거를 권장합니다.
Max와Min어노테이션이 import 되었지만 이 파일에서는 사용되지 않습니다.🔎 제안하는 수정
import jakarta.validation.Valid; -import jakarta.validation.constraints.Max; -import jakarta.validation.constraints.Min; import jakarta.validation.constraints.Size;src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2UpdateService.java (1)
324-330: 방어적 코딩으로 중복 검증이 존재합니다.이 검증은 호출자(
applyImagesWithSafeReorder)에서 이미 수행되지만, 메서드를 독립적으로 유지하는 방어적 코딩 관점에서는 허용됩니다. 향후 다른 곳에서 직접 호출될 가능성이 없다면 제거를 고려할 수 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/team/wego/wegobackend/group/domain/exception/GroupErrorCode.javasrc/main/java/team/wego/wegobackend/group/v2/application/dto/request/CreateGroupImageV2Request.javasrc/main/java/team/wego/wegobackend/group/v2/application/dto/request/UpdateGroupV2Request.javasrc/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2UpdateService.javasrc/test/http/group/v2/v2-group-create.httpsrc/test/http/group/v2/v2-group-kick-targets.httpsrc/test/http/group/v2/v2-group-update.http
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (java)
🔇 Additional comments (7)
src/test/http/group/v2/v2-group-kick-targets.http (2)
152-153: 일관성 있게 GET으로 변경되었습니다.예외 케이스 테스트(MEMBER가 HOST 전용 엔드포인트 호출)도 148라인과 동일하게 GET으로 변경하여 일관성을 유지했습니다. 이는 권한 검증 로직이 HTTP 메서드와 무관하게 작동함을 확인하는 데 도움이 됩니다.
148-149: git diff에 해당 HTTP 테스트 파일의 변경 사항이 없습니다.현재 커밋(6e9f868)은 "모임 생성과 수정 images 필드 동일하게 수정 .http 개선"으로 인코딩되어 있으며, v2-group-kick-targets.http 파일에 대한 변경 사항이 없습니다. 이미 백엔드 컨트롤러에는
/attendance/kick-targets엔드포인트에 대해@GetMapping이 정의되어 있고, HTTP 테스트 파일도 GET 메서드를 사용하고 있습니다.Likely an incorrect or invalid review comment.
src/main/java/team/wego/wegobackend/group/domain/exception/GroupErrorCode.java (1)
11-22: LGTM!새로운 이미지 유효성 검사 에러 코드들이 기존 패턴과 일관되게 추가되었습니다. 메시지 포맷과 HTTP 상태 코드가 적절합니다.
src/main/java/team/wego/wegobackend/group/v2/application/dto/request/UpdateGroupV2Request.java (1)
35-36: LGTM!
images필드가List<String>에서@Valid List<CreateGroupImageV2Request>로 변경되어 중첩된 DTO 유효성 검사가 활성화되었습니다. 최대 이미지 개수 검증은 서비스 레이어에서 처리됩니다.src/test/http/group/v2/v2-group-update.http (1)
101-102: LGTM!업데이트된 이미지 페이로드 구조(sortOrder + imageKey 객체 배열)를 테스트하며, 이미지 재정렬, 삭제, 추가 시나리오를 적절히 검증합니다.
src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2UpdateService.java (2)
204-314: LGTM!이미지 유효성 검사 및 순서 결정 로직이 잘 구현되었습니다.
sortOrder가 모두 null인 경우 요청 순서를 사용하고, 일부만 있는 경우 빈 자리를 자동 배정하는 로직이 적절합니다.
393-399: LGTM!정렬 시 원본 인덱스를 보존하기 위한 내부 레코드로 적절하게 구현되었습니다.
| @Max(value = 3, message = "모임 이미지: 모임 이미지 순서는 2 이하만 가능합니다.") | ||
| Integer sortOrder |
There was a problem hiding this comment.
@Max 값과 메시지가 불일치합니다.
@Max(value = 3)은 sortOrder가 3까지 허용되지만, 메시지는 "2 이하만 가능합니다"라고 표시됩니다. 또한 서비스 로직에서 sortOrder >= MAX_IMAGES(3)일 때 예외를 발생시키므로, 유효한 범위는 0~2입니다.
🔎 제안하는 수정
- @Max(value = 3, message = "모임 이미지: 모임 이미지 순서는 2 이하만 가능합니다.")
+ @Max(value = 2, message = "모임 이미지: 모임 이미지 순서는 2 이하만 가능합니다.")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Max(value = 3, message = "모임 이미지: 모임 이미지 순서는 2 이하만 가능합니다.") | |
| Integer sortOrder | |
| @Max(value = 2, message = "모임 이미지: 모임 이미지 순서는 2 이하만 가능합니다.") | |
| Integer sortOrder |
🤖 Prompt for AI Agents
In
src/main/java/team/wego/wegobackend/group/v2/application/dto/request/CreateGroupImageV2Request.java
around lines 8-9, the @Max annotation value (3) conflicts with its message ("2
이하만 가능합니다") and service logic that allows only 0..2; change the constraint to
@Max(value = 2, message = "모임 이미지: 모임 이미지 순서는 2 이하만 가능합니다.") and add a @Min(0,
message = "모임 이미지: 모임 이미지 순서는 0 이상이어야 합니다.") to explicitly enforce the valid
range 0–2 so the DTO validation matches the service behavior and message.
| < ../../image/resources/test-webp1.webp | ||
| --boundary | ||
| Content-Disposition: form-data; name="images"; filename="test-webp2.webp" | ||
| Content-Type: image/webp | ||
|
|
||
| < ../image/resources/test-webp2.webp | ||
| < ../../image/resources/test-webp2.webp | ||
| --boundary | ||
| Content-Disposition: form-data; name="images"; filename="img1.png" | ||
| Content-Type: image/png | ||
|
|
||
| < ../image/resources/img1.png | ||
| < ../../image/resources/img1.png |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the file structure to understand the directory layout
echo "=== Directory structure for test files ==="
fd -t d "http" src/test | head -20
echo ""
echo "=== File structure around v2-group-create.http ==="
fd -t f -E "*.webp" -E "*.png" "v2-group-create" src/test
echo ""
echo "=== Looking for image resource directories ==="
fd -t d "resources" src/test
echo ""
echo "=== Checking the actual paths in v2-group-create.http (lines 90-115) ==="
sed -n '90,115p' src/test/http/group/v2/v2-group-create.http
echo ""
echo "=== Checking paths at lines 160-230 ==="
sed -n '160,230p' src/test/http/group/v2/v2-group-create.http | grep -n "< \.\."Repository: WeGo-Together/WeGo_BackEnd
Length of output: 1313
🏁 Script executed:
#!/bin/bash
# Verify which paths are correct by checking actual file existence
echo "=== Verifying path correctness ==="
echo ""
echo "Testing ../image/resources/ (from v2-group-create.http location):"
if [ -d "src/test/http/image/resources" ]; then
echo "✓ Directory exists: src/test/http/image/resources"
ls -la src/test/http/image/resources/
else
echo "✗ Directory does NOT exist"
fi
echo ""
echo "Testing ../../image/resources/ (from v2-group-create.http location):"
if [ -d "src/test/image/resources" ]; then
echo "✓ Directory exists: src/test/image/resources"
else
echo "✗ Directory does NOT exist: src/test/image/resources"
fi
echo ""
echo "=== Counting path inconsistencies ==="
echo "Lines using ../../image/resources/:"
rg -c "< \.\./\.\./image/resources/" src/test/http/group/v2/v2-group-create.http
echo "Lines using ../image/resources/:"
rg -c "< \.\./image/resources/" src/test/http/group/v2/v2-group-create.httpRepository: WeGo-Together/WeGo_BackEnd
Length of output: 957
이미지 경로 수정 필요
5개 라인(99, 104, 109 포함)에서 ../../image/resources/ 경로를 사용하고 있으나, 실제 이미지 리소스는 src/test/http/image/resources/에 위치합니다. 파일이 있는 위치(src/test/http/group/v2/)를 기준으로 올바른 상대경로는 ../image/resources/입니다. 모든 경로를 ../image/resources/로 통일하세요.
🤖 Prompt for AI Agents
In src/test/http/group/v2/v2-group-create.http around lines 99 to 109, the image
paths use ../../image/resources/ but the actual resources live at
src/test/http/image/resources/ (relative to this file the correct path is
../image/resources/); update all occurrences on the listed lines to
../image/resources/ so each multipart file reference points to
../image/resources/<filename> and ensure all similar entries in this request use
the unified ../image/resources/ path.
|
아주 좋소 |
📝 Pull Request
📌 PR 종류
해당하는 항목에 체크해주세요.
✨ 변경 내용
요청에 따라서 모임에서 생성과 수정에서 images 필드를 모두 동일하게 수정합니다.
🔍 관련 이슈
🧪 테스트
변경된 기능에 대한 테스트 범위 또는 테스트 결과를 작성해주세요.
🚨 확인해야 할 사항 (Checklist)
PR을 제출하기 전에 아래 항목들을 확인해주세요.
🙋 기타 참고 사항
리뷰어가 참고하면 좋을 만한 추가 설명이 있다면 적어주세요.
Summary by CodeRabbit
릴리스 노트
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.