fix/KD-64 논문/자격증 제출 시 유효성 검사 추가 #315
Conversation
JangYeongHu
had a problem deploying
to
coverage
GitHub Actions
Failure
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 🔍 검토 요약Walkthrough이 PR은 사용자 조회 흐름을 리팩토링하여 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
aics-admin/src/testFixtures/java/graduationUser/application/GraduationUserAdminFacadeTest.java (1)
91-105: aics-auth/src/testFixtures/java/auth/application/AuthServiceTest.java의 GraduationUserQueryService 생성자 호출이 불완전합니다.
GraduationUserQueryService생성자의 첫 번째 파라미터인userQueryService가 누락되었습니다. 다른 테스트 파일들과 동일하게 5개의 파라미터를 모두 전달해야 합니다:new GraduationUserQueryService( userQueryService, // <- 누락된 파라미터 new FakeGraduationUserRepository(), new FakeThesisRepository(), new FakeCertificateRepository(), new GraduationUserExcelImpl() )
🤖 Fix all issues with AI agents
In
@aics-api/src/testFixtures/java/graduationUser/application/GraduationUserFacadeTest.java:
- Around line 35-43: The test creates fakeUserRepository and saves Users to it
but constructs UserQueryService with a separate new FakeUserRepository(),
causing inconsistent state; fix by injecting the same fakeUserRepository
instance into the UserQueryService constructor in GraduationUserFacadeTest so
userQueryService uses the identical FakeUserRepository (replace new
FakeUserRepository() with the existing fakeUserRepository variable).
In
@aics-domain/src/main/java/kgu/developers/domain/graduationUser/application/command/GraduationUserCommandService.java:
- Around line 32-33: The current call
graduationUser.validateAccessPermission(graduationUser.getUserId()) incorrectly
validates the entity against its own ID (always true); change the method
signatures so the service method (GraduationUserCommandService.* where this call
occurs) accepts a currentUserId parameter and pass that into
graduationUser.validateAccessPermission(currentUserId) instead of
graduationUser.getUserId(); update all callers of the service to supply the
authenticated user ID (e.g., from the security context or controller layer) so
the GraduationUser.validateAccessPermission(String userId) actually checks the
requesting user’s permission.
- Around line 44-45: The call
graduationUser.validateAccessPermission(graduationUser.getUserId()) is
incorrectly validating the entity against its own ID; change it to validate
against the caller/requesting user ID (the operator) instead. In the
updateGraduationType flow, pass the requester/operator user id (e.g., from the
incoming command object or security context) into validateAccessPermission
rather than graduationUser.getUserId(); if updateGraduationType lacks that
parameter, add an operatorUserId parameter (or fetch the authenticated user id
via SecurityContext) and use that when calling validateAccessPermission to
enforce proper access control.
🧹 Nitpick comments (10)
aics-domain/src/main/java/kgu/developers/domain/certificate/exception/CertificateNotInSubmissionPeriodException.java (1)
1-11: 예외 클래스 구조가 적절합니다.코드가 프로젝트의 기존 예외 패턴을 잘 따르고 있으며, 명확한 명명과 적절한 패키지 구조를 가지고 있습니다.
선택적으로 다음 개선 사항을 고려해볼 수 있습니다:
- serialVersionUID 추가: Serializable 클래스의 모범 사례로 버전 관리를 위해 추가할 수 있습니다.
- 매개변수를 받는 생성자: 추가 컨텍스트(예: 제출 기간 정보)를 전달할 수 있는 생성자를 고려해볼 수 있습니다.
♻️ 선택적 개선 제안
public class CertificateNotInSubmissionPeriodException extends CustomException { + private static final long serialVersionUID = 1L; + public CertificateNotInSubmissionPeriodException() { super(CERTIFICATE_NOT_IN_SUBMISSION_PERIOD_EXCEPTION); } }aics-common/src/main/java/kgu/developers/common/exception/GlobalExceptionHandler.java (1)
3-4: @slf4j 어노테이션이 사용되지 않습니다.코드에서
log필드를 사용하는 로깅 구문이 없습니다. 로깅이 필요하지 않다면@Slf4j어노테이션을 제거하는 것이 좋습니다.♻️ 제안: 사용하지 않는 어노테이션 제거
-import lombok.RequiredArgsConstructor; -import lombok.extern.slf4j.Slf4j; +import lombok.RequiredArgsConstructor; import org.springframework.beans.TypeMismatchException;-@Slf4j @RestControllerAdvice @RequiredArgsConstructor public class GlobalExceptionHandler extends ResponseEntityExceptionHandler {Also applies to: 28-30
aics-domain/src/main/java/kgu/developers/domain/graduationUser/application/command/GraduationUserCommandService.java (1)
50-61: 업데이트 메서드 간 권한 검증 일관성 부재
updateGraduationType과updateGraduationUserEmail에는 권한 검증이 추가되었지만,updateCertificate과updateThesis에는 권한 검증이 없습니다.모든 업데이트 메서드가 동일한 패턴으로 동작하므로, 일관성을 위해
updateCertificate과updateThesis에도validateAccessPermission호출을 추가하시기 바랍니다.aics-domain/src/main/java/kgu/developers/domain/graduationUser/application/query/GraduationUserQueryService.java (1)
114-118:getByUserId()메서드를 재사용하여 코드 중복을 줄일 수 있습니다.
me()메서드 내부 로직이getByUserId()와 동일합니다. 코드 중복을 줄이기 위해 기존 메서드를 재사용하는 것을 권장합니다.♻️ 제안된 수정
public GraduationUser me() { String userId = userQueryService.getMyId(); - return graduationUserRepository.findByUserIdAndDeletedAtIsNull(userId) - .orElseThrow(GraduationUserNotFoundException::new); + return getByUserId(userId); }aics-domain/src/main/java/kgu/developers/domain/thesis/application/command/ThesisCommandService.java (1)
35-35:LocalDateTime.now()사용은 단위 테스트를 어렵게 만듭니다.
LocalDateTime.now()를 직접 호출하면 시간에 의존하는 테스트가 어려워집니다.Clock또는 시간 제공자를 주입받아 사용하면 테스트에서 시간을 제어할 수 있습니다.♻️ Clock 주입 예시
private final Clock clock; public Long submitThesis(MultipartFile file, Long scheduleId) { // ... LocalDateTime referenceTime = LocalDateTime.now(clock); // ... }aics-domain/src/main/java/kgu/developers/domain/certificate/application/command/CertificateCommandService.java (2)
33-46:ThesisCommandService와 코드 구조를 일관되게 유지하는 것을 권장합니다.
ThesisCommandService에서는referenceTime을schedule조회 직후에 선언하지만, 여기서는 졸업 유형 검증 후에 선언합니다. 두 서비스의 검증 흐름을 일관되게 유지하면 코드 가독성과 유지보수성이 향상됩니다.♻️ ThesisCommandService와 일관된 구조
public Long submitCertificate(MultipartFile file, Long scheduleId) { Schedule schedule = scheduleQueryService.getScheduleManagement(scheduleId); + LocalDateTime referenceTime = LocalDateTime.now(); GraduationUser graduationUser = graduationUserQueryService.me(); if(graduationUser.getGraduationType() != GraduationType.CERTIFICATE) { throw new CertificateInvalidGraduationTypeException(); } - LocalDateTime referenceTime = LocalDateTime.now(); - if(!schedule.isInProgress(referenceTime)) { throw new CertificateNotInSubmissionPeriodException(); }
42-42:LocalDateTime.now()사용은 단위 테스트를 어렵게 만듭니다.
ThesisCommandService와 동일하게Clock주입을 고려해 주세요.aics-api/src/main/java/kgu/developers/api/thesis/application/ThesisFacade.java (1)
22-29: GraduationUser 및 Schedule 조회가 불필요하게 중복으로 실행됩니다.
ThesisCommandService.submitThesis()에서 이미graduationUserQueryService.me()(37줄)와scheduleQueryService.getScheduleManagement(scheduleId)(34줄)를 호출하여 유효성 검사를 수행하고 있습니다. Facade에서 동일한 조회를 다시 수행(24줄, 26줄)하면 불필요한 데이터베이스 쿼리가 발생합니다.다음 중 하나의 방식으로 리팩토링을 권장합니다:
submitThesis()가 필요한 정보(GraduationUser,Schedule)를 함께 반환하도록 수정- 검증 로직과 업데이트 로직의 책임 분리 재검토
aics-api/src/testFixtures/java/graduationUser/application/GraduationUserFacadeTest.java (1)
65-69: 테스트 간 간섭 방지를 위해SecurityContextHolder.clearContext()정리 로직을 권장합니다.
해당 클래스에서 SecurityContext를 세팅하므로,@AfterEach에서 clear 해두는 편이 안정적입니다.aics-domain/src/testFixtures/java/graduationUser/application/GraduationUserQueryServiceTest.java (1)
30-37:UserQueryService용FakeUserRepository를 변수로 분리해두는 편이 향후 테스트 안정성에 좋습니다.
지금은new FakeUserRepository()를 바로 주입해도 되지만,GraduationUserQueryService내부에서 사용자 조회 경로가 늘어나면 유저 시딩이 필요해질 수 있어 테스트 픽스처에서 repo를 명시적으로 들고 가는 구성이 더 안전합니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
aics-admin/src/testFixtures/java/graduationUser/application/GraduationUserAdminFacadeTest.javaaics-api/src/main/java/kgu/developers/api/certificate/application/CertificateFacade.javaaics-api/src/main/java/kgu/developers/api/graduationUser/application/GraduationUserFacade.javaaics-api/src/main/java/kgu/developers/api/thesis/application/ThesisFacade.javaaics-api/src/testFixtures/java/graduationUser/application/GraduationUserFacadeTest.javaaics-common/src/main/java/kgu/developers/common/exception/GlobalExceptionHandler.javaaics-domain/src/main/java/kgu/developers/domain/certificate/application/command/CertificateCommandService.javaaics-domain/src/main/java/kgu/developers/domain/certificate/exception/CertificateDomainExceptionCode.javaaics-domain/src/main/java/kgu/developers/domain/certificate/exception/CertificateInvalidGraduationTypeException.javaaics-domain/src/main/java/kgu/developers/domain/certificate/exception/CertificateNotInSubmissionPeriodException.javaaics-domain/src/main/java/kgu/developers/domain/graduationUser/application/command/GraduationUserCommandService.javaaics-domain/src/main/java/kgu/developers/domain/graduationUser/application/query/GraduationUserQueryService.javaaics-domain/src/main/java/kgu/developers/domain/schedule/application/query/ScheduleQueryService.javaaics-domain/src/main/java/kgu/developers/domain/schedule/domain/Schedule.javaaics-domain/src/main/java/kgu/developers/domain/thesis/application/command/ThesisCommandService.javaaics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisDomainExceptionCode.javaaics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisInvalidGraduationTypeException.javaaics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisNotInSubmissionPeriodException.javaaics-domain/src/testFixtures/java/graduationUser/application/GraduationUserQueryServiceTest.java
💤 Files with no reviewable changes (1)
- aics-domain/src/main/java/kgu/developers/domain/schedule/application/query/ScheduleQueryService.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-03T05:01:55.233Z
Learnt from: LeeShinHaeng
Repo: kgu-developers/aics-server PR: 124
File: aics-domain/src/testFixtures/java/mock/FakeLabRepository.java:13-57
Timestamp: 2024-12-03T05:01:55.233Z
Learning: `LabServiceTest.java`에서 Java의 `BeforeEach` 애너테이션을 사용하여 각 테스트 전에 `FakeLabRepository`를 초기화하므로, `FakeLabRepository.java`에 `clear()` 메서드를 추가할 필요가 없다.
Applied to files:
aics-domain/src/testFixtures/java/graduationUser/application/GraduationUserQueryServiceTest.java
🧬 Code graph analysis (2)
aics-domain/src/main/java/kgu/developers/domain/certificate/application/command/CertificateCommandService.java (3)
aics-domain/src/main/java/kgu/developers/domain/thesis/application/command/ThesisCommandService.java (1)
Service(23-63)aics-domain/src/main/java/kgu/developers/domain/graduationUser/application/query/GraduationUserQueryService.java (1)
Service(20-119)aics-domain/src/main/java/kgu/developers/domain/schedule/application/query/ScheduleQueryService.java (1)
Service(15-43)
aics-domain/src/main/java/kgu/developers/domain/graduationUser/application/query/GraduationUserQueryService.java (1)
aics-domain/src/main/java/kgu/developers/domain/graduationUser/exception/GraduationUserNotFoundException.java (1)
GraduationUserNotFoundException(7-11)
🔇 Additional comments (14)
aics-common/src/main/java/kgu/developers/common/exception/GlobalExceptionHandler.java (3)
20-21: 명시적 import 추가 - 좋습니다!
List와Collectors에 대한 명시적 import를 추가하여 코드의 의존성을 명확하게 했습니다.
23-26: static import를 통한 코드 가독성 개선예외 코드와 HTTP 상태 상수에 대해 static import를 사용하여 코드를 더 간결하고 읽기 쉽게 만들었습니다.
30-30: @requiredargsconstructor를 통한 생성자 자동 생성Lombok의
@RequiredArgsConstructor를 사용하여 final 필드에 대한 생성자를 자동으로 생성하도록 리팩토링했습니다. Spring의 의존성 주입과 잘 작동하며 보일러플레이트 코드를 줄입니다.Also applies to: 32-32
aics-domain/src/main/java/kgu/developers/domain/certificate/exception/CertificateDomainExceptionCode.java (1)
8-17: LGTM! 예외 코드 추가가 적절합니다.새로운 검증 로직을 위한 예외 코드들이 올바르게 추가되었습니다. BAD_REQUEST 상태 코드 사용이 적절하며, 에러 메시지도 명확합니다.
aics-domain/src/main/java/kgu/developers/domain/schedule/domain/Schedule.java (1)
45-47: LGTM! 가독성을 높이는 헬퍼 메서드입니다.기존
determineStatusAt()메서드를 활용한 간단하고 명확한 헬퍼 메서드입니다. 호출하는 곳에서 코드 가독성이 향상될 것입니다.aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisNotInSubmissionPeriodException.java (1)
1-11: LGTM! 표준 예외 클래스 패턴을 따릅니다.기존 프로젝트의 예외 처리 패턴과 일관성 있게 구현되었습니다.
CustomException을 확장하고 적절한 예외 코드를 사용하고 있습니다.aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisInvalidGraduationTypeException.java (1)
1-11: LGTM! 표준 예외 클래스 패턴을 따릅니다.기존 프로젝트의 예외 처리 패턴과 일관성 있게 구현되었습니다. 자격증 관련 예외 클래스와 동일한 구조로 작성되어 일관성이 유지됩니다.
aics-api/src/main/java/kgu/developers/api/certificate/application/CertificateFacade.java (1)
19-24: 검증 결과: 우려 사항 없음
GraduationUserQueryService.me()메서드의 구현을 확인한 결과, 다음 사항들이 모두 올바르게 처리되고 있습니다:
인증된 사용자 컨텍스트 조회:
UserQueryService.getMyId()는SecurityContextHolder.getContext().getAuthentication().getPrincipal()을 사용하여 Spring Security 컨텍스트에서 현재 인증된 사용자를 올바르게 추출합니다.인증 실패 시 예외 처리: 인증 관련 모든 예외 상황에서
UserNotAuthenticatedException을 발생시키며, 해당 사용자의 졸업 정보가 없으면GraduationUserNotFoundException을 발생시킵니다.Spring Security 일관성: 표준 Spring Security 패턴을 따르며, 테스트 코드에서도 SecurityContext가 올바르게 설정되어 검증됩니다.
aics-api/src/main/java/kgu/developers/api/graduationUser/application/GraduationUserFacade.java (1)
17-30: LGTM!
graduationUserQueryService.me()를 사용하여 현재 사용자 조회 로직을 일관되게 단순화했습니다.UserQueryService의존성 제거로 Facade의 결합도가 낮아졌습니다.aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisDomainExceptionCode.java (2)
13-13:THESIS_NOT_FOUND의 HTTP 상태 코드 변경을 확인해 주세요.
THESIS_NOT_FOUND를NOT_FOUND(404)에서BAD_REQUEST(400)로 변경하셨습니다. 일반적으로 리소스를 찾을 수 없는 경우404 NOT_FOUND가 더 적절합니다.400 BAD_REQUEST는 잘못된 요청 형식이나 유효하지 않은 입력에 사용됩니다.의도된 변경인지 확인 부탁드립니다.
14-15: LGTM!새로운 검증 예외 코드가 적절한
BAD_REQUEST상태와 명확한 한국어 메시지로 추가되었습니다.aics-domain/src/main/java/kgu/developers/domain/certificate/exception/CertificateInvalidGraduationTypeException.java (1)
1-11: LGTM!기존 예외 클래스 패턴을 따르는 깔끔한 구현입니다.
aics-domain/src/main/java/kgu/developers/domain/thesis/application/command/ThesisCommandService.java (1)
33-51: 검증 로직이 올바르게 구현되었습니다.졸업 방식 확인과 제출 기간 검증이 파일 저장 전에 수행되어, 유효하지 않은 제출에 대해 불필요한 파일 저장을 방지합니다.
aics-admin/src/testFixtures/java/graduationUser/application/GraduationUserAdminFacadeTest.java (1)
75-79:UserQueryService/GraduationUserQueryService주입 관계가 테스트 픽스처에서 일관적입니다.
fakeUserRepository를 그대로UserQueryService에 주입한 점이 좋습니다(다른 픽스처 대비 안정적).
aics-api/src/testFixtures/java/graduationUser/application/GraduationUserFacadeTest.java
Show resolved
Hide resolved
...a/kgu/developers/domain/graduationUser/application/command/GraduationUserCommandService.java
Outdated
Show resolved
Hide resolved
...a/kgu/developers/domain/graduationUser/application/command/GraduationUserCommandService.java
Outdated
Show resolved
Hide resolved
JangYeongHu
had a problem deploying
to
coverage
GitHub Actions
Failure
Test Coverage Report
Files
|
JangYeongHu
had a problem deploying
to
coverage
GitHub Actions
Failure
JangYeongHu
had a problem deploying
to
coverage
GitHub Actions
Failure
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## develop #315 +/- ##
=============================================
+ Coverage 86.51% 86.92% +0.40%
Complexity 67 67
=============================================
Files 24 24
Lines 267 260 -7
Branches 14 14
=============================================
- Hits 231 226 -5
+ Misses 24 22 -2
Partials 12 12
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
dkdltm221
left a comment
dkdltm221
left a comment
There was a problem hiding this comment.
LGTM 👍
따로 문제 없어 보입니다. 고생하셨어요
Summary
논문/자격증 제출 시 제출자의 졸업 방식과 일정의 진행 여부를 토대로 유효성을 검증합니다.
Tasks