Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
Controller: bulk delete contractbackend/src/main/java/moadong/club/controller/ClubApplyController.java |
DELETE 엔드포인트를 /apply/{appId}→/applicant로 변경. 메서드 시그니처를 @PathVariable appId→@RequestBody ClubApplicantDeleteRequest로 변경. 문서화(summary/description) 문구 업데이트. 서비스 호출을 새 시그니처에 맞게 수정. 임포트 와일드카드로 정리. |
DTO: request for bulk deletebackend/src/main/java/moadong/club/payload/request/ClubApplicantDeleteRequest.java |
다건 삭제 요청 DTO(레코드) 추가: @NotEmpty List<String> applicantIds. |
Service: bulk delete logicbackend/src/main/java/moadong/club/service/ClubApplyService.java |
deleteApplicant 시그니처를 String appId→ClubApplicantDeleteRequest request로 변경. findAllByIdInAndQuestionId(request.applicantIds(), clubId)로 조회 후 개수 불일치 시 예외(APPLICANT_NOT_FOUND) 발생, deleteAll로 일괄 삭제. |
Repository: finder for bulkbackend/src/main/java/moadong/club/repository/ClubApplicationRepository.java |
새 메서드 추가: List<ClubApplication> findAllByIdInAndQuestionId(List<String> ids, String clubId). |
Sequence Diagram(s)
sequenceDiagram
autonumber
actor Client
participant Controller as ClubApplyController
participant Service as ClubApplyService
participant Repo as ClubApplicationRepository
participant DB
Client->>Controller: DELETE /clubs/{clubId}/applicant\nBody: { applicantIds: [...] }
Controller->>Service: deleteApplicant(clubId, request, user)
Service->>Repo: findAllByIdInAndQuestionId(request.applicantIds, clubId)
Repo->>DB: SELECT * WHERE id IN (...) AND question_id = clubId
DB-->>Repo: applicants[]
Repo-->>Service: applicants[]
alt any missing ids
Service-->>Controller: throw APPLICANT_NOT_FOUND
Controller-->>Client: 404/400
else all found
Service->>Repo: deleteAll(applicants)
Repo->>DB: DELETE ...
DB-->>Repo: ok
Service-->>Controller: void
Controller-->>Client: 204 No Content
end
note over Service,Repo: 검증: 조회 수 == 요청 ID 수
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
| 다건 삭제 지원: 배열로 지원자 id 입력받음 MOA-196 | ✅ |
Assessment against linked issues: Out-of-scope changes
(없음)
Possibly related issues
- 동아리 지원자를 삭제한다 #693: 동일한 지원자 삭제 흐름 변경과 다건 삭제 지원과 직접적으로 연관.
- [feature] MOA-194 동아리 지원자를 삭제한다 #694: 컨트롤러/서비스 삭제 로직 변경으로 다건 삭제 지원과 밀접히 관련.
- [feature] MOA-196 동아리 지원자를 여러명을 삭제할 수 있다 #696: 본 PR과 동일 주제인 다건 지원자 삭제 구현과 일치.
Possibly related PRs
- [feautre] 지원서 관리 API #622: 동일 엔드포인트/서비스의 단건 삭제 구현과 관련된 이전 변경.
- [Release] BE v1.0.5 배포 #626: 같은 삭제 플로우를 다루지만 요청 계약이 단건→본 PR의 다건으로 상이.
Suggested reviewers
- Zepelown
- seongwon030
- oesnuj
- 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
feature/#696-support-multi-delete-application-MOA-196
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 5a6bee1. |
![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 (7)
backend/src/main/java/moadong/club/payload/request/ClubApplicantDeleteRequest.java (1)
7-10: 요소 단위 검증 추가 및 스키마 예시 보강 제안목록 자체에만 @notempty가 적용되어 있어 공백/빈 문자열 ID가 섞여 들어와도 통과합니다. 요소 단위 @notblank, 그리고 대량 요청 방지를 위한 @SiZe 상한을 고려해 주세요. OpenAPI 예시도 함께 추가하면 FE 연동이 수월합니다.
아래와 같이 변경을 제안합니다:
package moadong.club.payload.request; +import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.NotEmpty; +import jakarta.validation.constraints.Size; +import io.swagger.v3.oas.annotations.media.Schema; import java.util.List; public record ClubApplicantDeleteRequest( - @NotEmpty - List<String> applicantIds + @NotEmpty + @Size(max = 1000) + @Schema(description = "삭제할 지원자 ID 목록", example = "[\"app_1\",\"app_2\"]") + List<@NotBlank String> applicantIds ) { }backend/src/main/java/moadong/club/repository/ClubApplicationRepository.java (2)
16-16: 메서드 시그니처 파라미터명 혼동 가능성
findAllByIdInAndQuestionId(List<String> ids, String clubId)에서 두 번째 파라미터명clubId는 실제로는questionId필터에 바인딩됩니다. 팀 내 일관성을 위해 파라미터명을 의미에 맞게 정리해 주세요.-List<ClubApplication> findAllByIdInAndQuestionId(List<String> ids, String clubId); +List<ClubApplication> findAllByIdInAndQuestionId(List<String> ids, String questionId);
10-17: 단일 DB 오퍼레이션으로의 일괄 삭제 메서드 추가 제안현재 서비스에서 전체 엔티티를 읽고 나서
deleteAll(applicants)로 삭제합니다. Mongo에서 파생 쿼리deleteByIdInAndQuestionId(...)를 사용하면 다건 삭제를 단일 오퍼레이션으로 수행해 부분 삭제 가능성을 줄이고 트래픽도 절감합니다. 필요 시 삭제 건수를 반환받아 검증도 가능합니다.리포지토리에 아래 메서드를 추가하는 것을 권장합니다:
public interface ClubApplicationRepository extends MongoRepository<ClubApplication, String> { @@ Optional<ClubApplication> findByIdAndQuestionId(String id, String questionId); List<ClubApplication> findAllByIdInAndQuestionId(List<String> ids, String questionId); + + long deleteByIdInAndQuestionId(List<String> ids, String questionId); + long countByIdInAndQuestionId(List<String> ids, String questionId); }backend/src/main/java/moadong/club/service/ClubApplyService.java (1)
159-175: 일괄 삭제를 단일 쿼리로 수행하고 사전 검증은 카운트로 대체엔티티 전체를 읽지 않고 카운트 후 삭제하면 네트워크/메모리 비용이 줄고, Mongo 단일 오퍼레이션으로 부분 실패 가능성도 낮아집니다. 위 리포지토리 보강을 전제로 다음과 같이 개선을 제안합니다.
- @Transactional - public void deleteApplicant(String clubId, ClubApplicantDeleteRequest request, CustomUserDetails user) { + @Transactional + public void deleteApplicant(String clubId, ClubApplicantDeleteRequest request, CustomUserDetails user) { Club club = clubRepository.findById(clubId) .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)); @@ - List<ClubApplication> applicants = clubApplicationRepository.findAllByIdInAndQuestionId(request.applicantIds(), clubId); - - if (applicants.size() != request.applicantIds().size()) { - throw new RestApiException(ErrorCode.APPLICANT_NOT_FOUND); - } - - clubApplicationRepository.deleteAll(applicants); + List<String> uniqueIds = request.applicantIds().stream().distinct().toList(); + long found = clubApplicationRepository.countByIdInAndQuestionId(uniqueIds, clubId); + if (found != uniqueIds.size()) { + throw new RestApiException(ErrorCode.APPLICANT_NOT_FOUND); + } + clubApplicationRepository.deleteByIdInAndQuestionId(uniqueIds, clubId); }추가로, 대량 요청으로
$in조건이 비대해질 수 있으므로 상한(예: 1000건)을 DTO/서비스 양측에서 함께 제한하는 것을 권장합니다.backend/src/main/java/moadong/club/controller/ClubApplyController.java (3)
85-96: DELETE 요청 바디 사용의 호환성 및 문서/메시지 정교화
- 일부 클라이언트/프록시는 DELETE 바디를 제한할 수 있습니다. 인프라/게이트웨이에서 DELETE+JSON 바디가 문제없는지 확인해 주세요. 대안으로
POST /applicants:batch-delete같은 커맨드 엔드포인트도 고려할 수 있습니다.- 엔드포인트가 다건 삭제를 수행하므로, 요약/응답 메시지를 복수형으로 명확히 표현하면 좋습니다.
-@DeleteMapping("/applicant") -@Operation(summary = "지원자 삭제", - description = "클럽 지원자의 지원서를 삭제합니다" +@DeleteMapping("/applicants") +@Operation(summary = "지원자 여러 명 삭제", + description = "요청한 지원자들의 지원서를 일괄 삭제합니다" ) @@ - return Response.ok("success delete applicant"); + return Response.ok("success delete applicants");
7-7: 와일드카드 임포트 대신 명시 임포트 유지 권장(팀 컨벤션에 따름)리뷰 난이도와 변경 영향 추적을 위해 DTO 임포트는 명시적으로 두는 편을 권장합니다. 팀 규칙에 따라 결정해 주세요.
70-74: API 문구 정합성 확인
@Operation설명에 여전히appId안내 문구가 남아 있습니다(수정 엔드포인트). 실제로는 PathVariable 그대로 사용 중이므로 현재는 문제 없지만, 삭제 엔드포인트와 혼동되지 않도록 문구를 재점검해 주세요.
📜 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 (4)
backend/src/main/java/moadong/club/controller/ClubApplyController.java(3 hunks)backend/src/main/java/moadong/club/payload/request/ClubApplicantDeleteRequest.java(1 hunks)backend/src/main/java/moadong/club/repository/ClubApplicationRepository.java(1 hunks)backend/src/main/java/moadong/club/service/ClubApplyService.java(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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
📚 Learning: 2025-05-15T12:03:57.356Z
Learnt from: Zepelown
PR: Moadong/moadong#406
File: backend/src/main/java/moadong/club/entity/ClubApplicationQuestion.java:32-33
Timestamp: 2025-05-15T12:03:57.356Z
Learning: 엔티티 클래스는 요청/응답 객체(DTO)에 의존해서는 안 됩니다. 계층 간 의존성 문제를 방지하기 위해 엔티티와 DTO는 분리되어야 합니다. 예를 들어, `ClubApplicationQuestion` 엔티티가 `ClubApplicationRequest.Options`와 같은 요청 객체를 직접 참조하는 대신, 엔티티 패키지 내에 `QuestionOptions`와 같은 별도의 클래스를 정의하고 사용해야 합니다. 이렇게 하면 요청 객체 변경이 데이터베이스 스키마나 엔티티 계층에 영향을 미치지 않습니다.
Applied to files:
backend/src/main/java/moadong/club/service/ClubApplyService.java
🔇 Additional comments (2)
backend/src/main/java/moadong/club/payload/request/ClubApplicantDeleteRequest.java (1)
7-10: LGTM — 목적에 맞는 간결한 DTO 도입다건 삭제 요구사항에 맞는 레코드 DTO 추가는 방향성 좋습니다.
backend/src/main/java/moadong/club/service/ClubApplyService.java (1)
159-175: 요구사항 확인: 일부 유효/일부 무효 ID 혼합 시 동작 정책현재 구현은 하나라도 존재하지 않으면 전체 삭제를 중단하고 404를 반환합니다. 부분 삭제 허용 여부가 명확하지 않습니다. FE/기획과 정책을 합의해 주세요. 부분 삭제 허용 시에는 성공/실패 ID 목록을 응답 바디에 포함하는 패턴이 일반적입니다.
Comment on lines
+168
to
175
| List<ClubApplication> applicants = clubApplicationRepository.findAllByIdInAndQuestionId(request.applicantIds(), clubId); | ||
|
|
||
| if (applicants.size() != request.applicantIds().size()) { | ||
| throw new RestApiException(ErrorCode.APPLICANT_NOT_FOUND); | ||
| } | ||
|
|
||
| clubApplicationRepository.delete(application); | ||
| clubApplicationRepository.deleteAll(applicants); | ||
| } |
Contributor
There was a problem hiding this comment.
중복 ID 요청 시 오탐 404 발생 — 존재 확인 로직의 크리티컬 버그
applicants.size()와 request.applicantIds().size()를 직접 비교하면, 요청에 중복 ID가 포함될 때 실제 전부 존재해도 사이즈가 달라져 APPLICANT_NOT_FOUND가 잘못 발생합니다. 중복 제거 후 비교가 필요합니다.
최소 수정안:
- List<ClubApplication> applicants = clubApplicationRepository.findAllByIdInAndQuestionId(request.applicantIds(), clubId);
-
- if (applicants.size() != request.applicantIds().size()) {
+ List<String> uniqueIds = request.applicantIds().stream().distinct().toList();
+ List<ClubApplication> applicants = clubApplicationRepository.findAllByIdInAndQuestionId(uniqueIds, clubId);
+
+ if (applicants.size() != uniqueIds.size()) {
throw new RestApiException(ErrorCode.APPLICANT_NOT_FOUND);
}
clubApplicationRepository.deleteAll(applicants);📝 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
| List<ClubApplication> applicants = clubApplicationRepository.findAllByIdInAndQuestionId(request.applicantIds(), clubId); | |
| if (applicants.size() != request.applicantIds().size()) { | |
| throw new RestApiException(ErrorCode.APPLICANT_NOT_FOUND); | |
| } | |
| clubApplicationRepository.delete(application); | |
| clubApplicationRepository.deleteAll(applicants); | |
| } | |
| // Deduplicate incoming applicant IDs to avoid false negatives when duplicates are present | |
| List<String> uniqueIds = request.applicantIds().stream() | |
| .distinct() | |
| .toList(); | |
| List<ClubApplication> applicants = clubApplicationRepository | |
| .findAllByIdInAndQuestionId(uniqueIds, clubId); | |
| if (applicants.size() != uniqueIds.size()) { | |
| throw new RestApiException(ErrorCode.APPLICANT_NOT_FOUND); | |
| } | |
| clubApplicationRepository.deleteAll(applicants); |
🤖 Prompt for AI Agents
In backend/src/main/java/moadong/club/service/ClubApplyService.java around lines
168 to 175, the existence check wrongly compares applicants.size() to
request.applicantIds().size(), causing false 404s when the request contains
duplicate IDs; fix by deduplicating the requested IDs before querying and
comparing: compute a collection of distinct applicant IDs (e.g.,
stream().distinct() or a Set), use that distinct collection in the repository
call, and then compare applicants.size() against the distinct collection size
(or validate all distinct IDs were found) before deleting.
Zepelown
approved these changes
Aug 24, 2025
Contributor
Author
698은 프론트입니당 요건 백엔드 수정 |
This was referenced Aug 25, 2025
lepitaaar
deleted the
feature/#696-support-multi-delete-application-MOA-196
branch
Merged
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
2 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.
#️⃣연관된 이슈
#696
📝작업 내용
요청 dto를 배열로 받게 변경되었습니다.
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit