Skip to content

Recurring event improvements#2750

Merged
leog merged 117 commits intomainfrom
leog/recurring-event-improvements
Jun 10, 2022
Merged

Recurring event improvements#2750
leog merged 117 commits intomainfrom
leog/recurring-event-improvements

Conversation

@leog
Copy link
Contributor

@leog leog commented May 12, 2022

What does this PR do?

Thanks @hariombalhara for your feedback. Here are the points you mentioned to check one by one.

  • We should have a loader on Confirm button as recurring event takes long time to book as there are many requests being sent. FIXED
  • Rejected Recurring Booking still shows up in Upcoming. Non recurring booking if cancelled or rejected doesn't show up. It shows up in cancelled tab. FIXED
  • A recurring booking done for same day doesn't show up in upcoming tab. Though it does come up in Recurring Tab. Tested a non recurring booking and it shows up fine in upcoming tab. FIXED
  • "Rejecting All" recurring bookings doesn't close the popup. FIXED
  • Cancel all remaining events from a confirmed recurring event from recurring tab
  • Showing "X events remaining" from a confirmed recurring event from recurring tab
  • "Every X weeks for Y weeks" was revisited and changed to "Every X weeks for Y occurrences"

Mouse_Highlight_Overlay

Type of change

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

How should this be tested?

  • Create recurring events and see them show up in Upcoming or Recurring depending on confirmation status. Rejecting a whole series from Upcoming.

Loom: https://www.loom.com/share/bb3416c289884fc3905a78bca11d8a09

@leog leog requested review from hariombalhara and zomars May 12, 2022 14:35
@vercel
Copy link

vercel bot commented May 12, 2022

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

Name Status Preview Updated
cal-dev ❌ Failed (Inspect) Jun 10, 2022 at 8:27PM (UTC)
4 Ignored Deployments
Name Status Preview Updated
cal ⬜️ Ignored (Inspect) Jun 10, 2022 at 8:27PM (UTC)
docs ⬜️ Ignored (Inspect) Jun 10, 2022 at 8:27PM (UTC)
swagger ⬜️ Ignored (Inspect) Jun 10, 2022 at 8:27PM (UTC)
ui ⬜️ Ignored (Inspect) Jun 10, 2022 at 8:27PM (UTC)

@vercel vercel bot temporarily deployed to Preview – cal-dev May 12, 2022 14:37 Inactive
@vercel vercel bot temporarily deployed to Preview – cal-dev May 12, 2022 18:11 Inactive
zomars
zomars previously requested changes May 12, 2022
Copy link
Contributor

@zomars zomars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some observations:

  • With these changes there's currently no way to "Cancel All" bookings.
  • Since we only display the "Next upcoming recurring booking", we need to cancel it, refresh, cancel the next one, refresh, cancel.... etc.
  • It is unexpected that the individual bookings aren't appearing in "Upcoming"

So my two cents would be:

  • "Upcoming" should display ALL upcoming events regardless of type (recurring, single, paid, etc.)
  • Recurring should display all grouped recurring booking and maybe how many recurrences has left (ex: 5 repeats left...)

@vercel vercel bot temporarily deployed to Preview – cal-dev May 13, 2022 12:13 Inactive
@vercel vercel bot requested a deployment to Preview – cal May 13, 2022 12:16 Abandoned
@ciaranha
Copy link
Member

ciaranha commented May 16, 2022

@zomars I agree with your your two cents above. I will update the designs accordingly.

With regards to "Cancelling All" the idea was to have it so that you could do This & all following like in calendar apps but if we use the recurring events tabs to show them grouped we can probably have a cancel all remaining anyway.
CleanShot 2022-05-16 at 16 07 53@2x

@ciaranha ciaranha added this to the v.1.7 milestone May 16, 2022
@ciaranha ciaranha added the 🎨 needs design Before engineering kick-off, a designer needs to submit a mockup label May 16, 2022
@vercel vercel bot temporarily deployed to Preview – cal-dev May 17, 2022 16:31 Inactive
@leog leog added the ♻️ autoupdate tells kodiak to keep this branch up-to-date label May 22, 2022
import { useLocale } from "@calcom/lib/hooks/useLocale";
import showToast from "@calcom/lib/notification";
import { Frequency } from "@calcom/prisma/zod-utils";
import { getEveryFreqFor } from "@calcom/lib/recurringStrings";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A repeated string for recurring events was refactored into its own util

* the "Recurring" tab, to support confirming discretionally in the "Recurring" tab.
*/
if (booking.listingStatus === "upcoming" && booking.recurringEventId !== null) {
if (booking.listingStatus === "recurring" && booking.recurringEventId !== null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is now a behavior desired for the recurring tab, not upcoming

];

if (booking.listingStatus === "recurring" && booking.recurringEventId !== null) {
bookedActions = bookedActions.filter((action) => action.id !== "edit_booking");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case we are dealing with a recurring booking, no edition available

if (booking.status === BookingStatus.PENDING) {
// Only take into consideration next up instances if booking is confirmed
recurringDates = recurringDates.filter((aDate) => aDate >= today);
recurringStrings = recurringDates.map((_, key) => recurringStrings[key]);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to support the "X remaining events" copy in confirmed recurring events showing in the recurring tab

{booking.recurringCount &&
booking.eventType?.recurringEvent?.freq &&
booking.listingStatus === "upcoming" && (
(booking.listingStatus === "recurring" || booking.listingStatus === "cancelled") && (
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to show recurring information both in recurring and cancelled tabs


const shownBookings: Record<string, boolean> = {};
const filterBookings = (booking: BookingOutput) => {
if (status === "recurring" || status === "cancelled") {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to group recurring event instances into just one for cancelled and recurring tabs

@leog leog marked this pull request as ready for review June 10, 2022 19:05
@leog leog requested review from a team and zomars June 10, 2022 19:05
Copy link
Contributor

@zomars zomars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @leog 🙏🏽

id: "reject",
label:
booking.listingStatus === "upcoming" && booking.recurringEventId !== null
booking.listingStatus === "recurring" && booking.recurringEventId !== null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's extract this repeated condition to isRecurring and use it instead

@leog leog merged commit 514fca7 into main Jun 10, 2022
@leog leog deleted the leog/recurring-event-improvements branch June 10, 2022 20:38
@leog
Copy link
Contributor Author

leog commented Jun 10, 2022

Sorry @zomars, saw your approval and jump right into merging, it's been a long-lasting PR 😅 Will tackle refactoring that repetitive condition in a separate PR. Would it be cool to add that check in packages/lib/isRecurringEvent.ts?

@zomars
Copy link
Contributor

zomars commented Jun 11, 2022

Sorry @zomars, saw your approval and jump right into merging, it's been a long-lasting PR 😅 Will tackle refactoring that repetitive condition in a separate PR. Would it be cool to add that check in packages/lib/isRecurringEvent.ts?

It's ok for now

@zomars zomars mentioned this pull request Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

♻️ autoupdate tells kodiak to keep this branch up-to-date

Projects

No open projects
Status: In Progress

Development

Successfully merging this pull request may close these issues.

5 participants