cal.com icon indicating copy to clipboard operation
cal.com copied to clipboard

fix: before/after event buffer with seats

Open MehulZR opened this issue 1 year ago • 10 comments

What does this PR do?

Fixes #13605 (issue)

https://github.com/calcom/cal.com/assets/86384652/4ffcfed8-c4a2-4a91-a772-d8b1baabb319

https://github.com/calcom/cal.com/assets/86384652/ac01fbba-cf6f-4035-aca3-4f86917b7188

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.

MehulZR avatar Feb 19 '24 12:02 MehulZR

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

A member of the Team first needs to authorize it.

vercel[bot] avatar Feb 19 '24 12:02 vercel[bot]

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

github-actions[bot] avatar Feb 19 '24 12:02 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! 🙌

github-actions[bot] avatar Feb 19 '24 12:02 github-actions[bot]

https://github.com/calcom/cal.com/blob/851cd4d9938a979241d5fcc74691d4c0c96ad5f5/packages/core/getBusyTimes.ts#L157-L158

I have added some tests & in those tests the value of seatReferences returned is undefined. Causing the tests to fail. It probably has something to do with prisma mock which I was not able to figure out. So, I will need some help here.

Aside from that, the issue is resolved in actual dev server.

MehulZR avatar Feb 20 '24 12:02 MehulZR

apps/web/test/lib/getSchedule.test.ts > getSchedule > User Event > afterBuffer and beforeBuffer tests for seated event - Non Cal Busy Time is failing because we are subtracting formattedBusyTimes (GMT) from dateRanges (any tz).

Which is solved in this PR https://github.com/calcom/cal.com/blob/da22eda662318ae75d23520c92e41b9c9067f7e9/packages/core/getUserAvailability.ts#L334-L346

apps/web/test/lib/getSchedule.test.ts > getSchedule > User Event > afterBuffer and beforeBuffer tests for seated event - Cal Busy Time is failing which is explained here

MehulZR avatar Feb 20 '24 12:02 MehulZR

I marked the PR ready as I need some help regarding Prisma mock returning underfined.

MehulZR avatar Feb 20 '24 12:02 MehulZR

Graphite Automations

"Add community label" took an action on this PR • (02/20/24)

1 label was added to this PR based on Keith Williams's automation.

"Add consumer team as reviewer" took an action on this PR • (03/02/24)

1 reviewer was added to this PR based on Keith Williams's automation.

graphite-app[bot] avatar Feb 20 '24 12:02 graphite-app[bot]

This PR is being marked as stale due to inactivity.

github-actions[bot] avatar Apr 09 '24 00:04 github-actions[bot]

Fix looks pretty good already, but found one more issue. Could you take a look at that? Also, would be great to cover this case in a test as well

The test suits seem to be broken right now (even on the main branch); therefore, I can't confidently add a test. Once it fixes. I will surely add the suitable tests.

MehulZR avatar Apr 22 '24 12:04 MehulZR

This PR is being marked as stale due to inactivity.

github-actions[bot] avatar May 07 '24 00:05 github-actions[bot]

This PR is being marked as stale due to inactivity.

github-actions[bot] avatar May 31 '24 00:05 github-actions[bot]

apps/web/test/lib/getSchedule.test.ts > getSchedule > User Event > beforeBuffer tests for seated event - Cal Busy Time apps/web/test/lib/getSchedule.test.ts > getSchedule > User Event > afterBuffer tests for seated event - Cal Busy Time apps/web/test/lib/getSchedule.test.ts > getSchedule > User Event > afterBuffer and beforeBuffer tests for seated event - Cal Busy Time These tests are failing which is explained here

apps/web/test/lib/getSchedule.test.ts > getSchedule > User Event > afterBuffer and beforeBuffer tests - Non Cal Busy Time apps/web/test/lib/getSchedule.test.ts > getSchedule > User Event > afterBuffer and beforeBuffer tests - Cal Busy Time

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

were passing earlier because we were subtracting formattedBusyTimes (GMT) from dateRanges (any tz).

For example.

    test("afterBuffer and beforeBuffer tests - Non Cal Busy Time", async () => {
      const { dateString: plus2DateString } = getDate({ dateIncrement: 2 });
      const { dateString: plus3DateString } = getDate({ dateIncrement: 3 });

      CalendarManagerMock.getBusyCalendarTimes.mockResolvedValue([
        {
          start: `${plus3DateString}T04:00:00.000Z`, // 9 AM IST Day 3
          end: `${plus3DateString}T05:59:59.000Z`, //  11 AM IST Day 3
        },
      ]);

      const scenarioData = {
        eventTypes: [
          {
            id: 1,
            length: 120,
            beforeEventBuffer: 120,
            afterEventBuffer: 120,
            users: [
              {
                id: 101,
              },
            ],
          },
        ],
        users: [
          {
            ...TestData.users.example,
            id: 101,
            schedules: [TestData.schedules.IstWorkHours],
            credentials: [getGoogleCalendarCredential()],
            selectedCalendars: [TestData.selectedCalendars.google],
          },
        ],
        apps: [TestData.apps["google-calendar"]],
      };

      await createBookingScenario(scenarioData);

      const scheduleForEventOnADayWithNonCalBooking = await getSchedule({
        input: {
          eventTypeId: 1,
          eventTypeSlug: "",
          startTime: `${plus2DateString}T18:30:00.000Z`, // 0:00 IST Day 3
          endTime: `${plus3DateString}T18:29:59.999Z`, //  23:59 IST Day 3
          timeZone: Timezones["+5:30"],
          isTeamEvent: false,
        },
      });

      expect(scheduleForEventOnADayWithNonCalBooking).toHaveTimeSlots(
        [
          // Info.
          // Working hours / Availability is set to 9:30 AM - 6 PM IST
          // Event length is 2 hours so by default event interval is 2 hours too.

          // Earlier
          // `04:00:00.000Z`, // - 9:30 AM IST is booked
          // `06:00:00.000Z`, // - 11:30 AM is not available because 08:00AM slot has a `beforeEventBuffer`
          `08:00:00.000Z`, // - 1:30 AM is available because of availability of 06:00 - 07:59
          `10:00:00.000Z`, // - 3:30 AM is available
          `12:00:00.000Z`, // - 5:30 AM is available (it can't be because of IST working hours)

          // After
          // But First slot will actually start from `04:30:00.000Z` (10 AM IST) because of event interval being 2 hours.

          // `04:30:00.000Z` // - 10 AM IST (middle of the booking)
          // `06:30:00.000Z`, // - 12 AM IST is not available because 2 PM IST slot has a `beforeEventBuffer`
          `08:30:00.000Z`, // - 2 PM IST is available
          `10:30:00.000Z`, // - 4 PM IST is available
          // `12:30:00.000Z`, // - 6 PM IST is unavailable because of IST working hours
        ],
        {
          dateString: plus3DateString,
        }
      );
    });

MehulZR avatar Jun 04 '24 18:06 MehulZR

I have tried to explain but it's a bit complex (more like intertwined with other issues)

I will suggest to look a bit into my other PRs like this & this as they work on more deeper layers.

I discovered these while working on this PR. So a lot of the context is there too.

If you have any questions, please feel free to ask. Not putting a lot of time in those will help me too as a lot of the context around these problems gets skipped from mind if there is a long delay.

MehulZR avatar Jun 04 '24 19:06 MehulZR

Before: This is main branch without any edit's except the comments.

https://github.com/calcom/cal.com/assets/86384652/86643d1d-9b5f-4e36-b044-4cf2420f0e1a

After: Only edits are converting the formattedBusyTimes from UTC to host's timezone. So that it can subtract from the host's dateRanges without an issue. https://github.com/calcom/cal.com/blob/c2d4677dcd148dfbb7b708eda95fa561674f669f/packages/core/getUserAvailability.ts#L411-L416

https://github.com/calcom/cal.com/assets/86384652/66c31399-69cf-45af-8c8f-623b265905fa

I hope it clears some things up.

MehulZR avatar Jun 04 '24 22:06 MehulZR