Skip to content

Conversation

@kartik-212004
Copy link
Contributor

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:

image

https://www.loom.com/share/3936ca376d3843d290022e74f7b3d194

After:

image

https://www.loom.com/share/0459760a190b43709d47d4ec3a7c749d

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox. N/A
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  1. Create an event type
  2. Go to Limits tab → Enable "Limit future bookings"
  3. Set to "1 calendar days into the future"
  4. Save and open the public booking page
  5. Expected: Only today's date should be selectable. Tomorrow should be grayed out.
  6. If today has no remaining time slots, the calendar should stay on the current month (not jump to next month)

@vercel
Copy link

vercel bot commented Dec 25, 2025

@kartik-212004 is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Dec 25, 2025
@github-actions github-actions bot added High priority Created by Linear-GitHub Sync reactive⚡︎ 🐛 bug Something isn't working labels Dec 25, 2025
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a 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");
Copy link
Member

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

Copy link
Contributor Author

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

Copy link
Member

@dhairyashiil dhairyashiil left a 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

@dhairyashiil dhairyashiil marked this pull request as draft December 26, 2025 04:00
@kartik-212004
Copy link
Contributor Author

left one comment, please check. let me know what you think

I have addressed the suggested changes

@kartik-212004 kartik-212004 marked this pull request as ready for review December 26, 2025 05:00
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working community Created by Linear-GitHub Sync High priority Created by Linear-GitHub Sync reactive⚡︎ size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Same-day appointment bug + messy calendar UI + unclear error copy

2 participants