Conversation
…github.com/pknu-wap/Moadong into feature/#655-recommend-club-server-MOA-161
Zepelown
requested review from
PororoAndFriends,
lepitaaar,
oesnuj and
seongwon030
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
DTO: ClubDetailedResult 확장backend/src/main/java/moadong/club/payload/dto/ClubDetailedResult.java |
recommendClubs: List<ClubSearchResult> 필드 추가. of(Club) → of(Club, List<ClubSearchResult>)로 시그니처 변경 및 빌더에 신규 필드 매핑. |
Repository: 추천 검색 추가backend/src/main/java/moadong/club/repository/ClubSearchRepository.java |
searchRecommendClubs(String category, String excludeClubId) 추가. 상태/카테고리/모집중 필터, 현재 동아리 제외, 랜덤 5개 샘플, 필드 프로젝션 후 List<ClubSearchResult> 반환. |
Service: 추천 연동backend/src/main/java/moadong/club/service/ClubProfileService.java |
ClubSearchRepository 주입 및 사용. getClubDetail에서 추천 목록 조회 후 ClubDetailedResult.of(club, recommendClubs)로 전달. |
Sequence Diagram(s)
sequenceDiagram
actor User
participant Controller as ClubController
participant Service as ClubProfileService
participant Repo as ClubSearchRepository
participant DB as MongoDB
User->>Controller: GET /clubs/{clubId}
Controller->>Service: getClubDetail(clubId)
Service->>DB: find Club by id
DB-->>Service: Club
Service->>Repo: searchRecommendClubs(category, excludeClubId)
Repo->>DB: Aggregation (match, sample 5, project)
DB-->>Repo: List<ClubSearchResult>
Repo-->>Service: List<ClubSearchResult>
Service-->>Controller: ClubDetailedResult(with recommendClubs)
Controller-->>User: 200 OK (JSON)
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
| 같은 분과에 모집 중인 동아리 상위 5개 제공 (MOA-161) | ❓ | 5개 반환은 구현되었으나 무작위 샘플이며 “상위” 기준(정렬/점수)이 정의·적용되지 않음. |
Assessment against linked issues: Out-of-scope changes
(해당 없음)
Possibly related issues
- [feature] MOA-161 추천 동아리를 보여준다 - be #655: 동일한 “같은 분과 추천 동아리 5개 반환” 기능을 다루며 본 변경사항과 목적이 일치합니다.
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 003aac3. |
![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
🧹 Nitpick comments (9)
backend/src/main/java/moadong/club/payload/dto/ClubDetailedResult.java (3)
37-44: NPE 가능성 점검: ClubRecruitmentInformation null 가정 제거 필요
club.getClubRecruitmentInformation()가 null일 경우 Line 40의hasRecruitmentPeriod()에서 NPE가 발생합니다. 생성/업데이트 단계에서 항상 생성된다는 보장이 없다면, 안전 가드를 추가하거나 기본값으로 대체하는 처리가 필요합니다.가능한 대응:
- 서비스 계층에서 상세 조회 전
ClubRecruitmentInformation존재 보장- DTO 생성 시 null-safe 접근(예: Optional 활용) 및 기본값 처리
73-79: 동일 객체의 반복 접근 및 NPE 가능성 최소화
club.getClubRecruitmentInformation()를 이미 지역 변수로 가져왔는데(Lines 39), 여기서는 다시 체이닝으로 접근합니다. 지역 변수 재사용이 가독성과 NPE 리스크 측면에서 더 안전합니다.다음과 같이 지역 변수를 재사용하는 변경을 제안합니다:
- .externalApplicationUrl(club.getClubRecruitmentInformation().getExternalApplicationUrl() == null ? "" : - club.getClubRecruitmentInformation().getExternalApplicationUrl()) + .externalApplicationUrl( + clubRecruitmentInformation.getExternalApplicationUrl() == null ? "" : + clubRecruitmentInformation.getExternalApplicationUrl() + ) ... - .faqs(club.getClubRecruitmentInformation().getFaqs() == null ? List.of() - : club.getClubRecruitmentInformation().getFaqs()) + .faqs(clubRecruitmentInformation.getFaqs() == null ? List.of() + : clubRecruitmentInformation.getFaqs())
46-79: 중복된 null-coalescing 로직 간소화 제안여러 필드에서
== null ? "" : ...또는List.of()반복이 많습니다. 헬퍼 메서드(예:orEmpty(String),orEmpty(List<T>))를 도입하면 가독성과 유지보수성이 좋아집니다. 레거시와의 일관성을 깨지 않으면서도 적용 가능합니다.backend/src/main/java/moadong/club/service/ClubProfileService.java (1)
57-59: 코드 스타일 사소한 정리
,뒤 공백이 누락되어 있습니다.- ClubDetailedResult clubDetailedResult = ClubDetailedResult.of( - club,clubSearchResults - ); + ClubDetailedResult clubDetailedResult = ClubDetailedResult.of( + club, clubSearchResults + );backend/src/main/java/moadong/club/repository/ClubSearchRepository.java (5)
84-86: 요구사항 해석 확인: “상위 5개” vs 랜덤 샘플현재는
$sample(5)로 “랜덤 5개”를 반환합니다. PR 설명에는 “상위 5개” 및 “랜덤 정렬”이 함께 언급되어 있는데, “상위” 기준(조회수, 지원수, 인기 점수 등)이 정의되어 있다면 우선 정렬로 상위 5개를 선택하고, 그 5개를 랜덤 셔플하여 반환하는 해석이 자연스럽습니다. 현 구현은 “상위” 개념을 반영하지 않습니다.정렬 기준이 있다면:
$match→$sort({ popularity: -1 })→$limit(5)→ 서비스 계층에서 셔플- 정렬 기준이 없다면, 문서화 상 “랜덤 5개”로 요건을 정정하는 것도 방법입니다.
87-93: tags의 기본값을 빈 문자열이 아닌 빈 배열로 반환 권장
tags는 보통 배열(List)입니다. 현재ifNull(...).then("")은 문자열을 할당하여 DTO 매핑 타입 불일치 가능성이 있습니다. 빈 배열로 기본값을 설정하는 것을 권장합니다.다음과 같이 변경을 제안합니다:
- .and(ConditionalOperators.ifNull("$recruitmentInformation.tags").then("")).as("tags") + .and(ConditionalOperators.ifNull("$recruitmentInformation.tags").then(java.util.Collections.emptyList())).as("tags")참고: 위 변경을 적용하려면
java.util.Collections사용을 위해 정적 임포트가 필요 없습니다. 런타임에 빈 배열 리터럴로 직렬화됩니다.
72-83: 분과 기준 확인: category vs division요구사항의 “같은 분과”가 내부 도메인에서
division을 의미한다면, 현재의and("category").is(category)는 조건 해석이 어긋납니다. 반대로 “분과=category”라면 그대로 유지가 맞습니다.
division이 맞다면 다음과 같이 수정:- .and("category").is(category) + .and("division").is(category) // 메서드 파라미터명과 의미도 함께 정리 권장또는 메서드 시그니처를
searchRecommendClubs(String division, String excludeClubId)로 명확히 바꾸는 것을 권장합니다.
68-101: category=null 시 동작 정의 필요현재는
category가 null이면is(null)로 매칭되어 결과가 거의 비어질 수 있습니다. Fallback 전략(예: division으로 대체, 전역 랜덤 5개, 빈 리스트 반환)을 정의해 주세요.예: null이면 추천을 비활성화하고 빈 리스트 반환
- public List<ClubSearchResult> searchRecommendClubs(String category, String excludeClubId) { + public List<ClubSearchResult> searchRecommendClubs(String category, String excludeClubId) { + if (category == null || category.isBlank()) { + return java.util.List.of(); + }
68-101: 쿼리 성능을 위한 인덱스 검토다음 필드에 대한 복합 인덱스를 고려해 주세요:
(state, category, recruitmentInformation.clubRecruitmentStatus). 샘플링 이전의 매치 비용을 줄일 수 있습니다.
📜 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 (3)
backend/src/main/java/moadong/club/payload/dto/ClubDetailedResult.java(1 hunks)backend/src/main/java/moadong/club/repository/ClubSearchRepository.java(1 hunks)backend/src/main/java/moadong/club/service/ClubProfileService.java(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/src/main/java/moadong/club/payload/dto/ClubDetailedResult.java (1)
frontend/src/types/club.ts (1)
Club(3-12)
🔇 Additional comments (2)
backend/src/main/java/moadong/club/payload/dto/ClubDetailedResult.java (1)
34-35: 추천 동아리 필드 추가 아주 좋습니다상세 DTO에
recommendClubs를 포함해 API 일관성 있게 확장됐습니다. 직렬화 시에도 자연스럽게 포함될 것으로 보입니다.backend/src/main/java/moadong/club/service/ClubProfileService.java (1)
26-28: 리포지토리 의존성 주입 OK추천 검색을 위해
ClubSearchRepository를 주입한 방향성 적절합니다. 트랜잭션 범위가 필요 없고 조회 전용이라 현재 구조로 충분해 보입니다.
| public List<ClubSearchResult> searchRecommendClubs(String category, String excludeClubId) { | ||
|
|
||
| List<AggregationOperation> operations = new ArrayList<>(); | ||
| // 모집 상태 & 같은 category & 제외할 club _id 필터 | ||
| operations.add(Aggregation.match( | ||
| new Criteria() | ||
| .and("state").is(ClubState.AVAILABLE.getName()) | ||
| .and("category").is(category) | ||
| .and("_id").ne(excludeClubId) | ||
| .and("recruitmentInformation.clubRecruitmentStatus") | ||
| .in( | ||
| ClubRecruitmentStatus.ALWAYS.toString(), | ||
| ClubRecruitmentStatus.OPEN.toString(), | ||
| ClubRecruitmentStatus.UPCOMING.toString() | ||
| ) | ||
| )); |
There was a problem hiding this comment.
자기 자신 제외 필터가 작동하지 않을 수 있음(String vs ObjectId 비교)
and("_id").ne(excludeClubId)는 _id:ObjectId와 String의 타입 불일치로 매치되지 않을 수 있습니다. ObjectId로 변환해 비교해야 합니다.
다음 패치를 제안합니다:
@@
- public List<ClubSearchResult> searchRecommendClubs(String category, String excludeClubId) {
+ public List<ClubSearchResult> searchRecommendClubs(String category, String excludeClubId) {
@@
- operations.add(Aggregation.match(
- new Criteria()
- .and("state").is(ClubState.AVAILABLE.getName())
- .and("category").is(category)
- .and("_id").ne(excludeClubId)
- .and("recruitmentInformation.clubRecruitmentStatus")
- .in(
- ClubRecruitmentStatus.ALWAYS.toString(),
- ClubRecruitmentStatus.OPEN.toString(),
- ClubRecruitmentStatus.UPCOMING.toString()
- )
- ));
+ operations.add(Aggregation.match(
+ new Criteria()
+ .and("state").is(ClubState.AVAILABLE.getName())
+ .and("category").is(category)
+ .and("_id").ne(new org.bson.types.ObjectId(excludeClubId))
+ .and("recruitmentInformation.clubRecruitmentStatus")
+ .in(
+ ClubRecruitmentStatus.ALWAYS.toString(),
+ ClubRecruitmentStatus.OPEN.toString(),
+ ClubRecruitmentStatus.UPCOMING.toString()
+ )
+ ));또는 메서드 시그니처를 ObjectId excludeClubId로 변경하고 서비스에서 objectId를 직접 전달하는 것도 좋습니다.
📝 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.
| public List<ClubSearchResult> searchRecommendClubs(String category, String excludeClubId) { | |
| List<AggregationOperation> operations = new ArrayList<>(); | |
| // 모집 상태 & 같은 category & 제외할 club _id 필터 | |
| operations.add(Aggregation.match( | |
| new Criteria() | |
| .and("state").is(ClubState.AVAILABLE.getName()) | |
| .and("category").is(category) | |
| .and("_id").ne(excludeClubId) | |
| .and("recruitmentInformation.clubRecruitmentStatus") | |
| .in( | |
| ClubRecruitmentStatus.ALWAYS.toString(), | |
| ClubRecruitmentStatus.OPEN.toString(), | |
| ClubRecruitmentStatus.UPCOMING.toString() | |
| ) | |
| )); | |
| public List<ClubSearchResult> searchRecommendClubs(String category, String excludeClubId) { | |
| List<AggregationOperation> operations = new ArrayList<>(); | |
| // 모집 상태 & 같은 category & 제외할 club _id 필터 | |
| operations.add(Aggregation.match( | |
| new Criteria() | |
| .and("state").is(ClubState.AVAILABLE.getName()) | |
| .and("category").is(category) | |
| .and("_id").ne(new org.bson.types.ObjectId(excludeClubId)) | |
| .and("recruitmentInformation.clubRecruitmentStatus") | |
| .in( | |
| ClubRecruitmentStatus.ALWAYS.toString(), | |
| ClubRecruitmentStatus.OPEN.toString(), | |
| ClubRecruitmentStatus.UPCOMING.toString() | |
| ) | |
| )); | |
| // …rest of the method… | |
| } |
🤖 Prompt for AI Agents
backend/src/main/java/moadong/club/repository/ClubSearchRepository.java around
lines 68 to 83: the exclusion filter uses and("_id").ne(excludeClubId) which
compares a String to an ObjectId and can fail; convert the excludeClubId to an
ObjectId before comparing (e.g., import org.bson.types.ObjectId and use new
ObjectId(excludeClubId) in the Criteria) or change the method signature to
accept ObjectId excludeClubId and use that directly, and add null/invalid-id
handling to skip the ne(...) clause when the exclude id is blank/invalid.
| operations.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("")).as("tags") | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
_id(id) 필드 노출 필요 여부 확인 및 추가 제안
추천 목록을 클릭해 상세로 이동해야 한다면 id가 필요합니다. 현재 project에 _id가 포함되지 않아 DTO에 매핑되지 않을 수 있습니다.
다음과 같이 id를 포함시키는 것을 제안합니다:
- Aggregation.project("name", "state", "category", "division")
+ Aggregation.project("name", "state", "category", "division")
+ .and("_id").as("id")ClubSearchResult에 id 필드가 없다면 DTO에도 반영이 필요합니다.
다음 스크립트로 ClubSearchResult 정의를 확인해 주세요:
🏁 Script executed:
#!/bin/bash
# ClubSearchResult의 필드 정의 확인
rg -n -C2 --type=java 'record\s+ClubSearchResult|class\s+ClubSearchResult' backend/src/main/javaLength of output: 536
Aggregation 프로젝션에 _id 필드 매핑 누락
프로젝트 단계에서 MongoDB의 _id 값을 매핑하지 않아 ClubSearchResult.id 필드에 값이 들어오지 않습니다.
다음처럼 .and("_id").as("id")를 추가해주세요.
파일: backend/src/main/java/moadong/club/repository/ClubSearchRepository.java
위치: 87–93행
operations.add(
- Aggregation.project("name", "state", "category", "division")
+ Aggregation.project("name", "state", "category", "division")
+ .and("_id").as("id")
.and("recruitmentInformation.introduction").as("introduction")
.and("recruitmentInformation.clubRecruitmentStatus").as("recruitmentStatus")
.and(ConditionalOperators.ifNull("$recruitmentInformation.logo").then("")).as("logo")ClubSearchResult레코드에String id필드가 정의되어 있어(확인됨), DTO 매핑을 위해 반드시 프로젝션에 포함시켜야 합니다.- 반영 후, 클럽 상세 조회 링크 등에 정상적으로
id가 전달되는지 확인해주세요.
📝 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.
| operations.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("")).as("tags") | |
| ); | |
| operations.add( | |
| Aggregation.project("name", "state", "category", "division") | |
| .and("_id").as("id") | |
| .and("recruitmentInformation.introduction").as("introduction") | |
| .and("recruitmentInformation.clubRecruitmentStatus").as("recruitmentStatus") | |
| .and(ConditionalOperators.ifNull("$recruitmentInformation.logo").then("")).as("logo") | |
| .and(ConditionalOperators.ifNull("$recruitmentInformation.tags").then("")).as("tags") | |
| ); |
🤖 Prompt for AI Agents
In backend/src/main/java/moadong/club/repository/ClubSearchRepository.java
around lines 87–93, the aggregation projection omits MongoDB's _id so
ClubSearchResult.id remains empty; update the projection to include the _id
mapping by adding .and("_id").as("id") to the Aggregation.project chain, then
re-run tests or a sample query to confirm the id is correctly populated and
propagated to club detail links.
| List<ClubSearchResult> clubSearchResults = clubSearchRepository.searchRecommendClubs(club.getCategory(), clubId); | ||
|
|
||
| ClubDetailedResult clubDetailedResult = ClubDetailedResult.of( | ||
| club | ||
| club,clubSearchResults | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
자기 자신 제외 필터가 동작하지 않을 가능성(ObjectId 타입 불일치)
searchRecommendClubs(club.getCategory(), clubId)로 문자열 clubId를 전달하고, 리포지토리에서는 _id 필드를 비교합니다. MongoDB의 _id가 ObjectId 타입이라면 String 비교는 매치되지 않아 자기 자신이 제외되지 않을 수 있습니다. 리포지토리에서 new ObjectId(excludeClubId)로 변환해 비교하도록 수정하는 것이 안전합니다. (아래 리포지토리 코멘트에 패치 제안 포함)
변경 후, 동일 클럽이 추천 결과에 포함되지 않는지 꼭 수동/자동 테스트로 검증해 주세요.
🏁 Script executed:
#!/bin/bash
set -e
# searchRecommendClubs 메서드 선언 및 구현 위치 확인
rg -nC3 'searchRecommendClubs' -g '*.java'Length of output: 1708
🏁 Script executed:
#!/bin/bash
# searchRecommendClubs 구현부 전체 확인
sed -n '1,150p' backend/src/main/java/moadong/club/repository/ClubSearchRepository.javaLength of output: 6099
🏁 Script executed:
#!/bin/bash
# ClubProfileService 근처 코드 확인
sed -n '1,100p' backend/src/main/java/moadong/club/service/ClubProfileService.javaLength of output: 2535
ObjectId 타입 불일치로 자기 자신 제외 필터 미동작 가능성 확인 및 수정 필요
searchRecommendClubs(String category, String excludeClubId)에서 _id(ObjectId)와 excludeClubId(String)를 직접 비교하고 있어 타입 불일치로 필터가 적용되지 않을 수 있습니다. String을 ObjectId로 변환해 비교하도록 변경해야 합니다.
-
위치
- backend/src/main/java/moadong/club/repository/ClubSearchRepository.java
searchRecommendClubs메서드
- backend/src/main/java/moadong/club/repository/ClubSearchRepository.java
-
제안 수정사항
// 기존: String 타입 excludeClubId를 _id와 비교 .and("_id").ne(excludeClubId) +// 수정: String → ObjectId 변환 후 비교 +.and("_id").ne(new ObjectId(excludeClubId)) -
import 추가
import org.bson.types.ObjectId;
-
검증
변경 후 동일한 클럽이 추천 결과에서 정상적으로 제외되는지 자동/수동 테스트로 확인 부탁드립니다.
🤖 Prompt for AI Agents
backend/src/main/java/moadong/club/service/ClubProfileService.java around lines
55-59: the recommend-club query currently may fail to exclude the current club
because ClubSearchRepository.searchRecommendClubs compares MongoDB _id
(ObjectId) with a String excludeClubId; update searchRecommendClubs in
backend/src/main/java/moadong/club/repository/ClubSearchRepository.java to
convert the String excludeClubId to an org.bson.types.ObjectId (add import
org.bson.types.ObjectId) and use that ObjectId in the query/filter (handle
null/invalid id formats defensively), then run automated/manual tests to confirm
the same club is properly excluded from recommendations.
3 participants
#️⃣연관된 이슈
#655
📝작업 내용
상세 페이지 API 요청 시 추천 동아리를 추가했습니다.
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은
🫡 참고사항
Summary by CodeRabbit