Conversation
모임 상태 기반 필터링 옵션을 추가하여 모임 목록 API를 개선했습니다. 모임 목록 항목에 상태 배지 표시 및 참여 가능 상태 표시 기능을 추가했습니다. 모임 상세 정보 API를 개선하여 호스트의 참가자 상태와 남은 시간을 표시하고,호스트가 아닌 참가자에게는 참석 상태만 표시합니다. 남은 좌석 계산 및 참여 가능 상태 로직을 수정했습니다.
WalkthroughV2 모임 목록 조회 기능에 상태 필터링(활성, 보관, 전체)을 추가하고, 목록 응답에 남은 좌석 수와 참여 가능 여부 필드를 포함시키며, 상세 응답의 멤버 조회를 호스트/비호스트 관점으로 구분하고 저장소 계층의 QueryDSL 쿼리를 V2 엔티티 기반으로 마이그레이션했습니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 특별 주의 사항:
Possibly related issues
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: 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/java/team/wego/wegobackend/group/v2/application/service/GroupV2Service.java (1)
273-279: 정원 체크 로직의 동시성 문제정원 체크가
reAttend()호출 후 수행되어 트랜잭션 롤백에 의존하고 있습니다. 현재 구조에서는 동작하지만, TODO 주석(라인 230)에서 언급한 동시성 문제가 실제로 존재합니다.동시에 여러 사용자가 참석 요청을 보낼 경우 race condition이 발생할 수 있습니다:
- T1이
reAttend()로 상태를 ATTEND로 변경한 후 카운트- T2가 동시에
reAttend()로 상태를 ATTEND로 변경한 후 카운트- 둘 다 카운트를 통과할 수 있어 정원 초과 가능
GroupV2 엔티티에
@Version필드가 없으므로 낙관적 락이 미적용되어 있습니다. 비관적 락(Pessimistic Lock) 또는 낙관적 락(Optimistic Lock with@Version)을 고려하거나, 쿼리 레벨에서SELECT FOR UPDATE를 사용하여 동시성을 제어하세요.
🧹 Nitpick comments (3)
src/test/http/group/v2/v2-group-get-list.http (1)
239-241: include + exclude 동시 지정 시 우선순위 문서화 고려6-7번 테스트에서
includeStatuses=RECRUITING,FULL과excludeStatuses=FULL을 동시 지정하여 exclude 우선 동작을 테스트하고 있습니다. 이 우선순위 규칙이 API 문서나 코드 주석에 명시되어 있는지 확인해 주세요.src/main/java/team/wego/wegobackend/group/v2/application/dto/response/GetGroupListV2Response.java (1)
66-69:status가 null인 경우 방어 코드 고려현재 switch 표현식은
status가 null일 경우NullPointerException이 발생합니다. DB에서 status가 항상 non-null로 보장된다면 문제없지만, 방어적으로 null 체크를 추가하거나@NonNull어노테이션으로 계약을 명시하는 것을 고려해 보세요.🔎 방어적 null 처리 예시:
- boolean joinable = switch (status) { + boolean joinable = status == null ? false : switch (status) { case RECRUITING -> remainingSeats > 0; case FULL, CLOSED, CANCELLED, FINISHED -> false; };src/main/java/team/wego/wegobackend/group/v2/infrastructure/querydsl/GroupV2QueryRepositoryImpl.java (1)
167-180: 인메모리 제한 처리에 대한 성능 고려
perGroupLimit을 SQL이 아닌 인메모리에서 적용하고 있습니다. 현재GROUP_LIST_IMAGE_LIMIT이 3으로 작고 목록 조회 시 그룹 수도 제한되어 있으므로 큰 문제는 없지만, 이미지가 많아질 경우 불필요한 데이터를 가져올 수 있습니다.🔎 (선택) ROW_NUMBER() 윈도우 함수를 사용한 SQL 레벨 제한 예시:
-- Native Query 또는 서브쿼리로 ROW_NUMBER() 적용 가능 SELECT * FROM ( SELECT g.group_id, v.image_url, ROW_NUMBER() OVER (PARTITION BY g.group_id ORDER BY g.sort_order) AS rn FROM group_image_v2_variant v JOIN group_image_v2 g ON v.group_image_id = g.id WHERE g.group_id IN (:groupIds) AND v.type = 'CARD_440_240' ) sub WHERE rn <= :perGroupLimit
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/team/wego/wegobackend/group/v2/application/dto/request/GroupListFilter.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/application/dto/response/GetGroupListV2Response.java(4 hunks)src/main/java/team/wego/wegobackend/group/v2/application/dto/response/GetGroupV2Response.java(4 hunks)src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2Service.java(6 hunks)src/main/java/team/wego/wegobackend/group/v2/domain/repository/GroupV2QueryRepository.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/infrastructure/querydsl/GroupV2QueryRepositoryImpl.java(5 hunks)src/main/java/team/wego/wegobackend/group/v2/infrastructure/querydsl/projection/GroupListRow.java(1 hunks)src/main/java/team/wego/wegobackend/group/v2/presentation/GroupV2Controller.java(4 hunks)src/test/http/group/v2/V2-group-cancel.http(1 hunks)src/test/http/group/v2/v2-group-get-list.http(1 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 (14)
src/main/java/team/wego/wegobackend/group/v2/application/dto/request/GroupListFilter.java (1)
6-21: LGTM! 깔끔한 필터 enum 구현입니다.switch 표현식을 활용하여 모든 케이스를 명시적으로 처리하고 있으며,
ALL필터가 빈 리스트를 반환하여 조건 미적용을 의미하는 설계가 적절합니다.src/test/http/group/v2/V2-group-cancel.http (1)
184-190: LGTM! HOST/참가자 관점 분리 테스트가 잘 추가되었습니다.호스트와 일반 참가자의 상세 조회 응답 차이를 검증하기 위한 테스트 케이스가 적절하게 구성되어 있습니다. PR 목표인 호스트/비호스트 관점 구분과 일치합니다.
src/main/java/team/wego/wegobackend/group/v2/infrastructure/querydsl/projection/GroupListRow.java (1)
9-9: LGTM! status 필드 추가가 적절합니다.상태 기반 필터링 및 응답 매핑을 위해
GroupV2Status필드가 필요하며, projection에 올바르게 추가되었습니다.src/test/http/group/v2/v2-group-get-list.http (1)
1-251: LGTM! 포괄적인 E2E 테스트 시나리오입니다.V2 목록 조회 API의 다양한 필터링 옵션(ACTIVE, ALL, includeStatuses, excludeStatuses)과 페이지네이션, 키워드 검색을 모두 커버하고 있습니다. 에러 케이스(중복 취소, HOST 취소 시도)와 비로그인 접근 테스트도 포함되어 있어 테스트 커버리지가 우수합니다.
src/main/java/team/wego/wegobackend/group/v2/application/dto/response/GetGroupListV2Response.java (2)
62-69: LGTM! 남은 좌석 및 참여 가능 여부 로직이 잘 구현되었습니다.
Math.max(0, ...)로 음수 방지 처리가 되어 있고, switch 표현식으로 상태별 참여 가능 여부를 명확하게 정의했습니다. 새로운 상태가 추가될 경우 컴파일 타임에 오류가 발생하여 업데이트를 강제하는 점도 좋습니다.
36-37: 새 필드 추가로 API 응답이 확장되었습니다.
remainingSeats와joinable필드가 추가되어 프론트엔드에서 별도 계산 없이 바로 사용할 수 있습니다. API 버저닝(V2)이 되어 있어 기존 클라이언트에 영향이 없습니다.src/main/java/team/wego/wegobackend/group/v2/domain/repository/GroupV2QueryRepository.java (1)
10-16: LGTM! 필터링 파라미터 추가가 잘 설계되었습니다.
includeStatuses와excludeStatuses파라미터를 통해 유연한 상태 기반 필터링이 가능해졌습니다. 구현체에서 null/empty list 케이스를 적절히 처리하고 있으며 (라인 49, 53의 null 체크와 isEmpty() 조건 결합), 필터가 필요할 때만 조건에 추가되도록 구현되어 있습니다.src/main/java/team/wego/wegobackend/group/v2/infrastructure/querydsl/GroupV2QueryRepositoryImpl.java (2)
34-41: 메서드 시그니처 개선 확인V2 상태 필터링을 위한
includeStatuses와excludeStatuses파라미터 추가가 잘 반영되어 있습니다. 서비스 레이어에서 null 방어 처리를 하고 있으므로 Repository에서는 별도의 null 체크 없이 조건만 적용하는 구조가 적절합니다.
49-55: 상태 필터링 로직 검토 완료
includeStatuses와excludeStatuses가 동시에 제공될 경우 서비스 레이어에서 충돌을 해결하고 있으므로, Repository에서는 단순히 조건을 적용하는 현재 구조가 적합합니다.src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2Service.java (1)
133-159: 목록 매핑 로직 검토 완료
GroupListRow에서GroupListItemV2Response로의 매핑이 V2 필드와 일관성 있게 구현되어 있습니다.participantCountnull 방어 처리(Lines 135-136)가 적절합니다.src/main/java/team/wego/wegobackend/group/v2/presentation/GroupV2Controller.java (1)
48-57: 비인증 사용자 처리 개선 확인
userIdOrNull변수 도입으로 비인증 사용자의 모임 상세 조회를 안전하게 처리하고 있습니다. 코드 가독성도 향상되었습니다.src/main/java/team/wego/wegobackend/group/v2/application/dto/response/GetGroupV2Response.java (3)
67-72: 비호스트 사용자의 상태 노출 여부 검토 필요Line 70에서 비호스트 사용자에게도
exposeStatus=true를 전달하고 있습니다. 주석에서 "ATTEND만 들어오니 status 노출해도 OK"라고 설명하고 있지만, 원래 의도가 비호스트에게는 상태를 숨기는 것이었다면exposeStatus=false가 맞습니다.현재 구현대로라면 비호스트도 ATTEND 멤버의
status와leftAt필드를 볼 수 있습니다. 이것이 의도된 동작인지 확인해 주세요.🔎 상태를 숨기려면:
} else { // 비HOST: ATTEND만 노출 joinedMembers = users.stream() .filter(groupUserV2 -> groupUserV2.getStatus() == GroupUserV2Status.ATTEND) - .map(groupUserV2 -> JoinedMember.from(groupUserV2, true)) // 여기서는 ATTEND만 들어오니 status 노출해도 OK + .map(groupUserV2 -> JoinedMember.from(groupUserV2, false)) // 비HOST는 status/leftAt 숨김 .toList(); }
115-128: JoinedMember 팩토리 메서드 구현 확인
exposeStatus플래그를 사용한 조건부 필드 노출 구현이 깔끔합니다. 민감한 정보를 뷰어 권한에 따라 선택적으로 노출하는 패턴이 적절합니다.
56-56: null-safe 비교 사용 확인
Objects.equals()를 사용한 호스트 ID 비교가userIdOrNull이 null일 경우에도 안전하게 처리됩니다.
| // filter null 방어 | ||
| filter = (filter == null) ? GroupListFilter.ACTIVE : filter; | ||
|
|
||
| int pageSize = Math.max(1, Math.min(size, MAX_PAGE_SIZE)); | ||
|
|
||
| boolean hasInclude = includeStatuses != null && !includeStatuses.isEmpty(); | ||
| boolean hasExclude = excludeStatuses != null && !excludeStatuses.isEmpty(); | ||
|
|
||
| // 여기서부터 includes/excludes는 "절대 null 아님" 규칙으로 설정 | ||
| List<GroupV2Status> includes = filter.defaultIncludeStatuses(); | ||
| List<GroupV2Status> excludes = filter.defaultExcludeStatuses(); | ||
|
|
||
| // GroupListFilter의 default가 절대 null 반환하지 않게 보장 | ||
| if (includes == null) { | ||
| includes = List.of(); | ||
| } | ||
| if (excludes == null) { | ||
| excludes = List.of(); | ||
| } | ||
|
|
||
| // include가 오면 include가 우선: filter include 무시 | ||
| if (hasInclude) { | ||
| includes = includeStatuses; | ||
| // include가 명시된 순간 filter의 exclude까지 섞이면 예측이 어려워져서 초기화 | ||
| excludes = List.of(); | ||
| } | ||
|
|
||
| // exclude가 오면 exclude는 요청값으로 덮어쓰기 | ||
| if (hasExclude) { | ||
| excludes = excludeStatuses; | ||
| } | ||
|
|
||
| // 안전장치: filter=ALL이면 "전체"가 맞으므로 기본값 강제 X | ||
| // filter != ALL이고, 아무것도 안 왔고, includes가 비어있다면 ACTIVE 기본값 적용 | ||
| if (!hasInclude && !hasExclude && filter != GroupListFilter.ALL && includes.isEmpty()) { | ||
| includes = List.of(GroupV2Status.RECRUITING, GroupV2Status.FULL); | ||
| } | ||
|
|
||
| // 4) 충돌 제거: exclude 우선한다. 람다 캡처 문제 없애기 위해 Set 연산으로 처리 | ||
| if (!includes.isEmpty() && !excludes.isEmpty()) { | ||
| // includes가 "전체" 의미(빈 리스트)인 경우엔 여기 들어오지 않음 | ||
| var includeSet = java.util.EnumSet.copyOf(includes); | ||
| excludes.forEach(includeSet::remove); | ||
| includes = List.copyOf(includeSet); // 불변 리스트로 정리 | ||
| } |
There was a problem hiding this comment.
필터 우선순위 로직의 복잡성 및 잠재적 버그
필터 우선순위 로직이 복잡하여 유지보수가 어려울 수 있습니다. 특히 Line 107에서 EnumSet.copyOf(includes)는 includes가 빈 리스트일 경우 IllegalArgumentException을 발생시킵니다.
🔎 수정 제안:
// 4) 충돌 제거: exclude 우선한다. 람다 캡처 문제 없애기 위해 Set 연산으로 처리
if (!includes.isEmpty() && !excludes.isEmpty()) {
- var includeSet = java.util.EnumSet.copyOf(includes);
+ var includeSet = java.util.EnumSet.noneOf(GroupV2Status.class);
+ includeSet.addAll(includes);
excludes.forEach(includeSet::remove);
includes = List.copyOf(includeSet); // 불변 리스트로 정리
}또한 이 복잡한 로직을 별도의 private 메서드나 전략 패턴으로 분리하면 테스트와 유지보수가 용이해질 것입니다.
There was a problem hiding this comment.
@LimdaeIl, 알겠습니다! 나중에 리팩토링하시는 것으로 하겠습니다.
이 개선 사항을 추적하기 위해 GitHub 이슈를 생성해드릴까요? 이슈에 현재 PR과 이 코멘트의 링크를 포함하여 나중에 쉽게 찾아볼 수 있도록 도와드릴 수 있습니다.
| LocalDateTime endTime, | ||
| String description, | ||
| int maxParticipants, | ||
| Integer maxParticipants, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for GroupListRow.java and related files
find . -name "GroupListRow.java" -o -name "GetGroupListV2Response*" 2>/dev/null | head -20Repository: WeGo-Together/WeGo_BackEnd
Length of output: 269
🏁 Script executed:
#!/bin/bash
# Inspect GroupListRow.java
echo "=== GroupListRow.java ==="
cat -n "./src/main/java/team/wego/wegobackend/group/v2/infrastructure/querydsl/projection/GroupListRow.java"Repository: WeGo-Together/WeGo_BackEnd
Length of output: 900
🏁 Script executed:
#!/bin/bash
# Inspect GetGroupListV2Response.java focusing on the of() method
echo "=== GetGroupListV2Response.java ==="
cat -n "./src/main/java/team/wego/wegobackend/group/v2/application/dto/response/GetGroupListV2Response.java"Repository: WeGo-Together/WeGo_BackEnd
Length of output: 3946
🏁 Script executed:
#!/bin/bash
# Search for usages of GetGroupListV2Response.of() with maxParticipants
echo "=== Searching for maxParticipants usage patterns ==="
rg -n "maxParticipants" -B 2 -A 2 --type=javaRepository: WeGo-Together/WeGo_BackEnd
Length of output: 19970
maxParticipants를 Integer로 변경하면서 타입 불일치 문제가 발생했습니다.
QueryDSL 프로젝션에서 maxParticipants를 Integer(nullable)로 변경했으나, GroupV2Service.java 149번 줄에서 이를 GetGroupListV2Response.GroupListItemV2Response.of() 메서드에 전달할 때 primitive int를 기대하고 있습니다. 이로 인해 null 값이 존재할 경우 언박싱 시 NullPointerException이 발생합니다.
DB 제약 조건에서 nullable = false로 설정되어 있더라도, 프로젝션의 nullable 필드와 응답의 primitive 필드 간 타입 불일치는 명확한 문제입니다.
해결 방안:
- 의도가 null 가능성을 처리하는 것이라면, 소비 코드에서
.coalesce()또는 기본값 처리 필요 - 그렇지 않다면, 프로젝션에서 primitive
int로 유지하거나 응답 타입을Integer로 변경
🤖 Prompt for AI Agents
In
src/main/java/team/wego/wegobackend/group/v2/infrastructure/querydsl/projection/GroupListRow.java
around line 15, the field maxParticipants was changed to Integer (nullable)
while downstream code (GroupV2Service.java line ~149) still expects a primitive
int causing potential NPE on unboxing; fix by either making the projection use
primitive int again, or keep Integer and update the consumer to handle nulls
(apply coalesce/default in the QueryDSL projection or perform null-to-default
conversion before calling GetGroupListV2Response.GroupListItemV2Response.of()),
and ensure the response DTO signature matches the chosen type (change to Integer
if null is valid, or change projection back to int if null must never occur).
| @RequestParam(defaultValue = "20") int size, | ||
| @RequestParam(defaultValue = "ACTIVE") GroupListFilter filter, | ||
| @RequestParam(required = false) List<GroupV2Status> includeStatuses, | ||
| @RequestParam(required = false) List<GroupV2Status> excludeStatuses |
There was a problem hiding this comment.
API 문서에 파라미터 형식 명시 및 에러 처리 추가
List<GroupV2Status>는 기본적으로 두 가지 쿼리 파라미터 형식을 모두 지원합니다:
- 반복 형식:
?includeStatuses=RECRUITING&includeStatuses=FULL - 쉼표 구분 형식:
?includeStatuses=RECRUITING,FULL
단, 다음 사항을 주의해야 합니다:
- 열거형 이름 매칭은 대소문자를 구분합니다 (
RECRUITING만 인식,recruiting은 실패) - 유효하지 않은 값이 포함되면 HTTP 400 오류가 발생합니다
API 문서에 지원하는 파라미터 형식과 유효한 열거형 값을 명시하고, 필요시 전역 예외 처리로 사용자 친화적인 오류 응답을 구성하세요.
There was a problem hiding this comment.
Pull request overview
This PR enhances the V2 group list and detail APIs by adding status-based filtering, joinability indicators, and role-based visibility of participant information. The changes introduce a flexible filtering system with include/exclude status options, calculate remaining seats and join availability on the backend, and differentiate what information hosts vs. non-hosts can see about group participants.
- Adds
GroupListFilterenum and status filtering parameters (includeStatuses,excludeStatuses) to group list API - Introduces
status,remainingSeats, andjoinablefields to group list response items - Updates group detail response to show all participant statuses to hosts, but only ATTEND status to non-hosts
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/http/group/v2/v2-group-get-list.http | New comprehensive HTTP test file covering various filtering scenarios including include/exclude combinations |
| src/test/http/group/v2/V2-group-cancel.http | Adds test cases for host and participant viewing group details with different permissions |
| src/main/java/team/wego/wegobackend/group/v2/presentation/GroupV2Controller.java | Adds filter parameters to getGroupList endpoint with default values; renames variable for clarity |
| src/main/java/team/wego/wegobackend/group/v2/infrastructure/querydsl/projection/GroupListRow.java | Adds status field to projection for filtering support |
| src/main/java/team/wego/wegobackend/group/v2/infrastructure/querydsl/GroupV2QueryRepositoryImpl.java | Migrates to V2 entities; adds include/exclude status filtering in query; updates to use new image variant system |
| src/main/java/team/wego/wegobackend/group/v2/domain/repository/GroupV2QueryRepository.java | Updates method signature to accept include/exclude status parameters |
| src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2Service.java | Implements complex filtering logic handling filter presets, explicit includes/excludes, and conflict resolution; notes concurrency TODO |
| src/main/java/team/wego/wegobackend/group/v2/application/dto/response/GetGroupV2Response.java | Adds logic to show all participant statuses to hosts, only ATTEND status to others; uses Objects.equals for null-safe comparison |
| src/main/java/team/wego/wegobackend/group/v2/application/dto/response/GetGroupListV2Response.java | Adds status, remainingSeats, and joinable fields; implements joinability calculation based on status and capacity |
| src/main/java/team/wego/wegobackend/group/v2/application/dto/request/GroupListFilter.java | New enum defining ACTIVE, ARCHIVED, and ALL filter presets with default include/exclude status lists |
Comments suppressed due to low confidence (1)
src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2Service.java:279
- There's a race condition between checking attendCount and enforcing the capacity limit. Between lines 274-275 (counting participants) and line 276 (checking capacity), another user could join the group, causing the count to exceed maxParticipants. The TODO comment on line 230 acknowledges this issue but it hasn't been resolved. Consider using database-level constraints, optimistic locking, or pessimistic locking (e.g., SELECT FOR UPDATE on the group row) to prevent over-capacity scenarios.
// 정원 체크 수행. 재참여 포함해서 체크하는 게 안전
long attendCount = groupUserV2Repository.countByGroupIdAndStatus(groupId,
GroupUserV2Status.ATTEND);
if (attendCount > group.getMaxParticipants()) {
// 방금 reAttend로 늘었는데 초과하면 롤백시키기 위해 예외
throw new GroupException(GroupErrorCode.GROUP_IS_FULL, groupId);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // include가 오면 include가 우선: filter include 무시 | ||
| if (hasInclude) { | ||
| includes = includeStatuses; | ||
| // include가 명시된 순간 filter의 exclude까지 섞이면 예측이 어려워져서 초기화 | ||
| excludes = List.of(); | ||
| } | ||
|
|
||
| // exclude가 오면 exclude는 요청값으로 덮어쓰기 | ||
| if (hasExclude) { | ||
| excludes = excludeStatuses; | ||
| } |
There was a problem hiding this comment.
When user provides includeStatuses, the code clears the excludes list on line 90, even if the user also explicitly provided excludeStatuses. This means a request with both parameters (like test case 6-7 on line 240 of v2-group-get-list.http) will have excludeStatuses overwritten again on line 95 if hasExclude is true, but the filter's default excludes will be lost. The logic flow is: 1) set excludes from filter default (line 76), 2) clear excludes because includeStatuses exists (line 90), 3) restore excludes from request if hasExclude (line 95). This means filter defaults are incorrectly discarded. Consider preserving user-provided excludeStatuses throughout or clarifying the precedence rules.
| // 안전장치: filter=ALL이면 "전체"가 맞으므로 기본값 강제 X | ||
| // filter != ALL이고, 아무것도 안 왔고, includes가 비어있다면 ACTIVE 기본값 적용 | ||
| if (!hasInclude && !hasExclude && filter != GroupListFilter.ALL && includes.isEmpty()) { | ||
| includes = List.of(GroupV2Status.RECRUITING, GroupV2Status.FULL); | ||
| } |
There was a problem hiding this comment.
The condition on line 100 is checking if filter != ALL && includes.isEmpty() but this logic is executed after include/exclude parameters have already been processed (lines 87-96). The comment on line 99 says "filter != ALL이고, 아무것도 안 왔고" (if filter != ALL and nothing came) but at this point includes may be empty due to line 90's clearing logic, not because the user didn't provide anything. This makes the fallback logic unreliable. Consider restructuring to check the original parameter state (hasInclude/hasExclude) rather than the processed includes list.
| // GroupListFilter의 default가 절대 null 반환하지 않게 보장 | ||
| if (includes == null) { | ||
| includes = List.of(); | ||
| } | ||
| if (excludes == null) { | ||
| excludes = List.of(); | ||
| } | ||
|
|
There was a problem hiding this comment.
The null checks on lines 79-84 are defensive but unnecessary. The GroupListFilter enum methods defaultIncludeStatuses() and defaultExcludeStatuses() both return non-null List.of() values (as seen in GroupListFilter.java lines 11-21), so includes and excludes can never be null at this point. These checks add unnecessary code complexity.
| // GroupListFilter의 default가 절대 null 반환하지 않게 보장 | |
| if (includes == null) { | |
| includes = List.of(); | |
| } | |
| if (excludes == null) { | |
| excludes = List.of(); | |
| } |
| includes = List.of(GroupV2Status.RECRUITING, GroupV2Status.FULL); | ||
| } | ||
|
|
||
| // 4) 충돌 제거: exclude 우선한다. 람다 캡처 문제 없애기 위해 Set 연산으로 처리 |
There was a problem hiding this comment.
The comment says "4) 충돌 제거" (4) Remove conflicts) but there's no step 1, 2, or 3 in the comments. This numbering is misleading and makes the code harder to follow. Consider either adding consistent numbered steps throughout the method or removing the number.
| // 4) 충돌 제거: exclude 우선한다. 람다 캡처 문제 없애기 위해 Set 연산으로 처리 | |
| // 충돌 제거: exclude 우선한다. 람다 캡처 문제 없애기 위해 Set 연산으로 처리 |
| } else { // 비HOST: ATTEND만 노출 | ||
| joinedMembers = users.stream() | ||
| .filter(groupUserV2 -> groupUserV2.getStatus() == GroupUserV2Status.ATTEND) | ||
| .map(groupUserV2 -> JoinedMember.from(groupUserV2, true)) // 여기서는 ATTEND만 들어오니 status 노출해도 OK |
There was a problem hiding this comment.
For non-host viewers, the code filters to show only ATTEND members and then sets exposeStatus=true. This means the status field in the response will always be ATTEND for non-host viewers, which is redundant information. According to the comment on line 36, non-hosts should only see ATTEND status members, but exposing the status field when it's always the same value provides no additional information. Consider passing false for exposeStatus in the non-host case to be consistent with the privacy principle stated in the comment.
| } else { // 비HOST: ATTEND만 노출 | |
| joinedMembers = users.stream() | |
| .filter(groupUserV2 -> groupUserV2.getStatus() == GroupUserV2Status.ATTEND) | |
| .map(groupUserV2 -> JoinedMember.from(groupUserV2, true)) // 여기서는 ATTEND만 들어오니 status 노출해도 OK | |
| } else { // 비HOST: ATTEND만 노출 (status/leftAt 비노출) | |
| joinedMembers = users.stream() | |
| .filter(groupUserV2 -> groupUserV2.getStatus() == GroupUserV2Status.ATTEND) | |
| .map(groupUserV2 -> JoinedMember.from(groupUserV2, false)) |
| public ResponseEntity<ApiResponse<GetGroupListV2Response>> getGroupList( | ||
| @RequestParam(required = false) String keyword, | ||
| @RequestParam(required = false) Long cursor, | ||
| @RequestParam int size | ||
| @RequestParam(defaultValue = "20") int size, | ||
| @RequestParam(defaultValue = "ACTIVE") GroupListFilter filter, | ||
| @RequestParam(required = false) List<GroupV2Status> includeStatuses, | ||
| @RequestParam(required = false) List<GroupV2Status> excludeStatuses | ||
| ) { |
There was a problem hiding this comment.
The new filter parameters (filter, includeStatuses, excludeStatuses) lack API documentation. Consider adding @parameter annotations with descriptions to document the purpose and interaction of these filtering options, especially since they have complex precedence rules (as seen in GroupV2Service lines 86-110). This would help API consumers understand how to use these parameters correctly.
| // 안전장치: filter=ALL이면 "전체"가 맞으므로 기본값 강제 X | ||
| // filter != ALL이고, 아무것도 안 왔고, includes가 비어있다면 ACTIVE 기본값 적용 | ||
| if (!hasInclude && !hasExclude && filter != GroupListFilter.ALL && includes.isEmpty()) { | ||
| includes = List.of(GroupV2Status.RECRUITING, GroupV2Status.FULL); | ||
| } | ||
|
|
There was a problem hiding this comment.
This condition will never be true and is unreachable code. The check includes.isEmpty() can only be true when filter=ALL (which returns an empty list from defaultIncludeStatuses), but the condition also requires filter != GroupListFilter.ALL. These constraints are mutually exclusive. If the intent was to provide a fallback when no filters are specified, this logic needs to be restructured to check the original request parameters before applying filter defaults.
| // 안전장치: filter=ALL이면 "전체"가 맞으므로 기본값 강제 X | |
| // filter != ALL이고, 아무것도 안 왔고, includes가 비어있다면 ACTIVE 기본값 적용 | |
| if (!hasInclude && !hasExclude && filter != GroupListFilter.ALL && includes.isEmpty()) { | |
| includes = List.of(GroupV2Status.RECRUITING, GroupV2Status.FULL); | |
| } |
| //RECRUITING + 남은 자리 > 0: 참여 가능 | ||
| //FULL/CLOSED/CANCELLED/FINISHED: 참여 불가 |
There was a problem hiding this comment.
The comment is missing a space after the slashes. Standard Java formatting requires a space after the comment marker (e.g., "// RECRUITING" instead of "//RECRUITING").
| //RECRUITING + 남은 자리 > 0: 참여 가능 | |
| //FULL/CLOSED/CANCELLED/FINISHED: 참여 불가 | |
| // RECRUITING + 남은 자리 > 0: 참여 가능 | |
| // FULL/CLOSED/CANCELLED/FINISHED: 참여 불가 |
github-project-automation
bot
moved this from In progress
to Done
in WeGo-Together Backend
1 participant
📝 Pull Request
📌 PR 종류
해당하는 항목에 체크해주세요.
✨ 변경 내용
모임 상태 기반 필터링 옵션을 추가하여 모임 목록 API를 개선했습니다. 모임 목록 항목에 상태 배지 표시 및 참여 가능 상태 표시 기능을 추가했습니다. 모임 상세 정보 API를 개선하여 호스트의 참가자 상태와 남은 시간을 표시하고,호스트가 아닌 참가자에게는 참석 상태만 표시합니다.
남은 좌석 계산 및 참여 가능 상태 로직을 수정했습니다
🔍 관련 이슈
🧪 테스트
변경된 기능에 대한 테스트 범위 또는 테스트 결과를 작성해주세요.
🚨 확인해야 할 사항 (Checklist)
PR을 제출하기 전에 아래 항목들을 확인해주세요.
🙋 기타 참고 사항
리뷰어가 참고하면 좋을 만한 추가 설명이 있다면 적어주세요.
Summary by CodeRabbit
릴리스 노트
새로운 기능
테스트
✏️ Tip: You can customize this high-level summary in your review settings.