Conversation
Walkthrough클라이언트 axios에 PUT 카테고리 API를 추가하고, React Query 뮤테이션 훅을 도입해 Sidebar 편집 플로우에 연결했습니다. Popup 컴포넌트에 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor 사용자 as User
participant 사이드바 as Sidebar
participant 팝업포탈 as PopupPortal
participant 훅 as usePutCategory
participant AX as axios.putCategory
participant 서버 as API
User->>사이드바: 편집 클릭
사이드바->>팝업포탈: 팝업 표시 (defaultValue=현재이름)
User->>팝업포탈: 수정 후 확인
팝업포탈->>사이드바: onEditConfirm(id, newName)
사이드바->>훅: mutate({id, categoryName: newName})
훅->>AX: putCategory(id, newName)
AX->>서버: PUT /api/v1/categories/{id} { categoryName }
서버-->>AX: 응답(성공/실패)
AX-->>훅: 응답 전달
alt 성공
훅-->>사이드바: onSuccess
사이드바->>사이드바: 입력 초기화, 쿼리 무효화, 팝업 닫기
else 실패
훅-->>사이드바: onError
사이드바->>사이드바: 오류 로깅
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/client/src/shared/apis/axios.ts (1)
24-27: 중복 쿼리스트링 버그 —?now=제거 필요File: apps/client/src/shared/apis/axios.ts (라인 24-27)
URL에 이미?now=가 포함되어 있어 Axiosparams로 전달되는 now 값이 이중 전송됩니다. 서버가 중복 파라미터를 허용하지 않으면 400 에러 발생 가능.- const { data } = await apiRequest.get('/api/v1/users/acorns?now=', { + const { data } = await apiRequest.get('/api/v1/users/acorns', { params: { now }, });실제 호출로 서버의 중복 now 파라미터 허용 여부 확인 필요.
🧹 Nitpick comments (6)
apps/client/src/shared/apis/axios.ts (1)
16-21: PUT 요청 바디/반환값 계약 점검 + 반환 형태 일관화 제안
- 계약 확인: 백엔드가
{ categoryName }키를 요구하는지 스웨거/스펙으로 한 번만 더 확인해 주세요. 키가name등으로 다르면 바로 실패합니다.- 반환 일관성:
getDashboardCategories는data.data를 반환하지만postCategory/putCategory는 AxiosResponse 전체를 반환합니다. 소비 측 혼란을 줄이려면 동일하게 언랩하여 반환하는 편이 좋습니다.아래처럼 언랩 + 간단 유효성(공백 제거, 빈 문자열 차단)까지 적용을 권합니다.
-export const putCategory = async (id: number, categoryName: string) => { - const response = await apiRequest.put(`/api/v1/categories/${id}`, { - categoryName, - }); - return response; -}; +export const putCategory = async (id: number, categoryName: string) => { + const payload = { categoryName: categoryName.trim() }; + if (!payload.categoryName) { + throw new Error('categoryName is required'); + } + const { data } = await apiRequest.put(`/api/v1/categories/${id}`, payload); + return data?.data ?? data; +};스펙 상 요청 바디 키가
categoryName이 맞는지, 그리고 PUT이 부분 업데이트(이 필드만)로 동작하는지 확인 부탁드립니다.apps/client/src/shared/apis/queries.ts (2)
26-31: 뮤테이션 제네릭/키 지정으로 DX 강화 + 변수 타입 추출에러/데이터/변수 타입을 명시하면 소비 측 DX가 좋아집니다. 또한
mutationKey를 넣으면 디버깅과 개발자도구에서 식별이 쉬워집니다.-export const usePutCategory = () => { - return useMutation({ - mutationFn: ({ id, categoryName }: { id: number; categoryName: string }) => - putCategory(id, categoryName), - }); -}; +type UpdateCategoryVars = { id: number; categoryName: string }; +type UpdateCategoryData = unknown; // 서버 스키마에 맞게 교체 + +export const usePutCategory = () => { + return useMutation<UpdateCategoryData, AxiosError, UpdateCategoryVars>({ + mutationKey: ['category', 'update'], + mutationFn: ({ id, categoryName }) => putCategory(id, categoryName), + }); +};(axios 레이어에서
data.data를 반환하도록 맞추면UpdateCategoryData를 구체 타입으로 지정하기 쉬워집니다.)
33-39: 철자 통일(naming): Arcons → Acorns
useGetArcons/queryKey: ['arcons']는 오타로 보입니다. 응답 타입도AcornsResponse라 일치시키는 편이 낫습니다.-export const useGetArcons = (): UseQueryResult<AcornsResponse, AxiosError> => { +export const useGetAcorns = (): UseQueryResult<AcornsResponse, AxiosError> => { return useQuery({ - queryKey: ['arcons'], + queryKey: ['acorns'], queryFn: () => getAcorns(), }); };apps/client/src/shared/components/sidebar/Sidebar.tsx (1)
17-18: 명칭 일관성: PUT인데 변수/핸들러명이 patch로 되어 있어 혼란 유발API는 PUT을 사용합니다. 가독성과 유지보수성 위해 update/put 계열로 명칭 통일을 제안합니다.
아래처럼 최소 변경으로 정리 가능합니다:
- usePutCategory, + usePutCategory,- const { mutate: patchCategory } = usePutCategory(); + const { mutate: updateCategory } = usePutCategory();- const handlePatchCategory = (id: number) => { + const handleUpdateCategory = (id: number) => { - patchCategory( + updateCategory( { id, categoryName: newCategoryName }, { /* ... */ } ); };- onEditConfirm={(id) => handlePatchCategory(id)} + onEditConfirm={(id) => handleUpdateCategory(id)}Also applies to: 27-27, 174-175
packages/design-system/src/components/popup/Popup.tsx (2)
16-20: 공개 API 확장: defaultValue 추가 — 문서/스토리 반영 필요BasePopupProps에 defaultValue가 추가되었습니다. 디자인 시스템 소비처에 반영(문서/스토리/마이그레이션 노트) 부탁드립니다. 하위 호환에 문제는 없어 보입니다.
42-49: value와 defaultValue 동시 전달 — 제어/비제어 혼합 위험Input에 value와 defaultValue를 동시에 넘기면 제어/비제어 전환 경고가 발생할 수 있습니다. 하나만 조건부로 전달하세요.
- <Input - placeholder={placeholder} - helperText={helperText} - isError={isError} - value={inputValue} - onChange={(e) => onInputChange?.(e.target.value)} - defaultValue={defaultValue} - /> + <Input + placeholder={placeholder} + helperText={helperText} + isError={isError} + {...(inputValue !== undefined + ? { value: inputValue } + : { defaultValue })} + onChange={(e) => onInputChange?.(e.target.value)} + />또한, 설계상 Input 컴포넌트가 defaultValue를 지원/전달하는지(내부 까지) 한 번만 확인 부탁드립니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/client/src/shared/apis/axios.ts(1 hunks)apps/client/src/shared/apis/queries.ts(2 hunks)apps/client/src/shared/components/sidebar/PopupPortal.tsx(1 hunks)apps/client/src/shared/components/sidebar/Sidebar.tsx(4 hunks)packages/design-system/src/components/popup/Popup.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-17T09:18:13.818Z
Learnt from: constantly-dev
PR: Pinback-Team/pinback-client#102
File: apps/extension/src/components/modalPop/ModalPop.tsx:166-172
Timestamp: 2025-07-17T09:18:13.818Z
Learning: In apps/extension/src/components/modalPop/ModalPop.tsx, the categories array should include "안 읽은 정보" (Unread Information) as the first default category that cannot be deleted. This default category is used consistently across the client-side dashboard and should be protected from deletion in the extension as well.
Applied to files:
apps/client/src/shared/components/sidebar/Sidebar.tsx
🧬 Code graph analysis (2)
apps/client/src/shared/apis/queries.ts (1)
apps/client/src/shared/apis/axios.ts (1)
putCategory(16-21)
apps/client/src/shared/components/sidebar/Sidebar.tsx (1)
apps/client/src/shared/apis/queries.ts (1)
usePutCategory(26-31)
🔇 Additional comments (2)
apps/client/src/shared/apis/queries.ts (1)
2-6: PUT 훅 도입 OK
putCategory임포트 추가는 적절합니다. 변경 사항과 스펙(PUT 사용) 일치합니다.apps/client/src/shared/components/sidebar/Sidebar.tsx (1)
149-151: 기본 카테고리 보호(“안 읽은 정보”) 검증 필요조직적 합의사항에 따라 기본 카테고리는 삭제/편집이 불가해야 합니다. 현재 파일만으로는 보호 로직을 확정할 수 없습니다. OptionsMenuPortal/useCategoryPopups에서 해당 항목의 옵션 노출/동작이 차단되는지 확인 부탁드립니다. 필요 시 Sidebar 레벨에서도 가드 추가 권장합니다(이름 비교보다 ID/플래그 기반 권장).
| onInputChange={onChange} | ||
| defaultValue={popup.name} | ||
| onLeftClick={onClose} | ||
| onRightClick={() => onEditConfirm?.(popup.id)} | ||
| /> |
There was a problem hiding this comment.
편집 확인 시 값이 비어 전송될 수 있음(defaultValue + onChange만 사용)
defaultValue는 입력 초기값만 채우고, 사용자가 수정하지 않으면 onInputChange가 호출되지 않아 상위 상태가 비어 있을 수 있습니다. 이 상태에서 “확인” 클릭 시 빈 값이 전송될 가능성이 큽니다.
두 가지 중 하나로 막아주세요(권장 1):
- 편집 팝업 열릴 때 상위 초깃값을 시딩
-import { createPortal } from 'react-dom';
+import { createPortal } from 'react-dom';
+import { useEffect } from 'react';
@@
export default function PopupPortal({
popup,
onClose,
onChange,
@@
}: Props) {
if (!popup) return null;
+ useEffect(() => {
+ if (popup.kind === 'edit') {
+ onChange?.(popup.name);
+ }
+ }, [popup.kind, popup.name]);- 혹은 확인 클릭 시 초깃값을 백업 전달(상위에서 draft 우선 사용)
- onRightClick={() => onEditConfirm?.(popup.id)}
+ onRightClick={() => onEditConfirm?.(popup.id, popup.name)}(이 경우 Sidebar의 핸들러가 draft 파라미터를 우선하도록 함께 수정 필요)
📝 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.
| onInputChange={onChange} | |
| defaultValue={popup.name} | |
| onLeftClick={onClose} | |
| onRightClick={() => onEditConfirm?.(popup.id)} | |
| /> | |
| import { createPortal } from 'react-dom'; | |
| import { useEffect } from 'react'; | |
| export default function PopupPortal({ | |
| popup, | |
| onClose, | |
| onChange, | |
| onEditConfirm, | |
| }: Props) { | |
| if (!popup) return null; | |
| useEffect(() => { | |
| if (popup.kind === 'edit') { | |
| onChange?.(popup.name); | |
| } | |
| }, [popup.kind, popup.name]); | |
| onInputChange={onChange} | |
| defaultValue={popup.name} | |
| onLeftClick={onClose} | |
| onRightClick={() => onEditConfirm?.(popup.id)} | |
| /> | |
| } |
🤖 Prompt for AI Agents
In apps/client/src/shared/components/sidebar/PopupPortal.tsx around lines 47 to
51: the component uses defaultValue + onInputChange so if the user doesn't
change the input the parent draft remains empty and an empty value may be
submitted on confirm; fix by seeding the parent state when the edit popup opens
(set the draft in the parent to popup.name) so onEditConfirm always has the
current value, alternatively (less preferred) change confirm handling to pass
popup.name when draft is empty and update the Sidebar handler to prefer draft if
provided.
| const handlePatchCategory = (id: number) => { | ||
| patchCategory( | ||
| { id, categoryName: newCategoryName }, | ||
| { | ||
| onSuccess: () => { | ||
| setNewCategoryName(''); | ||
| queryClient.invalidateQueries({ queryKey: ['dashboardCategories'] }); | ||
| close(); | ||
| }, | ||
| onError: (error) => console.error('카테고리 수정 실패:', error), | ||
| } | ||
| ); | ||
| }; |
There was a problem hiding this comment.
편집 시 입력 변경이 없으면 빈 문자열로 업데이트되는 치명적 버그
편집 팝업에서 사용자가 값을 수정하지 않으면 onChange가 호출되지 않아 newCategoryName이 빈 문자열일 수 있습니다. 이 상태로 PUT하면 카테고리명이 공백으로 바뀝니다. 또한 변경 없음(no-op)일 때는 API 호출을 생략하는 것이 안전합니다.
아래처럼 현재 이름을 안전망으로 사용하고, 공백/변경 없음 케이스를 방지해 주세요:
- const handlePatchCategory = (id: number) => {
- patchCategory(
- { id, categoryName: newCategoryName },
- {
- onSuccess: () => {
- setNewCategoryName('');
- queryClient.invalidateQueries({ queryKey: ['dashboardCategories'] });
- close();
- },
- onError: (error) => console.error('카테고리 수정 실패:', error),
- }
- );
- };
+ const handlePatchCategory = (id: number) => {
+ const currentName = getCategoryName(id);
+ const trimmed = newCategoryName.trim();
+ const nextName = trimmed || currentName;
+
+ if (!nextName) {
+ console.error('카테고리 이름이 비어 있습니다.');
+ return;
+ }
+ if (nextName === currentName) {
+ // 변경 없음: API 호출 생략
+ close();
+ return;
+ }
+ patchCategory(
+ { id, categoryName: nextName },
+ {
+ onSuccess: () => {
+ setNewCategoryName('');
+ queryClient.invalidateQueries({ queryKey: ['dashboardCategories'] });
+ close();
+ },
+ onError: (error) => console.error('카테고리 수정 실패:', error),
+ }
+ );
+ };또는 편집 팝업을 열 때 초기 입력값을 state에 주입해 리스크를 더 낮출 수 있습니다(변경 범위: Line 149 인근).
예시:
onEdit={(id, name) => {
setNewCategoryName(name); // 또는 handleCategoryChange(name)
openEdit(id, name);
}}🤖 Prompt for AI Agents
In apps/client/src/shared/components/sidebar/Sidebar.tsx around lines 71 to 83,
the patch handler uses newCategoryName which can be an empty string if the user
didn't change the input, causing accidental blanking of the category; fix by
using the current category name as a safe fallback and short-circuiting no-op
updates: compute const finalName = newCategoryName?.trim() ||
existingCategoryName; if finalName is empty or identical to
existingCategoryName, do not call patchCategory (return early); otherwise call
patchCategory with { id, categoryName: finalName } and keep existing
onSuccess/onError behavior. Also consider setting newCategoryName when opening
the edit popup (around line ~149) so the input is initialized to the current
name.
jllee000
left a comment
jllee000
left a comment
There was a problem hiding this comment.
깔끔하고 통일된 api 연결! 굿입니당
질문 하나만 확인해주세요!
| inputValue?: string; | ||
| defaultValue?: string; |
There was a problem hiding this comment.
아항 팝업키자마자
예전에 지정된 카테고리 이름으로 띄어져야 해서 담은 텍스트인가요??
이걸 기존에 inputValue 속성으로는 활용이 불가한가요?
There was a problem hiding this comment.
inputValue로 하면 수정이 안되더라고요 그래서 defaultValue로 설정하고 수정가능하게 했습니다-!!
There was a problem hiding this comment.
value로 들어가는 값이 정적으로 들어가는 것이라 value로 두게 되면 onChange로 값이 바뀌어도 계속 고정되어 있기 때문에 수정이 불가능해요!
그래서 defaultValue로 넣어주신 것 같네요 👍
constantly-dev
left a comment
constantly-dev
left a comment
There was a problem hiding this comment.
로직 너무 깔끔한 것 같아요!! 굿굿 이후에 쿼리 키를 더 잘 관리하면 invalidate 해주는 로직도 더 쉽게 쓸 수 있을 것 같아요 👍
| inputValue?: string; | ||
| defaultValue?: string; |
There was a problem hiding this comment.
value로 들어가는 값이 정적으로 들어가는 것이라 value로 두게 되면 onChange로 값이 바뀌어도 계속 고정되어 있기 때문에 수정이 불가능해요!
그래서 defaultValue로 넣어주신 것 같네요 👍
into api/#78/category-patch
3 participants
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
📷 Screenshot
Summary by CodeRabbit