-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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: undefined in meeting url workflow variable #16771
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (09/23/24)1 reviewer was added to this PR based on Keith Williams's automation. |
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.
I noticed that for workflows the event is typed as BookingInfo
. Should we add the bookerUrl
as a property so we ensure we're always handling org domains?
Found in packages\features\ee\workflows\lib\reminders\smsReminderManager.ts
line 37
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.
Feedback addressed.
Just a heads up that we don't have to do Omit<CalendarEvent, "bookerUrl">
if we're declaring bookerUrl
in ExtendedCalendarEvent
@@ -2,7 +2,7 @@ import type { Prisma } from "@prisma/client"; | |||
import type z from "zod"; | |||
|
|||
import type { Workflow } from "@calcom/features/ee/workflows/lib/types"; | |||
import type { AppsStatus } from "@calcom/types/Calendar"; | |||
import type { AppsStatus, CalendarEvent } from "@calcom/types/Calendar"; |
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.
CalendarEvent
wasn't imported, so evt
was treated as any
instead. Importing it now caused some other type errors that needed to be fixed
E2E results are ready! |
@joeauyeung Fixed the merge conflicts, can you re-review? |
What does this PR do?
Fixes issue that {MEETING_URL} workflow variable sometimes shows undefined in url. Like that: undefined/reschedule/...