Skip to content
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.

Sign up for GitHub

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

fix : 요청시 지역 타입 변경 #13

Merged
merged 1 commit into from
Feb 5, 2025
Merged

fix : 요청시 지역 타입 변경 #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/main/java/com/yedu/backend/admin/application/dto/req/TeacherSearchRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
package com.yedu.backend.admin.application.dto.req;

import com.yedu.backend.domain.parents.domain.entity.constant.ClassType;
import com.yedu.backend.domain.teacher.domain.entity.constant.District;
import com.yedu.backend.domain.teacher.domain.entity.constant.TeacherGender;

import java.util.List;

public record TeacherSearchRequest(
List<District> districts,
List<String> districts,
List<ClassType> subjects,
List<String> universities,
List<TeacherGender> genders,
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/com/yedu/backend/admin/application/dto/res/AllFilteringTeacher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.yedu.backend.admin.application.dto.res;

import com.yedu.backend.domain.parents.domain.entity.constant.ClassType;
import com.yedu.backend.domain.teacher.domain.entity.constant.District;
import com.yedu.backend.domain.teacher.domain.entity.constant.TeacherStatus;

import java.util.List;
Expand All @@ -18,7 +17,7 @@ public record FilteringTeacher (
int accept,
int total,
String university,
List<District> districts,
List<String> districts,
String issue
){}
}
3 changes: 1 addition & 2 deletions src/main/java/com/yedu/backend/admin/application/mapper/AdminMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import com.yedu.backend.domain.teacher.domain.entity.Teacher;
import com.yedu.backend.domain.teacher.domain.entity.TeacherClassInfo;
import com.yedu.backend.domain.teacher.domain.entity.TeacherInfo;
import com.yedu.backend.domain.teacher.domain.entity.constant.District;

import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -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) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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 mapToAllFilteringTeacherResponse to iterate over the provided strings, validate each using District.fromString, and then use the resulting District value in the response.

TeacherInfo teacherInfo = teacher.getTeacherInfo();
TeacherClassInfo classInfo = teacher.getTeacherClassInfo();
List<ClassType> classTypes = new ArrayList<>();
Expand Down
5 changes: 2 additions & 3 deletions src/main/java/com/yedu/backend/admin/application/usecase/AdminInfoUseCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import com.yedu.backend.domain.parents.domain.entity.Goal;
import com.yedu.backend.domain.teacher.domain.entity.Teacher;
import com.yedu.backend.domain.teacher.domain.entity.TeacherDistrict;
import com.yedu.backend.domain.teacher.domain.entity.constant.District;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
Expand Down Expand Up @@ -74,8 +73,8 @@ public AllFilteringTeacher searchAllTeacher(TeacherSearchRequest request) {
return new AllFilteringTeacher(teachers.stream()
.map(teacher -> {
List<TeacherDistrict> teacherDistricts = adminGetService.allDistrictByTeacher(teacher);
List<District> districts = teacherDistricts.stream()
.map(TeacherDistrict::getDistrict)
List<String> districts = teacherDistricts.stream()
.map(teacherDistrict -> teacherDistrict.getDistrict().getDescription())
.toList();
return mapToAllFilteringTeacherResponse(teacher, districts);
})
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/com/yedu/backend/admin/presentation/AdminController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import com.yedu.backend.admin.application.usecase.AdminInfoUseCase;
import com.yedu.backend.admin.application.usecase.AdminManageUseCase;
import com.yedu.backend.domain.parents.domain.entity.constant.ClassType;
import com.yedu.backend.domain.teacher.domain.entity.constant.District;
import com.yedu.backend.domain.teacher.domain.entity.constant.TeacherGender;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
Expand Down Expand Up @@ -66,7 +65,7 @@ public ResponseEntity<ClassDetailsResponse> classDetails(@PathVariable String ap

@GetMapping("/details/matching/search")
public ResponseEntity<AllFilteringTeacher> searchTeachers(
@RequestParam(required = false) List<District> districts,
@RequestParam(required = false) List<String> districts,
@RequestParam(required = false) List<ClassType> subjects,
@RequestParam(required = false) List<String> universities,
@RequestParam(required = false) List<TeacherGender> genders,
Expand Down
4 changes: 2 additions & 2 deletions ...main/java/com/yedu/backend/domain/teacher/domain/repository/TeacherDslRepositoryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,13 @@ private BooleanExpression genderSpecifier(List<TeacherGender> genders) {
.orElse(TRUE);
}

private BooleanExpression districtSpecifier(List<District> districts) {
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);
}
Comment on lines +114 to 123
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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

⚠️ Potential issue

🧩 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 java

Length 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.

Expand Down
Loading