[refactor] 지원서를 여러개 수정할 수 있다. -BE#703
Conversation
lepitaaar
requested review from
PororoAndFriends,
Zepelown,
oesnuj,
seongwon030 and
suhyun113
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
컨트롤러: 다건 수신으로 변경backend/src/main/java/moadong/club/controller/ClubApplyController.java |
@PutMapping 경로를 /applicant로 변경하고 메서드 시그니처에서 @PathVariable appId 제거, 대신 @RequestBody @Valid @NotEmpty List<ClubApplicantEditRequest>를 수신하도록 변경. 서비스 호출에 리스트 전달 및 Operation 설명 업데이트. |
요청 DTO: applicantId 추가backend/src/main/java/moadong/club/payload/request/ClubApplicantEditRequest.java |
레코드에 @NotBlank String applicantId 추가(컴포넌트 순서: applicantId, memo, status). 유효성 어노테이션 및 생성자/구성 변경. |
서비스: 단건 → 배치 처리 로직backend/src/main/java/moadong/club/service/ClubApplyService.java |
시그니처를 List<ClubApplicantEditRequest>로 변경. 요청을 applicantId 맵으로 변환·ids 추출, findAllByIdInAndQuestionId로 일괄 조회, 조회 수 불일치 시 APPLICANT_NOT_FOUND 예외, 각 엔티티별 memo/status 적용 후 saveAll 호출로 변경. |
Sequence Diagram(s)
sequenceDiagram
autonumber
participant C as 클라이언트
participant Ctrl as ClubApplyController
participant Svc as ClubApplyService
participant Repo as ClubApplicationRepository
C->>Ctrl: PUT /api/club/{clubId}/applicant\nBody: List<ClubApplicantEditRequest>
Ctrl->>Svc: editApplicantDetail(clubId, List<EditRequest>, user)
Svc->>Svc: 클럽 소유자 검증\n요청 → applicantId 맵·ids 생성
Svc->>Repo: findAllByIdInAndQuestionId(ids, questionId)
Repo-->>Svc: List<ClubApplication>
alt 조회 수 != 요청 수
Svc-->>Ctrl: throw APPLICANT_NOT_FOUND
Ctrl-->>C: 4xx 에러
else 모두 존재
Svc->>Svc: 각 엔티티에 memo/status 적용
Svc->>Repo: saveAll(applications)
Repo-->>Svc: 저장 완료
Svc-->>Ctrl: 완료
Ctrl-->>C: 200 OK
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
| 요청 dto 변경 (MOA-200) | ✅ | |
| 상태변경 로직 변경 — 다건 처리 (MOA-200) | ✅ |
Possibly related issues
- [feature] MOA-199 여러명의 지원서를 변경할 수 있다 #701 — 다건 지원자 일괄 수정(요청 DTO와 서비스 배치 처리)와 목적 일치.
- [refactor] MOA-200 상태변경할 지원자를 여러명 받을 수 있다. #702 — DTO 확장(applicantId) 및 다건 상태 변경 처리 요구와 연관됨.
Possibly related PRs
- [feature] 지원서의 메모를 수정할 수 있다. #620 — 기존 단건 메모 편집 기능을 도입한 PR과 동일 클래스/경로를 변경하므로 연관성 있음.
- [Release] BE v1.0.5 배포 #626 — 단건 편집을 배치로 확장하는 변경과 직접적인 코드 충돌 가능성이 있음.
- [hotfix] 지원자 응답 순서 보장 #686 — applicant 업데이트 흐름(메모/상태 처리 분리) 관련 변경과 로직적 연관성 있음.
Suggested labels
🔨 Refactor
Suggested reviewers
- seongwon030
- oesnuj
- 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
refactor/#702-support-multiple-applicant-MOA-200
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.
Test Results75 tests 75 ✅ 2s ⏱️ Results for commit e95a6c0. ♻️ This comment has been updated with latest results. |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/main/java/moadong/club/controller/ClubApplyController.java (1)
72-83: 경로 스펙과 시그니처 불일치: {appId} 제거 필요@PutMapping("/apply/{appId}")인데 메서드 파라미터에 @PathVariable appId가 없습니다. 실행은 되더라도 API 스펙/Swagger 설명(라인 73-76)과 실제 동작이 어긋나 혼선을 유발합니다. 배치 업데이트로 전환된 목적에 맞게 경로를 "/apply"로 정정하고 문서에서 appId 관련 설명을 제거해 주세요.
권장 diff:
- @PutMapping("/apply/{appId}") + @PutMapping("/apply") @Operation(summary = "지원자의 지원서 정보 변경", - description = "클럽 지원자의 지원서 정보를 수정합니다.<br>" - + "appId - 지원서 아이디" + description = "여러 지원자의 지원서 정보를 일괄 수정합니다.<br>" + + "본문에 각 항목의 applicantId(또는 applicationId), memo, status를 전달하세요." )
🧹 Nitpick comments (4)
backend/src/main/java/moadong/club/payload/request/ClubApplicantEditRequest.java (1)
9-11: 도메인 네이밍 불일치: applicantId → applicationId로 변경 권고서비스에서 실제로 매핑하는 값은 ClubApplication의 식별자(app.getId())입니다. “지원자(applicant)”가 아닌 “지원서(application)”의 ID이므로 필드명을 applicationId로 두는 편이 혼동을 줄이고 API 문서/프론트 연동 시 오해를 방지합니다. 레코드 컴포넌트 이름 변경은 역직렬화에 영향이 있으므로 클라이언트 변경과 함께 진행해 주세요.
예시 diff:
- String applicantId, + String applicationId,서비스 측 참조 변경 예:
- .collect(Collectors.toMap(ClubApplicantEditRequest::applicantId, Function.identity())); + .collect(Collectors.toMap(ClubApplicantEditRequest::applicationId, Function.identity()));backend/src/main/java/moadong/club/controller/ClubApplyController.java (1)
73-76: Swagger 설명 업데이트 필요설명에 여전히 “appId - 지원서 아이디”가 남아 있습니다. 배치 업데이트 스펙에 맞게 본문(JSON 배열) 구조와 필드 설명(applicantId/applicationId, memo, status)을 반영해 주세요.
backend/src/main/java/moadong/club/service/ClubApplyService.java (2)
153-167: 가독성/의미 개선: 복수형 변수명 및 후속 참조 정리List를 단수형 변수명(application)으로 사용하고 있어 읽기 어려움이 있습니다. 복수형으로 정리하고 전반 참조를 맞추면 이해가 쉬워집니다.
권장 diff:
- List<String> applicationIds = new ArrayList<>(requestMap.keySet()); - List<ClubApplication> application = clubApplicationRepository.findAllByIdInAndQuestionId(applicationIds, clubId); + List<String> applicationIds = new ArrayList<>(requestMap.keySet()); + List<ClubApplication> applications = clubApplicationRepository.findAllByIdInAndQuestionId(applicationIds, clubId); - if (application.size() != applicationIds.size()) { + if (applications.size() != applicationIds.size()) { throw new RestApiException(ErrorCode.APPLICANT_NOT_FOUND); } - application.forEach(app -> { + applications.forEach(app -> { ClubApplicantEditRequest _request = requestMap.get(app.getId()); app.updateMemo(_request.memo()); app.updateStatus(_request.status()); }); - clubApplicationRepository.saveAll(application); + clubApplicationRepository.saveAll(applications);
150-156: 빈 요청 처리 정책 명시 권장요청 리스트가 비어있는 경우 현재는 조회/저장에 불필요한 왕복만 수행합니다. 빈 요청은 204로 무시하거나 400으로 거절하는 정책을 명확히 하는 편이 좋습니다.
예시 diff(조용히 무시):
+ if (request == null || request.isEmpty()) { + return; + }또는 검증 계층(컨트롤러)에서 @notempty로 차단하는 방법(권장)을 병행하세요.
📜 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/controller/ClubApplyController.java(2 hunks)backend/src/main/java/moadong/club/payload/request/ClubApplicantEditRequest.java(1 hunks)backend/src/main/java/moadong/club/service/ClubApplyService.java(1 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/service/ClubApplyService.javabackend/src/main/java/moadong/club/controller/ClubApplyController.java
🔇 Additional comments (5)
backend/src/main/java/moadong/club/payload/request/ClubApplicantEditRequest.java (1)
3-3: 검증 추가는 적절합니다@jakarta.validation.constraints.NotBlank 도입으로 ID 필드에 대한 기본 유효성 검증이 갖춰졌습니다. 이 변경 자체는 타당합니다.
backend/src/main/java/moadong/club/controller/ClubApplyController.java (2)
17-18: LGTM: List import 추가 적절배치 요청 수신을 위한 java.util.List import 추가는 타당합니다.
55-61: 호환성 고려 필요:/apply/{appId}PUT 요청 본문 타입이 단건→다건으로 변경되었습니다
ClubApplyController.editApplicantDetail메서드의 매핑(/apply/{appId})이
@RequestBody 타입ClubApplicantEditRequest→List<ClubApplicantEditRequest>로 바뀌면서
기존 클라이언트에서 단건 객체를 보내던 호출이 실패할 수 있습니다.필요 점검 사항:
- 클라이언트 릴리즈/PR 일정과 서버 배포 시점을 반드시 동기화할 것
- 테스트 및 API 문서(스웨거 등)에 반영된 요청 예시 확인
- 레거시 지원을 위해 단건용 엔드포인트 유지 또는 Deprecated 처리 방안 검토
호환성 점검 스크립트 예시:
# 해당 매핑이 하나만 존재하는지 확인 rg -nP '@PutMapping\("/apply/\{appId\}"' -n 1 backend/src/main/java/moadong/club/controller/ClubApplyController.java # 변경된 요청 타입 확인 rg -nP 'List<ClubApplicantEditRequest>' -C1 backend/src/main/java/moadong/club/controller/ClubApplyController.java # 레거시 사용처(테스트/문서) 탐색 rg -nP '/apply/\{appId\}' -C2backend/src/main/java/moadong/club/service/ClubApplyService.java (2)
142-147: 배치 전환 및 @transactional 적용 적절메서드 시그니처를 배치 처리에 맞게 수정했고 트랜잭션 적용도 적절합니다.
156-159: 부분 일치 검증 로직은 적절합니다요청 ID 수와 실제 조회된 엔티티 수 비교로 누락을 검출하는 방식은 명확합니다.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
backend/src/main/java/moadong/club/controller/ClubApplyController.java (1)
79-81: 컬렉션 원소 검증 미적용 및 빈 배열 허용 — @Valid(원소), @notempty(컬렉션) 추가 필요
@RequestBody @Validated List<...>만으로는 리스트 “각 원소”에 선언된 Bean Validation이 동작하지 않습니다. 또한 빈 리스트가 들어오면 서비스 단에서 무의미한 호출이 발생합니다. 컨테이너 원소에@Valid를, 컬렉션에는@NotEmpty를 추가해 주세요. (검증 그룹을 쓰고 있으면@Validated는 유지해도 됩니다.)적용 제안(diff):
- public ResponseEntity<?> editApplicantDetail(@PathVariable String clubId, - @RequestBody @Validated List<ClubApplicantEditRequest> request, + public ResponseEntity<?> editApplicantDetail(@PathVariable String clubId, + @RequestBody @Validated @NotEmpty List<@Valid ClubApplicantEditRequest> request, @CurrentUser CustomUserDetails user) {추가 import:
import java.util.List; +import jakarta.validation.Valid; +import jakarta.validation.constraints.NotEmpty;참고: 대량 입력을 제한하려면
@Size(max = 100)등도 고려해 주세요.
🧹 Nitpick comments (3)
backend/src/main/java/moadong/club/controller/ClubApplyController.java (3)
72-72: 배치 업데이트 URI/HTTP 메서드 컨벤션 재고 (선택 사항)컬렉션에 대한 부분 수정(batch)이라면
PUT /applicants보다는PATCH /applicants또는 액션성 엔드포인트POST /applicants/batch-update가 더 명확할 수 있습니다. 또한 리소스명은 복수형(applicants)이 일관성 면에서 자연스럽습니다.간단 제안(diff):
- @PutMapping("/applicant") + @PatchMapping("/applicants")
82-83: 배치 처리 결과를 응답 본문으로 반환하면 클라이언트 친화적문자열 고정 응답 대신 처리 건수 등을 돌려주면 프론트에서 바로 사용자 메시지로 활용 가능합니다. 서비스에서 유효성 검증으로 개수 불일치 시 예외를 던진다면
request.size()가 처리 건수와 동일합니다.적용 제안(diff):
clubApplyService.editApplicantDetail(clubId, request, user); - return Response.ok("success edit applicant"); + return Response.ok(java.util.Map.of("updatedCount", request.size()));
79-81: 대량 요청 방어적 제한(선택) — 과도한 payload 방지운영 안정성을 위해 리스트 크기에 상한을 두는 것을 권장합니다. 예:
@Size(min = 1, max = 100).적용 예시(diff):
- @RequestBody @Validated @NotEmpty List<@Valid ClubApplicantEditRequest> request, + @RequestBody @Validated @NotEmpty @jakarta.validation.constraints.Size(min = 1, max = 100) + List<@Valid ClubApplicantEditRequest> request,
📜 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/controller/ClubApplyController.java(2 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/controller/ClubApplyController.java
🔇 Additional comments (1)
backend/src/main/java/moadong/club/controller/ClubApplyController.java (1)
17-18: 필요한 컬렉션 import 추가 — OK
List사용을 위한 import 추가는 적절합니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
backend/src/main/java/moadong/club/controller/ClubApplyController.java (4)
6-7: 배치 요청 유효성 검증 import 추가는 적절합니다 (+ 상한 검증을 위해 @SiZe 도입 제안)
@Valid,@NotEmpty도입으로 컬렉션 유효성 검증 준비가 되었습니다. 대량 요청 방지/리소스 보호를 위해 최대 크기 상한을 두는@Size(max=...)도 함께 권장합니다.아래 import를 함께 추가해 주세요:
import jakarta.validation.Valid; import jakarta.validation.constraints.NotEmpty; +import jakarta.validation.constraints.Size;
82-82: 리스트 원소(@Valid) 검증 캐스케이딩 명시 + 배치 크기 상한 권장메서드 파라미터에 붙은
@Valid만으로도 컬렉션 원소 검증이 캐스케이드되지만, 타입 사용 위치 애노테이션으로 원소에 명시적으로 부여하면 가독성이 좋아집니다. 또한 방어적 프로그래밍 차원에서 요청 크기 상한을 두는 것을 권장합니다(값은 운영 요구에 맞게 조정).- @RequestBody @Valid @NotEmpty List<ClubApplicantEditRequest> request, + @RequestBody @NotEmpty @Size(max = 100) List<@Valid ClubApplicantEditRequest> request,참고: 위 변경에는 상단에
import jakarta.validation.constraints.Size;가 필요합니다(별도 코멘트 참조).
74-78: API 문서에 원자성 명시 필요서비스 로직을 확인해보니, 다음 두 곳에서 요청된 ID 수와 조회된 결과 수가 일치하지 않으면
APPLICANT_NOT_FOUND예외를 던져 전체 처리가 롤백되도록 구현되어 있습니다.
ClubApplyService.java:156–158(수정 로직)ClubApplyService.java:180–182(삭제 로직)이에 따라, API 소비자 입장에서 “요청이 원자적으로 처리된다(하나라도 유효하지 않으면 전체 실패)”는 점을 Swagger 문서에도 명시하시면 좋겠습니다.
필드명(
applicantId)은 DTO와 컨트롤러 문서에서 일관되게 사용되고 있어 추가 검증이 필요하지 않습니다.권장 diff:
@Operation(summary = "지원자의 지원서 정보 변경", - description = "여러 지원자의 지원서 정보를 일괄 수정합니다.<br>" - + "요청 본문은 ClubApplicantEditRequest 객체의 배열이며, 각 원소는 applicantId, memo, status를 포함합니다." + description = "여러 지원자의 지원서 정보를 일괄 수정합니다.<br>" + + "요청 본문은 ClubApplicantEditRequest 객체의 배열이며, 각 원소는 applicantId, memo, status를 포함합니다.<br>" + + "요청은 원자적으로 처리되며, 유효하지 않은 ID가 하나라도 포함되면 전체 요청이 실패(APPLICANT_NOT_FOUND)합니다." )
84-86: 서비스 배치 업데이트 메서드 검증 완료
editApplicantDetail메서드에@Transactional이 적용되어 있어, batch 처리 전체가 하나의 트랜잭션으로 묶여 있습니다.- 기존 단건 로직과 동일하게
user.getId().equals(club.getUserId())권한 검증이 배치 처리에도 일관되게 적용됩니다.clubApplicationRepository.saveAll(...)로 한 번에 저장하며, 일치하지 않는 ID가 있으면ErrorCode.APPLICANT_NOT_FOUND예외를 던져 일관된 에러 핸들링이 이뤄집니다.남은 선택적 제안
- 응답 메시지를 복수형으로 변경하면 batch 처리 의도를 명확히 표현할 수 있습니다.
- return Response.ok("success edit applicant"); + return Response.ok("success edit applicants");
📜 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/controller/ClubApplyController.java(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: lepitaaar
PR: Moadong/moadong#703
File: backend/src/main/java/moadong/club/controller/ClubApplyController.java:84-84
Timestamp: 2025-08-25T14:43:52.276Z
Learning: In the Moadong codebase, questionId and clubId are equivalent identifiers that represent the same entity. The ClubApplicationRepository.findAllByIdInAndQuestionId method correctly uses clubId as the questionId parameter for filtering club applications.
📚 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/controller/ClubApplyController.java
📚 Learning: 2025-08-25T14:43:52.276Z
Learnt from: lepitaaar
PR: Moadong/moadong#703
File: backend/src/main/java/moadong/club/controller/ClubApplyController.java:84-84
Timestamp: 2025-08-25T14:43:52.276Z
Learning: In the Moadong codebase, questionId and clubId are equivalent identifiers that represent the same entity. The ClubApplicationRepository.findAllByIdInAndQuestionId method correctly uses clubId as the questionId parameter for filtering club applications.
Applied to files:
backend/src/main/java/moadong/club/controller/ClubApplyController.java
🔇 Additional comments (1)
backend/src/main/java/moadong/club/controller/ClubApplyController.java (1)
19-20: List 사용을 위한 표준 import 적절합니다별도 이슈 없습니다.
Zepelown
left a comment
Zepelown
left a comment
There was a problem hiding this comment.
고생하셨습니다~
시간되시면 커맨트 달은 거 수정해주시면 감사하겠습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
backend/src/main/java/moadong/club/service/ClubApplyService.java (5)
141-141: 트랜잭션 경계 설정은 적절합니다. 다만 Spring @transactional로의 통일을 검토해주세요.jakarta.transaction.Transactional도 동작은 하지만, Spring의 org.springframework.transaction.annotation.Transactional을 사용하면 readOnly, rollbackFor 등 Spring 고유 옵션을 일관되게 활용할 수 있습니다. 프로젝트 전반의 관례에 맞춰 통일을 고려해 주세요.
150-153: 중복 applicantId가 들어오면 현재는 조용히 덮어씌웁니다. 중복 감지 또는 로깅을 권장합니다.Collectors.toMap에 머지 함수를 추가하셔서 500(IllegalStateException) 위험은 해소되었습니다. 다만 동일 ID가 반복될 경우 마지막 항목으로 “조용히” 덮어씌워져 클라이언트가 실수(예: 중복 전송)를 인지하기 어렵습니다. UX/감사(traceability) 관점에서 중복을 명시적으로 막거나 최소한 로깅하는 편이 좋습니다.
- 권장안 A(중복을 4xx로 처리): 중복 ID를 수집해 예외 반환
+ var dupIds = request.stream() + .collect(Collectors.groupingBy(ClubApplicantEditRequest::applicantId, Collectors.counting())) + .entrySet().stream() + .filter(e -> e.getValue() > 1) + .map(Map.Entry::getKey) + .toList(); + if (!dupIds.isEmpty()) { + // TODO: 프로젝트 규약에 맞는 에러코드로 교체 (예: INVALID_REQUEST) + throw new RestApiException(ErrorCode.INVALID_REQUEST); + } - Map<String, ClubApplicantEditRequest> requestMap = request.stream() - .collect(Collectors.toMap(ClubApplicantEditRequest::applicantId, - Function.identity(), (prev, next) -> next)); + Map<String, ClubApplicantEditRequest> requestMap = request.stream() + .collect(Collectors.toMap(ClubApplicantEditRequest::applicantId, Function.identity()));
- 대안 B(최소 변경): 덮어씌운 사실을 경고 로그로 남김
Map<String, ClubApplicantEditRequest> requestMap = request.stream() .collect(Collectors.toMap( ClubApplicantEditRequest::applicantId, Function.identity(), (prev, next) -> next )); + if (request.size() != requestMap.size()) { + log.warn("Duplicate applicantIds collapsed. requested={}, unique={}", request.size(), requestMap.size()); + }
154-156: 가독성: 복수형 네이밍 정리(application → applications)List 변수명이 단수형이라 후속 사용부 해석이 조금 어렵습니다. 복수형으로 정리하면 읽기 수월합니다.
- List<String> applicationIds = new ArrayList<>(requestMap.keySet()); - List<ClubApplication> application = clubApplicationRepository.findAllByIdInAndQuestionId(applicationIds, clubId); + List<String> applicationIds = new ArrayList<>(requestMap.keySet()); + List<ClubApplication> applications = clubApplicationRepository.findAllByIdInAndQuestionId(applicationIds, clubId); - if (application.size() != applicationIds.size()) { + if (applications.size() != applicationIds.size()) { throw new RestApiException(ErrorCode.APPLICANT_NOT_FOUND); } - application.forEach(app -> { + applications.forEach(app -> { ClubApplicantEditRequest editRequest = requestMap.get(app.getId()); app.updateMemo(editRequest.memo()); app.updateStatus(editRequest.status()); }); - clubApplicationRepository.saveAll(application); + clubApplicationRepository.saveAll(applications);Also applies to: 157-159, 161-165, 167-167
157-159: 누락된 ID를 로깅하면 장애 분석이 빨라집니다사이즈 불일치 시 어떤 ID가 누락되었는지 남겨두면 운영상 이점이 큽니다.
- if (application.size() != applicationIds.size()) { - throw new RestApiException(ErrorCode.APPLICANT_NOT_FOUND); - } + if (applications.size() != applicationIds.size()) { + Set<String> found = applications.stream() + .map(ClubApplication::getId) + .collect(Collectors.toSet()); + List<String> missing = applicationIds.stream() + .filter(id -> !found.contains(id)) + .toList(); + log.warn("APPLICANT_NOT_FOUND: missingIds={}", missing); + throw new RestApiException(ErrorCode.APPLICANT_NOT_FOUND); + }
167-167: saveAll는 선택사항입니다(영속성 컨텍스트 더티체킹으로도 반영 가능)findAll로 가져온 엔티티가 영속 상태라면 트랜잭션 커밋 시 더티체킹으로 플러시됩니다. saveAll 호출을 생략해도 동일하게 반영되므로 일관성 있는 팀 규약에 맞춰 선택하세요. 명시적 저장을 선호하지 않는다면 아래처럼 제거 가능.
- clubApplicationRepository.saveAll(applications); + // JPA dirty checking으로 커밋 시 반영
📜 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/service/ClubApplyService.java(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: lepitaaar
PR: Moadong/moadong#703
File: backend/src/main/java/moadong/club/controller/ClubApplyController.java:84-84
Timestamp: 2025-08-25T14:43:52.276Z
Learning: In the Moadong codebase, questionId and clubId are equivalent identifiers that represent the same entity. The ClubApplicationRepository.findAllByIdInAndQuestionId method correctly uses clubId as the questionId parameter for filtering club applications.
📚 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/service/ClubApplyService.java
📚 Learning: 2025-08-25T14:43:52.276Z
Learnt from: lepitaaar
PR: Moadong/moadong#703
File: backend/src/main/java/moadong/club/controller/ClubApplyController.java:84-84
Timestamp: 2025-08-25T14:43:52.276Z
Learning: In the Moadong codebase, questionId and clubId are equivalent identifiers that represent the same entity. The ClubApplicationRepository.findAllByIdInAndQuestionId method correctly uses clubId as the questionId parameter for filtering club applications.
Applied to files:
backend/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: test
🔇 Additional comments (2)
backend/src/main/java/moadong/club/service/ClubApplyService.java (2)
142-148: 권한 검증 플로우 LGTM클럽 소유자 검증 후 비소유자의 요청을 USER_UNAUTHORIZED로 차단하는 흐름이 명확합니다.
155-155: findAllByIdInAndQuestionId 호출의 clubId 인자가 올바릅니다본 코드베이스에서는 questionId와 clubId가 동일 개체 식별자로 취급됩니다. 따라서 questionId 파라미터에 clubId를 전달하는 것이 맞습니다. (참고: retrieved_learnings)
| application.forEach(app -> { | ||
| ClubApplicantEditRequest editRequest = requestMap.get(app.getId()); | ||
| app.updateMemo(editRequest.memo()); | ||
| app.updateStatus(editRequest.status()); | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
부분 업데이트 허용 여부 확인 필요. null 값이 들어오면 기존 데이터가 지워질 수 있습니다
DTO에서 memo/status가 필수인지 선택인지에 따라 처리 달라집니다. 선택 필드라면 null을 그대로 세터에 전달하면 기존 값이 덮여 지워질 수 있습니다. 아래처럼 null 세이프 업데이트를 고려해 주세요.
- app.updateMemo(editRequest.memo());
- app.updateStatus(editRequest.status());
+ if (editRequest.memo() != null) {
+ app.updateMemo(editRequest.memo());
+ }
+ if (editRequest.status() != null) {
+ app.updateStatus(editRequest.status());
+ }또한 상태 전이 제약(예: SUBMITTED→ACCEPTED는 가능, ACCEPTED→SUBMITTED는 불가 등)이 있다면 여기서 검증하거나 도메인 레벨에서 막아야 합니다.
아래 스크립트로 ClubApplicantEditRequest의 필드 제약(@NotNull/@notblank 등)과 상태 전이 유효성 유무를 확인해 주실 수 있나요?
🏁 Script executed:
#!/bin/bash
# ClubApplicantEditRequest 정의와 필드 제약 확인
rg -nP -C3 --type=java '\b(record|class)\s+ClubApplicantEditRequest\b'
# memo/status의 어노테이션 확인
rg -nP -C2 --type=java 'ClubApplicantEditRequest|memo\(|status\('
# 상태 전이 검증 로직(validator or service) 존재 여부 점검
rg -nP -C3 --type=java '(StatusTransition|canTransit|validate.*Status|Status.*Validator)'Length of output: 6959
ClubApplyService 수정 필요: null 안전 업데이트 및 상태 전이 검증 추가
ClubApplicantEditRequest 레코드에서 memo와 status 필드는 별도 제약 애노테이션이 없어 null 값이 허용됩니다. 즉, null이 전달될 경우 기존 데이터가 의도치 않게 삭제될 수 있습니다(기존 rg 결과 확인)📌
또한, 현재 코드베이스에 상태 전이 가능 여부를 검증하는 로직(Validator나 Transition 유효성 체크)이 존재하지 않습니다📌
따라서 아래와 같이 null 안전 업데이트와 상태 전이 검증을 함께 반영해야 합니다.
- ClubApplyService.editApplicantDetail (backend/src/main/java/moadong/club/service/ClubApplyService.java:161–165)
editRequest.memo()가 null이 아닐 때만app.updateMemo(...)호출editRequest.status()가 null이 아닐 때만app.updateStatus(...)호출app.updateStatus(...)호출 전/후에 도메인 상태 전이 규칙(예: SUBMITTED→ACCEPTED 허용, 그 외 불가)을 검증
예시 diff:
application.forEach(app -> {
- app.updateMemo(editRequest.memo());
- app.updateStatus(editRequest.status());
+ if (editRequest.memo() != null) {
+ app.updateMemo(editRequest.memo());
+ }
+ if (editRequest.status() != null) {
+ // TODO: ApplicationStatus 전이 유효성 검증 로직 호출
+ app.updateStatus(editRequest.status());
+ }
});추가로, 상태 전이 규칙은
ApplicationStatus내부에 메서드로 구현하거나- 별도 Validator/Service 레이어에 분리하여
canTransit(prevStatus, newStatus)형태로 검증하도록 반영해주세요.
📝 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.
| application.forEach(app -> { | |
| ClubApplicantEditRequest editRequest = requestMap.get(app.getId()); | |
| app.updateMemo(editRequest.memo()); | |
| app.updateStatus(editRequest.status()); | |
| }); | |
| application.forEach(app -> { | |
| ClubApplicantEditRequest editRequest = requestMap.get(app.getId()); | |
| if (editRequest.memo() != null) { | |
| app.updateMemo(editRequest.memo()); | |
| } | |
| if (editRequest.status() != null) { | |
| // TODO: ApplicationStatus 전이 유효성 검증 로직 호출 | |
| app.updateStatus(editRequest.status()); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In backend/src/main/java/moadong/club/service/ClubApplyService.java around lines
161–165, the loop unconditionally calls app.updateMemo(...) and
app.updateStatus(...), which can overwrite fields with null and lacks
state-transition validation; change it to only call
app.updateMemo(editRequest.memo()) if editRequest.memo() != null, and only call
app.updateStatus(editRequest.status()) if editRequest.status() != null after
verifying the transition is allowed (e.g., call
ApplicationStatus.canTransit(current, newStatus) or use a dedicated
Validator/Service canTransit(prev,new) that encapsulates allowed transitions
like SUBMITTED→ACCEPTED); implement the chosen canTransit check before invoking
app.updateStatus and throw or skip/update accordingly to preserve null-safety
and enforce domain transition rules.
lepitaaar
deleted the
refactor/#702-support-multiple-applicant-MOA-200
branch
2 participants
#️⃣연관된 이슈
#702
📝작업 내용
list로 응답을 받고, 반복하여 수정하게 변경하였습니다
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
신규 기능
버그 수정