fix: Issue with "Limit total booking duration" and offering seats

[MehulZR]

What does this PR do?

Fixes #13509 (issue)

https://github.com/calcom/cal.com/assets/86384652/0dd9920e-68ec-43cf-a942-dd474425a974

https://github.com/calcom/cal.com/assets/86384652/9a1cc3ef-eba7-4958-bbb7-be03a46f80ab

Type of change

• [x] Bug fix (non-breaking change which fixes an issue)

Mandatory Tasks

• [x] Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

[vercel[bot]]

@MehulZR is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

[github-actions[bot]]

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link.

[github-actions[bot]]

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

[MehulZR]

This commit is according to 2nd approach. As I looked into limits more, I think it might be better to go with the 1st approach.

What do you guys think? As for now, further commits will be in according to the 1st approach.

[MehulZR]

@keithwillcode Sorry, I have been busy past couple days. This PR contains some bugs which I have resolved locally, just have to test them thoroughly.

[MehulZR]

So for a long time, these particular tests were always failing for me (main branch with no changes). At that time I was not able to figure out why at a glance. But tests were passing on the github actions. I have checked this on multiple linux distros. I think it's probably due to timezone issues?

As I was working on the issue and fixing tests for limits with seats, I made the required changes.

https://github.com/calcom/cal.com/blob/01b5c1f11eba9ced597a2b5e33810bcd9f7b9b44/packages/core/getUserAvailability.ts#L334-L346

We were trying to subtract formattedBusyTimes (which are in GMT) from dateRanges (which can be in diff tz). Leading to some issue.

So after quick fix. I think the currently failing test have become more consistent.

[MehulZR]

https://github.com/calcom/cal.com/assets/86384652/87e23780-d943-432d-b6a5-34fdf3a675ac

It looks like it is related to timezone. I will look into this bit more.

[MehulZR]

Can anyone confirm what should be the case when both before event buffer and after event buffer are applied on an event.

https://github.com/calcom/cal.com/blob/0a795f5363b82fd5a5b6ffa1a4fa31a035a3f44f/apps/web/test/lib/getSchedule.test.ts#L584-L596

The test suggests

• 4-6 am is our google calendar busy time.
• 6-8 am is before event buffer for the 8 am slot.
• 8-10 am available slot

Isn't this incorrect? I used to think that it would mean something like

• 4-6 am google calendar busy time.
• 6-8 am after event buffer for 4 am slot.
• 8-10 am before event buffer for 10 am slot.
• 10-12 pm available slot.

After looking into it bit more I found that for Non-Cal busy times Case B is happening & for Cal busy times Case A is happening. (All this is being done in the unit tests. Unless the calendarManagerMock is faulty, It should be the same in the app)

[keithwillcode]

Not sure if related but just found important time zone fix in #13738

[MehulZR]

Can anyone confirm what should be the case when both before event buffer and after event buffer are applied on an event.

https://github.com/calcom/cal.com/blob/0a795f5363b82fd5a5b6ffa1a4fa31a035a3f44f/apps/web/test/lib/getSchedule.test.ts#L584-L596

The test suggests

* 4-6 am is our google calendar busy time.

* 6-8 am is before event buffer for the 8 am slot.

* 8-10 am available slot

Isn't this incorrect? I used to think that it would mean something like

* 4-6 am google calendar busy time.

* 6-8 am after event buffer for 4 am slot.

* 8-10 am before event buffer for 10 am slot.

* 10-12 pm available slot.

After looking into it bit more I found that for Non-Cal busy times Case B is happening & for Cal busy times Case A is happening. (All this is being done in the unit tests. Unless the calendarManagerMock is faulty, It should be the same in the app)

Okay, So I think the bookings which are not made throught cal don't have event buffers applied. In the test it's just that the 8:00AM slot needs before event buffer.

Correct me, If I am wrong.

[MehulZR]

~~I would love it if we can we test this before updating this branch with the main branch. If there is a issue remaining it would be easier to fix it in the current state.~~ The merge conflict was pretty small so, I just did it.

[MehulZR]

Not sure if related but just found important time zone fix in #13738

Probably not, It doesn't change anything below the getUserAvailability layer.

[MehulZR]

These PRs are also closely related. https://github.com/calcom/cal.com/pull/13707/ https://github.com/calcom/cal.com/pull/13651/

[github-actions[bot]]

This PR is being marked as stale due to inactivity.

[Udit-takkar]

@MehulZR any updates on this? Type checks and unit tests are failing

[MehulZR]

@Udit-takkar I will look into it soon; currently a bit busy.

I had already left some comments regarding the type errors. Unit test failure might be related to the issue brought up by Carina (for which I have also left a comment) .

[github-actions[bot]]

This PR is being marked as stale due to inactivity.

[MehulZR]

@Udit-takkar PR is ready for review.

[MehulZR]

@CarinaWolli Can you provide me some more details. I tried to replicate the issue with what I could observe from the video but didn't find anything unusual in my testing.

[CarinaWolli]

@CarinaWolli Can you provide me some more details. I tried to replicate the issue with what I could observe from the video but didn't find anything unusual in my testing.

That's what I did:

• Availability:

• Profile tz of event owner: London

• event type: seats, limit 2 bookings per week

• Book first seats of Tuesday and first seat of Wednesday next week in Pago Pago tz

• Slots are shown correctly, only these two slots are now open

• Change owner's profile tz to New York

• Now the whole week is blocked -> changing back to london opens up the slots again

[MehulZR]

@CarinaWolli I hope the explanation clears things up.

[CarinaWolli]

@CarinaWolli I hope the explanation clears things up.

ah that makes a lot of sense, thank you for clarify that for me 🙏

[keithwillcode]

@CarinaWolli Is this one good to go now with the clarification?

[MehulZR]

Was that a bug? if yes, how can I reproduce that bug?

@CarinaWolli Yes. It can produce values in incorrect tz and feed those to dependent functions such as getSlots(). More info regarding this bug & how to reproduce it can be found in this msg.

[MehulZR]

Found an issue, could you take a look at that?

* created seated event type (15 min), with:
  
  * Limit booking frequency: 1 per day
  * Limit total booking duration: 15 min per week

* Book one seat

* Now the whole week is blocked

* If disabling one of the limits it works, but together the slot with the already booked seat is not showing for me

It's because limit manager can have duplicate entries based on the ordering of the addition of busy times. Check the image above for what I mean by duplicate in the current scenario.

@CarinaWolli Should I pull a separate PR for this? As the issue isn't directly related to the current PR.