Refactor: #243 전체 일정 조회에서 단일 일정 조회 로직 분리 #244
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 (
|
![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 (9)
src/components/modal/project-status/UpdateModalProjectStatus.tsx (4)
27-27: 로딩 상태 관리 개선을 고려해보세요.현재 구현에서는 단일 로딩 상태만 사용하고 있습니다. 향후 더 세분화된 사용자 경험을 위해 다음과 같은 개선을 고려해볼 수 있습니다:
- 초기 로딩과 업데이트 로딩 상태 분리
- 에러 상태에 대한 별도 처리
예시 구현:
const { statusTaskList, isTasksLoading, isTasksError, isTasksUpdating } = useReadStatusTasks(project.projectId);
Line range hint
41-47: 에러 처리 개선이 필요합니다.현재 에러 처리가 console.error로만 되어있습니다. 사용자에게 적절한 피드백을 제공하도록 개선이 필요합니다.
다음과 같은 개선을 제안합니다:
const handleDeleteClick = (statusId: ProjectStatus['statusId']) => { try { const statusTasks = statusTaskList.find((statusTask) => statusTask.statusId === statusId); if (!statusTasks) { toastWarn('일치하는 프로젝트 상태가 없습니다.'); return; } if (statusTasks.tasks.length > 0) { toastWarn('프로젝트 상태에 일정이 등록되어 있습니다.'); return; } deleteStatusMutate(statusId); } catch (error) { toastWarn('상태 삭제 중 오류가 발생했습니다.'); console.error(error); } };
Line range hint
35-35: TODO 주석을 이슈로 관리하면 좋겠습니다.코드 내 TODO 주석이 발견되었습니다. 이를 GitHub 이슈로 관리하면 더 체계적인 추적이 가능할 것 같습니다.
GitHub 이슈를 생성하여 에러 처리 개선 작업을 추적하도록 도와드릴까요?
Line range hint
44-44: 타입 가드 적용을 고려해보세요.현재 Error 인스턴스 체크가 에러 처리의 마지막에 위치해 있습니다. 타입 가드를 활용하여 더 견고한 에러 처리가 가능할 것 같습니다.
const isError = (error: unknown): error is Error => { return error instanceof Error; }; // 사용 예시 } catch (error) { if (isError(error)) { toastWarn(error.message); console.error(`${error.name}: ${error.message}`); } else { toastWarn('알 수 없는 오류가 발생했습니다.'); console.error(error); } }src/components/modal/task/DetailModalTask.tsx (2)
Line range hint
41-75: 이벤트 핸들러 함수들의 에러 처리를 개선하면 좋을 것 같습니다.특히
handleDownloadClick함수에서 파일 다운로드 실패 시 사용자에게 더 구체적인 피드백을 제공하면 좋을 것 같습니다.다음과 같이 에러 메시지를 개선하는 것을 제안드립니다:
- toastError('파일 다운로드 중 문제가 발생했습니다. 잠시 후 다시 시도해주세요.'); + toastError('파일 다운로드에 실패했습니다. 파일이 존재하지 않거나 손상되었을 수 있습니다. 잠시 후 다시 시도해주세요.');
Line range hint
47-47: TODO 주석에 대한 후속 조치가 필요합니다.사용자 권한 확인 로직 추가에 대한 TODO 주석이 있습니다. 이는 보안상 중요한 기능이므로 우선적으로 구현이 필요합니다.
권한 확인 로직 구현을 위한 코드를 생성해드릴까요? 아니면 GitHub 이슈를 생성해드릴까요?
src/components/modal/task/ModalTaskForm.tsx (1)
48-50: 변수명 변경과 메모이제이션 처리가 적절합니다.변수명을 더 명확하게 변경하고
useMemo를 사용하여 성능을 최적화한 것이 좋습니다. 다만, 다음과 같은 작은 개선을 제안드립니다:- const taskNameList = useMemo(() => getTaskNameList(statusTaskList), [statusTaskList]); + const taskNameList = useMemo( + () => getTaskNameList(statusTaskList), + [statusTaskList] + );가독성을 위해
useMemo콜백을 여러 줄로 분리하는 것을 고려해보세요.src/hooks/query/useTaskQuery.ts (1)
Line range hint
70-85: useReadStatusTasks 훅의 책임이 명확해졌습니다.단일 일정 조회 로직이 분리되어 이 훅의 책임이 전체 일정 조회로 단순화되었습니다. 변수명도 더 명확해졌습니다.
하지만 statusTaskList의 타입 안전성을 위해 기본값 타입을 명시하는 것이 좋습니다.
- data: statusTaskList = [], + data: statusTaskList: TaskListWithStatus[] = [],src/components/modal/task/UpdateModalTask.tsx (1)
Line range hint
1-215: 아키텍처 관련 제안사항컴포넌트의 책임이 다소 과도해 보입니다. 다음과 같은 개선사항을 고려해보시기 바랍니다:
- 파일 업로드 로직을 별도의 컴포넌트로 분리
- 수행자 관리 로직을 별도의 컴포넌트로 분리
- 폼 로직을 커스텀 훅으로 분리
이러한 분리를 통해 코드의 유지보수성과 테스트 용이성이 향상될 것으로 예상됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- src/components/modal/project-status/UpdateModalProjectStatus.tsx (2 hunks)
- src/components/modal/task/DetailModalTask.tsx (5 hunks)
- src/components/modal/task/ModalTaskForm.tsx (3 hunks)
- src/components/modal/task/UpdateModalTask.tsx (5 hunks)
- src/hooks/query/useTaskQuery.ts (3 hunks)
- src/utils/extractNameList.ts (1 hunks)
🔇 Additional comments (14)
src/utils/extractNameList.ts (2)
1-1: 타입 임포트가 깔끔하게 구현되었습니다!절대 경로를 사용한 타입 임포트가 잘 되어있습니다.
3-10: 🛠️ Refactor suggestion입력값 유효성 검사 추가를 고려해주세요.
taskList가 null이거나 빈 배열일 경우, 또는 tasks 배열 내부의 요소가 null일 경우에 대한 처리가 필요합니다.다음과 같은 방어 로직 추가를 제안드립니다:
export function getTaskNameList(taskList: TaskListWithStatus[], excludedTaskName?: Task['taskName']) { + if (!taskList?.length) { + return []; + } + const taskNameList = taskList .flatMap((statusTask) => statusTask.tasks) + .filter((task): task is Task => task != null) .map((task) => task.taskName); return excludedTaskName ? taskNameList.filter((taskName) => taskName !== excludedTaskName) : taskNameList; }src/components/modal/project-status/UpdateModalProjectStatus.tsx (1)
27-27: 변수명 변경이 적절합니다!
isTaskLoading에서isTasksLoading으로의 변경은 복수의 일정을 로딩하는 상태를 더 정확하게 표현합니다.Also applies to: 53-53
src/components/modal/task/DetailModalTask.tsx (4)
12-12: 훅 임포트 구문이 적절히 업데이트되었습니다.단일 일정 조회 로직을 분리하면서 새로운
useReadStatusTask훅을 임포트하도록 변경되었습니다.
35-35: 단일 일정 조회를 위한 훅 사용이 올바르게 리팩토링되었습니다.전체 일정 조회에서 단일 일정 조회 로직이 분리되어
useReadStatusTask훅으로 변경되었고, 관련 변수명도 의미에 맞게 수정되었습니다.
76-76: 로딩 상태 체크 로직이 일관성있게 수정되었습니다.여러 데이터 로딩 상태를 확인하는 조건문에서
isTasksLoading으로 변수명이 변경되었습니다.
89-89: task 객체 참조가 statusTask로 일관되게 변경되었습니다.컴포넌트 전반에 걸쳐 task 객체 참조가 statusTask로 올바르게 변경되었습니다:
- 일정명 표시
- 기간 정보 표시
- 일정 내용 표시
Also applies to: 93-93, 108-108
src/components/modal/task/ModalTaskForm.tsx (2)
30-30: 유틸리티 함수 분리가 잘 이루어졌습니다!일정 이름 목록을 추출하는 로직을 별도의 유틸리티 함수로 분리한 것이 코드의 재사용성과 유지보수성을 향상시켰습니다.
142-142: 로딩 상태 처리가 포괄적입니다.모든 필요한 데이터의 로딩 상태를 확인하여 안전하게 처리하고 있습니다. 특히
isTasksLoading을 포함시켜 데이터 일관성을 보장하는 점이 좋습니다.src/hooks/query/useTaskQuery.ts (2)
33-33: 타입 임포트가 적절히 추가되었습니다.TaskFile 타입 임포트 추가는 코드의 타입 안정성을 향상시킵니다.
87-99: 🛠️ Refactor suggestion단일 일정 조회를 위한 새로운 훅이 잘 구현되었습니다.
useMemo를 사용하여 성능 최적화가 잘 되어있습니다. 하지만 몇 가지 개선사항을 제안드립니다:
- statusTask가 undefined일 수 있는 경우에 대한 처리가 필요합니다.
- 타입 안전성을 위해 반환 타입을 명시하는 것이 좋습니다.
export function useReadStatusTask( projectId: Project['projectId'], taskId: Task['taskId'] + ): { + statusTask: Task | undefined; + isTasksLoading: boolean; + isTasksError: boolean; + tasksError: Error | null; + }또한 이 훅의 사용 패턴을 검증해보는 것이 좋겠습니다.
src/components/modal/task/UpdateModalTask.tsx (3)
20-20: 훅 사용 및 임포트 구조가 개선되었습니다.단일 일정 조회 로직이 분리되어
useReadStatusTask훅이 추가된 것이 확인됩니다. 이는 PR의 목적에 잘 부합합니다.Also applies to: 26-30
65-70: 일정 이름 목록 생성 로직이 개선되었습니다.
getTaskNameList유틸리티 함수를 사용하여 일정 이름 목록을 생성하는 방식이 잘 구현되었습니다.useMemo를 통한 최적화도 적절합니다.
92-101: 🛠️ Refactor suggestion폼 초기화 로직 검증이 필요합니다.
statusTask가 undefined일 경우에 대한 처리가 명시적이지 않습니다. 초기 렌더링 시 발생할 수 있는 잠재적인 문제를 예방하기 위해 검증이 필요합니다.다음과 같이 개선하는 것을 제안드립니다:
useEffect(() => { - if (statusTask) { + if (!statusTask) { + return; + } reset({ statusId: statusTask.statusId.toString(), taskName: statusTask.taskName, content: statusTask.content, startDate: statusTask.startDate, endDate: statusTask.endDate, }); - } }, [statusTask, reset]);
PR Type
What kind of change does this PR introduce?
Related Issues
What does this PR do?