Conversation
모임 호스트가 그룹 세부 정보를 업데이트할 수 있는 기능을 추가합니다. 여기에는 필드 업데이트, 그룹 상태 변경, 태그 수정, 이미지 순서 변경 및 삭제 기능을 포함한 이미지 관리 등이 포함됩니다. 업데이트 과정 중 유효성 검사 및 권한 부여 실패에 대한 새로운 오류 코드가 도입되었습니다.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughV2 모임 수정 기능을 추가하고 관련 DTO/응답/도메인/서비스/컨트롤러를 구현했으며, 이미지 안전 재정렬과 태그 검증 로직을 도입하고 DB/Redis 관련 예외 처리 및 여러 그룹 오류 코드를 확장했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as GroupV2Controller
participant Service as GroupV2UpdateService
participant Repo as GroupV2Repository
participant TagSvc as TagService
participant Redis as RedisCache
participant DB
Client->>Controller: PATCH /api/v2/groups/{groupId} (UpdateGroupV2Request)
Controller->>Service: update(userId, groupId, request)
Service->>Repo: findById(groupId)
Repo-->>Service: GroupV2 entity
Service->>Service: assert host & assertUpdatable()
Service->>Service: apply scalar changes (title, desc, address, time, max)
Service->>TagSvc: findOrCreateTags(tagNames)
TagSvc-->>Service: tag entities
Service->>Service: remove/add GroupTagV2 entries
Service->>Redis: fetch pre-upload images by imageKey
Redis-->>Service: image metadata (validate owner)
Service->>Service: remove images not in request, set temp negative sortOrders
Service->>Service: add new images from Redis
Service->>Service: apply final sort order (0..N) or add defaultLogo()
Service->>Repo: save(groupV2)
Repo->>DB: INSERT/UPDATE
DB-->>Repo: success
Repo-->>Service: saved entity
Service-->>Controller: UpdateGroupV2Response
Controller-->>Client: 200 OK (ApiResponse<UpdateGroupV2Response>)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
src/main/java/team/wego/wegobackend/group/v2/application/dto/common/GroupImageItem.java (1)
15-18: 하드코딩된 S3 URL을 설정으로 외부화 권장기본 이미지 URL이 코드에 직접 하드코딩되어 있습니다. S3 버킷명이나 리전 변경 시 코드 수정이 필요하게 됩니다.
application.yml또는 환경 변수에서 base URL을 주입받는 방식을 고려해주세요.🔎 설정 외부화 예시:
// application.yml에서 주입받도록 변경 // @Value("${app.default-image.base-url}") 또는 @ConfigurationProperties 사용 // 또는 정적 팩토리 메서드에 URL을 파라미터로 전달 public static GroupImageItem defaultLogo(String baseUrl) { String default100 = baseUrl + "/default/group_logo_100x100.webp"; String default440 = baseUrl + "/default/group_logo_440x240.webp"; // ... }src/main/java/team/wego/wegobackend/group/v2/domain/entity/GroupV2.java (2)
149-152:removeImage와의 일관성 문제:unassign()메서드 사용 권장
removeImage()는image.unassign()을 호출하지만,removeTag()는groupTag.assignTo(null)을 호출합니다.GroupTagV2에unassign()메서드가 없다면 추가하여 일관성을 유지하는 것이 좋습니다.🔎 일관성 있는 구현 예시:
public void removeTag(GroupTagV2 groupTag) { this.groupTags.remove(groupTag); - groupTag.assignTo(null); + groupTag.unassign(); // GroupTagV2에 unassign() 메서드 추가 필요 }
GroupTagV2에 추가:void unassign() { this.group = null; }
252-258:softDelete사용 여부 확정 및 상태 변경 고려주석에 "사용할 지 안할지 아직 잘 모름"이라고 되어 있습니다. 사용하지 않을 예정이라면 제거하고, 사용할 예정이라면 주석을 정리해주세요.
또한 soft delete 시
status를CANCELLED로 변경하는 것이 일반적인 패턴인지 검토가 필요합니다. 현재는deletedAt만 설정하고 상태는 유지됩니다.src/main/java/team/wego/wegobackend/group/v2/application/dto/request/UpdateGroupV2Request.java (1)
25-25:maxParticipants에 최소값 검증 누락
maxParticipants가 null이 아닌 값으로 전달될 경우, 0 또는 음수 값이 허용될 수 있습니다. 서비스 레이어에서 검증하지 않는다면 데이터 무결성 문제가 발생할 수 있습니다.🔎 수정 제안:
+import jakarta.validation.constraints.Min; + public record UpdateGroupV2Request( @Size(max = 50) String title, @Size(max = 300) String description, String location, String locationDetail, LocalDateTime startTime, LocalDateTime endTime, + @Min(1) Integer maxParticipants,src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2UpdateService.java (3)
49-52: 하드코딩된 S3 URL을 설정으로 외부화하는 것을 권장합니다기본 이미지 URL이 하드코딩되어 있습니다. 환경별로 다른 버킷이나 CDN을 사용하거나 URL이 변경될 경우 코드 수정이 필요합니다.
@Value또는@ConfigurationProperties를 사용하여 설정 파일에서 주입받는 것이 좋습니다.🔎 설정 외부화 예시
@Value("${wego.group.default-image.100x100}") private String default100; @Value("${wego.group.default-image.440x240}") private String default440;
105-113: 불변 리스트 재할당 관련 코드 명확성 개선
toList()는 불변 리스트를 반환하므로 라인 112에서 재할당이 필요합니다. 현재 코드는 동작하지만, 처음부터 가변 리스트를 사용하거나 삼항 연산자로 한 번에 처리하면 의도가 더 명확해집니다.🔎 개선 예시
- List<GroupImageItem> imageItems = images.stream() - .sorted(Comparator.comparingInt(GroupImageV2::getSortOrder)) - .map(GroupImageItem::from) - .toList(); - -// 이미지가 0개면 기본 이미지(variants 2개) 1장 내려주기 - if (imageItems.isEmpty()) { - imageItems = List.of(defaultLogoItem()); - } + List<GroupImageItem> imageItems = images.isEmpty() + ? List.of(defaultLogoItem()) + : images.stream() + .sorted(Comparator.comparingInt(GroupImageV2::getSortOrder)) + .map(GroupImageItem::from) + .toList();
54-54: EntityManager 직접 주입에 대한 고려Repository 패턴과 함께
EntityManager를 직접 주입하여 사용하고 있습니다. 2-phase 재정렬을 위해flush()가 필요한 것은 이해되지만, 커스텀 Repository 메서드로 캡슐화하면 테스트 용이성과 일관성이 향상될 수 있습니다.src/main/java/team/wego/wegobackend/group/domain/exception/GroupErrorCode.java (1)
19-23: 중복된 의미의 에러 코드 정리 권장
TAG_DUPLICATED(라인 20)와TAG_NAME_DUPLICATED(라인 23)가 유사한 의미를 가집니다. 또한GROUP_ONLY_HOST_CAN_UPDATE(라인 33)와NO_PERMISSION_TO_UPDATE_GROUP(라인 62-63)도 비슷한 목적입니다.향후 유지보수 시 혼란을 방지하기 위해 하나로 통합하거나 명확한 사용 기준을 문서화하는 것을 권장합니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/main/resources/static/limdaeil-logo.jpegis excluded by!**/*.jpegsrc/test/http/image/resources/img4.jpgis excluded by!**/*.jpg
📒 Files selected for processing (15)
src/main/java/team/wego/wegobackend/common/exception/GlobalExceptionHandler.java(2 hunks)src/main/java/team/wego/wegobackend/group/domain/exception/GroupErrorCode.java(2 hunks)src/main/java/team/wego/wegobackend/group/v2/application/dto/common/GroupImageItem.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/application/dto/request/UpdateGroupV2Request.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/application/dto/response/CreateGroupV2Response.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/application/dto/response/UpdateGroupV2Response.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2Service.java(0 hunks)src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2UpdateService.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/domain/entity/GroupTagV2.java(2 hunks)src/main/java/team/wego/wegobackend/group/v2/domain/entity/GroupV2.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/presentation/GroupV2Controller.java(4 hunks)src/test/http/group/v2/v2-group-create.http(1 hunks)src/test/http/group/v2/v2-group-get-list.http(1 hunks)src/test/http/group/v2/v2-group-get.http(1 hunks)src/test/http/group/v2/v2-group-update.http(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2Service.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2UpdateService.java (1)
src/main/java/team/wego/wegobackend/group/domain/exception/GroupException.java (1)
GroupException(6-15)
⏰ 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 (15)
src/main/java/team/wego/wegobackend/group/v2/domain/entity/GroupTagV2.java (1)
21-26: LGTM! 복합 유니크 제약조건 추가
(group_id, tag_id)조합에 대한 유니크 제약조건은 동일 그룹에 중복 태그 할당을 방지하여 데이터 무결성을 보장합니다.기존 데이터에 중복이 있다면 마이그레이션 시 제약조건 위반이 발생할 수 있으니, 배포 전 기존 데이터 검증을 권장합니다.
src/main/java/team/wego/wegobackend/group/v2/application/dto/response/CreateGroupV2Response.java (1)
5-5: LGTM! 공통 Address DTO 사용으로 리팩터링중첩된
Address레코드를 제거하고 공통 패키지의Address를 사용하도록 변경한 것은 코드 중복을 줄이고 일관성을 높이는 좋은 개선입니다.src/main/java/team/wego/wegobackend/group/v2/application/dto/common/GroupImageItem.java (1)
30-54: LGTM! 기본 로고 팩토리 메서드이미지가 없는 그룹에 대해 기본 로고를 제공하는
defaultLogo()메서드 구현이 적절합니다.nullID를 사용하여 DB 엔티티가 아님을 명확히 표현하고 있습니다.src/main/java/team/wego/wegobackend/group/v2/domain/entity/GroupV2.java (3)
154-161: LGTM! 수정 가능 상태 검증 로직삭제된 그룹과 종료 상태(
CANCELLED,FINISHED)인 그룹에 대한 수정을 방지하는 검증 로직이 적절합니다.
200-241: LGTM! 시간 변경 로직시작/종료 시간 개별 수정 및 동시 수정을 지원하며,
endTime이 nullable인 정책을 일관되게 반영하고 있습니다.validateTimeRange헬퍼 메서드를 통해 중복을 제거한 점도 좋습니다.
163-187: LGTM! 제목/설명 변경 로직
null/공백 검증,trim()처리, 최대 길이 검증이@Column어노테이션의length속성과 일치하며, 일관된 패턴으로 구현되어 있습니다.src/test/http/group/v2/v2-group-get.http (1)
29-30: V2 이미지 업로드 엔드포인트로 올바르게 마이그레이션됨V1에서 V2 이미지 업로드 API 경로(
/api/v2/groups/images/upload)로 적절히 업데이트되었습니다. 테스트 흐름이 이미지 업로드 → 그룹 생성 → 상세 조회 순서로 논리적으로 구성되어 있습니다.src/main/java/team/wego/wegobackend/group/v2/application/dto/response/UpdateGroupV2Response.java (1)
10-22: 응답 DTO 구조가 적절합니다
UpdateGroupV2Responserecord가 그룹 업데이트 결과를 반환하기에 필요한 모든 필드를 포함하고 있습니다. 불변 DTO로서 적합한 구조입니다.src/main/java/team/wego/wegobackend/common/exception/GlobalExceptionHandler.java (1)
286-293: Redis 예외 처리 분리가 적절합니다
DataAccessException을 Redis 예외 핸들러에서 분리하여 별도 핸들러로 처리하도록 변경한 것은 올바른 설계입니다. Redis 관련 예외만REDIS_READ_FAILED로 처리됩니다.src/test/http/group/v2/v2-group-update.http (2)
99-133: 이미지 재정렬 PATCH 테스트가 잘 구성되어 있습니다테스트가 이미지 삭제(A), 대표 이미지 변경(C), 신규 이미지 추가(D) 시나리오를 포괄적으로 검증합니다.
sortOrder와imageKey일치 여부에 대한 단언(assertion)이 명확합니다.
155-176: 기본 로고 폴백 동작 테스트이미지를 전체 삭제(
imageKeys: [])했을 때 기본 로고가 반환되는지 검증하는 테스트입니다.DEFAULTimageKey와 2개의 variant URL을 확인하는 로직이 적절합니다.src/main/java/team/wego/wegobackend/group/v2/application/dto/request/UpdateGroupV2Request.java (1)
29-33: 서비스 레이어에서 개별 태그 및 이미지 키 검증이 이미 구현되어 있음개별 항목 검증이 DTO 레벨에서 적용되지 않는 점은 맞습니다. 다만
applyTags()및applyImagesWithSafeReorder()메서드에서 다음 검증이 이루어지므로 현재 구현은 문제없습니다:
- 빈 문자열/null 필터링 (
.isBlank()체크)- 공백 제거 (
.trim())- 중복 검증
- 최대 개수 검증
다만 DTO 레벨에
List<@NotBlank String> tags와 같은 TYPE_USE 제약을 추가하면 방어-심화 전략이 개선될 수 있습니다.src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2UpdateService.java (1)
193-215: LGTM!태그 교체 로직이 잘 구현되어 있습니다.
new ArrayList<>(group.getGroupTags())로 복사본을 만들어ConcurrentModificationException을 방지하고, 중복 검사와 최대 개수 검증도 적절합니다.src/main/java/team/wego/wegobackend/group/domain/exception/GroupErrorCode.java (1)
11-35: LGTM!새로운 에러 코드들이 적절한 HTTP 상태 코드와 함께 잘 정의되어 있습니다. 비즈니스 규칙 위반에
CONFLICT(409), 권한 실패에FORBIDDEN(403), 리소스 미존재에NOT_FOUND(404)를 사용하는 것이 REST API 관례에 부합합니다.src/main/java/team/wego/wegobackend/group/v2/presentation/GroupV2Controller.java (1)
109-118: 인증이 필수인 엔드포인트에서 null 체크 불필요Spring Security 설정에서 PATCH
/api/v2/groups/**엔드포인트는 인증이 필수입니다. GET 요청만 permitAll()로 허용되고(line 40), 다른 모든 요청은 authenticated() 상태여야 합니다(line 44). 따라서userDetails는 런타임에 절대 null이 될 수 없으며, NPE 발생 가능성은 없습니다.
getGroup메서드에만 null 체크가 있는 이유는 GET 요청이 permitAll()로 설정되어 있기 때문입니다. 반면 create, attend, left, update 같은 뮤테이션 메서드들은 인증이 필수이므로 null 체크가 불필요합니다. 이는 의도적이고 올바른 설계입니다.
src/main/java/team/wego/wegobackend/common/exception/GlobalExceptionHandler.java
Show resolved
Hide resolved
src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2UpdateService.java
Outdated
Show resolved
Hide resolved
| // 생성 (temp sortOrder는 서로 다르게) | ||
| int temp = TEMP_SORT_ORDER; | ||
| for (String key : toCreateKeys) { | ||
| PreUploadedGroupImage pre = preUploadedGroupImageRedisRepository.consume(key) | ||
| .orElseThrow(() -> new GroupException(GroupErrorCode.PRE_UPLOADED_IMAGE_NOT_FOUND, key)); | ||
|
|
||
| if (!userId.equals(pre.uploaderId())) | ||
| throw new GroupException(GroupErrorCode.PRE_UPLOADED_IMAGE_OWNER_MISMATCH, key); | ||
|
|
||
| GroupImageV2.create(group, temp--, pre.imageKey(), pre.url440x240(), pre.url100x100()); | ||
| } |
There was a problem hiding this comment.
루프 내 예외 발생 시 부분 생성 문제
toCreateKeys 루프에서 첫 번째 이미지 생성 후 두 번째 이미지의 preUploadedGroupImageRedisRepository.consume()이 실패하면, 첫 번째 이미지는 이미 생성된 상태입니다. @Transactional로 DB는 롤백되지만, Redis에서 consume된 첫 번째 이미지 키는 복구되지 않을 수 있습니다.
현재 최대 3장 제한으로 심각한 문제는 아니지만, Redis 작업을 별도 분리하거나 보상 로직을 고려해 볼 수 있습니다.
| public void changeMaxParticipants(Integer maxParticipants) { | ||
| assertUpdatable(); | ||
|
|
||
| if (maxParticipants == null || maxParticipants <= 0) { | ||
| throw new GroupException(GroupErrorCode.INVALID_MAX_PARTICIPANTS); | ||
| } | ||
| this.maxParticipants = maxParticipants; | ||
| } |
There was a problem hiding this comment.
현재 참가자 수 대비 검증 누락
maxParticipants를 현재 참가 중인 인원보다 작게 설정할 수 있습니다. 예를 들어 현재 10명이 참가 중인데 정원을 5명으로 변경하는 것이 허용됩니다.
비즈니스 정책에 따라 다르지만, 현재 참가자 수보다 작은 값으로 변경 시 예외를 발생시키는 것이 일반적입니다.
🔎 참가자 수 검증 추가 예시:
public void changeMaxParticipants(Integer maxParticipants) {
assertUpdatable();
if (maxParticipants == null || maxParticipants <= 0) {
throw new GroupException(GroupErrorCode.INVALID_MAX_PARTICIPANTS);
}
+ long currentAttendCount = this.users.stream()
+ .filter(u -> u.getStatus() == GroupUserV2Status.ATTEND)
+ .count();
+ if (maxParticipants < currentAttendCount) {
+ throw new GroupException(GroupErrorCode.MAX_PARTICIPANTS_LESS_THAN_CURRENT);
+ }
this.maxParticipants = maxParticipants;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/team/wego/wegobackend/group/v2/domain/entity/GroupV2.java
around lines 243 to 250, the changeMaxParticipants method currently allows
setting maxParticipants lower than the current number of participants; add a
validation after the null/<=0 check that retrieves the current participant count
(e.g., participants.size() or the appropriate getter) and if maxParticipants is
less than that count throw a GroupException (use an existing error code like
INVALID_MAX_PARTICIPANTS or define a more specific one such as
CANNOT_SET_BELOW_CURRENT_PARTICIPANTS), otherwise assign this.maxParticipants =
maxParticipants.
| Content-Disposition: form-data; name="images"; filename="img4.jpeg" | ||
| Content-Type: image/jpeg | ||
|
|
||
| < ../../image/resources/img2.jpg | ||
| < ../../image/resources/img4.jpeg |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "img4.jpeg" -type fRepository: WeGo-Together/WeGo_BackEnd
Length of output: 52
🏁 Script executed:
fd -t f "img4" . --extension jpegRepository: WeGo-Together/WeGo_BackEnd
Length of output: 52
🏁 Script executed:
find . -path "*/image/resources/*" -type f | head -20Repository: WeGo-Together/WeGo_BackEnd
Length of output: 330
파일명 확장자 오류: img4.jpeg가 존재하지 않음
img4.jpeg 파일이 리소스 디렉토리에 존재하지 않습니다. 실제로 존재하는 파일은 img4.jpg입니다. 코드의 파일 확장자를 .jpeg에서 .jpg로 수정해주세요.
🤖 Prompt for AI Agents
In src/test/http/group/v2/v2-group-create.http around lines 40 to 43, the
multipart entry references a non-existent filename "img4.jpeg"; update the
Content-Disposition filename and the referenced file path to use the actual
existing extension "img4.jpg" so both the header and the < ... file path match
the real resource (i.e., change img4.jpeg -> img4.jpg in both places).
| ### V2 로그인 후, 현재 유저 기준 참여 상태 포함하여 상세 조회 | ||
| GET http://localhost:8080/api/v2/groups/{{groupId}} | ||
| Authorization: Bearer {{hostAccessToken}} No newline at end of file |
There was a problem hiding this comment.
미정의 변수 {{groupId}} 사용으로 테스트 실패 가능성
이 테스트에서 {{groupId}}를 참조하지만, 이 파일에서는 groupId_cancelTest만 정의되어 있습니다. {{hostAccessToken}} 역시 이 파일에서 정의되지 않았습니다 (host2AccessToken만 정의됨).
다른 HTTP 파일에서 설정된 전역 변수에 의존하는 것으로 보이나, 이 테스트를 단독 실행 시 실패할 수 있습니다.
🔎 수정 제안:
### V2 로그인 후, 현재 유저 기준 참여 상태 포함하여 상세 조회
-GET http://localhost:8080/api/v2/groups/{{groupId}}
-Authorization: Bearer {{hostAccessToken}}
+GET http://localhost:8080/api/v2/groups/{{groupId_cancelTest}}
+Authorization: Bearer {{host2AccessToken}}📝 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.
| ### V2 로그인 후, 현재 유저 기준 참여 상태 포함하여 상세 조회 | |
| GET http://localhost:8080/api/v2/groups/{{groupId}} | |
| Authorization: Bearer {{hostAccessToken}} | |
| ### V2 로그인 후, 현재 유저 기준 참여 상태 포함하여 상세 조회 | |
| GET http://localhost:8080/api/v2/groups/{{groupId_cancelTest}} | |
| Authorization: Bearer {{host2AccessToken}} |
| { | ||
| "email": "test@example.com", | ||
| "password": "Test1234!@#", | ||
| "nickName": "Beemo" | ||
| } |
There was a problem hiding this comment.
회원가입 요청에 phoneNumber 필드 누락
다른 HTTP 테스트 파일(예: v2-group-get.http, v2-group-get-list.http)의 회원가입 요청에는 phoneNumber 필드가 포함되어 있지만, 이 파일에서는 누락되었습니다. API가 필수 필드로 요구하는 경우 테스트가 실패할 수 있습니다.
🔎 수정 제안:
{
"email": "test@example.com",
"password": "Test1234!@#",
- "nickName": "Beemo"
+ "nickName": "Beemo",
+ "phoneNumber": "010-1234-5678"
}📝 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.
| { | |
| "email": "test@example.com", | |
| "password": "Test1234!@#", | |
| "nickName": "Beemo" | |
| } | |
| { | |
| "email": "test@example.com", | |
| "password": "Test1234!@#", | |
| "nickName": "Beemo", | |
| "phoneNumber": "010-1234-5678" | |
| } |
🤖 Prompt for AI Agents
In src/test/http/group/v2/v2-group-update.http around lines 5 to 9, the signup
request body is missing the phoneNumber field (other tests like
v2-group-get.http include it); add a phoneNumber property to the JSON payload
using the same format/value convention as the other HTTP tests (e.g. a valid
E.164 or project's test phone format), and update any related assertions or
downstream test steps that expect phoneNumber to be present.
There was a problem hiding this comment.
Pull request overview
This PR implements group update functionality for V2 API, allowing group hosts to modify group details including field updates, status changes, tag modifications, and image management with reordering and deletion capabilities.
Key Changes:
- Added PATCH endpoint for updating V2 groups with comprehensive field update support
- Implemented image management with safe reordering logic to handle unique constraint conflicts
- Added validation and authorization checks with new error codes for update operations
Reviewed changes
Copilot reviewed 15 out of 19 changed files in this pull request and generated 39 comments.
Show a summary per file
| File | Description |
|---|---|
| GroupV2Controller.java | Added PATCH endpoint mapping for group update functionality |
| GroupV2.java | Added domain logic for field updates, validation methods, and updatability checks |
| GroupTagV2.java | Added unique constraint to prevent duplicate tag associations |
| GroupV2UpdateService.java | Core update service with scalar field updates, tag management, and complex image reordering logic |
| UpdateGroupV2Request.java | Request DTO for update operations with validation annotations |
| UpdateGroupV2Response.java | Response DTO containing updated group information |
| GroupImageItem.java | Added default logo support and imageKey field to response model |
| GroupErrorCode.java | Added new error codes for update validation and authorization failures |
| GlobalExceptionHandler.java | Enhanced exception handling for data integrity violations |
| CreateGroupV2Response.java | Refactored Address inner class to shared common DTO |
| v2-group-update.http | Added comprehensive HTTP test scenarios for update functionality |
| v2-group-get.http | Updated API version reference from V1 to V2 |
| v2-group-get-list.http | Added test case for authenticated user detail retrieval |
| v2-group-create.http | Fixed image file reference path |
| group_logo_*.webp | Added default group logo image resources |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Map<String, GroupImageV2> afterRemoveByKey = group.getImages().stream() | ||
| .collect(Collectors.toMap(GroupImageV2::getImageKey, img -> img)); | ||
|
|
||
| // 새로 생성해야 하는 키(요청에는 있는데 현재 없는 것) |
There was a problem hiding this comment.
Comment contains Korean language. The comment "새로 생성해야 하는 키(요청에는 있는데 현재 없는 것)" should be translated to English for consistency with standard code documentation practices.
| // 새로 생성해야 하는 키(요청에는 있는데 현재 없는 것) | |
| // Keys to create: present in the request but not currently in the group |
| applyImagesWithSafeReorder(group, userId, request.imageKeys()); | ||
| } | ||
|
|
||
| // dirty checking으로 충분. 그래도 명시적으로 save 해도 무방. |
There was a problem hiding this comment.
Comment contains Korean language. The comment "dirty checking으로 충분. 그래도 명시적으로 save 해도 무방." should be translated to English for consistency with standard code documentation practices.
| // dirty checking으로 충분. 그래도 명시적으로 save 해도 무방. | |
| // Dirty checking is sufficient, but explicitly calling save is also fine. |
| public void changeEndTime(LocalDateTime endTime) { | ||
| assertUpdatable(); | ||
|
|
||
| // endTime을 null 허용할지 정책 -> 안하는 중! |
There was a problem hiding this comment.
Comment contains Korean language. The comment "endTime을 null 허용할지 정책 -> 안하는 중!" should be translated to English for consistency with standard code documentation practices.
| this.maxParticipants = maxParticipants; | ||
| } | ||
|
|
||
| // 사용할 지 안할지 아직 잘 모름 |
There was a problem hiding this comment.
Comment contains Korean language. The comment "사용할 지 안할지 아직 잘 모름" should be translated to English for consistency with standard code documentation practices.
| // 사용할 지 안할지 아직 잘 모름 | |
| // Not sure yet whether this will be used |
| GroupImageV2.create(group, temp--, pre.imageKey(), pre.url440x240(), pre.url100x100()); | ||
| } | ||
|
|
||
| // 최종 매핑 + 검증 |
There was a problem hiding this comment.
Comment contains Korean language. The comment "최종 매핑 + 검증" should be translated to English for consistency with standard code documentation practices.
| group.changeAddress(GroupV2Address.of(request.location(), request.locationDetail())); | ||
| } | ||
|
|
||
| // 시간: 둘 중 하나만 와도 엔티티가 최종 검증하도록 설계했으면 각각 호출 |
There was a problem hiding this comment.
Comment contains Korean language. The comment "시간: 둘 중 하나만 와도 엔티티가 최종 검증하도록 설계했으면 각각 호출" should be translated to English for consistency with standard code documentation practices.
| // 시간: 둘 중 하나만 와도 엔티티가 최종 검증하도록 설계했으면 각각 호출 | |
| // Time: if only one of start or end is provided, call each setter separately so the entity can perform final validation |
| throw new GroupException(GroupErrorCode.GROUP_ONLY_HOST_CAN_UPDATE, groupId, userId); | ||
| } | ||
|
|
||
| // 엔티티 단 공통 가드: deleteAT + CANCELLED, FINISHED 체크하자 |
There was a problem hiding this comment.
Comment contains Korean language. The comment "엔티티 단 공통 가드: deleteAT + CANCELLED, FINISHED 체크하자" should be translated to English for consistency with standard code documentation practices.
| // 엔티티 단 공통 가드: deleteAT + CANCELLED, FINISHED 체크하자 | |
| // Entity-level common guard: check deletedAt and CANCELLED/FINISHED status |
| } | ||
|
|
||
|
|
||
| // 시간은 "부분 수정"을 위해 start/end 단건 변경도 지원하되, 항상 최종 상태에서 start < end 불변식을 만족하자. |
There was a problem hiding this comment.
Comment contains Korean language. The comment "시간은 '부분 수정'을 위해 start/end 단건 변경도 지원하되, 항상 최종 상태에서 start < end 불변식을 만족하자" should be translated to English for consistency with standard code documentation practices.
| this.maxParticipants = maxParticipants; | ||
| } | ||
|
|
||
| // 사용할 지 안할지 아직 잘 모름 |
There was a problem hiding this comment.
The comment indicates uncertainty about usage ("사용할 지 안할지 아직 잘 모름" / "not sure whether to use or not"). If this method's usage is uncertain, consider adding a TODO comment or removing it if it's truly not needed. Having dead or uncertain code can reduce code maintainability.
| // 사용할 지 안할지 아직 잘 모름 | |
| // TODO: Confirm and document the domain policy for using soft delete via deletedAt. |
| // DB에 이미지가 0개일 때 내려주는 기본 이미지(440/100) | ||
| public static GroupImageItem defaultLogo() { | ||
| return new GroupImageItem( | ||
| null, // DB row가 아니다. 그래서 null | ||
| "DEFAULT", // 테스트 편하게 DEFAULT로 설정 | ||
| 0, // 없으니까 어차피 대표 |
There was a problem hiding this comment.
Comment contains Korean language. The comment "DB에 이미지가 0개일 때 내려주는 기본 이미지(440/100)" should be translated to English for consistency with standard code documentation practices.
| // DB에 이미지가 0개일 때 내려주는 기본 이미지(440/100) | |
| public static GroupImageItem defaultLogo() { | |
| return new GroupImageItem( | |
| null, // DB row가 아니다. 그래서 null | |
| "DEFAULT", // 테스트 편하게 DEFAULT로 설정 | |
| 0, // 없으니까 어차피 대표 | |
| // Default image (440/100) returned when there are 0 images in the DB | |
| public static GroupImageItem defaultLogo() { | |
| return new GroupImageItem( | |
| null, // Not a DB row, therefore null | |
| "DEFAULT", // Set to DEFAULT for easier testing | |
| 0, // 0 because it is the representative image when none exist |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2UpdateService.java (2)
275-288: Redis consume 실패 시 부분 생성 문제는 이미 인지되었습니다.이전 리뷰에서 지적된 대로, 루프 중간에 Redis consume이 실패하면 이미 consume된 키는 복구되지 않습니다. 현재 최대 3장 제한으로 심각한 문제는 아니지만, 향후 Redis 작업을 별도로 분리하거나 보상 로직을 고려할 수 있습니다.
67-67: 한국어 주석은 이미 이전 리뷰에서 지적되었습니다.코드 전반의 한국어 주석을 영어로 번역하는 것이 국제 협업과 유지보수성 향상에 도움이 됩니다. 이 사항은 이전 리뷰(Copilot)에서 이미 여러 차례 지적되었습니다.
Also applies to: 75-75, 83-83, 88-88, 93-93, 96-96, 101-101, 104-104, 110-110, 158-158, 161-161, 165-165, 177-177, 189-189, 211-211, 218-218, 251-251, 258-258, 264-264, 266-266, 270-270, 275-275, 290-290, 301-301
🧹 Nitpick comments (2)
src/main/java/team/wego/wegobackend/common/exception/GlobalExceptionHandler.java (1)
292-307: 제약조건 검사 로직의 확장성 개선을 권장합니다.이전 리뷰 피드백이 반영되어 개선되었으나, 현재 구현은 하나의 제약조건(UK_GROUP_ID_SORT_ORDER_INDEX_D)만 특정 오류 코드로 매핑합니다. 추후 다른 제약조건(예: 사용자 이메일 중복, 다른 unique 제약 등)을 추가할 때마다 if 문을 추가해야 합니다.
향후 확장성을 위해 다음을 고려해보세요:
- 제약조건 이름과 오류 코드를 매핑하는 Map 또는 전략 패턴 활용
- SQLState 코드를 활용한 더 정확한 분류
- 데이터베이스 벤더별 메시지 포맷 차이 고려 (현재 H2 특정 로직)
🔎 제약조건 매핑 개선 예시
+ private static final Map<String, ErrorCode> CONSTRAINT_ERROR_MAP = Map.of( + "UK_GROUP_ID_SORT_ORDER_INDEX_D", GroupErrorCode.GROUP_IMAGE_SORT_ORDER_CONFLICT + // 추후 다른 제약조건 추가 가능 + ); + @ExceptionHandler(DataIntegrityViolationException.class) public ResponseEntity<ErrorResponse> handleDataIntegrity( DataIntegrityViolationException ex, HttpServletRequest request) { String msg = rootCauseMessage(ex); log.error("DB 무결성 위반(409): {}", msg, ex); - // 예: H2 메시지에 constraint 이름이 들어옴 - // "PUBLIC.UK_GROUP_ID_SORT_ORDER_INDEX_D" - if (msg != null && msg.contains("UK_GROUP_ID_SORT_ORDER_INDEX_D")) { - return handleApp(new AppException(GroupErrorCode.GROUP_IMAGE_SORT_ORDER_CONFLICT), request); + if (msg != null) { + ErrorCode matchedCode = CONSTRAINT_ERROR_MAP.entrySet().stream() + .filter(entry -> msg.contains(entry.getKey())) + .map(Map.Entry::getValue) + .findFirst() + .orElse(null); + + if (matchedCode != null) { + return handleApp(new AppException(matchedCode), request); + } } - // 나머지는 공통 무결성 위반 코드로 (AppErrorCode 하나 만드는 걸 추천) return handleApp(new AppException(AppErrorCode.DATA_INTEGRITY_VIOLATION), request); }src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2UpdateService.java (1)
130-154: LGTM - 기본 로고 생성 로직이 명확합니다.2개의 variant를 포함한 기본 로고 DTO 생성이 잘 구조화되어 있습니다.
하드코딩된 URL을
application.yml등의 설정 파일로 외부화하면 환경별 관리가 용이해질 수 있으나, 현재 구현도 충분히 실용적입니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/team/wego/wegobackend/common/exception/AppErrorCode.java(1 hunks)src/main/java/team/wego/wegobackend/common/exception/GlobalExceptionHandler.java(2 hunks)src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2UpdateService.java(1 hunks)
🔇 Additional comments (13)
src/main/java/team/wego/wegobackend/common/exception/AppErrorCode.java (1)
11-11: DB 무결성 위반을 위한 일반 오류 코드 추가가 적절합니다.
DATA_INTEGRITY_VIOLATION추가로 특정 제약조건 외의 DB 무결성 문제를 처리할 수 있게 되었습니다. HTTP 409 CONFLICT 상태와 명확한 메시지가 적절합니다.src/main/java/team/wego/wegobackend/common/exception/GlobalExceptionHandler.java (4)
10-10: 필요한 import 추가.새로운
DataIntegrityViolationException핸들러를 위한 import가 적절히 추가되었습니다.
286-290: Redis 예외 핸들러 리팩토링이 적절합니다.
DataAccessException을 별도 핸들러로 분리하여 Redis 관련 예외만 처리하도록 개선되었습니다. 관심사 분리가 잘 이루어졌습니다.
309-315: 일반 DB 접근 오류 처리가 적절합니다.
DataAccessException을INTERNAL_SERVER_ERROR로 매핑하는 것이 적절합니다. 특정 무결성 위반(DataIntegrityViolationException)과 분리되어 일반적인 DB 접근 문제를 처리합니다.
318-319: 메서드 시그니처 포맷 변경.가독성을 위한 줄바꿈 조정으로, 동작에는 영향이 없습니다.
src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2UpdateService.java (8)
37-56: LGTM - 서비스 구조 및 의존성 설정이 적절합니다.필요한 리포지토리와 서비스 의존성이 명확히 정의되어 있으며, EntityManager를 활용한 명시적 flush 제어는 unique constraint 충돌 방지를 위해 적절합니다.
57-94: LGTM - 업데이트 플로우와 권한 검증이 견고합니다.
- userId null 체크와 호스트 권한 검증이 적절히 구현되어 있습니다.
assertUpdatable()호출을 통한 엔티티 레벨 가드가 효과적입니다.- 스칼라 필드 → 상태 → 태그 → 이미지 순서의 업데이트 흐름이 논리적입니다.
96-128: LGTM - 응답 구성 로직이 안전하고 명확합니다.업데이트 후 이미지를 variants 포함하여 재조회하는 것은 데이터 일관성을 보장하는 안전한 접근입니다. 이미지가 없을 때 기본 로고를 제공하는 로직도 사용자 경험 측면에서 적절합니다.
165-175: LGTM - 주소 업데이트 로직의 null 처리가 올바르게 수정되었습니다.이전 리뷰에서 지적된
locationDetailnull 덮어쓰기 문제가 해결되었습니다. 현재 구현은 요청에서location또는locationDetail이 null일 때 기존 값을 적절히 보존합니다.
189-199: LGTM - 정원 변경 시 참석자 수 검증이 적절합니다.새로운 최대 참석자 수가 현재 참석 인원보다 작을 때 예외를 발생시켜 데이터 무결성을 보장하는 로직이 올바릅니다.
201-223: LGTM - 태그 업데이트 로직이 견고합니다.입력 검증(최대 개수, 중복 체크)이 철저하며, 기존 태그를 완전히 교체하는 방식이 명확하고 이해하기 쉽습니다.
TagService.findOrCreateAll을 활용한 태그 생성/조회도 적절합니다.
232-264: LGTM - 2-phase 이미지 재정렬 전략이 효과적입니다.임시 음수
sortOrder를 사용하여 unique constraint 충돌을 방지하는 접근이 매우 적절합니다. 삭제와 임시 업데이트를 flush하여 DB에 먼저 반영한 후 새 이미지를 생성하는 방식이 안전하고 명확합니다.
290-305: LGTM - 최종 검증 및 순서 부여 로직이 안전합니다.모든 요청된 키가 그룹에 존재하는지 최종 검증한 후 순차적으로 sortOrder를 부여하는 로직이 명확하고 안전합니다.
github-project-automation
bot
moved this from In progress
to Done
in WeGo-Together Backend
1 participant
📝 Pull Request
📌 PR 종류
해당하는 항목에 체크해주세요.
✨ 변경 내용
모임 호스트가 그룹 세부 정보를 업데이트할 수 있는 기능을 추가합니다.
여기에는 필드 업데이트, 그룹 상태 변경,
태그 수정, 이미지 순서 변경 및 삭제 기능을 포함한 이미지 관리 등이 포함됩니다.
업데이트 과정 중 유효성 검사 및 권한 부여 실패에 대한 새로운 오류 코드가 도입되었습니다.
🔍 관련 이슈
🧪 테스트
변경된 기능에 대한 테스트 범위 또는 테스트 결과를 작성해주세요.
🚨 확인해야 할 사항 (Checklist)
PR을 제출하기 전에 아래 항목들을 확인해주세요.
🙋 기타 참고 사항
리뷰어가 참고하면 좋을 만한 추가 설명이 있다면 적어주세요.
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
동작·검증 개선
✏️ Tip: You can customize this high-level summary in your review settings.