Conversation
Walkthrough관리자 툴 목록에 정렬 파라미터(criteria, direction)와 size 기본값 변경을 추가하고, 툴 색상 필드(bgColor, fontColor)를 제거했으며, 회원 탈퇴 시 게시판의 사용자 연관을 해제하도록 변경했습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant AdminController
participant AdminService
participant ToolRepository
Note over AdminController,AdminService: 툴 목록 정렬 파라미터(criteria, direction) 추가
Client->>AdminController: GET /admin/tools?page=&size=&criteria=&direction=
AdminController->>AdminService: fetchAllTool(criteria, direction, page, size)
AdminService->>AdminService: dir = Sort.Direction.valueOf(direction)\npageable = PageRequest.of(page,size, Sort.by(dir, criteria))
AdminService->>ToolRepository: findAll(pageable)
ToolRepository-->>AdminService: Page<Tool>
AdminService-->>AdminController: AdminToolPageRes
AdminController-->>Client: 200 OK (paged, sorted list)
sequenceDiagram
autonumber
participant Client
participant AuthService
participant BoardRepository
participant DB
Note over AuthService,BoardRepository: 회원 탈퇴 시 게시판 사용자 연관 해제
Client->>AuthService: DELETE /users/{id}
AuthService->>BoardRepository: clearUser(userEntity)
BoardRepository->>DB: UPDATE board SET user_id=0, updated_at=CURRENT_TIMESTAMP WHERE user_id=:id
DB-->>BoardRepository: updateCount
BoardRepository-->>AuthService: void
AuthService-->>Client: 200 OK (연관 해제 완료)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
Test Results110 tests 110 ✅ 4s ⏱️ Results for commit c0a409c. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/main/java/com/daruda/darudaserver/domain/admin/service/AdminService.java (1)
9-9: 정렬 파라미터(direction/criteria)에 대한 유효성 검증을 추가하는 편이 안전해 보입니다.현재 구현도 기능적으로는 동작하지만, 아래 두 케이스에서 런타임 예외로 500이 떨어질 수 있습니다.
direction에ASC/DESC외 값이 들어오는 경우:Sort.Direction.fromString(direction)에서IllegalArgumentException발생 가능.criteria에 존재하지 않는 필드명이 들어오는 경우:Sort.by(dir, criteria)이후 쿼리 생성 단계에서 예외 발생 가능.관리자 전용 API라 해도, 잘못된 파라미터는 400(Bad Request) 계열로 명확히 응답해 주는 것이 운영·디버깅 측면에서 더 좋습니다.
제안 옵션 예시입니다:
- 컨트롤러에서
direction을String대신Sort.Direction타입으로 받고(스프링 컨버전 사용), Swagger 쪽에는 허용 값(ASC/DESC)을 명시.criteria는 사전에 합의된 필드 목록(예:createdAt,toolMainName등)을 화이트리스트로 두고, 목록에 없으면 400 또는 기본 정렬(createdAt DESC)로 fallback.- 서비스에서
- 현재처럼
String으로 받되, 내부에서"ASC".equalsIgnoreCase(direction)/"DESC"외는 default 로 처리하는 간단한 분기와,criteria화이트리스트 체크를 추가하고, 위반 시 도메인용BadRequest예외로 매핑.QA 피드백에서 정렬 기능의 안정성이 중요하다면, 이 부분을 한 번만 정리해 두면 이후에도 에러 추적이 훨씬 쉬울 것 같습니다.
Also applies to: 367-373
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java (1)
39-42: 정렬 파라미터 값 제약(문서/검증)을 조금 더 명확히 하면 좋겠습니다.
direction을String으로 받고 예시로만ASC/DESC를 안내하고 있어서, 클라이언트 오타 시 서비스단에서 예외 → 500 으로 이어질 수 있습니다.criteria도 마찬가지로, 실제 허용되는 정렬 필드 목록(예:createdAt,toolMainName등)을 API 문서/검증 레벨에서 명시해 두면 사용성이 좋아집니다.고려해볼 수 있는 보완 방법들입니다:
- 컨트롤러 시그니처에서
@RequestParam Sort.Direction direction으로 변경해 스프링이 잘못된 값에 대해 400을 내려주도록 맡기기.- Swagger
@Parameter에schema의allowableValues등을 활용해ASC,DESC만 선택 가능하게 명시.criteria에 대해서도
- 허용 값 목록을 enum이나 상수 리스트로 정의하고,
@RequestParam을 해당 enum 타입으로 받거나,- 최소한 Swagger 문서에 "허용 값 리스트"를 구체적으로 기재.
size기본값을 10으로 줄인 부분은 제약(1~100)과도 잘 맞고, 페이지 네이션 UX 측면에서도 무난해 보입니다.Also applies to: 47-48, 50-50
src/main/java/com/daruda/darudaserver/domain/tool/entity/Tool.java (1)
97-104: LGTM!upperMainName 메서드가 중간 변수 없이 단일 return 문으로 간소화되어 가독성이 개선되었습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java(1 hunks)src/main/java/com/daruda/darudaserver/domain/admin/service/AdminService.java(2 hunks)src/main/java/com/daruda/darudaserver/domain/community/repository/BoardRepository.java(2 hunks)src/main/java/com/daruda/darudaserver/domain/search/document/ToolDocument.java(0 hunks)src/main/java/com/daruda/darudaserver/domain/search/dto/response/ToolSearchResponse.java(2 hunks)src/main/java/com/daruda/darudaserver/domain/tool/dto/res/ToolDetailGetRes.java(1 hunks)src/main/java/com/daruda/darudaserver/domain/tool/dto/res/ToolResponse.java(1 hunks)src/main/java/com/daruda/darudaserver/domain/tool/entity/Tool.java(2 hunks)src/main/java/com/daruda/darudaserver/domain/tool/service/ToolService.java(1 hunks)src/main/java/com/daruda/darudaserver/domain/user/service/AuthService.java(1 hunks)src/main/resources/logback-spring.xml(1 hunks)src/test/java/com/daruda/darudaserver/domain/user/service/UserServiceTest.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/daruda/darudaserver/domain/search/document/ToolDocument.java
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: yechan-kim
Repo: Tool-daruda/35-APPJAM-SERVER-DARUDA PR: 326
File: src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java:35-45
Timestamp: 2025-10-27T00:05:28.639Z
Learning: In the Tool-daruda/35-APPJAM-SERVER-DARUDA repository, yechan-kim prefers to implement admin authentication using role-based authorization in SecurityConfig (e.g., .hasRole("ADMIN")) with JWT roles, rather than using AuthenticationPrincipal Long userId parameters in controller methods. This centralizes authorization logic in the security configuration.
📚 Learning: 2025-05-07T11:50:09.221Z
Learnt from: yechan-kim
Repo: Tool-daruda/35-APPJAM-SERVER-DARUDA PR: 272
File: src/main/java/com/daruda/darudaserver/domain/user/service/AuthService.java:83-113
Timestamp: 2025-05-07T11:50:09.221Z
Learning: `commentRepository.deleteAllByUserId(userId)`는 사용자가 작성한 댓글을 삭제하고, `commentRepository.deleteByBoardId(board.getId())`는 사용자가 작성한 글에 달린 댓글을 삭제한다. 이 두 로직은 서로 다른 댓글 집합을 대상으로 하여 중복 호출이 아니다.
Applied to files:
src/main/java/com/daruda/darudaserver/domain/community/repository/BoardRepository.javasrc/main/java/com/daruda/darudaserver/domain/user/service/AuthService.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test-result
🔇 Additional comments (9)
src/main/resources/logback-spring.xml (1)
15-17: 로깅 구성 파일 변경이 PR 범위 밖입니다.이 파일의 변경사항은 PR #337의 목표(Tool 엔티티에서 미사용 필드 제거, 관리자 API 정렬 기능 추가, 사용자 탈퇴 시 게시물 작성자 변경)와 무관합니다. Logback 설정은 로깅 포매팅에만 영향을 미치므로, 이 변경사항이 의도된 것인지 확인해 주시기 바랍니다.
XML 포매팅 자체는 기능에 영향을 주지 않지만, PR의 범위를 명확히 하기 위해 검토가 필요합니다.
src/main/java/com/daruda/darudaserver/domain/community/repository/BoardRepository.java (1)
47-48: 타임스탬프 업데이트 추가를 확인했습니다.
clearTool쿼리에updated_at설정이 추가되어clearUser와 일관성을 유지합니다.src/test/java/com/daruda/darudaserver/domain/user/service/UserServiceTest.java (1)
107-108: LGTM!Tool.of 호출이 새로운 시그니처(bgColor, fontColor 제거)에 맞게 올바르게 업데이트되었습니다.
src/main/java/com/daruda/darudaserver/domain/tool/dto/res/ToolResponse.java (1)
10-29: LGTM!bgColor와 fontColor 필드가 깔끔하게 제거되었고, 빌더 메서드도 올바르게 업데이트되었습니다.
src/main/java/com/daruda/darudaserver/domain/tool/service/ToolService.java (1)
91-91: LGTM!ToolDetailGetRes.of 호출에서 toolLogo 매개변수가 제거되었습니다. DTO 내부에서 Tool 객체로부터 직접 logo를 추출하도록 리팩토링되어 더 깔끔한 API가 되었습니다.
src/main/java/com/daruda/darudaserver/domain/user/service/AuthService.java (1)
118-118: 게시글 처리 방식 변경을 확인하세요.게시글 삭제 방식이
deleteAllByUserId에서clearUser로 변경되어, 이제 게시글을 삭제하지 않고 작성자만 삭제된 사용자 플레이스홀더로 변경합니다. 이는 PR 목표에 명시된 대로 제품 요구사항을 반영한 것으로 보입니다.변경사항이 의도한 대로 동작하는지 확인해주세요.
src/main/java/com/daruda/darudaserver/domain/tool/entity/Tool.java (1)
72-86: LGTM!Tool.of 팩토리 메서드에서 bgColor와 fontColor 매개변수가 깔끔하게 제거되었고, 빌더도 올바르게 업데이트되었습니다.
src/main/java/com/daruda/darudaserver/domain/search/dto/response/ToolSearchResponse.java (1)
7-26: LGTM!ToolSearchResponse에서 bgColor와 fontColor 필드가 깔끔하게 제거되었고, from 팩토리 메서드도 올바르게 업데이트되었습니다.
src/main/java/com/daruda/darudaserver/domain/tool/dto/res/ToolDetailGetRes.java (1)
31-51: LGTM!ToolDetailGetRes.of 팩토리 메서드에서 toolLogo 매개변수가 제거되었습니다. 이제 Tool 객체로부터 직접 logo를 추출하도록 변경되어 API가 더 간결해졌습니다. bgColor와 fontColor 필드도 제거되어 미사용 필드 정리가 완료되었습니다.
src/main/java/com/daruda/darudaserver/domain/community/repository/BoardRepository.java
Show resolved
Hide resolved
src/main/java/com/daruda/darudaserver/domain/user/service/AuthService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/daruda/darudaserver/domain/admin/service/AdminService.java (2)
81-88: createTool에서 videos 필드 null 시 NPE 가능성이 있습니다List<ToolVideo> toolVideo = createToolRequest.videos().stream() .map(url -> ToolVideo.builder() .videoUrl(url) .tool(savedTool) .build()) .toList();
CreateToolRequest에서videos는 필수/검증 애노테이션이 없어서, JSON에서 필드를 생략하면 기본값이null이 됩니다.- 이 상태에서 위 로직이 호출되면
createToolRequest.videos()가 null이라NullPointerException이 발생합니다.- 다른 리스트 필드들(키워드, 이미지, 블로그 링크, cores, relatedToolIds 등)은 모두
null체크 후 처리하고 있어 일관성 측면에서도 예외입니다.간단히 다음 중 하나로 보완하는 게 좋겠습니다.
- DTO 쪽에서
videos에@NotNull을 붙여 필수로 만들거나,- 여기서도 다른 필드와 동일하게
if (videos != null && !videos.isEmpty()) { ... }가드를 추가.현재는 관리자 툴 생성 시 영상 링크를 안 보내는 요청이 들어오면 바로 500이 터질 수 있는 구조라, 이 PR에서 같이 정리해 두는 걸 추천드립니다.
- //ToolVideo 가공 - List<ToolVideo> toolVideo = createToolRequest.videos().stream() - .map(url -> ToolVideo.builder() - .videoUrl(url) - .tool(savedTool) - .build()) - .toList(); - - toolVideoRepository.saveAll(toolVideo); + // ToolVideo 가공 + List<String> videoUrls = createToolRequest.videos(); + if (videoUrls != null && !videoUrls.isEmpty()) { + List<ToolVideo> toolVideo = videoUrls.stream() + .filter(url -> url != null && !url.isBlank()) + .map(url -> ToolVideo.builder() + .videoUrl(url.trim()) + .tool(savedTool) + .build()) + .toList(); + if (!toolVideo.isEmpty()) { + toolVideoRepository.saveAll(toolVideo); + } + }
62-77:createTool에서 planType과 videos에 대한 null 방어가 부재하여 NPE 발생 위험웹 검색으로 확인한 결과, Java Enum 파싱 메서드에서 null이 입력되면 NPE가 발생합니다. 현재 코드에서 두 가지 임박한 문제가 있습니다:
planType null 처리 부재 (라인 76)
- CreateToolRequest의 planType은
requiredMode = NOT_REQUIRED로 null 가능PlanType.formString(createToolRequest.planType())에 null이 들어오면planType.getType().equals(type)실행 시 NullPointerException 발생- updateTool(라인 197)에서는
req.planType() != null체크가 있으나, createTool에는 없어 불일치videos null 처리 부재 (라인 82)
- CreateToolRequest의 videos도 null 가능 (NON_NULL JsonInclude)
createToolRequest.videos().stream()호출 시 videos가 null이면 NullPointerException 발생개선 방안:
- 라인 76:
createToolRequest.planType() != null ? PlanType.formString(createToolRequest.planType()) : [기본값]로 수정- 라인 82:
if (createToolRequest.videos() != null) { ... }로 감싸기- 또는 PlanType.formString 메서드 자체에 null 체크 추가 및 명확한 예외 처리
♻️ Duplicate comments (1)
src/main/java/com/daruda/darudaserver/domain/community/repository/BoardRepository.java (1)
16-16: id=0 플레이스홀더 전략 정리/자동화 권장
clearTool/clearUser모두tool_id = 0,user_id = 0으로 업데이트하는 방식이라, 운영 상으로는 반드시 id=0인 Tool/User를 모든 환경에서 일관되게 생성해 둬야 합니다. 이전 논의에서 말씀 주신 것처럼 수동 생성으로는 새 환경 구축 시 빠뜨리기 쉽기 때문에,
- Flyway/Liquibase 또는
data.sql등을 통해 id=0 시스템 계정을 자동으로 생성하거나- 최소한 README/운영 문서에 “id=0 플레이스홀더 유저/툴 생성” 절차를 명시
해 두는 쪽을 추천드립니다. 현재 PR의 비즈니스 로직 자체는 요구사항(작성자/툴을 ‘삭제된’ 플레이스홀더로 치환)에 맞게 잘 구현되어 있습니다.
Also applies to: 47-48, 50-53
🧹 Nitpick comments (4)
src/main/java/com/daruda/darudaserver/domain/search/dto/response/ToolSearchResponse.java (1)
14-25: 색상 필드 제거는 적절하지만 isScraped / isScrapped 네이밍 일관성이 아쉽습니다색상 관련 필드를 제거하고
isScraped만 남기는 구조는 단순하고 명확해서 좋아 보입니다. 다만,
- 레코드 필드명:
boolean isScraped- 팩토리 메서드 파라미터 및 인자:
boolean isScrapped처럼 철자가 서로 달라서(단일 p vs. 이중 p) 코드 읽을 때 혼란을 줄 수 있습니다. 기능적으로 문제는 없지만, 도메인 용어에 맞춰 둘 중 하나로 통일해 두면 추후 유지보수 시 더 안전할 것 같습니다.
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java (2)
55-81: 툴 생성/수정 Operation 설명이 매우 친절하지만 planType 표기 방식은 한 번 정리해 두면 좋겠습니다
createTool/updateTool의 Operation 설명에서:
- 상단 가이드에는
FREE,MONTHLY,PURCHASE,MONTHLY_ANNUAL같은 영문 enum 이름을 사용하고,- 예시 영역에서는
planType="무료","월간","구매","월간 & 연간"처럼 한글 값을 사용하고 있습니다.- DTO 쪽
@Schema(allowableValues = {"무료", "월간", "구매", "월간 & 연간"})와도 한글 값 기준으로 맞춰져 있습니다.실제 클라이언트가 보내야 하는 값이:
- 진짜 한글 문자열인지,
- enum 이름(영문)인지,
정책이 모호해질 수 있으니, 둘 중 하나로 통일하고 다른 하나는 설명용으로만 쓰는 식으로 정리해 두면 좋겠습니다. 예를 들어:
- “실제 값:
"무료"|"월간"|"구매"|"월간 & 연간"(내부적으로 FREE/MONTHLY/PURCHASE/MONTHLY_ANNUAL로 매핑)”처럼 명시.현재 코드 동작에는 영향 없고, 문서 가독성/혼란 방지 차원의 제안입니다.
39-48:direction과criteria파라미터에 Swagger 제약값 지정을 권장합니다Springdoc에서
@Parameter내schema.allowableValues를 지정하면 FE/테스터 오입력을 방지할 수 있습니다.제안 예시:
@Parameter(description = "정렬 순서 (ASC/DESC)", schema = @Schema(allowableValues = {"ASC", "DESC"})) @RequestParam(defaultValue = "DESC", value = "direction") String direction, @Parameter(description = "정렬 기준", schema = @Schema(allowableValues = {"createdAt", "updatedAt", "toolMainName"})) @RequestParam(defaultValue = "createdAt", value = "criteria") String criteria,Swagger UI는 enum 값을 드롭다운/선택 UI로 표시하여 유효한 값만 입력하도록 유도합니다. 서비스 레벨의 검증/화이트리스트와 함께 적용하면 클라이언트 측 500 에러를 줄일 수 있습니다.
src/main/java/com/daruda/darudaserver/domain/admin/service/AdminService.java (1)
367-373: 정렬 파라미터 검증 부재로 인한 500 에러 발생 가능성 – 입력 검증 또는 예외 핸들러 추가 권장현재 코드는
direction과criteria파라미터에 대한 검증이 없으며, 잘못된 값이 전달되면 다음과 같이 500 에러가 반환됩니다:
direction이"ASC"/"DESC"가 아닌 값(예:"invalid")으로 들어오면Sort.Direction.fromString()에서IllegalArgumentException이 발생하고, 현재GlobalExceptionHandler는 이를 구체적으로 처리하지 않아 제네릭Exception핸들러로 떨어져 500 응답을 반환합니다.criteria가 Tool 엔티티에 없는 필드명(예:"nonExistentField")으로 들어오면 Spring Data JPA의 쿼리 생성 시PropertyReferenceException또는IllegalArgumentException이 발생하며, 동일하게 500 에러가 됩니다.클라이언트 오류(잘못된 요청 파라미터)는 400 Bad Request로 응답하는 것이 REST API 관례입니다.
권장 해결책:
GlobalExceptionHandler에PropertyReferenceException과IllegalArgumentException을 구체적으로 처리하는 핸들러 추가 후 400 응답 반환, 또는- 서비스 메서드에서 허용된 필드 집합(예:
createdAt,updatedAt,toolMainName,toolSubName,viewCount,popular등 Tool 엔티티의 정렬 가능 필드)에 대한 화이트리스트 검증 추가Controller의 Swagger 문서(
createdAt기본값,ASC/DESC정렬 순서)와 실제 허용 가능한 정렬 필드 목록을 명확히 정의하여 API 계약을 강화하면 좋겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java(1 hunks)src/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolPlanRequest.java(1 hunks)src/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolRequest.java(2 hunks)src/main/java/com/daruda/darudaserver/domain/admin/dto/request/UpdateToolRequest.java(2 hunks)src/main/java/com/daruda/darudaserver/domain/admin/service/AdminService.java(2 hunks)src/main/java/com/daruda/darudaserver/domain/community/repository/BoardRepository.java(2 hunks)src/main/java/com/daruda/darudaserver/domain/search/document/ToolDocument.java(0 hunks)src/main/java/com/daruda/darudaserver/domain/search/dto/response/ToolSearchResponse.java(2 hunks)src/main/java/com/daruda/darudaserver/domain/tool/dto/res/ToolDetailGetRes.java(1 hunks)src/main/java/com/daruda/darudaserver/domain/tool/dto/res/ToolResponse.java(1 hunks)src/main/java/com/daruda/darudaserver/domain/tool/entity/Tool.java(2 hunks)src/main/java/com/daruda/darudaserver/domain/tool/service/ToolService.java(1 hunks)src/main/java/com/daruda/darudaserver/domain/user/service/AuthService.java(1 hunks)src/test/java/com/daruda/darudaserver/domain/user/service/AuthServiceTest.java(1 hunks)src/test/java/com/daruda/darudaserver/domain/user/service/UserServiceTest.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/daruda/darudaserver/domain/search/document/ToolDocument.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/daruda/darudaserver/domain/tool/service/ToolService.java
- src/main/java/com/daruda/darudaserver/domain/tool/dto/res/ToolResponse.java
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: yechan-kim
Repo: Tool-daruda/35-APPJAM-SERVER-DARUDA PR: 326
File: src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java:35-45
Timestamp: 2025-10-27T00:05:28.639Z
Learning: In the Tool-daruda/35-APPJAM-SERVER-DARUDA repository, yechan-kim prefers to implement admin authentication using role-based authorization in SecurityConfig (e.g., .hasRole("ADMIN")) with JWT roles, rather than using AuthenticationPrincipal Long userId parameters in controller methods. This centralizes authorization logic in the security configuration.
📚 Learning: 2025-05-07T11:50:09.221Z
Learnt from: yechan-kim
Repo: Tool-daruda/35-APPJAM-SERVER-DARUDA PR: 272
File: src/main/java/com/daruda/darudaserver/domain/user/service/AuthService.java:83-113
Timestamp: 2025-05-07T11:50:09.221Z
Learning: `commentRepository.deleteAllByUserId(userId)`는 사용자가 작성한 댓글을 삭제하고, `commentRepository.deleteByBoardId(board.getId())`는 사용자가 작성한 글에 달린 댓글을 삭제한다. 이 두 로직은 서로 다른 댓글 집합을 대상으로 하여 중복 호출이 아니다.
Applied to files:
src/main/java/com/daruda/darudaserver/domain/community/repository/BoardRepository.javasrc/test/java/com/daruda/darudaserver/domain/user/service/AuthServiceTest.javasrc/main/java/com/daruda/darudaserver/domain/user/service/AuthService.java
📚 Learning: 2025-10-27T00:05:28.639Z
Learnt from: yechan-kim
Repo: Tool-daruda/35-APPJAM-SERVER-DARUDA PR: 326
File: src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java:35-45
Timestamp: 2025-10-27T00:05:28.639Z
Learning: In the Tool-daruda/35-APPJAM-SERVER-DARUDA repository, yechan-kim prefers to implement admin authentication using role-based authorization in SecurityConfig (e.g., .hasRole("ADMIN")) with JWT roles, rather than using AuthenticationPrincipal Long userId parameters in controller methods. This centralizes authorization logic in the security configuration.
Applied to files:
src/main/java/com/daruda/darudaserver/domain/community/repository/BoardRepository.javasrc/main/java/com/daruda/darudaserver/domain/user/service/AuthService.java
📚 Learning: 2025-04-27T16:48:46.761Z
Learnt from: yechan-kim
Repo: Tool-daruda/35-APPJAM-SERVER-DARUDA PR: 271
File: src/main/java/com/daruda/darudaserver/domain/user/service/AuthService.java:82-112
Timestamp: 2025-04-27T16:48:46.761Z
Learning: AuthService의 withdraw 메서드는 여러 엔티티를 삭제하는 작업이므로 Transactional 어노테이션으로 묶어 데이터 정합성을 유지해야 합니다.
Applied to files:
src/main/java/com/daruda/darudaserver/domain/user/service/AuthService.java
📚 Learning: 2025-05-20T16:07:30.556Z
Learnt from: yechan-kim
Repo: Tool-daruda/35-APPJAM-SERVER-DARUDA PR: 276
File: src/main/java/com/daruda/darudaserver/domain/notification/dto/request/NoticeRequest.java:0-0
Timestamp: 2025-05-20T16:07:30.556Z
Learning: NoticeRequest와 같은 DTO 클래스에는 NotBlank와 Size와 같은 Bean Validation 애노테이션을 사용하여 입력값 유효성 검증을 구현합니다. 제목과 내용에 대한 최대 길이 값은 정책에 따라 조정될 수 있습니다.
Applied to files:
src/main/java/com/daruda/darudaserver/domain/admin/dto/request/UpdateToolRequest.javasrc/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolPlanRequest.java
🔇 Additional comments (7)
src/test/java/com/daruda/darudaserver/domain/user/service/AuthServiceTest.java (1)
193-217: 회원 탈퇴 시 게시글 작성자 clear 동작을 잘 검증하고 있습니다
withdrawSuccess테스트가deleteAllByUserId대신boardRepository.clearUser(mockUser)호출을 검증하도록 변경되어, “게시글은 유지하되 작성자만 탈퇴 사용자로 치환”이라는 현재 비즈니스 요구사항과 잘 맞습니다. 나머지 연관 데이터 삭제 검증도 그대로 유지되어 있어 전체 withdraw 플로우를 잘 커버하고 있습니다.src/main/java/com/daruda/darudaserver/domain/user/service/AuthService.java (1)
97-132: 게시글 삭제 → 작성자 연결 해제로의 전환이 요구사항에 잘 맞습니다
withdraw내에서 게시글을 실제 삭제하지 않고boardRepository.clearUser(userEntity)로 작성자만 분리하고, 로그도 이에 맞게"board의 사용자 연결을 성공적으로 해제하였습니다"로 수정하신 부분이 현재 기획(탈퇴 후에도 게시글은 노출, 작성자는 ‘탈퇴한 사용자’로 표시)에 정확히 맞습니다. 메서드가 그대로@Transactional하에 여러 삭제/해제 작업을 묶고 있는 점도 이전 설계 방향과 일관됩니다. (Based on learnings)src/test/java/com/daruda/darudaserver/domain/user/service/UserServiceTest.java (1)
107-110: Tool.of 시그니처 변경 반영 이상 없음
Tool.of호출부가 엔티티의 새로운 시그니처(색상 필드 제거 후)와 정확히 일치해서 테스트가 현재 도메인 모델과 잘 동기화되어 있습니다. 테스트 가독성/의미도 그대로 유지됩니다.src/main/java/com/daruda/darudaserver/domain/tool/dto/res/ToolDetailGetRes.java (1)
31-52: ToolDetailGetRes.of 정리 방향 적절함정적 팩토리에서 더 이상
toolLogo를 인자로 받지 않고Tool엔티티에서 직접 꺼내도록 정리한 점, 그리고 제거된 색상 필드들을 전혀 참조하지 않는 점이 일관성 있게 잘 반영되어 있습니다.keywords / images / videos / isScrapped매핑도 필드명 기준으로 정확합니다.src/main/java/com/daruda/darudaserver/domain/tool/entity/Tool.java (2)
72-87: Tool.of 정적 팩토리에서 불필요 필드 제거 정리 👍
Tool.of시그니처에서 색상 필드를 제거하고, 실제 컬럼으로 존재하는 필수 속성들만 인자로 받도록 정리된 부분이 도메인 모델과 잘 맞습니다. 빌더가 엔티티 필드 전체를 빠짐없이 채우고 있어 생성 시 누락/불일치 문제도 없어 보입니다.
97-104: upperMainName 단순화 변경 무관한 동작 유지
upperMainName가 이전과 동일한 로직을 보다 간결한 return 한 줄로 표현하고 있어 가독성이 좋아졌습니다. 입력이 비어 있지 않다는 전제가 기존부터 동일하게 유지되는 한, 동작 변화는 없습니다.src/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolPlanRequest.java (1)
10-32: 플랜 DTO에 Bean Validation을 추가한 구현은 합리적이며, 기존 API 문서와도 일치합니다@NotBlank/@NotNull/@PositiveOrZero 제약은 JSON 바인딩 시 ConstraintViolation을 생성하여 HTTP 400 Bad Request를 유발합니다.
이 DTO가 사용되는 패턴을 확인한 결과:
- CreateToolRequest와 UpdateToolRequest 양쪽에서
List<CreateToolPlanRequest> plans로 재사용되며, 두 시나리오 모두 현재 코드로도 처리됨- AdminController의 주석 (라인 77, 108)에 이미 명시:
"planName, planPrice, planDescription은 모두 필수입니다"- UpdateToolRequest의 주석 (라인 62)에도 동일 명시:
"planName, planPrice, planDescription이 null인 플랜은 저장되지 않습니다"- 서비스 레이어 (Plan.create, AdminService)에서 planDescription을 null 처리 로직 없이 직접 할당 중
즉, Bean Validation 애노테이션 추가는 기존에 문서상/로직상으로 이미 필수였던 정책을 명시적으로 강제하는 것이므로, 원래의 "기존 호출자가 null을 보냈을 때 처리되던 로직"은 코드상 존재하지 않습니다. 이미 API 문서상으로 필수라 했으므로, 이번 변경은 미준수 요청을 조기에 거부하는 체계적 개선입니다.
src/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolRequest.java
Show resolved
Hide resolved
src/main/java/com/daruda/darudaserver/domain/admin/dto/request/UpdateToolRequest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/daruda/darudaserver/domain/community/repository/BoardRepository.java (1)
16-16:clearUser로직이 기획 의도(탈퇴 후 게시글 유지)에 잘 맞습니다
- 회원 탈퇴 시 게시글을 삭제하지 않고,
user_id = 0으로 일괄 업데이트하여 “삭제된 사용자”로 치환하는 구조가 PR 설명 및 이전 논의와 잘 맞습니다.UserEntity를 직접 받아:#{#userEntity.id}로 바인딩하는 방식도 타입 안정성과 가독성 면에서 괜찮습니다.- 다만 이전 리뷰에서 이야기된 것처럼, id=0 시스템 유저가 모든 환경에서 일관되게 생성되도록 마이그레이션/시드 또는 문서화로 관리해 두면 운영 시 혼선을 줄일 수 있습니다.
이미 한 번 논의된 주제라 필수 변경 사항은 아니고, 나중에 인프라/DB 세팅 정리할 때 함께 봐 주시면 될 것 같습니다.
Also applies to: 55-63
🧹 Nitpick comments (1)
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java (1)
39-50: 정렬 파라미터에 대한 타입/검증 추가를 고려해 주세요
criteria,direction파라미터 추가로 관리자 툴 목록 정렬이 가능해진 점은 좋습니다. 다만:
direction이 단순String이라"ASC"/"DESC"외 값이나 대소문자 혼합 값이 들어오면, 서비스 계층에서 정렬 방향 파싱 시 런타임 예외가 발생할 수 있습니다.criteria도 허용되는 필드 목록이 명시되어 있지 않으면, 잘못된 필드명으로 정렬 요청 시 JPA 쿼리 에러가 날 수 있습니다.Spring Data를 사용하고 있다면, 예를 들어 다음처럼 enum 타입으로 받는 방식을 고려해 볼 수 있습니다.
+import org.springframework.data.domain.Sort; ... - @Parameter(description = "정렬 순서 (ASC/DESC)", example = "DESC") - @RequestParam(defaultValue = "DESC", value = "direction") String direction, + @Parameter(description = "정렬 순서", example = "DESC") + @RequestParam(defaultValue = "DESC", value = "direction") Sort.Direction direction,그리고
AdminService.fetchAllTool시그니처 및 내부 구현도 이에 맞게Sort.Direction을 사용하도록 정리하면, 유효하지 않은 값에 대한 방어와 문서화가 자연스럽게 되겠습니다.criteria쪽도 description에 “허용 값” 목록을 간단히 적거나, enum/화이트리스트로 제한하는 방식을 한 번 검토해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java(1 hunks)src/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolRequest.java(2 hunks)src/main/java/com/daruda/darudaserver/domain/admin/dto/request/UpdateToolRequest.java(2 hunks)src/main/java/com/daruda/darudaserver/domain/community/repository/BoardRepository.java(2 hunks)src/main/java/com/daruda/darudaserver/global/auth/jwt/service/TokenService.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/daruda/darudaserver/global/auth/jwt/service/TokenService.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolRequest.java
- src/main/java/com/daruda/darudaserver/domain/admin/dto/request/UpdateToolRequest.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-07T11:50:09.221Z
Learnt from: yechan-kim
Repo: Tool-daruda/35-APPJAM-SERVER-DARUDA PR: 272
File: src/main/java/com/daruda/darudaserver/domain/user/service/AuthService.java:83-113
Timestamp: 2025-05-07T11:50:09.221Z
Learning: `commentRepository.deleteAllByUserId(userId)`는 사용자가 작성한 댓글을 삭제하고, `commentRepository.deleteByBoardId(board.getId())`는 사용자가 작성한 글에 달린 댓글을 삭제한다. 이 두 로직은 서로 다른 댓글 집합을 대상으로 하여 중복 호출이 아니다.
Applied to files:
src/main/java/com/daruda/darudaserver/domain/community/repository/BoardRepository.java
📚 Learning: 2025-10-27T00:05:28.639Z
Learnt from: yechan-kim
Repo: Tool-daruda/35-APPJAM-SERVER-DARUDA PR: 326
File: src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java:35-45
Timestamp: 2025-10-27T00:05:28.639Z
Learning: In the Tool-daruda/35-APPJAM-SERVER-DARUDA repository, yechan-kim prefers to implement admin authentication using role-based authorization in SecurityConfig (e.g., .hasRole("ADMIN")) with JWT roles, rather than using AuthenticationPrincipal Long userId parameters in controller methods. This centralizes authorization logic in the security configuration.
Applied to files:
src/main/java/com/daruda/darudaserver/domain/community/repository/BoardRepository.java
🔇 Additional comments (2)
src/main/java/com/daruda/darudaserver/domain/community/repository/BoardRepository.java (1)
47-52:clearTool쿼리 리포맷 및updated_at갱신 방향 좋습니다기존 동작은 유지하면서 텍스트 블록으로 가독성을 높이고
updated_at = CURRENT_TIMESTAMP를 함께 갱신하도록 한 부분이 깔끔합니다. 네이티브 UPDATE +@Modifying(clearAutomatically = true, flushAutomatically = true)조합도 적절해 보입니다.src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java (1)
97-119: plans 필드 동작 설명이 구현과 정확히 일치하는지 한 번 더 점검해 주세요
plans에 대해null=변경 없음,[] = 전체 삭제, 배열 전달 시 “기존 삭제 후 교체” 규칙을 상세히 문서화해 둔 점이 매우 좋습니다. 다만 이 규칙이 서비스/도메인 로직과 100% 일치하지 않으면, 관리자가 일부만 수정한다고 생각했다가 의도치 않게 전체가 삭제/교체되는 등 데이터 손실 이슈가 생길 수 있습니다.
AdminService.updateTool및 관련 도메인 로직이 실제로 문서에 적힌 규칙:
plans전송 시 기존 플랜 전체 삭제 후 새 목록으로 교체plans=null이면 플랜 변경 없음plans=[]이면 전체 플랜 삭제을 그대로 따르는지 한 번만 더 확인 부탁드립니다. 구현과 문서가 이미 일치한다면, 현재 설명 수준이면 충분해 보입니다.
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java (1)
55-84: planType 예시 값 불일치 및 들여쓰기 문제이전 리뷰에서 지적된 planType 불일치 문제가 여전히 남아있습니다. Lines 63-66에서는
FREE,MONTHLY,PURCHASE,MONTHLY_ANNUAL같은 영문 enum 값을 언급하고 있지만, lines 68-77의 예시에서는planType="무료","월간","구매","월간 & 연간"처럼 한글 문자열을 사용하고 있습니다.추가로 line 62의 들여쓰기가 일관되지 않습니다 (탭이 하나 부족).
다음 diff를 적용하여 문제를 수정하세요:
- - planType과 plans는 함께 사용됩니다: + - planType과 plans는 함께 사용됩니다: * FREE: 무료 플랜인 경우 plans는 빈 배열이거나 무료 플랜 1개만 포함 * MONTHLY: 월간 구독 플랜들 (예: Basic, Pro, Enterprise 등) * PURCHASE: 일회성 구매 플랜들 (예: Starter, Professional 등) * MONTHLY_ANNUAL: 월간 & 연간 플랜 혼합 (예: 월간 Basic, 연간 Basic 등) **플랜 케이스 예시:** - 1. 무료 툴: planType="무료", plans=[] - 또는 plans=[{planName:"무료", planPrice:0, planDescription:"기본 기능 무료 제공"}] - 2. 월간 구독: planType="월간", + 1. 무료 툴: planType="FREE" (무료), plans=[] + 또는 plans=[{planName:"Free", planPrice:0, planDescription:"기본 기능 무료 제공"}] + 2. 월간 구독: planType="MONTHLY" (월간), plans=[{planName:"Basic", planPrice:10000, ...}, {planName:"Pro", planPrice:30000, ...}] - 3. 일회성 구매: planType="구매", + 3. 일회성 구매: planType="PURCHASE" (구매), plans=[{planName:"Starter", planPrice:50000, ...}, {planName:"Professional", planPrice:150000, ...}] - 4. 혼합: planType="월간 & 연간", + 4. 혼합: planType="MONTHLY_ANNUAL" (월간 & 연간), plans=[{planName:"Basic 월간", planPrice:10000, ...}, {planName:"Basic 연간", planPrice:100000, ...}]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java(1 hunks)src/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolRequest.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolRequest.java
🔇 Additional comments (2)
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java (2)
86-86: 문서화 개선이 적절합니다파라미터 설명에 플랜 정보가 포함됨을 명시하여 API 문서의 명확성이 향상되었습니다.
94-117: 플랜 수정 가이드가 명확하고 상세합니다플랜 필드 수정 시의 동작 방식(교체, 유지, 삭제)과 각 케이스별 예시가 잘 작성되어 있습니다. API 사용자가 플랜 수정 로직을 명확히 이해할 수 있도록 도와줍니다.
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java (1)
39-42: 정렬 파라미터 문서화 개선을 고려해보세요.새로운 정렬 기능이 잘 추가되었습니다. 다만
criteria파라미터의@Parameter설명에 유효한 정렬 기준 필드 목록을 명시하면 API 사용자에게 더 도움이 될 것 같습니다.예시:
@Parameter(description = "정렬 기준 (createdAt, updatedAt, toolMainName, viewCount, popular 중 선택)", example = "createdAt")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java(2 hunks)src/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolRequest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolRequest.java
🔇 Additional comments (2)
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java (2)
60-86: 플랜 사용 가이드 문서화가 명확하고 유용합니다.플랜 필드의 사용 방법, 케이스별 예시, 주의사항이 상세하게 작성되어 API 사용자가 이해하기 쉽습니다. 특히 다양한 플랜 타입별 예시(무료, 월간, 구매, 혼합)를 제공한 점이 좋습니다.
102-117: 플랜 수정 동작에 대한 설명이 명확합니다.플랜 필드의 수정 시맨틱스(교체 방식)와 null/빈 배열의 동작 차이를 명확히 설명했습니다. 특히 Line 117의 "기존 플랜을 유지하면서 일부만 수정하려면, 유지할 플랜도 포함하여 전체 플랜 목록을 다시 전송해야 합니다" 안내는 데이터 손실을 방지하는 데 중요한 정보입니다.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolRequest.java (1)
43-50:planType를 REQUIRED로 문서화했으면 Bean Validation까지 함께 맞추는 편이 안전합니다
planType에requiredMode = Schema.RequiredMode.REQUIRED를 주셔서 문서 상으로는 필수값이 되었는데, 런타임에서는 여전히 Bean Validation 제약이 없어(null/blank 가능)AdminService쪽에서 그대로 사용하면 여전히 NPE/IllegalArgumentException가능성이 남아 있습니다. 이전 리뷰에서 말씀드렸던 “문서/서비스/검증” 정렬 이슈의 연장선이라, 이번에 정리해 두면 좋겠습니다. Based on learnings.
- 정말 필수값이라면:
- DTO에
@NotBlank(또는 최소한@NotNull)를 붙여서 검증 단계에서 400으로 막거나,- 서비스에서 null/blank에 대한 방어 로직을 추가해 주세요.
예시(필수로 가져가는 방향):
-import jakarta.validation.Valid; -import jakarta.validation.constraints.Size; +import jakarta.validation.Valid; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Size; @@ @Schema( description = "플랜 유형", example = "무료", allowableValues = {"무료", "월간", "구매", "월간 & 연간"}, requiredMode = Schema.RequiredMode.REQUIRED ) - String planType, + @NotBlank(message = "플랜 유형(planType)은 필수입니다.") + String planType,이렇게 하면 Swagger, 비즈니스 로직, Bean Validation이 한 방향으로 맞춰져서 QA/운영 시 예측 가능성이 좋아집니다.
src/main/java/com/daruda/darudaserver/domain/admin/dto/request/UpdateToolRequest.java (1)
52-70:plans설명 문구가 Bean Validation 실제 동작과 아직 맞지 않습니다 (이전 리뷰 내용 재언급)
plans는@Valid List<CreateToolPlanRequest>이고,CreateToolPlanRequest쪽에planName/planPrice/planDescription에 대한@NotBlank,@NotNull등의 제약이 걸려 있는 것으로 알고 있습니다. 이 경우:
- 필드가 null/공백인 플랜이 하나라도 포함되면 요청 전체가 Bean Validation에서 실패 → HTTP 400이 됩니다.
- 그런데 설명에는
- planName, planPrice, planDescription이 null인 플랜은 저장되지 않습니다 (모두 필수)라고 되어 있어, 마치 서비스에서 “필터링해서 안 저장”하는 것처럼 읽힙니다. 이는 실제 동작(검증 실패로 400)과 다릅니다. Based on learnings.
정책에 따라 아래 둘 중 하나로 정리하시는 게 좋아 보입니다.
현재 구현 유지(필드 모두 필수)
→ 설명을 예를 들어
planName, planPrice, planDescription이 null 또는 공백인 플랜이 포함된 요청은 유효성 검사 실패로 HTTP 400을 반환합니다 (모두 필수)
정도로 바꿔 실제 Bean Validation 동작을 명시.문서대로 “불완전한 플랜은 무시”를 원하신다면
→ Update 전용 DTO에서 해당 필드 제약을 완화하거나, List 요소에 대한@Valid/제약 구성을 다르게 가져가야 합니다.이전 코멘트에서 한번 짚었던 부분인데, 이번에 텍스트 블록을 손보신 김에 실제 동작 기준으로 문구를 정리해 두시면 더 좋겠습니다.
🧹 Nitpick comments (2)
src/main/java/com/daruda/darudaserver/domain/tool/entity/Tool.java (1)
97-104:upperMainName리팩터링은 OK, null/빈 문자열 방어 로직은 선택적으로 고려
return한 줄로 정리한 리팩터링은 동작 영향 없이 잘 정리되었습니다.다만 현재 구현은
toolMainName이null이거나 빈 문자열일 경우charAt(0)에서 NPE /StringIndexOutOfBoundsException이 발생할 수 있습니다. 상위 계층에서 항상 유효한 값이 들어온다는 것이 보장된다면 그대로 두셔도 되지만, 방어적으로 가고 싶다면 아래와 같이 가드를 넣는 것도 방법입니다.public String upperMainName(String toolMainName) { - //첫 글자가 대문자이면 그대로 return - if (Character.isUpperCase(toolMainName.charAt(0))) { + // null 또는 빈 문자열이면 그대로 반환 + if (toolMainName == null || toolMainName.isEmpty()) { + return toolMainName; + } + //첫 글자가 대문자이면 그대로 return + if (Character.isUpperCase(toolMainName.charAt(0))) { return toolMainName; } //첫 글자가 대문자가 아닌 경우에만 대문자로 변경 return toolMainName.substring(0, 1).toUpperCase() + toolMainName.substring(1); }현재 PR 범위에서 필수 변경은 아니고, 입력값 보장 수준에 따라 선택적으로 적용하셔도 될 것 같습니다.
src/main/java/com/daruda/darudaserver/domain/admin/dto/request/UpdateToolRequest.java (1)
44-50: Update 시planType를 REQUIRED로 바꾸신 의도와 실제 동작을 한 번 더 맞춰 보면 좋겠습니다이제
UpdateToolRequest.planType도 Swagger 상requiredMode = REQUIRED로 표시되어 수정 시에도 항상 planType을 보내야 하는 것처럼 문서화되었습니다. 그런데:
- 필드 타입은
String planType이고,- Bean Validation 제약(
@NotBlank,@NotNull)이 없어 실제로는 null/blank가 그대로 컨트롤러까지 들어올 수 있습니다.- 서비스 쪽 로직이
null을 “미변경”으로 해석하는지, 아니면 무조건PlanType.fromString()처럼 사용하는지에 따라 실제 동작이 달라질 수 있습니다.정책에 따라 정리가 필요해 보입니다.
update에서도 planType을 항상 요구한다면:
- DTO에
@NotBlank등을 추가해 검증 단계에서 400으로 막고,- Swagger REQUIRED 설정과 런타임 검증을 일치시키는 것이 좋습니다.
**planType이 선택값(보내지 않으면 기존 유지)**이라면:
requiredMode를 다시NOT_REQUIRED로 돌리고,description에 “null 또는 미포함 시 기존 플랜 유형 유지” 같은 문구를 추가해 주세요.어느 쪽이 실제 비즈니스 정책인지 한 번만 더 확인하신 뒤, DTO/Swagger/서비스 로직을 한 방향으로 맞춰 두시면 이후 QA에서 헷갈릴 여지가 줄어듭니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolRequest.java(1 hunks)src/main/java/com/daruda/darudaserver/domain/admin/dto/request/UpdateToolRequest.java(1 hunks)src/main/java/com/daruda/darudaserver/domain/tool/entity/Tool.java(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-27T08:53:05.770Z
Learnt from: yechan-kim
Repo: Tool-daruda/35-APPJAM-SERVER-DARUDA PR: 340
File: src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java:55-87
Timestamp: 2025-11-27T08:53:05.770Z
Learning: In `src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java`, the `planType` field accepts Korean string values ("무료", "월간", "구매", "월간 & 연간") from the API request and internally converts them to enum values (FREE, MONTHLY, PURCHASE, MONTHLY_ANNUAL). The API documentation should show the Korean strings that clients need to send, not the internal enum names.
Applied to files:
src/main/java/com/daruda/darudaserver/domain/admin/dto/request/UpdateToolRequest.javasrc/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolRequest.java
📚 Learning: 2025-05-20T16:07:30.556Z
Learnt from: yechan-kim
Repo: Tool-daruda/35-APPJAM-SERVER-DARUDA PR: 276
File: src/main/java/com/daruda/darudaserver/domain/notification/dto/request/NoticeRequest.java:0-0
Timestamp: 2025-05-20T16:07:30.556Z
Learning: NoticeRequest와 같은 DTO 클래스에는 NotBlank와 Size와 같은 Bean Validation 애노테이션을 사용하여 입력값 유효성 검증을 구현합니다. 제목과 내용에 대한 최대 길이 값은 정책에 따라 조정될 수 있습니다.
Applied to files:
src/main/java/com/daruda/darudaserver/domain/admin/dto/request/UpdateToolRequest.javasrc/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolRequest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test-result
src/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolRequest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/daruda/darudaserver/domain/tool/entity/Tool.java (1)
98-105: 빈 문자열 처리가 누락되었습니다.
toolMainName이 빈 문자열일 경우charAt(0)에서StringIndexOutOfBoundsException이 발생합니다. 엔티티에서nullable=false로 설정되어 있지만, 빈 문자열은 허용될 수 있습니다.public String upperMainName(String toolMainName) { + if (toolMainName == null || toolMainName.isEmpty()) { + return toolMainName; + } //첫 글자가 대문자이면 그대로 return if (Character.isUpperCase(toolMainName.charAt(0))) { return toolMainName; } //첫 글자가 대문자가 아닌 경우에만 대문자로 변경 return toolMainName.substring(0, 1).toUpperCase() + toolMainName.substring(1); }
♻️ Duplicate comments (1)
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java (1)
55-87: API 문서화가 적절하게 작성되었습니다.
planType필드가 한글 문자열("무료", "월간", "구매", "월간 & 연간")을 받아 내부적으로 enum으로 변환하는 구조임을 이전 리뷰에서 확인했습니다. 문서와 예시가 이 동작에 맞게 일관성 있게 작성되었습니다. Based on learnings.
🧹 Nitpick comments (1)
src/main/java/com/daruda/darudaserver/domain/admin/service/AdminService.java (1)
355-361:criteria파라미터에 대한 검증 추가를 권장합니다.
direction은Sort.Direction.fromString()에서 유효성이 검증되지만,criteria는 검증 없이Sort.by()에 직접 전달됩니다. 잘못된 필드명이 전달되면PropertyReferenceException이 발생합니다.허용된 정렬 필드를 화이트리스트로 관리하는 것을 권장합니다:
+private static final Set<String> ALLOWED_SORT_FIELDS = Set.of( + "createdAt", "updatedAt", "toolMainName", "viewCount", "popular" +); + public AdminToolPageRes fetchAllTool(String criteria, String direction, int page, int size) { + if (!ALLOWED_SORT_FIELDS.contains(criteria)) { + throw new IllegalArgumentException("Invalid sort criteria: " + criteria); + } Sort.Direction dir = Sort.Direction.fromString(direction); Pageable pageable = PageRequest.of(page, size, Sort.by(dir, criteria));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java(3 hunks)src/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolRequest.java(1 hunks)src/main/java/com/daruda/darudaserver/domain/admin/service/AdminService.java(3 hunks)src/main/java/com/daruda/darudaserver/domain/tool/entity/Tool.java(3 hunks)src/test/java/com/daruda/darudaserver/domain/user/service/UserServiceTest.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/daruda/darudaserver/domain/admin/dto/request/CreateToolRequest.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-27T08:53:05.770Z
Learnt from: yechan-kim
Repo: Tool-daruda/35-APPJAM-SERVER-DARUDA PR: 340
File: src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java:55-87
Timestamp: 2025-11-27T08:53:05.770Z
Learning: In `src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java`, the `planType` field accepts Korean string values ("무료", "월간", "구매", "월간 & 연간") from the API request and internally converts them to enum values (FREE, MONTHLY, PURCHASE, MONTHLY_ANNUAL). The API documentation should show the Korean strings that clients need to send, not the internal enum names.
Applied to files:
src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test-result
🔇 Additional comments (5)
src/test/java/com/daruda/darudaserver/domain/user/service/UserServiceTest.java (1)
107-121: LGTM! Tool 엔티티 변경에 맞게 테스트가 올바르게 수정되었습니다.
Tool.of()대신Tool.builder()를 사용하여planType필드를 포함하도록 변경된 것은Tool엔티티의planType필드가nullable = false로 변경된 것과 일관됩니다.src/main/java/com/daruda/darudaserver/domain/admin/service/AdminService.java (1)
65-65: LGTM!Tool.from()팩토리 메서드 사용이 적절합니다.인라인 빌더 대신
Tool.from(CreateToolRequest)를 사용하여 Tool 생성 로직을 엔티티 내부로 캡슐화한 것은 코드 가독성과 유지보수성을 향상시킵니다.src/main/java/com/daruda/darudaserver/domain/tool/entity/Tool.java (2)
71-72: LGTM!planType필드의 non-nullable 설정이 적절합니다.
planType이nullable = false로 변경되었으며, 새로운Tool.from()팩토리 메서드가 이 필드를 올바르게 설정합니다. 이전 리뷰에서 지적된Tool.of()관련 문제가 해결되었습니다.
86-86:formStringis the correct method name—not a typo.The PlanType enum at
src/main/java/com/daruda/darudaserver/domain/tool/entity/PlanType.javaline 16 defines the method aspublic static PlanType formString(String type). WhilefromStringis a more conventional naming pattern in Java,formStringis the intentional implementation in this codebase. The code at line 86 in Tool.java is correct.src/main/java/com/daruda/darudaserver/domain/admin/controller/AdminController.java (1)
39-51: LGTM! 정렬 파라미터가 올바르게 추가되었습니다.
criteria와direction파라미터를 통해 관리자용 툴 목록의 정렬 기능이 적절하게 구현되었습니다.size기본값을 10으로 변경한 것도 합리적입니다.
2 participants
📣 Related Issue
📝 Summary
🙏 Question & PR point
📬 Postman
툴 생성
툴 목록 조회 (관리자)
회원 탈퇴
Summary by CodeRabbit
새로운 기능
개선 사항
테스트
✏️ Tip: You can customize this high-level summary in your review settings.