feat: add utcOffset in webhook payload#12709
Conversation
|
@jkcs is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link. |
📦 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! 🙌 |
CarinaWolli
left a comment
There was a problem hiding this comment.
Thank you for contributing 🙏 Left you one comment, everything else looks good
packages/lib/date-fns/index.ts
Outdated
| if (!timeZone) return null; | ||
|
|
||
| if (timeZoneWithDST(timeZone)) { | ||
| return getUTCOffsetInDST(timeZone); |
There was a problem hiding this comment.
Wouldn't this now always return DST offset of this timezone, even when we are not in DST like now in December?
There was a problem hiding this comment.
Wow, you reminded me so well. I was overthinking it. This is not necessary.
packages/lib/date-fns/index.ts
Outdated
| export function getUTCOffsetByTimezone(timeZone: string) { | ||
| if (!timeZone) return null; | ||
|
|
||
| return dayjs().tz(timeZone).utcOffset(); |
There was a problem hiding this comment.
But we still do need to take care of DST here.
This should work if we add another parameter to the function for the date:
dayjs(date).tz(timeZone).utcOffset()
|
Hello, @Shpadoinkle . I suggest that you open a new issue. The changes in this pull request only include the modification mentioned in #12415 "Add the attendee UTC offset as a field, rather than the string of the timezone." As for the issue you mentioned, I think it's worth addressing as well. |
|
@jkcs ah ignore me all together, the timeZone string was already on the webhook |
|
This PR is being marked as stale due to inactivity. |
| data: Omit<WebhookDataType, "createdAt" | "triggerEvent"> | ||
| ): WithUTCOffsetType<WebhookDataType> { | ||
| if (data.organizer?.timeZone) { | ||
| (data.organizer as Person & UTCOffset).utcOffset = getUTCOffsetByTimezone(data.organizer.timeZone); |
There was a problem hiding this comment.
we are missing the date parameter here (data.startTime), otherwise, we will always get the utc offset from the current date (important for DST)
|
|
||
| if (data?.attendees?.length) { | ||
| (data.attendees as (Person & UTCOffset)[]).forEach((attendee) => { | ||
| attendee.utcOffset = getUTCOffsetByTimezone(attendee.timeZone); |
CarinaWolli
left a comment
There was a problem hiding this comment.
Looks good now 👍 Thank you for your contribution 🙏
What does this PR do?
Fixes #12415
/claim #12415
Requirement/Documentation
Type of change
Mandatory Tasks