Conversation
Walkthrough관리자용 푸드트럭 전체 조회 기능을 추가합니다: 저장소 쿼리(상태별/ID목록), FoodTruckStatus 문자열 변환, 관련 오류 코드 및 Swagger 설명 추가, 관리자용 DTO·서비스·컨트롤러 엔드포인트와 문서 매핑 로직을 도입합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UserController
participant UserService
participant AdminValidator
participant AdminService
participant FoodTruckRepo as FoodTruckRepository
participant FoodTruckDocRepo as FoodTruckDocumentRepository
Client->>UserController: GET /users/admin/food-trucks (userId, status?)
UserController->>UserService: getAllFoodTrucks(userId, status)
UserService->>AdminValidator: validateAdmin(userId)
AdminValidator-->>UserService: ok
UserService->>AdminService: getAllFoodTrucks(status)
alt status provided
AdminService->>FoodTruckRepo: findAllByFoodTruckStatus(status)
else
AdminService->>FoodTruckRepo: findAll()
end
FoodTruckRepo-->>AdminService: List<FoodTruck>
AdminService->>FoodTruckDocRepo: findAllInFoodTruckIds(foodTruckIds)
FoodTruckDocRepo-->>AdminService: List<FoodTruckDocument>
note right of AdminService: 문서들을 foodTruckId별로 그룹화\n및 각 FoodTruck -> FoodTruckForAdminResponse 변환
AdminService-->>UserService: List<FoodTruckForAdminResponse>
UserService-->>UserController: List<FoodTruckForAdminResponse>
UserController-->>Client: BaseResponse.ok(list)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
src/main/java/konkuk/chacall/domain/foodtruck/domain/value/FoodTruckStatus.java (2)
3-3: 사용되지 않는 import를 제거하세요.
JsonCreator어노테이션이 import되었지만 실제로 사용되지 않고 있습니다.-import com.fasterxml.jackson.annotation.JsonCreator; import konkuk.chacall.global.common.exception.DomainRuleException;
18-25: API 계약에는 enum name 사용을 권장합니다.현재 구현은 description(한글 설명)으로 매칭하고 있지만, 다음과 같은 이유로 enum name(APPROVED, PENDING, REJECTED)을 사용하는 것이 더 안정적입니다:
- description은 UI 표시용 텍스트로 변경될 가능성이 있습니다
- 한글 문자열보다 영문 상수가 API 계약으로 더 명확합니다
- 국제화(i18n)를 고려할 때 description은 변경될 수 있습니다
다음과 같이 enum name을 사용하도록 리팩토링하는 것을 고려하세요:
public static FoodTruckStatus from(String status) { + try { + return FoodTruckStatus.valueOf(status.toUpperCase()); + } catch (IllegalArgumentException e) { + throw new DomainRuleException(ErrorCode.FOOD_TRUCK_STATUS_MISMATCH); + } - for (FoodTruckStatus foodTruckStatus : FoodTruckStatus.values()) { - if (foodTruckStatus.getDescription().equals(status)) { - return foodTruckStatus; - } - } - throw new DomainRuleException(ErrorCode.FOOD_TRUCK_STATUS_MISMATCH); }이렇게 하면 Controller의 @parameter 예시도 "승인 완료" 대신 "APPROVED"로 변경해야 합니다.
src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/FoodTruckDocumentRepository.java (2)
7-7: 사용되지 않는 import를 제거하세요.
Collectionimport가 사용되지 않습니다.-import java.util.Collection; import java.util.List;
11-16: Spring Data 네이밍 컨벤션 고려를 권장합니다.현재 메서드명
findAllInFoodTruckIds는 명확하지만, Spring Data JPA의 표준 네이밍 컨벤션(findAllByFoodTruckIdIn)을 따르면 더 일관성 있는 코드베이스를 유지할 수 있습니다.-@Query(""" - SELECT ftd - FROM FoodTruckDocument ftd - WHERE ftd.foodTruck.foodTruckId IN :foodTruckIds - """) -List<FoodTruckDocument> findAllInFoodTruckIds(List<Long> foodTruckIds); +List<FoodTruckDocument> findAllByFoodTruck_FoodTruckIdIn(List<Long> foodTruckIds);이렇게 하면 명시적인 @query 없이도 Spring Data가 자동으로 쿼리를 생성합니다.
src/main/java/konkuk/chacall/domain/user/application/admin/AdminService.java (1)
43-61: 컬렉션 변환 메서드의 일관성을 유지하세요.코드 내에서
toList()(Line 46)와Collectors.toList()(Lines 58, 61)를 혼용하고 있습니다. Java 16+의toList()를 사용하는 것으로 통일하면 코드 일관성이 향상됩니다.documentMap.getOrDefault(foodTruck.getFoodTruckId(), List.of()).stream() .map(FoodTruckDocument::getDocumentUrl) - .collect(Collectors.toList()) + .toList() ) ) - .collect(Collectors.toList()); + .toList();src/main/java/konkuk/chacall/domain/user/presentation/dto/response/FoodTruckForAdminResponse.java (1)
8-19: null 안전성 및 불변성 개선을 고려하세요.현재 레코드 필드에 null 안전성 어노테이션(
@Nullable또는@NotNull)이 없으며,List<String>타입이 외부에서 수정 가능한 리스트를 받을 경우 불변성이 깨질 수 있습니다.다음 개선사항을 고려해보세요:
- 각 필드에 명시적인 null 안전성 어노테이션 추가
- 생성자에서
List.copyOf(foodTruckDocumentUrls)를 사용하여 방어적 복사 수행
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/FoodTruckDocumentRepository.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/FoodTruckRepository.java(2 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/value/FoodTruckStatus.java(2 hunks)src/main/java/konkuk/chacall/domain/user/application/UserService.java(3 hunks)src/main/java/konkuk/chacall/domain/user/application/admin/AdminService.java(2 hunks)src/main/java/konkuk/chacall/domain/user/presentation/UserController.java(4 hunks)src/main/java/konkuk/chacall/domain/user/presentation/dto/response/FoodTruckForAdminResponse.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/konkuk/chacall/domain/user/application/admin/AdminService.java (1)
src/main/java/konkuk/chacall/domain/user/application/UserService.java (1)
Service(24-75)
🔇 Additional comments (3)
src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java (1)
86-86: LGTM!새로운 에러 코드가 적절하게 추가되었습니다. FoodTruck 에러 그룹 내에 논리적으로 배치되었으며, 코드 번호(110010)도 기존 패턴을 잘 따르고 있습니다.
src/main/java/konkuk/chacall/domain/user/application/admin/AdminService.java (1)
26-31: LGTM!
orElseThrow를 사용하여 명시적으로 예외를 처리하도록 개선된 것은 좋은 변경입니다.src/main/java/konkuk/chacall/domain/user/application/UserService.java (1)
70-74: LGTM!관리자 권한을 검증하고 AdminService에 위임하는 패턴이 올바르게 구현되었습니다.
src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/FoodTruckRepository.java
Show resolved
Hide resolved
| if(status != null && !status.isEmpty()) { | ||
| FoodTruckStatus foodTruckStatus = FoodTruckStatus.from(status); | ||
| foodTruckList = foodTruckRepository.findAllByFoodTruckStatus(foodTruckStatus); | ||
| } else { | ||
| foodTruckList = foodTruckRepository.findAll(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
문자열 검증 로직을 개선하세요.
isEmpty() 대신 isBlank()를 사용하여 공백 문자열도 처리하는 것이 더 안전합니다.
-if(status != null && !status.isEmpty()) {
+if(status != null && !status.isBlank()) {
FoodTruckStatus foodTruckStatus = FoodTruckStatus.from(status);📝 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.
| if(status != null && !status.isEmpty()) { | |
| FoodTruckStatus foodTruckStatus = FoodTruckStatus.from(status); | |
| foodTruckList = foodTruckRepository.findAllByFoodTruckStatus(foodTruckStatus); | |
| } else { | |
| foodTruckList = foodTruckRepository.findAll(); | |
| } | |
| if(status != null && !status.isBlank()) { | |
| FoodTruckStatus foodTruckStatus = FoodTruckStatus.from(status); | |
| foodTruckList = foodTruckRepository.findAllByFoodTruckStatus(foodTruckStatus); | |
| } else { | |
| foodTruckList = foodTruckRepository.findAll(); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/domain/user/application/admin/AdminService.java
around lines 36 to 41, the string validation uses status != null &&
!status.isEmpty(), which misses pure-whitespace values; change the condition to
status != null && !status.isBlank() so blank strings are treated as empty,
ensuring you use Java 11+ (or an equivalent utility) and update any unit tests
or callers if necessary.
| @@ -1,11 +1,13 @@ | |||
| package konkuk.chacall.domain.user.application; | |||
|
|
|||
| import konkuk.chacall.domain.foodtruck.presentation.dto.response.FoodTruckResponse; | |||
There was a problem hiding this comment.
🧩 Analysis chain
사용되지 않는 import인지 확인하세요.
FoodTruckResponse import가 추가되었지만 이 파일에서 사용되지 않는 것으로 보입니다. 실제로 사용되지 않는다면 제거하는 것이 좋습니다.
다음 스크립트로 사용 여부를 확인하세요:
🏁 Script executed:
#!/bin/bash
# FoodTruckResponse가 UserService.java에서 실제로 사용되는지 확인
rg -n "FoodTruckResponse" src/main/java/konkuk/chacall/domain/user/application/UserService.javaLength of output: 85
사용되지 않는 import 제거 필요
검증 결과 FoodTruckResponse는 파일 내에서 import 문 외에 다른 곳에서 사용되지 않습니다. 3번 줄의 import 문을 제거하세요.
import konkuk.chacall.domain.foodtruck.presentation.dto.response.FoodTruckResponse;
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/domain/user/application/UserService.java around
line 3, remove the unused import "import
konkuk.chacall.domain.foodtruck.presentation.dto.response.FoodTruckResponse;" —
delete that import line so the file no longer contains an unused dependency and
then run a quick build or IDE organize imports to ensure no other unused imports
remain.
| public static FoodTruckForAdminResponse from(FoodTruck foodTruck, List<String> foodTruckDocumentUrls) { | ||
| return new FoodTruckForAdminResponse( | ||
| foodTruck.getFoodTruckId(), | ||
| foodTruck.getFoodTruckInfo().getName(), | ||
| foodTruck.getOwner().getName(), | ||
| foodTruck.getFoodTruckStatus().getDescription(), | ||
| foodTruckDocumentUrls | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
팩토리 메서드에 null 안전성 검증을 추가하세요.
현재 구현은 여러 지점에서 NullPointerException 위험이 있습니다:
foodTruck파라미터 자체foodTruck.getFoodTruckInfo()반환값foodTruck.getOwner()반환값foodTruck.getFoodTruckStatus()반환값foodTruckDocumentUrls파라미터
관리자 승인 워크플로우에서 사용되는 코드이므로 런타임 안정성이 중요합니다.
다음과 같이 방어적 null 체크를 추가하는 것을 권장합니다:
public static FoodTruckForAdminResponse from(FoodTruck foodTruck, List<String> foodTruckDocumentUrls) {
+ if (foodTruck == null) {
+ throw new IllegalArgumentException("foodTruck cannot be null");
+ }
+ if (foodTruck.getFoodTruckInfo() == null || foodTruck.getOwner() == null || foodTruck.getFoodTruckStatus() == null) {
+ throw new IllegalStateException("FoodTruck has null required fields");
+ }
return new FoodTruckForAdminResponse(
foodTruck.getFoodTruckId(),
foodTruck.getFoodTruckInfo().getName(),
foodTruck.getOwner().getName(),
foodTruck.getFoodTruckStatus().getDescription(),
- foodTruckDocumentUrls
+ foodTruckDocumentUrls == null ? List.of() : foodTruckDocumentUrls
);
}또는, 도메인 모델이 null을 반환하지 않는다는 것이 보장된다면, 다음 스크립트로 FoodTruck 엔티티의 null 안전성 보장을 확인해주세요:
🏁 Script executed:
#!/bin/bash
# FoodTruck 엔티티와 관련 메서드의 null 안전성 확인
# FoodTruck 엔티티에서 @NotNull/@NonNull 어노테이션 확인
echo "=== FoodTruck 엔티티의 null 안전성 어노테이션 확인 ==="
fd -e java -x rg -n -A 2 -B 2 'class FoodTruck' {} \; | head -30
# getFoodTruckInfo, getOwner, getFoodTruckStatus 메서드 시그니처 확인
echo -e "\n=== getter 메서드 시그니처 확인 ==="
ast-grep --pattern $'getFoodTruckInfo() {
$$$
}'
ast-grep --pattern $'getOwner() {
$$$
}'
ast-grep --pattern $'getFoodTruckStatus() {
$$$
}'Length of output: 940
---
🏁 Script executed:
```shell
#!/bin/bash
# Find FoodTruck entity file
echo "=== FoodTruck 엔티티 파일 위치 찾기 ==="
fd -e java "FoodTruck.java" -type f
# Show full FoodTruck entity class definition
echo -e "\n=== FoodTruck 엔티티 전체 정의 ==="
fd -e java "FoodTruck.java" -type f -exec cat {} \;
# Check FoodTruckInfo, Owner, FoodTruckStatus for null annotations
echo -e "\n=== 관련 엔티티 파일 찾기 ==="
fd -e java -type f | xargs rg -l "class FoodTruckInfo|class.*Owner|class FoodTruckStatus" | head -10
Length of output: 1112
🏁 Script executed:
#!/bin/bash
# Find FoodTruck.java without type filter issues
echo "=== FoodTruck.java 파일 찾기 ==="
find . -name "FoodTruck.java" 2>/dev/null
# Show content of FoodTruck entity
echo -e "\n=== FoodTruck 엔티티 정의 ==="
find . -name "FoodTruck.java" -exec cat {} \; 2>/dev/null
# Find FoodTruckInfo entity
echo -e "\n=== FoodTruckInfo 엔티티 정의 ==="
find . -name "FoodTruckInfo.java" -exec cat {} \; 2>/dev/null
# Search for Owner entity
echo -e "\n=== Owner 엔티티 찾기 ==="
find . -path "*/domain/owner/domain/*.java" -name "*.java" | grep -i "owner" | head -5
find . -path "*/domain/owner/domain/*.java" -exec grep -l "class.*Owner.*Entity\|@Entity.*class.*Owner" {} \; 2>/dev/null | head -3Length of output: 8365
팩토리 메서드에 null 안전성 검증을 추가하세요.
도메인 모델 분석 결과 다음 지점에서 NullPointerException 위험이 있습니다:
foodTruckInfo:@Embedded필드이며 null-safety 어노테이션 없음 →getFoodTruckInfo().getName()호출 시 NPE 위험owner:@JoinColumn(nullable = false)DB 제약만 있고@NotNull어노테이션 없음 → 런타임 검증 필요foodTruckStatus:@Column(nullable = false)DB 제약만 있고@NotNull어노테이션 없음 → 런타임 검증 필요
관리자 승인 워크플로우의 중요한 메서드이므로 방어적 검증이 필수입니다.
다음과 같이 수정하세요:
public static FoodTruckForAdminResponse from(FoodTruck foodTruck, List<String> foodTruckDocumentUrls) {
+ Objects.requireNonNull(foodTruck, "foodTruck cannot be null");
+ Objects.requireNonNull(foodTruck.getFoodTruckInfo(), "foodTruckInfo cannot be null");
+ Objects.requireNonNull(foodTruck.getOwner(), "owner cannot be null");
+ Objects.requireNonNull(foodTruck.getFoodTruckStatus(), "foodTruckStatus cannot be null");
+
return new FoodTruckForAdminResponse(
foodTruck.getFoodTruckId(),
foodTruck.getFoodTruckInfo().getName(),
foodTruck.getOwner().getName(),
foodTruck.getFoodTruckStatus().getDescription(),
- foodTruckDocumentUrls
+ foodTruckDocumentUrls != null ? foodTruckDocumentUrls : Collections.emptyList()
);
}📝 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.
| public static FoodTruckForAdminResponse from(FoodTruck foodTruck, List<String> foodTruckDocumentUrls) { | |
| return new FoodTruckForAdminResponse( | |
| foodTruck.getFoodTruckId(), | |
| foodTruck.getFoodTruckInfo().getName(), | |
| foodTruck.getOwner().getName(), | |
| foodTruck.getFoodTruckStatus().getDescription(), | |
| foodTruckDocumentUrls | |
| ); | |
| } | |
| public static FoodTruckForAdminResponse from(FoodTruck foodTruck, List<String> foodTruckDocumentUrls) { | |
| Objects.requireNonNull(foodTruck, "foodTruck cannot be null"); | |
| Objects.requireNonNull(foodTruck.getFoodTruckInfo(), "foodTruckInfo cannot be null"); | |
| Objects.requireNonNull(foodTruck.getOwner(), "owner cannot be null"); | |
| Objects.requireNonNull(foodTruck.getFoodTruckStatus(), "foodTruckStatus cannot be null"); | |
| return new FoodTruckForAdminResponse( | |
| foodTruck.getFoodTruckId(), | |
| foodTruck.getFoodTruckInfo().getName(), | |
| foodTruck.getOwner().getName(), | |
| foodTruck.getFoodTruckStatus().getDescription(), | |
| foodTruckDocumentUrls != null ? foodTruckDocumentUrls : Collections.emptyList() | |
| ); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/user/presentation/dto/response/FoodTruckForAdminResponse.java
around lines 20 to 28, the factory method should defensively validate inputs to
avoid NullPointerException when accessing nested fields; add null checks for
foodTruck and for foodTruck.getFoodTruckInfo(), foodTruck.getOwner(), and
foodTruck.getFoodTruckStatus() before dereferencing them (and optionally for
foodTruckDocumentUrls), and if any are null throw a clear
IllegalArgumentException (or use Objects.requireNonNull with descriptive
messages) so the method fails fast with an informative error rather than NPE at
runtime.
src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/FoodTruckRepository.java (1)
54-55: 이전 리뷰 코멘트가 반영되어 N+1 문제가 해결되었습니다.
findAll()메서드에@EntityGraph를 추가하여 이전에 지적된 N+1 문제가 올바르게 해결되었습니다. 이제 status가 null일 때도 owner가 즉시 로딩되어 일관되게 N+1을 방지합니다.다만, 향후 푸드트럭 수가 많아질 경우를 대비해 페이지네이션 적용을 고려해볼 수 있습니다. 현재 관리자 승인 워크플로우에서는 문제없지만, 확장성 측면에서 개선 여지가 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/FoodTruckRepository.java(2 hunks)
🔇 Additional comments (1)
src/main/java/konkuk/chacall/domain/foodtruck/domain/repository/FoodTruckRepository.java (1)
51-52: N+1 문제 방지가 올바르게 적용되었습니다.
@EntityGraph를 사용하여 owner를 즉시 로딩함으로써 N+1 쿼리 문제를 효과적으로 방지하고 있습니다.
ksg1227
left a comment
ksg1227
left a comment
There was a problem hiding this comment.
수고하셨습니다~~ 역시 깔끔하게 잘 구현해주셔서 리뷰하기도 편하네요 굿굿
한 가지 여쭤보고싶은 점 남겨둘테니 확인 부탁드립니다~
src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java
Show resolved
Hide resolved
| @GetMapping("/admin/food-trucks") | ||
| public BaseResponse<List<FoodTruckForAdminResponse>> getAllFoodTrucks ( | ||
| @Parameter(hidden = true) @UserId final Long userId, | ||
| @Parameter(description = "푸드트럭 승인 상태 필터링 (APPROVED, PENDING, REJECTED) / 안 보낼 경우 전체", example = "APPROVED") |
There was a problem hiding this comment.
지금 와프상으로는 승인 상태 기반 필터링이 존재하지는 않는 상황인 것 같습니다. 이 부분은 확장을 여지에 둔 구현이라고 생각하면 될까요?
There was a problem hiding this comment.
넵 저는 관리자 입장에서 푸드트럭 승인 상태 정도는 필터링이 필요하다고 생각했습니다. 이 부분은 구현 후에 디자인/기획 측에 말씀드려야 할 것 같아요!
| Map<Long, List<FoodTruckDocument>> documentMap = foodTruckDocumentRepository.findAllInFoodTruckIds( | ||
| foodTruckList.stream() | ||
| .map(FoodTruck::getFoodTruckId) | ||
| .toList() | ||
| ).stream() | ||
| .collect( | ||
| Collectors.groupingBy(document -> document.getFoodTruck().getFoodTruckId()) | ||
| ); |
There was a problem hiding this comment.
stream 으로 처리를 깔끔하게 잘 하신 것 같습니다! 좋아요
다만 한가지 궁금한 점은, 지금 특정 푸드트럭에 대한 푸드트럭 서류를 조회하기 위해 별도의 쿼리를 날리는 상황이라고 이해되는데, 현재 운영자 - 푸드트럭 조회 로직에서는 페이징이 구현이 되어있지 않은 상황인 것 같습니다.
만약 페이징 로직이 존재하지 않고 모든 푸드트럭을 한 번에 반환하는 구조라면, FoodTruck 테이블을 조회하는 시점에 FoodTruckDocument 테이블까지 한 번에 조회해서 fetch join 으로 2번이 아닌 1번의 쿼리 만으로도 데이터를 조회해올 수 있을 것 같은데, 이 부분에 대한 현준님의 의견이 궁금합니다!
만약 페이징이 존재한다면 FoodTruck <-> FoodTruckDocument 가 OneToMany 관계를 갖고 있어서 fetch join 이 불가하지만, 현재처럼 페이징이 존재하지 않는다면 fetch join 이 가능할 것 같아서 여쭤봅니당
There was a problem hiding this comment.
상균님 말씀이 FoodTruck 엔티티에 OneToMany로 List<FoodTruckDocument> 양방향 연관관계를 걸자는 말씀 맞나요??
그게 맞다면, 그렇게 진행하는게 더 좋을 것 같긴하네요!
There was a problem hiding this comment.
아 지금 양방향이 안걸려있구나.. 그럼 좀 애매하긴 하네요.
일단 다른 엔티티들도 다 양방향 안걸린 상태로 유지중이니까 통일성 맞추도록 현재 구조로 가도 좋을 것 같습니다.
사실상 성능 차이는 거의 없다고 봐도 될 것 같아서요!
There was a problem hiding this comment.
네넵 그러면 일단 현재 코드 유지하고 머지하겠습니다~
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/konkuk/chacall/domain/user/presentation/UserController.java(4 hunks)
🔇 Additional comments (1)
src/main/java/konkuk/chacall/domain/user/presentation/UserController.java (1)
10-10: 필요한 import가 올바르게 추가되었습니다.새로운 엔드포인트의 반환 타입을 지원하기 위한 import 문들이 적절하게 추가되었습니다.
Also applies to: 21-21
src/main/java/konkuk/chacall/domain/user/presentation/UserController.java
Show resolved
Hide resolved
…-foodtruck # Conflicts: # src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java # src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/konkuk/chacall/domain/user/presentation/UserController.java (2)
94-95: API 파라미터를 enum 이름 기반으로 변경하는 것을 권장합니다.현재 한글 문자열("승인 대기", "승인 완료", "승인 거부")을 사용하고 있으나, 이전 리뷰에서도 지적되었듯이 REST API 설계 관점에서 enum 이름(
PENDING,APPROVED,REJECTED)을 사용하는 것이 더 표준적이고 국제화에 유리합니다.FoodTruckStatus.from()메서드를 enum name 기반으로 변경하는 것을 고려해보세요.다음과 같이 수정할 수 있습니다:
- @Parameter(description = "푸드트럭 승인 상태 필터링 (승인 대기, 승인 완료, 승인 거부)", example = "승인 대기") + @Parameter(description = "푸드트럭 승인 상태 필터링", schema = @Schema(implementation = FoodTruckStatus.class)) @RequestParam(required = false) final String status그리고
FoodTruckStatus.from()메서드를 다음과 같이 변경:public static FoodTruckStatus from(String name) { try { return FoodTruckStatus.valueOf(name); } catch (IllegalArgumentException e) { throw new CustomException(ErrorCode.FOOD_TRUCK_STATUS_MISMATCH); } }
86-98: 관리자 전용 컨트롤러 분리를 고려하세요.현재 관리자 전용 엔드포인트(
/admin/*)가UserController에 위치해 있습니다. 관리자 기능이 확장될수록 컨트롤러가 비대해질 수 있으므로, 별도의AdminController를 생성하여 관리자 관련 엔드포인트를 분리하는 것을 고려해보세요. 서비스 레이어에서는 이미AdminService가 존재하는 것으로 보이므로, 컨트롤러 레이어에서도 동일한 분리를 적용하면 유지보수성이 향상됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/konkuk/chacall/domain/user/presentation/UserController.java(4 hunks)src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java
🔇 Additional comments (4)
src/main/java/konkuk/chacall/domain/user/presentation/UserController.java (3)
10-10: LGTM!새 엔드포인트에 필요한 임포트가 적절히 추가되었습니다.
Also applies to: 21-21
59-59: LGTM!기존 승인 상태 변경 API 설명에 "[운영자용]" 접두사를 추가하여 일관성과 명확성이 개선되었습니다.
86-98: 관리자 권한 검증이 올바르게 구현되어 있습니다.
UserService.getAllFoodTrucks()메서드 내에서adminValidator.validateAdmin(userId)를 통해 관리자 권한을 검증하고 있습니다(71줄). PR 설명과 일치하게 관리자 권한 검증이 서비스 레이어의 첫 번째 단계로 구현되어 있으므로 별도의 수정이 필요하지 않습니다.src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java (1)
86-87: 리뷰 의견이 코드 변경 사항과 맞지 않습니다git 이력 검증 결과, 제시된 우려 사항은 실제 변경 사항과 일치하지 않습니다.
FOOD_TRUCK_NOT_VIEWABLE은 기존 코드 110010에서 110011로 변경된 것이 아닙니다- 이전 커밋에
FOOD_TRUCK_NOT_VIEWABLE이 존재하지 않으므로, 110011은 새로 추가되는 에러 코드입니다FOOD_TRUCK_STATUS_MISMATCH(110010)는 기존 에러 코드를 재사용하는 것이 아닙니다- 현재 변경사항은 순차적으로 새로운 에러 코드(110010, 110011)를 추가하는 것일 뿐입니다
따라서 에러 코드 번호 재사용으로 인한 호환성 문제가 없으며, 이 리뷰 의견은 무시해도 됩니다.
Likely an incorrect or invalid review comment.
2 participants
#️⃣연관된 이슈
📝작업 내용
푸드트럭 승인을 위한 관리자가 푸드트럭 목록을 조회하기 위한 API 입니다.
-> 총 3번의 쿼리로 호출되도록 처리하였습니다.
스크린샷
💬리뷰 요구사항(선택)
Summary by CodeRabbit
새로운 기능
개선사항
문서