Conversation
Zepelown
requested review from
PororoAndFriends,
lepitaaar,
oesnuj and
seongwon030
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
추천 클럽 검색 로직 단계화backend/src/main/java/moadong/club/repository/ClubSearchRepository.java |
searchRecommendClubs를 3단계(카테고리 내 모집중 → 카테고리 내 보충 → 랜덤 풀 우선선별)로 재구성, 총 6개 제한 및 중복 방지 도입 |
헬퍼 메서드 추가.../ClubSearchRepository.java |
findClubsByCategoryAndState, addClubs, isRecruiting, selectClubsByStatePriority, findRandomClubs 추가 |
프로젝션 기본값 조정.../ClubSearchRepository.java |
tags 기본값을 빈 문자열에서 빈 리스트(Collections.emptyList())로 변경; 로고 기본값 빈 문자열 유지 |
임포트 정리.../ClubSearchRepository.java |
java.util.* 와일드카드 임포트로 변경 |
Sequence Diagram(s)
sequenceDiagram
actor Client
participant Repo as ClubSearchRepository
Client->>Repo: searchRecommendClubs(category, excludeClubId)
activate Repo
Repo->>Repo: excludeIds 초기화(add excludeClubId)
Repo->>Repo: findClubsByCategoryAndState(category, excludeIds, onlyRecruit=true, limit=4)
Repo->>Repo: addClubs(result, excludeIds, stage1)
alt result < 4
Repo->>Repo: findClubsByCategoryAndState(category, excludeIds, onlyRecruit=false, limit=remaining)
Repo->>Repo: addClubs(result, excludeIds, stage2)
end
alt result < 6
Repo->>Repo: findRandomClubs(excludeIds, sampleSize=10)
Repo->>Repo: selectClubsByStatePriority(pool, maxCount=6-result)
Repo->>Repo: addClubs(result, excludeIds, stage3)
end
Repo-->>Client: result(최대 6개, 중복 제거)
deactivate Repo
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
| 같은 분과에 모집 중인 동아리 상위 5개를 보여준다 (MOA-161) | ❌ | 최대 6개 반환, 비모집 클럽 포함 가능, “상위” 정렬 기준 부재 |
Possibly related issues
- [feature] MOA-161 추천 동아리를 보여준다 - be #655: 추천 동아리 백엔드 로직 구현과 직접적으로 동일 영역(searchRecommendClubs) 변경.
Possibly related PRs
- [feature]추천 동아리를 보여주는 API 개발 #656: 동일 메서드의 단일-집계 샘플링 구현과 본 PR의 단계적 선별 로직이 상충/대체 관계.
- [fix] 동아리 검색 API #586: 모집 상태 우선순위 처리 로직을 다루며, 본 PR의 isRecruiting/우선선별과 연관.
Suggested labels
✨ Feature, 💾 BE
Suggested reviewers
- PororoAndFriends
- oesnuj
- lepitaaar
Tip
🔌 Remote MCP (Model Context Protocol) integration is now available!
Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.
✨ Finishing Touches
- 📝 Generate Docstrings
🧪 Generate unit tests
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feature/#655-recommend-club-server-MOA-161
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.
🪧 Tips
Chat
There are 3 ways to chat with CodeRabbit:
- Review comments: Directly reply to a review comment made by CodeRabbit. Example:
I pushed a fix in commit <commit_id>, please review it.Open a follow-up GitHub issue for this discussion.
- Files and specific lines of code (under the "Files changed" tab): Tag
@coderabbitaiin a new review comment at the desired location with your query. - PR comments: Tag
@coderabbitaiin a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
Support
Need help? Create a ticket on our support page for assistance with any issues or questions.
CodeRabbit Commands (Invoked using PR/Issue comments)
Type @coderabbitai help to get the list of available commands.
Other keywords and placeholders
- Add
@coderabbitai ignoreanywhere in the PR description to prevent this PR from being reviewed. - Add
@coderabbitai summaryto generate the high-level summary at a specific location in the PR description. - Add
@coderabbitaianywhere in the PR title to generate the title automatically.
Status, Documentation and Community
- Visit our Status Page to check the current availability of CodeRabbit.
- Visit our Documentation for detailed information on how to use CodeRabbit.
- Join our Discord Community to get help, request features, and share feedback.
- Follow us on X/Twitter for updates and announcements.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Test Results75 tests 75 ✅ 2s ⏱️ Results for commit c61dc77. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
backend/src/main/java/moadong/club/repository/ClubSearchRepository.java (6)
3-3: 와일드카드 import 대신 명시적 import 권장팀 컨벤션에 따라 다르지만,
java.util.*대신 사용하는 타입만 명시하는 것이 가독성/충돌 방지에 유리합니다. 큰 이슈는 아니므로 필요 시에만 반영하세요.
76-87: remainCount 계산은 result.size() 기준이 더 견고합니다현재는
remainCount = 4 - categoryClubs.size()로 계산하는데, 일반적으로는 이미 반영된 결과 리스트(result) 기준으로 남은 수량을 계산하는 편이 견고합니다. 향후 중복 필터링이나 쿼리 변경 시에도 안전합니다.다음과 같이 수정 제안드립니다:
- int remainCount = maxCategoryCount - categoryClubs.size(); + int remainCount = maxCategoryCount - result.size();
139-145: 방어 로직: id 누락 시 스킵예외적인 매핑/프로젝션 이슈로
club.id()가 null이 될 가능성에 대비해, null인 경우를 건너뛰면 안정성이 높아집니다.for (ClubSearchResult club : clubs) { - if (!excludeIds.contains(club.id())) { + if (club.id() == null) { + continue; + } + if (!excludeIds.contains(club.id())) { result.add(club); excludeIds.add(club.id()); } }
93-98: (선택) 랜덤 풀 샘플 크기를 요청 수량에 맞춰 가변화현재 샘플을 고정 10개로 가져와 우선순위 선별합니다. 모집 중인 대상이 적은 데이터셋에서는
randomNeeded를 충분히 채우지 못할 확률이 커집니다. 요청 수량에 비례해 샘플을 키우면 완성도 향상에 도움이 됩니다.- List<ClubSearchResult> randomPool = findRandomClubs(excludeIds, 10); + int sampleSize = Math.max(10, randomNeeded * 3); + List<ClubSearchResult> randomPool = findRandomClubs(excludeIds, sampleSize);
127-131: tags 기본값 타입 일관성 확인(빈 문자열 vs 빈 리스트)본 변경에서는
tags의 기본값을Collections.emptyList()로 투영했는데,searchClubsByKeyword에서는 빈 문자열("")을 사용 중입니다. DTO의tags타입이List<String>이라면 빈 문자열 투영은 매핑 오류를 유발할 수 있습니다. 전역적으로 빈 리스트로 맞추는 것을 권장합니다.searchClubsByKeyword의 project 단계(참고: 현재 코드는 빈 문자열)도 다음과 같이 맞추는 것을 제안드립니다. 이 수정은 본 코멘트 범위를 벗어난 위치에 있으므로 참고용 예시입니다.
// searchClubsByKeyword 내 project 단계 예시 Aggregation.project("name", "state", "category", "division") .and("recruitmentInformation.introduction").as("introduction") .and("recruitmentInformation.clubRecruitmentStatus").as("recruitmentStatus") .and(ConditionalOperators.ifNull("$recruitmentInformation.logo").then("")).as("logo") .and(ConditionalOperators.ifNull("$recruitmentInformation.tags").then(Collections.emptyList())).as("tags");Also applies to: 192-194
68-76: (선택) category 미지정 시 전체 랜덤만으로 구성하는 빠른 가드 추가
category == null/공백/“all”과 같은 값이 들어올 가능성이 있다면, 1,2단계를 건너뛰고 바로 3단계(전체 랜덤)로 가는 가드를 두면 불필요한 쿼리를 줄일 수 있습니다. 필요 시 말씀 주시면 패치 드리겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
backend/src/main/java/moadong/club/repository/ClubSearchRepository.java(2 hunks)
🔇 Additional comments (4)
backend/src/main/java/moadong/club/repository/ClubSearchRepository.java (4)
69-74: excludeIds 세팅 방식 LGTM중복 방지를 위한
excludeIds초기화 및 단건 제외 처리 흐름이 명확합니다.
76-78: 요건 정합성 확인: 4+2 구성과 MOA-161(동일 카테고리 5개) 상이본 로직은 동일 카테고리 최대 4개 + 전체 랜덤 2개(총 6개)인데, 연결 이슈(MOA-161) 체크리스트에는 “동일 카테고리의 모집 중 상위 5개” 노출이 요구사항으로 적혀 있습니다. 최신 기획 기준이 4+2인지, 아니면 5+1 등의 변형이 필요한지 확인 부탁드립니다. 필요 시
maxCategoryCount또는totalNeeded조정으로 쉽게 반영 가능합니다.Also applies to: 90-91
148-151: 모집 상태에 UPCOMING 포함 여부 확인 필요현재 우선순위는 ALWAYS/OPEN만 모집 중으로 간주합니다. 검색 조건(getMatchedCriteria)에서는 OPEN 선택 시 ALWAYS/OPEN/UPCOMING까지 포함시키고 있어, 추천에서도 UPCOMING을 모집 중으로 볼지 여부를 기획과 일치시킬 필요가 있습니다.
153-178: 상태 우선 선별 로직 LGTM풀에서 모집 중 → 비모집 순 선별이 명확합니다. 중복 방지용
ids도 안전 장치로 적절합니다.
| Criteria criteria = Criteria.where("category").is(category) | ||
| .and("_id").nin(excludeIds); | ||
|
|
||
| if (onlyRecruitAvailable) { |
There was a problem hiding this comment.
중요: 사용 불가 상태(state != AVAILABLE) 클럽이 추천에 포함될 수 있음 + id 별칭을 명시적으로 투영 권장
- 현재 카테고리 조회에서
state == AVAILABLE필터가 없습니다. 이렇게 되면 HIDDEN/DELETED 등 비노출 상태의 클럽이 추천에 섞일 수 있습니다. 검색(keyword) 파이프라인에서는state == AVAILABLE를 적용하고 있어 일관성도 깨집니다. - 또한 dedup에
club.id()를 사용하므로_id필드가 DTO의id로 매핑되지 않는 경우를 방지하기 위해 프로젝트 단계에서 명시적으로id를 투영하는 편이 안전합니다.
다음과 같이 보완을 제안드립니다.
- Criteria criteria = Criteria.where("category").is(category)
- .and("_id").nin(excludeIds);
+ Criteria criteria = Criteria.where("state").is(ClubState.AVAILABLE.getName())
+ .and("category").is(category)
+ .and("_id").nin(excludeIds); ops.add(
Aggregation.project("name", "state", "category", "division")
.and("recruitmentInformation.introduction").as("introduction")
.and("recruitmentInformation.clubRecruitmentStatus").as("recruitmentStatus")
.and(ConditionalOperators.ifNull("$recruitmentInformation.logo").then(""))
.as("logo")
.and(ConditionalOperators.ifNull("$recruitmentInformation.tags").then(Collections.emptyList()))
.as("tags")
+ .and("_id").as("id")
);Also applies to: 119-121, 124-131
| private List<ClubSearchResult> findRandomClubs(Set<String> excludeIds, int sampleSize) { | ||
| List<AggregationOperation> ops = new ArrayList<>(); | ||
| ops.add(Aggregation.match(Criteria.where("_id").nin(excludeIds))); | ||
| ops.add(Aggregation.sample((long) sampleSize)); | ||
|
|
||
| ops.add( | ||
| Aggregation.project("name", "state", "category", "division") | ||
| .and("recruitmentInformation.introduction").as("introduction") | ||
| .and("recruitmentInformation.clubRecruitmentStatus").as("recruitmentStatus") |
There was a problem hiding this comment.
중요: 랜덤 풀에서도 AVAILABLE 상태 필터 누락 + id 투영 명시
findRandomClubs 역시 state == AVAILABLE 조건이 없어 비노출 클럽이 추천될 수 있습니다. 또한 dedup 신뢰성을 위해 id 투영을 명시하는 것이 좋습니다.
- ops.add(Aggregation.match(Criteria.where("_id").nin(excludeIds)));
+ ops.add(Aggregation.match(new Criteria().andOperator(
+ Criteria.where("state").is(ClubState.AVAILABLE.getName()),
+ Criteria.where("_id").nin(excludeIds)
+ )));
ops.add(Aggregation.sample((long) sampleSize));
@@
ops.add(
Aggregation.project("name", "state", "category", "division")
.and("recruitmentInformation.introduction").as("introduction")
.and("recruitmentInformation.clubRecruitmentStatus").as("recruitmentStatus")
.and(ConditionalOperators.ifNull("$recruitmentInformation.logo").then(""))
.as("logo")
.and(ConditionalOperators.ifNull("$recruitmentInformation.tags").then(Collections.emptyList()))
- .as("tags")
+ .as("tags")
+ .and("_id").as("id")
);Also applies to: 187-194
🤖 Prompt for AI Agents
In backend/src/main/java/moadong/club/repository/ClubSearchRepository.java
around lines 181-189 (and similarly at 187-194), the aggregation for
findRandomClubs is missing a filter to include only clubs with state ==
AVAILABLE and does not explicitly project the id; add a
Criteria.where("state").is(ClubState.AVAILABLE) (or the appropriate enum/value)
to the Aggregation.match stage and add "_id" to the Aggregation.project call so
the pipeline filters out non-available clubs and returns the id for reliable
deduplication.
2 participants
#️⃣연관된 이슈
#655
📝작업 내용
동아리 추천 로직 개선
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit