-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
chore(aci): migration to fix fallthrough_type on email Actions #106902
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
base: master
Are you sure you want to change the base?
Conversation
src/sentry/workflow_engine/migrations/0106_fix_email_action_fallthrough_type.py
Show resolved
Hide resolved
|
This PR has a migration; here is the generated SQL for for --
-- Raw Python operation
--
-- THIS OPERATION CANNOT BE WRITTEN AS SQL |
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.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| workflow_action.data = translated["data"] | ||
| workflow_action.config = translated["config"] | ||
| workflow_action.integration_id = translated["integration_id"] | ||
| workflow_action.save() |
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.
Translation failure causes misaligned workflow action updates
Medium Severity
When translate_email_action raises ValueError for any rule action, that action is skipped and the loop continues. This causes translated_actions to have fewer items than rule_email_actions, but subsequent translated actions shift to fill the gap. The migration then applies translated_actions[i] to workflow_email_actions[i] by position, causing misalignment. If the 2nd of 3 rule actions fails translation, the 3rd rule action's data gets incorrectly applied to the 2nd workflow action, while the 3rd workflow action is left unmodified.
| # target_identifier is only set for Member/Team, not IssueOwners | ||
| if target_type_str in ("Member", "Team"): | ||
| target_identifier = rule_action.get("targetIdentifier") | ||
| config["target_identifier"] = str(target_identifier) if target_identifier else None |
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.
Migration produces schema-violating config for Member/Team actions
Medium Severity
For Member/Team email actions where targetIdentifier is missing or falsy in the Rule, the migration sets config["target_identifier"] = None. However, the EmailActionHandler.config_schema requires target_identifier to be a string (not null) when target_type is USER or TEAM. Since migrations use apps.get_model(), the pre_save schema validation signal is bypassed, allowing invalid data to be saved. When someone later edits the action through normal code paths, schema validation fails with a ValidationError.
We started handling fallthroughType correctly in dual writing here #106633
But we need a migration to fix the previously migrated email actions