Feat(Client): jobpin memo popup 구현 및 api 연결#288
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough잡핀 카드 클릭 시 아티클 상세를 on‑demand로 조회하는 뮤테이션 훅과 상세 응답 타입을 추가하고, 조회 결과를 표시하는 MemoPopup 모달 컴포넌트를 도입했습니다. JobPins 페이지는 카드 클릭으로 상세를 가져와 팝업을 표시하도록 변경되었습니다. Changes
Sequence DiagramsequenceDiagram
participant User as 사용자
participant UI as JobPins 페이지
participant API as API 서버
participant Popup as MemoPopup
User->>UI: 카드 클릭 (articleId)
UI->>API: getJobPinsArticleDetail(articleId)
API-->>UI: JobPinsDetailResponse (articleId, ownerName, memo, url)
UI->>Popup: jobPinDetail 전달 및 모달 표시
Popup-->>User: 모달 렌더(유저명, memo)
User->>Popup: "기사 보기" 클릭
Popup->>User: 새 탭에서 url 열기
User->>Popup: 닫기 클릭
Popup->>UI: onClose (상태 리셋)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 4
🧹 Nitpick comments (3)
apps/client/src/pages/jobPins/components/MemoPopup.tsx (2)
25-27: 접근성 개선: 닫기 버튼에aria-label추가 권장스크린 리더 사용자를 위해 닫기 버튼에 명시적인 레이블을 추가하면 좋습니다.
♿ 접근성 개선 제안
- <button onClick={onClose}> + <button onClick={onClose} aria-label="닫기"> <Icon name="ic_close" size={20} /> </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/jobPins/components/MemoPopup.tsx` around lines 25 - 27, The close button in the MemoPopup component lacks an accessible name for screen readers; update the button element in MemoPopup (the onClick handler rendering Icon "ic_close") to include a descriptive aria-label (for example aria-label="Close" or aria-label="Close memo") so assistive technologies can announce its purpose; ensure the aria-label is concise and use the same accessible text for any automated tests or stories that reference this button.
19-49: 모달 접근성: Escape 키 핸들링 및 포커스 관리 고려현재 모달은 오버레이 클릭이나 Escape 키로 닫히지 않습니다. 사용자 경험과 접근성을 위해 다음 기능을 추가하는 것을 고려해 주세요:
- Escape 키로 모달 닫기
- 백드롭 클릭으로 모달 닫기 (선택적)
⌨️ Escape 키 핸들링 추가 예시
+import { useEffect } from 'react'; import { Icon } from '@pinback/design-system/icons'; import { Button } from '@pinback/design-system/ui';export default function MemoPopup({ userName, memo, onClose, onGoArticle, }: MemoPopupProps) { const hasMemo = memo && memo.trim().length > 0; + + useEffect(() => { + const handleKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') onClose(); + }; + document.addEventListener('keydown', handleKeyDown); + return () => document.removeEventListener('keydown', handleKeyDown); + }, [onClose]); return (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/jobPins/components/MemoPopup.tsx` around lines 19 - 49, MemoPopup should support closing via Escape and backdrop click and manage focus: in MemoPopup add a ref for the modal container and for the close button, on mount focus the close button (and restore previously focused element on unmount), install a keydown listener in useEffect that calls onClose when event.key === 'Escape', and clean up the listener on unmount; make the outer overlay div listen for onClick and call onClose only when the click target is the overlay (i.e., ignore clicks inside the inner modal box by stopping propagation on the modal content div), and ensure to reference the existing onClose and modal container refs in this logic so hasMemo, memo, userName, and onGoArticle remain unchanged.apps/client/src/pages/jobPins/apis/queries.ts (1)
24-28:useMutation대신useQuery사용 고려단순 조회(GET) 요청에
useMutation을 사용하고 있습니다. 이 방식도 동작하지만,useQuery를 사용하면 캐싱, 자동 재시도, stale 데이터 관리 등의 이점을 얻을 수 있습니다.현재 방식이 의도된 설계라면 (명시적 트리거, 캐싱 불필요) 유지해도 됩니다.
💡 useQuery 대안 (enabled 옵션 활용)
export const useGetJobPinsArticleDetail = (articleId: number | null) => { return useQuery<JobPinsDetailResponse>({ queryKey: ['jobPinDetail', articleId], queryFn: () => getJobPinsArticleDetail(articleId!), enabled: articleId !== null, }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/jobPins/apis/queries.ts` around lines 24 - 28, The hook useGetJobPinsArticleDetail currently uses useMutation for a simple GET; replace it with useQuery to benefit from caching/retries/stale management by changing useGetJobPinsArticleDetail to accept articleId (number | null), call useQuery with a unique queryKey like ['jobPinDetail', articleId] and queryFn that calls getJobPinsArticleDetail(articleId!), and set enabled: articleId !== null so the query only runs when an id is provided; keep the JobPinsDetailResponse generic type on useQuery.
🤖 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/jobPins/apis/axios.ts`:
- Around line 24-28: There are two identical declarations of the generic
interface ApiResponse<T> in this file; remove the duplicate declaration so only
one ApiResponse<T> remains (keep the preferred declaration and delete the
other), and update any local references to use that single interface; search for
"ApiResponse" to ensure no accidental shadowing and remove the redundant block
at the location matching the second declaration (the duplicate interface
definition).
In `@apps/client/src/pages/jobPins/JobPins.tsx`:
- Line 5: The import path for the MemoPopup component uses incorrect
casing—update the import statement that currently references "memoPopup" to
match the actual filename "MemoPopup.tsx" (e.g., import MemoPopup from
'@pages/jobPins/components/MemoPopup';) so the module name exactly matches the
exported file (MemoPopup) and resolves correctly on case-sensitive CI systems;
check the import in JobPins.tsx and ensure any other references use the same
PascalCase.
- Line 99: Replace the forced full-page reload in the onClose handler with a
mutation state reset so the UI updates without losing scroll or other page
state: locate the onClose={() => window.location.reload()} usage in JobPins.tsx
(the modal/popup close handler) and call the appropriate mutation.reset() (e.g.,
pinsMutation.reset() or the specific useMutation instance used for pin
creation/deletion) and then close the modal normally; if you need to refresh
server data instead, call queryClient.invalidateQueries(...) for the pins query
rather than window.location.reload().
- Around line 16-17: The mutation returned by useGetJobPinsArticleDetail
(destructured as getJobPinDetail and jobPinDetail) currently lacks error
handling; update the mutation invocation or hook options to handle failures
(e.g., provide an onError callback or wrap mutate calls with
try/catch/then.catch) and surface feedback to the user (toast, snackbar, or set
an error state displayed in JobPins UI) so API errors from getJobPinDetail are
logged and shown; ensure you reference
useGetJobPinsArticleDetail/getJobPinDetail when adding the onError handler and
update jobPinDetail/error state handling in the component render to display the
message.
---
Nitpick comments:
In `@apps/client/src/pages/jobPins/apis/queries.ts`:
- Around line 24-28: The hook useGetJobPinsArticleDetail currently uses
useMutation for a simple GET; replace it with useQuery to benefit from
caching/retries/stale management by changing useGetJobPinsArticleDetail to
accept articleId (number | null), call useQuery with a unique queryKey like
['jobPinDetail', articleId] and queryFn that calls
getJobPinsArticleDetail(articleId!), and set enabled: articleId !== null so the
query only runs when an id is provided; keep the JobPinsDetailResponse generic
type on useQuery.
In `@apps/client/src/pages/jobPins/components/MemoPopup.tsx`:
- Around line 25-27: The close button in the MemoPopup component lacks an
accessible name for screen readers; update the button element in MemoPopup (the
onClick handler rendering Icon "ic_close") to include a descriptive aria-label
(for example aria-label="Close" or aria-label="Close memo") so assistive
technologies can announce its purpose; ensure the aria-label is concise and use
the same accessible text for any automated tests or stories that reference this
button.
- Around line 19-49: MemoPopup should support closing via Escape and backdrop
click and manage focus: in MemoPopup add a ref for the modal container and for
the close button, on mount focus the close button (and restore previously
focused element on unmount), install a keydown listener in useEffect that calls
onClose when event.key === 'Escape', and clean up the listener on unmount; make
the outer overlay div listen for onClick and call onClose only when the click
target is the overlay (i.e., ignore clicks inside the inner modal box by
stopping propagation on the modal content div), and ensure to reference the
existing onClose and modal container refs in this logic so hasMemo, memo,
userName, and onGoArticle remain unchanged.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/client/src/pages/jobPins/JobPins.tsxapps/client/src/pages/jobPins/apis/axios.tsapps/client/src/pages/jobPins/apis/queries.tsapps/client/src/pages/jobPins/components/MemoPopup.tsx
| const { mutate: getJobPinDetail, data: jobPinDetail } = | ||
| useGetJobPinsArticleDetail(); |
There was a problem hiding this comment.
mutation 에러 처리 누락
useGetJobPinsArticleDetail 호출 시 에러 상태를 처리하지 않고 있습니다. API 호출 실패 시 사용자에게 피드백이 없습니다.
🛡️ 에러 처리 추가 예시
- const { mutate: getJobPinDetail, data: jobPinDetail } =
+ const { mutate: getJobPinDetail, data: jobPinDetail, error: jobPinDetailError, reset: resetJobPinDetail } =
useGetJobPinsArticleDetail();
+
+ // 에러 발생 시 처리 (토스트 등)
+ useEffect(() => {
+ if (jobPinDetailError) {
+ // 에러 알림 표시
+ console.error('Failed to fetch article detail:', jobPinDetailError);
+ }
+ }, [jobPinDetailError]);📝 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 { mutate: getJobPinDetail, data: jobPinDetail } = | |
| useGetJobPinsArticleDetail(); | |
| const { mutate: getJobPinDetail, data: jobPinDetail, error: jobPinDetailError, reset: resetJobPinDetail } = | |
| useGetJobPinsArticleDetail(); | |
| // 에러 발생 시 처리 (토스트 등) | |
| useEffect(() => { | |
| if (jobPinDetailError) { | |
| // 에러 알림 표시 | |
| console.error('Failed to fetch article detail:', jobPinDetailError); | |
| } | |
| }, [jobPinDetailError]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/client/src/pages/jobPins/JobPins.tsx` around lines 16 - 17, The mutation
returned by useGetJobPinsArticleDetail (destructured as getJobPinDetail and
jobPinDetail) currently lacks error handling; update the mutation invocation or
hook options to handle failures (e.g., provide an onError callback or wrap
mutate calls with try/catch/then.catch) and surface feedback to the user (toast,
snackbar, or set an error state displayed in JobPins UI) so API errors from
getJobPinDetail are logged and shown; ensure you reference
useGetJobPinsArticleDetail/getJobPinDetail when adding the onError handler and
update jobPinDetail/error state handling in the component render to display the
message.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
apps/client/src/pages/jobPins/JobPins.tsx (2)
94-102:⚠️ Potential issue | 🟠 Major
window.location.reload()대신 mutation 상태 초기화 권장팝업을 닫을 때
window.location.reload()를 호출하면 전체 페이지가 리로드되어 스크롤 위치와 기존 데이터가 모두 손실됩니다. mutation의reset함수를 사용하면 상태만 초기화하여 더 나은 UX를 제공할 수 있습니다.♻️ 개선된 구현
- const { mutate: getJobPinDetail, data: jobPinDetail } = + const { mutate: getJobPinDetail, data: jobPinDetail, reset: resetJobPinDetail } = useGetJobPinsArticleDetail();{jobPinDetail && ( <MemoPopup userName={jobPinDetail.ownerName} memo={jobPinDetail.memo} - onClose={() => window.location.reload()} + onClose={() => resetJobPinDetail()} onGoArticle={() => window.open(jobPinDetail.url, '_blank')} /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/jobPins/JobPins.tsx` around lines 94 - 102, Replace the full-page reload in the MemoPopup onClose handler with a targeted mutation reset to avoid losing scroll position and data; locate the MemoPopup usage where onClose currently calls window.location.reload() and instead call the appropriate mutation's reset (or call its reset and/or refetch) that manages job pin detail state (e.g., the mutation or query hook that provides reset for job pin list/detail), ensuring you pass that reset function into the onClose handler (or wrap it in a local callback) so closing the popup only clears the mutation state rather than reloading the page.
16-17:⚠️ Potential issue | 🟡 MinorMutation 에러 처리 누락
API 호출 실패 시 사용자에게 피드백이 없습니다.
error상태를 구조 분해하여 에러 발생 시 토스트 알림 등으로 사용자에게 알려주는 것이 좋습니다.- const { mutate: getJobPinDetail, data: jobPinDetail } = + const { mutate: getJobPinDetail, data: jobPinDetail, error: jobPinDetailError, reset: resetJobPinDetail } = useGetJobPinsArticleDetail();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/jobPins/JobPins.tsx` around lines 16 - 17, The component uses useGetJobPinsArticleDetail() but does not handle API errors; update the hook usage to destructure the error (and isError) from useGetJobPinsArticleDetail() or supply an onError handler to the mutate call (getJobPinDetail) and show a user-facing notification (e.g., toast) when an error occurs; reference the useGetJobPinsArticleDetail hook, the mutate function getJobPinDetail, and the jobPinDetail state so the error path triggers a toast with a clear message and optionally logs the error for debugging.
🤖 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/jobPins/JobPins.tsx`:
- Around line 94-102: Replace the full-page reload in the MemoPopup onClose
handler with a targeted mutation reset to avoid losing scroll position and data;
locate the MemoPopup usage where onClose currently calls
window.location.reload() and instead call the appropriate mutation's reset (or
call its reset and/or refetch) that manages job pin detail state (e.g., the
mutation or query hook that provides reset for job pin list/detail), ensuring
you pass that reset function into the onClose handler (or wrap it in a local
callback) so closing the popup only clears the mutation state rather than
reloading the page.
- Around line 16-17: The component uses useGetJobPinsArticleDetail() but does
not handle API errors; update the hook usage to destructure the error (and
isError) from useGetJobPinsArticleDetail() or supply an onError handler to the
mutate call (getJobPinDetail) and show a user-facing notification (e.g., toast)
when an error occurs; reference the useGetJobPinsArticleDetail hook, the mutate
function getJobPinDetail, and the jobPinDetail state so the error path triggers
a toast with a clear message and optionally logs the error for debugging.
into feat/#286/jobpin-memo-popup
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/client/src/pages/jobPins/JobPins.tsx (1)
18-22:⚠️ Potential issue | 🟡 Minor상세 조회 실패 상태를 UI에 노출해 주세요.
상세 조회 mutation의 에러/로딩 상태를 처리하지 않아 실패 시 무반응처럼 보일 수 있습니다. 토스트 또는 인라인 에러 메시지와 로딩 상태를 함께 연결해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/jobPins/JobPins.tsx` around lines 18 - 22, The job-pin detail mutation (useGetJobPinsArticleDetail -> mutate: getJobPinDetail, data: jobPinDetail, reset: resetJobPinDetail) currently ignores loading/error states causing silent failures; update the component to read and handle the hook's status flags (e.g., isLoading/isError/error) and surface them in the UI by showing a loading indicator while fetching, disabling relevant interactions, and displaying an inline error or toast on error (with the error message) and a retry action that calls getJobPinDetail again or resetJobPinDetail; ensure the UI clears error state when the modal/panel closes so subsequent opens start fresh.
🧹 Nitpick comments (2)
apps/client/src/pages/jobPins/JobPins.tsx (1)
62-64: 제목 fallback 조건과 표시값을 동일하게 정제하는 편이 안전합니다.현재는 조건만
trim()하고 실제 렌더링은 원문 문자열이라, 공백/개행이 포함된 제목이 그대로 노출될 수 있습니다. 표시값도trim()한 값을 쓰도록 맞추는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/jobPins/JobPins.tsx` around lines 62 - 64, The fallback currently trims only the condition but renders article.title raw, so change displayTitle to use the trimmed value consistently: compute a trimmedTitle from article.title (e.g., article.title?.trim()) and set displayTitle to trimmedTitle if truthy else '제목 없음', ensuring both the condition and the rendered value use the same cleaned string (referencing article.title and displayTitle).apps/client/src/pages/jobPins/apis/axios.ts (1)
14-22:as단언 대신apiRequest.get<T>()제네릭으로 타입을 지정해 주세요.현재 방식은 런타임 응답 형태 불일치를 컴파일 단계에서 충분히 감지하지 못합니다. Axios 제네릭을 활용하여 응답 타입을 직접 지정하는 것이 더 안전합니다.
♻️ 권장 수정안
export const getJobPinsArticles = async ( page: number, size: number ): Promise<JobPinsResponse> => { - const { data } = await apiRequest.get('/api/v3/articles/shared/job', { + const { data } = await apiRequest.get<ApiResponse<JobPinsResponse>>( + '/api/v3/articles/shared/job', + { params: { page, size, }, - }); + } + ); - return (data as ApiResponse<JobPinsResponse>).data; + return data.data; }; @@ export const getJobPinsArticleDetail = async ( articleId: number ): Promise<JobPinsDetailResponse> => { - const { data } = await apiRequest.get( + const { data } = await apiRequest.get<ApiResponse<JobPinsDetailResponse>>( `/api/v3/articles/shared/job/${articleId}` ); - return (data as ApiResponse<JobPinsDetailResponse>).data; + 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/jobPins/apis/axios.ts` around lines 14 - 22, Replace the runtime-unsafe cast by passing the response type into the Axios call: call apiRequest.get with the generic ApiResponse<JobPinsResponse> so the returned "data" is correctly typed (refer to apiRequest.get, ApiResponse, JobPinsResponse and the local variable data), then return data.data instead of using "as" casting; update the GET call signature and subsequent return to rely on the inferred typed response.
🤖 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/jobPins/JobPins.tsx`:
- Line 101: The onGoArticle handler in JobPins.tsx (and the similar handlers in
Remind.tsx and MyBookmarkContent) opens links with window.open(jobPinDetail.url,
'_blank') which is vulnerable to tab‑hijacking; update these handlers to include
the security features by passing the third parameter with "noopener,noreferrer"
(i.e., use window.open(url, '_blank', 'noopener,noreferrer')) or alternatively
set the link target with rel="noopener noreferrer" when rendering an anchor;
locate the onGoArticle usage in JobPins.tsx and the equivalent functions in
Remind.tsx and MyBookmarkContent and apply the same change to all instances.
---
Duplicate comments:
In `@apps/client/src/pages/jobPins/JobPins.tsx`:
- Around line 18-22: The job-pin detail mutation (useGetJobPinsArticleDetail ->
mutate: getJobPinDetail, data: jobPinDetail, reset: resetJobPinDetail) currently
ignores loading/error states causing silent failures; update the component to
read and handle the hook's status flags (e.g., isLoading/isError/error) and
surface them in the UI by showing a loading indicator while fetching, disabling
relevant interactions, and displaying an inline error or toast on error (with
the error message) and a retry action that calls getJobPinDetail again or
resetJobPinDetail; ensure the UI clears error state when the modal/panel closes
so subsequent opens start fresh.
---
Nitpick comments:
In `@apps/client/src/pages/jobPins/apis/axios.ts`:
- Around line 14-22: Replace the runtime-unsafe cast by passing the response
type into the Axios call: call apiRequest.get with the generic
ApiResponse<JobPinsResponse> so the returned "data" is correctly typed (refer to
apiRequest.get, ApiResponse, JobPinsResponse and the local variable data), then
return data.data instead of using "as" casting; update the GET call signature
and subsequent return to rely on the inferred typed response.
In `@apps/client/src/pages/jobPins/JobPins.tsx`:
- Around line 62-64: The fallback currently trims only the condition but renders
article.title raw, so change displayTitle to use the trimmed value consistently:
compute a trimmedTitle from article.title (e.g., article.title?.trim()) and set
displayTitle to trimmedTitle if truthy else '제목 없음', ensuring both the condition
and the rendered value use the same cleaned string (referencing article.title
and displayTitle).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/client/src/pages/jobPins/JobPins.tsxapps/client/src/pages/jobPins/apis/axios.ts
| userName={jobPinDetail.ownerName} | ||
| memo={jobPinDetail.memo} | ||
| onClose={resetJobPinDetail} | ||
| onGoArticle={() => window.open(jobPinDetail.url, '_blank')} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 1. 지정된 파일의 line 101 확인
head -105 apps/client/src/pages/jobPins/JobPins.tsx | tail -10Repository: Pinback-Team/pinback-client
Length of output: 337
🏁 Script executed:
# 2. _blank 사용 시 noopener/noreferrer 누락 지점 확인
echo "=== All window.open with '_blank' ==="
rg -nP "window\.open\([^)]*['\"]_blank['\"]\s*\)" --type ts --type tsx
echo ""
echo "=== window.open with security parameters ==="
rg -nP "window\.open\([^)]*['\"]_blank['\"]\s*,\s*['\"][^'\"]*noopener[^'\"]*['\"]\s*\)" --type ts --type tsxRepository: Pinback-Team/pinback-client
Length of output: 227
🏁 Script executed:
# 3. 파일 존재 여부 확인
test -f apps/client/src/pages/jobPins/JobPins.tsx && echo "파일 존재함" || echo "파일 없음"Repository: Pinback-Team/pinback-client
Length of output: 77
🏁 Script executed:
# window.open() 사용 패턴 전체 검색 (파일 타입 필터 제거)
rg "window\.open" --no-type-list -A 1Repository: Pinback-Team/pinback-client
Length of output: 514
🏁 Script executed:
# 보안 파라미터 없이 '_blank' 사용하는 인스턴스 확인
rg "window\.open\([^)]*_blank[^)]*\)" --no-type-listRepository: Pinback-Team/pinback-client
Length of output: 514
🏁 Script executed:
# window.open 사용 패턴 전체 검색
rg "window\.open" -A 1Repository: Pinback-Team/pinback-client
Length of output: 650
🏁 Script executed:
# '_blank' 사용 시 보안 파라미터 여부 확인
rg "window\.open.*_blank"Repository: Pinback-Team/pinback-client
Length of output: 410
새 탭 오픈 시 보안 파라미터 noopener,noreferrer 추가 필요
window.open(..., '_blank')만 사용하면 opener를 통한 탭 하이재킹 공격에 취약합니다.
권장 수정안
- onGoArticle={() => window.open(jobPinDetail.url, '_blank')}
+ onGoArticle={() =>
+ window.open(jobPinDetail.url, '_blank', 'noopener,noreferrer')
+ }동일한 패턴의 취약점이 다른 파일에도 존재합니다:
apps/client/src/pages/remind/Remind.tsxapps/client/src/pages/myBookmark/components/myBookmarkContent/MyBookmarkContent.tsx
모든 인스턴스에 보안 파라미터를 추가해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/client/src/pages/jobPins/JobPins.tsx` at line 101, The onGoArticle
handler in JobPins.tsx (and the similar handlers in Remind.tsx and
MyBookmarkContent) opens links with window.open(jobPinDetail.url, '_blank')
which is vulnerable to tab‑hijacking; update these handlers to include the
security features by passing the third parameter with "noopener,noreferrer"
(i.e., use window.open(url, '_blank', 'noopener,noreferrer')) or alternatively
set the link target with rel="noopener noreferrer" when rendering an anchor;
locate the onGoArticle usage in JobPins.tsx and the equivalent functions in
Remind.tsx and MyBookmarkContent and apply the same change to all instances.
| <MemoPopup | ||
| userName={jobPinDetail.ownerName} | ||
| memo={jobPinDetail.memo} | ||
| onClose={resetJobPinDetail} |
There was a problem hiding this comment.
여기서 이렇게 reset을 사용해주셨군요! 👍
| export const useGetJobPinsArticleDetail = () => { | ||
| return useMutation<JobPinsDetailResponse, Error, number>({ | ||
| mutationFn: (articleId: number) => getJobPinsArticleDetail(articleId), | ||
| }); | ||
| }; |
There was a problem hiding this comment.
지금 막 생각을 해보니까 useMutation과 useQuery에 대한 선택을 이유에 따라 더 고민해봐도 좋을 것 같아요!
사실 단순하게 생각하면 아티클 카드를 클릭하는 트리거를 통해 데이터를 가져오니까 useMutation을 쓸 수 있겠지만, useMutation은 useQuery처럼 캐싱이 없으니까요!
물론 직무 아티클이 사용자가 바꾸면 변동이 있지만 메타데이터인 만큼 사실 변동이 자주 있는 것은 아닌 것 같아서, 캐시를 못 쓰고 이를 클릭할 때마다 데이터를 새롭게 받아오면 그만큼 로딩이 느려지는 UX 문제가 있을 것 같아요.
따라서 이 경계를 저희 기능/문제에 따라 잘 판단해야 할 것 같아요.
만약 이 로딩이 꽤 느리게 느껴진다면 캐시는 하되 열 때/포커스 때 최신화는 강하게 설정해서 useQuery를 이용할 수 있을 것 같아요.
queryKey에 id도 추가하고, staleTime/refetchOnMount/gcTime 등을 적절하게 조절하면 캐싱을 하면서도 최신 데이터를 잘 유지할 수 있다고 생각합니다! 아니면 enabled를 false로 둬서 처음은 패칭 안하고, 이후 refetch function을 통해 클릭하면 데이터 불러오게 하면서도 캐싱을 할 수 있지 않을까?? 라는 생각도 들어요!
[예시 코드]
const { data, refetch } = useQuery({
queryKey: ['searchResult'],
queryFn: fetchSearch,
enabled: false, // 처음에는 자동으로 실행되지 않음!
});
// 버튼 클릭 시 호출
const handleClick = () => {
refetch(); // 이때 데이터를 가져오고, 결과는 캐싱
};이 부분도 다시 한번 생각해보시고 의견 내주시면 좋을 것 같아요~~
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
테스트를 못해봤어요..
📷 Screenshot
Summary by CodeRabbit
새로운 기능
개선