[FE-Feat] 커스텀 Date, Time 클래스 구현 & 일정 확정 API 적용 #372
Conversation
WalkthroughThe changes improve date and time handling and import clarity across the frontend. A new ISO 8601 regex constant has been added, and the discussion confirmation flow has been updated to use validated response parsing and DTO transformation with local date-time handling. UI components now use custom date/time objects, and function signatures have been adjusted to include seconds. Additionally, the Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Discussion UI
participant API as Discussion API
participant Parser as Response Parser
participant Model as Discussion Model
participant DateUtil as EndolphinDate/Time
UI->>API: postDiscussionConfirm(request)
API-->>UI: response data
UI->>Parser: parse(response)
Parser->>Model: Create validated response
Model->>DateUtil: Transform date/time fields
DateUtil-->>Model: Return formatted values
Model-->>UI: Return updated discussion data
sequenceDiagram
participant User as User
participant CardList as OngoingCardList
participant TimeHook as useScrollToTime
User->>CardList: Select time
CardList->>TimeHook: handleSelectTime({hour, minute, second: 0})
TimeHook-->>CardList: Updated time state
Suggested labels
Poem
✨ Finishing Touches
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. 🪧 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: 1
🧹 Nitpick comments (2)
frontend/src/utils/endolphin-date/date.ts (1)
25-58: Add null checks to all formatting methods for consistencySome formatting methods check for null dates while others don't, which could lead to errors.
Consider adding null checks to all formatting methods for consistency:
formatDateToBarString () { + if (!this.#date) return ''; return format.formatDateToBarString(this.#date); } formatDateToDotString () { + if (!this.#date) return ''; return format.formatDateToDotString(this.#date); } formatDateToDateTimeString () { + if (!this.#date) return ''; return format.formatDateToDateTimeString(this.#date); } formatTimeToColonString () { + if (!this.#date) return ''; return format.formatTimeToColonString(this.#date); } formatDateToTimeString () { + if (!this.#date) return ''; return format.formatDateToTimeString(this.#date); } formatDateToString () { + if (!this.#date) return ''; return format.formatDateToString(this.#date); }frontend/src/utils/endolphin-date/time.ts (1)
60-64: Use imported constant for consistencyConsider using the imported constant for consistency rather than redefining it locally.
- const MINUTE_IN_MILLISECONDS = 60000; + const { MINUTE_IN_MILLISECONDS } = calc;This maintains consistency with the imported module and avoids duplicate definitions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
frontend/src/constants/regex.ts(1 hunks)frontend/src/features/discussion/api/index.ts(2 hunks)frontend/src/features/discussion/model/index.ts(3 hunks)frontend/src/features/discussion/ui/DiscussionConfirmCard/BadgeContainer.tsx(2 hunks)frontend/src/features/discussion/ui/DiscussionConfirmCard/index.tsx(1 hunks)frontend/src/features/my-calendar/ui/OngoingDiscussion/OngoingCardList.tsx(1 hunks)frontend/src/hooks/useScrollToTime.ts(1 hunks)frontend/src/utils/date/time.ts(1 hunks)frontend/src/utils/endolphin-date/date.ts(1 hunks)frontend/src/utils/endolphin-date/index.ts(1 hunks)frontend/src/utils/endolphin-date/time.ts(1 hunks)frontend/src/utils/endolphin-date/type.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
frontend/src/features/discussion/model/index.ts (1)
Learnt from: dioo1461
PR: softeer5th/Team4-enDolphin#222
File: frontend/src/features/discussion/model/index.ts:14-18
Timestamp: 2025-03-30T17:37:00.658Z
Learning: In the discussion feature's SharedEventDTO, use `z.string().datetime()` for validating date-time strings as it properly handles ISO 8601 format.
🧬 Code Definitions (4)
frontend/src/utils/endolphin-date/type.ts (2)
frontend/src/utils/endolphin-date/date.ts (1)
EndolphinDate(7-62)frontend/src/utils/date/time.ts (1)
Time(69-73)
frontend/src/utils/endolphin-date/time.ts (2)
frontend/src/utils/endolphin-date/type.ts (2)
InputTime(5-5)Time(7-11)frontend/src/utils/date/time.ts (2)
Time(69-73)MINUTE_IN_MILLISECONDS(3-3)
frontend/src/features/discussion/api/index.ts (1)
frontend/src/features/discussion/model/index.ts (2)
DiscussionConfirmResponse(80-90)DiscussionConfirmResponse(96-96)
frontend/src/features/my-calendar/ui/OngoingDiscussion/OngoingCardList.tsx (1)
frontend/src/utils/date/date.ts (1)
setTimeOnly(288-293)
🔇 Additional comments (25)
frontend/src/constants/regex.ts (1)
2-2: New ISO 8601 regex pattern added correctly.The added
DATETIMEregex correctly validates ISO 8601 formatted date-times (YYYY-MM-DDTHH:MM:SS). Note that this pattern is strict and doesn't accept timezone information, which is appropriate if all date-times are handled in a consistent timezone throughout the application.frontend/src/hooks/useScrollToTime.ts (1)
27-29: Time object now includes seconds.The
Timeobject now includes asecondproperty (set to 0), which aligns with the new Time class implementation mentioned in the PR objectives. This change ensures consistency with the new custom Date and Time classes being implemented.frontend/src/utils/endolphin-date/index.ts (1)
1-2: Well-organized barrel exports for custom date/time classes.Good practice using a barrel file to export the new
EndolphinDateandEndolphinTimeclasses. This provides a clean public API for the date/time utilities and simplifies imports for consumers.frontend/src/features/discussion/ui/DiscussionConfirmCard/index.tsx (1)
29-30: Updated to use custom date objects directly.The code now passes
EndolphinDateobjects directly to theBadgeContainercomponent instead of wrapping them innew Date(). This is a positive change that:
- Maintains the immutability mentioned in the PR objectives
- Preserves the encapsulation of the custom date classes
- Avoids unnecessary object creation
This change is part of the broader migration to the new custom Date and Time classes.
Also applies to: 31-32
frontend/src/features/my-calendar/ui/OngoingDiscussion/OngoingCardList.tsx (2)
51-52: Updated time handling to include secondsThe addition of
second: 0to bothsetTimeOnlycalls ensures consistent time handling with the updatedTimeinterface that now requires seconds.
54-54: Added seconds parameter to handleSelectTime for consistencyThis change properly aligns with the updated
Timeinterface requirements and ensures time selection works correctly with the new interface definition.frontend/src/utils/date/time.ts (1)
72-72:Details
❓ Verification inconclusive
Interface change: seconds now required in Time interface
Making
seconda required property in theTimeinterface enforces consistency across the application. This is a breaking change that requires all implementations to include seconds.Verify that all usages of the
Timeinterface have been updated to include seconds:
🏁 Script executed:
#!/bin/bash # Search for places where Time interface is used but might not include seconds # Look for object literals that might be constructing Time objects rg -t typescript "\{\s*hour:.*minute:" --no-filename | grep -v "second"Length of output: 106
Action: Verify All Time Interface Implementations Include
secondThe interface in
frontend/src/utils/date/time.tsnow requiressecond, which is a breaking change. However, our initial check using therg -t typescriptcommand returned an error (due to an unrecognized file type). Please run the following command manually to verify that every usage of theTimeinterface includes thesecondproperty:#!/bin/bash # Re-check Time object usages in TypeScript files ensuring inclusion of the `second` property. rg -t ts "\{\s*hour:.*minute:" --no-filename | grep -v "second"If this command produces no output, it indicates that all instances have been updated correctly. Otherwise, please address any cases where
secondis missing.frontend/src/utils/endolphin-date/type.ts (2)
1-6: Good type definitions for flexible date/time inputsThe types
InputDateandInputTimeprovide excellent flexibility for working with dates and times in different formats. The import ofEndolphinDatecreates a clear dependency between these types.
7-11: Time interface matches the updated interface in date/time.tsThe
Timeinterface correctly includes all three required fields: hour, minute, and second. This ensures consistency with the originalTimeinterface infrontend/src/utils/date/time.tsthat was updated.frontend/src/features/discussion/ui/DiscussionConfirmCard/BadgeContainer.tsx (6)
3-3: Updated imports to use new date/time utilitiesGood replacement of the generic date utilities with the custom
EndolphinDateandEndolphinTimeclasses as mentioned in the PR objectives.
7-11: Updated parameter types to use custom date classThe component now correctly accepts
EndolphinDateobjects for date parameters, and makes location nullable withstring | nulltype, improving type safety.
12-14: Instantiated EndolphinTime objects for time operationsCreating
EndolphinTimeinstances from theEndolphinDateparameters allows using the custom time methods consistently throughout the component.
22-23: Used EndolphinDate formatting methodGood use of the
formatDateToStringmethod from theEndolphinDateclass, replacing direct Date manipulation.
25-26: Used EndolphinTime for time range formattingThe component now uses the
getTimeRangeStringmethod fromEndolphinTime, encapsulating the time formatting logic within the class.
28-30: Used EndolphinTime for duration calculationThe component now uses the
getMinuteDiffmethod fromEndolphinTime, properly delegating the time difference calculation to the specialized class.frontend/src/features/discussion/model/index.ts (3)
5-5: Good addition of EndolphinDate importThe import of the custom EndolphinDate class aligns well with the PR objective of implementing custom Date and Time classes.
17-18: Enhanced datetime handling with local time supportThe change from
z.string().datetime()toz.string().datetime({ local: true })improves how date-time values are processed by treating them as local times rather than UTC. This is consistent with the previous learning wherez.string().datetime()was recommended for ISO 8601 format validation.
80-90: Well-implemented transformation to custom date objectsThe explicit export and transformation of date fields into EndolphinDate instances effectively implements the custom Date class integration. The nullable meetingMethodOrLocation field adds flexibility to handle cases where this information might not be available.
frontend/src/features/discussion/api/index.ts (2)
3-14: Improved import structureSeparating the imports for better organization enhances code readability. The explicit import of DiscussionConfirmResponse makes dependencies clearer.
62-62: Enhanced type safety with response validationUsing
DiscussionConfirmResponse.parse(response)instead of returning the raw response ensures proper validation and transformation of API data. This guarantees type safety and converts date strings to EndolphinDate instances as defined in the model.Also applies to: 67-67
frontend/src/utils/endolphin-date/date.ts (2)
7-13: Well-structured class with encapsulated date handlingThe EndolphinDate class follows good OOP principles with private fields for encapsulation and a clear constructor. This aligns perfectly with the PR objective of using class-based approaches and ensuring immutability.
59-61: Good use of getter methodThe getter method provides access to the internal date object without allowing direct modification, which supports immutability as mentioned in the PR objectives.
frontend/src/utils/endolphin-date/time.ts (3)
9-20: Well-implemented time class with private fieldsThe EndolphinTime class follows the same good OOP principles as EndolphinDate, with private fields for encapsulation and a clear constructor that handles multiple input formats.
22-33: Robust input validationThe formatToTime method properly handles different input types and includes appropriate error handling for invalid formats.
66-71: Good implementation of time range formattingThe getTimeRangeString method properly checks for null values before formatting and leverages the existing calc utility function.
| #formateToDate (input: InputDate): Date | null { | ||
| if (!input) return null; | ||
| if (input instanceof Date) return this.#date; | ||
| if (typeof input === 'number') { | ||
| return new Date(input); | ||
| } | ||
| if (DATE_BAR.test(input) || DATETIME.test(input)) return new Date(input); | ||
|
|
||
| throw new Error('올바르지 않은 날짜 형식입니다.'); | ||
| } |
There was a problem hiding this comment.
Fix issue in Date instance handling
There's a logical error in the date conversion method.
- if (input instanceof Date) return this.#date;
+ if (input instanceof Date) return input;When the input is a Date instance, you're returning this.#date which would be undefined during construction, instead of the actual input Date.
📝 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.
| #formateToDate (input: InputDate): Date | null { | |
| if (!input) return null; | |
| if (input instanceof Date) return this.#date; | |
| if (typeof input === 'number') { | |
| return new Date(input); | |
| } | |
| if (DATE_BAR.test(input) || DATETIME.test(input)) return new Date(input); | |
| throw new Error('올바르지 않은 날짜 형식입니다.'); | |
| } | |
| #formateToDate (input: InputDate): Date | null { | |
| if (!input) return null; | |
| - if (input instanceof Date) return this.#date; | |
| + if (input instanceof Date) return input; | |
| if (typeof input === 'number') { | |
| return new Date(input); | |
| } | |
| if (DATE_BAR.test(input) || DATETIME.test(input)) return new Date(input); | |
| throw new Error('올바르지 않은 날짜 형식입니다.'); | |
| } |
dioo1461
left a comment
dioo1461
left a comment
There was a problem hiding this comment.
고생하셨습니다~~ 저도 마이그레이션 진행해보겠습니다!
|
|
||
| import type { InputDate } from './type'; | ||
|
|
||
| export default class EndolphinDate { |
There was a problem hiding this comment.
just ask;
앞으로 매우 자주 사용될 클래스일 것 같은데, 클래스명이 불필요하게 긴 느낌이 들기도 합니다..! 축약된 형태로 네이밍하면 어떨까요?
ex) EndoDate, DateKit ?
There was a problem hiding this comment.
배포까지 생각해서 지은 네이밍인데, 조금 더 고민해 보겠습니다.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
|
현영님 혹시 이 pr에서 큰 변경사항 없으시다면 머지해도 괜찮을까요? |
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
커스텀 Date 및 Time 클래스를 구현하고, 일정 확정 API의 응답값에 적용했습니다.
지난 주 조사 후
get*으로만 값을 반환받을 수 있도록 했습니다.🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
이전의 유틸함수는 유지해 두었습니다... 마이그레이션이 끝날 때까지는 삭제하지 않는 것이 좋을 것 같아요.
Summary by CodeRabbit
New Features
Refactor