refactor: 임베디드 타입 DTO에 직접 노출 개선 및 응답 DTO 네이밍 컨벤션 통일 #261
Conversation
WalkthroughThe changes refactor the score-related API endpoints and service methods to return updated response DTOs that encapsulate multiple statuses. New DTO records have been added to convert embedded types (Gpa, LanguageTest) into dedicated response objects, and naming conventions have been standardized. Validation annotations have been enhanced in request DTOs. Deprecated response records have been removed and corresponding tests have been updated to work with the new data structures. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as API Client
participant SC as ScoreController
participant SS as ScoreService
participant DTO as DTO Converter (static from methods)
Client->>SC: Request GPA/LanguageTest Score Status
SC->>SS: Invoke service method
SS->>DTO: Convert domain objects to new DTO response(s)
DTO-->>SS: Return converted DTOs
SS-->>SC: Return aggregated response DTO
SC-->>Client: Send response
Assessment against linked issues
Poem
✨ Finishing Touches
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. 🪧 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 (
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the response DTOs for score-related endpoints by replacing direct exposure of embedded types with dedicated response DTO containers and standardizing the naming convention. Key changes include updating service and controller layers to return plural response DTOs, revising test assertions to match the new DTO formats, and removing legacy DTO classes.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/com/example/solidconnection/score/service/ScoreServiceTest.java | Updated test assertions and import statements to match new DTO names |
| src/main/java/com/example/solidconnection/score/service/ScoreService.java | Refactored methods to return plural response DTOs instead of singular ones |
| src/main/java/com/example/solidconnection/score/dto/LanguageTestScoreStatusesResponse.java | Introduced a new response container DTO for language test scores |
| src/main/java/com/example/solidconnection/score/dto/LanguageTestScoreStatusResponse.java | Updated DTO structure to use detailed fields instead of a list |
| src/main/java/com/example/solidconnection/score/dto/LanguageTestScoreStatus.java | Removed legacy DTO |
| src/main/java/com/example/solidconnection/score/dto/LanguageTestScoreRequest.java | Minor code style adjustment |
| src/main/java/com/example/solidconnection/score/dto/LanguageTestResponse.java | Added new DTO to encapsulate language test details |
| src/main/java/com/example/solidconnection/score/dto/GpaScoreStatusesResponse.java | Introduced a new response container DTO for GPA scores |
| src/main/java/com/example/solidconnection/score/dto/GpaScoreStatusResponse.java | Updated DTO structure to use detailed fields instead of a list |
| src/main/java/com/example/solidconnection/score/dto/GpaScoreStatus.java | Removed legacy DTO |
| src/main/java/com/example/solidconnection/score/dto/GpaScoreRequest.java | Minor code style adjustment |
| src/main/java/com/example/solidconnection/score/dto/GpaResponse.java | Added new DTO to encapsulate GPA details |
| src/main/java/com/example/solidconnection/score/controller/ScoreController.java | Updated API endpoints to return plural response DTOs |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
![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)
src/main/java/com/example/solidconnection/score/service/ScoreService.java (1)
50-57: Consider addressing TODOs in a future PRThere are several TODOs in this service regarding the relationship between
SiteUserand scores. Consider addressing these in a follow-up PR to improve the entity relationships and fetch strategies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/com/example/solidconnection/score/controller/ScoreController.java(2 hunks)src/main/java/com/example/solidconnection/score/dto/GpaResponse.java(1 hunks)src/main/java/com/example/solidconnection/score/dto/GpaScoreRequest.java(1 hunks)src/main/java/com/example/solidconnection/score/dto/GpaScoreStatus.java(0 hunks)src/main/java/com/example/solidconnection/score/dto/GpaScoreStatusResponse.java(1 hunks)src/main/java/com/example/solidconnection/score/dto/GpaScoreStatusesResponse.java(1 hunks)src/main/java/com/example/solidconnection/score/dto/LanguageTestResponse.java(1 hunks)src/main/java/com/example/solidconnection/score/dto/LanguageTestScoreRequest.java(1 hunks)src/main/java/com/example/solidconnection/score/dto/LanguageTestScoreStatus.java(0 hunks)src/main/java/com/example/solidconnection/score/dto/LanguageTestScoreStatusResponse.java(1 hunks)src/main/java/com/example/solidconnection/score/dto/LanguageTestScoreStatusesResponse.java(1 hunks)src/main/java/com/example/solidconnection/score/service/ScoreService.java(2 hunks)src/test/java/com/example/solidconnection/score/service/ScoreServiceTest.java(5 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/com/example/solidconnection/score/dto/LanguageTestScoreStatus.java
- src/main/java/com/example/solidconnection/score/dto/GpaScoreStatus.java
🔇 Additional comments (19)
src/main/java/com/example/solidconnection/score/dto/GpaScoreStatusesResponse.java (1)
5-11: Well-structured wrapper response for collectionsThis new record class follows a good pattern for API responses containing collections of items. The naming convention clearly indicates a collection with the plural "Statuses", and the static factory method provides a clean way to construct the response from domain objects.
src/main/java/com/example/solidconnection/score/dto/LanguageTestScoreRequest.java (1)
8-10: Good addition of validation for required fieldAdding the
@NotNullvalidation ensures the language test type is properly provided in requests, improving input validation and API robustness.src/main/java/com/example/solidconnection/score/dto/GpaScoreRequest.java (1)
6-8: Consistent validation pattern appliedThe addition of
@NotNullvalidation for the GPA field maintains consistency with other request DTOs and ensures all required fields are properly validated.src/main/java/com/example/solidconnection/score/dto/GpaResponse.java (1)
5-16: Good encapsulation of embedded type as dedicated DTOCreating a dedicated response DTO for the Gpa embedded type improves the API structure by properly encapsulating domain objects and creating a clear separation between the internal model and the API contract. The static factory method provides a clean way to construct the response from domain objects.
src/main/java/com/example/solidconnection/score/dto/LanguageTestResponse.java (1)
6-18: Well-structured DTO with proper encapsulationThis new LanguageTestResponse record provides a clean way to encapsulate the embedded LanguageTest type as a dedicated response object. The static factory method follows best practices for converting domain objects to DTOs.
src/main/java/com/example/solidconnection/score/dto/LanguageTestScoreStatusesResponse.java (1)
5-11: Good collection response structure with consistent namingThe LanguageTestScoreStatusesResponse record appropriately encapsulates a list of status responses, following a clear naming convention with pluralized form for collections. The static factory method provides a clean way to construct this response.
src/main/java/com/example/solidconnection/score/dto/GpaScoreStatusResponse.java (1)
3-20: Improved structure with dedicated embedded type responseThe refactored GpaScoreStatusResponse now properly encapsulates the GPA data with a dedicated GpaResponse object instead of exposing the embedded type directly. The static factory method provides a clean mapping from domain objects to DTOs, and the additional fields (id, verifyStatus, rejectedReason) provide more comprehensive information in the response.
src/main/java/com/example/solidconnection/score/dto/LanguageTestScoreStatusResponse.java (1)
3-20: Consistent refactoring with improved embedded type handlingThe changes to LanguageTestScoreStatusResponse mirror those in GpaScoreStatusResponse, creating a consistent pattern across the codebase. The record now properly uses the dedicated LanguageTestResponse to encapsulate the embedded type data, and includes important metadata like id, verification status, and rejection reason. The static factory method provides a clean way to create these responses from domain objects.
src/test/java/com/example/solidconnection/score/service/ScoreServiceTest.java (5)
11-14: Import changes align with new response DTOsThe import changes reflect the updated response DTO structure, adding
GpaScoreStatusesResponseandLanguageTestScoreStatusesResponsewhich are the wrapper classes for the status list responses.
65-76: Test updated to use new response wrapper patternThe test has been appropriately updated to work with the new response structure. The test now:
- Correctly retrieves a
GpaScoreStatusesResponseobject instead of a direct list- Uses
gpaScoreStatusResponseList()accessor method to get the inner list- Maps using
GpaScoreStatusResponse::fromto create the expected result objectsThis change is consistent with the refactoring to use container objects for collections of responses.
84-87: Empty response test updated correctlyThe test for empty GPA score lists has been properly updated to use the new wrapper response type and accessor method.
101-110: Language test scores test updated correctlySimilar to the GPA scores test, this test has been properly updated to use the new response wrapper pattern, consistently using:
LanguageTestScoreStatusesResponseas the container typelanguageTestScoreStatusResponseList()accessor methodLanguageTestScoreStatusResponse::fromfor mapping individual itemsThe changes are consistent with the overall refactoring approach.
119-122: Empty language test scores test updated correctlyThe test for empty language test score lists has been properly updated to use the new wrapper response type and accessor method.
src/main/java/com/example/solidconnection/score/controller/ScoreController.java (3)
5-7: Imports updated for new response wrapper DTOsThe import statements have been updated to use the new response wrapper classes, which is consistent with the refactoring to standardize DTO naming conventions.
52-57: GPA score status endpoint updated to use the wrapper responseThe API endpoint return type has been properly updated to return
ResponseEntity<GpaScoreStatusesResponse>instead of a direct list response. This change aligns with the refactoring goal of improving embedded type DTO exposure and standardizing response naming conventions.
61-66: Language test score status endpoint updated to use the wrapper responseThe API endpoint return type has been properly updated to return
ResponseEntity<LanguageTestScoreStatusesResponse>instead of a direct list response. This change is consistent with the other endpoints and follows the same pattern of using container objects for collections of responses.src/main/java/com/example/solidconnection/score/service/ScoreService.java (3)
12-15: Imports updated for new response wrapper DTOsThe import statements have been updated to include the new response wrapper classes, which is consistent with the refactoring to standardize DTO naming conventions.
77-88: GPA score status service method refactored to use wrapper responseThe method has been properly updated to:
- Return
GpaScoreStatusesResponseinstead of a direct list- Rename the variable to clearly indicate it contains response objects (
gpaScoreStatusResponseList)- Use the correct mapping function (
GpaScoreStatusResponse::from)- Create a wrapper response object using the static factory method
fromThis implementation consistently follows the refactoring goal of having proper container objects for collections.
90-101: Language test score status service method refactored to use wrapper responseThe method has been properly updated to:
- Return
LanguageTestScoreStatusesResponseinstead of a direct list- Rename the variable to clearly indicate it contains response objects (
languageTestScoreStatusResponseList)- Use the correct mapping function (
LanguageTestScoreStatusResponse::from)- Create a wrapper response object using the static factory method
fromThe implementation is consistent with the GPA score status method and follows the same pattern.
|
생기면 추가로 말씀드리겠습니다! |
3 participants
User description
관련 이슈
작업 내용
임베디드 타입을 그대로 DTO로 노출하는 구조 개선를 개선하고 응답 DTO 네이밍 컨벤션 통일하였습니다.
특이 사항
리뷰 요구사항 (선택)
그동안 개발된 거 보니까 @NotNull같은 어노테이션 붙어있는 필드는 한 칸 공백이 있어서 따라하긴 했는데 이것도 혹시 따로 정해진 게 있는걸까요?
예시)
PR Type
Enhancement, Tests
Description
Refactored DTOs to encapsulate embedded types for better structure.
GpaResponseandLanguageTestResponseto replace direct use of embedded types.Unified naming conventions for response DTOs.
GpaScoreStatusResponsetoGpaScoreStatusesResponse.LanguageTestScoreStatusResponsetoLanguageTestScoreStatusesResponse.Updated service and controller layers to use the new DTOs.
Adjusted test cases to align with the new DTO structure and naming conventions.
Changes walkthrough 📝
10 files
Updated controller to use new DTOs and naming conventionsAdded `GpaResponse` DTO for encapsulating GPA detailsRemoved `GpaScoreStatus` DTORefactored `GpaScoreStatusResponse` to use `GpaResponse`Added `GpaScoreStatusesResponse` DTO for list encapsulationAddedLanguageTestResponseDTO for encapsulating language test detailsRemoved `LanguageTestScoreStatus` DTORefactoredLanguageTestScoreStatusResponseto useLanguageTestResponseAdded `LanguageTestScoreStatusesResponse` DTO for list encapsulationUpdated service methods to use new DTOs and naming conventions2 files
Minor formatting update in `GpaScoreRequest`Minor formatting update in `LanguageTestScoreRequest`1 files
Updated test cases to align with new DTO structure and namingconventionsSummary by CodeRabbit
New Features
Refactor
Tests