[Feature/#58] 워크스페이스 CRUD API 연동 - #75
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough워크스페이스 CRUD API 및 관련 타입을 추가하고, 목록/생성/조회/수정/삭제를 UI(Workspace, WorkspaceSetting)에 연동했습니다. 공통 유틸(getAxiosMessage)과 일부 컴포넌트 prop/접근성 변경도 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Workspace as Workspace 컴포넌트
participant API as Backend /api/org
participant QC as QueryClient
User->>Workspace: 생성 모달 열고 입력
User->>Workspace: 생성 제출
Workspace->>Workspace: 입력 비활성화, 로딩 표시
Workspace->>API: POST /api/org/create (createWorkspace)
alt 성공
API-->>Workspace: TCreateOrgResponse
Workspace->>QC: invalidate getMyWorkspaces
QC->>API: GET /api/org/my (getMyWorkspaces)
API-->>QC: TWorkspace[]
QC-->>Workspace: 업데이트된 목록
Workspace->>User: 모달 닫기, 성공 토스트
else 실패
API-->>Workspace: Error
Workspace->>User: 모달 내 에러 표시, 입력 재활성화
end
sequenceDiagram
participant User as User
participant Setting as WorkspaceSetting 컴포넌트
participant API as Backend /api/org
participant Router as Router
Setting->>API: GET /api/org/{orgId} (getWorkspace) on mount
alt 성공
API-->>Setting: TWorkspaceDetail
Setting->>User: 폼 초기화 및 렌더
User->>Setting: 저장 클릭
Setting->>API: PATCH /api/org/{orgId} (updateWorkspace)
API-->>Setting: void
Setting->>User: 성공 토스트, 상세 재조회
User->>Setting: 삭제 클릭 -> 확인
Setting->>API: DELETE /api/org/{orgId} (deleteWorkspace)
API-->>Setting: void
Setting->>User: 성공 토스트
Setting->>Router: 목록 페이지로 이동
else 에러
API-->>Setting: Error
Setting->>User: 에러 패널 및 재시도
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 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 배포 완료
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/pages/workspace/WorkspaceSetting.tsx (2)
57-59:useEffect의존성 배열에fetchWorkspaceDetail누락
orgId가 의존성에 포함되어 있어 변경 시 재실행되지만,fetchWorkspaceDetail함수 자체는 포함되어 있지 않습니다.eslint-plugin-react-hooks에서 경고가 발생할 수 있습니다.♻️ 개선 방안
+ import { useCallback } from "react"; - const fetchWorkspaceDetail = async () => { + const fetchWorkspaceDetail = useCallback(async () => { if (orgId === null) { setErrorMsg("잘못된 워크스페이스ID 입니다"); return; } setLoading(true); try { const detail = await getWorkspace(orgId); setName(detail.name); setDesc(detail.description ?? ""); } catch (e) { toast.error( getAxiosMessage(e, "워크스페이스 정보를 불러오지 못했습니다"), ); } finally { setLoading(false); } - }; + }, [orgId]); useEffect(() => { void fetchWorkspaceDetail(); - }, [orgId]); + }, [fetchWorkspaceDetail]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/WorkspaceSetting.tsx` around lines 57 - 59, The useEffect in WorkspaceSetting currently depends only on orgId but omits the fetchWorkspaceDetail function, which will trigger linter warnings; fix this by either adding fetchWorkspaceDetail to the dependency array of the useEffect or by memoizing fetchWorkspaceDetail with useCallback (so it has stable identity) and then including that memoized function in the dependency list — reference the useEffect call, the fetchWorkspaceDetail function, and the orgId variable when making the change.
19-20:getAxiosMessage헬퍼 함수를 공통 유틸리티로 추출 고려이 함수가
WorkspaceSetting.tsx에만 정의되어 있고,Workspace.tsx에서는 인라인으로 에러 처리를 하고 있습니다. 향후 다른 페이지에서도 동일한 패턴이 필요할 수 있으므로, 공통 유틸리티로 추출하면 일관성과 재사용성이 높아집니다.♻️ 공통 유틸리티 추출 예시
// src/lib/error.ts import axios from "axios"; export const getAxiosMessage = (e: unknown, fallback: string): string => axios.isAxiosError(e) ? (e.response?.data?.message ?? fallback) : fallback;이후 각 페이지에서 import하여 사용:
import { getAxiosMessage } from "@/lib/error";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/WorkspaceSetting.tsx` around lines 19 - 20, Extract the getAxiosMessage helper from WorkspaceSetting.tsx into a shared error utility module (exported as getAxiosMessage) and replace the inline error handling in Workspace.tsx to import and use that exported function; ensure the signature remains (e: unknown, fallback: string): string, keep axios.isAxiosError logic intact, export the function from the new module and update both WorkspaceSetting.tsx and Workspace.tsx to import it so they use the common implementation.src/pages/workspace/Workspace.tsx (1)
104-106:useEffect의존성 배열 검토가 필요합니다.
fetchWorkspaces함수가 의존성 배열에 포함되어 있지 않습니다. 현재는 마운트 시 한 번만 실행되어 의도대로 동작하지만,eslint-plugin-react-hooks에서 경고가 발생할 수 있고 향후 유지보수 시 혼란을 줄 수 있습니다.♻️ 개선 방안
방안 1:
useCallback으로 감싸기+ import { useCallback } from "react"; - const fetchWorkspaces = async () => { + const fetchWorkspaces = useCallback(async () => { setLoading(true); setListErrorMsg(null); try { const list = await getMyWorkspaces(); setWorkspaces(list); } catch (e) { const message = e instanceof Error ? e.message : "워크스페이스 목록 조회중 오류가 발생했습니다"; setListErrorMsg(message); setWorkspaces([]); } finally { setLoading(false); } - }; + }, []); useEffect(() => { void fetchWorkspaces(); - }, []); + }, [fetchWorkspaces]);방안 2: React Query 활용 (권장)
서버 상태 관리를 위해 React Query를 도입하면 로딩/에러 상태, 캐싱, 재시도 로직이 자동으로 처리됩니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 104 - 106, The useEffect currently calls fetchWorkspaces without listing it in the dependency array, which will trigger React Hooks lint warnings and could cause stale closure bugs; wrap the fetchWorkspaces function in useCallback (e.g., const fetchWorkspaces = useCallback(..., [/* its deps */])) or move the data-loading into a React Query hook and then call that hook inside the component, and then update the useEffect to include fetchWorkspaces in its dependencies (or eliminate the useEffect if using React Query) so that the dependency array is correct and lint warnings are resolved.
🤖 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/pages/workspace/Workspace.tsx`:
- Around line 41-42: The unused state logoFile (and optionally logoPreview) is
causing the build failure; either remove/comment out the declarations const
[logoFile, setLogoFile] and const [logoPreview, setLogoPreview], or keep them
and wire them into the file-picker flow by ensuring onPickFile calls
setLogoFile(file) and the upload/path logic uses logoFile (instead of the
hardcoded logoUrl: null) so the state is actually referenced (update any payload
that sets logoUrl to consume logoFile or logoPreview as appropriate).
---
Nitpick comments:
In `@src/pages/workspace/Workspace.tsx`:
- Around line 104-106: The useEffect currently calls fetchWorkspaces without
listing it in the dependency array, which will trigger React Hooks lint warnings
and could cause stale closure bugs; wrap the fetchWorkspaces function in
useCallback (e.g., const fetchWorkspaces = useCallback(..., [/* its deps */]))
or move the data-loading into a React Query hook and then call that hook inside
the component, and then update the useEffect to include fetchWorkspaces in its
dependencies (or eliminate the useEffect if using React Query) so that the
dependency array is correct and lint warnings are resolved.
In `@src/pages/workspace/WorkspaceSetting.tsx`:
- Around line 57-59: The useEffect in WorkspaceSetting currently depends only on
orgId but omits the fetchWorkspaceDetail function, which will trigger linter
warnings; fix this by either adding fetchWorkspaceDetail to the dependency array
of the useEffect or by memoizing fetchWorkspaceDetail with useCallback (so it
has stable identity) and then including that memoized function in the dependency
list — reference the useEffect call, the fetchWorkspaceDetail function, and the
orgId variable when making the change.
- Around line 19-20: Extract the getAxiosMessage helper from
WorkspaceSetting.tsx into a shared error utility module (exported as
getAxiosMessage) and replace the inline error handling in Workspace.tsx to
import and use that exported function; ensure the signature remains (e: unknown,
fallback: string): string, keep axios.isAxiosError logic intact, export the
function from the new module and update both WorkspaceSetting.tsx and
Workspace.tsx to import it so they use the common implementation.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/api/workspace/org.tssrc/pages/workspace/Workspace.tsxsrc/pages/workspace/WorkspaceSetting.tsxsrc/types/workspace/mapper.tssrc/types/workspace/workspace.ts
📚 Storybook 배포 완료
|
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/pages/workspace/Workspace.tsx (4)
29-33: 서버 상태 관리에 React Query 사용을 권장합니다.현재
workspaces,loading,listErrorMsg등 서버 상태를useState로 직접 관리하고 있습니다. 코딩 가이드라인에서 언급된 대로 서버 상태는 React Query(useQuery,useMutation)로 분리하면 다음과 같은 이점이 있습니다:
- 캐싱 및 백그라운드 리페치 자동 처리
- 로딩/에러 상태 자동 관리
- 중복 요청 방지 및 요청 취소 처리
staleTime,refetchOnWindowFocus등 세밀한 제어PR 요약에서 별도 리팩토링 PR을 계획하고 있다고 언급되어 있으니, 해당 작업 시 고려해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 29 - 33, Replace the local useState-managed server state in the Workspace component (workspaces, setWorkspaces, loading, listErrorMsg, creating, createErrorMsg) with React Query hooks: useQuery to fetch and manage the workspace list (handle loading/error/cache/refetch) and useMutation for create workspace operations (handle creating state, error, and invalidate or refetch the workspace list on success); update any code that reads those state variables to use the query/mutation results and status fields (e.g., data, isLoading, isError, error, mutate, isLoading from useMutation) so server state is centralized via React Query.
239-264: 폼을<form>태그로 감싸면 Enter 키 제출이 가능합니다.현재 입력 필드에서 Enter 키를 눌러도 폼이 제출되지 않습니다. 사용자 경험 개선을 위해
<form>태그로 감싸고onSubmit핸들러를 사용하는 것을 권장합니다.♻️ 폼 태그 적용 제안
<div className="space-y-6 mx-auto w-full max-w-200"> + <form onSubmit={(e) => { e.preventDefault(); onSubmitCreate(); }}> <div className="max-w-140 mx-auto w-full mb-10"> {/* ... 로고 업로드 영역 ... */} </div> <Input label="워크스페이스 이름" placeholder="조직의 이름을 입력하세요." value={newName} onChange={(e) => setNewName(e.target.value)} /> {/* ... TextareaField, error message ... */} <Button size="big" variant="primary" - onClick={onSubmitCreate} + type="submit" disabled={!newName.trim() || creating} className="mx-auto px-10 mt-10" - type="button" > {creating ? "생성 중.. " : "생성하기"} </Button> + </form> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 239 - 264, Wrap the Input, TextareaField, error paragraph, and Button in a <form> and attach an onSubmit handler that calls onSubmitCreate so Enter submits the form; ensure onSubmitCreate either accepts the event and calls e.preventDefault() or create a small wrapper like const handleSubmit = (e) => { e.preventDefault(); onSubmitCreate(); } and use that as the form's onSubmit; change the Button from type="button" to type="submit" (you can remove the onClick or keep it for redundancy) and keep the existing disabled logic (disabled={!newName.trim() || creating}) so the form behaves the same while supporting Enter-key submission.
161-173: 로딩/에러 상태의 접근성을 개선할 수 있습니다.스크린 리더 사용자가 상태 변경을 인지할 수 있도록 ARIA 속성을 추가하는 것을 권장합니다.
♿ 접근성 개선 제안
- {loading && ( - <div className="bg-white p-10 text-center border border-gray-100 rounded-component-lg"> - <p className="font-body2 text-text-sub">불러오는중..</p> + {loading && ( + <div + className="bg-white p-10 text-center border border-gray-100 rounded-component-lg" + aria-live="polite" + aria-busy="true" + > + <p className="font-body2 text-text-sub">불러오는중..</p> </div> )} {!loading && listErrorMsg && ( - <div className="bg-white p-10 text-center border border-gray-100 rounded-component-lg space-y-4"> + <div + className="bg-white p-10 text-center border border-gray-100 rounded-component-lg space-y-4" + role="alert" + > <p className="font-body2 text-status-red">{listErrorMsg}</p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 161 - 173, The loading and error UI blocks (conditionals using loading, listErrorMsg, and the fetchWorkspaces retry button) lack ARIA attributes for screen readers; update the loading div and the error container in Workspace (where loading, listErrorMsg, fetchWorkspaces are used) to include appropriate ARIA roles and live regions (for example role="status" or role="alert" and aria-live="polite" or "assertive" respectively), ensure the spinner/text has aria-hidden set correctly if using decorative icons, and add descriptive aria-label or aria-live messages so screen reader users are notified when loading starts/ends and when an error appears and the retry Button is available.
84-100: 빠른 연속 호출 시 race condition 가능성이 있습니다."다시 시도" 버튼을 빠르게 여러 번 클릭하면 여러 요청이 동시에 발생할 수 있고, 응답 순서에 따라 오래된 데이터가 최신 상태를 덮어쓸 수 있습니다.
간단한 해결 방법으로
loading상태가true일 때 요청을 무시하는 가드를 추가할 수 있습니다:♻️ 제안하는 수정 방법
const fetchWorkspaces = async () => { + if (loading) return; setLoading(true); setListErrorMsg(null); try {또는 "다시 시도" 버튼에서
loading상태일 때 비활성화:- <Button type="button" variant="primary" onClick={fetchWorkspaces}> + <Button type="button" variant="primary" onClick={fetchWorkspaces} disabled={loading}> 다시 시도 </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 84 - 100, The fetchWorkspaces function can race when called repeatedly; add a guard at the top of fetchWorkspaces that returns immediately when loading is true (or use a dedicated ref like isFetchingRef to avoid stale state) so duplicate requests are ignored; keep the existing setLoading(true)/finally setLoading(false) flow and ensure setWorkspaces/setListErrorMsg are only called by the active invocation. Alternatively, disable the "다시 시도" button based on the loading state to prevent multiple clicks (reference fetchWorkspaces, loading, setLoading, getMyWorkspaces, setListErrorMsg, setWorkspaces).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/pages/workspace/Workspace.tsx`:
- Around line 29-33: Replace the local useState-managed server state in the
Workspace component (workspaces, setWorkspaces, loading, listErrorMsg, creating,
createErrorMsg) with React Query hooks: useQuery to fetch and manage the
workspace list (handle loading/error/cache/refetch) and useMutation for create
workspace operations (handle creating state, error, and invalidate or refetch
the workspace list on success); update any code that reads those state variables
to use the query/mutation results and status fields (e.g., data, isLoading,
isError, error, mutate, isLoading from useMutation) so server state is
centralized via React Query.
- Around line 239-264: Wrap the Input, TextareaField, error paragraph, and
Button in a <form> and attach an onSubmit handler that calls onSubmitCreate so
Enter submits the form; ensure onSubmitCreate either accepts the event and calls
e.preventDefault() or create a small wrapper like const handleSubmit = (e) => {
e.preventDefault(); onSubmitCreate(); } and use that as the form's onSubmit;
change the Button from type="button" to type="submit" (you can remove the
onClick or keep it for redundancy) and keep the existing disabled logic
(disabled={!newName.trim() || creating}) so the form behaves the same while
supporting Enter-key submission.
- Around line 161-173: The loading and error UI blocks (conditionals using
loading, listErrorMsg, and the fetchWorkspaces retry button) lack ARIA
attributes for screen readers; update the loading div and the error container in
Workspace (where loading, listErrorMsg, fetchWorkspaces are used) to include
appropriate ARIA roles and live regions (for example role="status" or
role="alert" and aria-live="polite" or "assertive" respectively), ensure the
spinner/text has aria-hidden set correctly if using decorative icons, and add
descriptive aria-label or aria-live messages so screen reader users are notified
when loading starts/ends and when an error appears and the retry Button is
available.
- Around line 84-100: The fetchWorkspaces function can race when called
repeatedly; add a guard at the top of fetchWorkspaces that returns immediately
when loading is true (or use a dedicated ref like isFetchingRef to avoid stale
state) so duplicate requests are ignored; keep the existing
setLoading(true)/finally setLoading(false) flow and ensure
setWorkspaces/setListErrorMsg are only called by the active invocation.
Alternatively, disable the "다시 시도" button based on the loading state to prevent
multiple clicks (reference fetchWorkspaces, loading, setLoading,
getMyWorkspaces, setListErrorMsg, setWorkspaces).
Seojegyeong
reviewed
Mar 2, 2026
Collaborator
|
id는 서버에 맞게 통일하는 게 변환 코드도 줄이고 이해하기에 더 깔끔할 것 같습니다! |
📚 Storybook 배포 완료
|
Collaborator
P3: 호버되는 부분까지 클릭 가능하면 좋을 것 같아요! |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/lib/getAxiosMessage.ts`:
- Around line 3-4: getAxiosMessage currently returns a value that may be
non-string because e.response?.data?.message is any; change getAxiosMessage to
explicitly return string and add a runtime type guard: if axios.isAxiosError(e)
then extract const msg = e.response?.data?.message and return typeof msg ===
'string' ? msg : fallback; otherwise return fallback; keep the
axios.isAxiosError check and ensure the function signature is annotated as ():
string so callers (e.g., toast.error) always receive a string.
In `@src/pages/workspace/Workspace.tsx`:
- Around line 30-35: Replace the local useState/useEffect + manual try/catch
server-state logic with React Query: remove workspaces, loading, creating,
listErrorMsg, createErrorMsg states and the effect that calls fetchWorkspaces;
instead use useQuery(['workspaces'], fetchWorkspaces) to load/list and
useMutation(createWorkspace) for creation, and on successful mutation call
queryClient.invalidateQueries(['workspaces']) (replacing the manual await
fetchWorkspaces() at the current call site) to refetch; ensure error and loading
indicators read from useQuery/useMutation states and that fetchWorkspaces and
createWorkspace remain the query/mutation functions wired to React Query.
In `@src/pages/workspace/WorkspaceSetting.tsx`:
- Around line 45-53: The fetch handler (fetchWorkspaceDetail / the try block
that calls getWorkspace) only shows a toast on error and never updates errorMsg,
so the error panel branch (!loading && errorMsg) never renders; fix by resetting
error state at start (call setErrorMsg(null) when initiating the request), and
in the catch setErrorMsg to a user-friendly message (in addition to the existing
toast) and ensure loading is cleared so the error panel can render; update the
same pattern where similar request code exists (e.g., lines ~122-134) so both
flows handle errorMsg and loading consistently.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/api/workspace/org.tssrc/components/common/textarea/TextareaField.tsxsrc/lib/getAxiosMessage.tssrc/pages/workspace/Workspace.tsxsrc/pages/workspace/WorkspaceSetting.tsxsrc/types/workspace/workspace.ts
YermIm
reviewed
Mar 2, 2026
📚 Storybook 배포 완료
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/common/dropdownmenu/DropdownMenu.tsx (1)
39-55:⚠️ Potential issue | 🟠 Major접근성 이슈:
tabIndex={0}누락으로 키보드 접근 불가
role="button"이 적용된div요소에서tabIndex={0}이 제거되어 키보드 사용자가 해당 trigger에 포커스할 수 없습니다.현재 상태에서는:
- 키보드 탭 탐색으로 trigger에 도달 불가
- Line 46-51의
onKeyDown핸들러가 실행될 수 없음- WCAG 2.1.1 (키보드 접근성) 위반
🔧 제안: tabIndex={0} 추가
<div role="button" aria-haspopup="menu" aria-expanded={open} aria-controls={menuId} aria-label={ariaLabel} + tabIndex={0} onClick={() => setOpen((v) => !v)} onKeyDown={(e) => { if (e.key === "Enter" || e.key === " ") { e.preventDefault(); setOpen((v) => !v); } }} className={twMerge(className)} >As per coding guidelines,
src/**파일에서 시맨틱 HTML, ARIA 속성 사용을 확인해야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/dropdownmenu/DropdownMenu.tsx` around lines 39 - 55, The trigger div inside DropdownMenu (the element with role="button", aria-haspopup, aria-expanded and onKeyDown handler) is missing tabIndex={0}, preventing keyboard focus; update the div in DropdownMenu.tsx to include tabIndex={0} so it becomes focusable and the onKeyDown handler (Enter/Space toggling via setOpen) can run, ensuring keyboard accessibility while keeping existing ARIA attributes and className/twMerge intact.
🧹 Nitpick comments (2)
src/components/common/dropdownmenu/DropdownMenu.tsx (1)
63-64: map에서 index를 key로 사용현재
idx를 key로 사용하고 있습니다. 메뉴 아이템이 동적으로 추가/삭제/재정렬되지 않는 정적 메뉴라면 괜찮지만, 향후 동적 메뉴로 확장될 경우label이나 고유 식별자를 key로 사용하는 것이 좋습니다.♻️ 제안: label을 key로 사용
- {items.map((it, idx) => ( - <div key={idx} className="px-2"> + {items.map((it) => ( + <div key={it.label} className="px-2">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/dropdownmenu/DropdownMenu.tsx` around lines 63 - 64, 현재 DropdownMenu 컴포넌트의 items.map(...)에서 key로 배열 index(idx)를 사용하고 있으니, 동적 변경에 안전하도록 key를 고유 식별자(예: it.id) 또는 it.label로 변경하도록 수정하세요; 구체적으로 DropdownMenu의 items 배열 요소에 id 필드가 있으면 key={it.id}로 사용하고, 없다면 key={it.label}을 사용하되 label이 중복될 가능성이 있다면 고유값 생성(예: `${it.label}-${someUnique}`) 또는 명시적 id 추가를 검토하세요.src/pages/workspace/WorkspaceSetting.tsx (1)
39-106: 페이지 컴포넌트의 비즈니스 로직을 커스텀 훅으로 분리하는 것을 권장합니다.
fetchWorkspaceDetail,onSave,onDelete와 관련 상태(loading/saving/deleting/errorMsg)가 페이지에 집중되어 있어 테스트/유지보수가 빠르게 어려워질 수 있습니다.useWorkspaceSetting(orgId)형태로 분리하면 UI와 로직 경계가 명확해집니다.As per coding guidelines
src/**: 구조와 책임 분리: 페이지에 비즈니스 로직이 과도하지 않은지 확인. 커스텀 훅으로의 분리 여부 검토.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/WorkspaceSetting.tsx` around lines 39 - 106, Move the workspace-fetching and mutation logic out of the component into a custom hook named useWorkspaceSetting(orgId): extract fetchWorkspaceDetail, onSave, onDelete and the related state variables (loading, saving, deleting, errorMsg, name, desc, logoUrl, setName, setDesc, setLogoUrl, setDeleteOpen, etc.) into the hook, keep the side-effect useEffect call inside the hook, expose handler functions and state via the hook's return value, and update the component to call const { loading, saving, deleting, errorMsg, name, setName, desc, setDesc, logoUrl, setLogoUrl, onSave, onDelete, fetchWorkspaceDetail, setDeleteOpen } = useWorkspaceSetting(orgId) so the UI only consumes the hook API. Ensure error handling and toast calls remain the same and that all references to getWorkspace, updateWorkspace, deleteWorkspace and getAxiosMessage are moved into the hook.
🤖 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/pages/workspace/Workspace.tsx`:
- Line 67: The logo preview Object URL (logoPreview state) is only revoked in
onOpenCreate; add the same cleanup to the modal close path by implementing an
onCloseCreate handler (used as Modal onClose) that calls
URL.revokeObjectURL(logoPreview) if logoPreview is set and then clears state via
setLogoPreview(null) and closes the modal (setCreateOpen(false)); ensure any
existing onOpenCreate logic that revokes/sets the URL is kept symmetric with
onCloseCreate to avoid leaked blob URLs.
In `@src/pages/workspace/WorkspaceSetting.tsx`:
- Around line 47-50: The fetched workspace logoUrl saved via setLogoUrl in the
WorkspaceSetting component is not shown because the JSX always renders
UpLoadImgIcon; update the rendering around UpLoadImgIcon (the place currently
showing the upload icon at lines ~147-150) to conditionally render the existing
logo image when the logoUrl state is non-null/non-empty (use the logoUrl state
populated by getWorkspace), and fall back to UpLoadImgIcon when there is no
logo; ensure the component reads the logoUrl state (from where setLogoUrl is
called) and uses it as the img src/alt attributes so the fetched logo is visible
in the UI.
- Around line 24-27: The orgId memo currently only checks Number.isFinite and
allows 0, negatives, and non-integers; update the validation inside the useMemo
that computes orgId (which reads workspaceId) to coerce workspaceId to a number
(e.g., Number(...) or parseInt) and then require Number.isInteger(n) && n > 0,
returning that integer or null otherwise so only positive integer workspace IDs
are accepted.
---
Outside diff comments:
In `@src/components/common/dropdownmenu/DropdownMenu.tsx`:
- Around line 39-55: The trigger div inside DropdownMenu (the element with
role="button", aria-haspopup, aria-expanded and onKeyDown handler) is missing
tabIndex={0}, preventing keyboard focus; update the div in DropdownMenu.tsx to
include tabIndex={0} so it becomes focusable and the onKeyDown handler
(Enter/Space toggling via setOpen) can run, ensuring keyboard accessibility
while keeping existing ARIA attributes and className/twMerge intact.
---
Nitpick comments:
In `@src/components/common/dropdownmenu/DropdownMenu.tsx`:
- Around line 63-64: 현재 DropdownMenu 컴포넌트의 items.map(...)에서 key로 배열 index(idx)를
사용하고 있으니, 동적 변경에 안전하도록 key를 고유 식별자(예: it.id) 또는 it.label로 변경하도록 수정하세요; 구체적으로
DropdownMenu의 items 배열 요소에 id 필드가 있으면 key={it.id}로 사용하고, 없다면 key={it.label}을
사용하되 label이 중복될 가능성이 있다면 고유값 생성(예: `${it.label}-${someUnique}`) 또는 명시적 id 추가를
검토하세요.
In `@src/pages/workspace/WorkspaceSetting.tsx`:
- Around line 39-106: Move the workspace-fetching and mutation logic out of the
component into a custom hook named useWorkspaceSetting(orgId): extract
fetchWorkspaceDetail, onSave, onDelete and the related state variables (loading,
saving, deleting, errorMsg, name, desc, logoUrl, setName, setDesc, setLogoUrl,
setDeleteOpen, etc.) into the hook, keep the side-effect useEffect call inside
the hook, expose handler functions and state via the hook's return value, and
update the component to call const { loading, saving, deleting, errorMsg, name,
setName, desc, setDesc, logoUrl, setLogoUrl, onSave, onDelete,
fetchWorkspaceDetail, setDeleteOpen } = useWorkspaceSetting(orgId) so the UI
only consumes the hook API. Ensure error handling and toast calls remain the
same and that all references to getWorkspace, updateWorkspace, deleteWorkspace
and getAxiosMessage are moved into the hook.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/common/dropdownmenu/DropdownMenu.tsxsrc/components/workspace/WorkspaceCard.tsxsrc/lib/getAxiosMessage.tssrc/pages/workspace/Workspace.tsxsrc/pages/workspace/WorkspaceSetting.tsx
💤 Files with no reviewable changes (1)
- src/components/workspace/WorkspaceCard.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/getAxiosMessage.ts
Collaborator
Author
저도 지금 따로 필요없을것같아요. MVP 후에 추가로 그 카드 박스 선택하면 수정페이지 처럼 조회페이지가 나오고, 그다음에 수정하기버튼을 눌러서 수정상태로 변경하는 방식이나, 아니면 박스선택하면 수정페이지가 나오는등으로 UX개선하는것도 나중에 논의해보면 좋을것같습니다!! |
📚 Storybook 배포 완료
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/pages/workspace/WorkspaceSetting.tsx (1)
24-28:⚠️ Potential issue | 🟡 Minor
orgId검증에서 소수/비정규 숫자 포맷이 통과됩니다.현재 조건은
1.5,1e3같은 값도 허용합니다.orgId는 양의 정수만 허용하도록 제한해 주세요.🔧 제안 수정
const orgId = useMemo(() => { - if (workspaceId === null) return null; + if (workspaceId == null) return null; const n = Number(workspaceId); - return Number.isFinite(n) && n > 0 ? n : null; + return Number.isInteger(n) && n > 0 ? n : null; }, [workspaceId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/WorkspaceSetting.tsx` around lines 24 - 28, The orgId computation currently allows non-standard numeric formats like "1.5" and "1e3"; update the useMemo block that computes orgId (references: orgId, workspaceId, useMemo) to only accept positive integer decimal strings by first verifying workspaceId matches a strict integer regex (e.g. /^\d+$/ or /^\s*[1-9]\d*\s*$/) and then converting with parseInt/Number to produce an integer, otherwise return null; replace the Number.isFinite(n) check with this string-based integer validation so inputs like "1.5" or "1e3" are rejected.
🧹 Nitpick comments (1)
src/pages/workspace/WorkspaceSetting.tsx (1)
40-107: 페이지 컴포넌트에 서버 액션 책임이 많이 모여 있습니다.조회/저장/삭제와 상태 전이가 모두 페이지에 집중되어 있어서, 커스텀 훅(
useWorkspaceSetting)으로 분리하면 테스트성과 유지보수성이 좋아집니다. 특히fetchWorkspaceDetail,onSave,onDelete를 훅으로 옮기고 UI는 렌더링/이벤트 연결에 집중시키는 구조를 권장합니다.As per coding guidelines
src/**: "상태 관리: 서버 상태(React Query)와 전역 상태(Zustand)의 분리 여부 확인." 및 "구조와 책임 분리: 페이지에 비즈니스 로직이 과도하지 않은지 확인."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/WorkspaceSetting.tsx` around lines 40 - 107, The page currently contains all server-action and state-transition logic (fetchWorkspaceDetail, onSave, onDelete and loading/error/saving/deleting state), so extract these into a custom hook useWorkspaceSetting that encapsulates orgId input, maintains name/desc/logoUrl and flags (loading, saving, deleting, errorMsg), exposes setters (setName, setDesc, setLogoUrl) and methods fetchWorkspaceDetail, onSave, onDelete; move the try/catch/finally logic and toast/error handling into the hook, run the initial fetch in a useEffect inside the hook, and update the WorkspaceSetting component to import useWorkspaceSetting and only handle rendering and event wiring to the returned state and methods. Ensure unique symbols referenced: fetchWorkspaceDetail, onSave, onDelete, useWorkspaceSetting, setName, setDesc, setLogoUrl, loading, saving, deleting, errorMsg.
🤖 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/pages/workspace/WorkspaceSetting.tsx`:
- Around line 164-165: Replace the blocking alert in the onClick handler for the
logo upload button in WorkspaceSetting: remove onClick={() => alert("TODO:추후
업로드")} and instead use a non-blocking "준비 중" UX—add a local state flag (e.g.,
isUploadDisabled or isPending) in the WorkspaceSetting component to disable the
button and set aria-disabled when the upload API is not ready, and show a
tooltip or non-blocking toast (instead of alert) when users try to interact;
ensure the button keeps its aria-label="로고 이미지 업로드 버튼" and update click handling
to either early-return while disabled or trigger the toast/tooltip to inform
users the feature is coming soon.
---
Duplicate comments:
In `@src/pages/workspace/WorkspaceSetting.tsx`:
- Around line 24-28: The orgId computation currently allows non-standard numeric
formats like "1.5" and "1e3"; update the useMemo block that computes orgId
(references: orgId, workspaceId, useMemo) to only accept positive integer
decimal strings by first verifying workspaceId matches a strict integer regex
(e.g. /^\d+$/ or /^\s*[1-9]\d*\s*$/) and then converting with parseInt/Number to
produce an integer, otherwise return null; replace the Number.isFinite(n) check
with this string-based integer validation so inputs like "1.5" or "1e3" are
rejected.
---
Nitpick comments:
In `@src/pages/workspace/WorkspaceSetting.tsx`:
- Around line 40-107: The page currently contains all server-action and
state-transition logic (fetchWorkspaceDetail, onSave, onDelete and
loading/error/saving/deleting state), so extract these into a custom hook
useWorkspaceSetting that encapsulates orgId input, maintains name/desc/logoUrl
and flags (loading, saving, deleting, errorMsg), exposes setters (setName,
setDesc, setLogoUrl) and methods fetchWorkspaceDetail, onSave, onDelete; move
the try/catch/finally logic and toast/error handling into the hook, run the
initial fetch in a useEffect inside the hook, and update the WorkspaceSetting
component to import useWorkspaceSetting and only handle rendering and event
wiring to the returned state and methods. Ensure unique symbols referenced:
fetchWorkspaceDetail, onSave, onDelete, useWorkspaceSetting, setName, setDesc,
setLogoUrl, loading, saving, deleting, errorMsg.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/workspace/Workspace.tsxsrc/pages/workspace/WorkspaceSetting.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/workspace/Workspace.tsx
Collaborator
|
P4: 추가로 예림님이랑 제가 남긴 UI/UX 관련 피드백들 이후 리팩토링 올려서 수정해주시면 좋을 것 같아요:) 고생하셨습니다! |
Seojegyeong
approved these changes
Mar 2, 2026
Collaborator
Author
적용해서 수정해두었습니다!! |
Collaborator
|
P4: 수고하셨습니다!! |
YermIm
approved these changes
Mar 2, 2026
This was referenced Mar 10, 2026
This was referenced Mar 25, 2026
9 tasks
This was referenced Jun 17, 2026
This was referenced Aug 1, 2026
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
3 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.
🚨 관련 이슈
Closed #58
✨ 변경사항
✏️ 작업 내용
😅 미완성 작업
📢 논의 사항 및 참고 사항
Summary by CodeRabbit
New Features
UX 개선
Style
Components