chore(ACI): Remove temporary fallthrough type logic#103996
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #103996 +/- ##
===========================================
+ Coverage 74.95% 80.62% +5.67%
===========================================
Files 9311 9313 +2
Lines 397391 397415 +24
Branches 25358 25358
===========================================
+ Hits 297860 320425 +22565
+ Misses 99074 76533 -22541
Partials 457 457 |
| action_data["fallthrough_type"] = action.data["fallthroughType"] | ||
|
|
||
| blob = EmailDataBlob(**action_data) | ||
| blob = EmailDataBlob(**action.data) |
There was a problem hiding this comment.
Bug: Removing fallthroughType backward compatibility before post-deployment migration causes TypeError.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The code removes backward compatibility logic for the fallthroughType field before its corresponding post-deployment migration has run. This causes a TypeError crash when EmailDataBlob(**action.data) is called with existing Action records that still contain fallthroughType (camelCase) in their action.data, as EmailDataBlob only accepts fallthrough_type (snake_case). This affects email notifications to issue owners immediately after deployment and before the migration 0104_action_data_fallthrough_type.py completes.
💡 Suggested Fix
Reinstate the backward compatibility logic to convert fallthroughType to fallthrough_type before passing action.data to EmailDataBlob, or ensure the migration runs before the code deployment.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
src/sentry/notifications/notification_action/issue_alert_registry/handlers/email_issue_alert_handler.py#L54
Potential issue: The code removes backward compatibility logic for the `fallthroughType`
field before its corresponding post-deployment migration has run. This causes a
`TypeError` crash when `EmailDataBlob(**action.data)` is called with existing `Action`
records that still contain `fallthroughType` (camelCase) in their `action.data`, as
`EmailDataBlob` only accepts `fallthrough_type` (snake_case). This affects email
notifications to issue owners immediately after deployment and before the migration
`0104_action_data_fallthrough_type.py` completes.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 3588791
Follow up to #103764 - once the migration is complete we can remove the temporary logic to support the incorrect case.
Since [migrating the action data fallthrough type](#103764) and [removing the logic to support both snake case and camel case](#103996) customers who have actions with a fallthrough type of "no one" have been receiving a surplus of notifications because the default value is `FallthroughChoiceType.ACTIVE_MEMBERS` and we were not properly fetching the `fallthrough_type`. Fixes https://getsentry.atlassian.net/browse/ACI-564 Fixes #104478 Note that this PR fixes only `fallthrough_type` and the other options for `target_type` and `target_identifier` are also incorrectly attempting to fetch a camel case value when it's actually stored in snake case in the database. This will need to be fixed in a follow up PR.
Follow up to #103764 - once the migration is complete we can remove the temporary logic to support the incorrect case.