Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR refactors multiple React components from self-contained state management to prop-driven APIs, introduces a new service layer for calendar/schedule/todo operations, and updates type definitions to align with backend API contracts. The Calendar component transitions from local state to controlled year/month props with async daily schedule fetching. Other components (TodoList, UpcomingSchedule, MemberHeader, DeadlineSelector) are similarly converted to accept external state and data via props. Pages (GroupHomePage, HomePage, CreateSchedulePage, CreateToDoPage, MemberPage) are updated to fetch and orchestrate data via the new service layer before passing it downstream. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello @nyoeng, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 홈 페이지, 멤버 페이지, 그리고 To-Do 및 일정 생성 기능에 대한 API 연동을 구현하여 애플리케이션의 데이터 동기화를 강화하고 사용자 경험을 개선합니다. 기존의 목업 데이터를 실제 백엔드 API 호출로 대체함으로써, 달력의 일별 일정, To-Do 및 다가오는 일정 목록을 동적으로 관리하고, 새로운 일정과 To-Do를 생성하는 기능을 백엔드와 연결합니다. 또한, 멤버 페이지는 스터디 멤버 정보를 동적으로 받아와 표시하도록 개선되었습니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| return; | ||
| } | ||
| // 더미 멤버 상태 초기화 | ||
| setMembers(dummyMembers.map((m) => ({ ...m, checked: false }))); |
There was a problem hiding this comment.
To-Do 생성 페이지에서 담당 멤버를 선택하는 부분에 dummyMembers라는 목(mock) 데이터를 사용하고 있습니다. GroupHomePage에서 studyId를 통해 실제 스터디 멤버를 조회하는 것처럼, 이 페이지에서도 실제 스터디 멤버 목록을 불러와야 기능이 올바르게 동작합니다. GroupHomePage에서 MemberPage로 이동할 때 location.state로 멤버 목록을 전달하는 방식을 참고하여 수정하면 좋을 것 같습니다.
| const [currentMonth, setCurrentMonth] = useState(today.getMonth()); // Month is 0-indexed | ||
|
|
||
| // Temporary hardcoded studyId for HomePage | ||
| const tempStudyId = "mock-study-id-for-homepage"; // Use a UUID here in a real app |
There was a problem hiding this comment.
홈페이지에서 tempStudyId를 하드코딩하여 사용하고 있습니다. 이렇게 되면 실제 사용자는 자신의 데이터가 아닌, 특정 테스트용 스터디의 정보만 보게 됩니다. 현재 로그인한 사용자의 모든 스터디 정보를 가져오거나, 사용자가 스터디를 선택할 수 있도록 API와 UI 로직 수정이 필요해 보입니다.
백엔드에 현재 사용자의 모든 To-Do와 캘린더 정보를 가져오는 API 엔드포인트(e.g. /me/todos, /me/calendar)를 요청하거나, 사용자가 앱 진입 시 스터디를 선택하는 흐름을 추가하는 것을 고려해 보세요.
|
|
||
| return ( | ||
| <li | ||
| key={`${s.title}-${index}`} // Use a combination for key as no unique ID for schedule item |
There was a problem hiding this comment.
li 요소의 key를 s.title과 index의 조합으로 생성하고 있습니다. 만약 같은 날짜에 동일한 제목의 일정이 여러 개 있다면 key가 중복될 수 있으며, 이는 React의 렌더링 오류를 유발할 수 있습니다. API 응답에서 각 일정(schedule)에 대한 고유 ID를 제공받아 key로 사용하는 것이 가장 이상적입니다.
백엔드 API를 수정하여 DailySchedule 객체에 고유한 id 필드를 포함하도록 요청하는 것을 권장합니다.
| // Group fetchedTodos by groupName | ||
| const grouped: { [key: string]: TodoGroup } = {}; | ||
| fetchedTodos.forEach(todoItem => { | ||
| if (!grouped[todoItem.groupName]) { | ||
| grouped[todoItem.groupName] = { | ||
| id: todoItem.groupId, // Assuming groupId is unique for a group | ||
| groupName: todoItem.groupName, | ||
| todos: [], | ||
| }; | ||
| } | ||
| grouped[todoItem.groupName].todos.push({ | ||
| id: todoItem.id, | ||
| name: todoItem.name, | ||
| description: todoItem.description, | ||
| dueDate: todoItem.dueDate, | ||
| certificationMethods: todoItem.certificationMethods, | ||
| isCompleted: todoItem.isCompleted, | ||
| completedParticipants: todoItem.completedParticipants, | ||
| totalParticipants: todoItem.totalParticipants, | ||
| // Map to old Todo properties if still used by TodoItem | ||
| taskName: todoItem.name, | ||
| completedCount: todoItem.completedParticipants, | ||
| totalCount: todoItem.totalParticipants, | ||
| isChecked: todoItem.isCompleted, | ||
| }); | ||
| }); | ||
| setGroupedTodos(Object.values(grouped)); |
There was a problem hiding this comment.
fetchTodos 함수 내에서 API로부터 받은 To-Do 목록을 groupName으로 그룹화하는 로직이 있습니다. 이 로직이 HomePage.tsx (lines 38-63)에서도 거의 동일하게 반복되고 있습니다. 코드 중복은 유지보수를 어렵게 만드므로, 이 그룹화 로직을 별도의 유틸리티 함수나 커스텀 훅으로 분리하여 재사용하는 것을 권장합니다.
개선 제안:
utils/todo.ts와 같은 유틸리티 파일을 만들어 그룹화 로직을 옮기고, 각 페이지에서 이를 가져와 사용해 보세요.
// utils/todo.ts
import type { GetTodosResponse, TodoGroup } from "../types/todo";
export function groupTodos(todos: GetTodosResponse): TodoGroup[] {
// ... 그룹화 로직 ...
return Object.values(grouped);
}| <div className="px-4 mt-4"> | ||
| <TodoList /> | ||
| {/* Pass fetched groupedTodos to TodoList */} | ||
| <TodoList isLeader={false} studyId={tempStudyId} todos={groupedTodos} /> |
There was a problem hiding this comment.
TodoList 컴포넌트에 isLeader prop을 false로 하드코딩하여 전달하고 있습니다. 홈페이지는 여러 스터디의 To-Do를 보여줄 수 있으므로, 각 To-Do 그룹(스터디)별로 사용자가 리더인지 여부를 동적으로 판단하여 전달해야 합니다.
API 응답에 스터디별 리더 여부 정보가 포함되도록 백엔드와 협의하거나, To-Do 데이터를 처리할 때 별도의 사용자 정보와 비교하여 리더 여부를 판별하는 로직이 필요합니다.
| <UpcomingScheduleItem key={item.id} item={{ | ||
| ...item, | ||
| scheduleName: item.title, // Map title to scheduleName | ||
| dateTime: item.scheduleTime || "", // Map scheduleTime to dateTime, handle undefined | ||
| }} |
There was a problem hiding this comment.
UpcomingScheduleItem 컴포넌트에 props를 전달하기 위해 부모 컴포넌트에서 scheduleName: item.title과 같이 객체를 새로 생성하여 매핑하고 있습니다. 이는 데이터 구조가 변경될 때마다 여러 곳을 수정해야 할 수 있습니다. 하위 컴포넌트인 UpcomingScheduleItem이 API 응답 타입인 ScheduleItem을 직접 받도록 수정하면, 이러한 중간 매핑 과정 없이 코드를 더 간결하고 유지보수하기 쉽게 만들 수 있습니다. GroupHomePage.tsx의 TodoList와 TodoItem 관계에서도 비슷한 개선을 할 수 있습니다.
개선 제안: UpcomingScheduleItem.tsx의 props 타입을 item: { scheduleName: string; ... } 대신 item: ScheduleItem으로 변경하고, 내부에서 item.title, item.scheduleTime 등을 직접 사용하도록 리팩토링하는 것을 권장합니다.
| try { | ||
| await createSchedule(studyId, requestData); | ||
| setShowModal(true); | ||
| } catch (err: any) { |
There was a problem hiding this comment.
catch 블록에서 에러 객체의 타입을 any로 지정하고 있습니다. unknown 타입을 사용하고 타입 가드를 통해 에러 객체의 구조를 확인하면 더 안전하게 에러를 처리할 수 있습니다. 이 방식은 CreateToDoPage.tsx (line 125) 등 다른 파일에서도 동일하게 적용할 수 있습니다.
| } catch (err: any) { | |
| } catch (err: unknown) { |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (13)
src/pages/MemberPage.tsx (1)
9-26: Consider unifying layout & header placement between normal and error states.Logic for pulling
membersfromlocation.stateand sorting leaders to the top looks fine. The only thing to consider is that the error branch (!members || members.length === 0) returns a different layout (header inside a centered flex container, no scroll area) than the success path, which always renders the header at the top with a scrollable body.To keep UX consistent and avoid duplicating the
<Header>markup, you could always render the same outer layout and conditionally render either the error message or the member list inside the content area.Also applies to: 28-33
src/components/CreateScheduleToDo/DeadlineSelector.tsx (2)
22-33: Consider consolidating repetitive handlers.The four nearly identical change handlers could be consolidated into a single generic handler or removed entirely by passing setters directly to
onChange.- const handleMonthChange = (e: React.ChangeEvent<HTMLInputElement>) => { - setMonth(e.target.value); - }; - const handleDayChange = (e: React.ChangeEvent<HTMLInputElement>) => { - setDay(e.target.value); - }; - const handleHourChange = (e: React.ChangeEvent<HTMLInputElement>) => { - setHour(e.target.value); - }; - const handleMinuteChange = (e: React.ChangeEvent<HTMLInputElement>) => { - setMinute(e.target.value); - };Then update usages to inline the setter:
<NumberInputBox value={month} onChange={(e) => setMonth(e.target.value)} />
4-13: No input validation for date/time values.The component accepts any string input without validating ranges (month 1-12, day 1-31, hour 0-23, minute 0-59). Invalid values could be submitted to the API.
Consider adding validation either in this component or in the parent form before submission. At minimum, the API call should validate input ranges.
src/types/todo.ts (2)
21-25: Legacy compatibility fields are unused and can be removed.Based on the
TodoListcomponent changes, the mapping from API fields to component props happens at the component level (e.g.,todo.name→taskName). These optional legacy fields (taskName,completedCount,totalCount,isChecked) appear to be dead code.totalParticipants: number; - // 기존 ToDoItem에서 사용되던 필드 중 필요한 것들을 유지 - taskName?: string; // 기존 TodoItem과 호환성을 위해 유지 - completedCount?: number; // 기존 TodoItem과 호환성을 위해 유지 - totalCount?: number; // 기존 TodoItem과 호환성을 위해 유지 - isChecked?: boolean; // 기존 TodoItem과 호환성을 위해 유지 }
45-57:GetTodoItemResponselargely duplicatesTodointerface.Consider extending
Todoto reduce duplication:-export interface GetTodoItemResponse { - id: number; - name: string; - description?: string; - dueDate: string; - certificationMethods: ("TEXT_NOTE" | "FILE_UPLOAD")[]; - isCompleted: boolean; - completedParticipants: number; - totalParticipants: number; - groupId: number; - groupName: string; -} +export interface GetTodoItemResponse extends Todo { + groupId: number; + groupName: string; +}src/pages/CreateSchedulePage.tsx (1)
26-31: Component may briefly render before redirect on invalid access.The
useEffectruns after the initial render, so the form may briefly appear before the redirect occurs whenstudyIdis missing. Consider an early return pattern.+ if (!studyId) { + return null; // Or a loading spinner + } + return ( <div className="flex flex-col flex-1 min-h-screen bg-white">Keep the
useEffectfor the alert and navigation, but add an early return to prevent rendering the form.src/pages/GroupHomePage.tsx (3)
30-30: Unused state variabletodos.The
todosstate (raw fetched todos) is set but never read. OnlygroupedTodosis used in the UI. Consider removing the unused state to reduce memory overhead and simplify the component.- const [todos, setTodos] = useState<GetTodosResponse>([]);And remove
setTodos(fetchedTodos);on line 98.
100-126: Duplicated todo grouping logic and redundant property mapping.
- This grouping logic is duplicated in
HomePage.tsx(lines 38-62). Extract to a shared utility:// src/utils/todoUtils.ts export function groupTodosByGroupName(todos: GetTodosResponse): TodoGroup[] { const grouped: { [key: string]: TodoGroup } = {}; todos.forEach(todoItem => { if (!grouped[todoItem.groupName]) { grouped[todoItem.groupName] = { id: todoItem.groupId, groupName: todoItem.groupName, todos: [], }; } grouped[todoItem.groupName].todos.push({ id: todoItem.id, name: todoItem.name, // ... rest of properties }); }); return Object.values(grouped); }
- Lines 119-123 map legacy property names (
taskName,completedCount, etc.) that duplicate existing fields. IfTodoListhas been updated to use the new API properties (as shown in the relevant snippet), these legacy mappings can be removed.
193-195: Type safety:studyIdmay be inferred asundefined.Although
studyIdis validated and redirects on line 44, TypeScript still sees it asstring | undefined. Consider adding a non-null assertion or early return guard to satisfy type checking:+ // After the redirect check, studyId is guaranteed to be defined + const safeStudyId = studyId!; // or use type narrowing + // ... later in JSX - <UpcomingSchedule isLeader={isLeader} studyId={studyId} schedules={schedules} /> + <UpcomingSchedule isLeader={isLeader} studyId={safeStudyId} schedules={schedules} />Alternatively, move the rendering logic inside a guard that narrows the type.
src/components/Calendar/CalendarModal.tsx (1)
22-24: Unnecessary wrapper function.
handleClosesimply delegates toonClosewith no additional logic. You can useonClosedirectly:- const handleClose = () => { - onClose(); - }; // ... - onClick={handleClose} + onClick={onClose}src/pages/HomePage.tsx (3)
27-28: Add a TODO comment for the hardcoded studyId.The temporary hardcoded
studyIdshould be tracked for replacement. Consider adding an explicit TODO:- // Temporary hardcoded studyId for HomePage - const tempStudyId = "mock-study-id-for-homepage"; // Use a UUID here in a real app + // TODO: Replace with actual studyId from user context or route state + const tempStudyId = "mock-study-id-for-homepage";Would you like me to open an issue to track replacing this with the actual user's study ID from authentication context?
13-13: Unused state variabletodos.Same as in
GroupHomePage.tsx: thetodosstate is set but never read. OnlygroupedTodosis consumed. Remove to reduce complexity.
114-120: Only first error is displayed when multiple requests fail.If both todo and calendar fetches fail, only
todosErroris shown due to short-circuit evaluation. Consider showing both errors:- <p className="text-red-500 text-center px-4">{todosError || calendarError}</p> + <p className="text-red-500 text-center px-4"> + {[todosError, calendarError].filter(Boolean).join(' ')} + </p>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/components/Calendar/Calendar.tsx(1 hunks)src/components/Calendar/CalendarModal.tsx(2 hunks)src/components/CreateScheduleToDo/DeadlineSelector.tsx(1 hunks)src/components/MemberHeader.tsx(2 hunks)src/components/TodoList.tsx(1 hunks)src/components/UpcomingSchedule.tsx(1 hunks)src/pages/CreateSchedulePage.tsx(1 hunks)src/pages/CreateToDoPage.tsx(3 hunks)src/pages/GroupHomePage.tsx(4 hunks)src/pages/HomePage.tsx(1 hunks)src/pages/MemberPage.tsx(1 hunks)src/services/calendar.service.ts(1 hunks)src/services/schedule.service.ts(1 hunks)src/services/todo.service.ts(1 hunks)src/types/calender.ts(1 hunks)src/types/schedule.ts(2 hunks)src/types/todo.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
src/services/schedule.service.ts (1)
src/types/schedule.ts (3)
CreateScheduleRequest(45-49)CreateScheduleResponse(51-53)GetSchedulesResponse(9-9)
src/components/UpcomingSchedule.tsx (2)
src/types/schedule.ts (1)
ScheduleItem(1-7)src/components/UpcomingScheduleItem.tsx (1)
UpcomingScheduleItem(8-21)
src/services/todo.service.ts (1)
src/types/todo.ts (3)
CreateTodoRequest(34-39)CreateTodoResponse(41-43)GetTodosResponse(59-59)
src/components/CreateScheduleToDo/DeadlineSelector.tsx (1)
src/components/CreateScheduleToDo/NumberInputBox.tsx (1)
NumberInputBox(1-11)
src/pages/CreateSchedulePage.tsx (6)
src/types/schedule.ts (1)
CreateScheduleRequest(45-49)src/services/schedule.service.ts (1)
createSchedule(5-14)src/components/Header.tsx (1)
Header(11-34)src/components/CreateScheduleToDo/LabeledInput.tsx (1)
LabeledInput(8-22)src/components/CreateScheduleToDo/NumberInputBox.tsx (1)
NumberInputBox(1-11)src/components/ActionButton.tsx (1)
ActionButton(8-32)
src/pages/MemberPage.tsx (2)
src/types/member.ts (1)
Member(1-6)src/components/Header.tsx (1)
Header(11-34)
src/components/TodoList.tsx (2)
src/types/todo.ts (1)
TodoGroup(28-32)src/components/ToDoItem.tsx (1)
TodoItem(13-31)
src/pages/HomePage.tsx (6)
src/types/todo.ts (2)
GetTodosResponse(59-59)TodoGroup(28-32)src/types/calender.ts (1)
ScheduleForCalendar(3-6)src/services/todo.service.ts (1)
getTodosByStudy(16-19)src/services/calendar.service.ts (1)
getCalendarSummary(5-10)src/components/Calendar/Calendar.tsx (1)
Calendar(21-113)src/components/TodoList.tsx (1)
TodoList(14-63)
src/components/Calendar/CalendarModal.tsx (1)
src/types/calender.ts (1)
DailySchedule(19-24)
src/services/calendar.service.ts (1)
src/types/calender.ts (2)
GetCalendarSummaryResponse(13-17)GetDailySchedulesResponse(26-29)
src/components/Calendar/Calendar.tsx (2)
src/types/calender.ts (2)
ScheduleForCalendar(3-6)DailySchedule(19-24)src/services/calendar.service.ts (1)
getDailySchedules(12-17)
src/pages/GroupHomePage.tsx (4)
src/types/todo.ts (2)
GetTodosResponse(59-59)TodoGroup(28-32)src/types/schedule.ts (1)
GetSchedulesResponse(9-9)src/services/todo.service.ts (1)
getTodosByStudy(16-19)src/services/schedule.service.ts (1)
getSchedulesByStudy(16-19)
🔇 Additional comments (11)
src/services/schedule.service.ts (1)
5-18: Schedule service wrappers look consistent and minimal.
createScheduleandgetSchedulesByStudycorrectly wrap the/studies/{studyId}/schedulesendpoints and return typedresponse.data, matching the todo service pattern. No issues from my side here.src/services/calendar.service.ts (1)
5-16: Calendar service matches the new API contract.Both
getCalendarSummaryandgetDailySchedulesare thin, typed wrappers around the/calendarendpoints withstudyIdinparams. This is clean and consistent with the rest of the service layer.src/services/todo.service.ts (1)
5-18: Todo service wrappers are consistent and correct.
createTodoandgetTodosByStudycorrectly wrap the/studies/{studyId}/todosendpoints and return the typedresponse.data. Implementation is consistent with the other service modules.src/pages/CreateToDoPage.tsx (1)
18-57: Overall create‑To‑Do flow is well structured oncedueDateis corrected.
- Guarding on missing
studyIdwith a redirect, plus the extra runtime check inhandleCreateSchedule, is sensible.- Loading and error states are wired correctly into the button, message, and modal.
selectedMethods→certificationTypesandmembers→participantIdsmapping is straightforward and readable.- Controlled deadline parts passed into
DeadlineSelectorkeep the component tree nicely prop‑driven.After fixing the
dueDateformatting bug above, the rest of this flow looks solid to ship.Also applies to: 90-131, 138-167
src/types/calender.ts (1)
8-29: Calendar types align with the new service and UI usage.
CalendarDaySummary,GetCalendarSummaryResponse,DailySchedule, andGetDailySchedulesResponseprovide clear, focused shapes for calendar summary and daily schedules, and they line up with the calendar service signatures. No issues from a typing perspective.src/components/MemberHeader.tsx (1)
8-9: MemberHeader props and callback pattern look good.Adding
onViewAllClickand delegating navigation to the parent simplifies this component and makes it more reusable. The rest of the rendering stays unchanged, so behavior should remain consistent.Also applies to: 11-16, 37-39
src/components/TodoList.tsx (1)
14-31: LGTM on the prop-driven refactor and empty state handling.The component correctly handles the empty todos case with appropriate messaging and conditionally renders the create button based on
isLeader. The navigation state properly passesstudyId.src/pages/CreateSchedulePage.tsx (1)
66-74: LGTM on async error handling and loading state.The try/catch properly handles API errors, displays user-friendly messages, and manages loading state correctly. The
finallyblock ensures loading is always reset.src/pages/GroupHomePage.tsx (1)
160-163: LGTM - parallel data fetching.The three async functions are invoked concurrently without awaiting each other, which is the correct approach for independent data fetches. The combined loading condition on line 165 properly gates the UI until all data is ready.
src/components/Calendar/CalendarModal.tsx (1)
39-54: LGTM - schedule rendering with graceful time extraction.The time extraction regex handles the documented
scheduleTimeformat correctly, and the fallback to empty string is appropriate for malformed data. Usingtitle-indexas a composite key is acceptable given schedules lack unique IDs.src/components/Calendar/Calendar.tsx (1)
55-79: LGTM - async date selection with proper loading states.The implementation correctly:
- Opens the modal immediately for responsive UX
- Shows loading state while fetching
- Handles errors gracefully with user-friendly messages
- Limits displayed schedules to 3 as per business logic
| setCurrentMonth: (month: number) => void; | ||
| setCurrentYear: (year: number) => void; |
There was a problem hiding this comment.
Type mismatch: setter props don't match updater function usage.
The prop types define simple value setters:
setCurrentMonth: (month: number) => void;
setCurrentYear: (year: number) => void;But handlePrev/handleNext (lines 38, 41, 47, 51) call them with updater functions:
setCurrentYear((y) => y - 1); // Passes a function, not a numberUpdate the types to support both patterns using React's Dispatch<SetStateAction<number>>:
+import type { Dispatch, SetStateAction } from "react";
+
interface CalendarProps {
schedulesByDate: Record<string, ScheduleForCalendar[]>;
onSelect: (date: Date) => void;
currentYear: number;
currentMonth: number;
- setCurrentMonth: (month: number) => void;
- setCurrentYear: (year: number) => void;
+ setCurrentMonth: Dispatch<SetStateAction<number>>;
+ setCurrentYear: Dispatch<SetStateAction<number>>;
studyId: string;
}🤖 Prompt for AI Agents
In src/components/Calendar/Calendar.tsx around lines 16-17, the prop types
declare setCurrentMonth and setCurrentYear as functions accepting a number, but
the component calls them with updater functions; change their types to accept
both a value or updater by using React.Dispatch<React.SetStateAction<number>>
(or import Dispatch and SetStateAction from React and use
Dispatch<SetStateAction<number>>), and update any imports accordingly so the
props type matches usage with functional updaters.
| setDailySchedulesLoading(true); | ||
| setDailySchedulesError(null); | ||
| try { | ||
| const formattedDate = date.toISOString().split("T")[0]; // YYYY-MM-DD |
There was a problem hiding this comment.
Potential timezone issue with date formatting.
toISOString() converts to UTC, which can cause date shifts for users in positive UTC offsets near midnight. For example, selecting January 2nd at 00:30 in JST (UTC+9) produces "2024-01-01".
Consider using local date formatting:
- const formattedDate = date.toISOString().split("T")[0];
+ const formattedDate = `${date.getFullYear()}-${String(date.getMonth() + 1).padStart(2, '0')}-${String(date.getDate()).padStart(2, '0')}`;🤖 Prompt for AI Agents
In src/components/Calendar/Calendar.tsx around line 64, the code uses
date.toISOString().split("T")[0] which converts the date to UTC and can shift
the day for users in positive timezones; replace this with local-date
formatting—either use date.toLocaleDateString('en-CA') to get a YYYY-MM-DD
string in the user's local timezone or construct the string from
date.getFullYear(), (date.getMonth()+1) and date.getDate() with zero-padding to
ensure two-digit month/day.
| {group.todos.map((todo, index) => ( | ||
| <TodoItem | ||
| key={todo.id} | ||
| taskName={todo.taskName} | ||
| completedCount={todo.completedCount} | ||
| totalCount={todo.totalCount} | ||
| isChecked={todo.isChecked} | ||
| taskName={todo.name} // Use todo.name from API | ||
| completedCount={todo.completedParticipants} // Use todo.completedParticipants | ||
| totalCount={todo.totalParticipants} // Use todo.totalParticipants | ||
| isChecked={todo.isCompleted} // Use todo.isCompleted | ||
| isLast={index === group.todos.length - 1} | ||
| path={`/todo/${todo.id}`} | ||
| /> |
There was a problem hiding this comment.
Potential division by zero when totalParticipants is 0.
The TodoItem component computes progress = (completedCount / totalCount) * 100. If todo.totalParticipants is 0, this results in NaN being displayed.
Consider handling this edge case either here or in TodoItem:
<TodoItem
key={todo.id}
taskName={todo.name}
completedCount={todo.completedParticipants}
- totalCount={todo.totalParticipants}
+ totalCount={todo.totalParticipants || 1}
isChecked={todo.isCompleted}
isLast={index === group.todos.length - 1}
path={`/todo/${todo.id}`}
/>Or add validation in TodoItem to handle zero denominators.
🤖 Prompt for AI Agents
In src/components/TodoList.tsx around lines 48-57, the code passes
todo.totalParticipants directly which can be 0 and cause a division-by-zero/NaN
in TodoItem; fix by ensuring the values passed are safe: when
todo.totalParticipants is 0, pass a safe non-zero total (e.g. Math.max(1,
todo.totalParticipants)) and clamp completedCount to at most that total (e.g.
Math.min(todo.completedParticipants, safeTotal)); alternatively, move the guard
into TodoItem and compute progress there as totalCount > 0 ? (completedCount /
totalCount) * 100 : 0 so progress is 0 when denominator is zero.
| interface UpcomingScheduleProps { | ||
| isLeader: boolean; | ||
| studyId?: string; | ||
| schedules: ScheduleItem[]; // Accept schedules as prop | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the ScheduleItem type definition
find . -name "*.ts" -o -name "*.tsx" | xargs grep -l "type ScheduleItem\|interface ScheduleItem" | head -5Repository: Studiz-dev/studiz-FE
Length of output: 87
🏁 Script executed:
# Also search for schedule.ts in common type locations
fd -type f "schedule.ts" --exclude node_modulesRepository: Studiz-dev/studiz-FE
Length of output: 234
🏁 Script executed:
# Examine the UpcomingSchedule.tsx file to see the current mapping
cat -n src/components/UpcomingSchedule.tsxRepository: Studiz-dev/studiz-FE
Length of output: 2815
🏁 Script executed:
# Search for UpcomingScheduleItem to understand its prop requirements
find . -name "*.ts" -o -name "*.tsx" | xargs grep -l "UpcomingScheduleItem" | head -10Repository: Studiz-dev/studiz-FE
Length of output: 143
🏁 Script executed:
cat -n src/types/schedule.tsRepository: Studiz-dev/studiz-FE
Length of output: 1917
🏁 Script executed:
cat -n src/components/UpcomingScheduleItem.tsxRepository: Studiz-dev/studiz-FE
Length of output: 1165
🏁 Script executed:
# Check if there's any type casting or interface extension happening
rg -A 5 "interface UpcomingScheduleItemProps" src/components/UpcomingScheduleItem.tsxRepository: Studiz-dev/studiz-FE
Length of output: 241
🏁 Script executed:
# Check if UpcomingScheduleItem actually expects different fields
rg "scheduleName|dateTime|dDay" src/components/UpcomingScheduleItem.tsxRepository: Studiz-dev/studiz-FE
Length of output: 326
🏁 Script executed:
# Verify the exact TypeScript error by checking if the current mapping would even compile
# Search for any type assertions or as any
rg "as any|as ScheduleItem" src/components/UpcomingSchedule.tsxRepository: Studiz-dev/studiz-FE
Length of output: 46
🏁 Script executed:
# Double-check the current UpcomingSchedule mapping - does the spread actually override the fields?
sed -n '42,50p' src/components/UpcomingSchedule.tsxRepository: Studiz-dev/studiz-FE
Length of output: 480
🏁 Script executed:
# Check if TypeScript would catch this - look for tsconfig and strict mode
cat tsconfig.json | grep -A 5 -B 5 "strict"Repository: Studiz-dev/studiz-FE
Length of output: 46
Fix the D‑day field mapping: item.dDay will be undefined because ScheduleItem exposes dday, not dDay.
The refactor to accept schedules: ScheduleItem[] and map title → scheduleName and scheduleTime → dateTime looks good, but the D-day field is not being mapped correctly:
ScheduleItemtype (line 6 ofsrc/types/schedule.ts) definesdday?: number(lowercase)UpcomingScheduleItemaccessesitem.dDay(camelCase) at line 17 to render the D-DAY badge- The current spread operation doesn't rename
ddaytodDay, so the component seesundefinedand renders"D-undefined"
Explicitly map the dday field to dDay:
{schedules.map((item, index) => (
<UpcomingScheduleItem
key={item.id}
item={{
...item,
scheduleName: item.title,
dateTime: item.scheduleTime || "",
dDay: item.dday ?? 0,
}}
isLast={index === schedules.length - 1}
/>
))}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/UpcomingSchedule.tsx around lines 7 to 11, the mapped schedule
objects leave ScheduleItem.dday unmapped so the component reads item.dDay as
undefined; update the mapping that converts ScheduleItem → UpcomingScheduleItem
to explicitly set dDay: item.dday (or 0/undefined fallback if needed) when
spreading other renamed fields (title → scheduleName, scheduleTime → dateTime)
so the D-day badge renders the correct value.
| const formatStartDate = (): string | null => { | ||
| if (!month || !day) return null; | ||
| const year = new Date().getFullYear(); | ||
| const formattedMonth = String(month).padStart(2, '0'); | ||
| const formattedDay = String(day).padStart(2, '0'); | ||
|
|
||
| const date = new Date(`${year}-${formattedMonth}-${formattedDay}`); | ||
| if (isNaN(date.getTime())) return null; | ||
|
|
||
| const isDisabled = scheduleName.trim() === ""; | ||
| return `${year}-${formattedMonth}-${formattedDay}`; | ||
| } |
There was a problem hiding this comment.
Year-end edge case: always uses current year.
formatStartDate always uses new Date().getFullYear(). If a user in December selects a January date, the resulting date will be in the past (current year's January) rather than next year.
Consider adding year selection or inferring the year based on whether the selected date has already passed:
const formatStartDate = (): string | null => {
if (!month || !day) return null;
- const year = new Date().getFullYear();
+ const today = new Date();
+ let year = today.getFullYear();
const formattedMonth = String(month).padStart(2, '0');
const formattedDay = String(day).padStart(2, '0');
const date = new Date(`${year}-${formattedMonth}-${formattedDay}`);
if (isNaN(date.getTime())) return null;
+
+ // If date is in the past, assume next year
+ if (date < today) {
+ year += 1;
+ }
- return `${year}-${formattedMonth}-${formattedDay}`;
+ return `${year}-${formattedMonth}-${formattedDay}`;
}📝 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 formatStartDate = (): string | null => { | |
| if (!month || !day) return null; | |
| const year = new Date().getFullYear(); | |
| const formattedMonth = String(month).padStart(2, '0'); | |
| const formattedDay = String(day).padStart(2, '0'); | |
| const date = new Date(`${year}-${formattedMonth}-${formattedDay}`); | |
| if (isNaN(date.getTime())) return null; | |
| const isDisabled = scheduleName.trim() === ""; | |
| return `${year}-${formattedMonth}-${formattedDay}`; | |
| } | |
| const formatStartDate = (): string | null => { | |
| if (!month || !day) return null; | |
| const today = new Date(); | |
| let year = today.getFullYear(); | |
| const formattedMonth = String(month).padStart(2, '0'); | |
| const formattedDay = String(day).padStart(2, '0'); | |
| const date = new Date(`${year}-${formattedMonth}-${formattedDay}`); | |
| if (isNaN(date.getTime())) return null; | |
| // If date is in the past, assume next year | |
| if (date < today) { | |
| year += 1; | |
| } | |
| return `${year}-${formattedMonth}-${formattedDay}`; | |
| } |
|
|
||
| return ( | ||
| <div className="flex flex-col flex-1 min-h-screen bg-white"> | ||
| <Header title="일정 생성하기" backPath="/GroupHome" /> |
There was a problem hiding this comment.
Back navigation loses studyId context.
The Header component's backPath="/GroupHome" navigates without passing studyId in state, while handleConfirm correctly passes it via navigate("/GroupHome", { state: { studyId } }). This inconsistency may cause issues on the destination page.
Consider using navigate(-1) or passing state consistently. You may need to update the Header component to accept navigation state, or handle the back navigation manually:
- <Header title="일정 생성하기" backPath="/GroupHome" />
+ <Header title="일정 생성하기" showBack={true} />Then ensure the default navigate(-1) behavior in Header is used, which preserves navigation history.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/CreateSchedulePage.tsx around line 84, the Header uses
backPath="/GroupHome" which navigates without preserving studyId state; change
the back navigation to preserve studyId by either (A) replacing the fixed
backPath with a back handler that calls navigate(-1) so history (and any state)
is preserved, or (B) pass the studyId explicitly when navigating back
(navigate("/GroupHome", { state: { studyId } })); if Header does not support a
custom back handler or back state yet, update the Header component to accept an
onBack callback or a backState prop and use that to perform the appropriate
navigate call that includes studyId.
| // Helper function to format date | ||
| const formatDateTime = (): string | null => { | ||
| if (!month || !day || !hour || !minute) return null; | ||
|
|
||
| const currentYear = new Date().getFullYear(); | ||
| // Pad single digits with leading zero | ||
| const formattedMonth = String(month).padStart(2, '0'); | ||
| const formattedDay = String(day).padStart(2, '0'); | ||
| const formattedHour = String(hour).padStart(2, '0'); | ||
| const formattedMinute = String(minute).padStart(2, '0'); | ||
|
|
||
| // Basic validation for date parts (can be more robust) | ||
| if ( | ||
| isNaN(currentYear) || isNaN(parseInt(formattedMonth)) || isNaN(parseInt(formattedDay)) || | ||
| isNaN(parseInt(formattedHour)) || isNaN(parseInt(formattedMinute)) | ||
| ) { | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| const date = new Date(`${currentYear}-${formattedMonth}-${formattedDay}T${formattedHour}:${formattedMinute}:00`); | ||
| if (isNaN(date.getTime())) { | ||
| return null; // Invalid date | ||
| } | ||
| return date.toISOString().slice(0, 19); // Format to YYYY-MM-DDTHH:mm | ||
| } catch (e) { | ||
| console.error("Date formatting error:", e); | ||
| return null; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Fix dueDate construction: current logic shifts timezone and produces an invalid format.
Right now:
formatDateTimebuilds aDatefrom local components, then usesdate.toISOString().slice(0, 19). This converts the local time to UTC before formatting.- In
handleCreateSchedule, you then dodueDate: dueDate + ":00", so the final string becomesYYYY-MM-DDTHH:mm:ss:00, which is not a valid ISO 8601 timestamp. - Combined, this will both shift the time (e.g. KST → UTC) and produce an incorrectly formatted string for the API.
Given CreateTodoRequest.dueDate is documented as "YYYY-MM-DDTHH:mm:ss", you can:
- Keep using
Datepurely for validation. - Return a manually built local string in the final format, and stop appending
":00"later.
For example:
- // Helper function to format date
- const formatDateTime = (): string | null => {
+ // Helper function to format dueDate as "YYYY-MM-DDTHH:mm:ss" (local time)
+ const formatDateTime = (): string | null => {
@@
- try {
- const date = new Date(`${currentYear}-${formattedMonth}-${formattedDay}T${formattedHour}:${formattedMinute}:00`);
- if (isNaN(date.getTime())) {
- return null; // Invalid date
- }
- return date.toISOString().slice(0, 19); // Format to YYYY-MM-DDTHH:mm
- } catch (e) {
- console.error("Date formatting error:", e);
- return null;
- }
+ const dateString = `${currentYear}-${formattedMonth}-${formattedDay}T${formattedHour}:${formattedMinute}:00`;
+ const date = new Date(dateString);
+
+ if (isNaN(date.getTime())) {
+ return null; // Invalid date
+ }
+
+ // Return local date-time string without timezone conversion
+ return dateString;
};
@@
- const dueDate = formatDateTime();
+ const dueDate = formatDateTime();
@@
- const requestData: CreateTodoRequest = {
- name: scheduleName,
- dueDate: dueDate + ":00", // Append seconds to match YYYY-MM-DDTHH:mm:ss format
+ const requestData: CreateTodoRequest = {
+ name: scheduleName,
+ dueDate: dueDate, // Already "YYYY-MM-DDTHH:mm:ss"
certificationTypes: certificationTypes,
participantIds: selectedParticipantIds,
};This keeps your validation, avoids timezone drift, and ensures the final dueDate matches the backend’s expected format.
Also applies to: 92-120
🤖 Prompt for AI Agents
In src/pages/CreateToDoPage.tsx around lines 59-88 (and also apply same fix to
92-120), formatDateTime currently creates a Date and calls toISOString() causing
a timezone shift and later code appends ":00" resulting in an invalid
"YYYY-MM-DDTHH:mm:ss:00" string; instead, keep using Date only to validate the
components, then build and return a local datetime string in the exact
"YYYY-MM-DDTHH:mm:ss" format (pad month/day/hour/minute and append ":00" exactly
once), and remove the extra ":00" append in handleCreateSchedule so the API
receives a correctly formatted local timestamp.
| const [schedulesError, setSchedulesError] = useState<string | null>(null); // Error state for schedules | ||
|
|
||
|
|
||
| const isLeader = members.some(member => member.isLeader); |
There was a problem hiding this comment.
Incorrect leader check logic.
members.some(member => member.isLeader) returns true if any member is a leader, not if the current user is the leader. This will likely always be true since every study should have at least one leader/owner.
You need to compare against the current authenticated user's ID:
- const isLeader = members.some(member => member.isLeader);
+ // Replace `currentUserId` with actual user context/auth state
+ const isLeader = members.some(member => member.isLeader && member.id === currentUserId);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/GroupHomePage.tsx around line 40, the leader check uses
members.some(member => member.isLeader) which only detects if any member is a
leader rather than whether the current authenticated user is the leader; change
the logic to compare the member's id to the current authenticated user's id and
check isLeader (e.g., find or some where member.id === currentUserId &&
member.isLeader), making sure to use the actual current user id variable from
your auth/context and handle the case where current user may be undefined.
| useEffect(() => { | ||
| const fetchTodos = async () => { | ||
| setTodosLoading(true); | ||
| setTodosError(null); | ||
| try { | ||
| const fetchedTodos = await getTodosByStudy(tempStudyId); | ||
| setTodos(fetchedTodos); | ||
|
|
||
| // Group fetchedTodos by groupName, similar to GroupHomePage | ||
| const grouped: { [key: string]: TodoGroup } = {}; | ||
| fetchedTodos.forEach(todoItem => { | ||
| if (!grouped[todoItem.groupName]) { | ||
| grouped[todoItem.groupName] = { | ||
| id: todoItem.groupId, | ||
| groupName: todoItem.groupName, | ||
| todos: [], | ||
| }; | ||
| } | ||
| grouped[todoItem.groupName].todos.push({ | ||
| id: todoItem.id, | ||
| name: todoItem.name, | ||
| description: todoItem.description, | ||
| dueDate: todoItem.dueDate, | ||
| certificationMethods: todoItem.certificationMethods, | ||
| isCompleted: todoItem.isCompleted, | ||
| completedParticipants: todoItem.completedParticipants, | ||
| totalParticipants: todoItem.totalParticipants, | ||
| taskName: todoItem.name, | ||
| completedCount: todoItem.completedParticipants, | ||
| totalCount: todoItem.totalParticipants, | ||
| isChecked: todoItem.isCompleted, | ||
| }); | ||
| }); | ||
| setGroupedTodos(Object.values(grouped)); | ||
|
|
||
| } catch (error) { | ||
| console.error("HomePage To-Do 목록 조회 실패:", error); | ||
| if (error instanceof AxiosError) { | ||
| setTodosError(error.response?.data?.message || "To-Do 목록을 불러오는 중 오류가 발생했습니다."); | ||
| } else { | ||
| setTodosError("To-Do 목록을 불러오는 중 알 수 없는 오류가 발생했습니다."); | ||
| } | ||
| } finally { | ||
| setTodosLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| const fetchCalendarData = async () => { | ||
| setCalendarLoading(true); | ||
| setCalendarError(null); | ||
| try { | ||
| const calendarSummary = await getCalendarSummary(tempStudyId, currentYear, currentMonth + 1); // API expects 1-indexed month | ||
| const transformedSchedules: Record<string, ScheduleForCalendar[]> = {}; | ||
| calendarSummary.days.forEach(daySummary => { | ||
| transformedSchedules[daySummary.date] = daySummary.scheduleTitles.map((title, index) => ({ | ||
| id: `${daySummary.date}-${index}`, // Generate synthetic ID for key prop | ||
| title: title | ||
| })); | ||
| }); | ||
| setCalendarSchedulesByDate(transformedSchedules); | ||
| } catch (error) { | ||
| console.error("HomePage 달력 데이터 조회 실패:", error); | ||
| if (error instanceof AxiosError) { | ||
| setCalendarError(error.response?.data?.message || "달력 데이터를 불러오는 중 오류가 발생했습니다."); | ||
| } else { | ||
| setCalendarError("달력 데이터를 불러오는 중 알 수 없는 오류가 발생했습니다."); | ||
| } | ||
| } finally { | ||
| setCalendarLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| fetchTodos(); | ||
| fetchCalendarData(); | ||
| }, [currentYear, currentMonth]); // Depend on currentYear and currentMonth for calendar data |
There was a problem hiding this comment.
Unnecessary todo refetch on calendar navigation.
fetchTodos doesn't depend on currentYear or currentMonth, yet it's called inside an effect that triggers on month changes. This causes redundant API calls when navigating the calendar.
Split into two separate effects:
+ // Fetch todos once on mount (doesn't depend on calendar month)
+ useEffect(() => {
+ const fetchTodos = async () => {
+ // ... todo fetching logic
+ };
+ fetchTodos();
+ }, []); // Empty dependency - fetch once
+
+ // Fetch calendar data when year/month changes
useEffect(() => {
- const fetchTodos = async () => { ... };
const fetchCalendarData = async () => { ... };
- fetchTodos();
fetchCalendarData();
}, [currentYear, currentMonth]);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/HomePage.tsx around lines 30 to 104, the current useEffect triggers
both fetchTodos and fetchCalendarData when currentYear/currentMonth change,
causing unnecessary todo refetches; split this into two effects: one effect that
runs fetchTodos and depends only on tempStudyId (and any other todo-related
state) and another effect that runs fetchCalendarData and depends on
currentYear, currentMonth, and tempStudyId (and any calendar-related state);
move each async function into its respective effect (or extract helpers) and
ensure loading/error state handling remains inside each effect so the two
concerns are separated and calendar navigation no longer refetches todos.
| id: string; // API 응답에 맞춰 string (UUID) | ||
| title: string; // 일정 이름 | ||
| location?: string; // 장소 (선택사항) | ||
| scheduleTime?: string; // YYYY M DD HH:mm 형식 (확정된 일정만) | ||
| dday?: number; // 오늘 기준 남은 일수 (확정된 일정만) |
There was a problem hiding this comment.
Inconsistent id types between ScheduleItem and CreateScheduleResponse.
ScheduleItem.id is typed as string (UUID), but CreateScheduleResponse.id is typed as number. This inconsistency could cause type mismatches when using the newly created schedule's ID elsewhere in the application.
Consider aligning the types:
export interface CreateScheduleResponse {
- id: number; // The ID of the newly created schedule
+ id: string; // The ID of the newly created schedule (UUID)
}Or verify with the backend API what the actual response type is.
Also applies to: 51-53
🤖 Prompt for AI Agents
In src/types/schedule.ts around lines 2-6 (and also check lines 51-53), the id
types are inconsistent: ScheduleItem.id is declared as string (UUID) while
CreateScheduleResponse.id is declared as number; update the type declarations so
both use the same type that matches the backend (most likely string UUID) —
change CreateScheduleResponse.id (and any other related response types at 51-53)
to string, or if backend returns numeric ids change ScheduleItem.id to number;
ensure all usages and imports reflect the chosen type.
1 participant
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.