대시보드_최근_목록_클릭_기능_추가#555
Open
Cassiiopeia wants to merge 2 commits intomainfrom
Hidden character warning
The head ref may contain hidden characters: "20260227_#552_\ub300\uc2dc\ubcf4\ub4dc_\ucd5c\uadfc_\ubaa9\ub85d_\ud074\ub9ad_\uae30\ub2a5_\ucd94\uac00"
Open
Conversation
Walkthrough관리자 API 문서화 인터페이스와 엔드포인트가 추가되고, AdminResponse DTO에 단일 Item/Member 필드가 도입되며, 서비스·레포지토리에 관리자용 상세 조회 메서드가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin User
participant Ctrl as AdminApiController
participant Docs as AdminApiControllerDocs
participant SvcItem as ItemService
participant SvcMember as MemberService
participant Repo as ItemRepository
participant DB as Database
participant Resp as AdminResponse
Admin->>Ctrl: POST /items/detail (AdminRequest)
Ctrl->>Docs: (Docs interface metadata)
Ctrl->>SvcItem: getItemDetailForAdmin(request)
SvcItem->>Repo: findByItemIdWithDetails(itemId)
Repo->>DB: SELECT with LEFT JOIN FETCH
DB-->>Repo: Item + Member + Images
Repo-->>SvcItem: Optional<Item>
SvcItem->>Resp: build AdminResponse with item
Resp-->>Ctrl: AdminResponse
Ctrl-->>Admin: 200 ResponseEntity<AdminResponse>
Admin->>Ctrl: POST /members/detail (AdminRequest)
Ctrl->>SvcMember: getMemberDetailForAdmin(request)
SvcMember->>DB: Query member by ID
DB-->>SvcMember: Member
SvcMember->>Resp: build AdminResponse with member
Resp-->>Ctrl: AdminResponse
Ctrl-->>Admin: 200 ResponseEntity<AdminResponse>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
RomRom-Web/src/main/java/com/romrom/web/controller/api/AdminApiController.java (3)
109-119:⚠️ Potential issue | 🟠 Major대시보드 통계:
totalMembers/totalItems에 “active count”를 넣는지 확인이 필요합니다.
getDashboardStats()에서dashboardStats.totalMembers에memberService.countActiveMembers()(Line 115)를,dashboardStats.totalItems에itemService.countActiveItems()(Line 116)를 세팅하고 있습니다. 이름 그대로라면 “전체”와 “활성”이 섞여 보이므로, 의도한 지표가 무엇인지 맞춰주는 게 좋겠습니다(필드명을 activeMembers/activeItems로 쓰거나, 서비스 메서드/리포지토리 쿼리를 total 기준으로 맞추거나).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@RomRom-Web/src/main/java/com/romrom/web/controller/api/AdminApiController.java` around lines 109 - 119, The dashboard stats currently assign active counts to fields named totalMembers and totalItems in getDashboardStats (AdminResponse.AdminDashboardStats), causing a naming/semantic mismatch; either change the fields to activeMembers/activeItems in AdminResponse.AdminDashboardStats and the builder calls in getDashboardStats, or call total-count service methods (e.g., memberService.countAllMembers()/itemService.countAllItems) so totalMembers/totalItems truly represent totals—update the builder usage in getDashboardStats and the AdminResponse model consistently to resolve the mismatch.
75-105:⚠️ Potential issue | 🟡 Minor
/logout의consumes=MULTIPART_FORM_DATA는 빈 바디 요청에서 415 오류를 유발합니다.
/logout엔드포인트(Line 76)는@PostMapping(..., consumes = MediaType.MULTIPART_FORM_DATA_VALUE)로 지정되어 있지만, 요청 바디가 없습니다. 표준 클라이언트는 보통 logout을 빈 바디로 호출하므로, Content-Type이 multipart가 아니면 Spring이 415를 반환합니다./login과 달리 logout은@ModelAttribute같은 바디 파라미터가 없어서consumes제약이 불필요합니다.
consumes를 제거하거나 클라이언트가 항상 multipart로 호출하도록 강제하는 중 하나로 정리하세요.- 추가로
adminAuthService.logout(...)이 실패해도 쿠키 삭제는 계속 진행되는 것이 UX상 좋으므로, try/finally 처리를 검토하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@RomRom-Web/src/main/java/com/romrom/web/controller/api/AdminApiController.java` around lines 75 - 105, Remove the unnecessary consumes constraint from the `@PostMapping` on AdminApiController.logout (i.e., delete the consumes = MediaType.MULTIPART_FORM_DATA_VALUE) so the endpoint accepts empty-body requests; then wrap the call to adminAuthService.logout(refreshTokenFromCookie) in a try/finally so that the cookie deletion logic (creating/setting accessToken, refreshToken, authStatus cookies with path "/" and maxAge 0 and adding them to the response) always runs even if adminAuthService.logout throws.
33-73:⚠️ Potential issue | 🟠 Major관리자 로그인: 토큰 쿠키의
Secure=false고정 + SameSite 미설정은 운영 보안 리스크입니다.현재 access/refresh token 쿠키에
setSecure(false)가 하드코딩되어 있습니다(45, 53줄). 운영 환경이 HTTPS라면 네트워크 상에서 토큰이 평문으로 노출될 수 있습니다. 또한SameSite속성이 명시되지 않아 CSRF 관점에서 브라우저 기본값에 의존하게 됩니다(관리자 기능은 영향이 큼).
- 환경별로
secure값을 true/false로 주입(프로파일/설정값)하고, SameSite도 명시해야 합니다.- Spring 3.4.1은
ResponseCookie를 지원하므로 이를 활용하면 SameSite까지 함께 안전하게 설정할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@RomRom-Web/src/main/java/com/romrom/web/controller/api/AdminApiController.java` around lines 33 - 73, The login method currently hardcodes cookie secure flags and omits SameSite, which is unsafe; modify AdminApiController.login to read an injected config/property (e.g., a boolean like cookieSecure or environment/profile flag) and use Spring's ResponseCookie builder instead of java.net.Cookie to set accessToken, refreshToken and authStatus cookies so you can call .secure(cookieSecure).sameSite("Strict" or "Lax" as appropriate) and other attributes (path, httpOnly, maxAge) before adding them to the HttpServletResponse (use response.addHeader("Set-Cookie", responseCookie.toString()) or ResponseCookie.toString()); remove the hardcoded setSecure(false) calls on accessTokenCookie/refreshTokenCookie/authStatusCookie and ensure adminAuthService.authenticateWithJwt still supplies tokens unchanged.RomRom-Common/src/main/java/com/romrom/common/dto/AdminResponse.java (1)
16-53:⚠️ Potential issue | 🟠 Major클래스 레벨
@ToString으로 인한 토큰/민감 정보 로그 노출 위험
AdminResponse의@ToString어노테이션(16줄)이accessToken,refreshToken,item,member필드를 포함하여 문자열화합니다. 이 객체를 로그로 출력하면 인증 토큰과 사용자 PII가 그대로 노출됩니다. 내부 클래스인AdminItemDto,AdminMemberDto,AdminDashboardStats도 동일하게@ToString이 적용되어 있어 연쇄 노출 위험이 있습니다.민감 필드에
@ToString.Exclude를 추가하여 로그에서 제외하세요:
accessToken,refreshTokenitem,member(엔티티 필드는 ID나 기본 정보만 필요한 경우가 많음)예시 적용
+import lombok.ToString; `@ToString` public class AdminResponse { + `@ToString.Exclude` private String accessToken; + `@ToString.Exclude` private String refreshToken; + `@ToString.Exclude` private Item item; + `@ToString.Exclude` private Member member;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@RomRom-Common/src/main/java/com/romrom/common/dto/AdminResponse.java` around lines 16 - 53, The class-level `@ToString` on AdminResponse causes sensitive data exposure; remove or limit it by adding Lombok `@ToString.Exclude` to the sensitive fields accessToken, refreshToken, item, and member in AdminResponse (or replace the class-level `@ToString` with a safe, custom toString), and apply the same treatment to nested DTOs AdminItemDto, AdminMemberDto, and AdminDashboardStats (either remove their `@ToString` or add `@ToString.Exclude` to fields containing PII/tokens) so tokens and personal data are excluded from logged string representations.RomRom-Web/src/main/java/com/romrom/web/controller/api/AdminApiControllerDocs.java (1)
17-295:⚠️ Potential issue | 🟡 Minor문서 컨벤션(
issueNumber) 정리와login에러 응답 형식을 맞춰주세요.
issueNumber = 0이 여러 곳에 들어가 있는데(Line 23, 47, 67, 82, 104, 164, 193, 249, 273), PR#552패턴처럼 GitHub Issue 번호를 쓸지 결정 필요합니다.login문서에서INVALID_CREDENTIALS에러코드를 명시했으나(Line 40-42), 실제 구현은 실패 시 HTTP 400 + 빈 바디만 반환합니다(AdminApiController 69-71줄). 문서를 "400 + 빈 바디"로 수정하거나 컨트롤러가 표준 에러 응답을 내려주도록 변경하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@RomRom-Web/src/main/java/com/romrom/web/controller/api/AdminApiControllerDocs.java` around lines 17 - 295, The documentation has inconsistent issueNumber usage and a mismatch between the login docs and the controller behavior; update AdminApiControllerDocs and/or the controller to make them consistent: (1) Replace repeated ApiChangeLog(issueNumber = 0) entries in AdminApiControllerDocs with the correct PR/issue number (e.g., 552) for the entries that correspond to PR `#552` (refer to the ApiChangeLog annotations in AdminApiControllerDocs) or decide a consistent convention and apply it across all ApiChangeLog annotations; (2) For the login mismatch, either update the login Operation description in AdminApiControllerDocs to state the actual behavior ("returns HTTP 400 with empty body on invalid credentials") or change the AdminApiController.login implementation to return a standard error response containing the INVALID_CREDENTIALS error payload (make the controller return the structured error instead of plain 400 empty body) so that AdminApiControllerDocs.login and AdminApiController.login are aligned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Around line 16-26: Update the "POST API 패턴 (전체 공통)" section to explicitly
state exceptions (e.g., read-only endpoints using GET and destructive endpoints
using DELETE) and mention that controllers like AdminApiController expose GET
/dashboard/*, GET /items, GET /members and DELETE /items/{itemId} as allowed
deviations from the POST+multipart+@ModelAttribute rule; also fix the missing
fenced-code-block language by adding the appropriate language tag (e.g., ```md)
to the fenced block in the documentation so markdownlint MD040 is satisfied.
In
`@RomRom-Domain-Member/src/main/java/com/romrom/member/service/MemberService.java`:
- Around line 311-326: Ensure AdminRequest.memberId is validated and remove
duplicate lookup: add a `@NotNull` on AdminRequest.memberId and make the
controller getMemberDetail endpoint use `@Valid` or `@Validated` so null memberId
yields a 400; then change MemberService.getMemberDetailForAdmin(AdminRequest) to
call the existing findMemberById(UUID memberId) helper (or explicitly null-check
request.getMemberId() and throw a 400-mapped CustomException) instead of calling
memberRepository.findById(...) inline, reusing the centralized error handling in
findMemberById and eliminating the duplicated lookup/logging.
---
Outside diff comments:
In `@RomRom-Common/src/main/java/com/romrom/common/dto/AdminResponse.java`:
- Around line 16-53: The class-level `@ToString` on AdminResponse causes sensitive
data exposure; remove or limit it by adding Lombok `@ToString.Exclude` to the
sensitive fields accessToken, refreshToken, item, and member in AdminResponse
(or replace the class-level `@ToString` with a safe, custom toString), and apply
the same treatment to nested DTOs AdminItemDto, AdminMemberDto, and
AdminDashboardStats (either remove their `@ToString` or add `@ToString.Exclude` to
fields containing PII/tokens) so tokens and personal data are excluded from
logged string representations.
In
`@RomRom-Web/src/main/java/com/romrom/web/controller/api/AdminApiController.java`:
- Around line 109-119: The dashboard stats currently assign active counts to
fields named totalMembers and totalItems in getDashboardStats
(AdminResponse.AdminDashboardStats), causing a naming/semantic mismatch; either
change the fields to activeMembers/activeItems in
AdminResponse.AdminDashboardStats and the builder calls in getDashboardStats, or
call total-count service methods (e.g.,
memberService.countAllMembers()/itemService.countAllItems) so
totalMembers/totalItems truly represent totals—update the builder usage in
getDashboardStats and the AdminResponse model consistently to resolve the
mismatch.
- Around line 75-105: Remove the unnecessary consumes constraint from the
`@PostMapping` on AdminApiController.logout (i.e., delete the consumes =
MediaType.MULTIPART_FORM_DATA_VALUE) so the endpoint accepts empty-body
requests; then wrap the call to adminAuthService.logout(refreshTokenFromCookie)
in a try/finally so that the cookie deletion logic (creating/setting
accessToken, refreshToken, authStatus cookies with path "/" and maxAge 0 and
adding them to the response) always runs even if adminAuthService.logout throws.
- Around line 33-73: The login method currently hardcodes cookie secure flags
and omits SameSite, which is unsafe; modify AdminApiController.login to read an
injected config/property (e.g., a boolean like cookieSecure or
environment/profile flag) and use Spring's ResponseCookie builder instead of
java.net.Cookie to set accessToken, refreshToken and authStatus cookies so you
can call .secure(cookieSecure).sameSite("Strict" or "Lax" as appropriate) and
other attributes (path, httpOnly, maxAge) before adding them to the
HttpServletResponse (use response.addHeader("Set-Cookie",
responseCookie.toString()) or ResponseCookie.toString()); remove the hardcoded
setSecure(false) calls on accessTokenCookie/refreshTokenCookie/authStatusCookie
and ensure adminAuthService.authenticateWithJwt still supplies tokens unchanged.
In
`@RomRom-Web/src/main/java/com/romrom/web/controller/api/AdminApiControllerDocs.java`:
- Around line 17-295: The documentation has inconsistent issueNumber usage and a
mismatch between the login docs and the controller behavior; update
AdminApiControllerDocs and/or the controller to make them consistent: (1)
Replace repeated ApiChangeLog(issueNumber = 0) entries in AdminApiControllerDocs
with the correct PR/issue number (e.g., 552) for the entries that correspond to
PR `#552` (refer to the ApiChangeLog annotations in AdminApiControllerDocs) or
decide a consistent convention and apply it across all ApiChangeLog annotations;
(2) For the login mismatch, either update the login Operation description in
AdminApiControllerDocs to state the actual behavior ("returns HTTP 400 with
empty body on invalid credentials") or change the AdminApiController.login
implementation to return a standard error response containing the
INVALID_CREDENTIALS error payload (make the controller return the structured
error instead of plain 400 empty body) so that AdminApiControllerDocs.login and
AdminApiController.login are aligned.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CLAUDE.mdRomRom-Common/src/main/java/com/romrom/common/dto/AdminResponse.javaRomRom-Domain-Item/src/main/java/com/romrom/item/service/ItemService.javaRomRom-Domain-Member/src/main/java/com/romrom/member/service/MemberService.javaRomRom-Web/src/main/java/com/romrom/web/controller/api/AdminApiController.javaRomRom-Web/src/main/java/com/romrom/web/controller/api/AdminApiControllerDocs.java
Comment on lines
+16
to
+26
| ### POST API 패턴 (전체 공통) | ||
| - **모든 API는 POST + `consumes = MediaType.MULTIPART_FORM_DATA_VALUE` + `@ModelAttribute` 패턴 사용** | ||
| - 예시: | ||
| ```java | ||
| @PostMapping(value = "/sign-in", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) | ||
| @LogMonitor | ||
| public ResponseEntity<AuthResponse> signIn(@ModelAttribute AuthRequest request) { | ||
| return ResponseEntity.ok(authService.signIn(request)); | ||
| } | ||
| ``` | ||
|
|
There was a problem hiding this comment.
문서 규칙과 실제 구현이 어긋날 수 있어 예외/범위를 명시해두는 게 좋겠습니다.
- Line 17-25에서 “모든 API는 POST + multipart +
@ModelAttribute”를 “전체 공통”으로 선언했는데, 이번 PR의AdminApiController만 봐도GET /dashboard/*,GET /items,GET /members,DELETE /items/{itemId}가 존재합니다. “예외 케이스(예: 조회는 GET, 삭제는 DELETE 등)”를 문서에 같이 적어두면 혼선이 줄어들어요.
또한 markdownlint 힌트(MD040)처럼 Line 55의 fenced code block에 언어가 빠져 있습니다.
제안 diff (MD040 해결)
@@
- ```
+ ```md
## 인증(JWT): **필요/불필요**
@@
- ```
+ ```Also applies to: 52-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` around lines 16 - 26, Update the "POST API 패턴 (전체 공통)" section to
explicitly state exceptions (e.g., read-only endpoints using GET and destructive
endpoints using DELETE) and mention that controllers like AdminApiController
expose GET /dashboard/*, GET /items, GET /members and DELETE /items/{itemId} as
allowed deviations from the POST+multipart+@ModelAttribute rule; also fix the
missing fenced-code-block language by adding the appropriate language tag (e.g.,
```md) to the fenced block in the documentation so markdownlint MD040 is
satisfied.
Comment on lines
+311
to
+326
| /** | ||
| * 관리자용 회원 단건 조회 | ||
| */ | ||
| @Transactional(readOnly = true) | ||
| public AdminResponse getMemberDetailForAdmin(AdminRequest request) { | ||
| Member member = memberRepository.findById(request.getMemberId()) | ||
| .orElseThrow(() -> { | ||
| log.error("관리자 회원 단건 조회 실패 - 존재하지 않는 memberId: {}", request.getMemberId()); | ||
| return new CustomException(ErrorCode.MEMBER_NOT_FOUND); | ||
| }); | ||
|
|
||
| return AdminResponse.builder() | ||
| .member(member) | ||
| .build(); | ||
| } | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== 1. AdminRequest 검증 어노테이션 확인 =="
fd -t f 'AdminRequest.java' | head -5 | while read file; do
echo "--- $file ---"
cat -n "$file" || true
done
echo
echo "== 2. AdminApiController에서 getMemberDetail 관련 코드 확인 =="
fd -t f 'AdminApiController.java' | head -1 | while read file; do
echo "--- $file ---"
cat -n "$file" | grep -A 20 -B 5 "getMemberDetail\|@Valid\|@Validated" || true
done
echo
echo "== 3. MemberService에서 findMemberById 메서드 확인 =="
fd -t f 'MemberService.java' | head -1 | while read file; do
echo "--- $file ---"
cat -n "$file" | sed -n '330,345p'
done
echo
echo "== 4. MemberService에서 Admin 관련 메서드 검색 =="
fd -t f 'MemberService.java' | head -1 | while read file; do
echo "--- $file ---"
rg -n 'public.*Admin|getMemberDetail' "$file" || true
doneRepository: TEAM-ROMROM/RomRom-BE
Length of output: 5210
관리자 회원 단건 조회: memberId 누락 시 500 가능 + 기존 조회 로직 중복입니다.
-
AdminRequest에memberId필드(line 76)가 검증 어노테이션 없이 선언되어 있고, 컨트롤러의getMemberDetail메서드(line 164)도@Valid또는@Validated가 없어서,null값이 그대로memberRepository.findById(null)로 전달되어 Spring Data JPA가IllegalArgumentException을 던집니다. 이는 500 에러를 발생시키는데, 400 응답이 올바릅니다. -
같은 클래스의
findMemberById(UUID memberId)메서드(line 331~)가 동일한 로직을 처리하고 있어서,getMemberDetailForAdmin에서 이를 재사용하면 중복을 제거할 수 있습니다.
제안 diff (중복 제거 방향)
`@Transactional`(readOnly = true)
public AdminResponse getMemberDetailForAdmin(AdminRequest request) {
- Member member = memberRepository.findById(request.getMemberId())
- .orElseThrow(() -> {
- log.error("관리자 회원 단건 조회 실패 - 존재하지 않는 memberId: {}", request.getMemberId());
- return new CustomException(ErrorCode.MEMBER_NOT_FOUND);
- });
+ // AdminRequest에 `@NotNull` + 컨트롤러에 `@Valid` 추가 필수
+ Member member = findMemberById(request.getMemberId());
return AdminResponse.builder()
.member(member)
.build();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@RomRom-Domain-Member/src/main/java/com/romrom/member/service/MemberService.java`
around lines 311 - 326, Ensure AdminRequest.memberId is validated and remove
duplicate lookup: add a `@NotNull` on AdminRequest.memberId and make the
controller getMemberDetail endpoint use `@Valid` or `@Validated` so null memberId
yields a 400; then change MemberService.getMemberDetailForAdmin(AdminRequest) to
call the existing findMemberById(UUID memberId) helper (or explicitly null-check
request.getMemberId() and throw a 400-mapped CustomException) instead of calling
memberRepository.findById(...) inline, reusing the centralized error handling in
findMemberById and eliminating the duplicated lookup/logging.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
RomRom-Common/src/main/java/com/romrom/common/dto/AdminResponse.java (1)
25-41:⚠️ Potential issue | 🟠 Major관리자 목록 응답용 페이지 메타 필드를 명시적으로 추가해 주세요.
totalPages,totalElements,currentPage가 DTO 최상위에 없어 목록 응답 계약이 불명확합니다. AdminResponse에 메타 필드를 추가하고 목록 API에서 값을 채우는 것이 맞습니다.🔧 제안 수정안
public class AdminResponse { `@Schema`(description = "전체 카운트") private Long totalCount; + + `@Schema`(description = "전체 페이지 수") + private Integer totalPages; + + `@Schema`(description = "전체 요소 수") + private Long totalElements; + + `@Schema`(description = "현재 페이지(0-based)") + private Integer currentPage;Based on learnings: Applies to **/AdminResponse.{java,kt} : Admin API 목록 조회 시 Page 정보(totalPages, totalElements, currentPage)를 Response에 포함
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@RomRom-Common/src/main/java/com/romrom/common/dto/AdminResponse.java` around lines 25 - 41, AdminResponse currently exposes totalCount, items (Page<AdminItemDto>), and members (Page<AdminMemberDto>) but lacks explicit pagination meta fields; add Long totalPages, Long totalElements, and Integer currentPage to AdminResponse and ensure the list endpoints that populate items or members also set these new fields from the Page object (use page.getTotalPages(), page.getTotalElements(), page.getNumber() or equivalent) so the response contract clearly includes pagination metadata.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@RomRom-Domain-Item/src/main/java/com/romrom/item/repository/postgres/ItemRepository.java`:
- Around line 59-60: The JPQL fetch join in the repository method
findByItemIdWithDetails can return duplicate rows (causing
NonUniqueResultException) because of the collection fetch (i.itemImages); update
the query string used by findByItemIdWithDetails to use SELECT DISTINCT i
instead of SELECT i (i.e., "SELECT DISTINCT i FROM Item i LEFT JOIN FETCH
i.member LEFT JOIN FETCH i.itemImages") so the Optional<Item> single-result
retrieval is safe.
---
Outside diff comments:
In `@RomRom-Common/src/main/java/com/romrom/common/dto/AdminResponse.java`:
- Around line 25-41: AdminResponse currently exposes totalCount, items
(Page<AdminItemDto>), and members (Page<AdminMemberDto>) but lacks explicit
pagination meta fields; add Long totalPages, Long totalElements, and Integer
currentPage to AdminResponse and ensure the list endpoints that populate items
or members also set these new fields from the Page object (use
page.getTotalPages(), page.getTotalElements(), page.getNumber() or equivalent)
so the response contract clearly includes pagination metadata.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
RomRom-Common/src/main/java/com/romrom/common/dto/AdminResponse.javaRomRom-Domain-Item/src/main/java/com/romrom/item/repository/postgres/ItemRepository.javaRomRom-Domain-Item/src/main/java/com/romrom/item/service/ItemService.javaRomRom-Domain-Member/src/main/java/com/romrom/member/service/MemberService.java
🚧 Files skipped from review as they are similar to previous changes (1)
- RomRom-Domain-Member/src/main/java/com/romrom/member/service/MemberService.java
Comment on lines
+59
to
+60
| @Query("SELECT i FROM Item i LEFT JOIN FETCH i.member LEFT JOIN FETCH i.itemImages WHERE i.itemId = :itemId") | ||
| Optional<Item> findByItemIdWithDetails(@Param("itemId") UUID itemId); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "ItemRepository.java" | head -5Repository: TEAM-ROMROM/RomRom-BE
Length of output: 155
🏁 Script executed:
find . -type f -name "Item.java" | head -5Repository: TEAM-ROMROM/RomRom-BE
Length of output: 141
🏁 Script executed:
fd 'ItemRepository\.java$' -x cat -n {}Repository: TEAM-ROMROM/RomRom-BE
Length of output: 3095
🏁 Script executed:
fd 'Item\.java$' -x cat -n {}Repository: TEAM-ROMROM/RomRom-BE
Length of output: 4982
🏁 Script executed:
# Check if there are any tests for this method
fd -type f -name "*Test.java" | xargs rg -l "findByItemIdWithDetails" 2>/dev/null || echo "No tests found for this method"Repository: TEAM-ROMROM/RomRom-BE
Length of output: 415
🏁 Script executed:
# Search for any other similar patterns to understand if DISTINCT is already used elsewhere
rg "SELECT DISTINCT" RomRom-Domain-Item/src/main/java/com/romrom/item/repository/ 2>/dev/null || echo "No DISTINCT queries found in ItemRepository"Repository: TEAM-ROMROM/RomRom-BE
Length of output: 198
LEFT JOIN FETCH i.itemImages 단건 조회는 DISTINCT 없으면 중복 결과 예외 위험입니다.
Line 59의 컬렉션 fetch join(@OneToMany)은 행 중복을 생성하여 Optional<Item> 단건 반환에서 NonUniqueResultException을 발생시킬 수 있습니다. SELECT DISTINCT i로 수정이 필요합니다.
수정안
- `@Query`("SELECT i FROM Item i LEFT JOIN FETCH i.member LEFT JOIN FETCH i.itemImages WHERE i.itemId = :itemId")
+ `@Query`("SELECT DISTINCT i FROM Item i LEFT JOIN FETCH i.member LEFT JOIN FETCH i.itemImages WHERE i.itemId = :itemId")
Optional<Item> findByItemIdWithDetails(`@Param`("itemId") UUID itemId);📝 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
| @Query("SELECT i FROM Item i LEFT JOIN FETCH i.member LEFT JOIN FETCH i.itemImages WHERE i.itemId = :itemId") | |
| Optional<Item> findByItemIdWithDetails(@Param("itemId") UUID itemId); | |
| `@Query`("SELECT DISTINCT i FROM Item i LEFT JOIN FETCH i.member LEFT JOIN FETCH i.itemImages WHERE i.itemId = :itemId") | |
| Optional<Item> findByItemIdWithDetails(`@Param`("itemId") UUID itemId); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@RomRom-Domain-Item/src/main/java/com/romrom/item/repository/postgres/ItemRepository.java`
around lines 59 - 60, The JPQL fetch join in the repository method
findByItemIdWithDetails can return duplicate rows (causing
NonUniqueResultException) because of the collection fetch (i.itemImages); update
the query string used by findByItemIdWithDetails to use SELECT DISTINCT i
instead of SELECT i (i.e., "SELECT DISTINCT i FROM Item i LEFT JOIN FETCH
i.member LEFT JOIN FETCH i.itemImages") so the Optional<Item> single-result
retrieval is safe.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
릴리스 노트
새로운 기능
문서