Browse Source

feat(owners): Add suggested commit box to issue details

This also improves some heuristics around suggesting owners, leveraging filename over abs path when possible.
David Cramer 7 years ago
parent
commit
f7237419e4

+ 5 - 2
bin/load-mocks

@@ -81,7 +81,10 @@ def create_sample_event(*args, **kwargs):
 def generate_commits(user):
     commits = []
     for i in range(random.randint(0, 20)):
-        filename = random.choice(loremipsum.words)
+        if i == 1:
+            filename = 'raven/base.py'
+        else:
+            filename = random.choice(loremipsum.words) + '.js'
         if random.randint(0, 5) == 1:
             author = (user.name, user.email)
         else:
@@ -97,7 +100,7 @@ def generate_commits(user):
 
         commits.append({
             'key': sha1(uuid4().hex).hexdigest(),
-            'message': 'modified {}.js}',
+            'message': 'feat: Do something to {}\n{}'.format(filename, make_sentence()),
             'author': author,
             'files': [
                 (filename, 'M'),

+ 11 - 9
src/sentry/api/endpoints/event_file_committers.py

@@ -54,20 +54,20 @@ class EventFileCommittersEndpoint(ProjectEndpoint):
 
     def _get_commit_file_changes(self, commits, path_name_set):
         # build a single query to get all of the commit file that might match the first n frames
-
         path_query = reduce(
             operator.or_,
             (Q(filename__endswith=next(tokenize_path(path))) for path in path_name_set)
         )
 
-        query = Q(commit__in=commits) & path_query
-
-        commit_file_change_matches = CommitFileChange.objects.filter(query)
+        commit_file_change_matches = CommitFileChange.objects.filter(
+            path_query,
+            commit__in=commits,
+        )
 
         return list(commit_file_change_matches)
 
     def _match_commits_path(self, commit_file_changes, path):
-        #  find commits that match the run time path the best.
+        # find commits that match the run time path the best.
         matching_commits = {}
         best_score = 1
         for file_change in commit_file_changes:
@@ -125,7 +125,7 @@ class EventFileCommittersEndpoint(ProjectEndpoint):
 
         return user_dicts
 
-    def get(self, _, project, event_id):
+    def get(self, request, project, event_id):
         """
         Retrieve Committer information for an event
         ```````````````````````````````````````````
@@ -167,11 +167,13 @@ class EventFileCommittersEndpoint(ProjectEndpoint):
             return Response({'detail': 'No Commits found for Release'}, status=404)
 
         frames = self._get_frame_paths(event)
-        frame_limit = 25
+        frame_limit = int(request.GET.get('frameLimit', 25))
         app_frames = [frame for frame in frames if frame['in_app']][-frame_limit:]
+        if not app_frames:
+            app_frames = [frame for frame in frames][-frame_limit:]
 
         # TODO(maxbittker) return this set instead of annotated frames
-        path_set = {frame['abs_path'] for frame in app_frames}
+        path_set = {frame.get('filename') or frame['abs_path'] for frame in app_frames}
 
         file_changes = []
         if path_set:
@@ -184,7 +186,7 @@ class EventFileCommittersEndpoint(ProjectEndpoint):
         annotated_frames = [
             {
                 'frame': frame,
-                'commits': commit_path_matches[frame['abs_path']]
+                'commits': commit_path_matches[frame.get('filename') or frame['abs_path']]
             } for frame in app_frames
         ]
 

+ 2 - 0
src/sentry/api/serializers/models/organization.py

@@ -117,6 +117,8 @@ class DetailedOrganizationSerializer(OrganizationSerializer):
             feature_list.append('repos')
         if features.has('organizations:internal-catchall', obj, actor=user):
             feature_list.append('internal-catchall')
+        if features.has('organizations:suggested-commits', obj, actor=user):
+            feature_list.append('suggested-commits')
 
         if getattr(obj.flags, 'allow_joinleave'):
             feature_list.append('open-membership')

+ 1 - 0
src/sentry/features/__init__.py

@@ -14,6 +14,7 @@ default_manager.add('organizations:sso-rippling', OrganizationFeature)  # NOQA
 default_manager.add('organizations:onboarding', OrganizationFeature)  # NOQA
 default_manager.add('organizations:repos', OrganizationFeature)  # NOQA
 default_manager.add('organizations:release-commits', OrganizationFeature)  # NOQA
+default_manager.add('organizations:suggested-commits', OrganizationFeature)  # NOQA
 default_manager.add('organizations:group-unmerge', OrganizationFeature)  # NOQA
 default_manager.add('organizations:invite-members', OrganizationFeature)  # NOQA
 default_manager.add('organizations:new-settings', OrganizationFeature)  # NOQA

+ 191 - 0
src/sentry/static/sentry/app/components/events/eventCause.jsx

@@ -0,0 +1,191 @@
+import PropTypes from 'prop-types';
+import React from 'react';
+import createReactClass from 'create-react-class';
+import ReactDOMServer from 'react-dom/server';
+import moment from 'moment';
+
+import Avatar from '../avatar';
+import TooltipMixin from '../../mixins/tooltip';
+import ApiMixin from '../../mixins/apiMixin';
+import GroupState from '../../mixins/groupState';
+import TimeSince from '../timeSince';
+import {assignTo} from '../../actionCreators/group';
+
+export default createReactClass({
+  displayName: 'EventCause',
+
+  propTypes: {
+    event: PropTypes.object,
+  },
+
+  mixins: [
+    ApiMixin,
+    GroupState,
+    TooltipMixin({
+      selector: '.tip',
+      html: true,
+      container: 'body',
+      template:
+        '<div class="tooltip" role="tooltip"><div class="tooltip-arrow"></div><div class="tooltip-inner tooltip-owners"></div></div>',
+    }),
+  ],
+
+  getInitialState() {
+    return {committers: undefined};
+  },
+
+  componentDidMount() {
+    this.fetchData(this.props.event);
+  },
+
+  componentWillReceiveProps(nextProps) {
+    if (this.props.event && nextProps.event) {
+      if (this.props.event.id !== nextProps.event.id) {
+        //two events, with different IDs
+        this.fetchData(nextProps.event);
+      }
+    } else if (nextProps.event) {
+      //going from having no event to having an event
+      this.fetchData(nextProps.event);
+    }
+  },
+
+  componentDidUpdate(_, nextState) {
+    //this shallow equality should be OK because it's being mutated fetchData as a new object
+    if (this.state.owners !== nextState.owners) {
+      this.removeTooltips();
+      this.attachTooltips();
+    }
+  },
+
+  fetchData(event) {
+    // TODO(dcramer): this API request happens twice, and we need a store for it
+    if (!event) return;
+    let org = this.getOrganization();
+    let project = this.getProject();
+    this.api.request(
+      `/projects/${org.slug}/${project.slug}/events/${event.id}/committers/`,
+      {
+        success: (data, _, jqXHR) => {
+          this.setState(data);
+        },
+        error: error => {
+          this.setState({
+            committers: undefined,
+          });
+        },
+      }
+    );
+  },
+
+  assignTo(member) {
+    if (member.id !== undefined) {
+      assignTo({id: this.props.event.groupID, member});
+    }
+  },
+
+  renderCommitter(owner) {
+    let {author, commits} = owner;
+    return (
+      <span
+        key={author.id || author.email}
+        className="avatar-grid-item tip"
+        onClick={() => this.assignTo(author)}
+        title={ReactDOMServer.renderToStaticMarkup(
+          <div>
+            {author.id ? (
+              <div className="tooltip-owners-name">{author.name}</div>
+            ) : (
+              <div className="tooltip-owners-unknown">
+                <p className="tooltip-owners-unknown-email">
+                  <span className="icon icon-circle-cross" />
+                  <strong>{author.email}</strong>
+                </p>
+                <p>
+                  Sorry, we don't recognize this member. Make sure to link alternative
+                  emails in Account Settings.
+                </p>
+                <hr />
+              </div>
+            )}
+            <ul className="tooltip-owners-commits">
+              {commits.slice(0, 6).map(c => {
+                return (
+                  <li key={c.id} className="tooltip-owners-commit">
+                    {c.message}
+                    <span className="tooltip-owners-date">
+                      {' '}
+                      - {moment(c.dateCreated).fromNow()}
+                    </span>
+                  </li>
+                );
+              })}
+            </ul>
+          </div>
+        )}
+      >
+        <Avatar user={author} />
+      </span>
+    );
+  },
+
+  render() {
+    if (!(this.state.committers && this.state.committers.length)) {
+      return null;
+    }
+
+    let commitsWithAge = [];
+    this.state.committers.forEach(committer => {
+      committer.commits.forEach(commit => {
+        commitsWithAge.push([moment(commit.dateCreated), commit]);
+      });
+    });
+    let firstSeen = moment(this.getGroup().firstSeen);
+    commitsWithAge
+      .filter(([age, commit]) => {
+        return age < 604800;
+      })
+      .sort((a, b) => {
+        return firstSeen - a[0] - (firstSeen - b[0]);
+      });
+    if (!commitsWithAge.length) return null;
+
+    let probablyTheCommit = commitsWithAge[0][1];
+    let commitBits = probablyTheCommit.message.split('\n');
+    let subject = commitBits[0];
+    let message =
+      commitBits.length > 1
+        ? commitBits
+            .slice(1)
+            .join('\n')
+            .replace(/^\s+|\s+$/g, '')
+        : null;
+    return (
+      <div className="box">
+        <div className="box-header">
+          <h3>Likely Culprit</h3>
+        </div>
+        <div style={{fontSize: '0.8em', fontWeight: 'bold', marginBottom: 10}}>
+          {subject}
+        </div>
+        {!!message && (
+          <pre
+            style={{marginBottom: 10, background: 'none', padding: 0, fontSize: '0.8em'}}
+          >
+            {message}
+          </pre>
+        )}
+        <div style={{marginBottom: 20, fontSize: '0.7em', color: '#999', lineHeight: 1}}>
+          {!!probablyTheCommit.author ? (
+            <strong>{probablyTheCommit.author.name}</strong>
+          ) : (
+            <strong>
+              <em>Unknown Author</em>
+            </strong>
+          )}{' '}
+          committed <TimeSince date={probablyTheCommit.dateCreated} />
+        </div>
+      </div>
+    );
+  },
+});

+ 10 - 0
src/sentry/static/sentry/app/views/groupEventDetails.jsx

@@ -2,6 +2,7 @@ import React from 'react';
 import createReactClass from 'create-react-class';
 import ApiMixin from '../mixins/apiMixin';
 import EventEntries from '../components/events/eventEntries';
+import EventCause from '../components/events/eventCause';
 import GroupEventToolbar from './groupDetails/eventToolbar';
 import GroupSidebar from '../components/group/sidebar';
 import GroupState from '../mixins/groupState';
@@ -75,6 +76,7 @@ const GroupEventDetails = createReactClass({
     let group = this.getGroup();
     let evt = this.state.event;
     let params = this.props.params;
+    let features = new Set(this.getOrganization().features);
 
     return (
       <div>
@@ -98,6 +100,14 @@ const GroupEventDetails = createReactClass({
                 )}
               </div>
             )}
+            {features.has('suggested-commits') && (
+              <EventCause
+                group={group}
+                event={evt}
+                orgId={params.orgId}
+                project={this.getProject()}
+              />
+            )}
             {this.state.loading ? (
               <LoadingIndicator />
             ) : this.state.error ? (

+ 1 - 1
src/sentry/testutils/fixtures.py

@@ -56,7 +56,7 @@ DEFAULT_EVENT_DATA = {
                 'context_line':
                 '                        string_max_length=self.string_max_length)',
                 'filename':
-                'raven/base.py',
+                'sentry/models/foo.py',
                 'function':
                 'build_msg',
                 'in_app':

+ 1 - 0
tests/sentry/api/endpoints/test_event_committers.py

@@ -7,6 +7,7 @@ from sentry.models import Event
 from sentry.testutils import APITestCase
 
 
+# TODO(dcramer): These tests rely too much on implicit fixtures
 class EventCommittersTest(APITestCase):
     def test_simple(self):
         self.login_as(user=self.user)