Skip to content

ref(shared-views): Remove starred view logic from get /groupsearchviews/ endpoint #89239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 11, 2025
Merged

ref(shared-views): Remove starred view logic from get /groupsearchviews/ endpoint #89239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a4d8cf5
Remove starred view logic from get /groupsearchviews/ endpoint
MichaelSun48 Apr 9, 2025
93b46eb
Refactor tests
MichaelSun48 Apr 9, 2025
ce8c107
More test fixes
MichaelSun48 Apr 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 45 additions & 95 deletions src/sentry/issues/endpoints/organization_group_search_views.py
View file
Open in desktop
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, if no query params were passed into this endpoint, it would return the user's starred views.

That default behavior has now been removed, and it defaults the query params to createdBy = "me" and sort = "-visited" if none are passed in.

Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@
from sentry.api.api_publish_status import ApiPublishStatus
from sentry.api.base import region_silo_endpoint
from sentry.api.bases.organization import OrganizationEndpoint, OrganizationPermission
from sentry.api.paginator import ChainPaginator, OffsetPaginator, SequencePaginator
from sentry.api.paginator import ChainPaginator, OffsetPaginator
from sentry.api.serializers import serialize
from sentry.api.serializers.models.groupsearchview import GroupSearchViewSerializer
from sentry.api.serializers.rest_framework.groupsearchview import (
GroupSearchViewPostValidator,
GroupSearchViewValidator,
GroupSearchViewValidatorResponse,
)
from sentry.models.groupsearchview import DEFAULT_VIEWS, GroupSearchView, GroupSearchViewVisibility
from sentry.models.groupsearchview import GroupSearchView, GroupSearchViewVisibility
from sentry.models.groupsearchviewstarred import GroupSearchViewStarred
from sentry.models.organization import Organization
from sentry.models.project import Project
Expand Down Expand Up @@ -83,35 +83,6 @@ def get(self, request: Request, organization: Organization) -> Response:
if not serializer.is_valid():
return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)

query = GroupSearchView.objects.filter(
organization=organization, user_id=request.user.id
).prefetch_related("projects")

# Return only the default view(s) if user has no custom views yet
# TODO(msun): Delete this logic once left-nav views have been fully rolled out.
if not query.exists() and not serializer.validated_data.get("createdBy"):
return self.paginate(
request=request,
paginator=SequencePaginator(
[
(
idx,
{
**view,
"projects": (
[]
if has_global_views
else [pick_default_project(organization, request.user)]
),
"starred": False,
},
)
for idx, view in enumerate(DEFAULT_VIEWS)
]
),
on_results=lambda results: serialize(results, request.user),
)

starred_view_ids = GroupSearchViewStarred.objects.filter(
organization=organization, user_id=request.user.id
).values_list("group_search_view_id", flat=True)
Expand All @@ -125,79 +96,58 @@ def get(self, request: Request, organization: Organization) -> Response:
data={"detail": "You do not have access to any projects."},
)

createdBy = serializer.validated_data.get("createdBy")
createdBy = serializer.validated_data.get("createdBy", "me")
sort = SORT_MAP[serializer.validated_data.get("sort", "-visited")]
if createdBy:
if createdBy == "me":
starred_query = (
GroupSearchView.objects.filter(
organization=organization,
user_id=request.user.id,
id__in=starred_view_ids,
)
.prefetch_related("projects")
.annotate(popularity=Count("groupsearchviewstarred"))
.order_by(sort)

if createdBy == "me":
starred_query = (
GroupSearchView.objects.filter(
organization=organization,
user_id=request.user.id,
id__in=starred_view_ids,
)
non_starred_query = (
GroupSearchView.objects.filter(
organization=organization,
user_id=request.user.id,
)
.exclude(id__in=starred_view_ids)
.prefetch_related("projects")
.annotate(popularity=Count("groupsearchviewstarred"))
.order_by(sort)
.prefetch_related("projects")
.annotate(popularity=Count("groupsearchviewstarred"))
.order_by(sort)
)
non_starred_query = (
GroupSearchView.objects.filter(
organization=organization,
user_id=request.user.id,
)
elif createdBy == "others":
starred_query = (
GroupSearchView.objects.filter(
organization=organization,
visibility=GroupSearchViewVisibility.ORGANIZATION,
id__in=starred_view_ids,
)
.exclude(user_id=request.user.id)
.prefetch_related("projects")
.annotate(popularity=Count("groupsearchviewstarred"))
.order_by(sort)
.exclude(id__in=starred_view_ids)
.prefetch_related("projects")
.annotate(popularity=Count("groupsearchviewstarred"))
.order_by(sort)
)
elif createdBy == "others":
starred_query = (
GroupSearchView.objects.filter(
organization=organization,
visibility=GroupSearchViewVisibility.ORGANIZATION,
id__in=starred_view_ids,
)
non_starred_query = (
GroupSearchView.objects.filter(
organization=organization,
visibility=GroupSearchViewVisibility.ORGANIZATION,
)
.exclude(user_id=request.user.id)
.exclude(id__in=starred_view_ids)
.prefetch_related("projects")
.annotate(popularity=Count("groupsearchviewstarred"))
.order_by(sort)
.exclude(user_id=request.user.id)
.prefetch_related("projects")
.annotate(popularity=Count("groupsearchviewstarred"))
.order_by(sort)
)
non_starred_query = (
GroupSearchView.objects.filter(
organization=organization,
visibility=GroupSearchViewVisibility.ORGANIZATION,
)

return self.paginate(
request=request,
sources=[starred_query, non_starred_query],
paginator_cls=ChainPaginator,
on_results=lambda x: serialize(
x,
request.user,
serializer=GroupSearchViewSerializer(
has_global_views=has_global_views,
default_project=default_project,
organization=organization,
),
),
.exclude(user_id=request.user.id)
.exclude(id__in=starred_view_ids)
.prefetch_related("projects")
.annotate(popularity=Count("groupsearchviewstarred"))
.order_by(sort)
)

user_starred_views = (
GroupSearchView.objects.filter(id__in=starred_view_ids)
.prefetch_related("projects")
.order_by("groupsearchviewstarred__position")
)

return self.paginate(
request=request,
queryset=user_starred_views,
paginator_cls=OffsetPaginator,
sources=[starred_query, non_starred_query],
paginator_cls=ChainPaginator,
on_results=lambda x: serialize(
x,
request.user,
Expand Down
91 changes: 90 additions & 1 deletion tests/sentry/issues/endpoints/test_organization_group_search_view_details.py
View file
Open in desktop
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test class depended on the BaseGSVTestCase, and refactoring it out of this file in favor of GroupSearchViewAPITestCase turned out to be a bit more work than I expected. I will do this in a separate PR.

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,96 @@
from sentry.models.groupsearchviewstarred import GroupSearchViewStarred
from sentry.testutils.cases import APITestCase
from sentry.testutils.helpers import with_feature
from tests.sentry.issues.endpoints.test_organization_group_search_views import BaseGSVTestCase


class BaseGSVTestCase(APITestCase):
def create_base_data(self) -> dict[str, list[GroupSearchView]]:
user_1 = self.user
self.user_2 = self.create_user()
self.user_3 = self.create_user()

self.create_member(organization=self.organization, user=self.user_2)
self.create_member(organization=self.organization, user=self.user_3)

first_custom_view_user_one = GroupSearchView.objects.create(
name="Custom View One",
organization=self.organization,
user_id=user_1.id,
query="is:unresolved",
query_sort="date",
)
GroupSearchViewStarred.objects.create(
organization=self.organization,
user_id=user_1.id,
group_search_view=first_custom_view_user_one,
position=0,
)

# This is out of order to test that the endpoint returns the views in the correct order
third_custom_view_user_one = GroupSearchView.objects.create(
name="Custom View Three",
organization=self.organization,
user_id=user_1.id,
query="is:ignored",
query_sort="freq",
)
GroupSearchViewStarred.objects.create(
organization=self.organization,
user_id=user_1.id,
group_search_view=third_custom_view_user_one,
position=2,
)

second_custom_view_user_one = GroupSearchView.objects.create(
name="Custom View Two",
organization=self.organization,
user_id=user_1.id,
query="is:resolved",
query_sort="new",
)
GroupSearchViewStarred.objects.create(
organization=self.organization,
user_id=user_1.id,
group_search_view=second_custom_view_user_one,
position=1,
)

first_custom_view_user_two = GroupSearchView.objects.create(
name="Custom View One",
organization=self.organization,
user_id=self.user_2.id,
query="is:unresolved",
query_sort="date",
)
GroupSearchViewStarred.objects.create(
organization=self.organization,
user_id=self.user_2.id,
group_search_view=first_custom_view_user_two,
position=0,
)

second_custom_view_user_two = GroupSearchView.objects.create(
name="Custom View Two",
organization=self.organization,
user_id=self.user_2.id,
query="is:resolved",
query_sort="new",
)
GroupSearchViewStarred.objects.create(
organization=self.organization,
user_id=self.user_2.id,
group_search_view=second_custom_view_user_two,
position=1,
)

return {
"user_one_views": [
first_custom_view_user_one,
second_custom_view_user_one,
third_custom_view_user_one,
],
"user_two_views": [first_custom_view_user_two, second_custom_view_user_two],
}


class OrganizationGroupSearchViewsGetTest(BaseGSVTestCase):
Expand Down
16 changes: 9 additions & 7 deletions tests/sentry/issues/endpoints/test_organization_group_search_view_visit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,18 @@
from sentry.models.groupsearchviewlastvisited import GroupSearchViewLastVisited
from sentry.testutils.helpers.datetime import freeze_time
from sentry.testutils.helpers.features import with_feature
from tests.sentry.issues.endpoints.test_organization_group_search_views import BaseGSVTestCase
from tests.sentry.issues.endpoints.test_organization_group_search_views import (
GroupSearchViewAPITestCase,
)


class OrganizationGroupSearchViewVisitTest(BaseGSVTestCase):
class OrganizationGroupSearchViewVisitTest(GroupSearchViewAPITestCase):
endpoint = "sentry-api-0-organization-group-search-view-visit"
method = "post"

def setUp(self) -> None:
self.login_as(user=self.user)
self.base_data = self.create_base_data()

# Get the first view's ID for testing
self.view = self.base_data["user_one_views"][0]
self.view = self.create_view(self.user)

self.url = reverse(
"sentry-api-0-organization-group-search-view-visit",
Expand Down Expand Up @@ -89,8 +88,11 @@ def test_update_nonexistent_view(self) -> None:

@with_feature({"organizations:issue-stream-custom-views": True})
def test_update_view_from_another_user(self) -> None:
user_two = self.create_user()
self.create_member(organization=self.organization, user=user_two)

# Get a view ID from user_two
view = self.base_data["user_two_views"][0]
view = self.create_view(user_two)
url = reverse(
"sentry-api-0-organization-group-search-view-visit",
kwargs={"organization_id_or_slug": self.organization.slug, "view_id": view.id},
Expand Down
Loading
Loading