Conversation
*chevron-left, chevron-right 추가
*TanStack Query 적용 및 meta 반환
4 tasks
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughPrompt 목록 응답 타입을 중첩 구조로 리팩토링하고, 클라이언트 페이지네이션 훅(usePagination)과 Pagination 컴포넌트를 추가했으며, usePromptList 훅을 React Query로 마이그레이션해 페이지네이션을 통합했습니다. Changes
Sequence DiagramsequenceDiagram
actor User
participant PromptListPage
participant usePromptList as Hook
participant ReactQuery as ReactQuery
participant API
participant PaginationComponent as Pagination
User->>PromptListPage: 페이지 변경 클릭
PromptListPage->>PromptListPage: page 상태 업데이트
PromptListPage->>Hook: GetPromptListParams 전달 (page, size)
Hook->>ReactQuery: useQuery 실행 (queryKey, queryFn)
ReactQuery->>API: getPromptList(page, size) 요청
API-->>ReactQuery: PromptListItemResponse[] 반환
ReactQuery->>Hook: select로 매핑(mapPromptListItemDTO 등)
Hook-->>PromptListPage: { prompts, meta }
PromptListPage->>Pagination: currentPage, totalSize 전달
Pagination-->>PromptListPage: onPageChange 콜백 호출
PromptListPage-->>User: 업데이트된 목록 및 페이지네이션 렌더링
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![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
🤖 Fix all issues with AI agents
In `@src/components/Pagination/Pagination.tsx`:
- Around line 15-58: paginationRange can be empty causing lastPage to be
undefined and enabling the Next button; ensure lastPage is derived safely (e.g.
fallback to Math.max(1, Math.ceil(totalSize / pageSize)) or 0 when
totalSize===0) and guard usePagination results: compute lastPage from totalSize
when paginationRange is falsy or empty, make onNext clamp to lastPage, and
update the Next button's disabled check to use this safe lastPage (references:
paginationRange, lastPage, usePagination, currentPage, onNext, DOTS).
In `@src/hooks/prompts/usePagination.ts`:
- Around line 15-55: The function usePagination has a missing return when
rendering all pages: in the computed paginationRnage you call range(1,
totalSize) inside the if but don't return it and the condition is reversed;
change the logic so that when totalSize is less than or equal to
totalPageNumbers you return range(1, totalSize) (i.e., replace the existing "if
(totalSize >= totalPageNumbers) range(1, totalSize);" with a proper early
return), ensuring paginationRnage yields the full page list; keep using the
existing symbols (usePagination, paginationRnage, totalPageNumbers, range, DOTS,
firstPageIndex, lastPageIndex) to locate and update the code.
🧹 Nitpick comments (3)
src/components/Card/Card.tsx (1)
18-18: className에 불필요한 trailing space가 있습니다.
gap-1뒤에 불필요한 공백이 있습니다. 기능에는 영향이 없지만, 코드 정리 차원에서 제거하는 것이 좋습니다.🧹 수정 제안
- <main className="flex flex-col h-18 gap-1 "> + <main className="flex flex-col h-18 gap-1">src/pages/PromptList/PromptListPage.tsx (1)
38-38: 콜백 함수를 간소화할 수 있습니다.
(page) => setPage(page)는setPage로 간소화할 수 있습니다. 또한 현재 파라미터page가 외부 state 변수page를 shadowing하고 있어 혼란을 줄 수 있습니다.♻️ 수정 제안
<Pagination currentPage={meta.page} totalSize={meta.totalPages} - onPageChange={(page) => setPage(page)} + onPageChange={setPage} />src/hooks/prompts/usePromptList.ts (1)
12-18: 초기 meta 값이 기본 파라미터와 불일치합니다.
초기 렌더에서 페이지네이션이 0페이지로 보일 수 있으니 기본값을 defaultParams 기준으로 맞추는 편이 안정적입니다.♻️ 수정 제안
-const defaultMeta: PromptListMeta = { - page: 0, - size: 0, +const defaultMeta: PromptListMeta = { + page: defaultParams.page ?? 1, + size: defaultParams.size ?? 0, totalElements: 0, totalPages: 0, hasNext: false, };
Comment on lines
+15
to
+58
| const paginationRange = usePagination({ totalSize, siblingCount, currentPage }); | ||
| const lastPage = paginationRange ? paginationRange[paginationRange.length - 1] : 0; | ||
|
|
||
| if (currentPage === 0) return null; | ||
|
|
||
| const onPrevious = () => onPageChange(currentPage - 1); | ||
| const onNext = () => onPageChange(currentPage + 1); | ||
|
|
||
| return ( | ||
| <div className={`flex items-center justify-center gap-1 ${className}`}> | ||
| <button | ||
| onClick={onPrevious} | ||
| disabled={currentPage === 1} | ||
| aria-label="이전 페이지" | ||
| className={`${ButtonStyles.base} ${ | ||
| currentPage === 1 ? ButtonStyles.disabled : ButtonStyles.inactive | ||
| }`}> | ||
| <SvgChevornleft /> | ||
| </button> | ||
| {paginationRange?.map((pageNumber, index) => { | ||
| if (pageNumber === DOTS) { | ||
| return ( | ||
| <span | ||
| key={`dots-${index}`} | ||
| className="px-2 text-gray-400 flex items-center select-none"> | ||
| {DOTS} | ||
| </span> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <button | ||
| key={pageNumber} | ||
| className={`${ButtonStyles.base} ${ | ||
| pageNumber === currentPage ? ButtonStyles.active : ButtonStyles.inactive | ||
| }`} | ||
| onClick={() => onPageChange(Number(pageNumber))}> | ||
| {pageNumber} | ||
| </button> | ||
| ); | ||
| })} | ||
| <button | ||
| onClick={onNext} | ||
| disabled={currentPage === lastPage} |
There was a problem hiding this comment.
빈 페이지(totalSize=0)에서 다음 버튼이 활성화될 수 있습니다.
paginationRange가 빈 배열이면 lastPage가 undefined가 되어 Next가 눌릴 수 있습니다. totalSize/빈 range를 가드하고 lastPage는 totalSize로 계산하는 편이 안전합니다.
🛠️ 수정 제안
- const paginationRange = usePagination({ totalSize, siblingCount, currentPage });
- const lastPage = paginationRange ? paginationRange[paginationRange.length - 1] : 0;
-
- if (currentPage === 0) return null;
+ const paginationRange = usePagination({ totalSize, siblingCount, currentPage });
+ if (currentPage === 0 || totalSize <= 0 || !paginationRange?.length) return null;
+ const lastPage = totalSize;📝 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
| const paginationRange = usePagination({ totalSize, siblingCount, currentPage }); | |
| const lastPage = paginationRange ? paginationRange[paginationRange.length - 1] : 0; | |
| if (currentPage === 0) return null; | |
| const onPrevious = () => onPageChange(currentPage - 1); | |
| const onNext = () => onPageChange(currentPage + 1); | |
| return ( | |
| <div className={`flex items-center justify-center gap-1 ${className}`}> | |
| <button | |
| onClick={onPrevious} | |
| disabled={currentPage === 1} | |
| aria-label="이전 페이지" | |
| className={`${ButtonStyles.base} ${ | |
| currentPage === 1 ? ButtonStyles.disabled : ButtonStyles.inactive | |
| }`}> | |
| <SvgChevornleft /> | |
| </button> | |
| {paginationRange?.map((pageNumber, index) => { | |
| if (pageNumber === DOTS) { | |
| return ( | |
| <span | |
| key={`dots-${index}`} | |
| className="px-2 text-gray-400 flex items-center select-none"> | |
| {DOTS} | |
| </span> | |
| ); | |
| } | |
| return ( | |
| <button | |
| key={pageNumber} | |
| className={`${ButtonStyles.base} ${ | |
| pageNumber === currentPage ? ButtonStyles.active : ButtonStyles.inactive | |
| }`} | |
| onClick={() => onPageChange(Number(pageNumber))}> | |
| {pageNumber} | |
| </button> | |
| ); | |
| })} | |
| <button | |
| onClick={onNext} | |
| disabled={currentPage === lastPage} | |
| const paginationRange = usePagination({ totalSize, siblingCount, currentPage }); | |
| if (currentPage === 0 || totalSize <= 0 || !paginationRange?.length) return null; | |
| const lastPage = paginationRange[paginationRange.length - 1]; | |
| const onPrevious = () => onPageChange(currentPage - 1); | |
| const onNext = () => onPageChange(currentPage + 1); | |
| return ( | |
| <div className={`flex items-center justify-center gap-1 ${className}`}> | |
| <button | |
| onClick={onPrevious} | |
| disabled={currentPage === 1} | |
| aria-label="이전 페이지" | |
| className={`${ButtonStyles.base} ${ | |
| currentPage === 1 ? ButtonStyles.disabled : ButtonStyles.inactive | |
| }`}> | |
| <SvgChevornleft /> | |
| </button> | |
| {paginationRange?.map((pageNumber, index) => { | |
| if (pageNumber === DOTS) { | |
| return ( | |
| <span | |
| key={`dots-${index}`} | |
| className="px-2 text-gray-400 flex items-center select-none"> | |
| {DOTS} | |
| </span> | |
| ); | |
| } | |
| return ( | |
| <button | |
| key={pageNumber} | |
| className={`${ButtonStyles.base} ${ | |
| pageNumber === currentPage ? ButtonStyles.active : ButtonStyles.inactive | |
| }`} | |
| onClick={() => onPageChange(Number(pageNumber))}> | |
| {pageNumber} | |
| </button> | |
| ); | |
| })} | |
| <button | |
| onClick={onNext} | |
| disabled={currentPage === lastPage} |
🤖 Prompt for AI Agents
In `@src/components/Pagination/Pagination.tsx` around lines 15 - 58,
paginationRange can be empty causing lastPage to be undefined and enabling the
Next button; ensure lastPage is derived safely (e.g. fallback to Math.max(1,
Math.ceil(totalSize / pageSize)) or 0 when totalSize===0) and guard
usePagination results: compute lastPage from totalSize when paginationRange is
falsy or empty, make onNext clamp to lastPage, and update the Next button's
disabled check to use this safe lastPage (references: paginationRange, lastPage,
usePagination, currentPage, onNext, DOTS).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/hooks/prompts/usePromptList.ts`:
- Around line 6-35: The merge of params into defaultParams can leave explicit
undefined values in queryParams causing cache key/request mismatch; update
usePromptList to build a normalizedParams object using nullish coalescing for
each field (e.g., page: params?.page ?? defaultParams.page, size: params?.size
?? defaultParams.size, sort: params?.sort ?? defaultParams.sort), use
normalizedParams for the queryKey and for calling getPromptList, and derive
defaultMeta from normalizedParams (so defaultMeta.page/size come from
normalizedParams.page/size).
Comment on lines
+6
to
+35
| const defaultParams: GetPromptListParams = { | ||
| page: 1, | ||
| size: 21, | ||
| sort: 'latest', | ||
| }; | ||
|
|
||
| const defaultMeta: PromptListMeta = { | ||
| page: defaultParams.page ?? 1, | ||
| size: defaultParams.size ?? 0, | ||
| totalElements: 0, | ||
| totalPages: 0, | ||
| hasNext: false, | ||
| }; | ||
|
|
||
| useEffect(() => { | ||
| (async () => { | ||
| try { | ||
| const res = await getPromptList(); | ||
| const mappedData = res.data.items.map(mapPromptListItemDTO); | ||
| export const usePromptList = (params?: GetPromptListParams) => { | ||
| const queryParams = { ...defaultParams, ...params }; | ||
|
|
||
| setPrompts(mappedData); | ||
| } catch (error) { | ||
| setError(error); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| })(); | ||
| }, []); | ||
| const { data, isLoading, isError, error } = useQuery({ | ||
| queryKey: ['prompts', 'list', queryParams], | ||
| queryFn: () => getPromptList(queryParams), | ||
| select: (response) => ({ | ||
| prompts: response.data.items.map(mapPromptListItemDTO), | ||
| meta: response.data.meta, | ||
| }), | ||
| placeholderData: keepPreviousData, | ||
| }); | ||
|
|
||
| return { prompts, loading, error }; | ||
| return { | ||
| prompts: data?.prompts || [], | ||
| meta: data?.meta || defaultMeta, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and examine its content
fd -t f "usePromptList.ts"Repository: promlog/promlog-client
Length of output: 100
🏁 Script executed:
# Look for the file in the hooks directory
find . -name "usePromptList.ts" -type f 2>/dev/nullRepository: promlog/promlog-client
Length of output: 102
🏁 Script executed:
# Check the repository structure
ls -la src/hooks/ 2>/dev/null || echo "Directory not found"Repository: promlog/promlog-client
Length of output: 233
🏁 Script executed:
# Read the file content to verify the code snippet
cat -n src/hooks/prompts/usePromptList.tsRepository: promlog/promlog-client
Length of output: 1475
🏁 Script executed:
# Also check for type definitions
fd -t f "\.ts" src/ | xargs grep -l "GetPromptListParams\|PromptListMeta" 2>/dev/null | head -10Repository: promlog/promlog-client
Length of output: 162
🏁 Script executed:
# Check the type definitions
cat -n src/apis/prompts/prompts.types.tsRepository: promlog/promlog-client
Length of output: 1934
🏁 Script executed:
# Also check the prompts.ts to understand the getPromptList function
cat -n src/apis/prompts/prompts.tsRepository: promlog/promlog-client
Length of output: 1064
🏁 Script executed:
# Let's verify the actual behavior by analyzing the parameter handling
# Check if there are any places where usePromptList is called with undefined params
grep -r "usePromptList" src/ --include="*.ts" --include="*.tsx" -B 2 -A 2Repository: promlog/promlog-client
Length of output: 1208
🏁 Script executed:
# Let's analyze the parameter spreading behavior and cache implications
# Check if there are any test files or other usages that might pass undefined
find . -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "usePromptList" 2>/dev/null || echo "No test files found with usePromptList"Repository: promlog/promlog-client
Length of output: 104
🏁 Script executed:
# Let's check the actual usage more carefully and see if partial params are passed
cat -n src/pages/PromptList/PromptListPage.tsx | head -30Repository: promlog/promlog-client
Length of output: 1230
명시적 파라미터 정규화로 잠재적 캐시 키/기본 메타 불일치 예방
현재 코드에서 params에 명시적으로 undefined 값이 포함되면 스프레드 연산자로 인해 기본값이 덮어씌워질 수 있습니다. 예를 들어 usePromptList({page: undefined})를 호출하면 queryParams.page가 undefined가 되지만, getPromptList의 기본 파라미터에서는 page = 1을 사용하므로 캐시 키와 실제 요청 값이 달라질 수 있습니다. 명시적 널 병합 연산자(??)로 각 파라미터를 정규화하고, 정규화된 queryParams에서 defaultMeta를 생성하도록 수정해 주세요.
🔧 제안 수정안
-const defaultMeta: PromptListMeta = {
- page: defaultParams.page ?? 1,
- size: defaultParams.size ?? 0,
- totalElements: 0,
- totalPages: 0,
- hasNext: false,
-};
+const buildDefaultMeta = (params: GetPromptListParams): PromptListMeta => ({
+ page: params.page ?? 1,
+ size: params.size ?? 0,
+ totalElements: 0,
+ totalPages: 0,
+ hasNext: false,
+});
export const usePromptList = (params?: GetPromptListParams) => {
- const queryParams = { ...defaultParams, ...params };
+ const queryParams: GetPromptListParams = {
+ page: params?.page ?? defaultParams.page,
+ size: params?.size ?? defaultParams.size,
+ sort: params?.sort ?? defaultParams.sort,
+ };
+ const defaultMeta = buildDefaultMeta(queryParams);
const { data, isLoading, isError, error } = useQuery({
queryKey: ['prompts', 'list', queryParams],
queryFn: () => getPromptList(queryParams),
@@
return {
prompts: data?.prompts || [],
- meta: data?.meta || defaultMeta,
+ meta: data?.meta ?? defaultMeta,🤖 Prompt for AI Agents
In `@src/hooks/prompts/usePromptList.ts` around lines 6 - 35, The merge of params
into defaultParams can leave explicit undefined values in queryParams causing
cache key/request mismatch; update usePromptList to build a normalizedParams
object using nullish coalescing for each field (e.g., page: params?.page ??
defaultParams.page, size: params?.size ?? defaultParams.size, sort: params?.sort
?? defaultParams.sort), use normalizedParams for the queryKey and for calling
getPromptList, and derive defaultMeta from normalizedParams (so
defaultMeta.page/size come from normalizedParams.page/size).
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.
✅ 체크리스트
📝 작업 상세 내용
프롬프트 카드 UI 변경
프롬프트 목록 페이지 페이지네이션 추가
📸 스크린샷 (선택 사항)
✅ 셀프 체크리스트
이슈 번호: #40
Summary by CodeRabbit
새로운 기능
스타일
아이콘
✏️ Tip: You can customize this high-level summary in your review settings.