Fixing unconfirmed/recurring bookings by leog · Pull Request #4881 · calcom/cal.diy

leog · 2022-10-06T14:00:01Z

What does this PR do?

When looking at "Unconfirmed" tab in Bookings, and you have recurring events to be accepted, they were not grouped correctly. Also, the number of instances in the tooltip info section for recurring events was dependant on loaded bookings, hence infinite scrolling was affecting its content, flickering the number.

See the number flickering:

Screen.Recording.2022-10-06.at.10.42.10.mov

Also as part of the fix, a new little feature is shown, the number of recurring events is adjusted to show actually dates in the future in case any instance already passed, and the tooltip shows passed instances as striked:

And the ungrouped unconfirmed recurring events:

Fixed ungrouped unconfirmed recurring events with better tooltip content:

Environment: Staging(main branch) / Production

Type of change

Bug fix (non-breaking change which fixes an issue)

How should this be tested?

Have at least 3 x12 unconfirmed recurring events and go to Unconfirmed tab or Recurring tab in Bookings page.

vercel · 2022-10-06T14:00:05Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
cal	✅ Ready (Inspect)	Visit Preview	Oct 6, 2022 at 6:53PM (UTC)

leog · 2022-10-06T15:16:09Z

 type BookingItemProps = BookingItem & {
  listingStatus: BookingListingStatus;
-  recurringBookings?: BookingItem[];
+  recurringBookings: inferQueryOutput<"viewer.bookings">["recurringInfo"];


Now relying on the returned information from tRPC viewer.bookings

leog · 2022-10-06T15:16:50Z

  const isPending = booking.status === BookingStatus.PENDING;
+  const isRecurring = booking.recurringEventId !== null;
+  const isTabRecurring = booking.listingStatus === "recurring";
+  const isTabUnconfirmed = booking.listingStatus === "unconfirmed";


Simplifying a bunch of conditions throughout this file

leog · 2022-10-06T15:17:57Z

            <Tooltip
              content={recurringStrings.map((aDate, key) => (
-                <p key={key}>{aDate}</p>
+                <p key={key} className={classNames(recurringDates[key] < now && "line-through")}>


This is to show striked instances in the tooltip:

leog · 2022-10-06T15:19:07Z

-                        recurringCount: booking.recurringBookings.length,
+                        recurringCount: recurringDates.filter((date) => {
+                          return date >= now;
+                        }).length,


This is to just show the number of "active" instances in the tooltip trigger:

leog · 2022-10-06T15:21:39Z

+  }).all();
+  const dateStrings = allDates.map((r) => {
+    return processDate(dayjs(r).tz(timeZone), i18n);
+  });


This implementation does not require the list of bookings anymore, which was flickering along the infinite scroll loaded more results. It calculates all the instances based on the new introduced recurring information coming from tRPC viewer.bookings which comes with actual count of recurring events selected by user (a.k.a. recurring count) and the date of the first instance of the recurring booking

leog · 2022-10-06T15:22:34Z

                                key={booking.id}
                                listingStatus={status}
-                                {...defineRecurrentBookings(booking, shownBookings)}
+                                recurringBookings={page.recurringInfo}


Repeating last comment: Not relying on a list of bookings anymore, which was flickering along the infinite scroll loaded more results. It calculates all the instances based on the new introduced recurring information coming from tRPC viewer.bookings which comes with actual count of recurring events selected by user (a.k.a. recurring count) and the date of the first instance of the recurring booking

leog · 2022-10-06T15:23:04Z

  const shownBookings: Record<string, BookingOutput[]> = {};
  const filterBookings = (booking: BookingOutput) => {
-    if (status === "recurring" || status === "cancelled") {
+    if (status === "recurring" || status == "unconfirmed" || status === "cancelled") {


Unconfirmed tab needs to behave the same as recurring and cancelled when it comes to grouping recurring instances

leog · 2022-10-06T15:23:41Z

            startTime: dayjs().add(1, "day").toDate(),
            endTime: dayjs().add(1, "day").add(30, "minutes").toDate(),
-            status: BookingStatus.PENDING,
+            status: BookingStatus.ACCEPTED,


Yoga class does not require confirmation, so changing its status to ACCEPTED to be aligned to that.

leog · 2022-10-06T15:24:23Z

+            {
+              status: { equals: BookingStatus.PENDING },
+            },
+          ],


Behavior similar to the upcoming status but taking into consideration just PENDING bookings.

leog · 2022-10-06T15:26:19Z

+            notIn: [BookingStatus.CANCELLED],
+          },
+        },
+      });


This new prisma query comes up with the following useful information to populate recurring bookings explained in other related files.

[ { "_min": { "startTime": "2022-09-30T18:48:22.495Z" }, "_count": { "recurringEventId": 5 }, "recurringEventId": "dGVubmlzLWNsYXNz" }, { "_min": { "startTime": "2022-09-29T18:48:22.491Z" }, "_count": { "recurringEventId": 6 }, "recurringEventId": "eW9nYS1jbGFzcw==" }, { "_min": { "startTime": "2022-10-31T13:00:00.000Z" }, "_count": { "recurringEventId": 12 }, "recurringEventId": "341f50b8-7d71-4817-96b2-fe2144487d25" } ]

leog · 2022-10-06T15:27:23Z

-          seenBookings[booking.recurringEventId] = true;
-          return true;
-        });
-      }


This code was grouping recurring events before grouping them in the UI, which was causing problems showing not-correlated recurring bookings.

leog · 2022-10-06T15:28:03Z

-        });
-      }

+      const bookingsFetched = bookings.length;


Moved from above, not new

zomars · 2022-10-06T18:33:23Z

+      (booking.listingStatus === "recurring" ||
+        booking.listingStatus === "unconfirmed" ||
+        booking.listingStatus === "cancelled") && (


Should we use the conditions constants here?

Different context as you surely noticed, and it was just used once there, so I found no need to create new constants.

zomars · 2022-10-06T18:34:14Z

 ): [string[], Date[]] => {
-  const dateStrings = bookings.map((booking) => processDate(dayjs(booking.startTime).tz(timeZone), i18n));
-  const allDates = dateStrings.map((dateString) => dayjs(dateString).tz(timeZone).toDate());
+  const { count = 0, ...rest } =


Is unused variable intentional?

Yes, removing the count as we are not interested in it, I could have left it there and then let the redefinition overwrite it.

zomars · 2022-10-06T19:12:09Z

Suggested edit:

diff --git a/packages/trpc/server/routers/viewer.tsx b/packages/trpc/server/routers/viewer.tsx
index 2eaf991fa..70f0bee39 100644
--- a/packages/trpc/server/routers/viewer.tsx
+++ b/packages/trpc/server/routers/viewer.tsx
@@ -495,12 +495,8 @@ const loggedInViewerRouter = createProtectedRouter()
           endTime: { gte: new Date() },
           OR: [
             {
-              AND: [
-                { NOT: { recurringEventId: { equals: null } } },
-                {
-                  status: { equals: BookingStatus.PENDING },
-                },
-              ],
+              recurringEventId: { not: null },
+              status: { equals: BookingStatus.PENDING },
             },
             {
               status: { equals: BookingStatus.PENDING },

zomars · 2022-10-06T19:16:43Z

+            {
+              AND: [
+                { NOT: { recurringEventId: { equals: null } } },
+                {
+                  status: { equals: BookingStatus.PENDING },
+                },
+              ],
+            },


Suggested change

{

AND: [

{ NOT: { recurringEventId: { equals: null } } },

{

status: { equals: BookingStatus.PENDING },

},

],

},

{

recurringEventId: { not: null },

status: { equals: BookingStatus.PENDING },

},

We can simplify this

zomars

Left some nitpicks here and there but overall very nice improvement @leog
Feel free to merge as you see fit

leog · 2022-10-06T20:19:03Z

Thanks for the feedback @zomars. Here is the follow-up PR taking into consideration your comments: #4887

Fixing unconfirmed/recurring bookings

909fe36

leog added the 🐛 bug Something isn't working label Oct 6, 2022

leog added this to the v.2.1 milestone Oct 6, 2022

leog requested a review from a team October 6, 2022 14:00

leog self-assigned this Oct 6, 2022

Missing change from stash

1514e82

vercel Bot had a problem deploying to Preview October 6, 2022 14:29 Failure

Merge branch 'main' into fix/bookings-unconfirmed-recurring

c5128e2

vercel Bot had a problem deploying to Preview October 6, 2022 14:46 Failure

leog commented Oct 6, 2022

View reviewed changes

Merge branch 'main' into fix/bookings-unconfirmed-recurring

002d443

leog added the ♻️ autoupdate tells kodiak to keep this branch up-to-date label Oct 6, 2022

vercel Bot had a problem deploying to Preview October 6, 2022 15:34 Failure

PeerRich enabled auto-merge (squash) October 6, 2022 16:33

kodiakhq Bot added 2 commits October 6, 2022 18:16

Merge branch 'main' into fix/bookings-unconfirmed-recurring

397714e

Merge branch 'main' into fix/bookings-unconfirmed-recurring

3af5760

vercel Bot deployed to Preview October 6, 2022 18:28 View deployment

zomars reviewed Oct 6, 2022

View reviewed changes

Comment thread apps/web/components/booking/BookingListItem.tsx

zomars reviewed Oct 6, 2022

View reviewed changes

zomars approved these changes Oct 6, 2022

View reviewed changes

PeerRich merged commit 4e49d32 into main Oct 6, 2022

PeerRich deleted the fix/bookings-unconfirmed-recurring branch October 6, 2022 19:23

leog mentioned this pull request Oct 6, 2022

Applying feedback from a merged PR #4887

Merged

zomars restored the fix/bookings-unconfirmed-recurring branch October 7, 2022 19:46

keithwillcode deleted the fix/bookings-unconfirmed-recurring branch September 17, 2025 11:19

Conversation

leog commented Oct 6, 2022

What does this PR do?

Type of change

How should this be tested?

Uh oh!

vercel Bot commented Oct 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zomars commented Oct 6, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zomars left a comment

Choose a reason for hiding this comment

Uh oh!

leog commented Oct 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vercel Bot commented Oct 6, 2022 •

edited

Loading