Repository: getsentry/sentry
Pull request: Show the relevant event if present when searching by event id
This pull request fixes #4283


Problem statement:
Search by event id is broken 
Searching by event id no longer shows you the event details. Instead it opens the error group with the latest event showing. This is problematic because there's no other way to get info about the exact event.

Hints from issue discussion:
I wouldn't consider this broken, but its a nice feature to have. It is very possible this used to once forward to an event and doesn't anymore, but if so its been >1yr since that change was made. It's also important to know that we can't always forward to an event.

Commit message: Ensures that the relevant event is shown if present when you search by the event id

Patch to review:
diff --git a/src/sentry/api/endpoints/project_group_index.py b/src/sentry/api/endpoints/project_group_index.py
index 6706f37143bea9..a56d45f3f41f5b 100644
--- a/src/sentry/api/endpoints/project_group_index.py
+++ b/src/sentry/api/endpoints/project_group_index.py
@@ -25,6 +25,7 @@
     GroupResolution, GroupSeen, GroupSnooze, GroupStatus, GroupSubscription,
     GroupSubscriptionReason, Release, TagKey
 )
+from sentry.models.event import Event
 from sentry.models.group import looks_like_short_id
 from sentry.search.utils import InvalidQuery, parse_query
 from sentry.tasks.deletion import delete_group
@@ -215,6 +216,7 @@ def get(self, request, project):
         query = request.GET.get('query', '').strip()
         if query:
             matching_group = None
+            matching_event = None
             if len(query) == 32:
                 # check to see if we've got an event ID
                 try:
@@ -226,6 +228,10 @@ def get(self, request, project):
                     pass
                 else:
                     matching_group = Group.objects.get(id=mapping.group_id)
+                    try:
+                        matching_event = Event.objects.get(event_id=query)
+                    except Event.DoesNotExist:
+                        pass
 
             # If the query looks like a short id, we want to provide some
             # information about where that is.  Note that this can return
@@ -241,7 +247,7 @@ def get(self, request, project):
             if matching_group is not None:
                 response = Response(serialize(
                     [matching_group], request.user, StreamGroupSerializer(
-                        stats_period=stats_period
+                        stats_period=stats_period, matching_event_id=getattr(matching_event, 'id', None)
                     )
                 ))
                 response['X-Sentry-Direct-Hit'] = '1'
diff --git a/src/sentry/api/serializers/models/group.py b/src/sentry/api/serializers/models/group.py
index 9f78f2be1d8f04..bc8c858917f0af 100644
--- a/src/sentry/api/serializers/models/group.py
+++ b/src/sentry/api/serializers/models/group.py
@@ -239,11 +239,12 @@ class StreamGroupSerializer(GroupSerializer):
         '24h': StatsPeriod(24, timedelta(hours=1)),
     }
 
-    def __init__(self, stats_period=None):
+    def __init__(self, stats_period=None, matching_event_id=None):
         if stats_period is not None:
             assert stats_period in self.STATS_PERIOD_CHOICES
 
         self.stats_period = stats_period
+        self.matching_event_id = matching_event_id
 
     def get_attrs(self, item_list, user):
         attrs = super(StreamGroupSerializer, self).get_attrs(item_list, user)
@@ -277,6 +278,9 @@ def serialize(self, obj, attrs, user):
                 self.stats_period: attrs['stats'],
             }
 
+        if self.matching_event_id:
+            result['matchingEventId'] = self.matching_event_id
+
         return result
 
 
diff --git a/src/sentry/static/sentry/app/views/stream.jsx b/src/sentry/static/sentry/app/views/stream.jsx
index adef73acfbd80d..821c34bffe8320 100644
--- a/src/sentry/static/sentry/app/views/stream.jsx
+++ b/src/sentry/static/sentry/app/views/stream.jsx
@@ -356,6 +356,10 @@ const Stream = React.createClass({
         // the current props one as the shortIdLookup can return results for
         // different projects.
         if (jqXHR.getResponseHeader('X-Sentry-Direct-Hit') === '1') {
+           if (data[0].matchingEventId){
+            return void browserHistory.pushState(null,
+              `/${this.props.params.orgId}/${data[0].project.slug}/issues/${data[0].id}/events/${data[0].matchingEventId}/`);
+          }
           return void browserHistory.pushState(null,
             `/${this.props.params.orgId}/${data[0].project.slug}/issues/${data[0].id}/`);
         }
diff --git a/tests/sentry/api/endpoints/test_project_group_index.py b/tests/sentry/api/endpoints/test_project_group_index.py
index dfe756ee4efd0c..c45b83cb365d9d 100644
--- a/tests/sentry/api/endpoints/test_project_group_index.py
+++ b/tests/sentry/api/endpoints/test_project_group_index.py
@@ -17,6 +17,7 @@
     GroupTagValue, GroupResolution, GroupSeen, GroupSnooze, GroupStatus,
     GroupSubscription, Release
 )
+from sentry.models.event import Event
 from sentry.testutils import APITestCase
 from sentry.testutils.helpers import parse_link_header
 
@@ -181,8 +182,13 @@ def test_lookup_by_event_id(self):
         project.update_option('sentry:resolve_age', 1)
         group = self.create_group(checksum='a' * 32)
         self.create_group(checksum='b' * 32)
+        event_id = 'c' * 32
+        event = Event.objects.create(
+            project_id=self.project.id,
+            event_id=event_id
+        )
         EventMapping.objects.create(
-            event_id='c' * 32,
+            event_id=event_id,
             project=group.project,
             group=group,
         )
@@ -193,6 +199,7 @@ def test_lookup_by_event_id(self):
         assert response.status_code == 200
         assert len(response.data) == 1
         assert response.data[0]['id'] == six.text_type(group.id)
+        assert response.data[0]['matchingEventId'] == event.id
 
     def test_lookup_by_event_id_with_whitespace(self):
         project = self.project


Code review comments:
diff_hunk: @@ -226,6 +228,10 @@ def get(self, request, project):
                     pass
                 else:
                     matching_group = Group.objects.get(id=mapping.group_id)
+                    try:
+                        matching_event = Event.objects.get(event_id=query)
path: src/sentry/api/endpoints/project_group_index.py
text: @user1:
This is incorrect and could be a security hole.

`event_id`'s aren't globally unique, so this query right now is making the assumption that it is, and we will only find one row in the entire database to match.

If we look at our model definition, https://github.com/getsentry/sentry/blob/master/src/sentry/models/event.py#L57

We are unique'd on `project_id, event_id`. This is just saying that `event_id`'s are unique per project scope.

Therefore, we also need to qualify the query here with `project_id`

So:

```python
Event.objects.get(
  project_id=project.id,
  event_id=query,
)
```

Would give us the results we expect.

@author:
makes sense, will fix that