-
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: add the X-Cal-Signature-256 header to MEETING_ENDED webhook requests #13986
fix: add the X-Cal-Signature-256 header to MEETING_ENDED webhook requests #13986
Conversation
@swain 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. |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (03/05/24)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (03/05/24)1 label was added to this PR based on Keith Williams's automation. |
📦 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! 🙌 |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (03/05/24)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (03/05/24)1 label 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.
Added a small change to handle webhooks that don't have an appId, other than that it looks good 🙏
// Fetch the webhook configuration so that we can get the secret. | ||
const [appId, subscriberId] = job.jobName.split("_"); | ||
const webhook = await prisma.webhook.findUniqueOrThrow({ | ||
where: { id: subscriberId, appId: appId !== "null" ? appId : null }, |
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.
webhooks without an appId have a job name looking like that null_12341234
Thanks for the quick response @CarinaWolli! |
What does this PR do?
Fixes #13985
Requirement/Documentation
Type of change
How should this be tested?
I tested this change by running locally to validate that the header is sent correctly.
As far as I can tell, there are no existing test cases that validate this functionality (unit or end-to-end). I'm afraid I'm not familiar enough with how this project works to be able to bootstrap a new test suite for this logic.
If a reviewer wants to offer guidance to that end, I'd be happy to put something together.
Mandatory Tasks
Checklist