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. Walkthrough
Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/team/wego/wegobackend/group/application/service/GroupService.java (3)
125-128: 메서드명과 에러 코드의 의미 불일치 검토 필요
findUserByUserId는 범용적인 이름이지만,HOST_USER_NOT_FOUND에러 코드는 호스트 컨텍스트에 한정됩니다. 현재는createGroup에서만 사용되어 문제없지만, 향후 다른 곳에서 재사용 시 혼란을 줄 수 있습니다.- private User findUserByUserId(Long userId) { - return userRepository.findById(userId) - .orElseThrow(() -> new GroupException(GroupErrorCode.HOST_USER_NOT_FOUND, userId)); - } + private User findHostByUserId(Long userId) { + return userRepository.findById(userId) + .orElseThrow(() -> new GroupException(GroupErrorCode.HOST_USER_NOT_FOUND, userId)); + }
171-171: import 문 사용 권장
java.util.ArrayList가 fully qualified name으로 사용되었습니다. 파일 상단에 import를 추가하여 일관성을 유지하세요.파일 상단 import 섹션에 추가:
import java.util.ArrayList;그 후 해당 라인 수정:
- List<GroupImage> entities = new java.util.ArrayList<>(); + List<GroupImage> entities = new ArrayList<>();
469-475: URL 매칭 방식 개선 고려
contains(token)은 URL 내 어디에서든 토큰이 포함되면 매칭됩니다. 현재 이미지 업로드 서비스에서 URL 형식을 제어한다면 문제없지만, 향후 다른 형식의 URL이 추가될 경우 의도치 않은 매칭이 발생할 수 있습니다.정규식이나 경로 패턴 매칭을 고려해 볼 수 있습니다:
private GroupImage findByTokenOrNull(List<GroupImage> images, String token) { String pattern = "/" + token + "/"; // 또는 "_" + token + "." 등 실제 URL 패턴에 맞게 return images.stream() .filter(Objects::nonNull) .filter(img -> img.getImageUrl() != null && img.getImageUrl().contains(pattern)) .findFirst() .orElse(null); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/team/wego/wegobackend/group/application/service/GroupService.java(9 hunks)
⏰ 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 (10)
src/main/java/team/wego/wegobackend/group/application/service/GroupService.java (10)
61-66: 상수 중앙화가 잘 되어 있습니다.하드코딩된 값들을 상수로 추출하여 유지보수성을 높인 좋은 리팩토링입니다.
80-115: 태그 저장 로직이 안정적입니다.null/empty 체크, 정규화, 중복 제거 후 기존 태그 조회 및 신규 태그 생성 흐름이 잘 구성되어 있습니다.
225-229: 참여 검증 로직이 명확합니다.이미 참여 중인 사용자에 대한 검증이 올바르게 구현되어 있고, 에러 메시지에 컨텍스트(groupId, userId)가 포함되어 디버깅에 유용합니다.
238-245: 재참여 로직이 잘 설계되었습니다.
validateNotAlreadyAttend가 먼저 호출되어 이미 ATTEND 상태인 경우를 걸러내므로,upsertAttend에서는 null이거나 LEFT 상태만 처리됩니다. 최초 참여 여부를 boolean으로 반환하여 카운트 증가 로직을 분리한 점이 좋습니다.
377-385: 인증/비인증 사용자 지원을 위한 오버로드가 잘 구현되었습니다.
getGroupInternal을 통해 공통 로직을 재사용하면서, 인증된 사용자와 익명 사용자 모두 지원하는 깔끔한 설계입니다.
525-535: 이미지 URL 추출 로직이 적절합니다.그룹 삭제 시 이미지 정리를 위해 사용되며, null 및 빈 URL에 대한 방어적 필터링이 잘 되어 있습니다.
568-593: 그룹 수정 로직이 안전하게 구현되었습니다.권한 검사, 현재 참여자 수 기반 maxParticipants 검증, 그리고 태그 업데이트까지 트랜잭션 내에서 일관되게 처리됩니다.
653-664: 커서 기반 페이지네이션 구현이 올바릅니다.
pageSize + 1개를 조회하여hasNext를 판단하고, 마지막 아이템의 ID를 다음 커서로 사용하는 표준 패턴입니다.
512-523: 사용자 상태 해석 로직이 명확합니다.익명 사용자(null currentUserId)와 인증된 사용자 모두에 대해 적절한 상태를 반환합니다.
490-499: 참여자 수 계산 로직이 적절합니다.
count()의 long을 int로 캐스팅하지만, 실제 그룹 참여자 수가 Integer.MAX_VALUE를 초과할 가능성은 없으므로 안전합니다.
There was a problem hiding this comment.
Pull request overview
This PR refactors the GroupService class to eliminate code duplication and extract hardcoded magic numbers into well-named constants. The refactoring improves code maintainability and readability by organizing methods into logical sections and extracting common patterns into reusable helper methods.
- Extracted constants for magic numbers (page size limit, image limits, size tokens)
- Organized code into logical sections with clear comments (Create, Common Finders, Attend/Cancel, etc.)
- Extracted repeated logic into dedicated helper methods to reduce duplication
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return toGroupListResponse(fetched, size); | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Remove the extra blank line for consistency with the rest of the code. There should be only one blank line between methods in this file.
| } | ||
|
|
||
| return byOrder.values().stream() | ||
| .limit(GroupService.GROUP_LIST_IMAGE_LIMIT) |
There was a problem hiding this comment.
The class name prefix 'GroupService.' is redundant when referencing a constant within the same class. Use 'GROUP_LIST_IMAGE_LIMIT' directly instead of 'GroupService.GROUP_LIST_IMAGE_LIMIT'.
| .limit(GroupService.GROUP_LIST_IMAGE_LIMIT) | |
| .limit(GROUP_LIST_IMAGE_LIMIT) |
📝 Pull Request
📌 PR 종류
해당하는 항목에 체크해주세요.
✨ 변경 내용
중복되거나 불필요한 하드 코딩을 제거합니다.
🔍 관련 이슈
🧪 테스트
변경된 기능에 대한 테스트 범위 또는 테스트 결과를 작성해주세요.
🚨 확인해야 할 사항 (Checklist)
PR을 제출하기 전에 아래 항목들을 확인해주세요.
🙋 기타 참고 사항
리뷰어가 참고하면 좋을 만한 추가 설명이 있다면 적어주세요.
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.