From dc28b3fccc7c771203386efee8c79cfb4e22bcad Mon Sep 17 00:00:00 2001 From: lzhao-sentry Date: Tue, 8 Sep 2026 12:32:59 -0400 Subject: [PATCH 1/5] feat(discover): add starred manager and endpoint --- .../endpoints/discover_saved_query_starred.py | 81 ++++++++++++ src/sentry/discover/models.py | 116 +++++++++++++++++- .../test_discover_saved_query_starred.py | 57 +++++++++ 3 files changed, 253 insertions(+), 1 deletion(-) create mode 100644 src/sentry/discover/endpoints/discover_saved_query_starred.py create mode 100644 tests/sentry/discover/test_discover_saved_query_starred.py diff --git a/src/sentry/discover/endpoints/discover_saved_query_starred.py b/src/sentry/discover/endpoints/discover_saved_query_starred.py new file mode 100644 index 000000000000..c30b7fd373da --- /dev/null +++ b/src/sentry/discover/endpoints/discover_saved_query_starred.py @@ -0,0 +1,81 @@ +from rest_framework import serializers, status +from rest_framework.request import Request +from rest_framework.response import Response + +from sentry import features +from sentry.api.api_owners import ApiOwner +from sentry.api.api_publish_status import ApiPublishStatus +from sentry.api.base import cell_silo_endpoint +from sentry.api.bases.organization import OrganizationEndpoint, OrganizationPermission +from sentry.discover.models import DiscoverSavedQuery, DiscoverSavedQueryStarred +from sentry.models.organization import Organization + + +class StarQuerySerializer(serializers.Serializer): + starred = serializers.BooleanField(required=True) + position = serializers.IntegerField(required=False) + + def validate(self, data): + if not data["starred"] and "position" in data: + raise serializers.ValidationError("Position is only allowed when starring a query.") + return data + + +class MemberPermission(OrganizationPermission): + scope_map = { + "POST": ["member:read", "member:write"], + } + + +@cell_silo_endpoint +class DiscoverSavedQueryStarredEndpoint(OrganizationEndpoint): + """ + Star or unstar a single saved Discover query. + """ + + publish_status = { + "POST": ApiPublishStatus.EXPERIMENTAL, + } + owner = ApiOwner.DATA_BROWSING + permission_classes = (MemberPermission,) + + def has_feature(self, organization, request): + return features.has( + "organizations:visibility-explore-view", organization, actor=request.user + ) and features.has( + "organizations:discover-queries-in-all-queries", organization, actor=request.user + ) + + def post(self, request: Request, organization: Organization, id: int) -> Response: + """ + Update the starred status of a saved Discover query for the current organization member. + """ + if not request.user.is_authenticated: + return Response(status=status.HTTP_400_BAD_REQUEST) + + if not self.has_feature(organization, request): + return self.respond(status=404) + + serializer = StarQuerySerializer(data=request.data) + if not serializer.is_valid(): + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + is_starred = serializer.validated_data["starred"] + + try: + query = DiscoverSavedQuery.objects.get(id=id, organization=organization) + except DiscoverSavedQuery.DoesNotExist: + return Response(status=status.HTTP_404_NOT_FOUND) + + if is_starred: + if DiscoverSavedQueryStarred.objects.insert_starred_query( + organization, request.user.id, query + ): + return Response(status=status.HTTP_200_OK) + else: + if DiscoverSavedQueryStarred.objects.delete_starred_query( + organization, request.user.id, query + ): + return Response(status=status.HTTP_200_OK) + + return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/src/sentry/discover/models.py b/src/sentry/discover/models.py index 8f13623acab9..b6ee1a7b8a45 100644 --- a/src/sentry/discover/models.py +++ b/src/sentry/discover/models.py @@ -3,7 +3,7 @@ from enum import Enum from typing import ClassVar -from django.db import models, router, transaction +from django.db import IntegrityError, models, router, transaction from django.db.models import Q, UniqueConstraint from django.utils import timezone @@ -15,6 +15,7 @@ from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey from sentry.db.models.manager.base import BaseManager from sentry.models.dashboard_widget import TypesClass +from sentry.models.organization import Organization from sentry.models.projectteam import ProjectTeam from sentry.tasks.relay import schedule_invalidate_project_config @@ -213,6 +214,117 @@ class Meta: unique_together = (("project_team", "transaction"),) +class DiscoverSavedQueryStarredManager(BaseManager["DiscoverSavedQueryStarred"]): + """ + Positions here are not local to this table, being shared with ExploreSavedQueryStarred. + See `explore/utils.py` and `saved_query_starred_order.py` for the implementation details. + """ + + def get_starred_query( + self, organization: Organization, user_id: int, query: DiscoverSavedQuery + ) -> DiscoverSavedQueryStarred | None: + """ + Returns the starred query if it exists, otherwise None. + """ + return self.filter( + organization=organization, user_id=user_id, discover_saved_query=query + ).first() + + def insert_starred_query( + self, + organization: Organization, + user_id: int, + query: DiscoverSavedQuery, + starred: bool = True, + ) -> bool: + """ + Inserts a new starred query at the end of the shared list. + + Args: + organization: The organization the queries belong to + user_id: The ID of the user whose starred queries are being updated + discover_saved_query: The query to insert + + Returns: + True if the query was starred, False if the query was already starred + """ + from sentry.explore.utils import next_starred_position + + try: + with transaction.atomic(using=router.db_for_write(DiscoverSavedQueryStarred)): + if self.get_starred_query(organization, user_id, query): + return False + + self.create( + organization=organization, + user_id=user_id, + discover_saved_query=query, + position=next_starred_position(organization, user_id), + starred=starred, + ) + return True + except IntegrityError: + # A concurrent request starred the same query first + return False + + def delete_starred_query( + self, organization: Organization, user_id: int, query: DiscoverSavedQuery + ) -> bool: + """ + Deletes a starred query from the list. + Decrements the position of all later queries in both tables to close the gap. + + Args: + organization: The organization the queries belong to + user_id: The ID of the user whose starred queries are being updated + discover_saved_query: The query to delete + + Returns: + True if the query was unstarred, False if the query was already unstarred + """ + from sentry.explore.utils import shift_starred_positions_by_one + + with transaction.atomic(using=router.db_for_write(DiscoverSavedQueryStarred)): + if not (starred_query := self.get_starred_query(organization, user_id, query)): + return False + + deleted_position = starred_query.position + starred_query.delete() + + # A row unstarred via ``updated_starred_query`` holds no position and so left no + # gap to close. Filtering on ``position__gt=None`` would raise, not match nothing. + if deleted_position is not None: + shift_starred_positions_by_one( + organization, user_id, from_position=deleted_position + ) + return True + + def updated_starred_query( + self, + organization: Organization, + user_id: int, + query: DiscoverSavedQuery, + starred: bool, + ) -> bool: + """ + Updates the starred status of a query. + """ + from sentry.explore.utils import next_starred_position + + with transaction.atomic(using=router.db_for_write(DiscoverSavedQueryStarred)): + if not (starred_query := self.get_starred_query(organization, user_id, query)): + return False + + starred_query.starred = starred + if starred: + starred_query.position = next_starred_position(organization, user_id) + else: + starred_query.position = None + + starred_query.save() + return True + + @cell_silo_model class DiscoverSavedQueryStarred(DefaultFieldsModel): __relocation_scope__ = RelocationScope.Excluded @@ -224,6 +336,8 @@ class DiscoverSavedQueryStarred(DefaultFieldsModel): position = models.PositiveSmallIntegerField(null=True, db_default=None) starred = models.BooleanField(db_default=True) + objects: ClassVar[DiscoverSavedQueryStarredManager] = DiscoverSavedQueryStarredManager() + class Meta: app_label = "discover" db_table = "sentry_discoversavedquerystarred" diff --git a/tests/sentry/discover/test_discover_saved_query_starred.py b/tests/sentry/discover/test_discover_saved_query_starred.py new file mode 100644 index 000000000000..809c561931ee --- /dev/null +++ b/tests/sentry/discover/test_discover_saved_query_starred.py @@ -0,0 +1,57 @@ +import pytest +from django.urls import reverse + +from sentry.discover.models import DiscoverSavedQuery, DiscoverSavedQueryStarred +from sentry.testutils.cases import APITestCase + + +@pytest.mark.skip(reason="API not public yet, this line will be removed in future") +class DiscoverSavedQueryStarredTest(APITestCase): + feature_flags = { + "organizations:visibility-explore-view": True, + "organizations:discover-queries-in-all-queries": True, + } + + def setUp(self) -> None: + super().setUp() + self.login_as(user=self.user) + self.org = self.create_organization(owner=self.user) + self.project_ids = [ + self.create_project(organization=self.org).id, + self.create_project(organization=self.org).id, + ] + query = {"fields": ["title"], "conditions": "", "limit": 10} + + model = DiscoverSavedQuery.objects.create( + organization=self.org, created_by_id=self.user.id, name="Test query", query=query + ) + + model.set_projects(self.project_ids) + + self.query_id = model.id + + self.url = reverse( + "sentry-api-0-discover-saved-query-starred", args=[self.org.slug, self.query_id] + ) + + def test_post(self) -> None: + with self.feature(self.feature_flags): + assert not DiscoverSavedQuery.objects.filter( + id__in=DiscoverSavedQueryStarred.objects.filter( + organization=self.org, user_id=self.user.id + ).values_list("discover_saved_query_id", flat=True) + ).exists() + response = self.client.post(self.url, data={"starred": "1"}) + assert response.status_code == 200, response.content + assert DiscoverSavedQuery.objects.filter( + id__in=DiscoverSavedQueryStarred.objects.filter( + organization=self.org, user_id=self.user.id + ).values_list("discover_saved_query_id", flat=True) + ).exists() + response = self.client.post(self.url, data={"starred": "0"}) + assert response.status_code == 200, response.content + assert not DiscoverSavedQuery.objects.filter( + id__in=DiscoverSavedQueryStarred.objects.filter( + organization=self.org, user_id=self.user.id + ).values_list("discover_saved_query_id", flat=True) + ).exists() From c18a593b96bd517ee6226ca1dc6a9a81aad53905 Mon Sep 17 00:00:00 2001 From: lzhao-sentry Date: Tue, 8 Sep 2026 16:01:59 -0400 Subject: [PATCH 2/5] make reordering work around dupes --- .../endpoints/saved_query_starred_order.py | 4 +-- src/sentry/explore/utils.py | 27 +++++++++---------- tests/sentry/explore/test_utils.py | 9 ------- 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/src/sentry/explore/endpoints/saved_query_starred_order.py b/src/sentry/explore/endpoints/saved_query_starred_order.py index 2c4dc348ef28..8ff3c71f844e 100644 --- a/src/sentry/explore/endpoints/saved_query_starred_order.py +++ b/src/sentry/explore/endpoints/saved_query_starred_order.py @@ -1,6 +1,6 @@ from typing import Any -from django.db import router, transaction +from django.db import IntegrityError, router, transaction from rest_framework import serializers, status from rest_framework.exceptions import ParseError from rest_framework.request import Request @@ -84,7 +84,7 @@ def put(self, request: Request, organization: Organization) -> Response: # DiscoverSavedQueryStarred should be in the same db as ExploreSavedQueryStarred. with transaction.atomic(using=router.db_for_write(ExploreSavedQueryStarred)): utils.reorder_starred_queries(organization, request.user.id, refs) - except ValueError: + except (IntegrityError, ValueError): raise ParseError("Mismatch between existing and provided starred queries.") return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/src/sentry/explore/utils.py b/src/sentry/explore/utils.py index 0351ff1a9bce..0d7343cbd29e 100644 --- a/src/sentry/explore/utils.py +++ b/src/sentry/explore/utils.py @@ -90,17 +90,14 @@ def reorder_starred_queries( starred query the user has, not just those of one product. Raises: - ValueError: if ``refs`` is not exactly the set of the user's starred rows, or - contains a duplicate. + ValueError: if ``refs`` is not exactly the set of the user's starred rows """ requested = list(refs) - if len(requested) != len(set(requested)): - raise ValueError("Single query cannot take up multiple positions.") # grab all starred queries in both tables, and map based on SavedQueryRef. discover_starred_queries = DiscoverSavedQueryStarred.objects.filter( - organization=organization, user_id=user_id, position__isnull=False - ).filter(organization=organization, user_id=user_id, position__isnull=False, starred=True) + organization=organization, user_id=user_id, position__isnull=False, starred=True + ) explore_starred_queries = ExploreSavedQueryStarred.objects.filter( organization=organization, user_id=user_id, position__isnull=False, starred=True @@ -123,17 +120,19 @@ def reorder_starred_queries( raise ValueError("Mismatch between existing and provided starred queries.") # normalize positions to 1...N, then assign them in order of the ref sequence provided - slots = range(1, len(requested) + 1) + position_map = {ref: position for position, ref in enumerate(requested, start=1)} discover_updates: list[DiscoverSavedQueryStarred] = [] explore_updates: list[ExploreSavedQueryStarred] = [] - for ref, new_position in zip(requested, slots): - row = combined_starred_queries_map[ref] - row.position = new_position - if isinstance(row, ExploreSavedQueryStarred): - explore_updates.append(row) - else: - discover_updates.append(row) + for row in discover_starred_queries: + ref = SavedQueryRef(SavedQueryType.DISCOVER, row.discover_saved_query_id) + row.position = position_map[ref] + discover_updates.append(row) + + for row in explore_starred_queries: + ref = SavedQueryRef(SavedQueryType.EXPLORE, row.explore_saved_query_id) + row.position = position_map[ref] + explore_updates.append(row) ExploreSavedQueryStarred.objects.bulk_update(explore_updates, ["position"]) DiscoverSavedQueryStarred.objects.bulk_update(discover_updates, ["position"]) diff --git a/tests/sentry/explore/test_utils.py b/tests/sentry/explore/test_utils.py index 3fb0e2c40652..eadd4fa6ee44 100644 --- a/tests/sentry/explore/test_utils.py +++ b/tests/sentry/explore/test_utils.py @@ -159,15 +159,6 @@ def test_moves_a_query_from_the_end_to_the_front(self) -> None: assert self.ordered_refs() == refs - def test_rejects_duplicate_ref(self) -> None: - discover = self.discover_star(1) - self.explore_star(2) - - ref = SavedQueryRef(SavedQueryType.DISCOVER, discover.discover_saved_query_id) - - with pytest.raises(ValueError, match="multiple positions"): - utils.reorder_starred_queries(self.org, self.user.id, [ref, ref]) - def test_rejects_missing_refs(self) -> None: # The failure mode this module exists to prevent: a caller that knows about one # product sends only its own queries, and the other product's positions are lost. From 02a75c797cd985fba738f79cec9752981d1a973f Mon Sep 17 00:00:00 2001 From: lzhao-sentry Date: Tue, 8 Sep 2026 16:32:26 -0400 Subject: [PATCH 3/5] fix mypy, remove unneeded test --- src/sentry/explore/utils.py | 36 +++++++++---------- .../test_saved_query_starred_order.py | 16 --------- 2 files changed, 17 insertions(+), 35 deletions(-) diff --git a/src/sentry/explore/utils.py b/src/sentry/explore/utils.py index 0d7343cbd29e..1ddd35b81fa4 100644 --- a/src/sentry/explore/utils.py +++ b/src/sentry/explore/utils.py @@ -92,7 +92,7 @@ def reorder_starred_queries( Raises: ValueError: if ``refs`` is not exactly the set of the user's starred rows """ - requested = list(refs) + new_query_positions = list(refs) # grab all starred queries in both tables, and map based on SavedQueryRef. discover_starred_queries = DiscoverSavedQueryStarred.objects.filter( @@ -103,36 +103,34 @@ def reorder_starred_queries( organization=organization, user_id=user_id, position__isnull=False, starred=True ) - combined_starred_queries_map: dict[ - SavedQueryRef, DiscoverSavedQueryStarred | ExploreSavedQueryStarred - ] = {} + existing_query_refs: set[SavedQueryRef] = set() for discover_row in discover_starred_queries: - combined_starred_queries_map[ + existing_query_refs.add( SavedQueryRef(SavedQueryType.DISCOVER, discover_row.discover_saved_query_id) - ] = discover_row + ) for explore_row in explore_starred_queries: - combined_starred_queries_map[ + existing_query_refs.add( SavedQueryRef(SavedQueryType.EXPLORE, explore_row.explore_saved_query_id) - ] = explore_row + ) - if combined_starred_queries_map.keys() != set(requested): + if existing_query_refs != set(new_query_positions): raise ValueError("Mismatch between existing and provided starred queries.") # normalize positions to 1...N, then assign them in order of the ref sequence provided - position_map = {ref: position for position, ref in enumerate(requested, start=1)} + position_map = {ref: position for position, ref in enumerate(new_query_positions, start=1)} discover_updates: list[DiscoverSavedQueryStarred] = [] explore_updates: list[ExploreSavedQueryStarred] = [] - for row in discover_starred_queries: - ref = SavedQueryRef(SavedQueryType.DISCOVER, row.discover_saved_query_id) - row.position = position_map[ref] - discover_updates.append(row) - - for row in explore_starred_queries: - ref = SavedQueryRef(SavedQueryType.EXPLORE, row.explore_saved_query_id) - row.position = position_map[ref] - explore_updates.append(row) + for discover_row in discover_starred_queries: + discover_ref = SavedQueryRef(SavedQueryType.DISCOVER, discover_row.discover_saved_query_id) + discover_row.position = position_map[discover_ref] + discover_updates.append(discover_row) + + for explore_row in explore_starred_queries: + explore_ref = SavedQueryRef(SavedQueryType.EXPLORE, explore_row.explore_saved_query_id) + explore_row.position = position_map[explore_ref] + explore_updates.append(explore_row) ExploreSavedQueryStarred.objects.bulk_update(explore_updates, ["position"]) DiscoverSavedQueryStarred.objects.bulk_update(discover_updates, ["position"]) diff --git a/tests/sentry/explore/endpoints/test_saved_query_starred_order.py b/tests/sentry/explore/endpoints/test_saved_query_starred_order.py index 285871602860..5d80532c4f2a 100644 --- a/tests/sentry/explore/endpoints/test_saved_query_starred_order.py +++ b/tests/sentry/explore/endpoints/test_saved_query_starred_order.py @@ -140,22 +140,6 @@ def test_rejects_a_partial_list(self) -> None: ("discover", self.discover_y.id), ] - def test_rejects_duplicate_refs(self) -> None: - with self.feature(self.feature_flags): - response = self.client.put( - self.url, - data={ - "queries": [ - self.ref(self.discover_x), - self.ref(self.discover_x), - self.ref(self.explore_a), - self.ref(self.explore_b), - ] - }, - ) - - assert response.status_code == 400 - def test_empty_list_is_a_noop_when_nothing_is_starred(self) -> None: DiscoverSavedQueryStarred.objects.all().delete() ExploreSavedQueryStarred.objects.all().delete() From 50ad0ed63bee35673a8bfe9d32f8224c98035fa5 Mon Sep 17 00:00:00 2001 From: lzhao-sentry Date: Wed, 9 Sep 2026 09:30:32 -0400 Subject: [PATCH 4/5] remove try catch --- src/sentry/discover/models.py | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/sentry/discover/models.py b/src/sentry/discover/models.py index b6ee1a7b8a45..310e9b0db252 100644 --- a/src/sentry/discover/models.py +++ b/src/sentry/discover/models.py @@ -3,7 +3,7 @@ from enum import Enum from typing import ClassVar -from django.db import IntegrityError, models, router, transaction +from django.db import models, router, transaction from django.db.models import Q, UniqueConstraint from django.utils import timezone @@ -250,22 +250,18 @@ def insert_starred_query( """ from sentry.explore.utils import next_starred_position - try: - with transaction.atomic(using=router.db_for_write(DiscoverSavedQueryStarred)): - if self.get_starred_query(organization, user_id, query): - return False - - self.create( - organization=organization, - user_id=user_id, - discover_saved_query=query, - position=next_starred_position(organization, user_id), - starred=starred, - ) - return True - except IntegrityError: - # A concurrent request starred the same query first - return False + with transaction.atomic(using=router.db_for_write(DiscoverSavedQueryStarred)): + if self.get_starred_query(organization, user_id, query): + return False + + self.create( + organization=organization, + user_id=user_id, + discover_saved_query=query, + position=next_starred_position(organization, user_id), + starred=starred, + ) + return True def delete_starred_query( self, organization: Organization, user_id: int, query: DiscoverSavedQuery From e4956629ed1165507dda75edce7db09a4fc5ee6f Mon Sep 17 00:00:00 2001 From: lzhao-sentry Date: Wed, 9 Sep 2026 11:53:21 -0400 Subject: [PATCH 5/5] add back missing test, remove endpoint code --- .../endpoints/discover_saved_query_starred.py | 81 ------------------- .../test_discover_saved_query_starred.py | 57 ------------- .../test_saved_query_starred_order.py | 16 ++++ 3 files changed, 16 insertions(+), 138 deletions(-) delete mode 100644 src/sentry/discover/endpoints/discover_saved_query_starred.py delete mode 100644 tests/sentry/discover/test_discover_saved_query_starred.py diff --git a/src/sentry/discover/endpoints/discover_saved_query_starred.py b/src/sentry/discover/endpoints/discover_saved_query_starred.py deleted file mode 100644 index c30b7fd373da..000000000000 --- a/src/sentry/discover/endpoints/discover_saved_query_starred.py +++ /dev/null @@ -1,81 +0,0 @@ -from rest_framework import serializers, status -from rest_framework.request import Request -from rest_framework.response import Response - -from sentry import features -from sentry.api.api_owners import ApiOwner -from sentry.api.api_publish_status import ApiPublishStatus -from sentry.api.base import cell_silo_endpoint -from sentry.api.bases.organization import OrganizationEndpoint, OrganizationPermission -from sentry.discover.models import DiscoverSavedQuery, DiscoverSavedQueryStarred -from sentry.models.organization import Organization - - -class StarQuerySerializer(serializers.Serializer): - starred = serializers.BooleanField(required=True) - position = serializers.IntegerField(required=False) - - def validate(self, data): - if not data["starred"] and "position" in data: - raise serializers.ValidationError("Position is only allowed when starring a query.") - return data - - -class MemberPermission(OrganizationPermission): - scope_map = { - "POST": ["member:read", "member:write"], - } - - -@cell_silo_endpoint -class DiscoverSavedQueryStarredEndpoint(OrganizationEndpoint): - """ - Star or unstar a single saved Discover query. - """ - - publish_status = { - "POST": ApiPublishStatus.EXPERIMENTAL, - } - owner = ApiOwner.DATA_BROWSING - permission_classes = (MemberPermission,) - - def has_feature(self, organization, request): - return features.has( - "organizations:visibility-explore-view", organization, actor=request.user - ) and features.has( - "organizations:discover-queries-in-all-queries", organization, actor=request.user - ) - - def post(self, request: Request, organization: Organization, id: int) -> Response: - """ - Update the starred status of a saved Discover query for the current organization member. - """ - if not request.user.is_authenticated: - return Response(status=status.HTTP_400_BAD_REQUEST) - - if not self.has_feature(organization, request): - return self.respond(status=404) - - serializer = StarQuerySerializer(data=request.data) - if not serializer.is_valid(): - return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) - - is_starred = serializer.validated_data["starred"] - - try: - query = DiscoverSavedQuery.objects.get(id=id, organization=organization) - except DiscoverSavedQuery.DoesNotExist: - return Response(status=status.HTTP_404_NOT_FOUND) - - if is_starred: - if DiscoverSavedQueryStarred.objects.insert_starred_query( - organization, request.user.id, query - ): - return Response(status=status.HTTP_200_OK) - else: - if DiscoverSavedQueryStarred.objects.delete_starred_query( - organization, request.user.id, query - ): - return Response(status=status.HTTP_200_OK) - - return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/tests/sentry/discover/test_discover_saved_query_starred.py b/tests/sentry/discover/test_discover_saved_query_starred.py deleted file mode 100644 index 809c561931ee..000000000000 --- a/tests/sentry/discover/test_discover_saved_query_starred.py +++ /dev/null @@ -1,57 +0,0 @@ -import pytest -from django.urls import reverse - -from sentry.discover.models import DiscoverSavedQuery, DiscoverSavedQueryStarred -from sentry.testutils.cases import APITestCase - - -@pytest.mark.skip(reason="API not public yet, this line will be removed in future") -class DiscoverSavedQueryStarredTest(APITestCase): - feature_flags = { - "organizations:visibility-explore-view": True, - "organizations:discover-queries-in-all-queries": True, - } - - def setUp(self) -> None: - super().setUp() - self.login_as(user=self.user) - self.org = self.create_organization(owner=self.user) - self.project_ids = [ - self.create_project(organization=self.org).id, - self.create_project(organization=self.org).id, - ] - query = {"fields": ["title"], "conditions": "", "limit": 10} - - model = DiscoverSavedQuery.objects.create( - organization=self.org, created_by_id=self.user.id, name="Test query", query=query - ) - - model.set_projects(self.project_ids) - - self.query_id = model.id - - self.url = reverse( - "sentry-api-0-discover-saved-query-starred", args=[self.org.slug, self.query_id] - ) - - def test_post(self) -> None: - with self.feature(self.feature_flags): - assert not DiscoverSavedQuery.objects.filter( - id__in=DiscoverSavedQueryStarred.objects.filter( - organization=self.org, user_id=self.user.id - ).values_list("discover_saved_query_id", flat=True) - ).exists() - response = self.client.post(self.url, data={"starred": "1"}) - assert response.status_code == 200, response.content - assert DiscoverSavedQuery.objects.filter( - id__in=DiscoverSavedQueryStarred.objects.filter( - organization=self.org, user_id=self.user.id - ).values_list("discover_saved_query_id", flat=True) - ).exists() - response = self.client.post(self.url, data={"starred": "0"}) - assert response.status_code == 200, response.content - assert not DiscoverSavedQuery.objects.filter( - id__in=DiscoverSavedQueryStarred.objects.filter( - organization=self.org, user_id=self.user.id - ).values_list("discover_saved_query_id", flat=True) - ).exists() diff --git a/tests/sentry/explore/endpoints/test_saved_query_starred_order.py b/tests/sentry/explore/endpoints/test_saved_query_starred_order.py index 5d80532c4f2a..285871602860 100644 --- a/tests/sentry/explore/endpoints/test_saved_query_starred_order.py +++ b/tests/sentry/explore/endpoints/test_saved_query_starred_order.py @@ -140,6 +140,22 @@ def test_rejects_a_partial_list(self) -> None: ("discover", self.discover_y.id), ] + def test_rejects_duplicate_refs(self) -> None: + with self.feature(self.feature_flags): + response = self.client.put( + self.url, + data={ + "queries": [ + self.ref(self.discover_x), + self.ref(self.discover_x), + self.ref(self.explore_a), + self.ref(self.explore_b), + ] + }, + ) + + assert response.status_code == 400 + def test_empty_list_is_a_noop_when_nothing_is_starred(self) -> None: DiscoverSavedQueryStarred.objects.all().delete() ExploreSavedQueryStarred.objects.all().delete()