-
Notifications
You must be signed in to change notification settings - Fork 2
feat: implement weekly booking deadline restriction #38
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Pull request overview
This pull request implements a weekly booking deadline restriction that prevents non-admin users from creating bookings less than 7 days in advance. Admin users are exempt from this restriction.
Key Changes:
- Added validation logic to check booking advance notice requirements
- Implemented admin exemption for the deadline restriction
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/lib/actions/booking.ts
Outdated
| const now = new Date(); | ||
| const bookingDate = new Date(data.startTime); | ||
| const daysUntilBooking = Math.floor( | ||
| (bookingDate.getTime() - now.getTime()) / (1000 * 60 * 60 * 24), |
Copilot
AI
Dec 15, 2025
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.
The magic number 1000 * 60 * 60 * 24 for milliseconds per day should be extracted to a named constant for better maintainability and clarity. This makes the code more readable and reduces the risk of calculation errors.
src/lib/actions/booking.ts
Outdated
| const daysUntilBooking = Math.floor( | ||
| (bookingDate.getTime() - now.getTime()) / (1000 * 60 * 60 * 24), | ||
| ); | ||
| if (daysUntilBooking < 7) { |
Copilot
AI
Dec 15, 2025
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.
The magic number 7 (representing 7 days) should be extracted to a named constant like MIN_BOOKING_ADVANCE_DAYS. This makes the requirement more explicit and easier to modify if business rules change.
src/lib/actions/booking.ts
Outdated
| const daysUntilBooking = Math.floor( | ||
| (bookingDate.getTime() - now.getTime()) / (1000 * 60 * 60 * 24), | ||
| ); |
Copilot
AI
Dec 15, 2025
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.
The date calculation doesn't account for timezones, which can lead to inconsistent behavior. When creating Date objects without timezone specification, they use the server's local timezone, but bookings may be created by users in different timezones. This can result in incorrect day calculations. Consider using UTC methods for consistent date comparison, or ensure both dates are normalized to the same timezone before calculating the difference.
| const daysUntilBooking = Math.floor( | |
| (bookingDate.getTime() - now.getTime()) / (1000 * 60 * 60 * 24), | |
| ); | |
| // Calculate difference in days using UTC to avoid timezone issues | |
| const utcNow = Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate()); | |
| const utcBooking = Date.UTC(bookingDate.getUTCFullYear(), bookingDate.getUTCMonth(), bookingDate.getUTCDate()); | |
| const daysUntilBooking = Math.floor((utcBooking - utcNow) / (1000 * 60 * 60 * 24)); |
src/lib/actions/booking.ts
Outdated
| if (token.isAdmin === false) { | ||
| const now = new Date(); | ||
| const bookingDate = new Date(data.startTime); | ||
| const daysUntilBooking = Math.floor( |
Copilot
AI
Dec 15, 2025
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.
The Math.floor operation can produce incorrect results for edge cases. For example, if a booking is made 6 days and 23 hours in advance, Math.floor will round down to 6 days, rejecting a booking that's nearly 7 days away. Consider using Math.ceil or adjusting the threshold to ensure the 7-day requirement is accurately enforced based on the intended business logic.
| const daysUntilBooking = Math.floor( | |
| const daysUntilBooking = Math.ceil( |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
TheMythologist
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.
Do look through the document linked in the issue #21 for more specific booking restrictions per venue. The current implementation is too basic. Also look through the comments by Copilot and address those issues
Resolves #21