Fix(group-details): multi tagvalue calls (#18414)

- This reverts commit d1fd7b4658a6ae962c694eb3686f1b3c4e4c5f4c.
    - fix: Issue when a Release object dosen't exist
- This resolves an issue when a Release doesn't exist, no longer passing
  a dictionary to the serializer which resulted in Sentry-G22

src/sentry/api/endpoints/group_details.py

@@ -34,6 +34,7 @@ from sentry.plugins.bases import IssueTrackingPlugin2
 from sentry.signals import issue_deleted
 from sentry.utils.safe import safe_execute
 from sentry.utils.apidocs import scenario, attach_scenarios
+from sentry.utils.compat import zip
 
 delete_logger = logging.getLogger("sentry.deletions.api")
 
@@ -160,9 +161,27 @@ class GroupDetailsEndpoint(GroupEndpoint, EnvironmentMixin):
                 version=version,
             )
         except Release.DoesNotExist:
-            return {"version": version}
+            release = {"version": version}
         return serialize(release, request.user)
 
+    def _get_first_last_release_info(self, request, group, versions):
+        releases = {
+            release.version: release
+            for release in Release.objects.filter(
+                projects=group.project,
+                organization_id=group.project.organization_id,
+                version__in=versions,
+            )
+        }
+        serialized_releases = serialize(
+            [releases.get(version) for version in versions], request.user,
+        )
+        # Default to a dictionary if the release object wasn't found and not serialized
+        return [
+            item if item is not None else {"version": version}
+            for item, version in zip(serialized_releases, versions)
+        ]
+
     @attach_scenarios([retrieve_aggregate_scenario])
     def get(self, request, group):
         """
@@ -199,9 +218,13 @@ class GroupDetailsEndpoint(GroupEndpoint, EnvironmentMixin):
 
         action_list = self._get_actions(request, group)
 
-        if first_release:
+        if first_release is not None and last_release is not None:
+            first_release, last_release = self._get_first_last_release_info(
+                request, group, [first_release, last_release]
+            )
+        elif first_release is not None:
             first_release = self._get_release_info(request, group, first_release)
-        if last_release:
+        elif last_release is not None:
             last_release = self._get_release_info(request, group, last_release)
 
         get_range = functools.partial(tsdb.get_range, environment_ids=environment_ids)

src/sentry/api/serializers/models/release.py

@@ -186,10 +186,10 @@ class ReleaseSerializer(Serializer):
 
         first_seen = {}
         last_seen = {}
-        tvs = tagstore.get_release_tags(
+        tag_values = tagstore.get_release_tags(
             project_ids, environment_id=None, versions=[o.version for o in item_list]
         )
-        for tv in tvs:
+        for tv in tag_values:
             first_val = first_seen.get(tv.value)
             last_val = last_seen.get(tv.value)
             first_seen[tv.value] = min(tv.first_seen, first_val) if first_val else tv.first_seen

tests/sentry/api/endpoints/test_group_details.py

@@ -25,11 +25,11 @@ from sentry.models import (
     Release,
     Integration,
 )
-from sentry.testutils import APITestCase
+from sentry.testutils import APITestCase, SnubaTestCase
 from sentry.plugins.base import plugins
 
 
-class GroupDetailsTest(APITestCase):
+class GroupDetailsTest(APITestCase, SnubaTestCase):
     def test_with_numerical_id(self):
         self.login_as(user=self.user)
 
@@ -81,6 +81,20 @@ class GroupDetailsTest(APITestCase):
         assert response.data["id"] == six.text_type(group.id)
         assert response.data["firstRelease"]["version"] == "1.0"
 
+    def test_no_releases(self):
+        self.login_as(user=self.user)
+
+        event = self.store_event(data={}, project_id=self.project.id)
+
+        group = event.group
+
+        url = u"/api/0/issues/{}/".format(group.id)
+
+        response = self.client.get(url, format="json")
+        assert response.status_code == 200, response.content
+        assert response.data["firstRelease"] is None
+        assert response.data["lastRelease"] is None
+
     def test_pending_delete_pending_merge_excluded(self):
         group1 = self.create_group(status=GroupStatus.PENDING_DELETION)
         group2 = self.create_group(status=GroupStatus.DELETION_IN_PROGRESS)

tests/snuba/api/endpoints/test_group_details.py

@@ -1,9 +1,11 @@
 from __future__ import absolute_import, print_function
 
+import six
 from sentry.utils.compat import mock
 
-from sentry.models import Environment
+from sentry.models import Environment, Release
 from sentry.testutils import APITestCase, SnubaTestCase
+from sentry.testutils.helpers.datetime import before_now, iso_format
 
 
 class GroupDetailsTest(APITestCase, SnubaTestCase):
@@ -31,3 +33,99 @@ class GroupDetailsTest(APITestCase, SnubaTestCase):
 
         response = self.client.get("%s?environment=invalid" % (url,), format="json")
         assert response.status_code == 404
+
+    def test_with_first_last_release(self):
+        self.login_as(user=self.user)
+        first_release = {
+            "firstEvent": before_now(minutes=3),
+            "lastEvent": before_now(minutes=2, seconds=30),
+        }
+        last_release = {
+            "firstEvent": before_now(minutes=1, seconds=30),
+            "lastEvent": before_now(minutes=1),
+        }
+
+        for timestamp in first_release.values():
+            self.store_event(
+                data={"release": "1.0", "timestamp": iso_format(timestamp)},
+                project_id=self.project.id,
+            )
+        self.store_event(
+            data={"release": "1.1", "timestamp": iso_format(before_now(minutes=2))},
+            project_id=self.project.id,
+        )
+        for timestamp in last_release.values():
+            event = self.store_event(
+                data={"release": "1.0a", "timestamp": iso_format(timestamp)},
+                project_id=self.project.id,
+            )
+
+        group = event.group
+
+        url = u"/api/0/issues/{}/".format(group.id)
+        response = self.client.get(url, format="json")
+
+        assert response.status_code == 200, response.content
+        assert response.data["id"] == six.text_type(group.id)
+        release = response.data["firstRelease"]
+        assert release["version"] == "1.0"
+        for event, timestamp in six.iteritems(first_release):
+            assert release[event].ctime() == timestamp.ctime()
+        release = response.data["lastRelease"]
+        assert release["version"] == "1.0a"
+        for event, timestamp in six.iteritems(last_release):
+            assert release[event].ctime() == timestamp.ctime()
+
+    def test_first_last_only_one_tagstore(self):
+        self.login_as(user=self.user)
+
+        event = self.store_event(
+            data={"release": "1.0", "timestamp": iso_format(before_now(days=3))},
+            project_id=self.project.id,
+        )
+        self.store_event(
+            data={"release": "1.1", "timestamp": iso_format(before_now(minutes=3))},
+            project_id=self.project.id,
+        )
+
+        group = event.group
+
+        url = u"/api/0/issues/{}/".format(group.id)
+
+        with mock.patch(
+            "sentry.api.endpoints.group_details.tagstore.get_release_tags"
+        ) as get_release_tags:
+            response = self.client.get(url, format="json")
+            assert response.status_code == 200
+            assert get_release_tags.call_count == 1
+
+    def test_first_release_only(self):
+        self.login_as(user=self.user)
+
+        first_event = before_now(days=3)
+
+        self.store_event(
+            data={"release": "1.0", "timestamp": iso_format(first_event)},
+            project_id=self.project.id,
+        )
+        event = self.store_event(
+            data={"release": "1.1", "timestamp": iso_format(before_now(days=1))},
+            project_id=self.project.id,
+        )
+        # Forcibly remove one of the releases
+        Release.objects.get(version="1.1").delete()
+
+        group = event.group
+
+        url = u"/api/0/issues/{}/".format(group.id)
+
+        response = self.client.get(url, format="json")
+        assert response.status_code == 200, response.content
+        assert response.data["firstRelease"]["version"] == "1.0"
+        # only one event
+        assert (
+            response.data["firstRelease"]["firstEvent"]
+            == response.data["firstRelease"]["lastEvent"]
+        )
+        assert response.data["firstRelease"]["firstEvent"].ctime() == first_event.ctime()
+        assert response.data["lastRelease"] == {"version": "1.1"}