feat/KD-74: 논문 및 자격증 제출의 개별적 승인 및 반려 API 구현#333
Merged
JangYeongHu merged 4 commits intodevelopfrom Mar 29, 2026
Merged
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:
요약졸업 사용자 관리 모듈에 단일 제출물 승인/반려 기능을 추가합니다. 변경 사항
예상 코드 리뷰 난이도🎯 3 (보통) | ⏱️ ~20분 관련 가능 PR
제안 리뷰어
🚥 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
|
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## develop #333 +/- ##
=============================================
- Coverage 86.97% 85.38% -1.60%
- Complexity 67 79 +12
=============================================
Files 24 24
Lines 261 301 +40
Branches 15 30 +15
=============================================
+ Hits 227 257 +30
- Misses 21 22 +1
- Partials 13 22 +9
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
![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: 2
🧹 Nitpick comments (3)
aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/GraduationUserAdminControllerImpl.java (2)
155-156: 변수명 camelCase 불일치동일하게
disapprovedGraduationuserId도disapprovedGraduationUserId로 수정이 필요합니다.✏️ 네이밍 수정 제안
- Long disapprovedGraduationuserId = graduationUserAdminFacade.disapproveSubmission(graduationUserId,submissionId); - return ResponseEntity.ok(GraduationUserPersistResponse.of(disapprovedGraduationuserId)); + Long disapprovedGraduationUserId = graduationUserAdminFacade.disapproveSubmission(graduationUserId, submissionId); + return ResponseEntity.ok(GraduationUserPersistResponse.of(disapprovedGraduationUserId));🤖 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/GraduationUserAdminControllerImpl.java` around lines 155 - 156, Rename the local variable disapprovedGraduationuserId to disapprovedGraduationUserId to fix the camelCase inconsistency; update the declaration and any subsequent usages (the assignment from graduationUserAdminFacade.disapproveSubmission(graduationUserId, submissionId) and the ResponseEntity return that calls GraduationUserPersistResponse.of(...)) so they reference disapprovedGraduationUserId.
145-146: 변수명 camelCase 불일치
approvedGraduationuserId에서u가 소문자입니다. camelCase 컨벤션에 따라approvedGraduationUserId로 수정하는 것이 좋습니다.✏️ 네이밍 수정 제안
- Long approvedGraduationuserId = graduationUserAdminFacade.approveSubmission(graduationUserId,submissionId); - return ResponseEntity.ok(GraduationUserPersistResponse.of(approvedGraduationuserId)); + Long approvedGraduationUserId = graduationUserAdminFacade.approveSubmission(graduationUserId, submissionId); + return ResponseEntity.ok(GraduationUserPersistResponse.of(approvedGraduationUserId));🤖 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/GraduationUserAdminControllerImpl.java` around lines 145 - 146, The local variable name approvedGraduationuserId in GraduationUserAdminControllerImpl violates camelCase; rename it to approvedGraduationUserId and update any subsequent uses (e.g., the call to GraduationUserPersistResponse.of and any other references) so the variable matches camelCase; keep the call to graduationUserAdminFacade.approveSubmission(graduationUserId, submissionId) intact and only change the variable name.aics-admin/src/testFixtures/java/graduationUser/application/GraduationUserAdminFacadeTest.java (1)
376-438: 소유권 검증 실패 케이스에 대한 테스트 추가 필요happy path 테스트는 잘 작성되었으나, facade에서 플래그된 보안 이슈와 관련하여 다음 케이스에 대한 테스트 추가를 권장합니다:
submissionId가 해당graduationUser에 속하지 않는 경우graduationType이 null인 사용자에 대한 승인 시도소유권 검증 로직이 추가된 후 해당 테스트도 함께 추가해 주세요.
🧪 추가 테스트 케이스 제안
`@Test` `@DisplayName`("approveSubmission은 다른 사용자의 제출물을 승인하려 할 때 예외를 던진다.") public void approveSubmission_InvalidSubmissionId_ThrowsException() { // given Long graduationUserId = 1L; // CERTIFICATE type user Long invalidSubmissionId = 999L; // 존재하지 않거나 다른 사용자의 제출물 // when & then assertThrows(IllegalArgumentException.class, () -> graduationUserAdminFacade.approveSubmission(graduationUserId, invalidSubmissionId) ); } `@Test` `@DisplayName`("approveSubmission은 graduationType이 null인 사용자에 대해 예외를 던진다.") public void approveSubmission_NullGraduationType_ThrowsException() { // given Long graduationUserId = 3L; // graduationType이 null인 사용자 Long submissionId = 1L; // when & then assertThrows(IllegalStateException.class, () -> graduationUserAdminFacade.approveSubmission(graduationUserId, submissionId) ); }🤖 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 376 - 438, Tests only cover happy paths; add negative tests that assert ownership and null-type checks for the facade methods. In the test class GraduationUserAdminFacadeTest add tests for approveSubmission (and similarly disapproveSubmission if desired) that call graduationUserAdminFacade.approveSubmission with a submissionId that does not belong to the given graduationUserId and assert the appropriate exception (e.g., IllegalArgumentException), and another test where the GraduationUser has graduationType == null and calling approveSubmission throws the expected exception (e.g., IllegalStateException); ensure these tests exercise the same fake repositories (fakeCertificateRepository / fakeThesisRepository) and IDs used elsewhere so they trigger the new ownership and null-type validation paths in approveSubmission/disapproveSubmission.
🤖 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/application/GraduationUserAdminFacade.java`:
- Around line 228-246: approveSubmission and disapproveSubmission call
CertificateCommandService/ThesisCommandService with a submissionId without
verifying the submission belongs to the GraduationUser, allowing unauthorized
approval/disapproval; before invoking
certificateCommandService.approve/disapprove or
thesisCommandService.approve/disapprove in GraduationUserAdminFacade, load the
target submission (e.g., via a CertificateQueryService.getById or
ThesisQueryService.getById or a submission lookup method), verify its
owner/graduationUserId equals the fetched GraduationUser.id, and if not throw an
appropriate access/validation exception; alternatively, change the command
methods to accept and verify the graduationUserId internally (update
CertificateCommandService.approve/disapprove and
ThesisCommandService.approve/disapprove to perform ownership checks) and ensure
both approveSubmission and disapproveSubmission perform or rely on that
ownership validation before returning graduationUserId.
- Around line 230-234: The current logic in GraduationUserAdminFacade (where
graduationUser.getGraduationType() is inspected to call
certificateCommandService.approve(submissionId) or
thesisCommandService.approve(submissionId)) ignores the case when graduationType
is null; add an explicit null check on graduationUser.getGraduationType() before
the existing if/else and throw a clear runtime exception (e.g.,
IllegalStateException or a domain-specific exception) that includes the
graduationUser id and submissionId, so callers get a failing response instead of
silent success.
---
Nitpick comments:
In
`@aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/GraduationUserAdminControllerImpl.java`:
- Around line 155-156: Rename the local variable disapprovedGraduationuserId to
disapprovedGraduationUserId to fix the camelCase inconsistency; update the
declaration and any subsequent usages (the assignment from
graduationUserAdminFacade.disapproveSubmission(graduationUserId, submissionId)
and the ResponseEntity return that calls GraduationUserPersistResponse.of(...))
so they reference disapprovedGraduationUserId.
- Around line 145-146: The local variable name approvedGraduationuserId in
GraduationUserAdminControllerImpl violates camelCase; rename it to
approvedGraduationUserId and update any subsequent uses (e.g., the call to
GraduationUserPersistResponse.of and any other references) so the variable
matches camelCase; keep the call to
graduationUserAdminFacade.approveSubmission(graduationUserId, submissionId)
intact and only change the variable name.
In
`@aics-admin/src/testFixtures/java/graduationUser/application/GraduationUserAdminFacadeTest.java`:
- Around line 376-438: Tests only cover happy paths; add negative tests that
assert ownership and null-type checks for the facade methods. In the test class
GraduationUserAdminFacadeTest add tests for approveSubmission (and similarly
disapproveSubmission if desired) that call
graduationUserAdminFacade.approveSubmission with a submissionId that does not
belong to the given graduationUserId and assert the appropriate exception (e.g.,
IllegalArgumentException), and another test where the GraduationUser has
graduationType == null and calling approveSubmission throws the expected
exception (e.g., IllegalStateException); ensure these tests exercise the same
fake repositories (fakeCertificateRepository / fakeThesisRepository) and IDs
used elsewhere so they trigger the new ownership and null-type validation paths
in approveSubmission/disapproveSubmission.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7cb48ebf-9687-4484-a02a-b9ff9d492cf6
📒 Files selected for processing (4)
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/testFixtures/java/graduationUser/application/GraduationUserAdminFacadeTest.java
...src/main/java/kgu/developers/admin/graduationUser/application/GraduationUserAdminFacade.java
Show resolved
Hide resolved
...src/main/java/kgu/developers/admin/graduationUser/application/GraduationUserAdminFacade.java
Show resolved
Hide resolved
Contributor
|
LGTM! 수고 많으셨습니다! |
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
논문 및 자격증 제출의 개별적 승인 및 반려 API를 구현하였습니다.
Tasks