refactor/#216 professor dto #217
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은 교수 관련 도메인에서 직위( role ) 처리를 개선하는 변경 사항을 포함합니다. 주요 변경 사항은 요청 객체와 관리자 페이사드에서 문자열 형태의 직위를 도메인 객체인 Changes
Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Test Coverage Report
Files
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop #217 +/- ##
=============================================
+ Coverage 92.15% 92.23% +0.07%
- Complexity 145 148 +3
=============================================
Files 49 49
Lines 408 412 +4
Branches 4 4
=============================================
+ Hits 376 380 +4
Misses 27 27
Partials 5 5
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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
aics-admin/src/main/java/kgu/developers/admin/professor/presentation/request/ProfessorRequest.java (1)
16-18: Schema 설명과 유효성 검사에 대한 제안
role필드 타입이Role열거형에서String으로 변경되었습니다. 이는 한글 입력 요구사항(교수, 조교수)을 수용하기 위한 적절한 변경입니다. 그러나, 문자열로 변경되면서 입력값이 제한되지 않게 되었습니다.다음과 같은 개선 사항을 고려해보세요:
- Schema 설명에 허용되는 값들을 명시하여 API 문서 사용자에게 안내
@Pattern또는 사용자 정의 유효성 검사기를 추가하여 허용된 값만 입력되도록 제한- @Schema(description = "직위", example = "조교수", requiredMode = REQUIRED) + @Schema(description = "직위 (허용값: 교수, 조교수)", example = "조교수", requiredMode = REQUIRED) @NotNull + @Pattern(regexp = "^(교수|조교수)$", message = "직위는 '교수' 또는 '조교수'만 입력 가능합니다.") String role,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
aics-admin/src/main/java/kgu/developers/admin/professor/application/ProfessorAdminFacade.java(2 hunks)aics-admin/src/main/java/kgu/developers/admin/professor/presentation/request/ProfessorRequest.java(1 hunks)aics-admin/src/testFixtures/java/professor/application/ProfessorAdminFacadeTest.java(4 hunks)aics-api/src/main/java/kgu/developers/api/professor/presentation/response/ProfessorListResponse.java(1 hunks)aics-api/src/testFixtures/java/professor/application/ProfessorFacadeTest.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/professor/application/response/ProfessorResponse.java(2 hunks)aics-domain/src/main/java/kgu/developers/domain/professor/domain/Role.java(2 hunks)aics-domain/src/main/java/kgu/developers/domain/professor/exception/NoSuchRoleException.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/professor/exception/ProfessorDomainExceptionCode.java(2 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
aics-domain/src/main/java/kgu/developers/domain/professor/domain/Role.java
[warning] 17-19: aics-domain/src/main/java/kgu/developers/domain/professor/domain/Role.java#L17-L19
Added lines #L17 - L19 were not covered by tests
🔇 Additional comments (18)
aics-domain/src/main/java/kgu/developers/domain/professor/exception/NoSuchRoleException.java (1)
1-11: 새로운 예외 클래스가 적절하게 구현되었습니다.역할(Role)에 대한 유효성 검증 예외를 명확하게 처리하기 위한 전용 예외 클래스를 추가한 것은 좋은 접근입니다.
CustomException을 확장하고 적절한 에러 코드를 전달하는 방식으로 일관성을 유지하고 있습니다.aics-api/src/main/java/kgu/developers/api/professor/presentation/response/ProfessorListResponse.java (1)
18-18: JSON 예제의 필드명이 정확히 변경되었습니다.DTO 필드명 통일화 작업의 일환으로 "type"에서 "role"로 변경된 것이 확인됩니다. 이는 PR의 목적과 일치하며 응답 모델의 일관성을 유지합니다.
aics-domain/src/main/java/kgu/developers/domain/professor/domain/Role.java (2)
3-3: 필요한 예외 클래스 import가 추가되었습니다.새로 구현된 정적 메서드에서 사용할
NoSuchRoleException을 적절히 import 했습니다.
15-21: 정적 팩토리 메서드가 잘 구현되었으나 테스트 커버리지가 필요합니다.한국어 문자열을 받아 적절한
Role열거형 값으로 변환하는 정적 메서드 구현이 잘 되었습니다. 스위치 표현식을 사용하여 가독성이 좋고, 유효하지 않은 값에 대한 예외 처리도 적절합니다.다만, 정적 분석 도구에 따르면 이 메서드의 switch 케이스들(17-19행)에 대한 테스트 커버리지가 없습니다. 다양한 입력값에 대한 단위 테스트를 추가하는 것이 좋겠습니다.
테스트 케이스 예시:
@Test void of_ValidRole_ReturnCorrectRole() { assertEquals(Role.PROFESSOR, Role.of("교수")); assertEquals(Role.ASSISTANT, Role.of("조교수")); } @Test void of_InvalidRole_ThrowNoSuchRoleException() { assertThrows(NoSuchRoleException.class, () -> Role.of("기타")); }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 17-19: aics-domain/src/main/java/kgu/developers/domain/professor/domain/Role.java#L17-L19
Added lines #L17 - L19 were not covered by testsaics-api/src/testFixtures/java/professor/application/ProfessorFacadeTest.java (2)
65-65: 필드명 변경이 테스트에 정확히 반영되었습니다.
type에서role로의 필드명 변경이 테스트 코드에 올바르게 반영되었습니다. 열거형의 description을 사용하여 적절히 검증하고 있습니다.
73-73: 필드명 변경이 테스트에 정확히 반영되었습니다.두 번째 교수 객체에 대해서도
type에서role로의 필드명 변경이 테스트 코드에 올바르게 반영되었습니다.aics-admin/src/main/java/kgu/developers/admin/professor/application/ProfessorAdminFacade.java (3)
3-3: 도메인 클래스 임포트가 적절하게 추가되었습니다도메인 계층의 Role 클래스를 임포트하여 요청으로부터 받은 문자열을 도메인 객체로 변환하는 패턴을 구현할 수 있게 되었습니다.
22-23: 도메인 모델의 일관성 유지를 위한 좋은 접근법입니다문자열로 받은 role 값을 Role.of() 메서드를 사용하여 도메인 객체로 변환하는 패턴이 적절합니다. 이는 외부 입력을 도메인 객체로 변환하는 책임을 명확히 하며, 파사드 계층에서 이러한 변환을 처리하는 것은 계층 분리 관점에서 좋은 선택입니다.
29-30: 일관된 접근법으로 업데이트 메서드도 수정되었습니다createProfessor와 마찬가지로 updateProfessor에서도 동일한 패턴을 적용하여 코드의 일관성을 유지하고 있습니다.
aics-domain/src/main/java/kgu/developers/domain/professor/exception/ProfessorDomainExceptionCode.java (2)
3-3: 적절한 HttpStatus 임포트가 추가되었습니다새로운 예외 코드에 필요한 BAD_REQUEST 상태 코드를 임포트하였습니다.
15-17: 명확한 예외 메시지로 API 사용성이 향상되었습니다NO_SUCH_ROLE 예외 코드가 추가되어 역할 값이 올바르지 않을 때 적절한 오류 메시지를 제공합니다. 한글로 "교수" 또는 "조교수"라는 입력 값만 유효하다는 것을 명시하여 API 사용자에게 명확한 안내를 제공합니다.
aics-domain/src/main/java/kgu/developers/domain/professor/application/response/ProfessorResponse.java (2)
18-18: 일관된 필드명 사용으로 가독성이 향상되었습니다"type"에서 "role"로 필드명이 변경되어 도메인 모델과 DTO 간의 용어 일관성이 개선되었습니다. 이는 코드 전반에 걸쳐 일관된 용어를 사용하는 좋은 사례입니다.
36-36: 필드명 변경에 따라 메서드 내부도 적절히 수정되었습니다from 메서드에서도 필드명 변경에 맞게 "role"로 업데이트되었습니다. 이는 리팩토링 시 놓치기 쉬운 부분을 잘 처리한 것입니다.
aics-admin/src/testFixtures/java/professor/application/ProfessorAdminFacadeTest.java (4)
9-10: 필요한 클래스 임포트가 적절히 추가되었습니다Role 클래스를 일반 임포트로 변경하여 Role.of() 메서드 사용을 지원합니다.
48-48: 문자열 기반 역할 입력을 테스트에 반영했습니다역할이 Enum에서 문자열로 변경된 사항이 테스트 케이스에 올바르게 반영되었습니다. 한글로 "교수"를 지정하여 실제 사용 사례와 일치하도록 구현되었습니다.
69-69: 일관된 접근법으로 모든 테스트가 수정되었습니다updateProfessor 테스트에서도 동일하게 한글 문자열 "교수"를 사용하여 일관성을 유지했습니다.
85-85: 검증 로직이 적절하게 업데이트되었습니다직접 문자열 비교 대신 Role.of() 메서드를 사용하여 변환된 도메인 객체를 비교하도록 assertion이 수정되었습니다. 이는 문자열이 올바르게 도메인 객체로 변환되는지 검증하는 적절한 방법입니다.
aics-admin/src/main/java/kgu/developers/admin/professor/presentation/request/ProfessorRequest.java (1)
8-9: import 구문 정리static import 구문을 별도 블록으로 이동한 것은 코드 가독성에 좋은 영향을 미칩니다.
Summary
Tasks
To Reviewers
또 사용자의 입력을 통해 Role을 생성하려다보니 Enum에 정적 팩토리 메소드를 추가했는데 올바른 구현인지 궁금합니다.
Screenshot
조회
생성
