-
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: #150 일정 수정 모달 수행자 삭제 기능 구현 #158
Conversation
Walkthrough이 변경 사항은 일정 수정 모달에서 수행자를 삭제하는 기능을 추가하는 것입니다. 새로운 API와 훅을 통해 수행자를 삭제할 수 있는 기능이 구현되었으며, 관련된 서비스와 모의 서비스도 업데이트되었습니다. 이를 통해 사용자 인터페이스는 삭제된 수행자에 대한 최신 상태를 반영하게 됩니다. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
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: 3
Outside diff range and nitpick comments (4)
src/services/taskService.ts (1)
113-131
: LGTM! 일정 수행자 삭제 기능이 잘 구현되었습니다.새로운
deleteAssignee
함수가 기존 코드 패턴을 잘 따르고 있으며, 타입 안전성을 보장하고 있습니다. API 엔드포인트 구성도 RESTful 관행을 잘 따르고 있습니다.다만, 에러 처리를 개선하기 위해 다음과 같은 작은 수정을 제안합니다:
export async function deleteAssignee( projectId: Project['projectId'], taskId: Task['taskId'], userId: User['userId'], axiosConfig: AxiosRequestConfig = {}, ) { - return authAxios.delete(`/project/${projectId}/task/${taskId}/assignee/${userId}`, axiosConfig); + try { + return await authAxios.delete(`/project/${projectId}/task/${taskId}/assignee/${userId}`, axiosConfig); + } catch (error) { + console.error('일정 수행자 삭제 중 오류 발생:', error); + throw error; + } }이렇게 하면 에러가 발생했을 때 로그를 남기고 상위 컴포넌트에서 에러를 처리할 수 있게 됩니다.
src/hooks/query/useTaskQuery.ts (1)
160-179
: LGTM:useDeleteAssignee
함수가 잘 구현되었습니다.
useDeleteAssignee
함수의 구현이 적절하게 이루어졌습니다. 에러 처리, 성공 알림, 그리고 캐시 무효화가 모두 올바르게 구현되어 있습니다. 이는 PR의 목표인 담당자 삭제 기능을 잘 구현하고 있습니다.함수에 JSDoc 주석을 추가하면 코드의 가독성과 유지보수성이 향상될 것 같습니다. 다음과 같이 추가해 보는 것은 어떨까요?
/** * 일정의 담당자를 삭제하는 훅 * @param projectId 프로젝트 ID * @param taskId 작업 ID * @returns 담당자 삭제 뮤테이션 객체 */ export function useDeleteAssignee(projectId: Project['projectId'], taskId: Task['taskId']) { // ... 현재 구현 ... }src/components/modal/task/UpdateModalTask.tsx (2)
59-59
: deleteAssigneeMutate 함수가 적절히 추가되었습니다.
useDeleteAssignee
훅을 사용하여deleteAssigneeMutate
함수를 정의한 것은 PR의 목표와 일치합니다.에러 처리를 개선하기 위해 다음과 같이
onError
콜백을 추가하는 것을 고려해보세요:const { mutate: deleteAssigneeMutate } = useDeleteAssignee(projectId, taskId, { onError: (error) => { console.error('담당자 삭제 중 오류 발생:', error); toastWarn('담당자 삭제 중 오류가 발생했습니다. 다시 시도해 주세요.'); } });
97-101
: handleAssigneeDeleteClick 함수가 적절히 구현되었습니다.담당자 삭제 로직이 올바르게 구현되었으며, 사용자가 담당자 목록에 없는 경우에 대한 오류 처리도 적절합니다.
toastInfo
를 사용한 사용자 피드백도 적절합니다.코드의 가독성을 높이기 위해 다음과 같이 early return 패턴을 사용하는 것을 고려해보세요:
const handleAssigneeDeleteClick = (user: SearchUser) => { const isIncludedUser = assigneeList.find((assignee) => assignee.userId === user.userId); if (!isIncludedUser) { toastInfo('수행자 목록에 없는 대상입니다. 확인 후 다시 시도 해주세요.'); return; } deleteAssigneeMutate(user.userId); };
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/components/modal/task/UpdateModalTask.tsx (3 hunks)
- src/hooks/query/useTaskQuery.ts (2 hunks)
- src/mocks/services/taskServiceHandler.ts (1 hunks)
- src/services/taskService.ts (1 hunks)
Additional context used
Learnings (1)
src/components/modal/task/UpdateModalTask.tsx (1)
Learnt from: Seok93 PR: GU-99/grow-up-fe#155 File: src/components/modal/task/UpdateModalTask.tsx:76-79 Timestamp: 2024-09-24T07:11:16.095Z Learning: `UpdateModalTask` 컴포넌트에서 `assigneeList`는 정의되어 있으며 올바르게 초기화되어 있습니다.
Additional comments not posted (4)
src/hooks/query/useTaskQuery.ts (2)
6-6
: LGTM: 새로운 import가 올바르게 추가되었습니다.
deleteAssignee
함수의 import가 적절하게 추가되었습니다. 이는 새로운useDeleteAssignee
훅에 필요한 것으로 보입니다.
Line range hint
1-179
: PR 목표와 일치하는 변경사항입니다.이 PR의 변경사항은 일정 수정 모달에서 담당자 삭제 기능을 구현하는 목표를 잘 달성하고 있습니다.
useDeleteAssignee
함수의 추가는 React Query를 사용하여 담당자 삭제 API를 처리하는 요구사항을 충족시키고 있습니다.구현된 기능은 다음과 같습니다:
- 담당자 삭제 API를 위한 React Query 기능 추가
- 에러 처리 및 성공 시 알림 구현
- 캐시 무효화를 통한 UI 업데이트 보장
전반적으로 코드는 잘 구조화되어 있으며, 기존 파일의 패턴을 따르고 있습니다. 제안된 JSDoc 주석 추가를 고려하면 코드의 문서화가 더욱 개선될 것 같습니다.
src/components/modal/task/UpdateModalTask.tsx (2)
19-25
: import 문 업데이트가 적절합니다.
useDeleteAssignee
훅의 추가는 PR의 목표인 담당자 삭제 기능 구현과 일치합니다. 다른 태스크 관련 훅들과 함께 적절히 배치되어 있습니다.
Line range hint
1-190
: PR 목표와 일치하는 변경 사항입니다.이 PR의 변경 사항들은 일정 수정 모달에서 담당자 삭제 기능을 구현하는 목표를 잘 달성하고 있습니다.
useDeleteAssignee
훅의 추가,deleteAssigneeMutate
함수의 정의, 그리고handleAssigneeDeleteClick
함수의 구현은 모두 요구사항을 충족시키고 있습니다.제안된 개선 사항들(에러 처리 추가 및 코드 가독성 개선)을 고려하여 구현하면 코드의 품질을 더욱 향상시킬 수 있을 것입니다.
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.
하루에 PR을 4개나 올리는 사람이 있다...???😱 삭제 작업까지 고생 많으셨습니다!! 저는 이제... 4개의 PR 코드와 리뷰 내용을 하나하나 읽어보려고요😂
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 @ice-bear98 리뷰 감사합니다! |
PR Type
What kind of change does this PR introduce?
Related Issues
What does this PR do?
View
수행자 추가 에러시 화면
수행자 추가 성공시 화면