-
Notifications
You must be signed in to change notification settings - Fork 1
[FIX] 관리자 페이지 페이지번호 기반 조회-slice 응답 page로 수정 #148
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.
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
Conversation
📝 WalkthroughWalkthroughThis PR replaces many Slice/SliceResponse-based pagination flows with a new PageResponse record and Page-based flows, updating controllers, services, repositories, and DTOs to return or produce PageResponse across multiple modules. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as "Client"
participant Controller as "Controller"
participant Service as "Service"
participant Repository as "Repository"
Client->>Controller: GET list (page,size,keyword)
Controller->>Service: getList(page,size,keyword) / build Pageable
Service->>Repository: query Page<T> (pageable, spec/keyword)
Repository-->>Service: Page<T> (content + totalElements)
Service->>Service: map Page<T> -> Page<DTO> (dtoPage)
Service->>Service: PageResponse.of(dtoPage)
Service-->>Controller: PageResponse<DTO>
Controller-->>Client: ApiResponse.onSuccess(PageResponse<DTO>)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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/cc/backend/admin/amateurShow/controller/AdminAmateurShowController.java (1)
40-46: Swagger schema doesn't match the new return type.The
@Contentannotation still referencesSliceResponse.class, but the endpoint now returnsPage<AdminAmateurShowListResponseDTO>. This will produce incorrect API documentation.📖 Proposed fix
responses = { `@io.swagger.v3.oas.annotations.responses.ApiResponse`( responseCode = "200", description = "조회 성공", - content = `@Content`(schema = `@Schema`(implementation = SliceResponse.class)) + content = `@Content`(schema = `@Schema`(implementation = Page.class)) ) }src/main/java/cc/backend/admin/inquiry/AdminInquiryController.java (1)
42-50: Add validation forpage/sizeto prevent 500s on invalid input.Negative or zero values will throw in
PageRequest.of(...). Consider adding bean validation constraints and a reasonable max size.✅ Suggested update (method params)
- public ApiResponse<Page<AdminInquiryResponseDTO.AdminInquirySummaryResponseDTO>> getInquiryList( + public ApiResponse<Page<AdminInquiryResponseDTO.AdminInquirySummaryResponseDTO>> getInquiryList( `@Parameter`(description = "검색 키워드(제목/내용)") `@RequestParam`(required = false) String keyword, `@Parameter`(description = "페이지 번호(0부터 시작)", example = "0") - `@RequestParam`(defaultValue = "0") int page, + `@RequestParam`(defaultValue = "0") `@Min`(0) int page, `@Parameter`(description = "페이지 크기", example = "20") - `@RequestParam`(defaultValue = "20") int size + `@RequestParam`(defaultValue = "20") `@Min`(1) `@Max`(100) int size ) {You’ll also need
@Validatedon the controller (or config) andjakarta.validation.constraints.*imports.
🤖 Fix all issues with AI agents
In `@src/main/java/cc/backend/admin/board/controller/AdminBoardController.java`:
- Around line 95-96: Remove the stray empty statement by deleting the orphan
semicolon before the return statement in AdminBoardController (the line
immediately preceding the return of
cc.backend.apiPayLoad.ApiResponse.onSuccess(...)); ensure the method returns
directly without the extraneous ";" so the return statement remains the only
statement at that position.
In `@src/main/java/cc/backend/admin/ticket/AdminTicketController.java`:
- Around line 36-38: The stray semicolon before the return in
AdminTicketController should be removed; locate the method that returns
ApiResponse.onSuccess(adminTicketService.getTicketList(page, size, keyword)) and
delete the lone ";" that sits on its own line so the method contains only the
return statement and no confusing dead code.
🧹 Nitpick comments (6)
src/main/java/cc/backend/admin/amateurShow/controller/AdminApprovalController.java (1)
9-9: Remove unused imports.
SliceResponse(line 9) andSlice(line 17) are no longer used in this file after the migration toPage-based pagination.🧹 Suggested cleanup
-import cc.backend.apiPayLoad.SliceResponse;-import org.springframework.data.domain.Slice;Also applies to: 17-17
src/main/java/cc/backend/admin/board/controller/AdminBoardController.java (1)
8-8: Remove unused imports.
SliceResponse(line 8) andSlice(line 18) are no longer used in this file after the migration toPage-based pagination.🧹 Suggested cleanup
-import cc.backend.apiPayLoad.SliceResponse;-import org.springframework.data.domain.Slice;Also applies to: 18-18
src/main/java/cc/backend/admin/amateurShow/service/AdminAmateurShowService.java (1)
9-10: Remove unused imports.
ApiResponse(line 9) andSliceResponse(line 10) are not used in this service file.🧹 Suggested cleanup
-import cc.backend.apiPayLoad.ApiResponse; -import cc.backend.apiPayLoad.SliceResponse;src/main/java/cc/backend/admin/amateurShow/controller/AdminAmateurShowController.java (1)
10-20: Remove unused imports after Slice → Page migration.
SliceResponse(line 10) andSlice(line 20) are no longer used after switching toPage-based pagination.🧹 Suggested cleanup
import cc.backend.apiPayLoad.ApiResponse; -import cc.backend.apiPayLoad.SliceResponse; import com.google.protobuf.Api; ... import org.springframework.data.domain.Page; -import org.springframework.data.domain.Slice;src/main/java/cc/backend/admin/member/AdminMemberController.java (1)
7-13: Remove unused imports after migration.
SliceResponse(line 7) andSlice(line 13) are no longer used after switching toPage-based pagination.🧹 Suggested cleanup
import cc.backend.apiPayLoad.ApiResponse; -import cc.backend.apiPayLoad.SliceResponse; import io.swagger.v3.oas.annotations.Operation; ... import org.springframework.data.domain.Page; -import org.springframework.data.domain.Slice;src/main/java/cc/backend/admin/inquiry/AdminInquiryController.java (1)
42-51: Confirm client/API contract for Spring DataPageserialization.Switching from
SliceResponsetoPagechanges the JSON shape (extrapageable,sort,totalElements, etc.) and ties the contract to Spring Data’s serialization. Please ensure the admin UI and API docs are updated accordingly, or consider mapping to a customPageResponseDTO for a stable contract.
src/main/java/cc/backend/admin/board/controller/AdminBoardController.java
Outdated
Show resolved
Hide resolved
src/main/java/cc/backend/admin/ticket/AdminTicketController.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (2)
src/main/java/cc/backend/admin/ticket/AdminTicketController.java (1)
24-37: Inconsistent response format in three admin ticket endpoints: useSliceResponsewrapper instead ofPage.The endpoints
getTicketHistory(),getReservationHistory(), andgetRefundHistory()returnApiResponse<Page<...>>, while the rest of the codebase consistently returnsApiResponse<SliceResponse<...>>. This breaks the API contract—clients will receive different JSON structures across endpoints.
SliceResponsewraps pagination metadata with field names (page,hasNext) optimized for infinite scroll, whilePageexposes different fields (number,totalElements,totalPages) with different semantics. Update all three endpoints to wrap thePageresult withSliceResponse.of()to maintain consistency.src/main/java/cc/backend/admin/board/controller/AdminBoardController.java (1)
76-95: Update OpenAPI schema to reflect Page response.The endpoint returns
ApiResponse<Page<AdminBoardListResponse>>, but the@ApiResponsecontent schema specifies onlyAdminBoardListResponse, which will cause springdoc to generate incomplete OpenAPI documentation missing pagination metadata (page number, size, totalElements, hasNext, etc.). Create a Page wrapper DTO extendingPageImpl<AdminBoardListResponse>and reference it in the schema, or use@Schema(implementation = PageWrapperClass.class)instead.
🧹 Nitpick comments (1)
src/main/java/cc/backend/admin/ticket/AdminTicketController.java (1)
84-86: Remove extra blank line for consistency.The blank line at line 84 is inconsistent with the other endpoint methods (
getTicketHistory,getReservationHistory) which don't have this extra blank line before the return statement.🧹 Proposed fix
) { - return ApiResponse.onSuccess(adminTicketService.getRefundList(page, size, keyword)); }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (1)
src/main/java/cc/backend/admin/amateurShow/service/AdminAmateurShowService.java (1)
11-27: Remove unused imports from this service class.The following imports are not used anywhere in the file and should be removed:
SliceResponse(line 11)ApproveShowEvent,NewShowEvent,RejectShowEvent(lines 14-16)ApplicationEventPublisher(line 19)PathVariable(line 23)isNotBlankstatic import (line 27)🧹 Proposed cleanup
import cc.backend.apiPayLoad.ApiResponse; import cc.backend.apiPayLoad.PageResponse; -import cc.backend.apiPayLoad.SliceResponse; import cc.backend.apiPayLoad.code.status.ErrorStatus; import cc.backend.apiPayLoad.exception.GeneralException; -import cc.backend.event.entity.ApproveShowEvent; -import cc.backend.event.entity.NewShowEvent; -import cc.backend.event.entity.RejectShowEvent; import cc.backend.member.entity.Member; import lombok.RequiredArgsConstructor; -import org.springframework.context.ApplicationEventPublisher; import org.springframework.data.domain.*; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; -import org.springframework.web.bind.annotation.PathVariable; import java.util.List; - -import static io.micrometer.common.util.StringUtils.isNotBlank;
🤖 Fix all issues with AI agents
In `@src/main/java/cc/backend/search/dto/SearchShowResponseDTO.java`:
- Around line 40-53: The response's status field should return the localized
label expected by clients rather than s.getStatus().toString(); update
SearchShowDTO.from to map AmateurShow.getStatus() to the original display
strings (e.g., "예매 진행 중", "공연 종료", "예정", "승인 대기", "반려" or "판매중" as per API docs)
instead of calling toString(), or call a new enum method like getDisplayLabel()
on the status enum if you add it; adjust the assignment to statusLabel and
ensure the private String status field is populated with that mapped/display
value (keep schedule creation via AmateurConverter.mergeSchedule(s.getStart(),
s.getEnd()) unchanged).
In `@src/main/java/cc/backend/search/SearchService.java`:
- Around line 26-39: The current SearchService uses a Slice query and a separate
countByNameOrPerformer call causing two DB queries; add a Page-based repository
method (e.g., in AmateurShowRepository add a method returning Page<AmateurShow>
with the same `@Query` logic as the existing Slice variant, e.g.,
findByNameOrPerformerPage or change findByNameOrPerformer to return Page) and
then update SearchService.searchAmateurShows to call that Page method, map
results with page.map(SearchShowResponseDTO.SearchShowDTO::from) and use
page.getTotalElements() to populate
SearchShowResponseDTO.SearchShowDTO.SearchShowResultDTO so only a single query
(paged + total) is executed.
🧹 Nitpick comments (5)
src/main/java/cc/backend/apiPayLoad/PageResponse.java (1)
8-22: Document 0‑based pageNumber/pageSize semantics
Page#getNumber()is 0‑based; a brief Javadoc/API note on the record components will help clients avoid off‑by‑one confusion when switching from SliceResponse.src/main/java/cc/backend/admin/board/service/AdminBoardService.java (1)
44-51: Consider usingPageResponse.of(...)for consistency.You already have a
Page<Board>; mapping toPage<AdminBoardListResponse>and delegating toPageResponse.of(...)avoids manual metadata wiring and stays aligned ifPageResponseevolves.♻️ Suggested refactor
- List<AdminBoardListResponse> content = boards.getContent().stream() - .map(AdminBoardListResponse::from) - .toList(); - return new PageResponse<>(content, boards.getNumber(), boards.getSize(), boards.getTotalElements(), boards.getTotalPages()); + Page<AdminBoardListResponse> dtoPage = boards.map(AdminBoardListResponse::from); + return PageResponse.of(dtoPage);src/main/java/cc/backend/admin/photoAlbum/service/AdminPhotoAlbumService.java (1)
35-59: Consider usingPageResponse.of(...)to reduce manual wiring.Since you already have a
Page<PhotoAlbum>, mapping to aPage<DTO>and usingPageResponse.of(...)avoids manual pagination fields and keeps future changes centralized.♻️ Suggested refactor
- List<AdminPhotoAlbumResponseDTO.SimplePhotoAlbumDTO> content = photoAlbums.getContent().stream() - .map(photoAlbum -> AdminPhotoAlbumResponseDTO.SimplePhotoAlbumDTO.builder() - .id(photoAlbum.getId()) - .amateurShowName(photoAlbum.getAmateurShow().getName()) - .uploaderId(photoAlbum.getAmateurShow().getMember().getId()) - .uploaderName(photoAlbum.getAmateurShow().getMember().getName()) - .updatedAt(photoAlbum.getUpdatedAt()) - .build()) - .toList(); - - return new PageResponse<>( - content, - photoAlbums.getNumber(), - photoAlbums.getSize(), - photoAlbums.getTotalElements(), - photoAlbums.getTotalPages() - ); + Page<AdminPhotoAlbumResponseDTO.SimplePhotoAlbumDTO> dtoPage = + photoAlbums.map(photoAlbum -> AdminPhotoAlbumResponseDTO.SimplePhotoAlbumDTO.builder() + .id(photoAlbum.getId()) + .amateurShowName(photoAlbum.getAmateurShow().getName()) + .uploaderId(photoAlbum.getAmateurShow().getMember().getId()) + .uploaderName(photoAlbum.getAmateurShow().getMember().getName()) + .updatedAt(photoAlbum.getUpdatedAt()) + .build()); + + return PageResponse.of(dtoPage);src/main/java/cc/backend/search/SearchController.java (1)
4-5: Unused imports and inconsistency with PR objectives.
PageResponseis imported on line 4 but not used—the return type isSearchShowResultDTO. Meanwhile,SliceResponse(line 5) andSlice(line 14) appear to be unused after this refactor.Additionally, this endpoint returns a custom DTO wrapper instead of
PageResponse, which is inconsistent with the broader PR goal of standardizing onPageResponsefor pagination.src/main/java/cc/backend/search/SearchService.java (1)
5-7: Remove unused/suspicious imports.
- Line 5:
PageResponseis imported but not used in this file.- Line 7:
io.micrometer.core.instrument.search.Searchappears to be an accidental import (this is a metrics instrumentation class, not related to application search logic).🧹 Proposed fix
-import cc.backend.apiPayLoad.PageResponse; import cc.backend.search.dto.SearchShowResponseDTO; -import io.micrometer.core.instrument.search.Search; import lombok.RequiredArgsConstructor;
#이슈번호,#이슈번호Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.