Skip to content

Conversation

@ceorourke
Copy link
Member

@ceorourke ceorourke commented Nov 19, 2025

If "suggested assignees" are selected when creating an action that uses the fallthrough type the schema was incorrectly expecting it to be camel case instead of snake case which prevented the action from being created. This updates that and adds a test case around that particular action.

For now we must support both types until we can run a migration, after which I can go back and remove support for the incorrect camel case type.

Fixes https://getsentry.atlassian.net/browse/ACI-504

Related migration #103764

@ceorourke ceorourke requested review from a team as code owners November 19, 2025 22:28
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 19, 2025
"sentry.notifications.notification_action.registry.action_validator_registry.get",
return_value=MockActionValidatorTranslator,
)
def test_create_workflow__with_fallthrough_type_action(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank youuuu!!

@codecov
Copy link

codecov bot commented Nov 19, 2025

Codecov Report

❌ Patch coverage is 37.50000% with 5 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ert_registry/handlers/email_issue_alert_handler.py 16.66% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #103694      +/-   ##
===========================================
- Coverage   80.51%    80.50%   -0.02%     
===========================================
  Files        9274      9278       +4     
  Lines      395967    397190    +1223     
  Branches    25250     25250              
===========================================
+ Hits       318819    319745     +926     
- Misses      76688     76985     +297     
  Partials      460       460              

@cathteng
Copy link
Member

Do we need a backfill migration to fix this for existing actions? All existing actions will have the old camelCase key

action_data = action.data.copy()
if action.data.get("fallthroughType"):
del action_data["fallthroughType"]
action_data["fallthrough_type"] = action.data["fallthroughType"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: CamelCase key silently overwrites snake_case key

When both fallthroughType and fallthrough_type keys exist in action.data, the camelCase version silently overwrites the snake_case version. The schema allows both keys simultaneously without mutual exclusivity constraints, and the conversion logic on line 58 unconditionally overwrites fallthrough_type with the value from fallthroughType, potentially discarding the user's intended value.

Fix in Cursor Fix in Web

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is the point

@ceorourke ceorourke requested a review from saponifi3d November 20, 2025 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants