Feature/policy api#43
Merged
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough라인 서비스(lineService)와 관련 타입들을 추가하고, UserInfo 타입을 도입해 여러 컴포넌트를 userData 단일 prop으로 리팩토링했으며, 일부 컴포넌트가 중앙화된 유틸/서비스(apiClient, blockService 등)를 사용하도록 변경했습니다. Changes
Sequence DiagramsequenceDiagram
participant Policy as Policy.tsx
participant UserStore as useUserStore
participant UserInfoCard as UserInfoCard
participant LineService as lineService
participant BlockService as blockService
participant APIClient as apiClient
Policy->>UserStore: userData 조회
UserStore-->>Policy: userData 반환
Policy->>UserInfoCard: userData 전달
UserInfoCard->>LineService: getLines()
LineService->>APIClient: GET /lines
APIClient-->>LineService: Line[] 반환
LineService-->>UserInfoCard: lines 반환
UserInfoCard->>BlockService: getBlockStatus(lineId)
BlockService->>APIClient: blockStatus 요청
APIClient-->>BlockService: blockStatus 반환
BlockService-->>UserInfoCard: isBlocked 상태 반환
UserInfoCard-->>Policy: 렌더된 사용자 정보/상태
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/page/Policy/Policy.tsx (2)
52-58:⚠️ Potential issue | 🟠 Major권한 양도 요청은 공용 API 클라이언트를 통해 처리해야 합니다.
여기서
fetch를 직접 쓰면src/api/client.ts의 timeout, 401 처리, 공통 credential/XSRF 흐름을 모두 우회합니다. 게다가 응답 상태 확인 없이 모달을 닫고 새로고침해서 실패도 성공처럼 보이게 됩니다. 최소한apiClient또는 기존 service 레이어를 타고, 성공 응답일 때만 UI를 정리해 주세요.수정 예시
+import { apiClient } from "@/api"; + async function handleTransfer() { - await fetch(`/api/families/owner?userId=${selectedTarget?.userId}`, { - method: "PATCH", - }); - setIsTransferModalOpen(false); - window.location.reload(); + if (!selectedTarget) return; + + try { + await apiClient.patch("/families/owner", undefined, { + params: { userId: selectedTarget.userId }, + }); + + setIsTransferModalOpen(false); + window.location.reload(); + } catch { + // TODO: 실패 토스트/에러 처리 + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/Policy/Policy.tsx` around lines 52 - 58, Replace the direct fetch call in handleTransfer with the shared API client/service so timeout/401/credentials/XSRF flows are preserved: call the existing apiClient (or the family service function) to PATCH the owner using selectedTarget?.userId, await the response and verify success (e.g., check response.ok or the service result) before calling setIsTransferModalOpen(false) and window.location.reload(); on error, surface/log the error and keep the modal open. Ensure you update the handleTransfer function to use the unique symbols apiClient or the family service function instead of fetch and only clean up UI after a confirmed success.
151-170:⚠️ Potential issue | 🟠 Major대표자 전용 액션은
isOwner일 때만 노출하세요.이미
isOwner를 계산하고 있는데 권한 양도 버튼과 모달은 항상 렌더링되고 있습니다. 지금 상태면 구성원도 대표자 전용 액션을 열고 요청까지 시도할 수 있습니다. 버튼과 모달 모두 대표자일 때만 렌더링되도록 막는 편이 맞습니다.수정 예시
- {/* 권한 양도 버튼 */} - <button - onClick={() => setIsTransferModalOpen(true)} - className="w-full flex items-center gap-4 px-4 py-3 bg-white/60 rounded-2xl shadow-sm border border-gray-100" - > - {/* 아이콘 영역 */} - <div className="w-12 h-12 rounded-xl bg-lime-100 flex items-center justify-center flex-shrink-0"> - <img src={AssignIcon} /> - </div> - - {/* 텍스트 */} - <div className="flex flex-col items-start"> - <span className="text-sm font-semibold text-gray-800"> - 권한 양도 - </span> - <span className="text-xs text-gray-400"> - 대표자 권한을 양도할 구성원을 고르세요. - </span> - </div> - </button> + {/* 권한 양도 버튼 */} + {isOwner && ( + <button + onClick={() => setIsTransferModalOpen(true)} + className="w-full flex items-center gap-4 px-4 py-3 bg-white/60 rounded-2xl shadow-sm border border-gray-100" + > + <div className="w-12 h-12 rounded-xl bg-lime-100 flex items-center justify-center flex-shrink-0"> + <img src={AssignIcon} /> + </div> + + <div className="flex flex-col items-start"> + <span className="text-sm font-semibold text-gray-800"> + 권한 양도 + </span> + <span className="text-xs text-gray-400"> + 대표자 권한을 양도할 구성원을 고르세요. + </span> + </div> + </button> + )} - {isTransferModalOpen && + {isOwner && isTransferModalOpen && createPortal( <>Also applies to: 174-259
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/Policy/Policy.tsx` around lines 151 - 170, The transfer button and modal are being rendered for all users even though you already compute isOwner; update the Policy component to conditionally render both the "권한 양도" button (the element that calls setIsTransferModalOpen(true)) and the transfer modal component so they are only included when isOwner is true—wrap the JSX for the button and the modal in an {isOwner && (...) } check or return null when !isOwner; ensure you reference the existing isOwner boolean and the existing modal state (setIsTransferModalOpen / isTransferModalOpen) so non-owners cannot open or see the transfer UI.src/page/Policy/components/UserInfoCard.tsx (1)
51-57:⚠️ Potential issue | 🟠 MajorAPI 호출 실패 시 에러 처리가 없습니다.
lineService.switchLine(lineId)호출이 실패해도 바텀시트가 닫히고 페이지가 새로고침됩니다. 사용자는 회선 전환 실패 여부를 알 수 없어 혼란스러운 UX가 발생할 수 있습니다.🐛 제안된 수정사항
async function handleSelectLine(lineId: number) { - await lineService.switchLine(lineId); - - setIsBottomSheetOpen(false); - // 전환 후 페이지 새로고침 or 상태 업데이트 - window.location.reload(); + try { + await lineService.switchLine(lineId); + setIsBottomSheetOpen(false); + // 전환 후 페이지 새로고침 or 상태 업데이트 + window.location.reload(); + } catch (error) { + // 에러 처리 (예: toast 알림) + console.error("회선 전환 실패:", error); + // TODO: 사용자에게 에러 알림 표시 + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/Policy/components/UserInfoCard.tsx` around lines 51 - 57, handleSelectLine lacks error handling: if lineService.switchLine(lineId) fails, the bottom sheet still closes and the page reloads, leaving the user unaware. Wrap the async call in try/catch inside handleSelectLine, only call setIsBottomSheetOpen(false) and window.location.reload() on success, and on failure keep the sheet open (or reopen it) and surface the error to the user (e.g., show a toast or set an error state). Reference handleSelectLine and lineService.switchLine to locate the change, and ensure any UI feedback uses existing mechanisms (toast/error state) instead of silent failure.
🧹 Nitpick comments (1)
src/page/Policy/components/UserInfoCard.tsx (1)
24-24:lineId소스가 일관되지 않습니다.Line 24에서는 스토어(
useUserStore)에서lineId를 가져오고, Line 196에서는 props인userData.lineId를 사용합니다. 두 값이 동기화되지 않을 경우 예기치 않은 동작이 발생할 수 있습니다.일관성을 위해 하나의 소스만 사용하는 것을 권장합니다.
userData가 이미 props로 전달되므로, 스토어에서 별도로lineId를 가져오는 대신userData.lineId를 사용하는 방안을 검토해 주세요.♻️ 제안된 수정사항
export default function UserInfo({ userData }: Props) { const navigate = useNavigate(); const [isBottomSheetOpen, setIsBottomSheetOpen] = useState(false); - const lineId = useUserStore((state) => state.userInfo?.lineId); + const lineId = userData.lineId; const isOwner = userData.role === "OWNER";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/Policy/components/UserInfoCard.tsx` at line 24, The component mixes two sources for the same value: lineId from useUserStore and userData.lineId prop, which can lead to inconsistent state; choose a single source—prefer props since userData is already passed in—so remove the useUserStore selector for lineId and replace all uses of lineId (from the store) inside UserInfoCard with userData.lineId, updating references in the component (e.g., where lineId is read or passed) and ensuring any dependent logic in functions or JSX inside UserInfoCard uses the prop instead of useUserStore.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/page/Policy/components/UserInfoCard.tsx`:
- Around line 9-21: Rename the React component function UserInfo to UserInfoCard
to avoid the name collision with the imported UserInfo type; update the function
declaration (export default function UserInfoCard({...})) and the default export
if necessary, and search for any local references within this file to replace
UserInfo with UserInfoCard (e.g., the component identifier and any JSX usage or
tests) so the imported type UserInfo remains unchanged and no name conflicts
remain.
In `@src/page/Policy/Policy.tsx`:
- Around line 16-20: The component currently only reads user data from
useUserStore, which can be stale; add a fetch-on-mount that calls the server
endpoint (e.g., getMyInfo or fetchMyInfo) and updates the store so the page has
fresh user and permission state when opened. In the Policy component wrap that
call in a useEffect that runs on mount, handle loading/error, and dispatch the
returned data into the store updater (e.g., useUserStore().setUserInfo or
similar) before relying on isOwner (userData?.role === "OWNER") to render;
ensure the same mount-refresh logic is applied around the other occurrence noted
at line 65.
---
Outside diff comments:
In `@src/page/Policy/components/UserInfoCard.tsx`:
- Around line 51-57: handleSelectLine lacks error handling: if
lineService.switchLine(lineId) fails, the bottom sheet still closes and the page
reloads, leaving the user unaware. Wrap the async call in try/catch inside
handleSelectLine, only call setIsBottomSheetOpen(false) and
window.location.reload() on success, and on failure keep the sheet open (or
reopen it) and surface the error to the user (e.g., show a toast or set an error
state). Reference handleSelectLine and lineService.switchLine to locate the
change, and ensure any UI feedback uses existing mechanisms (toast/error state)
instead of silent failure.
In `@src/page/Policy/Policy.tsx`:
- Around line 52-58: Replace the direct fetch call in handleTransfer with the
shared API client/service so timeout/401/credentials/XSRF flows are preserved:
call the existing apiClient (or the family service function) to PATCH the owner
using selectedTarget?.userId, await the response and verify success (e.g., check
response.ok or the service result) before calling setIsTransferModalOpen(false)
and window.location.reload(); on error, surface/log the error and keep the modal
open. Ensure you update the handleTransfer function to use the unique symbols
apiClient or the family service function instead of fetch and only clean up UI
after a confirmed success.
- Around line 151-170: The transfer button and modal are being rendered for all
users even though you already compute isOwner; update the Policy component to
conditionally render both the "권한 양도" button (the element that calls
setIsTransferModalOpen(true)) and the transfer modal component so they are only
included when isOwner is true—wrap the JSX for the button and the modal in an
{isOwner && (...) } check or return null when !isOwner; ensure you reference the
existing isOwner boolean and the existing modal state (setIsTransferModalOpen /
isTransferModalOpen) so non-owners cannot open or see the transfer UI.
---
Nitpick comments:
In `@src/page/Policy/components/UserInfoCard.tsx`:
- Line 24: The component mixes two sources for the same value: lineId from
useUserStore and userData.lineId prop, which can lead to inconsistent state;
choose a single source—prefer props since userData is already passed in—so
remove the useUserStore selector for lineId and replace all uses of lineId (from
the store) inside UserInfoCard with userData.lineId, updating references in the
component (e.g., where lineId is read or passed) and ensuring any dependent
logic in functions or JSX inside UserInfoCard uses the prop instead of
useUserStore.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 690209a0-cb03-4a68-909e-ac0a6cfc41ac
📒 Files selected for processing (9)
src/api/index.tssrc/api/services/lineService.tssrc/api/services/userService.tssrc/page/Policy/Policy.tsxsrc/page/Policy/components/DataRemainingCard.tsxsrc/page/Policy/components/UserInfoCard.tsxsrc/store/userStore.tssrc/types/line.tssrc/types/user.ts
Comment on lines
+9
to
+21
| import type { UserInfo } from "@/types/user"; | ||
| import { useUserStore } from "@/store/userStore"; | ||
| import { useQuery } from "@tanstack/react-query"; | ||
| import type { Line } from "@/types/line"; | ||
| import { lineService } from "@/api"; | ||
| import { blockService } from "@/api"; | ||
|
|
||
| type Props = { | ||
| // 프로필 | ||
| userName: string; | ||
| lineId: number; | ||
| isOwner?: boolean; | ||
| planName?: string; | ||
| isDualPhone?: boolean; // 투폰 여부 → 계정 전환 버튼 노출 | ||
|
|
||
| // 데이터 잔여량 | ||
| sharedDataRemaining: number; // MB | ||
| personalDataRemaining: number; // MB | ||
|
|
||
| // 차단 상태 | ||
| isBlocked?: boolean; | ||
| userData: UserInfo; | ||
| }; | ||
|
|
||
| export default function UserInfo({ | ||
| userName, | ||
| lineId, | ||
| isOwner = false, | ||
| planName, | ||
| isDualPhone = false, | ||
| sharedDataRemaining, | ||
| personalDataRemaining, | ||
| isBlocked = false, | ||
| }: Props) { | ||
| export default function UserInfo({ userData }: Props) { |
There was a problem hiding this comment.
컴포넌트 이름 UserInfo가 임포트된 타입 UserInfo와 충돌합니다.
파일명이 UserInfoCard.tsx이므로 컴포넌트 이름을 UserInfoCard로 변경하는 것이 적절합니다. 현재 구조에서는 타입과 함수가 동일한 이름을 사용하여 혼란을 야기하고 정적 분석 도구에서 경고가 발생합니다.
🔧 제안된 수정사항
-export default function UserInfo({ userData }: Props) {
+export default function UserInfoCard({ userData }: Props) {📝 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
| import type { UserInfo } from "@/types/user"; | |
| import { useUserStore } from "@/store/userStore"; | |
| import { useQuery } from "@tanstack/react-query"; | |
| import type { Line } from "@/types/line"; | |
| import { lineService } from "@/api"; | |
| import { blockService } from "@/api"; | |
| type Props = { | |
| // 프로필 | |
| userName: string; | |
| lineId: number; | |
| isOwner?: boolean; | |
| planName?: string; | |
| isDualPhone?: boolean; // 투폰 여부 → 계정 전환 버튼 노출 | |
| // 데이터 잔여량 | |
| sharedDataRemaining: number; // MB | |
| personalDataRemaining: number; // MB | |
| // 차단 상태 | |
| isBlocked?: boolean; | |
| userData: UserInfo; | |
| }; | |
| export default function UserInfo({ | |
| userName, | |
| lineId, | |
| isOwner = false, | |
| planName, | |
| isDualPhone = false, | |
| sharedDataRemaining, | |
| personalDataRemaining, | |
| isBlocked = false, | |
| }: Props) { | |
| export default function UserInfo({ userData }: Props) { | |
| import type { UserInfo } from "@/types/user"; | |
| import { useUserStore } from "@/store/userStore"; | |
| import { useQuery } from "@tanstack/react-query"; | |
| import type { Line } from "@/types/line"; | |
| import { lineService } from "@/api"; | |
| import { blockService } from "@/api"; | |
| type Props = { | |
| // 프로필 | |
| userData: UserInfo; | |
| }; | |
| export default function UserInfoCard({ userData }: Props) { |
🧰 Tools
🪛 Biome (2.4.6)
[error] 21-21: Shouldn't redeclare 'UserInfo'. Consider to delete it or rename it.
(lint/suspicious/noRedeclare)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/page/Policy/components/UserInfoCard.tsx` around lines 9 - 21, Rename the
React component function UserInfo to UserInfoCard to avoid the name collision
with the imported UserInfo type; update the function declaration (export default
function UserInfoCard({...})) and the default export if necessary, and search
for any local references within this file to replace UserInfo with UserInfoCard
(e.g., the component identifier and any JSX usage or tests) so the imported type
UserInfo remains unchanged and no name conflicts remain.
Comment on lines
+16
to
+20
| // store에 저장된 user 정보 가져오기 | ||
| const userData = useUserStore((state) => state.userInfo); | ||
|
|
||
| //대표자인가 | ||
| const isOwner = userData?.role === "OWNER"; |
There was a problem hiding this comment.
persisted store만 읽으면 사용자 정보와 권한 상태가 쉽게 stale해집니다.
이 컴포넌트는 useUserStore 값만 읽고 있고, 현재 파일 안에는 페이지 진입 시 서버에서 getMyInfo를 다시 받아 store를 동기화하는 흐름이 없습니다. 그래서 새로고침, 재로그인, 회선 전환 직후에는 사용자 카드가 비어 있거나 이전 회선/이전 사용자 기준으로 isOwner가 계산될 수 있습니다. 이 페이지를 열 때 한 번은 서버 응답으로 store를 갱신하는 경로가 필요합니다.
Also applies to: 65-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/page/Policy/Policy.tsx` around lines 16 - 20, The component currently
only reads user data from useUserStore, which can be stale; add a fetch-on-mount
that calls the server endpoint (e.g., getMyInfo or fetchMyInfo) and updates the
store so the page has fresh user and permission state when opened. In the Policy
component wrap that call in a useEffect that runs on mount, handle
loading/error, and dispatch the returned data into the store updater (e.g.,
useUserStore().setUserInfo or similar) before relying on isOwner (userData?.role
=== "OWNER") to render; ensure the same mount-refresh logic is applied around
the other occurrence noted at line 65.
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.
이슈
✔️ 체크리스트
🔍 작업 내용
정책 페이지 사용자 정보 컴포넌트 api 연동 완료
Summary by CodeRabbit
New Features
Refactor