Merged
Conversation
[hotfix] 지원자 응답 순서 보장
lepitaaar
requested review from
PororoAndFriends,
Zepelown,
seongwon030 and
suhyun113
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
엔티티 mutator 분리backend/src/main/java/moadong/club/entity/ClubApplication.java |
updateDetail(memo,status) 제거. 새 메서드 updateMemo(String), updateStatus(ApplicationStatus), updateAnswers(List<ClubQuestionAnswer>) 추가(내부 리스트 교체: clear 후 addAll). |
서비스 조회/수정 로직 조정backend/src/main/java/moadong/club/service/ClubApplyService.java |
getClubApplyInfo에서 답변을 질문 순서대로 정렬하는 sortApplicationAnswers(ClubQuestion, ClubApplication) 추가 및 적용. editApplicantDetail에서 updateDetail 단일 호출을 updateMemo와 updateStatus 개별 호출로 대체. |
Sequence Diagram(s)
sequenceDiagram
autonumber
participant C as Client
participant S as ClubApplyService
participant QR as ClubQuestionRepository
participant AR as ClubApplicationRepository
participant A as ClubApplication
C->>S: getClubApplyInfo(clubId)
S->>QR: findByClubId(clubId)
QR-->>S: ClubQuestion (questions[])
S->>AR: findApplications(clubId)
AR-->>S: List<ClubApplication>
loop for each application
S->>S: sortApplicationAnswers(ClubQuestion, application)
note over S: 질문 순서대로 답변 재배열 후 A.updateAnswers(...)
S->>A: updateAnswers(sortedAnswers)
A-->>S: ok
end
S-->>C: ClubApplicantsResult
sequenceDiagram
autonumber
participant C as Client
participant S as ClubApplyService
participant AR as ClubApplicationRepository
participant A as ClubApplication
C->>S: editApplicantDetail(appId, memo, status)
S->>AR: findById(appId)
AR-->>S: ClubApplication
S->>A: updateMemo(memo)
S->>A: updateStatus(status)
S-->>C: ok
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- [hotfix] 지원자 응답 순서 보장 #685: 동일한 ClubApplication mutator 분리와 answer 정렬 로직을 다루며, 본 변경과 직접적으로 동일 영역 수정.
- [feautre] 지원서 관리 API #622: updateDetail 도입/변경 이력과 맞물려 본 PR에서 이를 제거하고 분리 메서드로 대체한 부분과 직접 관련.
- [Release] BE v1.0.6 배포 #675: ClubApplyService의 신청 흐름과 메모/상태 처리 변경을 함께 다루어 코드 레벨에서 연관.
Suggested reviewers
- seongwon030
- Zepelown
- PororoAndFriends
- suhyun113
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
develop/be
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
backend/src/main/java/moadong/club/entity/ClubApplication.java (3)
42-44: 메모 값의 null 안전성 보완 제안요청 DTO에서 memo가 null로 올 가능성이 있다면, 빈 문자열로 정규화해두면 NPE와 불필요한 null 저장을 피할 수 있습니다.
public void updateMemo(String memo) { - this.memo = memo; + this.memo = (memo == null) ? "" : memo; }
46-48: 상태 업데이트 시 null 방지 또는 유효성 검증 권장status가 null로 들어올 여지가 있다면, 무시하거나 예외를 던지는 식으로 방어해두는 것이 안전합니다. 현재 구현은 null이 그대로 저장될 수 있습니다.
public void updateStatus(ApplicationStatus status) { - this.status = status; + if (status == null) { + return; // 또는 throw new IllegalArgumentException("status must not be null"); + } + this.status = status; }
50-53: updateAnswers의 null 안전성 및 방어적 복사
- null이 전달되면 NPE가 발생합니다.
- 외부 리스트 변경의 간섭을 최소화하려면 방어적 복사를 권장합니다.
간단히 null-safe만 적용해도 충분하며, 방어적 복사까지 원하시면 아래 대안을 참고해 주세요.
옵션 A: null-safe (간단)
public void updateAnswers(List<ClubQuestionAnswer> answers) { - this.answers.clear(); - this.answers.addAll(answers); + this.answers.clear(); + if (answers != null) { + this.answers.addAll(answers); + } }옵션 B: 방어적 복사 (리스트 교체 방식)
public void updateAnswers(List<ClubQuestionAnswer> answers) { - this.answers.clear(); - this.answers.addAll(answers); + this.answers = new ArrayList<>( + answers == null ? List.of() : answers + ); }주의: JPA 엔티티였다면 컬렉션 교체는 지양하지만, 본 클래스는 MongoDB(@document)이며 java.util.*를 이미 import하고 있어 교체도 무방합니다.
backend/src/main/java/moadong/club/service/ClubApplyService.java (3)
102-104: 혼동을 줄이는 변수명으로 변경 제안: clubApplication → clubQuestionClubQuestion 타입을 clubApplication 변수명으로 받으면 ClubApplication 엔티티와 혼동될 여지가 큽니다. 가독성 향상을 위해 변수명을 clubQuestion으로 변경하는 것을 권장합니다.
- ClubQuestion clubApplication = clubQuestionRepository.findByClubId(clubId) + ClubQuestion clubQuestion = clubQuestionRepository.findByClubId(clubId) .orElseThrow(() -> new RestApiException(ErrorCode.APPLICATION_NOT_FOUND));
111-114: 정렬 결과의 영속화 의도 확인현재 getClubApplyInfo에서는 정렬된 순서를 메모리 상에서만 사용하고 저장소에는 반영하지 않습니다. 이는 조회 응답 순서만 보장하고, DB 내 순서는 그대로 유지합니다. 이 의도가 맞는지 확인해 주세요. 만약 관리자 화면 외 다른 조회 경로에서도 일관된 순서를 원하신다면, 공용 변환 계층에서 정렬을 적용하거나 저장 시 정렬을 보장하는 전략을 고려할 수 있습니다.
Also applies to: 131-142
102-104: 회귀 방지를 위한 테스트 케이스 제안질문 순서가 바뀌어도 응답이 동일한 순서로 재정렬되는지, 누락/초과 응답이 있어도 안전하게 동작하는지에 대한 테스트를 추가하면 회귀를 막는 데 도움이 됩니다. 원하시면 테스트 스캐폴딩을 준비해드리겠습니다.
Also applies to: 111-114, 131-142
📜 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 (2)
📓 Common learnings
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.
📚 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
⏰ 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). (1)
- GitHub Check: Deploy
🔇 Additional comments (3)
backend/src/main/java/moadong/club/entity/ClubApplication.java (1)
42-48: 필드별 업데이트로의 분리는 좋습니다.기존 단일 업데이트 메서드를 메모/상태로 분리해 의도가 명확해졌습니다. 유지보수성과 테스트 용이성이 좋아졌습니다.
backend/src/main/java/moadong/club/service/ClubApplyService.java (2)
111-114: 응답 정렬 적용은 의도에 부합합니다.정렬 후 DTO 변환에 사용하도록 변경되어, 질문 순서와 응답 순서의 정합성이 확보되었습니다. 성능도 O(n) 맵 구성 + O(m) 순회로 충분히 가볍습니다.
이와 함께 Line 102에서 제안한 변수명 변경을 반영하면 아래도 함께 수정됩니다.
- ClubApplication sortedApp = sortApplicationAnswers(clubApplication, app); + ClubApplication sortedApp = sortApplicationAnswers(clubQuestion, app); applications.add(ClubApplicantsResult.of(sortedApp, cipher));
156-158: 레거시 updateDetail 호출 잔존 없음 확인저장소 전체 검색 결과
updateDetail(...)호출이 발견되지 않아, 기존 레거시 코드는 모두 제거된 상태입니다.
updateMemo()/updateStatus()분리 호출 방식이 명확하므로 이대로 진행하셔도 됩니다.
Comment on lines
+131
to
+142
| 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; | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
NPE 방지 및 중복 키 안전성 개선
- app.getAnswers() 또는 application.getQuestions()가 null이면 NPE가 발생합니다. 기존 데이터 호환성을 고려하면 null-safe가 필요합니다.
- Collectors.toMap은 키가 중복되면 IllegalStateException을 던집니다. 데이터 이상치가 들어와도 안전하게 동작하도록 머지 전략을 지정하는 것을 권장합니다.
- 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 clubQuestion, ClubApplication app) {
+ Map<Long, ClubQuestionAnswer> answerMap = Optional.ofNullable(app.getAnswers())
+ .orElseGet(Collections::emptyList)
+ .stream()
+ .collect(Collectors.toMap(
+ ClubQuestionAnswer::getId,
+ Function.identity(),
+ (a, b) -> a // 중복 키 발생 시 첫 번째 값 유지
+ ));
+
+ List<ClubQuestionAnswer> sortedAnswers = Optional.ofNullable(clubQuestion.getQuestions())
+ .orElseGet(Collections::emptyList)
+ .stream()
+ .map(question -> answerMap.get(question.getId()))
+ .filter(Objects::nonNull)
+ .collect(Collectors.toList());
+
+ app.updateAnswers(sortedAnswers);
+ return app;
+ }📝 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.
Suggested change
| 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 clubQuestion, ClubApplication app) { | |
| Map<Long, ClubQuestionAnswer> answerMap = Optional.ofNullable(app.getAnswers()) | |
| .orElseGet(Collections::emptyList) | |
| .stream() | |
| .collect(Collectors.toMap( | |
| ClubQuestionAnswer::getId, | |
| Function.identity(), | |
| (a, b) -> a // 중복 키 발생 시 첫 번째 값 유지 | |
| )); | |
| List<ClubQuestionAnswer> sortedAnswers = Optional.ofNullable(clubQuestion.getQuestions()) | |
| .orElseGet(Collections::emptyList) | |
| .stream() | |
| .map(question -> answerMap.get(question.getId())) | |
| .filter(Objects::nonNull) | |
| .collect(Collectors.toList()); | |
| app.updateAnswers(sortedAnswers); | |
| return app; | |
| } |
🤖 Prompt for AI Agents
In backend/src/main/java/moadong/club/service/ClubApplyService.java around lines
131 to 142, the method assumes app.getAnswers() and application.getQuestions()
are non-null and uses Collectors.toMap without a merge function; make it
null-safe by treating null answer/question lists as empty lists (e.g., use
Collections.emptyList() or Stream.ofNullable) and build the map with a merge
strategy to avoid IllegalStateException on duplicate keys (e.g., (existing,
incoming) -> existing or incoming). Then map questions to answers using the safe
collections, filter nulls, update the app with the resulting list, and return
it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#️⃣연관된 이슈
📝작업 내용
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit