feature/KD-73: 자격증 신청 관리 메뉴와 졸업 논문 관리 메뉴에서 승인된 사용자 승인 취소 기능 추가#330
Merged
feature/KD-73: 자격증 신청 관리 메뉴와 졸업 논문 관리 메뉴에서 승인된 사용자 승인 취소 기능 추가#330
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Walkthrough졸업 사용자의 승인을 취소하는 배치 작업 기능을 추가합니다. 컨트롤러 계층, 파사드, 도메인 모델에 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 분 Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Test Coverage Report
Files
|
![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 (3)
aics-admin/src/testFixtures/java/graduationUser/application/GraduationUserAdminFacadeTest.java (1)
318-339: 테스트 구현이 적절합니다.테스트가 기존
approveGraduationUsers_Success패턴을 따르고 있습니다. 선택적으로 다음 개선을 고려해 볼 수 있습니다:
assertEquals(false, ...)대신assertFalse(...)사용disapproveGraduationUsers반환값인GraduationUserBatchDisapproveResponse에 대한 검증 추가♻️ 선택적 개선 제안
// then - assertEquals(false, fakeThesisRepository.findApprovalByIdAndDeletedAtIsNull(1L).get()); - assertEquals(false, fakeThesisRepository.findApprovalByIdAndDeletedAtIsNull(2L).get()); - assertEquals(false, fakeCertificateRepository.findApprovalByIdAndDeletedAtIsNull(1L).get()); + GraduationUserBatchDisapproveResponse response = graduationUserAdminFacade.disapproveGraduationUsers(request); + + assertNotNull(response); + assertFalse(fakeThesisRepository.findApprovalByIdAndDeletedAtIsNull(1L).get()); + assertFalse(fakeThesisRepository.findApprovalByIdAndDeletedAtIsNull(2L).get()); + assertFalse(fakeCertificateRepository.findApprovalByIdAndDeletedAtIsNull(1L).get());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aics-admin/src/testFixtures/java/graduationUser/application/GraduationUserAdminFacadeTest.java` around lines 318 - 339, Replace the boolean equality assertions with assertFalse for readability and add assertions to verify the returned GraduationUserBatchDisapproveResponse from graduationUserAdminFacade.disapproveGraduationUsers(request) (e.g., check that the response is not null and contains expected ids/count), while keeping the existing repository checks using fakeThesisRepository.findApprovalByIdAndDeletedAtIsNull(1L/2L) and fakeCertificateRepository.findApprovalByIdAndDeletedAtIsNull(1L) to ensure approvals were cleared; update the test method disapproveGraduationUsers_Success accordingly to capture the response and assert its expected fields and use assertFalse for the repository booleans.aics-admin/src/main/java/kgu/developers/admin/graduationUser/application/GraduationUserAdminFacade.java (2)
203-223:approveGraduationUsers와 분기/루프 구조가 거의 동일해 이후 변경 시 드리프트 위험이 큽니다.승인/승인취소 공통 루프를 private helper로 추출하면 로직 일관성 유지가 쉬워집니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aics-admin/src/main/java/kgu/developers/admin/graduationUser/application/GraduationUserAdminFacade.java` around lines 203 - 223, Extract the duplicated approval/disapproval loop in GraduationUserAdminFacade into a private helper (e.g., processGraduationUsers) that iterates the List<GraduationUser> and invokes a passed-in action per user to handle GraduationType.CERTIFICATE and GraduationType.THESIS cases; update both approveGraduationUsers and the current disapproval loop (the code using certificateCommandService.disapprove, thesisCommandService.disapprove, and collecting disapprovedUserIds) to call this helper with appropriate lambdas or method references so the certificateId/midThesisId/finalThesisId checks and result-collection logic are centralized and not duplicated.
197-199: 배치 입력 ID 중복 제거를 먼저 적용하면 불필요한 조회/승인취소 호출을 줄일 수 있습니다.
request.ids()에 중복이 들어오면 동일 사용자에 대해 같은 작업이 반복될 수 있습니다.제안 수정안
List<GraduationUser> users = request.ids().stream() + .distinct() .map(graduationUserQueryService::getById) .toList();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aics-admin/src/main/java/kgu/developers/admin/graduationUser/application/GraduationUserAdminFacade.java` around lines 197 - 199, The code maps request.ids() directly to graduationUserQueryService.getById causing duplicate IDs to result in repeated DB/service calls; update GraduationUserAdminFacade to deduplicate the IDs first (e.g., use request.ids().stream().distinct() or collect into a Set) before mapping to getById so the users List and any subsequent operations (e.g., approval/cancel methods that use users) are only executed once per unique user.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/response/GraduationUserBatchDisapproveResponse.java`:
- Around line 15-18: In GraduationUserBatchDisapproveResponse.from(List<Long>
disapprovedUserIds) avoid storing the incoming list reference directly; create a
defensive copy (e.g., new ArrayList<>(disapprovedUserIds)) and pass that to the
builder for the disapprovedIds field (or wrap it with
Collections.unmodifiableList(...) if immutability is desired) so external
mutations of disapprovedUserIds cannot alter the response object's state.
---
Nitpick comments:
In
`@aics-admin/src/main/java/kgu/developers/admin/graduationUser/application/GraduationUserAdminFacade.java`:
- Around line 203-223: Extract the duplicated approval/disapproval loop in
GraduationUserAdminFacade into a private helper (e.g., processGraduationUsers)
that iterates the List<GraduationUser> and invokes a passed-in action per user
to handle GraduationType.CERTIFICATE and GraduationType.THESIS cases; update
both approveGraduationUsers and the current disapproval loop (the code using
certificateCommandService.disapprove, thesisCommandService.disapprove, and
collecting disapprovedUserIds) to call this helper with appropriate lambdas or
method references so the certificateId/midThesisId/finalThesisId checks and
result-collection logic are centralized and not duplicated.
- Around line 197-199: The code maps request.ids() directly to
graduationUserQueryService.getById causing duplicate IDs to result in repeated
DB/service calls; update GraduationUserAdminFacade to deduplicate the IDs first
(e.g., use request.ids().stream().distinct() or collect into a Set) before
mapping to getById so the users List and any subsequent operations (e.g.,
approval/cancel methods that use users) are only executed once per unique user.
In
`@aics-admin/src/testFixtures/java/graduationUser/application/GraduationUserAdminFacadeTest.java`:
- Around line 318-339: Replace the boolean equality assertions with assertFalse
for readability and add assertions to verify the returned
GraduationUserBatchDisapproveResponse from
graduationUserAdminFacade.disapproveGraduationUsers(request) (e.g., check that
the response is not null and contains expected ids/count), while keeping the
existing repository checks using
fakeThesisRepository.findApprovalByIdAndDeletedAtIsNull(1L/2L) and
fakeCertificateRepository.findApprovalByIdAndDeletedAtIsNull(1L) to ensure
approvals were cleared; update the test method disapproveGraduationUsers_Success
accordingly to capture the response and assert its expected fields and use
assertFalse for the repository booleans.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9974e23f-501e-422f-bfbb-e46d6fba9090
📒 Files selected for processing (10)
aics-admin/src/main/java/kgu/developers/admin/graduationUser/application/GraduationUserAdminFacade.javaaics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/GraduationUserAdminController.javaaics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/GraduationUserAdminControllerImpl.javaaics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/request/GraduationUserBatchDisapproveRequest.javaaics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/response/GraduationUserBatchDisapproveResponse.javaaics-admin/src/testFixtures/java/graduationUser/application/GraduationUserAdminFacadeTest.javaaics-domain/src/main/java/kgu/developers/domain/certificate/application/command/CertificateCommandService.javaaics-domain/src/main/java/kgu/developers/domain/certificate/domain/Certificate.javaaics-domain/src/main/java/kgu/developers/domain/thesis/application/command/ThesisCommandService.javaaics-domain/src/main/java/kgu/developers/domain/thesis/domain/Thesis.java
Comment on lines
+15
to
+18
| public static GraduationUserBatchDisapproveResponse from(List<Long> disapprovedUserIds) { | ||
| return GraduationUserBatchDisapproveResponse.builder() | ||
| .disapprovedIds(disapprovedUserIds) | ||
| .build(); |
Contributor
There was a problem hiding this comment.
from()에서 입력 리스트 참조를 그대로 보관하면 응답 값이 외부 변경에 오염될 수 있습니다.
호출자가 disapprovedUserIds를 이후에 수정하면 응답 객체의 내용도 함께 바뀔 수 있어, 방어적 복사를 권장합니다.
제안 수정안
import java.util.List;
+import java.util.Objects;
@@
public static GraduationUserBatchDisapproveResponse from(List<Long> disapprovedUserIds) {
return GraduationUserBatchDisapproveResponse.builder()
- .disapprovedIds(disapprovedUserIds)
+ .disapprovedIds(List.copyOf(Objects.requireNonNull(disapprovedUserIds)))
.build();
}📝 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
| public static GraduationUserBatchDisapproveResponse from(List<Long> disapprovedUserIds) { | |
| return GraduationUserBatchDisapproveResponse.builder() | |
| .disapprovedIds(disapprovedUserIds) | |
| .build(); | |
| public static GraduationUserBatchDisapproveResponse from(List<Long> disapprovedUserIds) { | |
| return GraduationUserBatchDisapproveResponse.builder() | |
| .disapprovedIds(List.copyOf(Objects.requireNonNull(disapprovedUserIds))) | |
| .build(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/response/GraduationUserBatchDisapproveResponse.java`
around lines 15 - 18, In GraduationUserBatchDisapproveResponse.from(List<Long>
disapprovedUserIds) avoid storing the incoming list reference directly; create a
defensive copy (e.g., new ArrayList<>(disapprovedUserIds)) and pass that to the
builder for the disapprovedIds field (or wrap it with
Collections.unmodifiableList(...) if immutability is desired) so external
mutations of disapprovedUserIds cannot alter the response object's state.
JangYeongHu
approved these changes
Mar 25, 2026
Contributor
JangYeongHu
left a comment
JangYeongHu
left a comment
There was a problem hiding this comment.
LGTM!
이슈만 추가해서 연관 이슈에 달아주세요.
Contributor
|
LGTM~ |
dkdltm221
approved these changes
Mar 25, 2026
6 tasks
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
4 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
자격증 신청 관리 메뉴와 졸업 논문 관리 메뉴에서 승인된 사용자 승인 취소 기능을 추가했습니다.
Tasks