-
Notifications
You must be signed in to change notification settings - Fork 6.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: meeting url variable in workflow notifitcations #12308
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Ignored Deployments
|
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link to collect XP and win prizes! |
📦 Next.js Bundle Analysis for @calcom/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
No failed tests 🎉 |
} | ||
} | ||
|
||
await sendScheduledEmails( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure confirmation emails are not sent when disabled because of an existing workflow
@@ -169,6 +213,7 @@ export async function handleConfirmation(args: { | |||
references: { | |||
create: scheduleResult.referencesToCreate, | |||
}, | |||
metadata: { ...(typeof booking.metadata === "object" ? booking.metadata : {}), videoCallUrl }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bookings that require confirmation never had the videoCallUrl added to metadata
@@ -218,8 +263,6 @@ export async function handleConfirmation(args: { | |||
const eventTypeSlug = updatedBookings[index].eventType?.slug || ""; | |||
|
|||
const isFirstBooking = index === 0; | |||
const videoCallUrl = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original booking didn't have videoCallUrl in metadata, that's why it was missing in the workflow emails that trigger when a meeting was confirmed
@@ -2524,7 +2524,7 @@ async function handler( | |||
...evt, | |||
...{ metadata: metadataFromEvent, eventType: { slug: eventType.slug } }, | |||
}, | |||
isNotConfirmed: evt.requiresConfirmation || false, | |||
isNotConfirmed: !isConfirmedByDefault, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also includes bookings with payments not just event types that require confirmation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but a add-on is needed soon to add tests to ensure emails aren't send when e.g. host confirmation emails are disabled.
Co-authored-by: CarinaWolli <wollencarina@gmail.com>
Co-authored-by: CarinaWolli <wollencarina@gmail.com>
Co-authored-by: CarinaWolli <wollencarina@gmail.com>
What does this PR do?
Fixes that the {MEETING_URL} from custom workflow templates wasn't populated for event types that require confirmation or payment.
Also fixes that confirmation emails were still sent after they were confirmed even when the default confirmation emails were disabled.
Fixes #10758
Requirement/Documentation
Have Sendgrid set up to send workflow emails.
Type of change
How should this be tested?
Mandatory Tasks