Skip to content

feat: add paid webhook#8721

Merged
keithwillcode merged 22 commits into
calcom:mainfrom
wesleymatosdev:feature/#7299/add-paid-webhook
Jun 13, 2023
Merged

feat: add paid webhook#8721
keithwillcode merged 22 commits into
calcom:mainfrom
wesleymatosdev:feature/#7299/add-paid-webhook

Conversation

@wesleymatosdev
Copy link
Copy Markdown
Contributor

@wesleymatosdev wesleymatosdev commented May 6, 2023

  • feat: add BOOKING_PAID on enums
  • feat: trigger the BOOKING_PAID webhook

What does this PR do?

Adds the BOOKING_PAID webhook from #7299

edit: Also creates BOOKING_REQUESTED and BOOKING_REJECTED on the database

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How should this be tested?

  • We need to ensure that the webhook is really being triggered when the charge of the payment occurs

Checklist

  • I haven't checked if my PR needs changes to the documentation
  • I haven't added tests that prove my fix is effective or that my feature works

@wesleymatosdev wesleymatosdev requested a review from zomars as a code owner May 6, 2023 07:05
@wesleymatosdev wesleymatosdev requested a review from a team May 6, 2023 07:05
@vercel
Copy link
Copy Markdown

vercel Bot commented May 6, 2023

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

Name Status Preview Comments Updated (UTC)
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 9, 2023 10:44pm

@vercel
Copy link
Copy Markdown

vercel Bot commented May 6, 2023

@ologbonowiwi is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2023

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

Copy link
Copy Markdown
Member

@alishaz-polymath alishaz-polymath left a comment

Choose a reason for hiding this comment

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

Two major things:

  1. It's missing a migration. There's change in the DB (prisma/schema.prisma) so it needs to have a migration as well, and please ensure that the migration doesn't destroy the existing webhooks.
  2. Ideally we'd want to have as little migrations as possible, so please add the BOOKING_REQUESTED and BOOKING_REJECTED in the same migration, hence the respective change in constants.ts and schema.prisma.
    We can have them function with a follow up PR, but we definitely want them in the DB in one go, to minimize subsequent migrations
    Thanks for the PR 🙌

@github-actions github-actions Bot added the ❗️ migrations contains migration files label May 13, 2023
@wesleymatosdev
Copy link
Copy Markdown
Contributor Author

All webhooks added.

Can you review it again, @alishaz-polymath?

@alannnc alannnc self-requested a review May 19, 2023 23:00
@PeerRich PeerRich changed the title feature/#7299/add paid webhook feat: add paid webhook May 24, 2023
@PeerRich PeerRich added Medium priority Created by Linear-GitHub Sync ✨ feature New feature or request labels May 25, 2023
@wesleymatosdev
Copy link
Copy Markdown
Contributor Author

The original issue was close, will anyone review this? @alishaz-polymath @PeerRich

Copy link
Copy Markdown
Member

@alishaz-polymath alishaz-polymath left a comment

Choose a reason for hiding this comment

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

Please make sure to accommodate changes made in the PR #8884 so as to not repeat the same modifications and duplicate them. 🙏

Comment thread packages/features/webhooks/lib/constants.ts Outdated
Comment thread packages/prisma/schema.prisma Outdated
Comment thread packages/prisma/migrations/20230513235419_add_booking_webhooks/migration.sql Outdated
wesleymatosdev and others added 3 commits May 30, 2023 22:47
Co-authored-by: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com>
Co-authored-by: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com>
…/migration.sql

Co-authored-by: Syed Ali Shahbaz <52925846+alishaz-polymath@users.noreply.github.com>
@wesleymatosdev
Copy link
Copy Markdown
Contributor Author

Changes applied, thanks @alishaz-polymath

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

@wesleymatosdev
Copy link
Copy Markdown
Contributor Author

this will be merged?

changes already had been addressed @PeerRich @alishaz-polymath @zomars

Copy link
Copy Markdown
Member

@alishaz-polymath alishaz-polymath left a comment

Choose a reason for hiding this comment

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

Let's go! 🚀
Thanks a lot for your contribution 🙏

@keithwillcode keithwillcode added this pull request to the merge queue Jun 13, 2023
Merged via the queue into calcom:main with commit 895b3ca Jun 13, 2023
@wesleymatosdev wesleymatosdev deleted the feature/#7299/add-paid-webhook branch June 17, 2023 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ feature New feature or request Medium priority Created by Linear-GitHub Sync ❗️ migrations contains migration files

Projects

No open projects
Status: Medium priority

Development

Successfully merging this pull request may close these issues.

6 participants