Api(client): 아티클 읽음 상태 변경 API 연결#96
Conversation
Walkthrough책갈피 및 리마인드 페이지에 아티클 읽음 상태 변경 흐름을 추가했다. 카드 클릭 시 새 탭으로 아티클을 열고 읽음 상태 PUT 뮤테이션을 호출하며, 성공 시 관련 React Query 캐시를 무효화하도록 쿼리/타입/컴포넌트가 연동됐다. Changes
Sequence Diagram(s)sequenceDiagram
participant U as 사용자
participant P as 페이지 (MyBookmark/Remind)
participant C as 카드 컴포넌트
participant MQ as usePutArticleReadStatus (React Query)
participant API as PUT /api/v1/articles/{id}/readStatus
participant QC as QueryClient
U->>C: 카드 클릭
C->>P: onClick 콜백 실행
P->>U: 새 탭으로 아티클 URL 오픈
P->>MQ: mutate(articleId)
MQ->>API: PUT 요청
API-->>MQ: ArticleReadStatusResponse
MQ-->>P: onSuccess
P->>QC: invalidateQueries(관련 키들)
QC-->>P: 재패치 트리거
alt 오류
MQ-->>P: onError
P-->>P: 콘솔에 에러 로깅
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
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.
Actionable comments posted: 7
🧹 Nitpick comments (2)
packages/design-system/src/components/card/RemindCard.tsx (1)
49-56: aria-label 의미 명확화 제안."카테고리 상세"보다는 실제 기능(옵션/메뉴 열기)을 반영한 레이블이 스크린리더에 더 적절합니다. 예: "옵션 열기".
-aria-label="카테고리 상세" +aria-label="옵션 열기"apps/client/src/shared/apis/queries.ts (1)
70-77: useMutation 제네릭 명시로 타입 안전 강화.반환 타입은 지정돼 있으나, useMutation 제네릭을 함께 명시하면 추론 안정성이 좋아집니다.
-export const usePutArticleReadStatus = (): UseMutationResult< - ArticleReadStatusResponse, - AxiosError, - number -> => { - return useMutation({ - mutationFn: (articleId: number) => putArticleReadStatus(articleId), - }); -}; +export const usePutArticleReadStatus = (): UseMutationResult< + ArticleReadStatusResponse, + AxiosError, + number +> => { + return useMutation<ArticleReadStatusResponse, AxiosError, number>({ + mutationFn: (articleId: number) => putArticleReadStatus(articleId), + }); +};
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/client/src/pages/myBookmark/MyBookmark.tsx(2 hunks)apps/client/src/pages/remind/Remind.tsx(2 hunks)apps/client/src/shared/apis/axios.ts(1 hunks)apps/client/src/shared/apis/queries.ts(2 hunks)apps/client/src/shared/types/api.ts(1 hunks)packages/design-system/src/components/card/BaseCard.tsx(1 hunks)packages/design-system/src/components/card/MyBookmarkCard.tsx(1 hunks)packages/design-system/src/components/card/RemindCard.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: constantly-dev
PR: Pinback-Team/pinback-client#2
File: pnpm-workspace.yaml:3-3
Timestamp: 2025-08-18T13:48:59.065Z
Learning: constantly-dev는 docs 디렉터리를 컨벤션 문서 추가용으로 사용할 예정이라고 명시했습니다.
🧬 Code graph analysis (3)
apps/client/src/pages/remind/Remind.tsx (3)
apps/client/src/shared/utils/formatDateTime.ts (1)
formatLocalDateTime(1-9)apps/client/src/shared/apis/queries.ts (1)
usePutArticleReadStatus(70-78)apps/client/src/pages/remind/apis/queries.ts (1)
useGetRemindArticles(6-16)
apps/client/src/pages/myBookmark/MyBookmark.tsx (2)
apps/client/src/pages/myBookmark/apis/queries.ts (3)
useGetBookmarkArticles(14-22)useGetBookmarkUnreadArticles(24-32)useGetCategoryBookmarkArticles(34-44)apps/client/src/shared/apis/queries.ts (1)
usePutArticleReadStatus(70-78)
apps/client/src/shared/apis/queries.ts (2)
apps/client/src/shared/types/api.ts (1)
ArticleReadStatusResponse(16-19)apps/client/src/shared/apis/axios.ts (1)
putArticleReadStatus(42-47)
| onClick={() => { | ||
| window.open(article.url, '_blank'); | ||
|
|
||
| updateToReadStatus(article.articleId, { | ||
| onSuccess: () => { | ||
| // TODO: 쿼리키 팩토리 패턴 적용 | ||
| queryClient.invalidateQueries({ | ||
| queryKey: ['bookmarkReadArticles'], | ||
| }); | ||
| queryClient.invalidateQueries({ | ||
| queryKey: ['bookmarkUnreadArticles'], | ||
| }); | ||
| queryClient.invalidateQueries({ | ||
| queryKey: ['categoryBookmarkArticles'], | ||
| }); | ||
| }, | ||
| onError: (error) => { | ||
| console.error(error); | ||
| }, | ||
| }); | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
새 탭 열기 보안 + 읽음 상태 중복 업데이트 방지.
Remind와 동일한 개선이 필요합니다. reverse tabnabbing 차단, notRead에서만 업데이트 호출 권장.
- onClick={() => {
- window.open(article.url, '_blank');
-
- updateToReadStatus(article.articleId, {
- onSuccess: () => {
- // TODO: 쿼리키 팩토리 패턴 적용
- queryClient.invalidateQueries({
- queryKey: ['bookmarkReadArticles'],
- });
- queryClient.invalidateQueries({
- queryKey: ['bookmarkUnreadArticles'],
- });
- queryClient.invalidateQueries({
- queryKey: ['categoryBookmarkArticles'],
- });
- },
- onError: (error) => {
- console.error(error);
- },
- });
- }}
+ onClick={() => {
+ const win = window.open(
+ article.url,
+ '_blank',
+ 'noopener,noreferrer'
+ );
+ if (win) win.opener = null;
+
+ if (activeBadge === 'notRead') {
+ updateToReadStatus(article.articleId, {
+ onSuccess: () => {
+ // TODO: 쿼리키 팩토리 패턴 적용
+ queryClient.invalidateQueries({
+ queryKey: ['bookmarkReadArticles'],
+ });
+ queryClient.invalidateQueries({
+ queryKey: ['bookmarkUnreadArticles'],
+ });
+ queryClient.invalidateQueries({
+ queryKey: ['categoryBookmarkArticles'],
+ });
+ },
+ onError: (error) => {
+ console.error(error);
+ },
+ });
+ }
+ }}📝 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.
| onClick={() => { | |
| window.open(article.url, '_blank'); | |
| updateToReadStatus(article.articleId, { | |
| onSuccess: () => { | |
| // TODO: 쿼리키 팩토리 패턴 적용 | |
| queryClient.invalidateQueries({ | |
| queryKey: ['bookmarkReadArticles'], | |
| }); | |
| queryClient.invalidateQueries({ | |
| queryKey: ['bookmarkUnreadArticles'], | |
| }); | |
| queryClient.invalidateQueries({ | |
| queryKey: ['categoryBookmarkArticles'], | |
| }); | |
| }, | |
| onError: (error) => { | |
| console.error(error); | |
| }, | |
| }); | |
| }} | |
| onClick={() => { | |
| const win = window.open( | |
| article.url, | |
| '_blank', | |
| 'noopener,noreferrer' | |
| ); | |
| if (win) win.opener = null; | |
| if (activeBadge === 'notRead') { | |
| updateToReadStatus(article.articleId, { | |
| onSuccess: () => { | |
| // TODO: 쿼리키 팩토리 패턴 적용 | |
| queryClient.invalidateQueries({ | |
| queryKey: ['bookmarkReadArticles'], | |
| }); | |
| queryClient.invalidateQueries({ | |
| queryKey: ['bookmarkUnreadArticles'], | |
| }); | |
| queryClient.invalidateQueries({ | |
| queryKey: ['categoryBookmarkArticles'], | |
| }); | |
| }, | |
| onError: (error) => { | |
| console.error(error); | |
| }, | |
| }); | |
| } | |
| }} |
| onClick={() => { | ||
| window.open(article.url, '_blank'); | ||
|
|
||
| updateToReadStatus(article.articleId, { | ||
| onSuccess: () => { | ||
| queryClient.invalidateQueries({ | ||
| queryKey: ['remindArticles'], | ||
| }); | ||
| }, | ||
| onError: (error) => { | ||
| console.error(error); | ||
| }, | ||
| }); | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
새 탭 열기 보안(Reverse Tabnabbing) 및 불필요한 중복 호출 방지.
- window.open 시
noopener,noreferrer없이 열면 보안 이슈가 있습니다. - 이미 읽은 목록(activeBadge === 'read')에서도 읽음 처리 API를 호출합니다. 불필요한 호출을 피하세요.
- onClick={() => {
- window.open(article.url, '_blank');
-
- updateToReadStatus(article.articleId, {
- onSuccess: () => {
- queryClient.invalidateQueries({
- queryKey: ['remindArticles'],
- });
- },
- onError: (error) => {
- console.error(error);
- },
- });
- }}
+ onClick={() => {
+ const win = window.open(
+ article.url,
+ '_blank',
+ 'noopener,noreferrer'
+ );
+ if (win) win.opener = null;
+
+ if (activeBadge === 'notRead') {
+ updateToReadStatus(article.articleId, {
+ onSuccess: () => {
+ queryClient.invalidateQueries({
+ queryKey: ['remindArticles'],
+ });
+ },
+ onError: (error) => {
+ console.error(error);
+ },
+ });
+ }
+ }}📝 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.
| onClick={() => { | |
| window.open(article.url, '_blank'); | |
| updateToReadStatus(article.articleId, { | |
| onSuccess: () => { | |
| queryClient.invalidateQueries({ | |
| queryKey: ['remindArticles'], | |
| }); | |
| }, | |
| onError: (error) => { | |
| console.error(error); | |
| }, | |
| }); | |
| }} | |
| onClick={() => { | |
| const win = window.open( | |
| article.url, | |
| '_blank', | |
| 'noopener,noreferrer' | |
| ); | |
| if (win) win.opener = null; | |
| if (activeBadge === 'notRead') { | |
| updateToReadStatus(article.articleId, { | |
| onSuccess: () => { | |
| queryClient.invalidateQueries({ | |
| queryKey: ['remindArticles'], | |
| }); | |
| }, | |
| onError: (error) => { | |
| console.error(error); | |
| }, | |
| }); | |
| } | |
| }} |
🤖 Prompt for AI Agents
In apps/client/src/pages/remind/Remind.tsx around lines 80–93, the onClick
handler opens article.url without noopener,noreferrer which is vulnerable to
reverse tabnabbing and also calls updateToReadStatus even when the article is
already marked read; fix by opening the URL with noopener and noreferrer (e.g.
pass 'noopener,noreferrer' or set newWindow.opener = null) to prevent
tabnabbing, and wrap the updateToReadStatus call in a conditional that skips the
API call when activeBadge === 'read' (only call the update and invalidate
queries when the article is not already read).
| export const putArticleReadStatus = async (articleId: number) => { | ||
| const { data } = await apiRequest.put( | ||
| `/api/v1/articles/${articleId}/readStatus` | ||
| ); | ||
| return data; | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
반환 형태 일관화 및 응답 타입 지정.
다른 API는 data.data를 반환합니다. 본 함수만 data를 그대로 반환하면 상층 타입과 어긋날 수 있습니다. 응답 제네릭과 반환 타입을 명시해 주세요.
+import { ArticleReadStatusResponse } from '@shared/types/api';
-export const putArticleReadStatus = async (articleId: number) => {
- const { data } = await apiRequest.put(
- `/api/v1/articles/${articleId}/readStatus`
- );
- return data;
-};
+export const putArticleReadStatus = async (
+ articleId: number
+): Promise<ArticleReadStatusResponse> => {
+ const { data } = await apiRequest.put<{ data: ArticleReadStatusResponse }>(
+ `/api/v1/articles/${articleId}/readStatus`
+ );
+ return data.data;
+};🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== search for putArticleReadStatus ==="
rg -n --hidden --no-ignore -S "putArticleReadStatus" || true
echo "=== search for axios.ts under apps/client/src/shared/apis ==="
rg -n --hidden --no-ignore -S "apps/client/src/shared/apis/.*axios\.ts" || true
echo "=== show apps/client/src/shared/apis/axios.ts (if exists) ==="
if [ -f "apps/client/src/shared/apis/axios.ts" ]; then
sed -n '1,240p' apps/client/src/shared/apis/axios.ts || true
else
echo "file not found at apps/client/src/shared/apis/axios.ts; listing axios.ts files in repo..."
rg -n --hidden --no-ignore -S --files | rg "axios\.ts" || true
fi
echo "=== occurrences of 'return data.data' ==="
rg -n --hidden --no-ignore -S "return\s+data\.data" || true
echo "=== search for ArticleReadStatusResponse type ==="
rg -n --hidden --no-ignore -S "ArticleReadStatusResponse" || true
echo "=== locate apiRequest definition/usages ==="
rg -n --hidden --no-ignore -S "apiRequest" || true
echo "=== show top matches for files defining apiRequest (first 200 lines each) ==="
for f in $(rg -n --hidden --no-ignore -S --files-with-matches "apiRequest" | head -n 10); do
echo "---- $f ----"
sed -n '1,200p' "$f" || true
doneLength of output: 11952
반환 형태 일관화 및 응답 타입 지정 — apps/client/src/shared/apis/axios.ts:42-47
다른 API들이 data.data를 반환하므로 이 함수도 ArticleReadStatusResponse 제네릭을 지정하고 data.data를 반환하도록 수정하세요. apps/client/src/shared/types/api.ts에 ArticleReadStatusResponse 타입이 존재합니다.
+import { ArticleReadStatusResponse } from '@shared/types/api';
-export const putArticleReadStatus = async (articleId: number) => {
- const { data } = await apiRequest.put(
- `/api/v1/articles/${articleId}/readStatus`
- );
- return data;
-};
+export const putArticleReadStatus = async (
+ articleId: number
+): Promise<ArticleReadStatusResponse> => {
+ const { data } = await apiRequest.put<{ data: ArticleReadStatusResponse }>(
+ `/api/v1/articles/${articleId}/readStatus`
+ );
+ return data.data;
+};📝 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.
| export const putArticleReadStatus = async (articleId: number) => { | |
| const { data } = await apiRequest.put( | |
| `/api/v1/articles/${articleId}/readStatus` | |
| ); | |
| return data; | |
| }; | |
| import { ArticleReadStatusResponse } from '@shared/types/api'; | |
| export const putArticleReadStatus = async ( | |
| articleId: number | |
| ): Promise<ArticleReadStatusResponse> => { | |
| const { data } = await apiRequest.put<{ data: ArticleReadStatusResponse }>( | |
| `/api/v1/articles/${articleId}/readStatus` | |
| ); | |
| return data.data; | |
| }; |
🤖 Prompt for AI Agents
In apps/client/src/shared/apis/axios.ts around lines 42 to 47, the
putArticleReadStatus function currently returns data directly but should match
other APIs by using the ArticleReadStatusResponse generic and returning
data.data; update the apiRequest.put call to
apiRequest.put<ArticleReadStatusResponse>(...) (import ArticleReadStatusResponse
from apps/client/src/shared/types/api.ts if not already imported) and change the
return to return data.data so the function returns the typed payload.
| export interface ArticleReadStatusResponse { | ||
| acornCount: number; | ||
| acornCollected: boolean; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
API 응답 래핑 일관성 점검 필요 (data vs data.data).
axios.ts의 다른 API들은 data.data를 반환하지만, putArticleReadStatus는 data를 그대로 반환합니다. 백엔드 응답이 공통 래퍼({ data: ... })인지 확인 후 타입과 반환 형태를 일치시켜 주세요. 현재 ArticleReadStatusResponse 사용부에서 런타임 타입 미스매치 가능성이 있습니다.
아래처럼 axios 측을 통일하는 것을 권장합니다(실제 응답 스키마에 맞춰 조정).
// apps/client/src/shared/apis/axios.ts
+import { ArticleReadStatusResponse } from '@shared/types/api';
-export const putArticleReadStatus = async (articleId: number) => {
- const { data } = await apiRequest.put(
- `/api/v1/articles/${articleId}/readStatus`
- );
- return data;
-};
+export const putArticleReadStatus = async (
+ articleId: number
+): Promise<ArticleReadStatusResponse> => {
+ const { data } = await apiRequest.put<{ data: ArticleReadStatusResponse }>(
+ `/api/v1/articles/${articleId}/readStatus`
+ );
+ return data.data;
+};🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== find definitions/usages ==="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' "ArticleReadStatusResponse" || true
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' "putArticleReadStatus" || true
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' "apiRequest.put" || true
echo "=== show types file (if exists) ==="
if [ -f apps/client/src/shared/types/api.ts ]; then
sed -n '1,240p' apps/client/src/shared/types/api.ts || true
else
echo "apps/client/src/shared/types/api.ts not found"
fi
echo "=== show axios api file (if exists) ==="
if [ -f apps/client/src/shared/apis/axios.ts ]; then
sed -n '1,320p' apps/client/src/shared/apis/axios.ts || true
else
echo "apps/client/src/shared/apis/axios.ts not found"
fi
echo "=== scan for return patterns in apis directory ==="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' "return data;|return data.data;" apps/client/src/shared/apis -S || true
echo "=== show occurrences of ArticleReadStatusResponse usage ==="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' "ArticleReadStatusResponse" -S || true
echo "=== show occurrences of axios response wrapper typing (\"{ data:\") ==="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \"<\\{ data: [^}]+\\}\" -S || true || trueLength of output: 2573
API 응답 래핑 불일치 — putArticleReadStatus 반환형 일관화 필요
검증 결과 apps/client/src/shared/apis/axios.ts에서 대부분 엔드포인트는 data.data를 반환하나 putArticleReadStatus는 data를 그대로 반환하고 있어 ArticleReadStatusResponse와 런타임 불일치가 발생할 수 있습니다. 아래처럼 axios 측 반환을 통일하세요.
- 수정(권장): apps/client/src/shared/apis/axios.ts의 putArticleReadStatus를 data.data 반환으로 변경
// apps/client/src/shared/apis/axios.ts
+import { ArticleReadStatusResponse } from '@shared/types/api';
-export const putArticleReadStatus = async (articleId: number) => {
- const { data } = await apiRequest.put(
- `/api/v1/articles/${articleId}/readStatus`
- );
- return data;
-};
+export const putArticleReadStatus = async (
+ articleId: number
+): Promise<ArticleReadStatusResponse> => {
+ const { data } = await apiRequest.put<{ data: ArticleReadStatusResponse }>(
+ `/api/v1/articles/${articleId}/readStatus`
+ );
+ return data.data;
+};- 추가 검토: postSignUp / postCategory / putCategory 등에서도 반환형(response / data / data.data)이 혼재되어 있으니 전사 정책(예: 모든 API는 data.data 반환 또는 예외 문서화)을 결정해 일괄 적용하세요.
📝 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.
| export interface ArticleReadStatusResponse { | |
| acornCount: number; | |
| acornCollected: boolean; | |
| } | |
| import { ArticleReadStatusResponse } from '@shared/types/api'; | |
| export const putArticleReadStatus = async ( | |
| articleId: number | |
| ): Promise<ArticleReadStatusResponse> => { | |
| const { data } = await apiRequest.put<{ data: ArticleReadStatusResponse }>( | |
| `/api/v1/articles/${articleId}/readStatus` | |
| ); | |
| return data.data; | |
| }; |
🤖 Prompt for AI Agents
In apps/client/src/shared/types/api.ts around lines 16 to 19, the
ArticleReadStatusResponse interface is defined but the axios wrapper
putArticleReadStatus currently returns the full axios response (data) instead of
the inner payload (data.data), causing a runtime type mismatch; update
apps/client/src/shared/apis/axios.ts so putArticleReadStatus returns
response.data.data (i.e., the inner payload matching ArticleReadStatusResponse)
and ensure the function signature reflects that return type; additionally scan
and normalize other API wrappers (postSignUp/postCategory/putCategory, etc.) to
consistently return data.data per project policy.
| onClick?: () => void; | ||
| children: React.ReactNode; | ||
| } | ||
|
|
||
| const BaseCard = ({ children }: BaseCardProps) => { | ||
| const BaseCard = ({ children, onClick }: BaseCardProps) => { | ||
| return ( | ||
| <div className="border-gray200 w-[24.8rem] overflow-hidden rounded-[1.2rem] border bg-white"> | ||
| <div | ||
| onClick={onClick} | ||
| className="border-gray200 w-[24.8rem] overflow-hidden rounded-[1.2rem] border bg-white" | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
클릭 가능한 div의 접근성/키보드 조작 지원 보완.
onClick이 있을 때 role/tabIndex/키보드 핸들링이 없어 키보드 사용자가 사용할 수 없습니다. 최소한 Enter/Space를 지원하고 시각적으로도 클릭 가능함을 표현해 주세요.
-const BaseCard = ({ children, onClick }: BaseCardProps) => {
+const BaseCard = ({ children, onClick }: BaseCardProps) => {
+ const handleKeyDown = (e: React.KeyboardEvent<HTMLDivElement>) => {
+ if (!onClick) return;
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ onClick();
+ }
+ };
return (
- <div
- onClick={onClick}
- className="border-gray200 w-[24.8rem] overflow-hidden rounded-[1.2rem] border bg-white"
- >
+ <div
+ onClick={onClick}
+ onKeyDown={handleKeyDown}
+ role={onClick ? 'button' : undefined}
+ tabIndex={onClick ? 0 : undefined}
+ className={`border-gray200 w-[24.8rem] overflow-hidden rounded-[1.2rem] border bg-white ${onClick ? 'cursor-pointer' : ''}`}
+ >📝 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.
| onClick?: () => void; | |
| children: React.ReactNode; | |
| } | |
| const BaseCard = ({ children }: BaseCardProps) => { | |
| const BaseCard = ({ children, onClick }: BaseCardProps) => { | |
| return ( | |
| <div className="border-gray200 w-[24.8rem] overflow-hidden rounded-[1.2rem] border bg-white"> | |
| <div | |
| onClick={onClick} | |
| className="border-gray200 w-[24.8rem] overflow-hidden rounded-[1.2rem] border bg-white" | |
| > | |
| onClick?: () => void; | |
| children: React.ReactNode; | |
| } | |
| const BaseCard = ({ children, onClick }: BaseCardProps) => { | |
| const handleKeyDown = (e: React.KeyboardEvent<HTMLDivElement>) => { | |
| if (!onClick) return; | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| onClick(); | |
| } | |
| }; | |
| return ( | |
| <div | |
| onClick={onClick} | |
| onKeyDown={handleKeyDown} | |
| role={onClick ? 'button' : undefined} | |
| tabIndex={onClick ? 0 : undefined} | |
| className={`border-gray200 w-[24.8rem] overflow-hidden rounded-[1.2rem] border bg-white ${onClick ? 'cursor-pointer' : ''}`} | |
| > |
🤖 Prompt for AI Agents
In packages/design-system/src/components/card/BaseCard.tsx around lines 2 to 11,
the div with onClick is not keyboard-accessible or visually indicated as
interactive; when onClick is provided, make the element behave like a button by
conditionally adding role="button", tabIndex={0}, and a keydown handler that
triggers the onClick callback for Enter and Space keys, add a visible focus
style and cursor-pointer class to indicate clickability, and ensure no
interactive attributes are added when onClick is undefined.
| onClick, | ||
| onOptionsClick, | ||
| }: MyBookmarkCardProps) => { | ||
| return ( | ||
| <BaseCard> | ||
| <BaseCard onClick={onClick}> | ||
| <div className="flex h-[12rem] w-full items-center justify-center overflow-hidden bg-[#F8F8FA]"> |
There was a problem hiding this comment.
옵션 버튼 클릭 버블링 방지 누락.
RemindCard와 동일하게 옵션 버튼 클릭이 카드 onClick으로 전파될 수 있습니다. 전파를 막아 주세요.
- <button
+ <button
type="button"
aria-label="카테고리 상세"
className="cursor-pointer self-start"
- onClick={(e) => onOptionsClick?.(e)}
+ onClick={(e) => {
+ e.stopPropagation();
+ onOptionsClick?.(e);
+ }}
>📝 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.
| onClick, | |
| onOptionsClick, | |
| }: MyBookmarkCardProps) => { | |
| return ( | |
| <BaseCard> | |
| <BaseCard onClick={onClick}> | |
| <div className="flex h-[12rem] w-full items-center justify-center overflow-hidden bg-[#F8F8FA]"> | |
| onClick, | |
| onOptionsClick, | |
| }: MyBookmarkCardProps) => { | |
| return ( | |
| <BaseCard onClick={onClick}> | |
| <div className="flex h-[12rem] w-full items-center justify-center overflow-hidden bg-[#F8F8FA]"> | |
| <button | |
| type="button" | |
| aria-label="카테고리 상세" | |
| className="cursor-pointer self-start" | |
| onClick={(e) => { | |
| e.stopPropagation(); | |
| onOptionsClick?.(e); | |
| }} | |
| > |
🤖 Prompt for AI Agents
In packages/design-system/src/components/card/MyBookmarkCard.tsx around lines 21
to 26, the options button click currently bubbles up and triggers the card's
onClick (same as RemindCard); update the options button click handler to accept
the click event and call event.stopPropagation() before invoking the existing
onOptionsClick callback (or guard if undefined), ensuring the handler is passed
to the options button element so clicks do not propagate to the BaseCard
onClick.
| onClick, | ||
| onOptionsClick, | ||
| }: RemindCardProps) => { | ||
| return ( | ||
| <BaseCard> | ||
| <BaseCard onClick={onClick}> | ||
| <div className="bg-gray900 flex items-center gap-[0.4rem] py-[1.2rem] pl-[1.6rem] text-sm text-white"> |
There was a problem hiding this comment.
옵션 버튼 클릭 시 카드 onClick까지 전파되는 버그 가능성.
카드에 onClick이 추가되어 옵션 버튼 클릭이 부모로 버블링될 수 있습니다. 옵션 버튼 onClick에서 e.stopPropagation()을 호출해 주세요.
- <button
+ <button
type="button"
aria-label="카테고리 상세"
className="cursor-pointer self-start"
- onClick={(e) => onOptionsClick?.(e)}
+ onClick={(e) => {
+ e.stopPropagation();
+ onOptionsClick?.(e);
+ }}
>📝 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.
| onClick, | |
| onOptionsClick, | |
| }: RemindCardProps) => { | |
| return ( | |
| <BaseCard> | |
| <BaseCard onClick={onClick}> | |
| <div className="bg-gray900 flex items-center gap-[0.4rem] py-[1.2rem] pl-[1.6rem] text-sm text-white"> | |
| onClick, | |
| onOptionsClick, | |
| }: RemindCardProps) => { | |
| return ( | |
| <BaseCard onClick={onClick}> | |
| <div className="bg-gray900 flex items-center gap-[0.4rem] py-[1.2rem] pl-[1.6rem] text-sm text-white"> | |
| <button | |
| type="button" | |
| aria-label="카테고리 상세" | |
| className="cursor-pointer self-start" | |
| onClick={(e) => { | |
| e.stopPropagation(); | |
| onOptionsClick?.(e); | |
| }} | |
| > |
🤖 Prompt for AI Agents
In packages/design-system/src/components/card/RemindCard.tsx around lines 21 to
26, the options button click can bubble up to the card's onClick; update the
options button's onClick handler to accept the event parameter and call
e.stopPropagation() (typed as React.MouseEvent) before executing onOptionsClick
to prevent the parent card click from firing.
jjangminii
left a comment
There was a problem hiding this comment.
코드 깔끔하게 잘 구현하셨네요 👍 👍 처음에 query invalidate도 괜찮다고 생각했는데 확실히 한 파일 안에 api 연결하는게 많아지니 복잡하다는 생각이 드네요 이부분은 저도 더 공부해보겠습니다-!
6588975 to
e827bed
Compare
|
✅ Storybook chromatic 배포 확인: |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/design-system/src/components/card/BaseCard.tsx (1)
2-11: 클릭 가능한 카드의 접근성(키보드/role/tabIndex/포커스) 보완이 필요합니다.onClick만 있는 div는 스크린리더/키보드 사용자에게 비접근성입니다. Enter/Space 키 처리, role, tabIndex, 포커스 스타일을 추가해 주세요.
- <div - onClick={onClick} - className="border-gray200 w-[24.8rem] overflow-hidden rounded-[1.2rem] border bg-white" - > + <div + onClick={onClick} + onKeyDown={handleKeyDown} + role={onClick ? 'button' : undefined} + tabIndex={onClick ? 0 : undefined} + className={`border-gray200 w-[24.8rem] overflow-hidden rounded-[1.2rem] border bg-white ${onClick ? 'cursor-pointer focus:outline-none focus:ring-2 focus:ring-blue500' : ''}`} + >컴포넌트 내부에 다음 핸들러를 추가하세요:
const handleKeyDown = (e: React.KeyboardEvent<HTMLDivElement>) => { if (!onClick) return; if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); onClick(); } };apps/client/src/pages/remind/Remind.tsx (1)
80-93: 새 탭 열기 보안(noopener/noreferrer) + 이미 읽음 탭에서 불필요한 API 호출 방지reverse tabnabbing을 막고, activeBadge가 'read'일 때는 PUT 호출을 생략하세요.
- onClick={() => { - window.open(article.url, '_blank'); - - updateToReadStatus(article.articleId, { - onSuccess: () => { - queryClient.invalidateQueries({ - queryKey: ['remindArticles'], - }); - }, - onError: (error) => { - console.error(error); - }, - }); - }} + onClick={() => { + const win = window.open( + article.url, + '_blank', + 'noopener,noreferrer' + ); + if (win) win.opener = null; + + if (activeBadge === 'notRead') { + updateToReadStatus(article.articleId, { + onSuccess: () => { + queryClient.invalidateQueries({ + queryKey: ['remindArticles'], + }); + }, + onError: (error) => { + console.error(error); + }, + }); + } + }}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/client/src/pages/myBookmark/MyBookmark.tsx(2 hunks)apps/client/src/pages/remind/Remind.tsx(2 hunks)apps/client/src/shared/apis/axios.ts(1 hunks)apps/client/src/shared/apis/queries.ts(2 hunks)apps/client/src/shared/types/api.ts(1 hunks)packages/design-system/src/components/card/BaseCard.tsx(1 hunks)packages/design-system/src/components/card/MyBookmarkCard.tsx(1 hunks)packages/design-system/src/components/card/RemindCard.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/design-system/src/components/card/RemindCard.tsx
- apps/client/src/shared/types/api.ts
- apps/client/src/shared/apis/axios.ts
- apps/client/src/pages/myBookmark/MyBookmark.tsx
- apps/client/src/shared/apis/queries.ts
- packages/design-system/src/components/card/MyBookmarkCard.tsx
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: constantly-dev
PR: Pinback-Team/pinback-client#2
File: pnpm-workspace.yaml:3-3
Timestamp: 2025-08-18T13:48:59.065Z
Learning: constantly-dev는 docs 디렉터리를 컨벤션 문서 추가용으로 사용할 예정이라고 명시했습니다.
🧬 Code graph analysis (1)
apps/client/src/pages/remind/Remind.tsx (3)
apps/client/src/shared/utils/formatDateTime.ts (1)
formatLocalDateTime(1-9)apps/client/src/shared/apis/queries.ts (1)
usePutArticleReadStatus(77-85)apps/client/src/pages/remind/apis/queries.ts (1)
useGetRemindArticles(6-16)
🔇 Additional comments (1)
apps/client/src/pages/remind/Remind.tsx (1)
12-14: 새 뮤테이션/쿼리 클라이언트 의존성 추가 OK의도와 사용이 일관적입니다.
| const [activeBadge, setActiveBadge] = useState<'read' | 'notRead'>('notRead'); | ||
| const formattedDate = formatLocalDateTime(); | ||
|
|
||
| const queryClient = useQueryClient(); | ||
|
|
||
| const { mutate: updateToReadStatus } = usePutArticleReadStatus(); | ||
| const { data } = useGetRemindArticles( | ||
| formattedDate, | ||
| activeBadge === 'read', | ||
| 1, | ||
| 10 | ||
| ); |
There was a problem hiding this comment.
formattedDate가 매 렌더마다 바뀌어 쿼리 키가 계속 생성됩니다.
쿼리 키(['remindArticles', formattedDate, ...])가 리렌더마다 달라져 과도한 캐시/리패치가 발생합니다. 마운트 시 1회만 고정하세요.
- const formattedDate = formatLocalDateTime();
+ const [formattedDate] = useState(() => formatLocalDateTime());📝 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.
| const [activeBadge, setActiveBadge] = useState<'read' | 'notRead'>('notRead'); | |
| const formattedDate = formatLocalDateTime(); | |
| const queryClient = useQueryClient(); | |
| const { mutate: updateToReadStatus } = usePutArticleReadStatus(); | |
| const { data } = useGetRemindArticles( | |
| formattedDate, | |
| activeBadge === 'read', | |
| 1, | |
| 10 | |
| ); | |
| const [activeBadge, setActiveBadge] = useState<'read' | 'notRead'>('notRead'); | |
| const [formattedDate] = useState(() => formatLocalDateTime()); | |
| const queryClient = useQueryClient(); | |
| const { mutate: updateToReadStatus } = usePutArticleReadStatus(); | |
| const { data } = useGetRemindArticles( | |
| formattedDate, | |
| activeBadge === 'read', | |
| 1, | |
| 10 | |
| ); |
🤖 Prompt for AI Agents
In apps/client/src/pages/remind/Remind.tsx around lines 17-28, formattedDate is
recomputed on every render which makes the react-query key change each render
and causes excessive cache entries and refetches; compute and store the
formatted date once on mount (e.g., useState(() => formatLocalDateTime()) or
useRef/useMemo with empty deps) and use that stable value in the
useGetRemindArticles query key and call so the key stays constant across
renders.
|
|
||
| export interface ArticleReadStatusResponse { | ||
| acornCount: number; | ||
| acornCollected: boolean; | ||
| } |
There was a problem hiding this comment.
요거 /aritcles/{articleId}/readStatus메서드인 것 같은데
받는데이터가 해당 타입이 맞을까요?? api 노션상에서는 다음과 같아서요!
"isRead" : true,
"finalAcornCount": 1, //현재 최종 도토리 개수
"isCollected": false // 해당 요청으로 인해 도토리가 수집되었는지 여부
| interface BaseCardProps { | ||
| onClick?: () => void; | ||
| children: React.ReactNode; | ||
| } | ||
|
|
||
| const BaseCard = ({ children }: BaseCardProps) => { | ||
| const BaseCard = ({ children, onClick }: BaseCardProps) => { |
There was a problem hiding this comment.
그냥 onClick이 아닌, 좀더 네이밍이 어떤 기능인지에 맞게 지어주면 좋을 것 같다는 생각도 듭니다!
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
크게 이슈되는 것은 없는데, query invalidate 하는 로직이 너무 지저분해지고 쿼리키 관리가 너무 어렵네요.
그래서 이번 스프린트 마무리하고
관련 리팩토링 진행하겠습니다!
📷 Screenshot
2025-09-13.11.00.41.mov
Summary by CodeRabbit
신규 기능
리팩터링
디자인 시스템