-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
feat(Stripe Trigger Node): Add Stripe webhook descriptions based on the workflow ID and name #9956
Conversation
…he workflow ID and name
Hey @JavierCane I really like this idea, the version of the node doesn't need increasing where you have done it so that can be reverted. As workflow names can contain any character including emojis does this cause any issues on the Stripe side? It may be safer to just use the workflow ID which solves the issue, what do you think? |
Hi, would it make it easy to merge this PR if I remove the workflow name from the Stripe Webhook description? Thanks! |
Hey @JavierCane, This is waiting for my next PR review session, I typically look at PRs twice a week. I think I would be more comfortable with just the ID which solves the same problem with less risk. |
…bhook descriptions Reduce risks (#9956 (comment))
No problem then @Joffcom, workflow name removed 🤟 |
👍🏻 , I will add this to the review list to test this week. |
Got released with |
* master: refactor(core): Centralize scaling mode (no-changelog) (#9835) fix(editor): Remove body padding from storybook previews (no-changelog) (#10317) feat(MySQL Node): Return decimal types as numbers (#10313) 🚀 Release 1.54.0 (#10315) feat(Elasticsearch Node): Add bulk operations for Elasticsearch (#9940) feat(Stripe Trigger Node): Add Stripe webhook descriptions based on the workflow ID and name (#9956) feat(MongoDB Node): Add projection to query options on Find (#9972) fix(Invoice Ninja Node): Fix payment types (#10196) feat(HTTP Request Tool Node): Use DynamicStructuredTool with models supporting it (no-changelog) (#10246) feat: Return scopes on executions (no-changelog) (#10310) feat(Webflow Node): Update to use the v2 API (#9996) feat(Lemlist Trigger Node): Update Trigger events (#10311) feat(Calendly Trigger Node): Update event names (no-changelog) (#10129) refactor(core): Reorganize webhook related components under src/webhooks (no-changelog) (#10296) docs: Fix links to license files in readme (no-changelog) (#10257) fix(editor): Update design system Avatar component to show initials also when only firstName or lastName is given (#10308) fix(editor): Update tags filter/editor to not show non existing tag as a selectable option (#10297) fix(editor): Update project tabs test (no-changelog) (#10300) fix(core): VM2 sandbox should not throw on `new Promise` (#10298) # Conflicts: # packages/design-system/src/components/N8nAvatar/Avatar.vue
Summary
Right now n8n creates the needed Stripe Webhooks while adding Stripe Trigger nodes without specifying a description for that Stripe Webhook.
That is, you can end up having a list of Stripe Webhooks which do not reveal any kind of contextual information. In case of receiving an errors spike alert from Stripe, which demands a quick response, you do not have any more information rather than the webhook URL where it points to:
With this PR n8n will start adding the following description to these webhooks:
This allows to have a better idea of the implications of an error spike in webhook calls, or just knowing why you have N declared Stripe Webhooks and where they come from.
Take into account that, even if you manually add the Stripe Webhook description from the Stripe Workbench, it will be removed while updating n8n because it de-register the webhooks and create them again. So this is the only approach I can think of in order to maintain the webhook descriptions in place 😊
Review / Merge checklist
release/backport
(if the PR is an urgent fix that needs to be backported)