-
Notifications
You must be signed in to change notification settings - Fork 11.4k
fix: correct off-by-one error in rolling period booking limit #26196
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?
fix: correct off-by-one error in rolling period booking limit #26196
Conversation
|
@kartik-212004 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
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.
No issues found across 5 files
| if (periodData?.periodType === "ROLLING" && periodData.periodDays) { | ||
| const today = dayjs(); | ||
| const periodDays = periodData.periodDays; | ||
| const maxBookableDate = today.add(Math.max(0, periodDays - 1), "day"); |
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.
looks like the current implementation always uses calendar days, are we not considering business days?
also consider using the existing utilities
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.
now using periodCountCalendarDays to determine the calculation method
dhairyashiil
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.
left one comment, please check. let me know what you think
I have addressed the suggested changes |
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.
No issues found across 5 files
7f0a7e9 to
2d6a1cc
Compare
What does this PR do?
Fixes off-by-one error in rolling period booking limit calculation where setting "1 calendar day" would incorrectly allow booking for 2 days instead of just today.
Visual Demo (For contributors especially)
Before:
https://www.loom.com/share/3936ca376d3843d290022e74f7b3d194
After:
https://www.loom.com/share/0459760a190b43709d47d4ec3a7c749d
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?