ref(workflow_engine): Dual-write NotifyEventAction to be handled by webhook handler#119121
Merged
Christinarlong merged 4 commits intoJul 7, 2026
Merged
Conversation
The legacy NotifyEventAction issue-alert action currently dual-writes a workflow-engine PLUGIN action, whose sole live delivery path fires legacy project webhooks via send_legacy_webhooks_for_invocation. The identical delivery already happens through the WEBHOOK action handler when target_identifier == "webhooks" (dual-written from NotifyEventServiceAction), so PLUGIN is a duplicate tied to the now-no-op NotifyEventAction. Repoint the NotifyEventAction translator to emit a WEBHOOK action targeting the legacy webhooks service instead of PLUGIN. NotifyEventActionTranslator now subclasses WebhookActionTranslator, dropping the service required-field and returning the literal "webhooks" target_identifier so delivery routes through WebhookActionHandler._handle_legacy_webhooks. Runtime delivery stays gated by send_legacy_webhooks_for_invocation (webhooks:enabled/urls), unchanged from the PLUGIN path. No new type='plugin' rows are created via the dual-write. The PLUGIN enum and handlers are intentionally kept; they are removed in a later PR after the existing rows are migrated. Co-Authored-By: Claude <noreply@anthropic.com>
github-actions
Bot
added
the
Scope: Backend
Christinarlong
changed the title
…bhook config
Only dual-write the repointed WEBHOOK("webhooks") action for a NotifyEventAction rule
when the project has the legacy webhook enabled. On projects without it enabled, the
action delivers nothing, so skip it rather than persist a dead webhook action -- the
workflow is left as a valid "No actions" automation.
Threads the rule's project through the dual-write path
(translate_rule_data_actions_to_notification_actions /
build_notification_actions_from_rule_data_actions / create_workflow_actions and the four
call sites) and adds is_legacy_webhook_enabled(project), gating on webhooks:enabled.
Also fixes the dual-read serializer parity checks: NotifyEventAction now reverse-serializes
to the equivalent NotifyEventServiceAction(service="webhooks"), which is an intentional
divergence, and repoints the test-fire mock to the webhook handler.
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
The test-notification endpoint translates each action blob individually and then does actions[0]. Now that NotifyEventAction is gated on the project's legacy webhook config, translating it on a webhooks-disabled project returns an empty list, so actions[0] raised IndexError -- surfaced to the caller as a 400 'An unexpected error occurred'. Skip the blob when it translates to nothing, restoring the prior clean result (nothing to deliver, no error). Co-Authored-By: Claude <noreply@anthropic.com>
Christinarlong
commented
Jul 7, 2026
Comment on lines
+41
to
+43
| # TODO: After the script migrates/removes all plugin actions, delete this helper and the | ||
| # `project` threading through the dual-write path. | ||
| return bool(ProjectOption.objects.get_value(project, "webhooks:enabled", default=False)) |
Contributor
Author
There was a problem hiding this comment.
This is the helper/crux for passing down the project throughout all the dual write layers, since we only want to write a webhook action if we know it'll work/the person has legacy webhooks enabled. Otherwise the state makes no sense. Once the migration is done (i.e the plugin action rows are either deleted or migrated to webhook ones) we can delete this helper and the project passing
ceorourke
approved these changes
Jul 7, 2026
saponifi3d
approved these changes
Jul 7, 2026
saponifi3d
left a comment
saponifi3d
left a comment
Contributor
There was a problem hiding this comment.
lgtm - thanks for chatting a bit in slack about how this is covering the legacy API use cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR mainly changes the dual write flow such that when we see a write operation to the NotifyEventAction (ActionType.PLUGIN) we instead create a webhook action. Otherwise the write operation still saves the Rule but no workflow Action is saved.
We added a new translator for NotifyEventAction -> webhooks action. AND we're passing through the project through the dual write layers so we can check if an enabled ProjectOption exists (i.e they have the legacy Webhooks enabled) then make a webhook action. This checking is a little jank because we need to pass the project down a bunch of layers (but otherwise idk how we can get project). But it's temporary for this migration, because for the migration script I want to delete all actions that are plugins and don't have an enabled webhooks config for a clean state.
PR 1 of the effort to remove
Action.Type.PLUGINfrom the workflow engine