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

A member of the Team first needs to authorize it.

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

docs – ./apps/docs

🔍 Inspect: https://vercel.com/cal/docs/3k2rWuuS5FKWw3skci5RvR8VhaJf
✅ Preview: https://docs-git-fork-louishaftmann-fix-double-booking-cal.vercel.app

[Deployment for ed8ee19 canceled]

calendso – ./apps/web

🔍 Inspect: https://vercel.com/cal/calendso/DCyWNY8VYBC2x7Yt7BC9SeBu2DTC
✅ Preview: https://calendso-git-fork-louishaftmann-fix-double-booking-cal.vercel.app

are you using any calendar integrations (google/apple/microsoft)?

yea that's the problem, i'll look into it

I just noticed that we aren't checking for conflicts before creating the booking, not even whether the time slot is out of bounds. Any reason for that?

i believe this line is supposed create a unique seed for each time slot but why does it use new Date().getTime()?

const seed = `${users[0].username}:${dayjs(req.body.start).utc().format()}:${new Date().getTime()}`;

since this touches quite a bit of core, @emrysal and @zomars can you test this?

@LouisHaftmann

i believe this line is supposed create a unique seed for each time slot but why does it use new Date().getTime()?

const seed = `${users[0].username}:${dayjs(req.body.start).utc().format()}:${new Date().getTime()}`;

You are entirely right about this; this is a bug that still exists and was a "hack" whilst integrating Daily to get it to work. (Not quite sure what broke, maybe @baileypumfleet remembers, or @lunchpaillola.

The idea of this seed before then was to prevent conflicts on database level (which was disabled causing the bugs). When this seed works, it would create a unique seed for each slot, causing a P2002 on database level (unique constraint violation). Database was originally deemed to be the ideal place of doing this as any other location would cause race conditions in times of high traffic.

Edit: It used to be: https://github.com/calcom/cal.com/blame/58de9209512c45e253ba3737311a0e21e0d1cf10/pages/api/book/event.ts#L252

Even with the old seed it wouldn't be 100% safe because it doesn't include any info about end time or time slot duration. Changing an event type's duration (eg: from 30min to 15min) would break this conflict check because a booking could be created without conflict error although it conflicts with the old booking.

Example (event type duration: 30min):

1. Create booking at 1pm
2. Trying to book event at 1pm again will result in conflict
3. Change event type duration to 15min
4. Trying to book event at 1pm still results in conflict
5. Trying to book event at 1:15pm works although it should conflict with old events

In this commit I made it so it fetches all existing bookings for the user and event type and checks for conflicts (including integration and video busy times) before even trying to create a booking in the db.

Yeah this is an improvement on the current broken logic, but does need further development.

The event type duration isn't as much of a problem as the seed logic is intended to detect double bookings made at the same time, it's not meant to check freebusy - that happens before then.

Reasons for future (Approving this in principle) development are:

• Booking becomes slower because of conflict checking
• Higher chance of integration failure
• Still can be a race condition - but there is a smaller window.

Virtual calendar is a feature that will focus on improving this

A seed like

userId:eventTypeId:startTime

would probably work for now. It doesn't cover all edge cases but would atleast prevent simple double booking

Prisma's @@unique constraint could also work

A seed like

userId:eventTypeId:startTime

would probably work for now. It doesn't cover all edge cases but would atleast prevent simple double booking

once @zomars reviewed this PR, he can merge it

Checking...

@LouisHaftmann

i believe this line is supposed create a unique seed for each time slot but why does it use new Date().getTime()?

const seed = `${users[0].username}:${dayjs(req.body.start).utc().format()}:${new Date().getTime()}`;

You are entirely right about this; this is a bug that still exists and was a "hack" whilst integrating Daily to get it to work. (Not quite sure what broke, maybe @baileypumfleet remembers, or @lunchpaillola.

The idea of this seed before then was to prevent conflicts on database level (which was disabled causing the bugs). When this seed works, it would create a unique seed for each slot, causing a P2002 on database level (unique constraint violation). Database was originally deemed to be the ideal place of doing this as any other location would cause race conditions in times of high traffic.

Edit: It used to be: https://github.com/calcom/cal.com/blame/58de9209512c45e253ba3737311a0e21e0d1cf10/pages/api/book/event.ts#L252

It's been a while so I don't know if that hack is still relevant since I'm sure the code has changed a bit. But for some historical context the new date had to do with rebooking a time that had been previously canceled. Without the hack if you had canceled a time you wouldn't be able to rebook that time even if the slot had been canceled.

Sounds like it's all sorted now though!