-
Notifications
You must be signed in to change notification settings - Fork 243
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
Add oauth2 for webhook2.0 #2334
Conversation
hi @cyberlord29 please ping me directly when this PR is ready for review. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2334 +/- ##
===========================================
+ Coverage 33.18% 78.98% +45.79%
===========================================
Files 14 1210 +1196
Lines 693 21954 +21261
Branches 109 4448 +4339
===========================================
+ Hits 230 17340 +17110
- Misses 463 3542 +3079
- Partials 0 1072 +1072 ☔ View full report in Codecov by Sentry. |
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.
Overall, the changes look good to me. Left couple of questions.
We need to ensure that actions-core changes don't break any existing behaviour. A hadron regression run can be helpful. I can help you do that tomorrow.
packages/destination-actions/src/destinations/webhook-extensible/index.ts
Show resolved
Hide resolved
hi @sayan-das-in - would you be able to review the core change in this PR please? I'm not concerned about the Destination specific code so you can ignore that - but there is a core change in Destination Kit which I'd like someone experienced to review. @cyberlord29 is keen to get this out today if possible. edit: I can see that the folks in India are OOO today. So unfortunately this PR will have to wait. |
I've asked @brennan to include this PR in the same deploy as this other core change (so that it can be monitored more closely) |
For Webhooks 2.0 which is an extensible destination, more like a generic webhook that has OAuth2 and custom settings that are dynamic.
This PR adds Oauth 2 for the new destination
webhook-extensible
and an HTTP send action.SDD
Testing
Added unit tests and ran a regression test