Skip to content

feat(assignee): Allow GroupAssignee to also reference a Team #7080

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattrobenolt merged 3 commits into from
Feb 2, 2018
Merged

feat(assignee): Allow GroupAssignee to also reference a Team #7080

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a320370
feat(assignee): Allow GroupAssignee to also reference a Team
mattrobenolt Jan 26, 2018
e1bfec9
Add some tests and fix broken behavior as pointed out by my buddy Ted
mattrobenolt Feb 2, 2018
d2f7ddf
Bring back new migration
mattrobenolt Feb 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 36 additions & 11 deletions src/sentry/models/groupassignee.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,37 @@

class GroupAssigneeManager(BaseManager):
def assign(self, group, assigned_to, acting_user=None):
from sentry.models import User, Team

if isinstance(assigned_to, User):
Copy link
Contributor Author

@mattrobenolt mattrobenolt Jan 26, 2018

Choose a reason for hiding this comment

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

@tkaemming I want your opinion on this.

I originally wanted to do some abstraction of Actor, but turns out, this Actor also would need to encapsulate the same logic, and in this case GroupAssignee is just this actor abstraction. I also chose to do this with two different columns, user_id and team_id instead of an alternative Django "generic" FK style, where there's a column for model and object_id, this allows us to still use actual ForeignKeys, which being a tad more rigid since it can't FK against anything, which is a good thing here.

assignee_type = 'user'
other_type = 'team'
elif isinstance(assigned_to, Team):
assignee_type = 'team'
other_type = 'user'
else:
raise AssertionError('Invalid type to assign to: %r' % type(assigned_to))

now = timezone.now()
assignee, created = GroupAssignee.objects.get_or_create(
group=group,
defaults={
'project': group.project,
'user': assigned_to,
assignee_type: assigned_to,
'date_added': now,
}
)

if not created:
affected = GroupAssignee.objects.filter(
group=group,
).exclude(
user=assigned_to,
).update(
user=assigned_to, date_added=now
)
).exclude(**{
assignee_type: assigned_to,
}).update(**{
assignee_type: assigned_to,
other_type: None,
'date_added': now,
})
else:
affected = True
issue_assigned.send(project=group.project, group=group, sender=acting_user)
Expand All @@ -51,8 +64,9 @@ def assign(self, group, assigned_to, acting_user=None):
user=acting_user,
data={
'assignee': six.text_type(assigned_to.id),
'assigneeEmail': assigned_to.email,
}
'assigneeEmail': getattr(assigned_to, 'email', None),
'assigneeType': assignee_type,
},
)
activity.send_notification()

Expand All @@ -76,7 +90,7 @@ def deassign(self, group, acting_user=None):

class GroupAssignee(Model):
"""
Identifies an assignment relationship between a user and an
Identifies an assignment relationship between a user/team and an
aggregated event (Group).
"""
__core__ = False
Expand All @@ -85,11 +99,22 @@ class GroupAssignee(Model):

project = FlexibleForeignKey('sentry.Project', related_name="assignee_set")
group = FlexibleForeignKey('sentry.Group', related_name="assignee_set", unique=True)
user = FlexibleForeignKey(settings.AUTH_USER_MODEL, related_name="sentry_assignee_set")
user = FlexibleForeignKey(
settings.AUTH_USER_MODEL,
related_name="sentry_assignee_set",
null=True)
team = FlexibleForeignKey('sentry.Team', null=True)
date_added = models.DateTimeField(default=timezone.now)

class Meta:
app_label = 'sentry'
db_table = 'sentry_groupasignee'

__repr__ = sane_repr('group_id', 'user_id')
__repr__ = sane_repr('group_id', 'user_id', 'team_id')

def save(self, *args, **kwargs):
assert (
not (self.user_id is not None and self.team_id is not None) and
not (self.user_id is None and self.team_id is None)
), 'Must have Team or User, not both'
super(GroupAssignee, self).save(*args, **kwargs)
2 changes: 2 additions & 0 deletions src/sentry/plugins/sentry_mail/activity/assigned.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ def get_activity_name(self):
return 'Assigned'

def get_description(self):
# TODO(mattrobenolt): Handle when assignee is Team

activity = self.activity
data = activity.data
if activity.user_id and six.text_type(activity.user_id) == data['assignee']:
Expand Down
Loading