Skip to content
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

Enhance webhook integration UX #2488

Merged
merged 6 commits into from
Nov 26, 2020
Merged

Enhance webhook integration UX #2488

merged 6 commits into from
Nov 26, 2020

Conversation

Twixes
Copy link
Collaborator

@Twixes Twixes commented Nov 24, 2020

Changes

Extracting some out-of-scope improvements from #2111, this modernizes and improves webhook integration UX – closes PostHog/posthog.com#564 and should also resolve #2203.
Also renames Team field slack_incoming_webhook to more general incoming_webhook.

Success: enabled

Errrror

Success: disabled

Checklist

  • Django backend tests (if this PR affects the backend).
  • Cypress end-to-end tests (if this PR affects the frontend).

@timgl timgl temporarily deployed to posthog-enhance-webhook-uhu6mn November 24, 2020 05:28 Inactive
@mariusandra
Copy link
Collaborator

mariusandra commented Nov 24, 2020

I think this is a solid refactor, however I do have to add (and I feel I'm repeating myself), that any alter/change/delete migration will result in downtime for users. Same here. We're in a situation where thousands of users of varying scale depend on posthog and we must minimise disruptions to service of any of them.

I know this feels dirty, but the best way forward here is either 1) not rename the field in the db and perhaps even wait until we move webhooks into plugins, 2) create a new field, incoming_webhook, copy over all the data and then just deprecate the old field without removing it.

Eventually when we release posthog 2.0, we can remove all the deprecations.

@Twixes Twixes temporarily deployed to posthog-enhance-webhook-uhu6mn November 24, 2020 13:39 Inactive
@Twixes
Copy link
Collaborator Author

Twixes commented Nov 24, 2020

Hm, yep, that's true, that field does also appear in tasks, so I removed the migration – it was an unimportant change anyway.

@timgl timgl merged commit 37243ee into master Nov 26, 2020
@timgl timgl deleted the enhance-webhook branch November 26, 2020 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slack/Microsoft Teams Integration UX is a little confusing Add Discord as another webhook integration
3 participants