Fix: Restrict event times to working hours (8 AM - 6 PM) #226
Conversation
|
@Aadhira22 is attempting to deploy a commit to the Ansul Agrawal's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe update introduces a helper method to clamp event times within specified scheduler hours in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CustomTimeComponent
participant SchedulerData
participant ConfirmationDialog
User->>CustomTimeComponent: Initiates event create/update/move
CustomTimeComponent->>CustomTimeComponent: clampEventTimes(schedulerData, start, end)
CustomTimeComponent->>ConfirmationDialog: Show confirmation with clamped times
ConfirmationDialog-->>CustomTimeComponent: User confirms
CustomTimeComponent->>SchedulerData: Update event with clamped times
SchedulerData-->>CustomTimeComponent: Update state
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json(1 hunks)src/examples/pages/Custom-Time/class-based.jsx(7 hunks)
🔇 Additional comments (7)
package.json (1)
93-93: Webpack version update validated
- Confirmed that
[email protected]exists and is the latest published version.- No moderate-or-higher security advisories were found when running
npm audit(via lockfile or registry data).- As a minor update within the same major version, this bump should not introduce breaking changes.
src/examples/pages/Custom-Time/class-based.jsx (6)
19-20: LGTM! Clear configuration with helpful comments.The inline comments clearly document the working hours configuration (8 AM to 6 PM), which aligns perfectly with the PR objective.
136-156: LGTM! Proper integration of time clamping in event creation.The
newEventmethod correctly uses theclampEventTimeshelper and applies the clamped values to both the confirmation dialog and the new event object.
158-164: LGTM! Correct implementation of clamped start time updates.The
updateEventStartmethod properly clamps the new start time while preserving the existing end time, and updates the scheduler with the clamped value.
166-172: LGTM! Correct implementation of clamped end time updates.The
updateEventEndmethod properly clamps the new end time while preserving the existing start time, and updates the scheduler with the clamped value.
174-184: LGTM! Proper integration of time clamping in event moves.The
moveEventmethod correctly applies time clamping to both start and end times when moving events, ensuring consistency with the working hours constraint.
93-97: Minor formatting improvements look good.The removal of unnecessary parentheses in arrow functions and formatting adjustments improve code readability without affecting functionality.
Also applies to: 99-103, 121-121
| // Helper function to clamp times to 8 AM and 7 PM | ||
| clampEventTimes = (schedulerData, start, end) => { | ||
| const { dayStartFrom, dayStopTo } = schedulerData.config; | ||
| const startDate = new Date(start); | ||
| const endDate = new Date(end); | ||
|
|
||
| // Get the date part to preserve it | ||
| const dateStr = startDate.toISOString().split('T')[0]; | ||
|
|
||
| // Clamp start time to 8 AM | ||
| if (startDate.getHours() < dayStartFrom) { | ||
| startDate.setHours(dayStartFrom, 0, 0, 0); | ||
| } | ||
|
|
||
| // Clamp end time to 7 PM | ||
| if (endDate.getHours() > dayStopTo || (endDate.getHours() === dayStopTo && endDate.getMinutes() > 0)) { | ||
| endDate.setHours(dayStopTo, 0, 0, 0); | ||
| } | ||
|
|
||
| // Ensure end is not before start | ||
| if (endDate < startDate) { | ||
| endDate.setTime(startDate.getTime() + 30 * 60 * 1000); // Default to 30 minutes after start | ||
| } | ||
|
|
||
| return { | ||
| clampedStart: `${dateStr}T${startDate.getHours().toString().padStart(2, '0')}:00:00`, | ||
| clampedEnd: `${dateStr}T${endDate.getHours().toString().padStart(2, '0')}:00:00`, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Critical issues in time clamping logic need to be addressed.
The clampEventTimes helper method has several logical and implementation issues:
-
Inconsistent hour boundary checking: Line 52 checks if
endDate.getHours() > dayStopTobutdayStopTois 18 (6 PM), so events ending at exactly 6 PM would be incorrectly allowed. -
Incomplete time preservation: The method only preserves hours but discards minutes and seconds from the original times, potentially losing precision.
-
Timezone handling: Using
new Date()constructor with ISO strings may cause timezone conversion issues. -
Inconsistent return format: Returns ISO-like strings but only with hour precision, not matching typical ISO format.
Apply this diff to fix the time clamping logic:
- clampEventTimes = (schedulerData, start, end) => {
- const { dayStartFrom, dayStopTo } = schedulerData.config;
- const startDate = new Date(start);
- const endDate = new Date(end);
-
- // Get the date part to preserve it
- const dateStr = startDate.toISOString().split('T')[0];
-
- // Clamp start time to 8 AM
- if (startDate.getHours() < dayStartFrom) {
- startDate.setHours(dayStartFrom, 0, 0, 0);
- }
-
- // Clamp end time to 7 PM
- if (endDate.getHours() > dayStopTo || (endDate.getHours() === dayStopTo && endDate.getMinutes() > 0)) {
- endDate.setHours(dayStopTo, 0, 0, 0);
- }
-
- // Ensure end is not before start
- if (endDate < startDate) {
- endDate.setTime(startDate.getTime() + 30 * 60 * 1000); // Default to 30 minutes after start
- }
-
- return {
- clampedStart: `${dateStr}T${startDate.getHours().toString().padStart(2, '0')}:00:00`,
- clampedEnd: `${dateStr}T${endDate.getHours().toString().padStart(2, '0')}:00:00`,
- };
- };
+ clampEventTimes = (schedulerData, start, end) => {
+ const { dayStartFrom, dayStopTo } = schedulerData.config;
+ const startDate = new Date(start);
+ const endDate = new Date(end);
+
+ // Get the date part to preserve it
+ const dateStr = startDate.toISOString().split('T')[0];
+
+ // Clamp start time to working hours start
+ if (startDate.getHours() < dayStartFrom) {
+ startDate.setHours(dayStartFrom, 0, 0, 0);
+ }
+
+ // Clamp end time to working hours end (6 PM = 18:00)
+ if (endDate.getHours() >= dayStopTo) {
+ endDate.setHours(dayStopTo - 1, 59, 59, 999); // Set to 17:59:59 for 6 PM boundary
+ }
+
+ // Ensure end is not before start
+ if (endDate <= startDate) {
+ endDate.setTime(startDate.getTime() + 30 * 60 * 1000); // Default to 30 minutes after start
+ }
+
+ return {
+ clampedStart: startDate.toISOString(),
+ clampedEnd: endDate.toISOString(),
+ };
+ };Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/examples/pages/Custom-Time/class-based.jsx between lines 37 and 65, the
clampEventTimes function has issues with boundary checks, time precision,
timezone handling, and return format. Fix it by adjusting the end time check to
correctly clamp events ending exactly at the boundary hour, preserve minutes and
seconds when clamping start and end times, avoid timezone conversion by
manipulating date components directly rather than relying on ISO string parsing,
and return fully formatted ISO 8601 strings including hours, minutes, and
seconds to maintain consistency and precision.
Docstrings generation was requested by @ansulagrawal. * #226 (comment) The following files were modified: * `src/examples/pages/Custom-Time/class-based.jsx`
|
Note Generated docstrings for this pull request at #227 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@Aadhira22 I don't think this is req |
I noticed that the event scheduler was allowing events to be scheduled outside of the defined working hours (8 AM to 6 PM). I modified the event creation and update logic to ensure events can only be scheduled within this time range.
This change:
Summary by CodeRabbit
New Features
Chores