Skip to content

Apps Status + Updates to non-traditional calendars#5034

Merged
leog merged 20 commits intomainfrom
feat/apps-status
Oct 19, 2022
Merged

Apps Status + Updates to non-traditional calendars#5034
leog merged 20 commits intomainfrom
feat/apps-status

Conversation

@leog
Copy link
Copy Markdown
Contributor

@leog leog commented Oct 16, 2022

What does this PR do?

Implements Apps Status on emails for organizers. It also fixes updating meetings when rescheduling for non-traditional calendars. Recurring events get a (xNumber) alongside the status for each app to let the user know if every booking was successfully created. If any of them fails, it shows a cross emoji.

Some screenshots:

Environment: Staging(main branch) / Production

Type of change

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

How should this be tested?

Book a regular or recurring event and see as the organizer an email sent to you with the apps status.

@vercel
Copy link
Copy Markdown

vercel Bot commented Oct 16, 2022

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

Name Status Preview Updated
cal ✅ Ready (Inspect) Visit Preview Oct 19, 2022 at 4:09PM (UTC)

@leog leog self-assigned this Oct 17, 2022
@leog leog requested a review from a team October 17, 2022 20:40
@leog leog marked this pull request as ready for review October 17, 2022 20:41
Copy link
Copy Markdown
Contributor Author

@leog leog left a comment

Choose a reason for hiding this comment

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

Self-review done

const createdBookings: BookingResponse[] = [];
// Reversing to accumulate results for noEmail instances first, to then lastly, create the
// emailed booking taking into account accumulated results to send app status accurately
for (let key = 0; key < data.length; key++) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As said in the comment, in order to have the apps status for every recurring event booking created to be included in the single email sent to attendee and organizer, we need to reverse the order to the booking creation. The last booking, the one that enables sending the email is the first recurring event instance, and will include its apps status to the rest of the results.

// If multiple events are created, subsequent events may fail due to
// contact was created by previous event creation, so we introduce a
// fallback taking advantage of the error message providing the contact id
if (error.body.message.includes("Contact already exists. Existing ID:")) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As said in the comment, recurring events will look to the same person which may not be created by the first booking creation but will be created by the second, so we just make sure we catch the proper error from HubSpot to take into account the returned user id to let the flow continue without failing.


type createdEventSchema = z.infer<typeof createdEventSchema>;

export type ExtendedCredential = Credential & { appName: string };
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We need the app name belonging to the credential to use it in the email, so an ExtendedCredential type was created.

}
}

// Taking care of non-traditional calendar integrations
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When a booking was rescheduled, the updated booking was just being updated in the designated calendar, skipping the non-traditional calendars such as HubSpot or Close.com. This takes care of that, making sure to pass down the BookingReference booking ID, not the Cal Booking ID, so the apps can find the booking on their system.

Comment thread packages/emails/email-manager.ts Outdated
Comment thread packages/emails/email-manager.ts Outdated
new Promise((resolve, reject) => {
try {
const requestRescheduleEmail = new AttendeeWasRequestedToRescheduleEmail(calEvent, metadata);
const requestRescheduleEmail = new AttendeeWasRequestedToRescheduleEmail(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Attendees shouldn't get organizers' apps status info in their email, removing it forcefully.

Comment thread packages/emails/email-manager.ts Outdated
return new Promise((resolve, reject) => {
try {
const scheduledEmail = new AttendeeLocationChangeEmail(calEvent, attendee);
const scheduledEmail = new AttendeeLocationChangeEmail(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Attendees shouldn't get organizers' apps status info in their email, removing it forcefully.


import { Info } from "./Info";

export const AppsStatus = (props: { calEvent: CalendarEvent; t: TFunction }) => {
Copy link
Copy Markdown
Contributor Author

@leog leog Oct 18, 2022

Choose a reason for hiding this comment

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

Component to be used in the email to include Apps Status.
It will show:

  • For single booking: ✅ or ❌ for each app
  • For recurring booking (for example 6): ✅ (x6) or ✅ (x5) ❌ (x1) or any other combination, for each app

const credentials = await refreshCredentials(organizerUser.credentials);
const eventManager = new EventManager({ ...organizerUser, credentials });

function handleAppsStatus(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This function makes sure it accumulates the resulting apps status coming from the request with the one created last, all in case of a recurring event.

return customInputsString;
};

export const getAppsStatus = (calEvent: CalendarEvent) => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same thing as the HTML version but in plain text

Copy link
Copy Markdown
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.

Very nice addition @leog left some feedback to address first 🙏🏽

"link_shared": "Link shared!",
"title": "Title",
"description": "Description",
"apps_status": "Apps Status",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would "Apps results" make more sense in this context?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We need you @Jaibles!

Comment thread packages/emails/email-manager.ts Outdated
Comment thread packages/features/bookings/lib/handleNewBooking.ts Outdated
Comment thread packages/features/bookings/lib/handleNewBooking.ts Outdated
metadata.hangoutLink = updatedEvent.hangoutLink;
metadata.conferenceData = updatedEvent.conferenceData;
metadata.entryPoints = updatedEvent.entryPoints;
handleAppsStatus(results, booking);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

Co-authored-by: Omar López <zomars@me.com>
Comment thread packages/emails/src/templates/BaseScheduledEmail.tsx
Comment thread packages/emails/src/templates/OrganizerScheduledEmail.tsx Outdated
Copy link
Copy Markdown
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.

LGTM

@leog leog merged commit b6be94f into main Oct 19, 2022
@leog leog deleted the feat/apps-status branch October 19, 2022 16:11
haffla pushed a commit to tourlane/cal.com that referenced this pull request Nov 22, 2022
* Apps Status + Updates to non-traditional calendars

* Recurring booking tweaks

* Last tweaks

* Fixing checks

* Fixing eslint

* Reverting unneeded changes, using plain text email

* More unneeded changes revert

* Update packages/features/bookings/lib/handleNewBooking.ts

Co-authored-by: Omar López <zomars@me.com>

* Fixing lint

* Refactored appsStatus in emails

* Update packages/emails/src/templates/OrganizerScheduledEmail.tsx

Co-authored-by: Omar López <zomars@me.com>
haffla pushed a commit to tourlane/cal.com that referenced this pull request Nov 22, 2022
* Apps Status + Updates to non-traditional calendars

* Recurring booking tweaks

* Last tweaks

* Fixing checks

* Fixing eslint

* Reverting unneeded changes, using plain text email

* More unneeded changes revert

* Update packages/features/bookings/lib/handleNewBooking.ts

Co-authored-by: Omar López <zomars@me.com>

* Fixing lint

* Refactored appsStatus in emails

* Update packages/emails/src/templates/OrganizerScheduledEmail.tsx

Co-authored-by: Omar López <zomars@me.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants