Browse Source

feat(codeowners): Track Issue Owner and Manual assignment (#27784)

Objective:
We want to be able to chart assignment by source: Code Owners, Issue Owners, Manual Assignment.

Manual Assignment:
- Can be done through the UI by the assignee_selector or the suggested_assignee. suggested_assignee uses Code Owners and Issue Owners for its suggestions.
- Manual Assignment can create a new Assignment or clear/reassign the Issue. Tracking reassignment will be useful gauge of effectiveness for orgs that use Code Owners.
NisanthanNanthakumar 3 years ago
parent
commit
c1b36105f9

+ 14 - 0
src/sentry/analytics/events/issueowners_assignment.py

@@ -0,0 +1,14 @@
+from sentry import analytics
+
+
+class IssueOwnersAssignment(analytics.Event):
+    type = "issueowners.assignment"
+
+    attributes = (
+        analytics.Attribute("organization_id"),
+        analytics.Attribute("project_id"),
+        analytics.Attribute("group_id"),
+    )
+
+
+analytics.register(IssueOwnersAssignment)

+ 16 - 0
src/sentry/analytics/events/manual_issue_assignment.py

@@ -0,0 +1,16 @@
+from sentry import analytics
+
+
+class ManualIssueAssignment(analytics.Event):
+    type = "manual.issue_assignment"
+
+    attributes = (
+        analytics.Attribute("organization_id"),
+        analytics.Attribute("project_id"),
+        analytics.Attribute("group_id"),
+        analytics.Attribute("assigned_by", required=False),
+        analytics.Attribute("had_to_deassign", required=False),
+    )
+
+
+analytics.register(ManualIssueAssignment)

+ 1 - 0
src/sentry/api/endpoints/group_details.py

@@ -243,6 +243,7 @@ class GroupDetailsEndpoint(GroupEndpoint, EnvironmentMixin):
                                   this issue. Can be of the form ``"<user_id>"``,
                                   ``"user:<user_id>"``, ``"<username>"``,
                                   ``"<user_primary_email>"``, or ``"team:<team_id>"``.
+        :param string assignedBy: ``"suggested_assignee"`` | ``"assignee_selector"``
         :param boolean hasSeen: in case this API call is invoked with a user
                                 context this allows changing of the flag
                                 that indicates if the user has seen the

+ 24 - 3
src/sentry/api/helpers/group_index.py

@@ -10,7 +10,7 @@ from rest_framework import serializers
 from rest_framework.exceptions import ParseError
 from rest_framework.response import Response
 
-from sentry import eventstream, features, search
+from sentry import analytics, eventstream, features, search
 from sentry.api.base import audit_logger
 from sentry.api.fields import ActorField
 from sentry.api.issue_search import convert_query_values, parse_search_query
@@ -952,18 +952,39 @@ def update_groups(request, group_ids, projects, organization_id, search_fn):
 
     if "assignedTo" in result:
         assigned_actor = result["assignedTo"]
+        assigned_by = (
+            request.data.get("assignedBy")
+            if request.data.get("assignedBy") in ["assignee_selector", "suggested_assignee"]
+            else None
+        )
         if assigned_actor:
             for group in group_list:
                 resolved_actor = assigned_actor.resolve()
 
-                GroupAssignee.objects.assign(group, resolved_actor, acting_user)
+                created = GroupAssignee.objects.assign(group, resolved_actor, acting_user)
+                analytics.record(
+                    "manual.issue_assignment",
+                    organization_id=project_lookup[group.project_id].organization_id,
+                    project_id=group.project_id,
+                    group_id=group.id,
+                    assigned_by=assigned_by,
+                    had_to_deassign=(not created),
+                )
             result["assignedTo"] = serialize(
                 assigned_actor.resolve(), acting_user, ActorSerializer()
             )
+
         else:
             for group in group_list:
                 GroupAssignee.objects.deassign(group, acting_user)
-
+                analytics.record(
+                    "manual.issue_assignment",
+                    organization_id=project_lookup[group.project_id].organization_id,
+                    project_id=group.project_id,
+                    group_id=group.id,
+                    assigned_by=assigned_by,
+                    had_to_deassign=True,
+                )
     is_member_map = {
         project.id: project.member_set.filter(user=acting_user).exists() for project in projects
     }

+ 2 - 0
src/sentry/models/groupassignee.py

@@ -179,6 +179,8 @@ class GroupAssigneeManager(BaseManager):
             ):
                 sync_group_assignee_outbound(group, assigned_to.id, assign=True)
 
+        return created
+
     def deassign(self, group, acting_user=None):
         from sentry import features
 

+ 7 - 0
src/sentry/tasks/post_process.py

@@ -111,6 +111,13 @@ def handle_owner_assignment(project, group, event):
                     project_id=project.id,
                     group_id=group.id,
                 )
+            else:
+                analytics.record(
+                    "issueowners.assignment",
+                    organization_id=project.organization_id,
+                    project_id=project.id,
+                    group_id=group.id,
+                )
 
         if owners and not owners_exists:
             try:

+ 8 - 3
static/app/actionCreators/group.tsx

@@ -8,6 +8,7 @@ import {Actor, Group, Member, Note, User} from 'app/types';
 import {buildTeamId, buildUserId} from 'app/utils';
 import {uniqueId} from 'app/utils/guid';
 
+type AssignedBy = 'suggested_assignee' | 'assignee_selector';
 type AssignToUserParams = {
   /**
    * Issue id
@@ -15,6 +16,7 @@ type AssignToUserParams = {
   id: string;
   user: User | Actor;
   member?: Member;
+  assignedBy: AssignedBy;
 };
 
 export function assignToUser(params: AssignToUserParams) {
@@ -35,6 +37,7 @@ export function assignToUser(params: AssignToUserParams) {
     // current assignee.
     data: {
       assignedTo: params.user ? buildUserId(params.user.id) : '',
+      assignedBy: params.assignedBy,
     },
   });
 
@@ -49,7 +52,7 @@ export function assignToUser(params: AssignToUserParams) {
   return request;
 }
 
-export function clearAssignment(groupId: string) {
+export function clearAssignment(groupId: string, assignedBy: AssignedBy) {
   const api = new Client();
 
   const endpoint = `/issues/${groupId}/`;
@@ -65,6 +68,7 @@ export function clearAssignment(groupId: string) {
     // Sending an empty value to assignedTo is the same as "clear"
     data: {
       assignedTo: '',
+      assignedBy,
     },
   });
 
@@ -85,9 +89,10 @@ type AssignToActorParams = {
    */
   id: string;
   actor: Pick<Actor, 'id' | 'type'>;
+  assignedBy: AssignedBy;
 };
 
-export function assignToActor({id, actor}: AssignToActorParams) {
+export function assignToActor({id, actor, assignedBy}: AssignToActorParams) {
   const api = new Client();
 
   const endpoint = `/issues/${id}/`;
@@ -116,7 +121,7 @@ export function assignToActor({id, actor}: AssignToActorParams) {
   return api
     .requestPromise(endpoint, {
       method: 'PUT',
-      data: {assignedTo: actorId},
+      data: {assignedTo: actorId, assignedBy},
     })
     .then(data => {
       GroupActions.assignToSuccess(guid, id, data);

+ 7 - 3
static/app/components/assigneeSelector.tsx

@@ -167,12 +167,16 @@ class AssigneeSelector extends React.Component<Props, State> {
   }
 
   assignToUser(user: User | Actor) {
-    assignToUser({id: this.props.id, user});
+    assignToUser({id: this.props.id, user, assignedBy: 'assignee_selector'});
     this.setState({loading: true});
   }
 
   assignToTeam(team: Team) {
-    assignToActor({actor: {id: team.id, type: 'team'}, id: this.props.id});
+    assignToActor({
+      actor: {id: team.id, type: 'team'},
+      id: this.props.id,
+      assignedBy: 'assignee_selector',
+    });
     this.setState({loading: true});
   }
 
@@ -203,7 +207,7 @@ class AssigneeSelector extends React.Component<Props, State> {
 
   clearAssignTo = (e: React.MouseEvent<HTMLDivElement>) => {
     // clears assignment
-    clearAssignment(this.props.id);
+    clearAssignment(this.props.id, 'assignee_selector');
     this.setState({loading: true});
     e.stopPropagation();
   };

+ 10 - 2
static/app/components/group/suggestedOwners/suggestedOwners.tsx

@@ -137,11 +137,19 @@ class SuggestedOwners extends React.Component<Props, State> {
       // TODO(ts): `event` here may not be 100% correct
       // in this case groupID should always exist on event
       // since this is only used in Issue Details
-      assignToUser({id: event.groupID as string, user: actor});
+      assignToUser({
+        id: event.groupID as string,
+        user: actor,
+        assignedBy: 'suggested_assignee',
+      });
     }
 
     if (actor.type === 'team') {
-      assignToActor({id: event.groupID as string, actor});
+      assignToActor({
+        id: event.groupID as string,
+        actor,
+        assignedBy: 'suggested_assignee',
+      });
     }
   };
 

+ 6 - 6
tests/js/spec/components/assigneeSelector.spec.jsx

@@ -200,7 +200,7 @@ describe('AssigneeSelector', function () {
     expect(assignMock).toHaveBeenLastCalledWith(
       '/issues/1337/',
       expect.objectContaining({
-        data: {assignedTo: 'user:1'},
+        data: {assignedTo: 'user:1', assignedBy: 'assignee_selector'},
       })
     );
 
@@ -229,7 +229,7 @@ describe('AssigneeSelector', function () {
     expect(assignMock).toHaveBeenCalledWith(
       '/issues/1337/',
       expect.objectContaining({
-        data: {assignedTo: 'team:3'},
+        data: {assignedTo: 'team:3', assignedBy: 'assignee_selector'},
       })
     );
 
@@ -254,7 +254,7 @@ describe('AssigneeSelector', function () {
     expect(assignMock).toHaveBeenCalledWith(
       '/issues/1337/',
       expect.objectContaining({
-        data: {assignedTo: 'team:3'},
+        data: {assignedTo: 'team:3', assignedBy: 'assignee_selector'},
       })
     );
 
@@ -273,7 +273,7 @@ describe('AssigneeSelector', function () {
     expect(assignMock).toHaveBeenLastCalledWith(
       '/issues/1337/',
       expect.objectContaining({
-        data: {assignedTo: ''},
+        data: {assignedTo: '', assignedBy: 'assignee_selector'},
       })
     );
   });
@@ -310,7 +310,7 @@ describe('AssigneeSelector', function () {
     expect(assignMock).toHaveBeenLastCalledWith(
       '/issues/1337/',
       expect.objectContaining({
-        data: {assignedTo: 'user:2'},
+        data: {assignedTo: 'user:2', assignedBy: 'assignee_selector'},
       })
     );
     expect(assigneeSelector.find('LoadingIndicator')).toHaveLength(1);
@@ -351,7 +351,7 @@ describe('AssigneeSelector', function () {
     expect(assignGroup2Mock).toHaveBeenCalledWith(
       '/issues/1338/',
       expect.objectContaining({
-        data: {assignedTo: 'user:1'},
+        data: {assignedTo: 'user:1', assignedBy: 'assignee_selector'},
       })
     );