[Refactor/#154] 워크스페이스 생성/수정 단계 이미지 API 연동 방식 변경 - #159
Conversation
📝 WalkthroughWalkthrough워크스페이스 생성/수정 API를 JSON에서 multipart/form-data로 전환하고, 클라이언트 단의 별도 이미지 업로드( Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (브라우저)
participant API as API Server
participant Storage as S3 / Object Storage
Client->>API: multipart/form-data (fields + optional image file)
API->>Storage: 업로드(이미지 파일) / 기존 이미지 정리
Storage-->>API: 업로드 결과 (url)
API-->>Client: 응답 (워크스페이스 정보, logoUrl)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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 docstrings
🧪 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 배포 완료
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/workspace/WorkspaceSetting.tsx (1)
138-168:⚠️ Potential issue | 🟠 Major새 이미지를 취소해도 기존 로고가 삭제 요청으로 바뀝니다.
기존 로고가 있는 상태에서 새 파일을 고른 뒤
초기화를 누르면, 지금 구현은 “교체 취소”가 아니라 “기존 로고 삭제” 상태로 전환됩니다. 그래서 사용자가유지 / 교체 / 삭제중유지로 돌아갈 수 없고, 잘못 고른 파일을 취소하다가 로고를 지워버릴 수 있습니다.예시 수정
const onResetLogo = () => { + const hasPendingReplacement = Boolean(logoFile || logoPreview); setLogoFile(null); - setIsImageDeleted(true); setImageError(false); setLogoPreview((prev) => { if (prev) URL.revokeObjectURL(prev); return null; }); - setServerLogoUrl(null); + + if (hasPendingReplacement) { + setIsImageDeleted(false); + return; + } + + setIsImageDeleted(Boolean(serverLogoUrl)); + if (serverLogoUrl) { + setServerLogoUrl(null); + } };🤖 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 138 - 168, The bug is that cancelling a newly picked file via onResetLogo flips state to "delete existing logo"; change the logic so cancelling a pick restores the previous server logo and does not mark deletion: in onPickLogo avoid clearing serverLogoUrl when a new file is selected (only set logoFile, logoPreview and setIsImageDeleted(false)); in onResetLogo, if there is an existing serverLogoUrl keep it and setIsImageDeleted(false) and clear only logoFile and logoPreview, and only set isImageDeleted(true) when the user explicitly performs a delete action (not when cancelling a pick). Update uses of logoFile, isImageDeleted, logoPreview and serverLogoUrl accordingly so reset is a true "cancel pick" and not a "delete".
🧹 Nitpick comments (1)
src/types/workspace/workspace.ts (1)
39-43: 업데이트 요청 타입에 이미지 상태를 상호배타적으로 표현해 주세요.지금 타입은
imageFile과isImageDeleted: true를 동시에 허용해서, PR에서 정한유지 / 교체 / 삭제3상태 중 모순된 조합도 그대로 컴파일됩니다. 이 계약을 타입으로 잠가두면 다음 호출부에서 잘못된 payload를 만드는 실수를 줄이기 좋습니다.예시 수정
+type TWorkspaceImageChange = + | { imageFile?: null; isImageDeleted: false } + | { imageFile: File; isImageDeleted: false } + | { imageFile?: null; isImageDeleted: true }; + export type TUpdateWorkspaceRequest = { name: string; description: string; - imageFile?: File | null; - isImageDeleted: boolean; -}; +} & TWorkspaceImageChange;As per coding guidelines, "타입 안정성: TypeScript 타입의 명확성 확인. any 사용 지양, 제네릭 활용 검토."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/workspace/workspace.ts` around lines 39 - 43, The TUpdateWorkspaceRequest currently allows conflicting combinations of imageFile and isImageDeleted; replace it with a discriminated union representing the three mutually exclusive image states (keep existing, replace with new File, or delete existing) so callers cannot provide both imageFile and isImageDeleted simultaneously; use a union of distinct shapes (e.g., { imageAction: "keep" }, { imageAction: "replace"; imageFile: File }, { imageAction: "delete" }) while keeping other fields (name, description) intact to preserve type safety and call sites that destructure TUpdateWorkspaceRequest.
🤖 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 91-98: 업데이트 성공 후 fetchWorkspaceDetail()만 호출해서 현재 페이지 상태만 갱신되고 다른
구독자(워크스페이스 목록 등)는 이전 값을 유지하므로, updateWorkspace 성공 경로에서 React Query의 해당 캐시 키
["my-workspaces"]도 invalidate해 다른 구독자들이 최신 데이터로 갱신되게 하세요; 구체적으로
WorkspaceSetting에서 updateWorkspace 호출 직후 toast.success(...) 다음에 react-query의
queryClient.invalidateQueries(["my-workspaces"])를 호출하여 onDelete와 동일한 키를 무효화하도록
수정하세요.
---
Outside diff comments:
In `@src/pages/workspace/WorkspaceSetting.tsx`:
- Around line 138-168: The bug is that cancelling a newly picked file via
onResetLogo flips state to "delete existing logo"; change the logic so
cancelling a pick restores the previous server logo and does not mark deletion:
in onPickLogo avoid clearing serverLogoUrl when a new file is selected (only set
logoFile, logoPreview and setIsImageDeleted(false)); in onResetLogo, if there is
an existing serverLogoUrl keep it and setIsImageDeleted(false) and clear only
logoFile and logoPreview, and only set isImageDeleted(true) when the user
explicitly performs a delete action (not when cancelling a pick). Update uses of
logoFile, isImageDeleted, logoPreview and serverLogoUrl accordingly so reset is
a true "cancel pick" and not a "delete".
---
Nitpick comments:
In `@src/types/workspace/workspace.ts`:
- Around line 39-43: The TUpdateWorkspaceRequest currently allows conflicting
combinations of imageFile and isImageDeleted; replace it with a discriminated
union representing the three mutually exclusive image states (keep existing,
replace with new File, or delete existing) so callers cannot provide both
imageFile and isImageDeleted simultaneously; use a union of distinct shapes
(e.g., { imageAction: "keep" }, { imageAction: "replace"; imageFile: File }, {
imageAction: "delete" }) while keeping other fields (name, description) intact
to preserve type safety and call sites that destructure TUpdateWorkspaceRequest.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e85bedb1-9346-4ac0-83dc-71c4253fbbf0
📒 Files selected for processing (4)
src/api/workspace/org.tssrc/pages/workspace/Workspace.tsxsrc/pages/workspace/WorkspaceSetting.tsxsrc/types/workspace/workspace.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/pages/workspace/WorkspaceSetting.tsx (1)
76-78:useEffect의존성 배열 검토가 필요할 수 있습니다.
fetchWorkspaceDetail함수가 의존성 배열에 포함되어 있지 않아 ESLintexhaustive-deps규칙에서 경고가 발생할 수 있습니다. 현재 코드는orgId변경 시 정상적으로 동작하지만,useCallback으로 함수를 감싸거나 함수 내용을useEffect내부로 이동하는 것을 고려해 볼 수 있습니다.♻️ useCallback을 사용한 리팩터링 예시
+ const fetchWorkspaceDetail = useCallback(async () => { + if (orgId === null) { + setErrorMsg("잘못된 워크스페이스ID 입니다"); + return; + } + // ... 기존 로직 + }, [orgId]); useEffect(() => { void fetchWorkspaceDetail(); - }, [orgId]); + }, [fetchWorkspaceDetail]);코딩 가이드라인에 따라, "Hook 사용: useEffect 의존성 배열 및 불필요한 사용 검토" 항목을 참고했습니다.
🤖 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 76 - 78, 현재 useEffect([... , [orgId])에서 fetchWorkspaceDetail가 의존성 배열에 없어 ESLint exhaustive-deps 경고가 발생할 수 있으니 fetchWorkspaceDetail을 useCallback으로 래핑하거나 해당 함수 내용을 useEffect 내부로 옮겨 의존성을 명확히 하세요; 구체적으로는 fetchWorkspaceDetail(현재 정의된 함수명)을 React.useCallback으로 감싸 orgId(및 함수 내부에서 참조하는 다른 상태/props)를 의존성에 포함시키거나, fetchWorkspaceDetail 호출 로직 전체를 useEffect 내부로 이동시켜 useEffect의 의존성 배열에 orgId와 해당 내부 변수들만 포함되도록 수정하세요.
🤖 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/WorkspaceSetting.tsx`:
- Around line 76-78: 현재 useEffect([... , [orgId])에서 fetchWorkspaceDetail가 의존성
배열에 없어 ESLint exhaustive-deps 경고가 발생할 수 있으니 fetchWorkspaceDetail을 useCallback으로
래핑하거나 해당 함수 내용을 useEffect 내부로 옮겨 의존성을 명확히 하세요; 구체적으로는 fetchWorkspaceDetail(현재
정의된 함수명)을 React.useCallback으로 감싸 orgId(및 함수 내부에서 참조하는 다른 상태/props)를 의존성에 포함시키거나,
fetchWorkspaceDetail 호출 로직 전체를 useEffect 내부로 이동시켜 useEffect의 의존성 배열에 orgId와 해당
내부 변수들만 포함되도록 수정하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 20865d70-4ab9-4e70-9993-5f122d29f246
📒 Files selected for processing (1)
src/pages/workspace/WorkspaceSetting.tsx
Seojegyeong
left a comment
There was a problem hiding this comment.
P4: 확인했습니다! PR 잘 정리해주셔서 감사합니다. 코드도 훨씬 깔끔해진 것 같네요!
|
P4: 잘 이해가 됐습니다! 수고하셨습니다! |
🚨 관련 이슈
Closed #154
✨ 변경사항
✏️ 작업 내용
기존에 워크스페이스 생성/수정 시에는 image파일을 따로 업로드전용 API를 통해 이미지만 먼저 업로드 후 URL을 받아서, 그 URL을 JSON 객체 형식에 넣어서 생성/수정 API에 업로드 하는 방식으로 진행했습니다.
그러나 이 방식으로는 조직을 삭제한 후에 조직에 관련된 데이터는 삭제되지만, S3에 업로드해둔 이미지 파일만 남아서 관리가 어려워 고아 파일이 남을 수 있는 문제가 있었습니다.
따라서, 백엔드와 협의 후에 기존 JSON 형식에서 multipart/form-data 방식으로 변경후 다른 데이터들과 같이 이미지 파일을 직접 전송하는 방식으로 변경하였습니다.
FormData 방식으로 변경됨에 따라 API연동 로직을 수정하였고, 기존에 사용하던 이미지 업로드용
uploadImage함수제거하면서 이미지 업로드 관련 작업에 대한 의존성을 제거하였습니다.정리하면,
😅 미완성 작업
N/A
📢 논의 사항 및 참고 사항
처음에 이 이미지파일 처리가 어려워서 정리를 해보면,
JSON 객체 방식은 보통 텍스트 데이터만 보낼때 주로 사용하고,
만약에 이미지나 동영상 같이 바이너리 데이터를 같이 보내야할 때는 **FormData(multipart/form-data)**를 사용하는것을 주로 활용한다고 합니다.
이렇게 이미지 파일을 별도의 API로 업로드한 뒤에 URL 반환받아서 JSON객체에넣어서 API호출을 진행했고,
이렇게 JSON(request) + image(file)을 같이 전송해서, 서버에서 이미지업로들아 기존 이미지 삭제, URL 저장을 전부 자동처리할 수 있도록 변경하였습니다.
Summary by CodeRabbit
릴리스 노트
New Features
Bug Fixes