ref(shared-views): Remove position usage in groupsearchview serializer (#87297)

This PR refactors the groupsearchview serializer and consequently the
`PUT` `/groupsearchviews` endpoint by removing the usage of the
GroupSearchView.position column.

This was the last non-test use case of the position column that needed
to be removed before dropping the column.

Author: MichaelSun48

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

@@ -13,7 +13,6 @@ class GroupSearchViewSerializerResponse(TypedDict):
     name: str
     query: str
     querySort: SORT_LITERALS
-    position: int
     projects: list[int]
     isAllProjects: bool
     environments: list[str]
@@ -23,6 +22,10 @@ class GroupSearchViewSerializerResponse(TypedDict):
     dateUpdated: str
 
 
+class GroupSearchViewStarredSerializerResponse(GroupSearchViewSerializerResponse):
+    position: int
+
+
 @register(GroupSearchView)
 class GroupSearchViewSerializer(Serializer):
     def __init__(self, *args, **kwargs):
@@ -67,7 +70,6 @@ def serialize(self, obj, attrs, user, **kwargs) -> GroupSearchViewSerializerResp
             "name": obj.name,
             "query": obj.query,
             "querySort": obj.query_sort,
-            "position": obj.position,
             "projects": projects,
             "isAllProjects": is_all_projects,
             "environments": obj.environments,
@@ -86,7 +88,7 @@ def __init__(self, *args, **kwargs):
         self.organization = kwargs.pop("organization", None)
         super().__init__(*args, **kwargs)
 
-    def serialize(self, obj, attrs, user, **kwargs) -> GroupSearchViewSerializerResponse:
+    def serialize(self, obj, attrs, user, **kwargs) -> GroupSearchViewStarredSerializerResponse:
         serialized_view: GroupSearchViewSerializerResponse = serialize(
             obj.group_search_view,
             user,

src/sentry/issues/endpoints/organization_group_search_views.py

@@ -13,15 +13,13 @@
 from sentry.api.helpers.group_index.validators import ValidationError
 from sentry.api.paginator import SequencePaginator
 from sentry.api.serializers import serialize
-from sentry.api.serializers.models.groupsearchview import (
-    GroupSearchViewSerializer,
-    GroupSearchViewStarredSerializer,
-)
+from sentry.api.serializers.models.groupsearchview import GroupSearchViewStarredSerializer
 from sentry.api.serializers.rest_framework.groupsearchview import (
     GroupSearchViewValidator,
     GroupSearchViewValidatorResponse,
 )
 from sentry.models.groupsearchview import DEFAULT_TIME_FILTER, GroupSearchView
+from sentry.models.groupsearchviewlastvisited import GroupSearchViewLastVisited
 from sentry.models.groupsearchviewstarred import GroupSearchViewStarred
 from sentry.models.organization import Organization
 from sentry.models.project import Project
@@ -157,7 +155,9 @@ def put(self, request: Request, organization: Organization) -> Response:
 
         try:
             with transaction.atomic(using=router.db_for_write(GroupSearchView)):
-                bulk_update_views(organization, request.user.id, validated_data["views"])
+                new_view_state = bulk_update_views(
+                    organization, request.user.id, validated_data["views"]
+                )
         except IntegrityError as e:
             if (
                 len(e.args) > 0
@@ -171,13 +171,38 @@ def put(self, request: Request, organization: Organization) -> Response:
                 )
             return Response(status=status.HTTP_500_INTERNAL_SERVER_ERROR)
 
-        query = GroupSearchView.objects.filter(organization=organization, user_id=request.user.id)
+        last_visited_views = GroupSearchViewLastVisited.objects.filter(
+            organization=organization,
+            user_id=request.user.id,
+            group_search_view_id__in=[view.id for view in new_view_state],
+        )
+        last_visited_map = {lv.group_search_view_id: lv for lv in last_visited_views}
 
         return self.paginate(
             request=request,
-            queryset=query,
-            order_by="position",
-            on_results=lambda x: serialize(x, request.user, serializer=GroupSearchViewSerializer()),
+            paginator=SequencePaginator(
+                [
+                    (
+                        idx,
+                        {
+                            "id": str(view.id),
+                            "name": view.name,
+                            "query": view.query,
+                            "querySort": view.query_sort,
+                            "projects": list(view.projects.values_list("id", flat=True)),
+                            "isAllProjects": view.is_all_projects,
+                            "environments": view.environments,
+                            "timeFilters": view.time_filters,
+                            "dateCreated": view.date_added,
+                            "dateUpdated": view.date_updated,
+                            "lastVisited": last_visited_map.get(view.id, None),
+                            "position": idx,
+                        },
+                    )
+                    for idx, view in enumerate(new_view_state)
+                ]
+            ),
+            on_results=lambda results: serialize(results, request.user),
         )
 
 
@@ -198,16 +223,18 @@ def validate_projects(
 
 def bulk_update_views(
     org: Organization, user_id: int, views: list[GroupSearchViewValidatorResponse]
-) -> None:
+) -> list[GroupSearchView]:
     existing_view_ids = [view["id"] for view in views if "id" in view]
 
     _delete_missing_views(org, user_id, view_ids_to_keep=existing_view_ids)
-
+    created_views = []
     for idx, view in enumerate(views):
         if "id" not in view:
-            _create_view(org, user_id, view, position=idx)
+            created_views.append(_create_view(org, user_id, view, position=idx))
         else:
-            _update_existing_view(org, user_id, view, position=idx)
+            created_views.append(_update_existing_view(org, user_id, view, position=idx))
+
+    return created_views
 
 
 def pick_default_project(org: Organization, user: User | AnonymousUser) -> int | None:
@@ -230,7 +257,7 @@ def _delete_missing_views(org: Organization, user_id: int, view_ids_to_keep: lis
 
 def _update_existing_view(
     org: Organization, user_id: int, view: GroupSearchViewValidatorResponse, position: int
-) -> None:
+) -> GroupSearchView:
     try:
         gsv = GroupSearchView.objects.get(id=view["id"], user_id=user_id)
         gsv.name = view["name"]
@@ -255,17 +282,18 @@ def _update_existing_view(
             group_search_view=gsv,
             defaults={"position": position},
         )
+        return gsv
     except GroupSearchView.DoesNotExist:
         # It is possible – though unlikely under normal circumstances – for a view to come in that
         # doesn't exist anymore. If, for example, the user has the issue stream open in separate
         # windows, deletes a view in one window, then updates it in the other before refreshing.
         # In this case, we decide to recreate the tab instead of leaving it deleted.
-        _create_view(org, user_id, view, position)
+        return _create_view(org, user_id, view, position)
 
 
 def _create_view(
     org: Organization, user_id: int, view: GroupSearchViewValidatorResponse, position: int
-) -> None:
+) -> GroupSearchView:
     gsv = GroupSearchView.objects.create(
         organization=org,
         user_id=user_id,
@@ -286,3 +314,4 @@ def _create_view(
         group_search_view=gsv,
         position=position,
     )
+    return gsv

tests/sentry/issues/endpoints/test_organization_group_search_views.py

@@ -2,7 +2,6 @@
 from django.utils import timezone
 from rest_framework.exceptions import ErrorDetail
 
-from sentry.api.serializers.base import serialize
 from sentry.api.serializers.rest_framework.groupsearchview import GroupSearchViewValidatorResponse
 from sentry.issues.endpoints.organization_group_search_views import DEFAULT_VIEWS
 from sentry.models.groupsearchview import GroupSearchView
@@ -136,7 +135,12 @@ def test_get_user_one_custom_views(self) -> None:
         self.login_as(user=self.user)
         response = self.get_success_response(self.organization.slug)
 
-        assert response.data == serialize(objs["user_one_views"])
+        assert response.data[0]["id"] == str(objs["user_one_views"][0].id)
+        assert response.data[0]["position"] == 0
+        assert response.data[1]["id"] == str(objs["user_one_views"][1].id)
+        assert response.data[1]["position"] == 1
+        assert response.data[2]["id"] == str(objs["user_one_views"][2].id)
+        assert response.data[2]["position"] == 2
 
     @with_feature({"organizations:issue-stream-custom-views": True})
     @with_feature({"organizations:global-views": True})
@@ -169,7 +173,10 @@ def test_get_user_two_custom_views(self) -> None:
         self.login_as(user=self.user_2)
         response = self.get_success_response(self.organization.slug)
 
-        assert response.data == serialize(objs["user_two_views"])
+        assert response.data[0]["id"] == str(objs["user_two_views"][0].id)
+        assert response.data[0]["position"] == 0
+        assert response.data[1]["id"] == str(objs["user_two_views"][1].id)
+        assert response.data[1]["position"] == 1
 
     @with_feature({"organizations:issue-stream-custom-views": True})
     @with_feature({"organizations:global-views": True})