Feat(client): 나의 북마크 관련 API v3로 변경 및 연결#278
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough북마크 관련 API를 v1→v3로 전환하고 응답 타입을 재구조화했습니다. FetchCard 컴포넌트를 제거하고, 단일 통합 훅(useMyBookmarkContentData)과 읽음 상태 필터(readStatus)를 반영한 쿼리/캐시 키 및 카운트 엔드포인트들이 추가/갱신되었습니다. Changes
Sequence Diagram(s)(생성 조건 미충족 — 생략) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
|
✅ Storybook chromatic 배포 확인: |
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)
apps/client/src/pages/myBookmark/apis/axios.ts (1)
32-41:⚠️ Potential issue | 🔴 Critical카테고리 북마크 조회가 아직 v1 엔드포인트를 호출합니다.
Line 35, Line 40이
/api/v1/articles/category를 사용하고 있어 PR 목표인 v3 마이그레이션과 충돌합니다. 현재 타입(CategoryBookmarkArticleResponse)은 v3 구조를 전제하고 있어 응답 포맷 불일치 시 화면 데이터가 깨질 수 있습니다.🔧 제안 수정
export const getCategoryBookmarkArticles = async ( categoryId: string | null, readStatus: boolean | null, page: number, size: number ) : Promise<CategoryBookmarkArticleResponse> => { if (readStatus === null) { const { data } = await apiRequest.get( - `/api/v1/articles/category?categoryId=${categoryId}&page=${page}&size=${size}` + `/api/v3/articles/category?categoryId=${categoryId}&page=${page}&size=${size}` ); return data.data; } else { const { data } = await apiRequest.get( - `/api/v1/articles/category?categoryId=${categoryId}&read-status=${readStatus}&page=${page}&size=${size}` + `/api/v3/articles/category?categoryId=${categoryId}&read-status=${readStatus}&page=${page}&size=${size}` ); return data.data; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/myBookmark/apis/axios.ts` around lines 32 - 41, The GET requests that fetch category bookmarks still call the v1 endpoint; update both apiRequest.get calls that currently use `/api/v1/articles/category` to the v3 endpoint (`/api/v3/articles/category`) and ensure the query parameter names and response shape match the v3 contract so the returned data can be assigned to CategoryBookmarkArticleResponse; verify the branches that use readStatus (the readStatus variable and the two apiRequest.get invocations) are updated and any response mapping is adjusted if v3 returns a different structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/client/src/pages/myBookmark/components/myBookmarkContent/MyBookmarkContent.tsx`:
- Around line 61-78: The branching for category-based data should consistently
use categoryId rather than category; update the ternaries for articlesToDisplay,
hasNextPage, and fetchNextPage to check categoryId (like
totalArticle/totalUnread already do) so they select categoryList,
categoryCountData-derived sources, hasNextCategoryArticles, and
fetchNextCategoryArticles when categoryId is present, otherwise use
bookmarkArticlesData/pages, bookmarkCountData, hasNextBookmarkArticles, and
fetchNextBookmarkArticles; verify variable names articlesToDisplay,
totalArticle, totalUnread, hasNextPage, fetchNextPage, categoryList,
bookmarkArticlesData, categoryCountData, bookmarkCountData,
hasNextCategoryArticles, hasNextBookmarkArticles, fetchNextCategoryArticles, and
fetchNextBookmarkArticles are used consistently.
---
Outside diff comments:
In `@apps/client/src/pages/myBookmark/apis/axios.ts`:
- Around line 32-41: The GET requests that fetch category bookmarks still call
the v1 endpoint; update both apiRequest.get calls that currently use
`/api/v1/articles/category` to the v3 endpoint (`/api/v3/articles/category`) and
ensure the query parameter names and response shape match the v3 contract so the
returned data can be assigned to CategoryBookmarkArticleResponse; verify the
branches that use readStatus (the readStatus variable and the two apiRequest.get
invocations) are updated and any response mapping is adjusted if v3 returns a
different structure.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/client/src/pages/myBookmark/MyBookmark.tsxapps/client/src/pages/myBookmark/apis/axios.tsapps/client/src/pages/myBookmark/apis/queries.tsapps/client/src/pages/myBookmark/components/fetchCard/FetchCard.tsxapps/client/src/pages/myBookmark/components/myBookmarkContent/MyBookmarkContent.tsxapps/client/src/pages/myBookmark/types/api.tsapps/client/src/shared/components/cardEditModal/CardEditModal.tsx
💤 Files with no reviewable changes (1)
- apps/client/src/pages/myBookmark/components/fetchCard/FetchCard.tsx
apps/client/src/pages/myBookmark/components/myBookmarkContent/MyBookmarkContent.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/client/src/pages/myBookmark/components/myBookmarkContent/MyBookmarkContent.tsx (1)
61-63:⚠️ Potential issue | 🟠 Major카테고리 분기 기준이 다시 혼재되어 있습니다.
Line 61, Line 75, Line 79는
category를 기준으로 분기하고, Line 65~70은categoryId를 기준으로 분기합니다.category만 있고categoryId가 없는 케이스에서 목록/카운트/무한스크롤 소스가 어긋납니다.🔧 제안 수정
+ const isCategoryView = Boolean(categoryId); + - const articlesToDisplay = category + const articlesToDisplay = isCategoryView ? categoryList : (bookmarkArticlesData?.pages.flatMap((page) => page.articles) ?? []); @@ - const hasNextPage = category + const hasNextPage = isCategoryView ? hasNextCategoryArticles : hasNextBookmarkArticles; - const fetchNextPage = category + const fetchNextPage = isCategoryView ? fetchNextCategoryArticles : fetchNextBookmarkArticles;Also applies to: 75-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/myBookmark/components/myBookmarkContent/MyBookmarkContent.tsx` around lines 61 - 63, The code mixes branching on category and categoryId causing sources for list, count, and infinite scroll to diverge; standardize to use a single criterion (preferably categoryId) across articlesToDisplay, count logic (bookmarkArticlesCount), and pagination/infinite-scroll data (bookmarkArticlesData and categoryList) so that when category exists but categoryId is absent the same data source is used; update the conditional expressions in the blocks that compute articlesToDisplay, total count, and load-more logic (references: articlesToDisplay, categoryList, bookmarkArticlesData, bookmarkArticlesCount, category, categoryId, and any fetchMore/loadMore handlers) to consistently branch on categoryId and fall back predictably when it's undefined.
🧹 Nitpick comments (1)
apps/client/src/pages/myBookmark/apis/queries.ts (1)
22-23: 마지막 페이지에서 빈 요청 1회가 추가로 발생합니다.현재는
articles.length === 0일 때만 중단되어, 마지막 non-empty 페이지 다음 요청이 한 번 더 발생합니다. 총 개수 기반으로 종료하면 불필요한 요청을 줄일 수 있습니다.♻️ 제안 수정
return useSuspenseInfiniteQuery({ @@ - getNextPageParam: (lastPage, allPages) => - lastPage.articles.length === 0 ? undefined : allPages.length, + getNextPageParam: (lastPage, allPages) => { + const fetched = allPages.reduce( + (sum, page) => sum + page.articles.length, + 0 + ); + return fetched >= lastPage.totalArticleCount ? undefined : allPages.length; + }, }); }; @@ - getNextPageParam: (lastPage, allPages) => { - if (!lastPage || lastPage.articles.length === 0) return undefined; - return allPages.length; - }, + getNextPageParam: (lastPage, allPages) => { + const fetched = allPages.reduce( + (sum, page) => sum + page.articles.length, + 0 + ); + return fetched >= lastPage.totalArticleCount ? undefined : allPages.length; + }, }); };Also applies to: 57-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/myBookmark/apis/queries.ts` around lines 22 - 23, The current getNextPageParam uses lastPage.articles.length to stop pagination which causes one extra empty request after the final non-empty page; replace this with a total-count based check in getNextPageParam: compute whether (allPages.length * pageSize) >= lastPage.total (or lastPage.totalCount / lastPage.meta.total depending on your API shape) and return undefined if true, otherwise return allPages.length; apply the same fix to the other occurrence referenced (lines 57-60) so both getNextPageParam implementations use the total-count vs page-size check rather than articles.length.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/client/src/pages/myBookmark/components/myBookmarkContent/MyBookmarkContent.tsx`:
- Around line 92-96: The current logic in MyBookmarkContent computes totalCount
from categoryCountData/bookmarkCountData and treats missing data as 0, causing
NoArticles to render while categoryCountData is still loading; change the guard
so you first detect whether the relevant count data is undefined (e.g., check
categoryId and then categoryCountData !== undefined, or bookmarkCountData !==
undefined) and only treat totalCount as zero after the data has loaded—render
NoUnreadArticles (or a loading state) when the count data is not yet available,
and render NoArticles only when the loaded totalCount is 0; update the
conditional around totalCount/categoryCountData/bookmarkCountData and the
returns of NoArticles/NoUnreadArticles accordingly.
---
Duplicate comments:
In
`@apps/client/src/pages/myBookmark/components/myBookmarkContent/MyBookmarkContent.tsx`:
- Around line 61-63: The code mixes branching on category and categoryId causing
sources for list, count, and infinite scroll to diverge; standardize to use a
single criterion (preferably categoryId) across articlesToDisplay, count logic
(bookmarkArticlesCount), and pagination/infinite-scroll data
(bookmarkArticlesData and categoryList) so that when category exists but
categoryId is absent the same data source is used; update the conditional
expressions in the blocks that compute articlesToDisplay, total count, and
load-more logic (references: articlesToDisplay, categoryList,
bookmarkArticlesData, bookmarkArticlesCount, category, categoryId, and any
fetchMore/loadMore handlers) to consistently branch on categoryId and fall back
predictably when it's undefined.
---
Nitpick comments:
In `@apps/client/src/pages/myBookmark/apis/queries.ts`:
- Around line 22-23: The current getNextPageParam uses lastPage.articles.length
to stop pagination which causes one extra empty request after the final
non-empty page; replace this with a total-count based check in getNextPageParam:
compute whether (allPages.length * pageSize) >= lastPage.total (or
lastPage.totalCount / lastPage.meta.total depending on your API shape) and
return undefined if true, otherwise return allPages.length; apply the same fix
to the other occurrence referenced (lines 57-60) so both getNextPageParam
implementations use the total-count vs page-size check rather than
articles.length.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/client/src/pages/myBookmark/apis/axios.tsapps/client/src/pages/myBookmark/apis/queries.tsapps/client/src/pages/myBookmark/components/myBookmarkContent/MyBookmarkContent.tsxapps/client/src/pages/myBookmark/types/api.ts
apps/client/src/pages/myBookmark/components/myBookmarkContent/MyBookmarkContent.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/client/src/pages/myBookmark/components/myBookmarkContent/MyBookmarkContent.tsx (1)
39-45:⚠️ Potential issue | 🟡 Minor카운트 로딩 중
NoArticles가 오표시될 수 있습니다.
counts.total이 아직undefined인 순간을0으로 간주해서 빈 상태가 잘못 노출될 수 있습니다. 카운트가 준비되기 전에는 빈 상태 판단을 지연해 주세요.🔧 제안 수정
- const totalCount = counts.total ?? 0; + const totalCount = counts.total; /** Empty 상태 컴포넌트 */ const EmptyStateComponent = () => { if (list.articles.length === 0) { + if (totalCount === undefined) return null; // 또는 로딩 UI if (totalCount === 0) return <NoArticles />; return <NoUnreadArticles />; } return null; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/myBookmark/components/myBookmarkContent/MyBookmarkContent.tsx` around lines 39 - 45, EmptyStateComponent misclassifies empty state because totalCount defaults counts.total to 0; change the logic to defer empty-state rendering until counts.total is defined: stop using const totalCount = counts.total ?? 0; instead reference counts.total explicitly in EmptyStateComponent and if counts.total is undefined/null return null (or a loading placeholder) to avoid showing <NoArticles /> prematurely, then when counts.total is a number use it to decide between <NoArticles /> and <NoUnreadArticles />; update references to totalCount and ensure EmptyStateComponent uses counts.total (or a guarded local variable) for the check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/client/src/pages/myBookmark/components/myBookmarkContent/MyBookmarkContent.tsx`:
- Around line 39-45: EmptyStateComponent misclassifies empty state because
totalCount defaults counts.total to 0; change the logic to defer empty-state
rendering until counts.total is defined: stop using const totalCount =
counts.total ?? 0; instead reference counts.total explicitly in
EmptyStateComponent and if counts.total is undefined/null return null (or a
loading placeholder) to avoid showing <NoArticles /> prematurely, then when
counts.total is a number use it to decide between <NoArticles /> and
<NoUnreadArticles />; update references to totalCount and ensure
EmptyStateComponent uses counts.total (or a guarded local variable) for the
check.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/client/src/pages/myBookmark/components/myBookmarkContent/MyBookmarkContent.tsxapps/client/src/pages/myBookmark/hooks/useMyBookmarkContentData.ts
| return { | ||
| view: { | ||
| isCategoryView, | ||
| categoryName: categoryNameFromResponse, | ||
| }, | ||
| list: { | ||
| articles: articlesToDisplay, | ||
| }, | ||
| counts: { | ||
| total: totalArticle, | ||
| unread: totalUnread, | ||
| }, | ||
| pagination: { | ||
| sentinelRef, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
이런 방식은 생각해본적 없는데 확실히 관심사 분리와 가독성에 좋을것 같네요
jjangminii
left a comment
There was a problem hiding this comment.
서스펜스 사용을 위해 바운더리 안에 내용을 전부 북마크 콘텐츠로 분리하였는데 너무 안일하게 나눴던것 같네요,, 확실히 책임 분리하니 알아보기도 더 좋네요-! 수고하셨습니다
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
1. API 연결 및 v3로 수정
해당 API v3로 오면서 바뀐 field 추가 및 변경했습니다.
그 중에서 아티클 개수 조회하는 부분이 전체 조회에서 빠져 따로 API로 분리가 되어서 새로 연결했습니다.
2. useMyBookmarkContentData custom hook으로 추상화
추가로 나의 북마크라는 큰 하나의 페이지 아래서 일반 전체 아티클과 카테고리 아티클이 한번에 처리가 되어야 해서 리스트 데이터나 count 등도 이 두개를 항상 구분해서 작성이 되어 있었어요.
위와 같이 모든 대부분의 데이터가 하나의
mybookmarkContent에서isCategoryView로 분기처리가 되어야했는데 이러한 데이터 처리 및 분기 로직들이mybookmarkContent안에 있어서 여러가지 관심사가 섞여있는 상황이었어요.따라서 이를 데이터 처리 로직을 담당하는
useMyBookmarkContentData훅을 따로 분리해주고 사용하도록 리팩터링 했습니다.🤔 물론 여기서 고민한 지점이 한 가지 있어요.
어떻게 보면 데이터 처리 로직을 저 훅으로 분리하기만 하는 작업 같아보이는데, 이는 물리적인 코드의 위치 변화가 위주다 보니 이것이 정말 의미있는 추상화인가? 라는 고민을 많이 한 것 같아요.
하지만 생각해보면 content라는 컴포넌트는 이름에서도 볼 수 있듯이 UI를 담당하는 컴포넌트예요. 여기에 데이터 처리 로직이 함께 있는 것부터 책임이 너무 과도할 수 있다고 판단했어요. 또한 추상화 관점에서
useMyBookmarkContentData라는 이름이 저 content의 비즈니스 로직 등을 관리한다는 것을 네이밍만으로도 추측이 가능하고, return 값도 어느정도 예측이 가능한 범위라고 판단했어요.Note
data 관련 처리에 대한 hook은 공식문서에도 예시로 나오고, 해당 분리에 대한 접근은 권장하고 있어요.
https://ko.react.dev/learn/reusing-logic-with-custom-hooks#when-to-use-custom-hooks
그래서 처음에는 분리 후 모든 값을 flat하게 return을 해줬어요. 하지만 이럴 경우에 필드가 만약 더 늘어나게 되면 값을 추측하기 점점 어려워 질 것이라고 생각했어요. (물론 네이밍만으로 판단이 쉽지 않다면 설계를 다시 고민해야 될 수도 있겠지만요!)
따라서 반환을 개념 단위로 묶어서 return 하도록 수정해봤어요.
이렇게 되면 return 받는 곳에서는 view에 대한 data와 llist, count 등을 통해 한눈에 의미를 파악하기 쉬울 것 같다고 판단했어요.
추가적인 피드백 및 리뷰는 언제나 환영입니다!!!!
Summary by CodeRabbit
출시 노트
New Features
Refactor
Bug Fixes