Skip to content

Commit

Permalink
feat(issue-views): Update GET endpoint to validate view's projects (#…
Browse files Browse the repository at this point in the history
…84338)

This PR updates the GET `organization/.../group-search-views` endpoint
to validate a views projecst before being sent. The logic is:

If your org doesn't have the multi project stream feature
(`organizations:global-views`) any views that have an invalid projects
parameter ("All Projects", "My Projects", or len(projects) > 1) will be
set to the first project you have, alphabetically.

No validation is necessary for organizations that do have the multi
project stream feature.
  • Loading branch information
MichaelSun48 authored Feb 6, 2025
1 parent 8ead265 commit f928557
Show file tree
Hide file tree
Showing 3 changed files with 268 additions and 15 deletions.
21 changes: 19 additions & 2 deletions src/sentry/api/serializers/models/groupsearchview.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,32 @@ class GroupSearchViewSerializerResponse(TypedDict):

@register(GroupSearchView)
class GroupSearchViewSerializer(Serializer):
def __init__(self, *args, **kwargs):
self.has_global_views = kwargs.pop("has_global_views", None)
self.default_project = kwargs.pop("default_project", None)
super().__init__(*args, **kwargs)

def serialize(self, obj, attrs, user, **kwargs) -> GroupSearchViewSerializerResponse:
if self.has_global_views is False:
is_all_projects = False

projects = list(obj.projects.values_list("id", flat=True))
num_projects = len(projects)
if num_projects != 1:
projects = [projects[0] if num_projects > 1 else self.default_project]

else:
is_all_projects = obj.is_all_projects
projects = list(obj.projects.values_list("id", flat=True))

return {
"id": str(obj.id),
"name": obj.name,
"query": obj.query,
"querySort": obj.query_sort,
"position": obj.position,
"projects": list(obj.projects.values_list("id", flat=True)),
"isAllProjects": obj.is_all_projects,
"projects": projects,
"isAllProjects": is_all_projects,
"environments": obj.environments,
"timeFilters": obj.time_filters,
"dateCreated": obj.date_added,
Expand Down
50 changes: 41 additions & 9 deletions src/sentry/issues/endpoints/organization_group_search_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
GroupSearchViewValidator,
GroupSearchViewValidatorResponse,
)
from sentry.models.groupsearchview import GroupSearchView
from sentry.models.groupsearchview import DEFAULT_TIME_FILTER, GroupSearchView
from sentry.models.organization import Organization
from sentry.models.project import Project
from sentry.models.savedsearch import SortOptions
Expand All @@ -31,6 +31,9 @@
"query": "is:unresolved issue.priority:[high, medium]",
"querySort": SortOptions.DATE.value,
"position": 0,
"isAllProjects": False,
"environments": [],
"timeFilters": DEFAULT_TIME_FILTER,
"dateCreated": None,
"dateUpdated": None,
}
Expand Down Expand Up @@ -65,23 +68,55 @@ def get(self, request: Request, organization: Organization) -> Response:
):
return Response(status=status.HTTP_404_NOT_FOUND)

query = GroupSearchView.objects.filter(organization=organization, user_id=request.user.id)
has_global_views = features.has("organizations:global-views", organization)

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

# Return only the prioritized view if user has no custom views yet
# Return only the default view(s) if user has no custom views yet
if not query.exists():
return self.paginate(
request=request,
paginator=SequencePaginator(
[(idx, view) for idx, view in enumerate(DEFAULT_VIEWS)]
[
(
idx,
{
**view,
"projects": (
[]
if has_global_views
else [pick_default_project(organization, request.user)]
),
},
)
for idx, view in enumerate(DEFAULT_VIEWS)
]
),
on_results=lambda results: serialize(results, request.user),
)

default_project = None
if not has_global_views:
default_project = pick_default_project(organization, request.user)
if default_project is None:
return Response(
status=status.HTTP_400_BAD_REQUEST,
data={"detail": "You do not have access to any projects."},
)

return self.paginate(
request=request,
queryset=query,
order_by="position",
on_results=lambda x: serialize(x, request.user, serializer=GroupSearchViewSerializer()),
on_results=lambda x: serialize(
x,
request.user,
serializer=GroupSearchViewSerializer(
has_global_views=has_global_views, default_project=default_project
),
),
)

def put(self, request: Request, organization: Organization) -> Response:
Expand All @@ -90,7 +125,6 @@ def put(self, request: Request, organization: Organization) -> Response:
will delete any views that are not included in the request, add views if
they are new, and update existing views if they are included in the request.
This endpoint is explcititly designed to be used by our frontend.
"""
if not features.has(
"organizations:issue-stream-custom-views", organization, actor=request.user
Expand Down Expand Up @@ -166,7 +200,7 @@ def bulk_update_views(
_update_existing_view(org, user_id, view, position=idx)


def pick_default_project(org: Organization, user: User | AnonymousUser) -> int:
def pick_default_project(org: Organization, user: User | AnonymousUser) -> int | None:
user_teams = Team.objects.get_for_user(organization=org, user=user)
user_team_ids = [team.id for team in user_teams]
default_user_project = (
Expand All @@ -175,8 +209,6 @@ def pick_default_project(org: Organization, user: User | AnonymousUser) -> int:
.values_list("id", flat=True)
.first()
)
if default_user_project is None:
raise ValidationError("You do not have access to any projects")
return default_user_project


Expand Down
212 changes: 208 additions & 4 deletions tests/sentry/issues/endpoints/test_organization_group_search_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

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
from sentry.testutils.cases import APITestCase, TransactionTestCase
from sentry.testutils.helpers.features import with_feature
Expand Down Expand Up @@ -366,7 +367,7 @@ def create_base_data_with_page_filters(self) -> list[GroupSearchView]:
query_sort="date",
position=0,
time_filters={"period": "14d"},
environments=["production"],
environments=[],
)
first_custom_view_user_one.projects.set([self.project1])

Expand All @@ -378,7 +379,7 @@ def create_base_data_with_page_filters(self) -> list[GroupSearchView]:
query_sort="new",
position=1,
time_filters={"period": "7d"},
environments=["staging"],
environments=["staging", "production"],
)
second_custom_view_user_one.projects.set([self.project1, self.project2, self.project3])

Expand Down Expand Up @@ -413,7 +414,7 @@ def test_not_including_page_filters_does_not_reset_them_for_existing_views(self)
# Original Page filters
assert views[0]["timeFilters"] == {"period": "14d"}
assert views[0]["projects"] == [self.project1.id]
assert views[0]["environments"] == ["production"]
assert views[0]["environments"] == []

view = views[0]
# Change nothing but the name
Expand All @@ -428,7 +429,7 @@ def test_not_including_page_filters_does_not_reset_them_for_existing_views(self)
# Ensure these have not been changed
assert views[0]["timeFilters"] == {"period": "14d"}
assert views[0]["projects"] == [self.project1.id]
assert views[0]["environments"] == ["production"]
assert views[0]["environments"] == []

@with_feature({"organizations:issue-stream-custom-views": True})
@with_feature({"organizations:global-views": True})
Expand Down Expand Up @@ -584,6 +585,209 @@ def test_invalid_project_ids(self) -> None:
assert response.content == b'{"detail":"One or more projects do not exist"}'


class OrganizationGroupSearchViewsGetPageFiltersTest(APITestCase):
def create_base_data_with_page_filters(self) -> None:
self.team_1 = self.create_team(organization=self.organization, slug="team-1")
self.team_2 = self.create_team(organization=self.organization, slug="team-2")

# User 1 is on team 1 only
user_1 = self.user
self.create_team_membership(user=user_1, team=self.team_1)
# User 2 is on team 1 and team 2
self.user_2 = self.create_user()
self.create_member(
organization=self.organization, user=self.user_2, teams=[self.team_1, self.team_2]
)
# User 3 has no views and should get the default views
self.user_3 = self.create_user()
self.create_member(organization=self.organization, user=self.user_3, teams=[self.team_1])
# User 4 is part of no teams, should error out
self.user_4 = self.create_user()
self.create_member(organization=self.organization, user=self.user_4)

# This project should NEVER get chosen as a default since it does not belong to any teams
self.project1 = self.create_project(
organization=self.organization, slug="project-a", teams=[]
)
# This project should be User 2's default project since it's the alphabetically the first one
self.project2 = self.create_project(
organization=self.organization, slug="project-b", teams=[self.team_2]
)
# This should be User 1's default project since it's the only one that the user has access to
self.project3 = self.create_project(
organization=self.organization, slug="project-c", teams=[self.team_1, self.team_2]
)

first_issue_view_user_one = GroupSearchView.objects.create(
name="Issue View One",
organization=self.organization,
user_id=user_1.id,
query="is:unresolved",
query_sort="date",
position=0,
is_all_projects=False,
time_filters={"period": "14d"},
environments=[],
)
first_issue_view_user_one.projects.set([self.project3])

second_issue_view_user_one = GroupSearchView.objects.create(
name="Issue View Two",
organization=self.organization,
user_id=user_1.id,
query="is:resolved",
query_sort="new",
position=1,
is_all_projects=False,
time_filters={"period": "7d"},
environments=["staging", "production"],
)
second_issue_view_user_one.projects.set([])

third_issue_view_user_one = GroupSearchView.objects.create(
name="Issue View Three",
organization=self.organization,
user_id=user_1.id,
query="is:ignored",
query_sort="freq",
position=2,
is_all_projects=True,
time_filters={"period": "30d"},
environments=["development"],
)
third_issue_view_user_one.projects.set([])

first_issue_view_user_two = GroupSearchView.objects.create(
name="Issue View One",
organization=self.organization,
user_id=self.user_2.id,
query="is:unresolved",
query_sort="date",
position=0,
is_all_projects=False,
time_filters={"period": "14d"},
environments=[],
)
first_issue_view_user_two.projects.set([])

first_issue_view_user_four = GroupSearchView.objects.create(
name="Issue View One",
organization=self.organization,
user_id=self.user_4.id,
query="is:unresolved",
query_sort="date",
position=0,
is_all_projects=False,
time_filters={"period": "14d"},
environments=[],
)
first_issue_view_user_four.projects.set([])

def setUp(self) -> None:
self.create_base_data_with_page_filters()
self.url = reverse(
"sentry-api-0-organization-group-search-views",
kwargs={"organization_id_or_slug": self.organization.slug},
)

@with_feature({"organizations:issue-stream-custom-views": True})
@with_feature({"organizations:global-views": True})
def test_basic_get_page_filters_with_global_filters(self) -> None:
self.login_as(user=self.user)
response = self.client.get(self.url)

assert response.data[0]["timeFilters"] == {"period": "14d"}
assert response.data[0]["projects"] == [self.project3.id]
assert response.data[0]["environments"] == []
assert response.data[0]["isAllProjects"] is False

assert response.data[1]["timeFilters"] == {"period": "7d"}
assert response.data[1]["projects"] == []
assert response.data[1]["environments"] == ["staging", "production"]
assert response.data[1]["isAllProjects"] is False

assert response.data[2]["timeFilters"] == {"period": "30d"}
assert response.data[2]["projects"] == []
assert response.data[2]["environments"] == ["development"]
assert response.data[2]["isAllProjects"] is True

@with_feature({"organizations:issue-stream-custom-views": True})
@with_feature({"organizations:global-views": False})
def test_get_page_filters_without_global_filters(self) -> None:
self.login_as(user=self.user)
response = self.client.get(self.url)

assert response.data[0]["timeFilters"] == {"period": "14d"}
assert response.data[0]["projects"] == [self.project3.id]
assert response.data[0]["environments"] == []
assert response.data[0]["isAllProjects"] is False

assert response.data[1]["timeFilters"] == {"period": "7d"}
assert response.data[1]["projects"] == [self.project3.id]
assert response.data[1]["environments"] == ["staging", "production"]
assert response.data[1]["isAllProjects"] is False

assert response.data[2]["timeFilters"] == {"period": "30d"}
assert response.data[2]["projects"] == [self.project3.id]
assert response.data[2]["environments"] == ["development"]
assert response.data[2]["isAllProjects"] is False

@with_feature({"organizations:issue-stream-custom-views": True})
@with_feature({"organizations:global-views": False})
def test_get_page_filters_without_global_filters_user_2(self) -> None:
self.login_as(user=self.user_2)
response = self.client.get(self.url)

assert response.data[0]["timeFilters"] == {"period": "14d"}
assert response.data[0]["projects"] == [self.project2.id]
assert response.data[0]["environments"] == []
assert response.data[0]["isAllProjects"] is False

@with_feature({"organizations:issue-stream-custom-views": True})
@with_feature({"organizations:global-views": True})
def test_default_page_filters_with_global_views(self) -> None:
self.login_as(user=self.user_3)
response = self.client.get(self.url)

default_view_queries = {view["query"] for view in DEFAULT_VIEWS}
received_queries = {view["query"] for view in response.data}

assert default_view_queries == received_queries

for view in response.data:
assert view["timeFilters"] == {"period": "14d"}
# Global views means default project should be "My Projects"
assert view["projects"] == []
assert view["environments"] == []
assert view["isAllProjects"] is False

@with_feature({"organizations:issue-stream-custom-views": True})
@with_feature({"organizations:global-views": False})
def test_default_page_filters_without_global_views(self) -> None:
self.login_as(user=self.user_3)
response = self.client.get(self.url)

default_view_queries = {view["query"] for view in DEFAULT_VIEWS}
received_queries = {view["query"] for view in response.data}

assert default_view_queries == received_queries

for view in response.data:
assert view["timeFilters"] == {"period": "14d"}
# No global views means default project should be a single project
assert view["projects"] == [self.project3.id]
assert view["environments"] == []
assert view["isAllProjects"] is False

@with_feature({"organizations:issue-stream-custom-views": True})
@with_feature({"organizations:global-views": False})
def test_error_when_no_projects_found(self) -> None:
self.login_as(user=self.user_4)
response = self.client.get(self.url)
assert response.status_code == 400
assert response.data == {"detail": "You do not have access to any projects."}


class OrganizationGroupSearchViewsPutRegressionTest(APITestCase):
endpoint = "sentry-api-0-organization-group-search-views"
method = "put"
Expand Down

0 comments on commit f928557

Please sign in to comment.