fix : 요청시 지역 타입 변경 #13
Conversation
WalkthroughThis pull request updates how district information is represented across several application modules. The primary change is converting the Changes
Sequence Diagram(s)sequenceDiagram
participant C as AdminController
participant U as AdminInfoUseCase
participant M as AdminMapper
participant R as TeacherDslRepositoryImpl
C->>U: searchAllTeacher(request with districts as List<String>)
U->>M: mapToAllFilteringTeacherResponse(teacher, districts)
M-->>U: FilteringTeacher response
U->>R: districtSpecifier(List<String> districts)
R->>R: Convert each district string to District enum (District.fromString)
R-->>U: BooleanExpression for filtering
U-->>C: AllFilteringTeacher response
Possibly related PRs
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![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
🧹 Nitpick comments (1)
src/main/java/com/yedu/backend/admin/application/usecase/AdminInfoUseCase.java (1)
76-78: Add null safety checks in the district mapping.The current implementation might throw NullPointerException if either
teacherDistrictorgetDistrict()returns null.List<String> districts = teacherDistricts.stream() - .map(teacherDistrict -> teacherDistrict.getDistrict().getDescription()) + .map(teacherDistrict -> Optional.ofNullable(teacherDistrict) + .map(TeacherDistrict::getDistrict) + .map(District::getDescription) + .orElse("")) + .filter(district -> !district.isEmpty()) .toList();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/yedu/backend/admin/application/dto/req/TeacherSearchRequest.java(1 hunks)src/main/java/com/yedu/backend/admin/application/dto/res/AllFilteringTeacher.java(1 hunks)src/main/java/com/yedu/backend/admin/application/mapper/AdminMapper.java(1 hunks)src/main/java/com/yedu/backend/admin/application/usecase/AdminInfoUseCase.java(1 hunks)src/main/java/com/yedu/backend/admin/presentation/AdminController.java(1 hunks)src/main/java/com/yedu/backend/domain/teacher/domain/repository/TeacherDslRepositoryImpl.java(1 hunks)
🔇 Additional comments (3)
src/main/java/com/yedu/backend/admin/application/dto/req/TeacherSearchRequest.java (1)
9-9: Add validation for district values.Consider adding validation to ensure that the district strings match expected values and formats, especially for special cases like "경기도" (Gyeonggi-do).
@Pattern(regexp = "^[가-힣]+도?$", message = "Invalid district format") List<String> districts,✅ Verification successful
Error: No shell scripts or web queries found
Validation for district field in TeacherSearchRequest DTO
The current DTO now accepts district names as simple strings without enforcing any format. It’s a good idea to add validation—such as using a @pattern annotation—to ensure inputs match expected patterns (e.g., only valid Korean district names like "경기도").
- Consider adding a validation annotation (e.g.,
@Pattern) to the districts field.- Use a regex (like
"^[가-힣]+도?$") to allow only valid Korean district names.- This will help catch any client-side input errors early.
src/main/java/com/yedu/backend/admin/application/dto/res/AllFilteringTeacher.java (1)
20-20: LGTM!The change to
List<String>is consistent with the request DTO and makes sense for client consumption.src/main/java/com/yedu/backend/admin/presentation/AdminController.java (1)
68-68: LGTM!The parameter type change is consistent with the DTO changes, and the optional nature is appropriate for search functionality.
| @@ -103,7 +102,7 @@ else if (count.equals("주 4회")) | |||
| ); | |||
| } | |||
|
|
|||
| public static FilteringTeacher mapToAllFilteringTeacherResponse(Teacher teacher, List<District> districts) { | |||
| public static FilteringTeacher mapToAllFilteringTeacherResponse(Teacher teacher, List<String> districts) { | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add validation for district strings.
The method accepts raw strings without validating if they represent valid districts. Consider adding validation to ensure the strings match valid district values.
🏁 Script executed:
#!/bin/bash
# Check if District enum has a fromString method with proper validation
ast-grep --pattern 'enum District {
$$$
fromString($$$) {
$$$
}
$$$
}'Length of output: 84
🏁 Script executed:
#!/bin/bash
# Locate the file containing the District enum and display its content
fd -e java "District.java" --exec cat {} \;Length of output: 1764
District String Validation Needed in Mapping Method
The District enum provides a fromString method for validating district strings. However, the mapToAllFilteringTeacherResponse method in AdminMapper.java still accepts a List<String> without converting or validating the district strings. Consider updating the mapper to invoke District.fromString for each supplied string to ensure only valid districts are used or to explicitly handle invalid input.
- Location:
src/main/java/com/yedu/backend/admin/application/mapper/AdminMapper.java - Suggestion: Update the implementation of
mapToAllFilteringTeacherResponseto iterate over the provided strings, validate each usingDistrict.fromString, and then use the resultingDistrictvalue in the response.
| private BooleanExpression districtSpecifier(List<String> districts) { | ||
| if (districts == null || districts.isEmpty()) { | ||
| return TRUE; // where() 절에서 무시됨 | ||
| } | ||
|
|
||
| return districts.stream() | ||
| .map(teacherDistrict.district::eq) | ||
| .map(district -> teacherDistrict.district.eq(District.fromString(district))) | ||
| .reduce(BooleanExpression::or) | ||
| .orElse(TRUE); | ||
| } |
There was a problem hiding this comment.
Add error handling for invalid district strings.
The current implementation might throw an exception if District.fromString() fails to parse an invalid district string. Consider adding error handling:
private BooleanExpression districtSpecifier(List<String> districts) {
if (districts == null || districts.isEmpty()) {
return TRUE;
}
return districts.stream()
- .map(district -> teacherDistrict.district.eq(District.fromString(district)))
+ .map(district -> {
+ try {
+ return teacherDistrict.district.eq(District.fromString(district));
+ } catch (IllegalArgumentException e) {
+ // Log the error
+ return TRUE;
+ }
+ })
.reduce(BooleanExpression::or)
.orElse(TRUE);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private BooleanExpression districtSpecifier(List<String> districts) { | |
| if (districts == null || districts.isEmpty()) { | |
| return TRUE; // where() 절에서 무시됨 | |
| } | |
| return districts.stream() | |
| .map(teacherDistrict.district::eq) | |
| .map(district -> teacherDistrict.district.eq(District.fromString(district))) | |
| .reduce(BooleanExpression::or) | |
| .orElse(TRUE); | |
| } | |
| private BooleanExpression districtSpecifier(List<String> districts) { | |
| if (districts == null || districts.isEmpty()) { | |
| return TRUE; // where() 절에서 무시됨 | |
| } | |
| return districts.stream() | |
| .map(district -> { | |
| try { | |
| return teacherDistrict.district.eq(District.fromString(district)); | |
| } catch (IllegalArgumentException e) { | |
| // Log the error | |
| return TRUE; | |
| } | |
| }) | |
| .reduce(BooleanExpression::or) | |
| .orElse(TRUE); | |
| } |
💡 Verification agent
🧩 Analysis chain
Implement special handling for "경기도" regions.
According to the PR objectives, special handling for "경기도" regions should be implemented. Consider adding logic to handle Gyeonggi-do districts:
private BooleanExpression districtSpecifier(List<String> districts) {
if (districts == null || districts.isEmpty()) {
return TRUE;
}
return districts.stream()
- .map(district -> teacherDistrict.district.eq(District.fromString(district)))
+ .map(district -> {
+ if (district.contains("경기도")) {
+ // Handle Gyeonggi-do districts
+ return teacherDistrict.district.stringValue().like("경기도%");
+ }
+ return teacherDistrict.district.eq(District.fromString(district));
+ })
.reduce(BooleanExpression::or)
.orElse(TRUE);
}Let's verify if there are any existing Gyeonggi-do district handling patterns in the codebase:
🏁 Script executed:
#!/bin/bash
# Search for existing Gyeonggi-do handling patterns
rg -i "경기도" --type javaLength of output: 362
Special handling for "경기도" districts is missing.
The search output only shows hardcoded district constants (e.g., "경기도 안양", "경기도 용인", "경기도 고양") in the District.java file. There is no evidence of dedicated "경기도" handling logic in the TeacherDslRepositoryImpl. As special handling is required, you should add the conditional logic (using like("경기도%")) in the district specifier method as suggested.
- Verify the changes in
src/main/java/com/yedu/backend/domain/teacher/domain/repository/TeacherDslRepositoryImpl.java(lines 114-123). - Note that the only references to "경기도" are in the constants defined in
src/main/java/com/yedu/backend/domain/teacher/domain/entity/constant/District.java.
🐈 PR 요약
✨ PR 상세
🚨 참고사항
✅ 체크리스트
Summary by CodeRabbit