Skip to content

Commit

Permalink
feat: Add assigneeName to issue assigned activities (#85554)
Browse files Browse the repository at this point in the history
Originally written by: @gabriellanata; It supersedes: #85399

> When an issue is assigned to a team, the activity only shows the team
ID which is not very visual. I'm adding assigneeName (which matches the
`assignedTo` when requesting an issue's details) to activity data.

If you visit [this
URL](https://us.sentry.io/api/0/organizations/sentry/issues/4704765109/activities/)
you can see the current format of the activity (trimmed data):
```
{
  "activity": [
    {
      "type": "assigned",
      "data": {
        "assignee": "2006237",
        "assigneeEmail": null,
        "assigneeType": "team"
      }
    }
  ]
}
```
The `issues` team is not mentioned in the API, however, the UI handles
it well:
<img width="309" alt="image"
src="https://github.com/user-attachments/assets/fbe76b20-ad50-40a8-b32c-35189a44ed39"
/>

Ideally, we would store less information in `data` and make the
serializer smarter by fetching the models it is indirectly referring to
(see
[code](https://github.com/getsentry/sentry/blob/e96e32a095e1e2ff89c8fb0ea2681848decaf4c3/src/sentry/models/activity.py#L57-L66)).
  • Loading branch information
armenzg authored Feb 28, 2025
1 parent 9cfea0e commit efd7381
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 5 deletions.
1 change: 1 addition & 0 deletions src/sentry/models/groupassignee.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def get_assigned_to_data(
data = {
"assignee": str(assigned_to.id),
"assigneeEmail": getattr(assigned_to, "email", None),
"assigneeName": getattr(assigned_to, "name", None),
"assigneeType": assignee_type,
}
if extra:
Expand Down
9 changes: 8 additions & 1 deletion src/sentry/web/frontend/debug/debug_assigned_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ def get_activity(self, request: AuthenticatedHttpRequest, event):
"data": {
"assignee": "10000000",
"assigneeEmail": "[email protected]",
"assigneeName": "Example User",
"assigneeType": "user",
},
}
Expand All @@ -25,6 +26,7 @@ def get_activity(self, request: AuthenticatedHttpRequest, event):
"data": {
"assignee": str(request.user.id),
"assigneeEmail": request.user.email,
"assigneeName": request.user.name,
"assigneeType": "user",
},
}
Expand All @@ -35,5 +37,10 @@ def get_activity(self, request: AuthenticatedHttpRequest, event):
return {
"type": ActivityType.ASSIGNED.value,
"user_id": request.user.id,
"data": {"assignee": "1", "assigneeEmail": None, "assigneeType": "team"},
"data": {
"assignee": "1",
"assigneeEmail": None,
"assigneeName": "example-team",
"assigneeType": "team",
},
}
2 changes: 2 additions & 0 deletions tests/sentry/integrations/msteams/test_action_state_change.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ def test_assign_to_team(self, verify):
assert activity.data == {
"assignee": str(self.team.id),
"assigneeEmail": None,
"assigneeName": self.team.name,
"assigneeType": "team",
"integration": ActivityIntegration.MSTEAMS.value,
}
Expand All @@ -242,6 +243,7 @@ def test_assign_to_me(self, verify, mock_record):
assert activity.data == {
"assignee": str(self.user.id),
"assigneeEmail": self.user.email,
"assigneeName": self.user.name,
"assigneeType": "user",
"integration": ActivityIntegration.MSTEAMS.value,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -506,12 +506,14 @@ def test_assign_issue(self, mock_tags):
assert group_activity[0].data == {
"assignee": str(user2.id),
"assigneeEmail": user2.email,
"assigneeName": user2.name,
"assigneeType": "user",
"integration": ActivityIntegration.SLACK.value,
}
assert group_activity[-1].data == {
"assignee": str(self.team.id),
"assigneeEmail": None,
"assigneeName": self.team.name,
"assigneeType": "team",
"integration": ActivityIntegration.SLACK.value,
}
Expand Down Expand Up @@ -547,6 +549,7 @@ def test_assign_issue_error(self, mock_logger):
assert group_activity[0].data == {
"assignee": str(user2.id),
"assigneeEmail": user2.email,
"assigneeName": user2.name,
"assigneeType": "user",
"integration": ActivityIntegration.SLACK.value,
}
Expand Down Expand Up @@ -584,12 +587,14 @@ def test_assign_issue_through_unfurl(self):
assert group_activity[0].data == {
"assignee": str(user2.id),
"assigneeEmail": user2.email,
"assigneeName": user2.name,
"assigneeType": "user",
"integration": ActivityIntegration.SLACK.value,
}
assert group_activity[-1].data == {
"assignee": str(self.team.id),
"assigneeEmail": None,
"assigneeName": self.team.name,
"assigneeType": "team",
"integration": ActivityIntegration.SLACK.value,
}
Expand Down
8 changes: 8 additions & 0 deletions tests/sentry/models/test_groupassignee.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def test_assign_user(self):

assert activity.data["assignee"] == str(self.user.id)
assert activity.data["assigneeEmail"] == self.user.email
assert activity.data["assigneeName"] == self.user.name
assert activity.data["assigneeType"] == "user"

def test_assign_team(self):
Expand All @@ -59,6 +60,7 @@ def test_assign_team(self):

assert activity.data["assignee"] == str(self.team.id)
assert activity.data["assigneeEmail"] is None
assert activity.data["assigneeName"] == self.team.name
assert activity.data["assigneeType"] == "team"

def test_create_only(self):
Expand All @@ -73,6 +75,7 @@ def test_create_only(self):
)
assert activity.data["assignee"] == str(self.user.id)
assert activity.data["assigneeEmail"] == self.user.email
assert activity.data["assigneeName"] == self.user.name
assert activity.data["assigneeType"] == "user"

other_user = self.create_user()
Expand All @@ -88,6 +91,7 @@ def test_create_only(self):
)
assert activity.data["assignee"] == str(self.user.id)
assert activity.data["assigneeEmail"] == self.user.email
assert activity.data["assigneeName"] == self.user.name
assert activity.data["assigneeType"] == "user"

def test_reassign_user_to_team(self):
Expand All @@ -111,10 +115,12 @@ def test_reassign_user_to_team(self):

assert activity[0].data["assignee"] == str(self.user.id)
assert activity[0].data["assigneeEmail"] == self.user.email
assert activity[0].data["assigneeName"] == self.user.name
assert activity[0].data["assigneeType"] == "user"

assert activity[1].data["assignee"] == str(self.team.id)
assert activity[1].data["assigneeEmail"] is None
assert activity[1].data["assigneeName"] == self.team.name
assert activity[1].data["assigneeType"] == "team"

@mock.patch.object(ExampleIntegration, "sync_assignee_outbound")
Expand Down Expand Up @@ -174,6 +180,7 @@ def test_assignee_sync_outbound_assign(self, mock_sync_assignee_outbound):

assert activity.data["assignee"] == str(self.user.id)
assert activity.data["assigneeEmail"] == self.user.email
assert activity.data["assigneeName"] == self.user.name
assert activity.data["assigneeType"] == "user"

@mock.patch.object(ExampleIntegration, "sync_assignee_outbound")
Expand Down Expand Up @@ -233,6 +240,7 @@ def test_assignee_sync_outbound_assign_with_matching_source_integration(

assert activity.data["assignee"] == str(self.user.id)
assert activity.data["assigneeEmail"] == self.user.email
assert activity.data["assigneeName"] == self.user.name
assert activity.data["assigneeType"] == "user"

@mock.patch.object(ExampleIntegration, "sync_assignee_outbound")
Expand Down
28 changes: 24 additions & 4 deletions tests/sentry/notifications/notifications/test_assigned.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,12 @@ def test_sends_reassignment_notification_user(self, mock_post):
data={"assignedTo": user1.username, "assignedBy": user1.username},
)
assert response.status_code == 200, response.content
data = {"assignee": str(user1.id), "assigneeEmail": user1.email, "assigneeType": "user"}
data = {
"assignee": str(user1.id),
"assigneeEmail": user1.email,
"assigneeName": user1.name,
"assigneeType": "user",
}
assert Activity.objects.filter(
group_id=self.group.id, type=ActivityType.ASSIGNED.value, user_id=user1.id, data=data
).exists()
Expand All @@ -143,7 +148,12 @@ def test_sends_reassignment_notification_user(self, mock_post):
data={"assignedTo": user2.username, "assignedBy": user1.username},
)
assert response.status_code == 200, response.content
data = {"assignee": str(user2.id), "assigneeEmail": user2.email, "assigneeType": "user"}
data = {
"assignee": str(user2.id),
"assigneeEmail": user2.email,
"assigneeName": user2.name,
"assigneeType": "user",
}
assert Activity.objects.filter(
group_id=self.group.id, type=ActivityType.ASSIGNED.value, user_id=user1.id, data=data
).exists()
Expand Down Expand Up @@ -190,7 +200,12 @@ def test_sends_reassignment_notification_team(self, mock_post):
data={"assignedTo": f"team:{team1.id}", "assignedBy": self.user.username},
)
assert response.status_code == 200, response.content
data = {"assignee": str(team1.id), "assigneeEmail": None, "assigneeType": "team"}
data = {
"assignee": str(team1.id),
"assigneeEmail": None,
"assigneeName": team1.name,
"assigneeType": "team",
}
assert Activity.objects.filter(
group_id=group.id, user_id=user1.id, type=ActivityType.ASSIGNED.value, data=data
).exists()
Expand All @@ -213,7 +228,12 @@ def test_sends_reassignment_notification_team(self, mock_post):
data={"assignedTo": f"team:{team2.id}", "assignedBy": self.user.username},
)
assert response.status_code == 200, response.content
data = {"assignee": str(team2.id), "assigneeEmail": None, "assigneeType": "team"}
data = {
"assignee": str(team2.id),
"assigneeEmail": None,
"assigneeName": team2.name,
"assigneeType": "team",
}
assert Activity.objects.filter(
group_id=group.id, user_id=user1.id, type=ActivityType.ASSIGNED.value, data=data
).exists()
Expand Down
1 change: 1 addition & 0 deletions tests/sentry/receivers/test_releases.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ def test_matching_author_with_assignment(self):
)[0].data == {
"assignee": str(user.id),
"assigneeEmail": user.email,
"assigneeName": user.name,
"assigneeType": "user",
}

Expand Down
1 change: 1 addition & 0 deletions tests/sentry/tasks/test_post_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,7 @@ def test_owner_assignment_order_precedence(self):
assert activity.data == {
"assignee": str(self.user.id),
"assigneeEmail": self.user.email,
"assigneeName": self.user.name,
"assigneeType": "user",
"integration": ActivityIntegration.PROJECT_OWNERSHIP.value,
"rule": str(Rule(Matcher("path", "src/*"), [Owner("user", self.user.email)])),
Expand Down

0 comments on commit efd7381

Please sign in to comment.