Skip to content

Commit 70c1f12

Browse files
authored
fix(aci): Encode owner as string when triggering find_channel_id_for_rule (#103204)
It's unclear that this ever worked properly, but presumably this is a narrow enough case that it hasn't been terribly noticeable. Fixes SENTRY-54ZZ.
1 parent 261e83f commit 70c1f12

File tree

5 files changed

+98
-4
lines changed

5 files changed

+98
-4
lines changed

src/sentry/api/endpoints/project_rule_details.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,11 @@ def put(self, request: Request, project, rule) -> Response:
295295
if data.get("pending_save"):
296296
client = RedisRuleStatus()
297297
kwargs.update({"uuid": client.uuid, "rule_id": rule.id})
298-
find_channel_id_for_rule.apply_async(kwargs=kwargs)
298+
# Serialize Actor object to string identifier for task queueing
299+
task_kwargs = kwargs.copy()
300+
if owner:
301+
task_kwargs["owner"] = owner.identifier
302+
find_channel_id_for_rule.apply_async(kwargs=task_kwargs)
299303

300304
context = {"uuid": client.uuid}
301305
return Response(context, status=202)

src/sentry/api/endpoints/project_rules.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,11 @@ def post(self, request: Request, project) -> Response:
833833
client = RedisRuleStatus()
834834
uuid_context = {"uuid": client.uuid}
835835
kwargs.update(uuid_context)
836-
find_channel_id_for_rule.apply_async(kwargs=kwargs)
836+
# Serialize Actor object to string identifier for task queueing
837+
task_kwargs = kwargs.copy()
838+
if owner:
839+
task_kwargs["owner"] = owner.identifier
840+
find_channel_id_for_rule.apply_async(kwargs=task_kwargs)
837841
return Response(uuid_context, status=202)
838842

839843
try:

src/sentry/integrations/slack/tasks/find_channel_id_for_rule.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
from sentry.silo.base import SiloMode
2020
from sentry.tasks.base import instrumented_task
2121
from sentry.taskworker.namespaces import integrations_tasks
22+
from sentry.types.actor import Actor
2223

2324
logger = logging.getLogger("sentry.integrations.slack.tasks")
2425

@@ -109,6 +110,11 @@ def find_channel_id_for_rule(
109110
kwargs["actions"] = actions
110111
kwargs["project"] = project
111112

113+
# Deserialize owner string identifier back to Actor object
114+
owner = kwargs.get("owner")
115+
if owner and isinstance(owner, str):
116+
kwargs["owner"] = Actor.from_identifier(owner)
117+
112118
if rule_id:
113119
rule = Rule.objects.get(id=rule_id)
114120
rule = ProjectRuleUpdater(

tests/sentry/api/endpoints/test_project_rules.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
from sentry.testutils.cases import APITestCase
2222
from sentry.testutils.helpers import install_slack, with_feature
2323
from sentry.testutils.silo import assume_test_silo_mode
24-
from sentry.types.actor import Actor
2524
from sentry.users.models.user import User
2625

2726

@@ -751,7 +750,7 @@ def test_kicks_off_slack_async_job(
751750
"actions": payload.get("actions", []),
752751
"frequency": payload.get("frequency"),
753752
"user_id": self.user.id,
754-
"owner": Actor.from_id(user_id=self.user.id),
753+
"owner": f"user:{self.user.id}",
755754
"uuid": "abc123",
756755
}
757756
call_args = mock_find_channel_id_for_alert_rule.call_args[1]["kwargs"]

tests/sentry/integrations/slack/tasks/test_tasks.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,45 @@ def test_task_new_rule_project_id(self, mock_set_value: MagicMock) -> None:
159159
]
160160
assert rule.created_by_id == self.user.id
161161

162+
@responses.activate
163+
@patch.object(RedisRuleStatus, "set_value", return_value=None)
164+
def test_task_new_rule_with_owner(self, mock_set_value: MagicMock) -> None:
165+
"""Test that owner identifier string is deserialized to Actor correctly."""
166+
team = self.create_team(organization=self.organization)
167+
owner_identifier = f"team:{team.id}"
168+
169+
data = {
170+
"name": "New Rule with Owner",
171+
"environment": None,
172+
"project_id": self.project.id,
173+
"action_match": "all",
174+
"filter_match": "all",
175+
"conditions": [
176+
{"id": "sentry.rules.conditions.first_seen_event.FirstSeenEventCondition"}
177+
],
178+
"actions": [
179+
{
180+
"channel": "#my-channel",
181+
"id": "sentry.integrations.slack.notify_action.SlackNotifyServiceAction",
182+
"tags": "",
183+
"workspace": self.integration.id,
184+
}
185+
],
186+
"frequency": 5,
187+
"uuid": self.uuid,
188+
"user_id": self.user.id,
189+
"owner": owner_identifier,
190+
}
191+
192+
with self.tasks():
193+
find_channel_id_for_rule(**data)
194+
195+
rule = Rule.objects.exclude(label__in=[DEFAULT_RULE_LABEL]).get(project_id=self.project.id)
196+
mock_set_value.assert_called_with("success", rule.id)
197+
assert rule.label == "New Rule with Owner"
198+
assert rule.owner_team_id == team.id
199+
assert rule.owner_user_id is None
200+
162201
@responses.activate
163202
@patch.object(RedisRuleStatus, "set_value", return_value=None)
164203
def test_task_existing_rule(self, mock_set_value: MagicMock) -> None:
@@ -205,6 +244,48 @@ def test_task_existing_rule(self, mock_set_value: MagicMock) -> None:
205244
}
206245
]
207246

247+
@responses.activate
248+
@patch.object(RedisRuleStatus, "set_value", return_value=None)
249+
def test_task_existing_rule_with_owner(self, mock_set_value: MagicMock) -> None:
250+
"""Test that owner identifier string is deserialized to Actor correctly during update."""
251+
action_data = {"id": "sentry.rules.actions.notify_event.NotifyEventAction"}
252+
condition_data = {"id": "sentry.rules.conditions.every_event.EveryEventCondition"}
253+
rule = Rule.objects.create(
254+
project=self.project, data={"actions": [action_data], "conditions": [condition_data]}
255+
)
256+
257+
owner_identifier = f"user:{self.user.id}"
258+
259+
data = {
260+
"name": "Updated Rule with Owner",
261+
"environment": None,
262+
"project_id": self.project.id,
263+
"action_match": "all",
264+
"filter_match": "all",
265+
"conditions": [condition_data],
266+
"actions": [
267+
{
268+
"channel": "#my-channel",
269+
"id": "sentry.integrations.slack.notify_action.SlackNotifyServiceAction",
270+
"tags": "",
271+
"workspace": self.integration.id,
272+
}
273+
],
274+
"frequency": 5,
275+
"uuid": self.uuid,
276+
"rule_id": rule.id,
277+
"owner": owner_identifier,
278+
}
279+
280+
with self.tasks():
281+
find_channel_id_for_rule(**data)
282+
283+
updated_rule = Rule.objects.get(id=rule.id)
284+
mock_set_value.assert_called_with("success", rule.id)
285+
assert updated_rule.label == "Updated Rule with Owner"
286+
assert updated_rule.owner_user_id == self.user.id
287+
assert updated_rule.owner_team_id is None
288+
208289
@responses.activate
209290
@patch.object(RedisRuleStatus, "set_value", return_value=None)
210291
@patch(

0 commit comments

Comments
 (0)