Feat(client): job selection popup open condition change#291
Merged
constantly-dev merged 5 commits intodevelopfrom Feb 28, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughLayout에서 인증·정책 페이지 여부와 로컬 토큰을 기준으로 서버의 hasJob 상태를 조회해 로그인한 사용자에게 JobSelectionFunnel 모달을 조건부로 전체화면 오버레이로 렌더링하고, 펀넬 완료 시 hasJob 쿼리를 무효화하도록 변경했습니다. Remind 페이지의 로컬 펀넬 로직은 제거되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Layout as Layout
participant QueryHook as useGetHasJob
participant API as GET /api/v3/users/job
participant Modal as JobSelectionFunnel
User->>Layout: 앱 접근
rect rgba(120,160,200,0.5)
Note over Layout: 토큰으로 isLoggedIn 결정\nisAuthPage/isPolicyPage 판단
alt isLoggedIn && !isAuthPage
Layout->>QueryHook: fetch hasJob (enabled=true)
QueryHook->>API: GET /api/v3/users/job
API-->>QueryHook: { hasJob: false }
QueryHook-->>Layout: hasJob 데이터 반환
end
end
rect rgba(160,120,200,0.5)
alt shouldShowJobSelectionFunnel
Layout->>Modal: show JobSelectionFunnel
Modal->>User: 직무 선택 UI 표시
User->>Modal: 선택 완료
Modal-->>Layout: onComplete
end
end
rect rgba(100,200,150,0.5)
Layout->>QueryHook: queryClient.invalidateQueries(['hasJob'])
QueryHook->>API: GET /api/v3/users/job (re-fetch)
API-->>QueryHook: { hasJob: true }
QueryHook-->>Layout: 업데이트
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 배포 확인: |
![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
🤖 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/layout/Layout.tsx`:
- Around line 24-33: The funnel can open on policy/terms pages; update the
shouldShowJobSelectionFunnel condition to exclude legal pages by adding checks
like && !isPolicyPage && !isTermsPage (or use a single isLegalPage flag) so the
expression becomes: isLoggedIn && !isAuthPage && !isPolicyPage && !isTermsPage
&& !isHasJobLoading && hasJobData?.hasJob === false; ensure
isPolicyPage/isTermsPage (or derived isLegalPage) are defined/derived in the
same component before use.
In `@apps/client/src/shared/apis/queries.ts`:
- Around line 187-194: The hasJob query uses a global key ['hasJob'] which
causes cached results to leak across user sessions; update useGetHasJob to
include a per-user identifier in its queryKey (e.g., ['hasJob', userId] or
['hasJob', getCurrentAccountId()]) so the cache is scoped to the active user
(ensure getHasJob accepts/uses that id if needed), or alternatively ensure
logout logic calls queryClient.invalidateQueries({ queryKey: ['hasJob'] }) in
the ProfilePopup logout handler; modify useGetHasJob (and any callers) to pass
and use the user identifier and/or add the invalidate call to prevent
cross-account cache reuse.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/client/src/layout/Layout.tsxapps/client/src/pages/remind/Remind.tsxapps/client/src/shared/apis/axios.tsapps/client/src/shared/apis/queries.tsapps/client/src/shared/types/api.ts
💤 Files with no reviewable changes (1)
- apps/client/src/pages/remind/Remind.tsx
Comment on lines
24
to
33
| const { data: hasJobData, isLoading: isHasJobLoading } = useGetHasJob( | ||
| isLoggedIn && !isAuthPage | ||
| ); | ||
|
|
||
| const shouldShowJobSelectionFunnel = | ||
| isLoggedIn && | ||
| !isAuthPage && | ||
| !isHasJobLoading && | ||
| hasJobData?.hasJob === false; | ||
|
|
There was a problem hiding this comment.
정책/약관 페이지에서도 퍼널이 열릴 수 있어 법적 문서 접근을 막을 수 있습니다.
현재 조건은 Line [29]-[32]에서 !isPolicyPage를 제외하지 않아 /policy, /terms에서도 모달이 뜰 수 있습니다. 해당 경로는 퍼널에서 제외하는 게 안전합니다.
🔧 제안 수정안
const { data: hasJobData, isLoading: isHasJobLoading } = useGetHasJob(
- isLoggedIn && !isAuthPage
+ isLoggedIn && !isAuthPage && !isPolicyPage
);
const shouldShowJobSelectionFunnel =
isLoggedIn &&
!isAuthPage &&
+ !isPolicyPage &&
!isHasJobLoading &&
hasJobData?.hasJob === false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/client/src/layout/Layout.tsx` around lines 24 - 33, The funnel can open
on policy/terms pages; update the shouldShowJobSelectionFunnel condition to
exclude legal pages by adding checks like && !isPolicyPage && !isTermsPage (or
use a single isLegalPage flag) so the expression becomes: isLoggedIn &&
!isAuthPage && !isPolicyPage && !isTermsPage && !isHasJobLoading &&
hasJobData?.hasJob === false; ensure isPolicyPage/isTermsPage (or derived
isLegalPage) are defined/derived in the same component before use.
There was a problem hiding this comment.
@jjangminii 네, 맞습니다! CodeRabbit이 지적한 부분이 타당한 이슈입니다.
현재 코드를 보면 isPolicyPage가 정의되어 있지만, 직무 선택 퍼널을 표시하는 조건에서는 제외되지 않고 있습니다. 이로 인해 로그인한 사용자가 이용약관이나 개인정보처리방침 페이지를 볼 때도 퍼널 모달이 나타날 수 있어서, 법적 문서 접근을 방해할 수 있는 문제가 있습니다.
다음과 같이 수정하는 것을 권장합니다:
const { data: hasJobData, isLoading: isHasJobLoading } = useGetHasJob(
isLoggedIn && !isAuthPage && !isPolicyPage
);
const shouldShowJobSelectionFunnel =
isLoggedIn &&
!isAuthPage &&
!isPolicyPage &&
!isHasJobLoading &&
hasJobData?.hasJob === false;이렇게 하면:
- 정책/약관 페이지에서는
hasJobAPI 요청 자체를 하지 않음 - 해당 페이지에서는 퍼널이 절대 표시되지 않음
- 사용자가 법적 문서를 자유롭게 확인할 수 있음
Member
Author
There was a problem hiding this comment.
엇 그렇네요! 해당 부분 조건 추가하겠습니다 👍
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/client/src/shared/apis/queries.ts (1)
187-195:⚠️ Potential issue | 🟠 Major
hasJob쿼리 키를 사용자 스코프로 분리하는 편이 안전합니다.Line 191의
queryKey: ['hasJob']는 전역 키라서, 인증 전환 경로 중 캐시 정리가 누락되면 이전 사용자 값 재사용 위험이 남습니다.['hasJob', userId]형태로 스코프를 분리하거나, 모든 토큰 전환 경로에서['hasJob']제거를 강제해 주세요.#!/bin/bash # 목적: # 1) hasJob 쿼리 키가 사용자 식별자를 포함하는지 확인 # 2) 인증 상태 전환(로그인/로그아웃/토큰 교체) 경로에서 hasJob 캐시 정리가 보장되는지 확인 echo "== hasJob query key usage ==" rg -n --type ts --type tsx "queryKey:\s*\[\s*'hasJob'" echo echo "== hasJob cache invalidation/removal/clear ==" rg -n --type ts --type tsx "invalidateQueries\(|removeQueries\(|clear\(\)" apps/client/src -A3 -B2 | rg -n "hasJob|queryClient" echo echo "== auth/token transition points ==" rg -n --type ts --type tsx "setItem\('token'|removeItem\('token'|logout|signOut|login|postSignUp" apps/client/src -A4 -B2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/shared/apis/queries.ts` around lines 187 - 195, The hasJob query is currently using a global cache key in useGetHasJob (queryKey: ['hasJob']), which can leak prior-user data across auth transitions; update useGetHasJob to include a per-user scope (e.g., queryKey: ['hasJob', userId] where userId is sourced from the same auth state used by getHasJob) or, if you cannot add userId, ensure every auth/token transition path explicitly invalidates/removes that key (calls to queryClient.invalidateQueries(['hasJob']) / removeQueries(['hasJob']) wherever login/logout/token swap occurs). Locate useGetHasJob and the getHasJob caller, add the user identifier into the key or add missing invalidate/remove calls at auth transition points to guarantee cache is cleared.
🤖 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/shared/apis/queries.ts`:
- Around line 187-195: The hasJob query is currently using a global cache key in
useGetHasJob (queryKey: ['hasJob']), which can leak prior-user data across auth
transitions; update useGetHasJob to include a per-user scope (e.g., queryKey:
['hasJob', userId] where userId is sourced from the same auth state used by
getHasJob) or, if you cannot add userId, ensure every auth/token transition path
explicitly invalidates/removes that key (calls to
queryClient.invalidateQueries(['hasJob']) / removeQueries(['hasJob']) wherever
login/logout/token swap occurs). Locate useGetHasJob and the getHasJob caller,
add the user identifier into the key or add missing invalidate/remove calls at
auth transition points to guarantee cache is cleared.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/client/src/shared/apis/queries.tsapps/client/src/shared/components/profilePopup/ProfilePopup.tsx
jjangminii
approved these changes
Feb 27, 2026
Collaborator
jjangminii
left a comment
jjangminii
left a comment
There was a problem hiding this comment.
기존에는 hasJob 값을 로컬스토리지에 저장해 조건 분기를 처리하면서 사용자가 개발자 도구 등을 통해 값을 직접 변경하는 경우는 어느 정도 사용자 책임의 영역이라고 생각해 로컬스토리지 방식 자체가 크게 문제되지는 않을 것이라고 판단했습니다.
다만 어제 회의에서 사용자가 URL로 직접 접근했을 때 발생할 수 있는 예외 상황에 대해 듣고 안일했다고 생각이들었습니다. 다양한 접근 방식과 예외 상황까지 더 폭넓게 고려해 봐야겠네요 😢
고생 많으셨습니다-!
Comment on lines
24
to
33
| const { data: hasJobData, isLoading: isHasJobLoading } = useGetHasJob( | ||
| isLoggedIn && !isAuthPage | ||
| ); | ||
|
|
||
| const shouldShowJobSelectionFunnel = | ||
| isLoggedIn && | ||
| !isAuthPage && | ||
| !isHasJobLoading && | ||
| hasJobData?.hasJob === false; | ||
|
|
| localStorage.removeItem('token'); | ||
| localStorage.removeItem('email'); | ||
| localStorage.removeItem('userId'); | ||
| queryClient.clear(); |
Collaborator
There was a problem hiding this comment.
로그아웃 시 단순히 토큰만 제거하면 된다고 생각했는데 React Query 캐시에 이전 사용자 데이터가 남아 있을 수 있다는 점을 놓쳤네요
토큰 삭제뿐 아니라 캐시 초기화까지 함께 처리하는 것이 사용자 전환 시 데이터 일관성과 보안 측면에서 더 안전한 방법이라고 이해했습니다. 하나 더 배워가요-!
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
2 participants
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.
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
기존에 로그인 api에서 res로 오는
hasJob을 로컬스토리지에 넣고 이를 통해 기존 사용자 직무 선택 퍼널을 띄울지 말지 분기 처리를 했었어요.다만 이렇게 하면 문제가
등이 있을 수 있다고 판단했어요. 따라서 서버팀원분께
hasJob을 주는 api를 분리해달라고 말씀을 드렸고, 이를 Layout에서 공통으로 요청을 하도록 설정했어요.해당 조건 분기를 통해 funnel이 떠야하는 특정 조건을 분리해서 사용했어요. 추가로 캐싱도 사용해서 매번 요청을 보내지는 않아요!
📷 Screenshot
Summary by CodeRabbit
새로운 기능
버그 수정
리팩토링