Browse Source

feat(mentions) mention teams! (#7153)

* add actor field
* allow team mentions
Max Bittker 7 years ago
parent
commit
138af2ca57

+ 1 - 1
package.json

@@ -70,7 +70,7 @@
     "react-hot-loader": "^3.1.3",
     "react-icon-base": "^2.0.4",
     "react-lazy-load": "3.0.13",
-    "react-mentions": "^1.0.0",
+    "react-mentions": "^1.2.0",
     "react-router": "3.2.0",
     "react-sparklines": "1.7.0",
     "react-sticky": "6.0.1",

+ 30 - 10
src/sentry/api/endpoints/group_notes.py

@@ -8,7 +8,10 @@ from rest_framework.response import Response
 from sentry.api.base import DocSection
 from sentry.api.bases.group import GroupEndpoint
 from sentry.api.serializers import serialize
-from sentry.api.serializers.rest_framework.group_notes import NoteSerializer
+from sentry.api.serializers.rest_framework.group_notes import NoteSerializer, seperate_resolved_actors
+
+from sentry.api.fields.actor import Actor
+
 from sentry.models import Activity, GroupSubscription, GroupSubscriptionReason, User
 from sentry.utils.functional import extract_lazy_object
 
@@ -37,6 +40,7 @@ class GroupNotesEndpoint(GroupEndpoint):
             return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
 
         data = dict(serializer.object)
+
         mentions = data.pop('mentions', [])
 
         if Activity.objects.filter(
@@ -57,14 +61,31 @@ class GroupNotesEndpoint(GroupEndpoint):
             reason=GroupSubscriptionReason.comment,
         )
 
-        if mentions:
-            users = User.objects.filter(id__in=mentions)
-            for user in users:
-                GroupSubscription.objects.subscribe(
-                    group=group,
-                    user=user,
-                    reason=GroupSubscriptionReason.mentioned,
-                )
+        actors = Actor.resolve_many(mentions)
+        actor_mentions = seperate_resolved_actors(actors)
+
+        for user in actor_mentions.get('users'):
+            GroupSubscription.objects.subscribe(
+                group=group,
+                user=user,
+                reason=GroupSubscriptionReason.mentioned,
+            )
+
+        mentioned_teams = actor_mentions.get('teams')
+
+        mentioned_team_users = User.objects.filter(
+            sentry_orgmember_set__organization_id=group.project.organization_id,
+            sentry_orgmember_set__organizationmemberteam__team__in=mentioned_teams,
+            sentry_orgmember_set__organizationmemberteam__is_active=True,
+            is_active=True,
+        ).exclude(id__in={u.id for u in actor_mentions.get('users')})
+
+        for user in mentioned_team_users:
+            GroupSubscription.objects.subscribe(
+                group=group,
+                user=user,
+                reason=GroupSubscriptionReason.team_mentioned,
+            )
 
         activity = Activity.objects.create(
             group=group,
@@ -75,5 +96,4 @@ class GroupNotesEndpoint(GroupEndpoint):
         )
 
         activity.send_notification()
-
         return Response(serialize(activity, request.user), status=201)

+ 59 - 0
src/sentry/api/fields/actor.py

@@ -0,0 +1,59 @@
+from __future__ import absolute_import, print_function
+
+import six
+
+from collections import defaultdict, namedtuple
+from rest_framework import serializers
+
+from sentry.models import User, Team
+
+
+class Actor(namedtuple('Actor', 'id type')):
+    def get_actor_id(self):
+        return '%s:%d' % (self.type.__name__.lower(), self.id)
+
+    @classmethod
+    def from_actor_id(cls, actor_id):
+        # If we have an integer, fall back to assuming it's a User
+        if isinstance(actor_id, six.integer_types):
+            return Actor(actor_id, User)
+
+        # If the actor_id is a simple integer as a string,
+        # we're also a User
+        if actor_id.isdigit():
+            return Actor(int(actor_id), User)
+
+        type_name, _, id = actor_id.partition(':')
+
+        return cls(int(id), {'user': User, 'team': Team}[type_name])
+
+    def resolve(self):
+        return self.type.objects.get(id=self.id)
+
+    @classmethod
+    def resolve_many(cls, actors):
+        actors_by_type = defaultdict(list)
+        for actor in actors:
+            actors_by_type[actor.type].append(actor)
+
+        results = []
+        for type, actors in actors_by_type.items():
+            results.extend(list(type.objects.filter(
+                id__in=[a.id for a in actors]
+            )))
+
+        return results
+
+
+class ActorField(serializers.WritableField):
+    def to_native(self, obj):
+        return obj.get_actor_id()
+
+    def from_native(self, data):
+        if not data:
+            return None
+
+        try:
+            return Actor.from_actor_id(data)
+        except Exception:
+            raise serializers.ValidationError('Unknown actor input')

+ 41 - 6
src/sentry/api/serializers/rest_framework/group_notes.py

@@ -3,21 +3,56 @@ from __future__ import absolute_import
 from rest_framework import serializers
 
 from .list import ListField
+from sentry.api.fields.actor import ActorField
+
+from sentry.models import User, Team
+
+
+def seperate_actors(actors):
+    users = [actor for actor in actors if actor.type is User]
+    teams = [actor for actor in actors if actor.type is Team]
+
+    return {'users': users, 'teams': teams}
+
+
+def seperate_resolved_actors(actors):
+    users = [actor for actor in actors if isinstance(actor, User)]
+    teams = [actor for actor in actors if isinstance(actor, Team)]
+
+    return {'users': users, 'teams': teams}
 
 
 class NoteSerializer(serializers.Serializer):
     text = serializers.CharField()
-    mentions = ListField(required=False)
+    mentions = ListField(child=ActorField(), required=False)
 
     def validate_mentions(self, attrs, source):
         if source in attrs and 'group' in self.context:
+
             mentions = attrs[source]
+            seperated_actors = seperate_actors(mentions)
+            # Validate that all mentioned users exist and are on the project.
+            users = seperated_actors['users']
+
+            mentioned_user_ids = {user.id for user in users}
+
             project = self.context['group'].project
-            member_ids = set(
-                project.member_set.filter(user_id__in=mentions).values_list('user_id', flat=True)
+
+            user_ids = set(
+                project.member_set.filter(user_id__in=mentioned_user_ids)
+                .values_list('user_id', flat=True)
             )
-            invalid_user_ids = [m for m in mentions if int(m) not in member_ids]
-            if invalid_user_ids:
-                raise serializers.ValidationError('Cannot mention a non-team member')
+
+            if len(mentioned_user_ids) > len(user_ids):
+                raise serializers.ValidationError('Cannot mention a non team member')
+
+            # Validate that all mentioned teams exist and are on the project.
+            teams = seperated_actors['teams']
+            mentioned_team_ids = {team.id for team in teams}
+
+            if len(mentioned_team_ids) > project.teams.filter(
+                    id__in={t.id for t in teams}).count():
+                raise serializers.ValidationError(
+                    'Mentioned team not found or not associated with project')
 
         return attrs

+ 3 - 0
src/sentry/models/groupsubscription.py

@@ -22,6 +22,7 @@ class GroupSubscriptionReason(object):
     status_change = 4
     deploy_setting = 5
     mentioned = 6
+    team_mentioned = 7
 
     descriptions = {
         implicit:
@@ -43,6 +44,8 @@ class GroupSubscriptionReason(object):
         u"opted to receive all deploy notifications for this organization",
         mentioned:
         u"have been mentioned in this issue",
+        team_mentioned:
+        u"are a member of a team mentioned in this issue",
     }
 
 

+ 108 - 34
src/sentry/static/sentry/app/components/activity/noteInput.jsx

@@ -2,10 +2,16 @@ import PropTypes from 'prop-types';
 import React from 'react';
 import createReactClass from 'create-react-class';
 import marked from 'marked';
+import classNames from 'classnames';
+
 import {MentionsInput, Mention} from 'react-mentions';
+import _ from 'lodash';
 
 import ApiMixin from '../../mixins/apiMixin';
+import OrganizationState from '../../mixins/organizationState';
+
 import GroupStore from '../../stores/groupStore';
+import TeamStore from '../../stores/teamStore';
 import IndicatorStore from '../../stores/indicatorStore';
 import {logException} from '../../utils/logging';
 import localStorage from '../../utils/localStorage';
@@ -18,6 +24,9 @@ function makeDefaultErrorJson() {
   return {detail: t('Unknown error. Please try again.')};
 }
 
+const buildUserId = id => `user:${id}`;
+const buildTeamId = id => `team:${id}`;
+
 const NoteInput = createReactClass({
   displayName: 'NoteInput',
 
@@ -29,21 +38,13 @@ const NoteInput = createReactClass({
     sessionUser: PropTypes.object.isRequired,
   },
 
-  mixins: [ApiMixin],
+  mixins: [ApiMixin, OrganizationState],
 
   getInitialState() {
     let {item, group} = this.props;
     let updating = !!item;
     let defaultText = '';
 
-    let mentionsList = this.props.memberList
-      .filter(member => this.props.sessionUser.id !== member.id)
-      .map(member => ({
-        id: member.id,
-        display: member.name,
-        email: member.email,
-      }));
-
     if (updating) {
       defaultText = item.data.text;
     } else {
@@ -64,12 +65,21 @@ const NoteInput = createReactClass({
       preview: false,
       updating,
       value: defaultText,
-      mentionsList,
-      mentions: [],
+      memberMentions: [],
+      teamMentions: [],
+      mentionableUsers: this.mentionableUsers(),
+      mentionableTeams: this.mentionableTeams(),
     };
   },
 
   componentWillUpdate(nextProps, nextState) {
+    if (!_.isEqual(nextProps.memberList, this.props.memberList)) {
+      this.setState({
+        mentionableUsers: this.mentionableUsers(),
+        mentionableTeams: this.mentionableTeams(),
+      });
+    }
+
     // We can't support this when editing an existing Note since it'll
     // clobber the other storages
     if (this.state.updating) return;
@@ -117,17 +127,22 @@ const NoteInput = createReactClass({
     }
   },
 
+  cleanMarkdown(text) {
+    return text
+      .replace(/\[sentry\.strip:member\]/g, '@')
+      .replace(/\[sentry\.strip:team\]/g, '#');
+  },
+
   create() {
     let {group} = this.props;
-    let mentions = this.finalMentions();
 
     let loadingIndicator = IndicatorStore.add(t('Posting comment..'));
 
     this.api.request('/issues/' + group.id + '/comments/', {
       method: 'POST',
       data: {
-        text: this.state.value,
-        mentions,
+        text: this.cleanMarkdown(this.state.value),
+        mentions: this.finalizeMentions(),
       },
       error: error => {
         this.setState({
@@ -202,18 +217,27 @@ const NoteInput = createReactClass({
     this.finish();
   },
 
-  onAdd(id, display) {
-    let mentions = this.state.mentions.concat([[id, display]]);
-    this.setState({mentions});
+  onAddMember(id, display) {
+    this.setState(({memberMentions}) => ({
+      memberMentions: [...memberMentions, [id, display]],
+    }));
+  },
+
+  onAddTeam(id, display) {
+    this.setState(({teamMentions}) => ({
+      teamMentions: [...teamMentions, [id, display]],
+    }));
   },
 
   finish() {
     this.props.onFinish && this.props.onFinish();
   },
 
-  finalMentions() {
-    // mention = [id, display]
-    return this.state.mentions
+  finalizeMentions() {
+    let {memberMentions, teamMentions} = this.state;
+
+    // each mention looks like [id, display]
+    return [...memberMentions, ...teamMentions]
       .filter(mention => this.state.value.indexOf(mention[1]) !== -1)
       .map(mention => mention[0]);
   },
@@ -238,20 +262,59 @@ const NoteInput = createReactClass({
     }
   },
 
+  mentionableUsers() {
+    let {memberList, sessionUser} = this.props;
+    return _.uniqBy(memberList, ({id}) => id)
+      .filter(member => sessionUser.id !== member.id)
+      .map(member => ({
+        id: buildUserId(member.id),
+        display: member.name,
+        email: member.email,
+      }));
+  },
+
+  mentionableTeams() {
+    let {group} = this.props;
+
+    return _.uniqBy(TeamStore.getAll(), ({id}) => id)
+      .filter(({projects}) => !!projects.find(p => p.slug === group.project.slug))
+      .map(team => ({
+        id: buildTeamId(team.id),
+        display: team.slug,
+        email: team.id,
+      }));
+  },
+
   render() {
-    let {error, errorJSON, loading, preview, updating, value, mentionsList} = this.state;
-    let classNames = 'activity-field';
-    if (error) {
-      classNames += ' error';
-    }
-    if (loading) {
-      classNames += ' loading';
-    }
+    let {
+      error,
+      errorJSON,
+      loading,
+      preview,
+      updating,
+      value,
+      mentionableUsers,
+      mentionableTeams,
+    } = this.state;
+
+    let hasTeamMentions = new Set(this.getOrganization().features).has(
+      'internal-catchall'
+    );
+    let placeHolderText = hasTeamMentions
+      ? t('Add details or updates to this event. \nTag users with @, or teams with #')
+      : t('Add details or updates to this event. \nTag users with @');
 
     let btnText = updating ? t('Save Comment') : t('Post Comment');
 
     return (
-      <form noValidate className={classNames} onSubmit={this.onSubmit}>
+      <form
+        noValidate
+        className={classNames('activity-field', {
+          error,
+          loading,
+        })}
+        onSubmit={this.onSubmit}
+      >
         <div className="activity-notes">
           <ul className="nav nav-tabs">
             <li className={!preview ? 'active' : ''}>
@@ -273,22 +336,33 @@ const NoteInput = createReactClass({
           ) : (
             <MentionsInput
               style={mentionsStyle}
-              placeholder={t('Add details or updates to this event')}
+              placeholder={placeHolderText}
               onChange={this.onChange}
               onBlur={this.onBlur}
               onKeyDown={this.onKeyDown}
               value={value}
               required={true}
               autoFocus={true}
-              displayTransform={(id, display) => `@${display}`}
-              markup="**__display__**"
+              displayTransform={(id, display, type) =>
+                `${type === 'member' ? '@' : '#'}${display}`}
+              markup="**[sentry.strip:__type__]__display__**"
             >
               <Mention
+                type="member"
                 trigger="@"
-                data={mentionsList}
-                onAdd={this.onAdd}
+                data={mentionableUsers}
+                onAdd={this.onAddMember}
                 appendSpaceOnAdd={true}
               />
+              {hasTeamMentions ? (
+                <Mention
+                  type="team"
+                  trigger="#"
+                  data={mentionableTeams}
+                  onAdd={this.onAddTeam}
+                  appendSpaceOnAdd={true}
+                />
+              ) : null}
             </MentionsInput>
           )}
           <div className="activity-actions">

+ 4 - 2
tests/js/spec/components/__snapshots__/noteInput.spec.jsx.snap

@@ -46,10 +46,11 @@ exports[`NoteInput renders 1`] = `
     <withDefaultStyle(MentionsInput)
       autoFocus={true}
       displayTransform={[Function]}
-      markup="**__display__**"
+      markup="**[sentry.strip:__type__]__display__**"
       onChange={[Function]}
       onKeyDown={[Function]}
-      placeholder="Add details or updates to this event"
+      placeholder="Add details or updates to this event. 
+Tag users with @"
       required={true}
       style={
         Object {
@@ -118,6 +119,7 @@ exports[`NoteInput renders 1`] = `
         onRemove={[Function]}
         renderSuggestion={null}
         trigger="@"
+        type="member"
       />
     </withDefaultStyle(MentionsInput)>
     <div

+ 8 - 3
tests/js/spec/components/noteInput.spec.jsx

@@ -26,13 +26,17 @@ describe('NoteInput', function() {
   });
 
   it('renders', function() {
-    let wrapper = shallow(<NoteInput group={{}} memberList={[]} sessionUser={{}} />);
+    let wrapper = shallow(
+      <NoteInput group={{}} memberList={[]} sessionUser={{}} />,
+      TestStubs.routerContext()
+    );
     expect(wrapper).toMatchSnapshot();
   });
 
   it('submits when meta + enter is pressed', function() {
     let wrapper = mount(
-      <NoteInput group={{id: 'groupId'}} memberList={[]} sessionUser={{}} />
+      <NoteInput group={{id: 'groupId'}} memberList={[]} sessionUser={{}} />,
+      TestStubs.routerContext()
     );
 
     let input = wrapper.find('textarea');
@@ -43,7 +47,8 @@ describe('NoteInput', function() {
 
   it('submits when ctrl + enter is pressed', function() {
     let wrapper = mount(
-      <NoteInput group={{id: 'groupId'}} memberList={[]} sessionUser={{}} />
+      <NoteInput group={{id: 'groupId'}} memberList={[]} sessionUser={{}} />,
+      TestStubs.routerContext()
     );
 
     let input = wrapper.find('textarea');

+ 51 - 2
tests/sentry/api/endpoints/test_group_notes.py

@@ -2,7 +2,7 @@ from __future__ import absolute_import
 
 import six
 
-from sentry.models import Activity
+from sentry.models import Activity, GroupSubscription, GroupSubscriptionReason
 from sentry.testutils import APITestCase
 
 
@@ -117,4 +117,53 @@ class GroupNoteCreateTest(APITestCase):
                   'mentions': [u'%s' % user_id]}
         )
 
-        assert response.content == '{"mentions": ["Cannot mention a non-team member"]}'
+        assert response.content == '{"mentions": ["Cannot mention a non team member"]}'
+
+    def test_with_team_mentions(self):
+        user = self.create_user(email='redTeamUser@example.com')
+
+        self.org = self.create_organization(
+            name='Gnarly Org',
+            owner=None,
+        )
+        # team that IS part of the project
+        self.team = self.create_team(organization=self.org, name='Red Team', members=[user])
+        # team that IS NOT part of the project
+        self.team2 = self.create_team(organization=self.org, name='Blue Team')
+
+        self.create_member(
+            user=self.user,
+            organization=self.org,
+            role='member',
+            teams=[self.team],
+        )
+
+        group = self.group
+
+        self.login_as(user=self.user)
+
+        url = '/api/0/issues/{}/comments/'.format(group.id)
+
+        # mentioning a team that does not exist returns 400
+        response = self.client.post(
+            url,
+            format='json',
+            data={'text': 'hey **blue-team** fix this bug',
+                  'mentions': [u'team:%s' % self.team2.id]}
+        )
+        assert response.status_code == 400, response.content
+
+        assert response.content == '{"mentions": ["Mentioned team not found or not associated with project"]}'
+
+        # mentioning a team in the project returns 201
+        response = self.client.post(
+            url,
+            format='json',
+            data={'text': 'hey **red-team** fix this bug',
+                  'mentions': [u'team:%s' % self.team.id]}
+        )
+        assert response.status_code == 201, response.content
+        assert len(
+            GroupSubscription.objects.filter(
+                group=group,
+                reason=GroupSubscriptionReason.team_mentioned)) == 1

+ 49 - 1
tests/sentry/api/serializers/test_fields.py

@@ -2,9 +2,12 @@ from __future__ import absolute_import
 
 from rest_framework import serializers
 
-from sentry.api.serializers.rest_framework import ListField
 from sentry.testutils import TestCase
 
+from sentry.api.serializers.rest_framework import ListField, ActorField
+from sentry.api.fields.actor import Actor
+from sentry.models import User, Team
+
 
 class ChildSerializer(serializers.Serializer):
     b_field = serializers.CharField(max_length=64)
@@ -17,6 +20,7 @@ class DummySerializer(serializers.Serializer):
         required=False,
         allow_null=False,
     )
+    actor_field = ActorField(required=False)
 
 
 class TestListField(TestCase):
@@ -58,3 +62,47 @@ class TestListField(TestCase):
         serializer = DummySerializer(data=data)
         assert not serializer.is_valid()
         assert serializer.errors == {'a_field': [u'd_field: This field is required.']}
+
+
+class TestActorField(TestCase):
+    def test_simple(self):
+        data = {
+            'actor_field': "user:1",
+        }
+
+        serializer = DummySerializer(data=data)
+        assert serializer.is_valid()
+        assert serializer.object == {
+            'actor_field': Actor(id=1, type=User)
+        }
+
+    def test_legacy_user_fallback(self):
+        data = {
+            'actor_field': "1",
+        }
+
+        serializer = DummySerializer(data=data)
+        assert serializer.is_valid()
+        assert serializer.object == {
+            'actor_field': Actor(id=1, type=User)
+        }
+
+    def test_team(self):
+        data = {
+            'actor_field': "team:1",
+        }
+
+        serializer = DummySerializer(data=data)
+        assert serializer.is_valid()
+        assert serializer.object == {
+            'actor_field': Actor(id=1, type=Team)
+        }
+
+    def test_validates(self):
+        data = {
+            'actor_field': "foo:1",
+        }
+
+        serializer = DummySerializer(data=data)
+        assert not serializer.is_valid()
+        assert serializer.errors == {'actor_field': [u'Unknown actor input']}

Some files were not shown because too many files changed in this diff