Skip to content

feat(ACI): Handle owner passed to workflow#110785

Merged
ceorourke merged 8 commits intomasterfrom
ceorourke/ISWF-2239
Mar 16, 2026
Merged

feat(ACI): Handle owner passed to workflow#110785
ceorourke merged 8 commits intomasterfrom
ceorourke/ISWF-2239

Conversation

@ceorourke
Copy link
Member

The Workflow model inherits the OwnerModel but the WorkflowValidator doesn't actually do anything if passed owner. This PR adds handling to the validator for the owner field so that it's saved and updated if passed. Thankfully the OwnerActorField handles all the validation needed. Tests are copied from the existing behavior in issue alert rules for creating and updating. API docs are updated (and fixed) to indicate that there can be an owner for detectors and workflows (externally, monitors and alerts).

@linear-code
Copy link

linear-code bot commented Mar 16, 2026

@ceorourke ceorourke marked this pull request as ready for review March 16, 2026 19:13
@ceorourke ceorourke requested a review from a team as a code owner March 16, 2026 19:13
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 16, 2026
Copy link
Contributor

@saponifi3d saponifi3d left a comment

Choose a reason for hiding this comment

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

lgtm - not quite sure why you're getting some of that type failure though, might just need to update from master to fix that, then the tests will need to be updated for the api docs.

thanks for implementing this so quickly 🙏

@ceorourke ceorourke force-pushed the ceorourke/ISWF-2239 branch from 2fea957 to 670304c Compare March 16, 2026 20:21
Copy link
Contributor

@cursor cursor bot left a 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 3 potential issues.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

owner_user_id = None
owner_team_id = None

return owner_user_id, owner_team_id
Copy link
Contributor

Choose a reason for hiding this comment

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

UnboundLocalError when owner is not user or team

Medium Severity

In update_owner, if owner is truthy but neither is_user nor is_team, the if/elif branches are both skipped, and owner_user_id and owner_team_id are never assigned. The return statement on line 32 then raises an UnboundLocalError. An else clause within the if owner: block (or initializing the variables before the conditional) would prevent this.

Fix in Cursor Fix in Web

@github-actions
Copy link
Contributor

github-actions bot commented Mar 16, 2026

Backend Test Failures

Failures on fd5704f in this run:

tests/sentry/workflow_engine/endpoints/validators/test_base_workflow.py::TestWorkflowValidatorUpdate::test_update_owner_typelog
.venv/lib/python3.13/site-packages/django/db/models/options.py:683: in get_field
    return self.fields_map[field_name]
E   KeyError: 'owner'

During handling of the above exception, another exception occurred:
tests/sentry/workflow_engine/endpoints/validators/test_base_workflow.py:828: in test_update_owner_type
    workflow = validator.update(self.workflow, validator.validated_data)
src/sentry/workflow_engine/endpoints/validators/base/workflow.py:262: in update
    instance.update(**validated_data)
src/sentry/silo/base.py:158: in override
    return original_method(*args, **kwargs)
src/sentry/db/models/query.py:92: in update
    instance.__class__.objects.using(using)
src/sentry/silo/base.py:158: in override
    return original_method(*args, **kwargs)
src/sentry/db/models/manager/base_query_set.py:92: in update
    return super().update(**kwargs)
.venv/lib/python3.13/site-packages/django/db/models/query.py:1237: in update
    query.add_update_values(kwargs)
.venv/lib/python3.13/site-packages/django/db/models/sql/subqueries.py:88: in add_update_values
    field = self.get_meta().get_field(name)
.venv/lib/python3.13/site-packages/django/db/models/options.py:685: in get_field
    raise FieldDoesNotExist(
E   django.core.exceptions.FieldDoesNotExist: Workflow has no field named 'owner'
tests/sentry/workflow_engine/endpoints/validators/test_base_workflow.py::TestWorkflowValidatorUpdate::test_reassign_owner_from_own_team_to_any_teamlog
.venv/lib/python3.13/site-packages/django/db/models/options.py:683: in get_field
    return self.fields_map[field_name]
E   KeyError: 'owner'

During handling of the above exception, another exception occurred:
tests/sentry/workflow_engine/endpoints/validators/test_base_workflow.py:898: in test_reassign_owner_from_own_team_to_any_team
    workflow = validator.update(self.workflow, validator.validated_data)
src/sentry/workflow_engine/endpoints/validators/base/workflow.py:262: in update
    instance.update(**validated_data)
src/sentry/silo/base.py:158: in override
    return original_method(*args, **kwargs)
src/sentry/db/models/query.py:92: in update
    instance.__class__.objects.using(using)
src/sentry/silo/base.py:158: in override
    return original_method(*args, **kwargs)
src/sentry/db/models/manager/base_query_set.py:92: in update
    return super().update(**kwargs)
.venv/lib/python3.13/site-packages/django/db/models/query.py:1237: in update
    query.add_update_values(kwargs)
.venv/lib/python3.13/site-packages/django/db/models/sql/subqueries.py:88: in add_update_values
    field = self.get_meta().get_field(name)
.venv/lib/python3.13/site-packages/django/db/models/options.py:685: in get_field
    raise FieldDoesNotExist(
E   django.core.exceptions.FieldDoesNotExist: Workflow has no field named 'owner'
tests/sentry/workflow_engine/endpoints/validators/test_base_workflow.py::TestWorkflowValidatorUpdate::test_team_owner_not_member_with_team_admin_scopelog
.venv/lib/python3.13/site-packages/django/db/models/options.py:683: in get_field
    return self.fields_map[field_name]
E   KeyError: 'owner'

During handling of the above exception, another exception occurred:
tests/sentry/workflow_engine/endpoints/validators/test_base_workflow.py:869: in test_team_owner_not_member_with_team_admin_scope
    workflow = validator.update(self.workflow, validator.validated_data)
src/sentry/workflow_engine/endpoints/validators/base/workflow.py:262: in update
    instance.update(**validated_data)
src/sentry/silo/base.py:158: in override
    return original_method(*args, **kwargs)
src/sentry/db/models/query.py:92: in update
    instance.__class__.objects.using(using)
src/sentry/silo/base.py:158: in override
    return original_method(*args, **kwargs)
src/sentry/db/models/manager/base_query_set.py:92: in update
    return super().update(**kwargs)
.venv/lib/python3.13/site-packages/django/db/models/query.py:1237: in update
    query.add_update_values(kwargs)
.venv/lib/python3.13/site-packages/django/db/models/sql/subqueries.py:88: in add_update_values
    field = self.get_meta().get_field(name)
.venv/lib/python3.13/site-packages/django/db/models/options.py:685: in get_field
    raise FieldDoesNotExist(
E   django.core.exceptions.FieldDoesNotExist: Workflow has no field named 'owner'

@ceorourke ceorourke merged commit eb1a70b into master Mar 16, 2026
59 of 60 checks passed
@ceorourke ceorourke deleted the ceorourke/ISWF-2239 branch March 16, 2026 21:19
JonasBa pushed a commit that referenced this pull request Mar 16, 2026
The `Workflow` model inherits the `OwnerModel` but the
`WorkflowValidator` doesn't actually do anything if passed `owner`. This
PR adds handling to the validator for the `owner` field so that it's
saved and updated if passed. Thankfully the `OwnerActorField` handles
all the validation needed. Tests are copied from the existing behavior
in issue alert rules for creating and updating. API docs are updated
(and fixed) to indicate that there can be an owner for detectors and
workflows (externally, monitors and alerts).
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.

2 participants