ref(shared-views): Refactor delete endpoint permissions and model deletions

src/sentry/issues/endpoints/organization_group_search_view_details.py

@@ -1,6 +1,5 @@
 from typing import Any, NotRequired, TypedDict
 
-from django.db.models import F
 from rest_framework import status
 from rest_framework.request import Request
 from rest_framework.response import Response
@@ -15,6 +14,7 @@
 from sentry.api.serializers.rest_framework.groupsearchview import GroupSearchViewDetailsPutValidator
 from sentry.issues.endpoints.organization_group_search_views import pick_default_project
 from sentry.models.groupsearchview import GroupSearchView
+from sentry.models.groupsearchviewlastvisited import GroupSearchViewLastVisited
 from sentry.models.groupsearchviewstarred import GroupSearchViewStarred
 from sentry.models.organization import Organization
 from sentry.models.savedsearch import SORT_LITERALS
@@ -111,30 +111,33 @@ def delete(self, request: Request, organization: Organization, view_id: str) ->
             return Response(status=status.HTTP_404_NOT_FOUND)
 
         try:
-            view = GroupSearchView.objects.get(
-                id=view_id, organization=organization, user_id=request.user.id
-            )
+            view = GroupSearchView.objects.get(id=view_id)
         except GroupSearchView.DoesNotExist:
             return Response(status=status.HTTP_404_NOT_FOUND)
 
-        # Check if the view is starred by the current user
+        # Only view creators and org admins can delete views
+        has_delete_access = (
+            view.user_id == request.user.id
+            or request.access.has_scope("org:admin")
+            or request.access.has_scope("team:admin")
+        )
+        if not has_delete_access:
+            return Response(status=status.HTTP_403_FORBIDDEN)
+
         try:
-            starredView = GroupSearchViewStarred.objects.get(
-                organization=organization, user_id=request.user.id, group_search_view=view
+            GroupSearchViewStarred.objects.clear_starred_view_for_all_members(
+                organization=organization, view=view
             )
-            deleted_position = starredView.position
-            # Delete the starred view
-            starredView.delete()
-
-            # Decrement the position of all other starred views with higher position
-            GroupSearchViewStarred.objects.filter(
-                organization=organization,
-                user_id=request.user.id,
-                position__gt=deleted_position,
-            ).update(position=F("position") - 1)
-
         except GroupSearchViewStarred.DoesNotExist:
             pass
 
+        try:
+            GroupSearchViewLastVisited.objects.filter(
+                organization=organization, group_search_view=view
+            ).delete()
+        except GroupSearchViewLastVisited.DoesNotExist:
+            pass
+
         view.delete()
+
         return Response(status=status.HTTP_204_NO_CONTENT)

src/sentry/models/groupsearchviewstarred.py

@@ -142,6 +142,10 @@ def delete_starred_view(
             ).update(position=models.F("position") - 1)
             return True
 
+    def clear_starred_view_for_all_members(self, organization: Organization, view: GroupSearchView):
+        for starred_view in self.filter(organization=organization, group_search_view=view):
+            self.delete_starred_view(organization, starred_view.user_id, view)
+
 
 @region_silo_model
 class GroupSearchViewStarred(DefaultFieldsModel):

tests/sentry/issues/endpoints/test_organization_group_search_view_details.py

@@ -1,8 +1,11 @@
 from django.urls import reverse
+from django.utils import timezone
 
 from sentry.models.groupsearchview import GroupSearchView
+from sentry.models.groupsearchviewlastvisited import GroupSearchViewLastVisited
 from sentry.models.groupsearchviewstarred import GroupSearchViewStarred
 from sentry.silo.base import SiloMode
+from sentry.testutils.cases import APITestCase
 from sentry.testutils.helpers import with_feature
 from sentry.testutils.silo import assume_test_silo_mode
 from tests.sentry.issues.endpoints.test_organization_group_search_views import BaseGSVTestCase
@@ -54,18 +57,39 @@ def test_delete_nonexistent_view(self) -> None:
     @with_feature({"organizations:issue-stream-custom-views": True})
     def test_delete_view_from_another_user(self) -> None:
         # Get a view ID from user_two
-        view_id = str(self.base_data["user_two_views"][0].id)
+        self.login_as(user=self.user_2)
+        view_id = str(self.base_data["user_one_views"][0].id)
         url = reverse(
             "sentry-api-0-organization-group-search-view-details",
             kwargs={"organization_id_or_slug": self.organization.slug, "view_id": view_id},
         )
 
         response = self.client.delete(url)
-        assert response.status_code == 404
+        assert response.status_code == 403
 
         # Verify the view still exists (this will error out if not)
         GroupSearchView.objects.get(id=view_id)
 
+    @with_feature({"organizations:issue-stream-custom-views": True})
+    def test_admin_can_delete_view_from_another_user(self) -> None:
+        self.admin_user = self.create_user()
+        self.create_member(
+            user=self.admin_user,
+            organization=self.organization,
+            role="admin",
+        )
+        self.login_as(user=self.admin_user)
+        view_id = str(self.base_data["user_one_views"][0].id)
+        url = reverse(
+            "sentry-api-0-organization-group-search-view-details",
+            kwargs={"organization_id_or_slug": self.organization.slug, "view_id": view_id},
+        )
+
+        response = self.client.delete(url)
+        assert response.status_code == 204
+
+        assert not GroupSearchView.objects.filter(id=view_id).exists()
+
     @with_feature({"organizations:issue-stream-custom-views": True})
     def test_delete_first_starred_view_decrements_succeeding_positions(self) -> None:
         # Delete the first view
@@ -123,6 +147,112 @@ def test_delete_without_feature_flag(self) -> None:
         GroupSearchView.objects.get(id=self.view_id)
 
 
+class OrganizationGroupSearchViewsDeleteStarredAndLastVisitedTest(APITestCase):
+    endpoint = "sentry-api-0-organization-group-search-view-details"
+
+    def setUp(self) -> None:
+        super().setUp()
+
+        self.user_1 = self.user
+        self.user_2 = self.create_user()
+        self.create_member(organization=self.organization, user=self.user_2)
+
+        self.user_1_view = GroupSearchView.objects.create(
+            organization=self.organization,
+            user_id=self.user_1.id,
+            name="User 1's View",
+            query="is:unresolved",
+        )
+        GroupSearchViewStarred.objects.create(
+            organization=self.organization,
+            user_id=self.user_1.id,
+            group_search_view=self.user_1_view,
+            position=0,
+        )
+        GroupSearchViewLastVisited.objects.create(
+            organization=self.organization,
+            user_id=self.user_1.id,
+            group_search_view=self.user_1_view,
+            last_visited=timezone.now(),
+        )
+
+        self.user_2_view = GroupSearchView.objects.create(
+            organization=self.organization,
+            user_id=self.user_2.id,
+            name="User 2's View",
+            query="is:unresolved",
+        )
+
+        GroupSearchViewStarred.objects.create(
+            organization=self.organization,
+            user_id=self.user_2.id,
+            group_search_view=self.user_1_view,
+            position=0,
+        )
+        GroupSearchViewStarred.objects.create(
+            organization=self.organization,
+            user_id=self.user_2.id,
+            group_search_view=self.user_2_view,
+            position=1,
+        )
+
+        GroupSearchViewLastVisited.objects.create(
+            organization=self.organization,
+            user_id=self.user_2.id,
+            group_search_view=self.user_1_view,
+            last_visited=timezone.now(),
+        )
+
+    @with_feature({"organizations:issue-stream-custom-views": True})
+    def test_cannot_delete_other_users_view(self) -> None:
+        self.login_as(user=self.user_2)
+
+        response = self.client.delete(
+            reverse(
+                self.endpoint,
+                kwargs={
+                    "organization_id_or_slug": self.organization.slug,
+                    "view_id": self.user_1_view.id,
+                },
+            )
+        )
+
+        assert response.status_code == 403
+
+    @with_feature({"organizations:issue-stream-custom-views": True})
+    def test_deleting_my_view_deletes_from_others_starred_views(self) -> None:
+        self.login_as(user=self.user_1)
+
+        response = self.client.delete(
+            reverse(
+                self.endpoint,
+                kwargs={
+                    "organization_id_or_slug": self.organization.slug,
+                    "view_id": self.user_1_view.id,
+                },
+            )
+        )
+
+        assert response.status_code == 204
+        # User 2 starred User 1's view. After User 1 deletes the view, it should not be starred for anyone anymore
+        assert not GroupSearchViewStarred.objects.filter(
+            organization_id=self.organization.id, group_search_view=self.user_1_view
+        ).exists()
+        # User 2's other starred view should be moved up to position 0
+        assert (
+            GroupSearchViewStarred.objects.get(
+                organization_id=self.organization.id,
+                user_id=self.user_2.id,
+                group_search_view=self.user_2_view,
+            ).position
+            == 0
+        )
+        # All last visited entries should be deleted as well
+        assert not GroupSearchViewLastVisited.objects.filter(
+            organization_id=self.organization.id, group_search_view=self.user_1_view
+        ).exists()
+
+
 class OrganizationGroupSearchViewsPutTest(BaseGSVTestCase):
     endpoint = "sentry-api-0-organization-group-search-view-details"
     method = "put"