Conversation
lepitaaar
requested review from
PororoAndFriends,
Zepelown,
oesnuj,
seongwon030 and
suhyun113
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
Entity mutator 리팩터링backend/src/main/java/moadong/club/entity/ClubApplication.java |
updateDetail(memo, status) 제거. updateMemo(String), updateStatus(ApplicationStatus), updateAnswers(List) 추가. updateAnswers는 기존 답변을 대체(클리어 후 추가). 기타 필드/생성 로직 변경 없음. |
Service 로직 정렬/호출 변경backend/src/main/java/moadong/club/service/ClubApplyService.java |
getClubApplyInfo에서 clubId로 ClubQuestion 조회, 미존재 시 APPLICATION_NOT_FOUND 예외. sortApplicationAnswers(ClubQuestion, ClubApplication) 추가하여 질문 순서대로 답변 재정렬 후 매핑. editApplicantDetail에서 updateMemo 및 updateStatus 각각 호출. 외부 API 시그니처 변경 없음. |
Sequence Diagram(s)
sequenceDiagram
autonumber
participant Client
participant ClubApplyService as Service
participant ClubQuestionRepo as ClubQuestionRepo
participant ClubApplication as Application
Client->>Service: getClubApplyInfo(clubId)
Service->>ClubQuestionRepo: findByClubId(clubId)
alt ClubQuestion 없음
Service-->>Client: throw APPLICATION_NOT_FOUND
else ClubQuestion 있음
loop 각 지원서
Service->>Application: sortApplicationAnswers(question, app)
note right of Service: 답변을 질문 순서대로 재배치 후<br/>app.updateAnswers(sorted)
Service->>Application: updateAnswers(sortedAnswers)
end
Service-->>Client: ClubApplicantsResult 목록
end
sequenceDiagram
autonumber
participant Client
participant ClubApplyService as Service
participant ClubApplication as Application
Client->>Service: editApplicantDetail(appId, memo, status)
Service->>Application: updateMemo(memo)
Service->>Application: updateStatus(status)
Service-->>Client: OK
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- [feature] 지원서 제출 상태 변경 API 추가 #621: editApplicantDetail API와 ClubApplication.updateDetail 사용 추가—이번 PR에서 해당 메서드 제거 및 대체 호출로 변경된 부분과 직접 겹침.
- [feautre] 지원서 관리 API #622: ClubApplication mutator 리팩터링 관련—updateDetail 제거와 개별 mutator 추가가 동일 주제를 다룸.
- [refactor] be 리팩토링 #673: ClubApplyService.getClubApplyInfo 변경과 겹침—이번 PR의 답변 정렬/질문 조회 로직이 동일 메서드에 추가됨.
Suggested reviewers
- Zepelown
- seongwon030
- oesnuj
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
fix/sort-application-answer
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 77ad972. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
backend/src/main/java/moadong/club/service/ClubApplyService.java (3)
102-104: 변수명 혼동: ClubQuestion을 clubApplication 변수명으로 사용가독성/유지보수 측면에서 혼동을 유발합니다. 동일 메서드 내에서 ClubApplication도 함께 다루고 있어 변수명 충돌 위험이 큽니다. 아래처럼 clubQuestion으로 명확히 변경하는 것을 제안합니다.
- ClubQuestion clubApplication = clubQuestionRepository.findByClubId(clubId) + ClubQuestion clubQuestion = clubQuestionRepository.findByClubId(clubId) .orElseThrow(() -> new RestApiException(ErrorCode.APPLICATION_NOT_FOUND));- ClubApplication sortedApp = sortApplicationAnswers(clubApplication, app); + ClubApplication sortedApp = sortApplicationAnswers(clubQuestion, app); applications.add(ClubApplicantsResult.of(sortedApp, cipher));Also applies to: 112-114
112-114: 정렬 결과를 응답에만 반영하는 접근은 OKDB 저장 없이 응답 직전에만 정렬해 FE 요구사항(질문 순서 보장)을 만족시키는 선택은 합리적입니다. 루프 내부에서 status 참조는 정렬과 무관하므로 기능적으로 문제 없습니다. 다만 일관성을 위해 switch도 sortedApp을 사용하면 읽기 쉬워집니다.
131-143: 정렬 유틸의 NPE/중복 키 방어 및 폴백 처리 보강 제안현재 구현은 app.getAnswers()가 null이면 NPE, 중복 answer id가 존재하면 IllegalStateException이 발생할 수 있습니다. 또한 폼 수정 등으로 질문 목록에 없는 기존 답변은 모두 drop됩니다. 아래처럼 방어 로직과 “질문 외 답변은 꼬리에 보존” 폴백을 제안합니다.
- private ClubApplication sortApplicationAnswers(ClubQuestion application, ClubApplication app) { - Map<Long, ClubQuestionAnswer> answerMap = app.getAnswers().stream() - .collect(Collectors.toMap(ClubQuestionAnswer::getId, answer -> answer)); - - List<ClubQuestionAnswer> sortedAnswers = application.getQuestions().stream() - .map(question -> answerMap.get(question.getId())) - .filter(Objects::nonNull) - .collect(Collectors.toList()); - - app.updateAnswers(sortedAnswers); - return app; - } + private ClubApplication sortApplicationAnswers(ClubQuestion application, ClubApplication app) { + List<ClubQuestionAnswer> safeAnswers = + Optional.ofNullable(app.getAnswers()).orElseGet(Collections::emptyList); + + Map<Long, ClubQuestionAnswer> answerMap = safeAnswers.stream() + .collect(Collectors.toMap(ClubQuestionAnswer::getId, Function.identity(), (a, b) -> a)); + + Set<Long> orderedIds = application.getQuestions().stream() + .map(ClubApplicationQuestion::getId) + .collect(Collectors.toCollection(LinkedHashSet::new)); + + List<ClubQuestionAnswer> sortedAnswers = orderedIds.stream() + .map(answerMap::get) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + + // 폼 변경으로 질문에 매칭되지 않는 기존 답변은 꼬리에 보존 + if (sortedAnswers.size() != safeAnswers.size()) { + for (ClubQuestionAnswer ans : safeAnswers) { + if (!orderedIds.contains(ans.getId())) { + sortedAnswers.add(ans); + } + } + } + + app.updateAnswers(sortedAnswers); + return app; + }backend/src/main/java/moadong/club/entity/ClubApplication.java (2)
46-49: status null 방어 추가 제안도메인적으로 status가 null이면 일관성 문제가 생깁니다. 간단한 선검증 추가를 권장합니다.
- public void updateStatus(ApplicationStatus status) { - this.status = status; - } + public void updateStatus(ApplicationStatus status) { + if (status == null) { + throw new IllegalArgumentException("status must not be null"); + } + this.status = status; + }
50-53: updateAnswers의 null 안전성 보강 제안현재 인자 answers가 null이면 NPE가 발생합니다. 아래처럼 null을 허용하고 빈 리스트로 취급하면 호출부가 단순해집니다. 기존 리스트를 유지하면서 clear/addAll로 갱신하는 방식은 변경 추적(특히 JPA 케이스)과도 잘 맞습니다.
- public void updateAnswers(List<ClubQuestionAnswer> answers) { - this.answers.clear(); - this.answers.addAll(answers); - } + public void updateAnswers(List<ClubQuestionAnswer> answers) { + this.answers.clear(); + if (answers != null) { + this.answers.addAll(answers); + } + }
📜 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 (2)
backend/src/main/java/moadong/club/entity/ClubApplication.java(1 hunks)backend/src/main/java/moadong/club/service/ClubApplyService.java(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-19T05:45:52.957Z
Learnt from: lepitaaar
PR: Moadong/moadong#406
File: backend/src/main/java/moadong/club/service/ClubApplyService.java:34-38
Timestamp: 2025-05-19T05:45:52.957Z
Learning: The code duplication between createClubApplication and editClubApplication methods in ClubApplyService.java is acknowledged but will be addressed in a future refactoring, as per the developer's plan.
Applied to files:
backend/src/main/java/moadong/club/entity/ClubApplication.javabackend/src/main/java/moadong/club/service/ClubApplyService.java
🔇 Additional comments (3)
backend/src/main/java/moadong/club/service/ClubApplyService.java (2)
307-307: 변경 없음파일 말미 포맷팅 변경으로 보이며 기능 영향 없습니다.
156-158: 레거시updateDetail호출 없음 확인전체 코드베이스에서
updateDetail(호출을 검색한 결과가 없으므로,updateMemo/updateStatus분리 적용이 안전하며 의도한 대로 동작할 것으로 보입니다. 리뷰 승인합니다.backend/src/main/java/moadong/club/entity/ClubApplication.java (1)
42-45: 메모 업데이트 분리: 단일 책임에 부합, 문제 없음단순 세터형 변경으로 사이드이펙트 없고 의도가 명확합니다.
2 participants
#️⃣연관된 이슈
📝작업 내용
지원자의 응답을 지원서의 순서에 맞게 정렬하여 응답해줍니다.
이를통해 프론트에서 이름을 가져오는 부분에 문제가 생기지않도록 보장합니다
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
신규 기능
버그 수정
리팩터링