-
Notifications
You must be signed in to change notification settings - Fork 1
refactor/KD-65 Certificate/Thesis 제출 API의 request 명세 변경 #319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
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은 인증서와 논문 제출 API를 리팩토링합니다. 인증서 제출에서 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
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 |
Test Coverage Report
Files
|
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (10.00%). You can increase the patch coverage or adjust the target coverage. @@ Coverage Diff @@
## develop #319 +/- ##
=============================================
+ Coverage 86.92% 87.25% +0.33%
Complexity 67 67
=============================================
Files 24 24
Lines 260 259 -1
Branches 14 14
=============================================
Hits 226 226
+ Misses 22 21 -1
Partials 12 12
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
aics-domain/src/testFixtures/java/graduationUser/application/GraduationUserCommandServiceTest.java (1)
142-154: 사용되지 않는 변수 제거 필요
midSchedule과finalSchedule변수가 생성되었지만 더 이상 사용되지 않습니다.updateThesis호출이SubmissionType을 직접 사용하도록 변경되었으므로 이 변수들과 Line 7의Scheduleimport를 제거해야 합니다.♻️ 미사용 코드 제거
Line 7의 import 제거:
-import kgu.developers.domain.schedule.domain.Schedule;테스트 메서드 수정:
`@Test` `@DisplayName`("updateThesis은 주어진 타입에 따라 GraduationUser의 논문ID를 저장한다.") public void updateThesis_Success() { //given Long midThesisId = 1L; Long finalThesisId = 2L; - Schedule midSchedule = Schedule.builder() - .id(1L) - .submissionType(SubmissionType.MIDTHESIS) - .build(); - - Schedule finalSchedule = Schedule.builder() - .id(1L) - .submissionType(SubmissionType.FINALTHESIS) - .build(); //when graduationUserCommandService.updateThesis(graduationUser, midThesisId, SubmissionType.MIDTHESIS); graduationUserCommandService.updateThesis(graduationUser, finalThesisId, SubmissionType.FINALTHESIS);
🤖 Fix all issues with AI agents
In
`@aics-domain/src/main/java/kgu/developers/domain/thesis/application/command/ThesisCommandService.java`:
- Around line 37-39: The thrown exception uses the wrong error code constant:
update ThesisInvalidSubmissionTypeException to use
THESIS_INVALID_SUBMISSION_TYPE_EXCEPTION (instead of
THESIS_INVALID_GRADUATION_TYPE_EXCEPTION) so the correct code/message is
returned to clients; locate the ThesisInvalidSubmissionTypeException class and
replace the incorrect enum/constant reference with
THESIS_INVALID_SUBMISSION_TYPE_EXCEPTION and ensure any constructors or factory
methods forward that constant.
In
`@aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisInvalidSubmissionTypeException.java`:
- Around line 5-10: ThesisInvalidSubmissionTypeException is using the wrong enum
constant; replace the import/reference to
THESIS_INVALID_GRADUATION_TYPE_EXCEPTION with
THESIS_INVALID_SUBMISSION_TYPE_EXCEPTION so the class
(ThesisInvalidSubmissionTypeException) calls
super(THESIS_INVALID_SUBMISSION_TYPE_EXCEPTION) and imports the correct constant
from ThesisDomainExceptionCode.
🧹 Nitpick comments (1)
aics-domain/src/main/java/kgu/developers/domain/graduationUser/application/command/GraduationUserCommandService.java (1)
51-57: switch 문에 다른 SubmissionType 값 처리를 위한 default case 추가 필요
SubmissionTypeenum은SUBMITTED,MIDTHESIS,FINALTHESIS,CERTIFICATE,APPROVED,OTHER총 6개의 값을 가지고 있으나, 현재 switch 문은MIDTHESIS와FINALTHESIS만 처리합니다. 예상치 못한 값이 전달될 경우,save()메서드만 호출되고 thesis 업데이트가 이루어지지 않을 수 있습니다.ThesisCommandService에서 사용되는 패턴과 일관성 있게ThesisInvalidSubmissionTypeException을 발생시키는 default case를 추가하세요.♻️ default case 추가
public void updateThesis(GraduationUser graduationUser, Long thesisId, SubmissionType thesisType) { switch (thesisType) { case MIDTHESIS -> graduationUser.updateMidThesisId(thesisId); case FINALTHESIS -> graduationUser.updateFinalThesisId(thesisId); + default -> throw new ThesisInvalidSubmissionTypeException(); } graduationUserRepository.save(graduationUser); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
aics-api/src/main/java/kgu/developers/api/certificate/application/CertificateFacade.javaaics-api/src/main/java/kgu/developers/api/certificate/presentation/CertificateController.javaaics-api/src/main/java/kgu/developers/api/certificate/presentation/CertificateControllerImpl.javaaics-api/src/main/java/kgu/developers/api/certificate/presentation/request/CertificateSubmitRequest.javaaics-api/src/main/java/kgu/developers/api/thesis/application/ThesisFacade.javaaics-api/src/main/java/kgu/developers/api/thesis/presentation/ThesisControllerImpl.javaaics-api/src/main/java/kgu/developers/api/thesis/presentation/request/ThesisSubmitRequest.javaaics-domain/src/main/java/kgu/developers/domain/certificate/application/command/CertificateCommandService.javaaics-domain/src/main/java/kgu/developers/domain/graduationUser/application/command/GraduationUserCommandService.javaaics-domain/src/main/java/kgu/developers/domain/thesis/application/command/ThesisCommandService.javaaics-domain/src/main/java/kgu/developers/domain/thesis/domain/ThesisType.javaaics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisDomainExceptionCode.javaaics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisInvalidSubmissionTypeException.javaaics-domain/src/testFixtures/java/carousel/application/CarouselCommandServiceTest.javaaics-domain/src/testFixtures/java/carousel/application/CarouselQueryServiceTest.javaaics-domain/src/testFixtures/java/graduationUser/application/GraduationUserCommandServiceTest.java
💤 Files with no reviewable changes (2)
- aics-domain/src/main/java/kgu/developers/domain/thesis/domain/ThesisType.java
- aics-api/src/main/java/kgu/developers/api/certificate/presentation/request/CertificateSubmitRequest.java
🧰 Additional context used
🧬 Code graph analysis (4)
aics-domain/src/main/java/kgu/developers/domain/thesis/application/command/ThesisCommandService.java (2)
aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisInvalidGraduationTypeException.java (1)
ThesisInvalidGraduationTypeException(7-11)aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisInvalidSubmissionTypeException.java (1)
ThesisInvalidSubmissionTypeException(7-11)
aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisInvalidSubmissionTypeException.java (2)
aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisInvalidGraduationTypeException.java (2)
ThesisInvalidGraduationTypeException(7-11)ThesisInvalidGraduationTypeException(8-10)aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisNotInSubmissionPeriodException.java (1)
ThesisNotInSubmissionPeriodException(7-11)
aics-domain/src/testFixtures/java/carousel/application/CarouselQueryServiceTest.java (1)
aics-domain/src/testFixtures/java/mock/repository/FakeCarouselRepository.java (1)
FakeCarouselRepository(14-53)
aics-api/src/main/java/kgu/developers/api/certificate/presentation/CertificateControllerImpl.java (1)
aics-api/src/main/java/kgu/developers/api/thesis/presentation/ThesisControllerImpl.java (1)
RestController(13-30)
🔇 Additional comments (13)
aics-domain/src/testFixtures/java/carousel/application/CarouselQueryServiceTest.java (1)
3-12: PR 목적과의 불일치를 확인해주세요.이 파일의 변경사항은 import 구문 재구성일 뿐이며, PR의 주요 목적인 Certificate/Thesis 제출 API 리팩토링과는 관련이 없는 Carousel 테스트 픽스처입니다. 이 파일이 이 PR에 포함된 이유를 확인해주세요.
Import 변경 자체는 정확하며 모든 import가 코드에서 사용되고 있습니다.
aics-domain/src/testFixtures/java/carousel/application/CarouselCommandServiceTest.java (1)
9-14: PR 목적과의 불일치를 확인해주세요.이 파일도 마찬가지로 import 구문 재구성만 포함하고 있으며, PR의 주요 목적인 Certificate/Thesis 제출 API 리팩토링과는 무관한 Carousel 테스트 픽스처입니다. CarouselQueryServiceTest.java와 동일한 패턴입니다.
Import 변경 자체는 정확하며 모든 import가 코드에서 사용되고 있습니다.
aics-domain/src/main/java/kgu/developers/domain/certificate/application/command/CertificateCommandService.java (2)
52-53:schedule.getId()사용이 적절합니다.조회된 스케줄에서 ID를 추출하여 Certificate 생성에 사용하는 것이 일관성 있는 접근입니다.
34-35: 리팩토링이 깔끔합니다.
scheduleId를 파라미터로 받는 대신SubmissionType.CERTIFICATE로 스케줄을 조회하는 방식으로 변경되어 API가 단순해졌습니다. 호출자가 올바른scheduleId를 알 필요가 없어졌습니다.
getBySubmissionType()에서 해당 스케줄이 없는 경우ScheduleNotFoundException을 통해 적절히 처리되고 있습니다.aics-api/src/main/java/kgu/developers/api/certificate/application/CertificateFacade.java (1)
19-24: LGTM!도메인 서비스의 시그니처 변경이 Facade 레이어에 올바르게 전파되었습니다.
aics-api/src/main/java/kgu/developers/api/certificate/presentation/CertificateController.java (1)
28-34: Breaking API 변경 확인 필요
CertificateSubmitRequest파라미터 제거는 API 소비자에게 breaking change입니다. API 버전 관리 또는 클라이언트 측 업데이트가 필요할 수 있습니다.aics-api/src/main/java/kgu/developers/api/certificate/presentation/CertificateControllerImpl.java (2)
21-30: 구현이 인터페이스 변경과 일치합니다.
ThesisControllerImpl(relevant snippet 참조)은 여전히ThesisSubmitRequest를 통해type을 받는 반면, Certificate는 내부적으로SubmissionType.CERTIFICATE를 사용하는 것이 PR 목표에 부합합니다. Certificate는 단일 제출 유형만 있으므로 클라이언트에서 지정할 필요가 없습니다.
3-13: Import 정리가 잘 되었습니다.와일드카드 import에서 명시적 import로 변경하여 코드 가독성이 향상되었습니다.
aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisDomainExceptionCode.java (1)
16-16: LGTM!새로운 예외 코드가 기존 패턴과 일관성 있게 추가되었습니다. BAD_REQUEST 상태와 메시지가 적절합니다.
aics-api/src/main/java/kgu/developers/api/thesis/presentation/ThesisControllerImpl.java (1)
25-25: LGTM!
scheduleId에서SubmissionType으로의 변경이 PR 목표에 맞게 올바르게 적용되었습니다.aics-api/src/main/java/kgu/developers/api/thesis/presentation/request/ThesisSubmitRequest.java (1)
6-10: LGTM!
scheduleId에서SubmissionTypeenum으로의 변경이 깔끔하게 구현되었습니다. 검증 메시지도 새 필드에 맞게 적절히 업데이트되었습니다.aics-domain/src/main/java/kgu/developers/domain/thesis/application/command/ThesisCommandService.java (1)
35-59: 전체 로직 흐름 승인
SubmissionType기반의 스케줄 조회 및 검증 로직이 적절하게 구현되었습니다.MIDTHESIS/FINALTHESIS검증을 통해 잘못된 제출 유형을 방지하고, 스케줄 조회 방식 변경도 PR 목적에 부합합니다.aics-api/src/main/java/kgu/developers/api/thesis/application/ThesisFacade.java (1)
23-29: LGTM!Facade 레이어의 리팩토링이 깔끔하게 완료되었습니다.
SubmissionType을 사용하여 하위 서비스들에 올바르게 전달하고 있으며, 책임 분리가 잘 유지되고 있습니다.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
...ain/src/main/java/kgu/developers/domain/thesis/application/command/ThesisCommandService.java
Show resolved
Hide resolved
...c/main/java/kgu/developers/domain/thesis/exception/ThesisInvalidSubmissionTypeException.java
Outdated
Show resolved
Hide resolved
LeeHanEum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
훌륭하네요 LGTM 👍
...rc/main/java/kgu/developers/domain/certificate/exception/CertificateDomainExceptionCode.java
Outdated
Show resolved
Hide resolved
dkdltm221
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
제가 보기엔 문제없어 보입니다! 고생하셨어요
Summary
Certificate와 Thesis 제출 API의 scheduleId를 제거/변경 하였습니다.
Tasks