[FEAT] 마커 삭제 #71
[FEAT] 마커 삭제 #71
Conversation
Walkthrough이 PR은 Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Update: 2024년 11월 24일 20시 03분 44초 |
![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
🧹 Outside diff range and nitpick comments (5)
src/hooks/api/marker/useDeleteMarkers.ts (2)
1-4: import 구문 정리가 필요합니다import 구문의 순서를 다음과 같이 정리하면 코드의 가독성이 향상될 것 같습니다:
- 외부 라이브러리 (
@tanstack/react-query)- 내부 모듈 (
@/lib/axios)다음과 같이 수정해보세요:
-import { UseMutationResult, useMutation, useQueryClient } from '@tanstack/react-query'; -import { AxiosError } from 'axios'; - -import { del } from '@/lib/axios'; +import { UseMutationResult, useMutation, useQueryClient } from '@tanstack/react-query'; +import { AxiosError } from 'axios'; +import { del } from '@/lib/axios';🧰 Tools
🪛 eslint (1.23.1)
[error] 2-2: There should be no empty line within import group
(import/order)
[error] 4-4:
@/lib/axiosimport should occur before import of@tanstack/react-query(import/order)
11-26: 에러 처리 개선이 필요합니다현재 구현은 기본적인 기능은 잘 동작하지만, 다음 사항들을 고려해보시면 좋을 것 같습니다:
- 에러 발생 시 처리 로직 추가
- 로딩 상태 처리 방법 가이드 제공
다음과 같이 개선해보세요:
export const useDeleteMarkers = (): UseMutationResult<unknown, AxiosError, DeleteMarkProps> => { const queryClient = useQueryClient(); return useMutation({ mutationFn: async ({ token, markerId }: DeleteMarkProps) => { return await del(`api/markers/${markerId}`, { headers: { Authorization: `Bearer ${token}`, }, }); }, onSuccess: () => { queryClient.invalidateQueries({ queryKey: ['marker'] }); }, + onError: (error: AxiosError) => { + console.error('마커 삭제 중 오류 발생:', error); + // 에러 처리 로직 추가 (예: 토스트 메시지 표시) + }, }); };사용 예시:
// 컴포넌트에서 사용 시 const { mutate, isLoading, isError, error } = useDeleteMarkers(); // 로딩 상태 처리 {isLoading && <LoadingSpinner />} // 에러 상태 처리 {isError && <ErrorMessage error={error} />} // 삭제 실행 mutate({ token, markerId });src/hooks/api/marker/useMarkerList.ts (1)
34-34: 페이지네이션 구현 개선이 필요합니다.현재 구현은 고정된 페이지 크기(
size=10)와 커서(cursor=0)를 사용하고 있습니다. 동적 페이지네이션을 지원하도록 개선이 필요합니다.다음과 같이 개선을 제안합니다:
-export const useMarkerList = (markerId: number, token: string) => { +export const useMarkerList = ( + markerId: number, + token: string, + cursor: number = 0, + pageSize: number = 10 +) => { return useQuery<Marker>({ queryKey: ['marker', markerId, token], queryFn: () => - get<Marker>(`api/markers/${markerId}?cursor=0&size=10`, { + get<Marker>(`api/markers/${markerId}?cursor=${cursor}&size=${pageSize}`, { headers: { Authorization: `Bearer ${token}`, }, }), }); };src/components/features/BookmarkDetail/BookmarkDetail.tsx (2)
31-35: 삭제 처리 중 로딩 상태 표시 필요사용자 경험 향상을 위해 삭제 작업 중에 로딩 상태를 표시하는 것이 좋습니다.
다음과 같이 수정을 제안합니다:
const handleDelete = () => { + setIsLoading(true); deleteMarkersMutation.mutate({ token, markerId: selectedId }, { + onSuccess: () => { setIsOpen(false); setSelectedId(0); + setIsLoading(false); + }, + onError: () => { + setIsLoading(false); + } }); };
마커 삭제 기능 구현 검토 의견
코드베이스 분석 결과, 다음과 같은 개선사항을 제안드립니다:
- 삭제 작업 중 로딩 상태 표시가 필요합니다.
deleteMarkersMutation.isLoading을 활용하여 삭제 버튼에 로딩 상태를 표시해주세요.- 삭제 작업 실패 시 에러 처리가 누락되어 있습니다.
useDeleteMarkers훅의 에러 핸들링을 추가해주세요.- 다른 mutation 구현체들과의 일관성을 위해 삭제 성공/실패 시 사용자 피드백이 필요합니다.
구체적인 수정이 필요한 위치:
handleDelete함수에 에러 처리 로직 추가- 삭제 버튼에
isLoading상태 반영- 성공/실패 피드백 UI 구현
🔗 Analysis chain
Line range hint
17-124: PR 목표 달성 확인마커 삭제 기능이 성공적으로 구현되었습니다. 다만, 다음 사항들을 고려해보시면 좋을 것 같습니다:
- 로딩 상태 표시
- 접근성 개선
- 에러 처리 및 사용자 피드백
Let me gather more information about loading states and error handling in mutations specifically.
Let me check one more aspect specifically about the mutation implementation patterns in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of error handling in other similar features # Check for toast notifications in other features rg "useToast|toast\." -A 3 "src/components/features" # Check for loading states in other mutations rg "setIsLoading|isLoading" -A 3 "src/components/features"Length of output: 2555
Script:
#!/bin/bash # Check for mutation-related loading states and error handling ast-grep --pattern 'const $_ = use$_Mutation($$$)' # Check for error handling patterns in features rg "catch|error" -A 3 "src/components/features" # Check toast implementations fd "toast" --exec cat {}Length of output: 515
Script:
#!/bin/bash # Check mutation implementations rg "useMutation" -A 5 "src/" # Check loading state patterns in mutations rg "isLoading.*mutation" "src/" # Look for similar delete operations ast-grep --pattern 'const $_ = useDelete$_($$$)'Length of output: 4473
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
src/components/features/BookmarkDetail/BookmarkDetail.tsx(3 hunks)src/hooks/api/marker/useDeleteMarkers.ts(1 hunks)src/hooks/api/marker/useMarkerList.ts(1 hunks)
🧰 Additional context used
🪛 eslint (1.23.1)
src/hooks/api/marker/useDeleteMarkers.ts
[error] 2-2: There should be no empty line within import group
(import/order)
[error] 4-4: @/lib/axios import should occur before import of @tanstack/react-query
(import/order)
🔇 Additional comments (4)
src/hooks/api/marker/useDeleteMarkers.ts (2)
6-9: 타입 정의가 명확합니다
DeleteMarkProps 타입이 인증을 위한 token과 삭제 대상을 특정하기 위한 markerId를 잘 정의하고 있습니다.
11-26: BookmarkDetail 컴포넌트와의 연동을 확인해주세요
BookmarkDetail 컴포넌트에서 이 훅이 올바르게 사용되고 있는지 확인이 필요합니다.
✅ Verification successful
Let me gather more context about the integration by checking the complete implementation of BookmarkDetail component.
useDeleteMarkers 훅이 BookmarkDetail 컴포넌트에 올바르게 통합되어 있습니다
BookmarkDetail 컴포넌트에서 다음과 같이 정상적으로 구현되어 있음을 확인했습니다:
- 필요한 의존성(
token,selectedId)이 올바르게 제공됨 deleteMarkersMutation.mutate()가handleDelete함수 내에서 적절하게 호출됨- 삭제 후 상태 관리(
setIsOpen,setSelectedId)가 정상적으로 처리됨
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# BookmarkDetail 컴포넌트에서 useDeleteMarkers 훅의 사용을 확인합니다.
# useDeleteMarkers 훅의 사용 패턴 검색
rg -l "useDeleteMarkers" src/components/features/BookmarkDetail/
# handleDelete 함수 구현 확인
ast-grep --pattern 'const handleDelete = $_' src/components/features/BookmarkDetail/
Length of output: 661
Script:
#!/bin/bash
# BookmarkDetail 컴포넌트의 전체 구현을 확인
rg -A 5 -B 5 "useDeleteMarkers" src/components/features/BookmarkDetail/BookmarkDetail.tsx
# deleteMarkersMutation 사용 패턴 확인
ast-grep --pattern 'const deleteMarkersMutation = useDeleteMarkers()' src/components/features/BookmarkDetail/BookmarkDetail.tsx
Length of output: 1271
src/hooks/api/marker/useMarkerList.ts (1)
30-32: 매개변수와 쿼리 키 변경에 대한 검증이 필요합니다.
bookmarkId에서 markerId로의 변경은 큰 아키텍처 변경을 의미합니다. 이 변경이 다른 컴포넌트나 기능에 영향을 미치지 않는지 확인이 필요합니다.
다음 스크립트를 실행하여 영향을 받는 컴포넌트를 확인하세요:
✅ Verification successful
매개변수 변경이 적절하게 구현되어 있습니다.
검증 결과, bookmarkId에서 markerId로의 변경은 새로운 마커 기능을 위한 독립적인 구현으로 확인됩니다:
useMarkerList훅은BookmarkDetail컴포넌트에서만 사용되며, 기존 북마크 관련 기능과 분리되어 있습니다.- 마커 관련 타입(
Marker,MarkersList,MarkerDetail)이 명확하게 정의되어 있습니다. - 기존 북마크 관련 컴포넌트들은 영향을 받지 않고 독립적으로 동작합니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 기존 bookmarkId 사용 여부 확인
# bookmarkId를 사용하는 컴포넌트 검색
echo "bookmarkId 사용 컴포넌트 검색:"
rg -l "bookmarkId" --type ts --type tsx
# useMarkerList 훅 사용 검색
echo "\nuseMarkerList 사용 검색:"
rg -l "useMarkerList" --type ts --type tsx
Length of output: 248
Script:
#!/bin/bash
# Description: 기존 bookmarkId와 useMarkerList 사용 여부 확인 (수정된 버전)
# bookmarkId를 사용하는 파일 검색
echo "bookmarkId 사용 파일 검색:"
rg -l "bookmarkId"
# useMarkerList 훅 사용 검색
echo -e "\nuseMarkerList 사용 검색:"
rg -l "useMarkerList"
# marker 관련 타입 정의 검색
echo -e "\nmarker 타입 정의 검색:"
rg -l "type\s+Marker"
# 파일 내용 확인
echo -e "\nuseMarkerList.ts 파일 내용:"
cat src/hooks/api/marker/useMarkerList.ts
Length of output: 1647
src/components/features/BookmarkDetail/BookmarkDetail.tsx (1)
18-20: 상태 관리가 잘 구현되어 있습니다!
각각의 상태(isOpen, isEditing, selectedId)가 단일 책임을 가지고 있어 관리가 용이합니다.
| <Delete | ||
| item={selectedMarkerName} | ||
| onClickDelete={handleDelete} | ||
| isOpen={isOpen} | ||
| setIsOpen={setIsOpen} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 처리 및 사용자 피드백 개선 필요
삭제 작업의 성공/실패에 대한 피드백이 사용자에게 제공되지 않습니다.
다음과 같이 수정을 제안합니다:
+ import { useToast } from '@/hooks/useToast';
export const BookmarkDetail = ({ bookmarkId, onPrev }: Props) => {
+ const toast = useToast();
// ...
const handleDelete = () => {
deleteMarkersMutation.mutate({ token, markerId: selectedId }, {
onSuccess: () => {
setIsOpen(false);
setSelectedId(0);
+ toast.success('마커가 성공적으로 삭제되었습니다.');
},
+ onError: (error) => {
+ toast.error('마커 삭제 중 오류가 발생했습니다.');
+ console.error('마커 삭제 실패:', error);
+ }
});
};Committable suggestion skipped: line range outside the PR's diff.
| {isEditing && ( | ||
| <Icon | ||
| name="check" | ||
| className="cursor-pointer h-7 w-7 flex-shrink-0" | ||
| color={selectedId === item.markerId ? 'primary' : 'gray'} | ||
| onClick={() => handleToggleSelect(item.markerId)} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성 개선이 필요합니다
체크 아이콘에 대한 접근성 속성이 누락되어 있습니다.
다음과 같이 수정을 제안합니다:
{isEditing && (
<Icon
name="check"
className="cursor-pointer h-7 w-7 flex-shrink-0"
color={selectedId === item.markerId ? 'primary' : 'gray'}
onClick={() => handleToggleSelect(item.markerId)}
+ role="button"
+ aria-label={`${item.title} 선택하기`}
+ aria-pressed={selectedId === item.markerId}
/>
)}📝 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.
| {isEditing && ( | |
| <Icon | |
| name="check" | |
| className="cursor-pointer h-7 w-7 flex-shrink-0" | |
| color={selectedId === item.markerId ? 'primary' : 'gray'} | |
| onClick={() => handleToggleSelect(item.markerId)} | |
| /> | |
| )} | |
| {isEditing && ( | |
| <Icon | |
| name="check" | |
| className="cursor-pointer h-7 w-7 flex-shrink-0" | |
| color={selectedId === item.markerId ? 'primary' : 'gray'} | |
| onClick={() => handleToggleSelect(item.markerId)} | |
| role="button" | |
| aria-label={`${item.title} 선택하기`} | |
| aria-pressed={selectedId === item.markerId} | |
| /> | |
| )} |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/components/features/BookmarkDetail/BookmarkDetail.tsx (2)
42-47: useEffect 정리(cleanup) 함수 추가 필요컴포넌트가 언마운트될 때 마커 상태를 정리해야 합니다.
다음과 같이 수정을 제안합니다:
useEffect(() => { if (data?.markers?.data) { setMarkers(data.markers.data); } + return () => { + setMarkers([]); + }; }, [data?.markers?.data, setMarkers]);
105-112: 삭제 버튼에 로딩 상태 표시 필요사용자에게 삭제 작업의 진행 상태를 표시해야 합니다.
다음과 같이 수정을 제안합니다:
<Button variant="primary" size="large" - disabled={selectedId === 0} + disabled={selectedId === 0 || deleteMarkersMutation.isLoading} onClick={() => setIsOpen(true)} > - 삭제하기 + {deleteMarkersMutation.isLoading ? '삭제 중...' : '삭제하기'} </Button>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/components/features/BookmarkDetail/BookmarkDetail.tsx(3 hunks)
🔇 Additional comments (1)
src/components/features/BookmarkDetail/BookmarkDetail.tsx (1)
1-21: 상태 관리가 잘 구현되어 있습니다!
상태 변수들이 적절하게 분리되어 있고, 각각의 책임이 명확합니다.
| const handleDelete = () => { | ||
| deleteMarkersMutation.mutate({ token, markerId: selectedId }); | ||
| setIsOpen(false); | ||
| setSelectedId(0); | ||
| }; |
There was a problem hiding this comment.
삭제 작업의 에러 처리가 필요합니다
현재 삭제 작업 실패 시 사용자에게 피드백이 없습니다. 또한 로딩 상태도 처리되지 않습니다.
다음과 같이 수정을 제안합니다:
const handleDelete = () => {
- deleteMarkersMutation.mutate({ token, markerId: selectedId });
- setIsOpen(false);
- setSelectedId(0);
+ deleteMarkersMutation.mutate(
+ { token, markerId: selectedId },
+ {
+ onSuccess: () => {
+ setIsOpen(false);
+ setSelectedId(0);
+ setIsEditing(false);
+ },
+ onError: (error) => {
+ console.error('마커 삭제 실패:', error);
+ alert('마커 삭제 중 오류가 발생했습니다.');
+ }
+ }
+ );
};📝 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.
| const handleDelete = () => { | |
| deleteMarkersMutation.mutate({ token, markerId: selectedId }); | |
| setIsOpen(false); | |
| setSelectedId(0); | |
| }; | |
| const handleDelete = () => { | |
| deleteMarkersMutation.mutate( | |
| { token, markerId: selectedId }, | |
| { | |
| onSuccess: () => { | |
| setIsOpen(false); | |
| setSelectedId(0); | |
| setIsEditing(false); | |
| }, | |
| onError: (error) => { | |
| console.error('마커 삭제 실패:', error); | |
| alert('마커 삭제 중 오류가 발생했습니다.'); | |
| } | |
| } | |
| ); | |
| }; |
관련 이슈
close #61
📑 작업 내용
2024-11-24.7.34.01.mov
💬 리뷰 중점 사항/기타 참고 사항
Summary by CodeRabbit
새 기능
useDeleteMarkers도입.버그 수정
문서화