-
Notifications
You must be signed in to change notification settings - Fork 1
[FE-Feat] 개인 일정 생성, 조회 #169
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
Conversation
WalkthroughThis pull request delivers a set of updates across the application. A new discussion component, DiscussionRank, with tab state logic is introduced and integrated into the discussion UI. The personal event data model and API for the MyCalendar feature are implemented, including query and mutation hooks with corresponding UI adjustments. Additionally, form handling is refactored via a simplified hook, and utility functions for date formatting and fetching data have been enhanced. Import paths for discussion pages have been reorganized to reflect new file structure conventions. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant DR as DiscussionRank Component
U->>DR: Click on tab (e.g., "빠른 시간 순")
DR->>DR: Call handleChange to update state
DR-->>DR: Re-render with content ("순위")
sequenceDiagram
participant U as User
participant MC as MyCalendar Component
participant Q as usePersonalEventsQuery
participant API as personalEventApi
participant R as request
participant M as usePersonalEventMutation
participant QC as QueryClient
U->>MC: Select date range
MC->>Q: Fetch events with start/end dates
Q->>API: Call getPersonalEvent
API->>R: Execute GET request
R-->>API: Return events data
API-->>Q: Provide event data
Q-->>MC: Render events in CalendarCardList
U->>MC: Open SchedulePopover and edit event
MC->>M: Trigger mutation for new event
M->>API: Call postPersonalEvent
API->>R: Execute POST request
R-->>API: Return created event
API-->>M: Signal mutation success
M->>QC: Invalidate personalEvents query
Possibly Related PRs
Suggested Reviewers
Poem
✨ Finishing Touches
🪧 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 (
|
c2169fa to
e092f60
Compare
e092f60 to
334e51c
Compare
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: 5
🔭 Outside diff range comments (1)
frontend/src/utils/fetch/index.ts (1)
34-61: 🛠️ Refactor suggestionEnhance error handling.
The error handling could be improved with more specific error types and response status handling.
Consider implementing more detailed error handling:
+interface ApiError extends Error { + status?: number; + data?: unknown; +} export const executeFetch = async ( method: Method, endpoint: string, { params, body, options }: FetchRequest = {}, ) => { try { const response = await fetch(fullUrl, { method: method, body: JSON.stringify(body), ...fetchOptions, }); if (!response.ok) { - throw new Error(`HTTP Error: ${response.status}`); + const error = new Error(`HTTP Error: ${response.status}`) as ApiError; + error.status = response.status; + try { + error.data = await response.json(); + } catch { + // Response wasn't JSON + } + throw error; } const data = await response.json(); return data; } catch (error) { - throw new Error(`Network Error : ${error}`); + if (error instanceof Error) { + throw error; + } + throw new Error(`Network Error: ${String(error)}`); } };
🧹 Nitpick comments (15)
frontend/src/utils/date/format.ts (2)
21-21: Add JSDoc documentation for consistency.Consider adding JSDoc documentation similar to formatDateToBarString to maintain consistency and improve code maintainability.
+/** + * Converts a Date object to an ISO 8601 formatted date-time string (YYYY-MM-DDThh:mm:ss). + * @param date - The date object to format + * @returns The formatted string or empty string if date is null + */ export const formatDateToDateTimeString = (date: Date | null): string => {
21-24: LGTM! Well-structured date formatting function.The implementation is solid and follows the established patterns:
- Consistent null handling
- Reuses existing utility functions
- Follows ISO 8601 format suitable for API interactions
Consider making the seconds portion configurable if different precision is needed in the future.
frontend/src/features/discussion/ui/DiscussionRank/index.tsx (2)
6-6: Add type safety for tab values.Consider defining an enum or union type for tab values to prevent potential typos and improve maintainability.
+type TabValue = 'eventsRankedDefault' | 'eventsRankedOfTime'; + const DiscussionRank = () => { - const [tab, setTab] = useState('eventsRankedDefault'); + const [tab, setTab] = useState<TabValue>('eventsRankedDefault');
13-20: Enhance accessibility with ARIA attributes.The Tab component should have proper ARIA attributes for better accessibility.
- <Tab onChange={handleChange} selectedValue={tab}> + <Tab + onChange={handleChange} + selectedValue={tab} + aria-label="이벤트 정렬 옵션" + >frontend/src/utils/date/date.ts (4)
94-99: Add parameter description in JSDoc comment.The JSDoc comment is missing a description of the function's purpose and the expected format of the date parameter.
Add a more descriptive JSDoc comment:
/** - * + * Returns the start and end dates of the week containing the given date. * @param date - 날짜 객체. + * @example + * // Returns { startDate: 2024-02-11, endDate: 2024-02-17 } for date 2024-02-15 * @returns - 특정 날짜가 포함된 주의 첫째 날과 마지막 날의 날짜 객체. */
100-112: Consider reusing existing utility function.The implementation duplicates logic from
formatDateToWeekDates. Consider reusing that function to maintain DRY principles and ensure consistent behavior.Here's a suggested refactor:
export const formatDateToWeekRange = (date: Date): { startDate: Date; endDate: Date; } => { - const selected = new Date(date); - const firstDateOfWeek = new Date(selected); - firstDateOfWeek.setDate(firstDateOfWeek.getDate() - selected.getDay()); - - const lastDateOfWeek = new Date(firstDateOfWeek); - lastDateOfWeek.setDate(firstDateOfWeek.getDate() + 6); - - return { startDate: firstDateOfWeek, endDate: lastDateOfWeek }; + const weekDates = formatDateToWeekDates(date); + return { + startDate: weekDates[0], + endDate: weekDates[6] + }; };
180-181: Consider removing or implementing the TODO comment.The TODO comment about connecting to a holiday API has been left without implementation. Either implement the feature or create an issue to track this enhancement.
Would you like me to help create an issue to track the implementation of the holiday API integration?
193-195: Fix misleading variable name.The variable
isSameYearis used to check if years are different, which is opposite to what the name suggests.Apply this diff to improve clarity:
- const isSameYear = startY !== endY; + const isDifferentYear = startY !== endY; const format = (year: number, month: number, day: number): string => - isSameYear ? `${year}년 ${month + 1}월 ${day}일` : `${month + 1}월 ${day}일`; + isDifferentYear ? `${year}년 ${month + 1}월 ${day}일` : `${month + 1}월 ${day}일`;frontend/src/features/my-calendar/ui/SchedulePopover/index.tsx (2)
17-21: Consider externalizing literal strings for better localization
The default event’s title “제목 없음” might benefit from an i18n approach if this text is user-facing in a multilingual environment.
29-33: Confirm merge order of defaults
You declarestartDateTimeandendDateTimeexplicitly, then spread...defaultEvent. WhileOmitcurrently prevents field collisions, be aware of future expansions where default fields might accidentally override event properties.frontend/src/features/my-calendar/api/queries.ts (1)
7-16: Provide error handling or custom retry logic
Consider adding anonErrorcallback or capturing errors in this query to enhance the user experience. Returning theerrorobject in addition topersonalEventsandisLoadingis also a common pattern.frontend/src/hooks/useFormRef.ts (1)
4-6: Consider using a more specific type instead ofany.While the comment explains the need for flexibility, using
anyreduces type safety. Consider using a union type of expected value types or a generic constraint.-// Form의 value로는 다양한 값이 올 수 있습니다. -// eslint-disable-next-line @typescript-eslint/no-explicit-any -export type FormValues<T> = { [K in keyof T]: any }; +// Form values can be of various types +export type FormValues<T> = { [K in keyof T]: string | number | boolean | Date | null };frontend/src/features/my-calendar/ui/MyCalendar/index.tsx (2)
49-52: Consider adding error handling for the query.The query implementation looks good, but error states aren't handled.
Consider destructuring and handling the error state:
- const { personalEvents, isLoading } = usePersonalEventsQuery({ + const { personalEvents, isLoading, error } = usePersonalEventsQuery({ startDateTime: formatDateToBarString(startDate), endDateTime: formatDateToBarString(endDate), });
58-58: Enhance loading state UI.The loading state could be improved with a more user-friendly loading indicator.
Consider using a loading skeleton or spinner:
- {isLoading ? <div>로딩중...</div> : <CalendarTable personalEvents={personalEvents} />} + {isLoading ? ( + <div role="status" aria-live="polite" className="loading-skeleton"> + <span className="sr-only">로딩중...</span> + </div> + ) : ( + <CalendarTable personalEvents={personalEvents} /> + )}frontend/src/features/my-calendar/ui/SchedulePopover/PopoverForm.tsx (1)
27-36: Consider adding aria-label for accessibility.The toggle component should have an accessible label.
<Toggle inputProps={{ name: 'syncWithGoogleCalendar', checked: valuesRef.current.syncWithGoogleCalendar, onChange: handleChange, + 'aria-label': '구글 캘린더 연동 토글', }} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
frontend/src/features/discussion/ui/DiscussionRank/index.tsx(1 hunks)frontend/src/features/discussion/ui/DiscussionTab/index.tsx(2 hunks)frontend/src/features/my-calendar/api/index.ts(1 hunks)frontend/src/features/my-calendar/api/keys.ts(1 hunks)frontend/src/features/my-calendar/api/mutations.ts(1 hunks)frontend/src/features/my-calendar/api/queries.ts(1 hunks)frontend/src/features/my-calendar/model/index.ts(1 hunks)frontend/src/features/my-calendar/ui/CalendarCardList/index.tsx(1 hunks)frontend/src/features/my-calendar/ui/MyCalendar/index.tsx(2 hunks)frontend/src/features/my-calendar/ui/SchedulePopover/PopoverForm.tsx(1 hunks)frontend/src/features/my-calendar/ui/SchedulePopover/index.tsx(2 hunks)frontend/src/hooks/useFormRef.ts(2 hunks)frontend/src/routes/_main/discussion/invite/$id.tsx(1 hunks)frontend/src/routes/discussion/create/$id.tsx(1 hunks)frontend/src/utils/date/date.ts(2 hunks)frontend/src/utils/date/format.ts(2 hunks)frontend/src/utils/fetch/index.ts(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- frontend/src/routes/discussion/create/$id.tsx
- frontend/src/routes/_main/discussion/invite/$id.tsx
🧰 Additional context used
🧠 Learnings (2)
frontend/src/features/my-calendar/ui/CalendarCardList/index.tsx (1)
Learnt from: dioo1461
PR: softeer5th/Team4-enDolphin#158
File: frontend/src/features/my-calendar/ui/CalendarCardList/index.tsx:28-28
Timestamp: 2025-02-13T05:27:18.088Z
Learning: In React components, avoid using array indices as key props. Instead, use unique identifiers from the data to ensure stable component rendering and proper state management.
frontend/src/features/my-calendar/ui/SchedulePopover/PopoverForm.tsx (1)
Learnt from: hamo-o
PR: softeer5th/Team4-enDolphin#101
File: frontend/src/routes/my-schedule/@components/SchedulePopover/PopoverForm.tsx:10-35
Timestamp: 2025-02-06T05:14:12.328Z
Learning: Form state management and validation for the SchedulePopover component in frontend/src/routes/my-schedule/@components/SchedulePopover/PopoverForm.tsx will be implemented in a future PR.
🔇 Additional comments (16)
frontend/src/utils/date/format.ts (1)
2-2: LGTM! Import statement is well-organized.The new import follows the project's conventions and is appropriately grouped with other date-related imports.
frontend/src/features/discussion/ui/DiscussionTab/index.tsx (1)
6-6: LGTM! Clean integration of DiscussionRank component.The import and usage of the DiscussionRank component is clean and well-integrated with the existing tab structure.
Also applies to: 29-29
frontend/src/features/my-calendar/ui/SchedulePopover/index.tsx (3)
1-1: Looks good
No issues found with the new imports or the updated interface signature. The usage ofusePersonalEventMutationand extendedSchedulePopoverPropsappears consistent and straightforward.Also applies to: 5-6, 12-12
24-27: ValidatestartDateTimesafely
When destructuringeventand callingnew Date(event.startDateTime), ensurestartDateTimeis always valid. Adding a guard for null/undefined or invalid date formats can prevent runtime bugs.
53-53: EnsurePopoverFormmanages all relevant fields
PopoverFormreceives onlyhandleChangeandvaluesRef. Confirm that additional form fields are correctly consumed or extended as needs evolve.frontend/src/features/my-calendar/api/keys.ts (1)
1-7: Confirm query key serialization approach
Relying onstartDateTimeandendDateTimein the query key is fine if they are strings. If they’re Date objects or other non-serializable values, React Query’s cache might misbehave.frontend/src/features/my-calendar/api/queries.ts (1)
1-5: No concerns
The imports and types align well with the updated data model.frontend/src/features/my-calendar/api/mutations.ts (1)
7-20: LGTM! Well-structured mutation hook implementation.The implementation follows React Query's best practices with proper typing and cache invalidation.
frontend/src/hooks/useFormRef.ts (1)
13-21: LGTM! Clean and focused hook implementation.The hook provides a simple and reusable solution for form state management using refs.
frontend/src/features/my-calendar/ui/CalendarCardList/index.tsx (3)
13-14: LGTM! Props type has been improved.The component now uses a more specific type
PersonalEventDTOwith proper omission of unnecessary fields, improving type safety.
17-22: Verify date parsing safety.The direct creation of Date objects from strings could potentially throw if
startDateTimeorendDateTimeare invalid.Consider adding date validation or using a date parsing library:
- const start = new Date(card.startDateTime); - const end = new Date(card.endDateTime); + const start = new Date(card.startDateTime); + if (isNaN(start.getTime())) { + console.error(`Invalid startDateTime: ${card.startDateTime}`); + return null; + } + const end = new Date(card.endDateTime); + if (isNaN(end.getTime())) { + console.error(`Invalid endDateTime: ${card.endDateTime}`); + return null; + }
26-38: LGTM! Card rendering props are well structured.The component correctly uses the card's properties for rendering, with proper positioning calculations and status handling.
frontend/src/features/my-calendar/ui/MyCalendar/index.tsx (1)
15-17: LGTM! Props type is well defined.The component properly types the
personalEventsprop with a default empty array, preventing undefined access issues.frontend/src/features/my-calendar/ui/SchedulePopover/PopoverForm.tsx (1)
13-25: LGTM! AdjustableCheckbox component is well structured.The component properly handles form state using FormRef and provides clear labeling.
frontend/src/utils/fetch/index.ts (2)
28-32: LGTM! FetchRequest interface is well defined.The interface properly types all possible request parameters with clear optionality.
78-87: LGTM! Request helper methods are well structured.The helper methods properly use TypeScript's Pick utility type to ensure type safety for different request types.
dioo1461
left a comment
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.
고생하셨습니다~~~
| startDateTime: z.string().datetime(), | ||
| endDateTime: z.string().datetime(), |
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.
p3;
저희 나중에 DTO 쓰기 편하도록, 서버에서 string 형태로 들어오는 날짜 혹은 시간 문자열을 Date type으로 쓸 수 있게끔, zod에서 전처리를 하면 좋을 것 같은데 어떻게 생각하시나요!
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
fetch유틸에서 옵셔널로 들어가는 값을 객체로 넘기게 수정, 쿼리파라미터 처리🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
Summary by CodeRabbit