-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat: #170 일정 삭제 기능 구현 #177
Conversation
Walkthrough이번 변경 사항은 새로운 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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (7)
src/components/modal/ModalButton.tsx (2)
1-5
: 타입 정의가 명확하고 잘 구조화되어 있습니다.
ModalButtonProps
타입 정의가 잘 되어 있습니다.React.PropsWithChildren
을 사용하여 유연한 콘텐츠를 허용하고,backgroundColor
prop에 대한 명확한 유니온 타입을 정의하여 타입 안정성을 보장하고 있습니다.다만, 더 나은 유지보수를 위해
backgroundColor
타입을 별도로 정의하는 것을 고려해 보세요. 예를 들어:type BackgroundColor = 'bg-main' | 'bg-delete' | 'bg-sub' | 'bg-button' | 'bg-kakao'; type ModalButtonProps = React.PropsWithChildren<{ formId?: string; backgroundColor: BackgroundColor; onClick?: () => void; }>;이렇게 하면 나중에 배경색을 추가하거나 수정할 때 더 쉽게 관리할 수 있습니다.
7-20
: 컴포넌트 구현이 잘 되어 있습니다. 몇 가지 개선 사항을 제안합니다.
ModalButton
컴포넌트의 구현이 전반적으로 잘 되어 있습니다. React 모범 사례를 따르고 있으며, 조건부 버튼 타입 설정과 동적 스타일링을 위한 템플릿 리터럴 사용이 좋습니다.개선을 위한 제안:
onClick
핸들러에 옵셔널 체이닝을 사용하세요:const handleClick = () => onClick?.();
- 성능 최적화를 위해
useCallback
을 고려해 보세요:const handleClick = useCallback(() => onClick?.(), [onClick]);
- 접근성을 위해
aria-label
속성을 추가하는 것이 좋습니다:<button type={formId ? 'submit' : 'button'} form={formId} className={`h-full w-full rounded-md px-10 text-white ${backgroundColor}`} onClick={handleClick} aria-label={/* 버튼의 목적을 설명하는 레이블 */} > {children} </button>
- 버튼의 비활성화 상태를 지원하는 것을 고려해 보세요. 이는 사용자 경험을 향상시킬 수 있습니다.
이러한 변경사항들은 컴포넌트의 재사용성과 유지보수성을 더욱 향상시킬 것입니다.
🧰 Tools
🪛 Biome
[error] 8-8: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/components/modal/task/DetailModalTask.tsx (2)
22-22
: 일정 삭제 기능이 올바르게 구현되었습니다.
useDeleteTask
훅을 사용하여 일정 삭제 기능을 구현한 것은 좋습니다. PR의 목표와 일치합니다.다만, 코드의 가독성을 높이기 위해 구조 분해 할당을 사용하는 것이 좋겠습니다:
const { mutate: deleteTaskMutate } = useDeleteTask(project.projectId);이렇게 하면
deleteTaskMutate
함수의 용도가 더 명확해집니다.
96-103
: 모달 버튼이 적절히 구현되었습니다.
ModalButton
컴포넌트를 사용하여 수정 및 삭제 버튼을 구현한 것은 좋습니다. PR의 목표와 일치하며 UI의 일관성을 향상시킵니다.개선 제안:
- 일관성을 위해
handleUpdateClick
도handleDeleteClick
과 같은 방식으로 호출하는 것이 좋습니다:<ModalButton backgroundColor="bg-main" onClick={() => handleUpdateClick(task.taskId)}> 수정 </ModalButton>
- 접근성을 높이기 위해 버튼에
aria-label
을 추가하는 것을 고려해보세요:<ModalButton backgroundColor="bg-delete" onClick={() => handleDeleteClick(task.taskId)} aria-label="일정 삭제" > 삭제 </ModalButton>이러한 변경사항들은 코드의 일관성과 접근성을 향상시킬 것입니다.
src/services/taskService.ts (1)
135-151
: 일정 삭제 기능이 올바르게 구현되었습니다.
deleteTask
함수가 PR 목표에 맞게 잘 구현되었습니다. 함수 서명과 구현이 파일의 다른 API 서비스 함수들과 일관성을 유지하고 있습니다.제안사항:
- 에러 처리를 개선하기 위해 try-catch 블록을 추가하는 것을 고려해보세요. 이를 통해 네트워크 오류나 서버 오류를 더 세밀하게 처리할 수 있습니다.
다음과 같이 에러 처리를 추가할 수 있습니다:
export async function deleteTask( projectId: Project['projectId'], taskId: Task['taskId'], axiosConfig: AxiosRequestConfig = {}, ) { try { return await authAxios.delete(`/project/${projectId}/task/${taskId}`, axiosConfig); } catch (error) { console.error('일정 삭제 중 오류 발생:', error); throw error; } }src/hooks/query/useTaskQuery.ts (1)
193-214
:useDeleteTask
함수가 잘 구현되었습니다.이 함수는 다음과 같은 장점을 가지고 있습니다:
- React Query의
useMutation
훅을 올바르게 사용하여 구현되었습니다.- 성공 및 오류 케이스에 대한 처리가 toast 알림을 통해 적절히 이루어졌습니다.
- 성공 시 관련 쿼리를 무효화하고 특정 쿼리를 제거하여 UI 일관성을 보장합니다.
- PR 목표와 연결된 이슈 요구사항에 부합합니다.
다만, 한 가지 제안사항이 있습니다:
- 성능 최적화를 위해
queryClient.invalidateQueries
와queryClient.removeQueries
호출을 Promise.all로 병렬 처리하는 것을 고려해 보세요.다음과 같이 수정할 수 있습니다:
onSuccess: (res, taskId) => { const tasksQueryKey = generateTasksQueryKey(projectId); const filesQueryKey = generateTaskFilesQueryKey(projectId, taskId); const assigneesQueryKey = generateTaskAssigneesQueryKey(projectId, taskId); toastSuccess('일정을 삭제 했습니다.'); - queryClient.invalidateQueries({ queryKey: tasksQueryKey }); - queryClient.removeQueries({ queryKey: filesQueryKey }); - queryClient.removeQueries({ queryKey: assigneesQueryKey }); + Promise.all([ + queryClient.invalidateQueries({ queryKey: tasksQueryKey }), + queryClient.removeQueries({ queryKey: filesQueryKey }), + queryClient.removeQueries({ queryKey: assigneesQueryKey }) + ]); },src/mocks/services/taskServiceHandler.ts (1)
217-220
: 인증 토큰 유효성 검증 로직 추가 필요현재 인증 토큰의 존재 여부만 확인하고 있습니다. 토큰의 유효성을 검증하고, 권한 체크를 수행하여 보안을 강화해야 합니다.
JWT 토큰을 검증하고 사용자 권한을 확인하는 로직을 추가하여 미승인 접근을 방지하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
tailwind.config.js
is excluded by none and included by none
📒 Files selected for processing (6)
- src/components/modal/ModalButton.tsx (1 hunks)
- src/components/modal/task/DetailModalTask.tsx (4 hunks)
- src/globals.css (1 hunks)
- src/hooks/query/useTaskQuery.ts (3 hunks)
- src/mocks/services/taskServiceHandler.ts (1 hunks)
- src/services/taskService.ts (2 hunks)
🧰 Additional context used
🪛 Biome
src/components/modal/ModalButton.tsx
[error] 8-8: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (6)
src/globals.css (1)
39-39
: 새로운 색상 변수가 올바르게 추가되었습니다.
--color-delete
변수가 적절하게 추가되었습니다. 이 밝은 빨간색은 삭제 작업에 적합하며, 기존 색상 변수들과 일관된 명명 규칙을 따르고 있습니다.다음 스크립트를 실행하여 새로운 색상 변수의 사용을 확인하세요:
src/components/modal/task/DetailModalTask.tsx (2)
9-9
: 가져오기 문이 적절히 업데이트되었습니다.새로운 기능과 UI 변경사항을 위해 필요한
useDeleteTask
와ModalButton
컴포넌트가 올바르게 가져와졌습니다. 이는 PR의 목표와 일치합니다.Also applies to: 13-13
Line range hint
1-110
: 전반적인 변경사항 요약 및 주요 개선 포인트이 PR은 일정 삭제 기능 구현과 UI 업데이트라는 목표를 잘 달성했습니다.
useDeleteTask
훅의 사용,ModalButton
컴포넌트의 도입, 그리고 삭제 기능의 구현은 모두 적절합니다.주요 개선 포인트:
- 사용자 권한 확인 로직 구현: 이는 보안상 중요하며, 우선적으로 처리해야 합니다.
- 미구현된
handleUpdateClick
함수 제거 또는 주석 처리- 버튼 컴포넌트의 일관성 및 접근성 개선
이러한 변경사항들을 적용하면 코드의 품질과 안정성이 크게 향상될 것입니다. 전반적으로 좋은 작업이었습니다!
src/services/taskService.ts (2)
122-122
: 문서 주석 서식이 개선되었습니다.
formData
매개변수 설명의 정렬이 개선되어 가독성이 향상되었습니다. 이는 파일 내의 다른 함수 문서와의 일관성을 유지하는 데 도움이 됩니다.
Line range hint
1-151
: PR 목표에 부합하는 변경사항입니다.이 파일의 변경사항은 PR 목표와 잘 일치합니다:
deleteTask
함수 추가로 일정 삭제 API 기능이 구현되었습니다.- 문서 주석의 minor 포맷팅 개선으로 코드의 일관성이 향상되었습니다.
이러한 변경사항들은 일정 관리 기능을 강화하고 코드의 가독성을 개선하는 데 기여합니다.
src/hooks/query/useTaskQuery.ts (1)
4-4
: 가져오기 문이 적절히 업데이트되었습니다.새로운
useDeleteTask
함수와 관련된generateStatusesQueryKey
와deleteTask
가 올바르게 가져와졌습니다. 이는 새로운 기능 구현에 필요한 적절한 변경사항입니다.Also applies to: 13-13
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
모달 버튼 컴포넌트를 하나 따로 만드셨군요! 디자인이 늘어나서 컴포넌트 작성해야 할 것도 계속 추가되네요😂 삭제 작업까지 고생 많으셨습니다!
@Yoonyesol 리뷰 감사합니다😆 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
일정 삭제 기능 구현 고생하셨습니다!
@ice-bear98 리뷰 감사합니다! |
PR Type
What kind of change does this PR introduce?
Related Issues
What does this PR do?
Other information
기존의 디자인에서 상세 페이지가 추가되고, 수정 모달쪽의 UI가 전반적으로 수정되어서 ModalFormButton 컴포넌트 사용이 애매해졌습니다. 그래서 새롭게 모달의 버튼을 구성해줄 ModalButton 컴포넌트를 추가한 상태고, 이번 PR 이후로 다른 모달 컴포넌트의 버튼들을 모두 대체하려고 생각하고 있습니다.
View
일정 삭제 기능 실패한 경우
일정 추가 기능 성공한 경우