diff --git a/src/sentry/api/bases/organization.py b/src/sentry/api/bases/organization.py index 55965102abb81e..2f70f475412e6c 100644 --- a/src/sentry/api/bases/organization.py +++ b/src/sentry/api/bases/organization.py @@ -6,7 +6,8 @@ import sentry_sdk from django.core.cache import cache -from django.http.request import HttpRequest +from django.db.models import Q +from django.http.request import HttpRequest, QueryDict from rest_framework.exceptions import ParseError, PermissionDenied from rest_framework.permissions import BasePermission from rest_framework.request import Request @@ -15,11 +16,12 @@ from sentry.api.base import Endpoint from sentry.api.exceptions import ResourceDoesNotExist from sentry.api.helpers.environments import get_environments +from sentry.api.helpers.projects import ParsedProjectIdOrSlugParams, parse_id_or_slug_params from sentry.api.permissions import DemoSafePermission, StaffPermissionMixin from sentry.api.utils import get_date_range_from_params, is_member_disabled_from_limit from sentry.auth.staff import is_active_staff from sentry.auth.superuser import is_active_superuser -from sentry.constants import ALL_ACCESS_PROJECT_ID, ALL_ACCESS_PROJECTS_SLUG, ObjectStatus +from sentry.constants import ObjectStatus from sentry.exceptions import InvalidParams from sentry.models.apikey import is_api_key_auth from sentry.models.apitoken import is_api_token_auth @@ -349,8 +351,11 @@ def _validate_fetched_projects( """ Validates that user has access to the specific projects they are requesting. """ - missing_project_ids = ids and ids != {p.id for p in filtered_projects} - missing_project_slugs = slugs and slugs != {p.slug for p in filtered_projects} + fetched_ids = {p.id for p in filtered_projects} + fetched_slugs = {p.slug for p in filtered_projects} + + missing_project_ids = ids and not ids.issubset(fetched_ids) + missing_project_slugs = slugs and not slugs.issubset(fetched_slugs) if missing_project_ids or missing_project_slugs: raise PermissionDenied @@ -389,32 +394,30 @@ def get_projects( :return: A list of Project objects, or raises PermissionDenied. When project_ids or project_slugs are explicitly provided, the returned list is guaranteed non-empty (or PermissionDenied is raised). - NOTE: If both project_ids and project_slugs are passed, we will default - to fetching projects via project_id list. + NOTE: Passing both project_ids and project_slugs raises ``ParseError``. """ qs = Project.objects.filter(organization_id=organization.id, status=ObjectStatus.ACTIVE) if project_slugs and project_ids: raise ParseError(detail="Cannot query for both ids and slugs") - slugs = project_slugs or set(filter(None, request.GET.getlist("projectSlug"))) - ids = project_ids or self.get_requested_project_ids_unchecked(request) - - if project_ids is None and slugs: - # If we're querying for project slugs specifically - if ALL_ACCESS_PROJECTS_SLUG in slugs: - # All projects I have access to - include_all_accessible = True - else: - qs = qs.filter(slug__in=slugs) + if project_ids: + requested_projects = ParsedProjectIdOrSlugParams(ids=project_ids, slugs=set()) + elif project_slugs: + requested_projects = ParsedProjectIdOrSlugParams(ids=set(), slugs=set(project_slugs)) else: - # If we are explicitly querying for projects via id - # Or we're querying for an empty set of ids - if ALL_ACCESS_PROJECT_ID in ids: - # All projects i have access to - include_all_accessible = True - elif ids: - qs = qs.filter(id__in=ids) - # No project ids === `all projects I am a member of` + requested_projects = self.get_requested_project_params_unchecked(request) + ids = requested_projects.ids + slugs = requested_projects.slugs + + if requested_projects.has_all_projects_sentinel: + include_all_accessible = True + elif ids and slugs: + qs = qs.filter(Q(id__in=ids) | Q(slug__in=slugs)) + elif slugs: + qs = qs.filter(slug__in=slugs) + elif ids: + qs = qs.filter(id__in=ids) + # No project ids or slugs === `all projects I am a member of` with sentry_sdk.start_span(op="fetch_organization_projects") as span: projects = list(qs) @@ -474,15 +477,65 @@ def _filter_projects_by_permissions( def get_requested_project_ids_unchecked(self, request: HttpRequest) -> set[int]: """ - Returns the project ids that were requested by the request. + Returns the numeric project ids that were requested by the request. + Non-numeric values (slugs) are silently ignored. To determine the projects to filter this endpoint by with full permission checking, use ``get_projects``, instead. """ - try: - return set(map(int, request.GET.getlist("project"))) - except ValueError: - raise ParseError(detail="Invalid project parameter. Values must be numbers.") + return self.get_requested_project_ids_and_slugs_unchecked(request).ids + + def get_requested_project_ids_and_slugs_unchecked( + self, request: HttpRequest + ) -> ParsedProjectIdOrSlugParams: + """ + Returns the project ids and slugs requested via the project query param. + + To determine the projects to filter this endpoint by with full + permission checking, use ``get_projects``, instead. + """ + return parse_id_or_slug_params(request.GET.getlist("project")) + + def get_requested_project_params_unchecked( + self, request: HttpRequest + ) -> ParsedProjectIdOrSlugParams: + """ + Returns requested project ids and slugs from projectSlug or project query params. + + To determine the projects to filter this endpoint by with full + permission checking, use ``get_projects``, instead. + """ + project_slugs = set(filter(None, request.GET.getlist("projectSlug"))) + if project_slugs: + return ParsedProjectIdOrSlugParams(ids=set(), slugs=project_slugs) + return self.get_requested_project_ids_and_slugs_unchecked(request) + + def get_query_params_without_empty_project_params(self, request: HttpRequest) -> QueryDict: + """ + Return query params with blank project filters treated as absent. + """ + query_params = request.GET.copy() + if "project" in query_params: + query_params.setlist( + "project", [project for project in query_params.getlist("project") if project] + ) + return query_params + + def get_query_params_with_project_slug_precedence(self, request: HttpRequest) -> QueryDict: + """ + Return query params for serializers that accept project and projectSlug. + + This mirrors get_projects() precedence: non-empty legacy projectSlug + filters win over project filters, and blank project filters are treated + as absent. + """ + query_params = self.get_query_params_without_empty_project_params(request) + project_slug_params = [slug for slug in query_params.getlist("projectSlug") if slug] + if "projectSlug" in query_params: + query_params.setlist("projectSlug", project_slug_params) + if project_slug_params: + query_params.pop("project", None) + return query_params def get_environments( self, request: Request, organization: Organization | RpcOrganization @@ -723,7 +776,7 @@ def has_release_permission( they cannot access. The all-projects check respects Open Membership via has_global_access. - Results are cached for 60s per actor/org/release/project-ids/mode. + Results are cached for 60s per actor/org/release/effective-project-scope/mode. """ actor_id = None has_perms = None @@ -733,9 +786,10 @@ def has_release_permission( elif request.auth is not None: actor_id = "apikey:%s" % request.auth.entity_id if actor_id is not None: - requested_project_ids = project_ids - if requested_project_ids is None: - requested_project_ids = self.get_requested_project_ids_unchecked(request) + if project_ids: + requested_projects = ParsedProjectIdOrSlugParams(ids=project_ids, slugs=set()) + else: + requested_projects = self.get_requested_project_params_unchecked(request) key = "release_perms:1:%s" % hash_values( [ actor_id, @@ -743,7 +797,8 @@ def has_release_permission( release.id if release is not None else 0, int(require_all_projects), ] - + sorted(requested_project_ids) + + sorted(requested_projects.ids) + + sorted(requested_projects.slugs) ) has_perms = cache.get(key) if has_perms is None: diff --git a/src/sentry/api/endpoints/organization_stats_summary.py b/src/sentry/api/endpoints/organization_stats_summary.py index aa6da1d54b34ab..a2816d97e3429f 100644 --- a/src/sentry/api/endpoints/organization_stats_summary.py +++ b/src/sentry/api/endpoints/organization_stats_summary.py @@ -21,7 +21,6 @@ from sentry.apidocs.examples.organization_examples import OrganizationExamples from sentry.apidocs.parameters import GlobalParams from sentry.apidocs.utils import inline_sentry_response_serializer -from sentry.constants import ALL_ACCESS_PROJECTS from sentry.exceptions import InvalidParams from sentry.models.organization import Organization from sentry.models.project import Project @@ -188,19 +187,14 @@ def build_outcomes_query(self, request: Request, organization): return QueryDefinition.from_query_dict(query_dict, params) def _get_projects_for_orgstats_query(self, request: Request, organization): - req_proj_ids = self.get_requested_project_ids_unchecked(request) - # the projects table always filters by project # the projects in the table should be those the user has access to - projects = self.get_projects(request, organization, project_ids=req_proj_ids) + projects = self.get_projects(request, organization) if not projects: raise NoProjects("No projects available") return [p.id for p in projects] - def _is_org_total_query(self, project_ids): - return all([not project_ids or project_ids == ALL_ACCESS_PROJECTS]) - def _generate_csv(self, projects): if not len(projects): return diff --git a/src/sentry/api/endpoints/organization_stats_v2.py b/src/sentry/api/endpoints/organization_stats_v2.py index 7ca56189dc9b4c..4ba51ecc891ff4 100644 --- a/src/sentry/api/endpoints/organization_stats_v2.py +++ b/src/sentry/api/endpoints/organization_stats_v2.py @@ -17,7 +17,7 @@ from sentry.apidocs.examples.organization_examples import OrganizationExamples from sentry.apidocs.parameters import GlobalParams from sentry.apidocs.utils import inline_sentry_response_serializer -from sentry.constants import ALL_ACCESS_PROJECTS +from sentry.constants import ALL_ACCESS_PROJECTS, ALL_ACCESS_PROJECTS_SLUG from sentry.exceptions import InvalidParams from sentry.models.organization import Organization from sentry.ratelimits.config import RateLimitConfig @@ -205,21 +205,22 @@ def _get_projects_for_orgstats_query(self, request: Request, organization): # look at the raw project_id filter passed in, if its empty # and project_id is not in groupBy filter, treat it as an # org wide query and don't pass project_id in to QueryDefinition - req_proj_ids = self.get_requested_project_ids_unchecked(request) - if self._is_org_total_query(request, req_proj_ids): + requested_projects = self.get_requested_project_params_unchecked(request) + if self._is_org_total_query(request, requested_projects): return None else: - projects = self.get_projects(request, organization, project_ids=req_proj_ids) + projects = self.get_projects(request, organization) if not projects: raise NoProjects("No projects available") return [p.id for p in projects] - def _is_org_total_query(self, request: Request, project_ids): - return all( - [ - not project_ids or project_ids == ALL_ACCESS_PROJECTS, - "project" not in request.GET.get("groupBy", []), - ] + def _is_org_total_query(self, request: Request, requested_projects): + no_project_filter = not requested_projects.has_values + all_access_filter = ( + requested_projects.ids == ALL_ACCESS_PROJECTS and not requested_projects.slugs + ) or (requested_projects.slugs == {ALL_ACCESS_PROJECTS_SLUG} and not requested_projects.ids) + return (no_project_filter or all_access_filter) and "project" not in request.GET.getlist( + "groupBy" ) @contextmanager diff --git a/src/sentry/api/helpers/projects.py b/src/sentry/api/helpers/projects.py index a34bcbafba592b..744df24a496063 100644 --- a/src/sentry/api/helpers/projects.py +++ b/src/sentry/api/helpers/projects.py @@ -3,7 +3,6 @@ from collections.abc import Iterable from typing import NamedTuple, TypeAlias -from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import extend_schema_field from rest_framework import serializers @@ -12,6 +11,8 @@ ProjectIdOrSlug: TypeAlias = int | str +PROJECT_ID_OR_SLUG_SCHEMA = {"anyOf": [{"type": "integer"}, {"type": "string"}]} + class ParsedProjectIdOrSlugParams(NamedTuple): ids: set[int] @@ -52,7 +53,7 @@ def parse_id_or_slug_params( return ParsedProjectIdOrSlugParams(ids=ids, slugs=slugs) -@extend_schema_field(field=OpenApiTypes.STR) +@extend_schema_field(field=PROJECT_ID_OR_SLUG_SCHEMA) class ProjectIdOrSlugField(serializers.Field[ProjectIdOrSlug, object, ProjectIdOrSlug, object]): default_error_messages = { "invalid": "Expected a project ID or slug.", diff --git a/src/sentry/apidocs/parameters.py b/src/sentry/apidocs/parameters.py index 34d7b035e965e8..9de0cc2e8651a7 100644 --- a/src/sentry/apidocs/parameters.py +++ b/src/sentry/apidocs/parameters.py @@ -5,6 +5,7 @@ from drf_spectacular.utils import OpenApiParameter from sentry import constants +from sentry.api.helpers.projects import PROJECT_ID_OR_SLUG_SCHEMA from sentry.snuba.dataset import Dataset from sentry.snuba.sessions import STATS_PERIODS @@ -44,7 +45,7 @@ class GlobalParams: ) PROJECT_ID_OR_SLUG = OpenApiParameter( name="project_id_or_slug", - description="The ID or slug of the project the resource belongs to.", + description="The ID or slug of the project the resource belongs to. Project slugs are unique within each organization.", required=True, type=str, location="path", @@ -143,7 +144,18 @@ class OrganizationParams: required=False, many=True, type=str, - description="""The project slugs to filter by. Use `$all` to include all available projects. For example, the following are valid parameters: + description="""The legacy project slug filter. Prefer `project`, which accepts project IDs or slugs. Use `$all` to include all available projects. For example, the following are valid parameters: +- `/?projectSlug=$all` +- `/?projectSlug=android&projectSlug=javascript-react` +""", + ) + PROJECT_SLUG = OpenApiParameter( + name="projectSlug", + location="query", + required=False, + many=True, + type=str, + description="""The project slugs to filter by. This legacy parameter takes precedence over `project` if both are provided. Prefer `project`, which accepts project IDs or slugs. Use `$all` to include all available projects. For example, the following are valid parameters: - `/?projectSlug=$all` - `/?projectSlug=android&projectSlug=javascript-react` """, @@ -152,11 +164,11 @@ class OrganizationParams: name="project", location="query", required=False, - many=True, - type=int, - description="""The IDs of projects to filter by. `-1` means all available projects. + type={"type": "array", "items": PROJECT_ID_OR_SLUG_SCHEMA}, + description="""The IDs or slugs of projects to filter by. Project slugs are unique within each organization. Omit this parameter to include all accessible projects. `-1` is also accepted to include all accessible projects. For example, the following are valid parameters: - `/?project=1234&project=56789` +- `/?project=android&project=javascript-react` - `/?project=-1` """, ) diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_index.py b/src/sentry/incidents/endpoints/organization_alert_rule_index.py index c309bda611dd89..eb7161ed10f1d4 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_index.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_index.py @@ -67,7 +67,6 @@ from sentry.middleware import is_frontend_request from sentry.models.groupopenperiod import GroupOpenPeriod from sentry.models.organization import Organization -from sentry.models.organizationmemberteam import OrganizationMemberTeam from sentry.models.project import Project from sentry.models.team import Team from sentry.monitors.models import ( @@ -525,31 +524,10 @@ def get(self, request: Request, organization: Organization) -> Response: """ Fetches metric, issue, crons, and uptime alert rules for an organization """ - # Common setup: project resolution - project_ids = self.get_requested_project_ids_unchecked(request) or None - if project_ids == {-1}: # All projects for org: - project_ids = set( - Project.objects.filter( - organization=organization, status=ObjectStatus.ACTIVE - ).values_list("id", flat=True) - ) - elif project_ids is None: # All projects for user - org_team_list = Team.objects.filter(organization=organization).values_list( - "id", flat=True - ) - user_team_list = OrganizationMemberTeam.objects.filter( - organizationmember__user_id=request.user.id, team__in=org_team_list - ).values_list("team", flat=True) - project_ids = set( - Project.objects.filter( - teams__in=user_team_list, status=ObjectStatus.ACTIVE - ).values_list("id", flat=True) - ) - # Materialize the project ids here. This helps us to not overwhelm the query planner with # overcomplicated subqueries. Previously, this was causing Postgres to use a suboptimal # index to filter on. Also enforces permission checks. - projects = self.get_projects(request, organization, project_ids=project_ids) + projects = self.get_projects(request, organization) # Common setup: team parsing teams = request.GET.getlist("team", []) diff --git a/src/sentry/notifications/api/endpoints/notification_actions_index.py b/src/sentry/notifications/api/endpoints/notification_actions_index.py index 7d1e7ea92b4215..b76cdc1c818452 100644 --- a/src/sentry/notifications/api/endpoints/notification_actions_index.py +++ b/src/sentry/notifications/api/endpoints/notification_actions_index.py @@ -18,6 +18,7 @@ from sentry.apidocs.examples.notification_examples import NotificationActionExamples from sentry.apidocs.parameters import GlobalParams, NotificationParams, OrganizationParams from sentry.apidocs.response_types import ValidationErrorResponse, as_validation_errors +from sentry.constants import ALL_ACCESS_PROJECTS, ALL_ACCESS_PROJECTS_SLUG from sentry.models.organization import Organization from sentry.notifications.api.serializers.notification_action_request import ( NotificationActionSerializer, @@ -81,14 +82,19 @@ def get( Notification Actions notify a set of members when an action has been triggered through a notification service such as Slack or Sentry. For example, organization owners and managers can receive an email when a spike occurs. - You can use either the `project` or `projectSlug` query parameter to filter for certain projects. Note that if both are present, `projectSlug` takes priority. + You can use the `project` query parameter with project IDs or slugs to filter for certain projects. + A legacy project slug query parameter is also supported and takes priority if both are present. """ queryset = NotificationAction.objects.filter(organization_id=organization.id) # If a project query is specified, filter out non-project-specific actions # otherwise, include them but still ensure project permissions are enforced + requested_projects = self.get_requested_project_params_unchecked(request) + all_access_filter = ( + requested_projects.ids == ALL_ACCESS_PROJECTS and not requested_projects.slugs + ) or (requested_projects.slugs == {ALL_ACCESS_PROJECTS_SLUG} and not requested_projects.ids) project_query = ( Q(projects__in=self.get_projects(request, organization)) - if self.get_requested_project_ids_unchecked(request) + if requested_projects.has_values and not all_access_filter else Q(projects=None) | Q(projects__in=self.get_projects(request, organization)) ) queryset = queryset.filter(project_query).distinct() @@ -102,7 +108,8 @@ def get( extra={ "organization_id": organization.id, "trigger_type_query": trigger_type_query, - "project_query": self.get_requested_project_ids_unchecked(request), + "project_query": requested_projects.ids, + "project_slug_query": requested_projects.slugs, }, ) return self.paginate( diff --git a/tests/sentry/api/bases/test_organization.py b/tests/sentry/api/bases/test_organization.py index ed3e767dd22bea..5c856fbf1db6ad 100644 --- a/tests/sentry/api/bases/test_organization.py +++ b/tests/sentry/api/bases/test_organization.py @@ -17,6 +17,7 @@ OrganizationAndStaffPermission, OrganizationEndpoint, OrganizationPermission, + OrganizationReleasesBaseEndpoint, ) from sentry.api.exceptions import ( MemberDisabledOverLimit, @@ -556,15 +557,8 @@ def test_all_accessible_sigil_value_allow_joinleave(self) -> None: @mock.patch( "sentry.api.bases.organization.OrganizationEndpoint._filter_projects_by_permissions" ) - @mock.patch( - "sentry.api.bases.organization.OrganizationEndpoint.get_requested_project_ids_unchecked" - ) - def test_get_projects_no_slug_fallsback_to_ids( - self, mock_get_project_ids_unchecked, mock__filter_projects_by_permissions - ): - project_slugs = [""] - request = self.build_request(projectSlug=project_slugs) - mock_get_project_ids_unchecked.return_value = {self.project_1.id} + def test_get_projects_no_slug_fallsback_to_ids(self, mock__filter_projects_by_permissions): + request = self.build_request(projectSlug=[""], project=[str(self.project_1.id)]) def side_effect( projects, @@ -579,7 +573,6 @@ def side_effect( self.org, ) - mock_get_project_ids_unchecked.assert_called_with(request) mock__filter_projects_by_permissions.assert_called_with( projects=[self.project_1], request=request, @@ -657,6 +650,137 @@ def test_get_projects_by_slugs_no_projects_with_slug(self) -> None: with pytest.raises(PermissionDenied): self.endpoint.get_projects(request, self.org) + def test_project_param_with_slug(self) -> None: + self.create_team_membership(user=self.user, team=self.team_1) + request = self.build_request(project=[self.project_1.slug]) + + result = self.endpoint.get_projects(request, self.org) + + assert {p.id for p in result} == {self.project_1.id} + + def test_project_param_with_mixed_ids_and_slugs(self) -> None: + self.create_team_membership(user=self.user, team=self.team_3) + request = self.build_request(project=[str(self.project_1.id), self.project_2.slug]) + + result = self.endpoint.get_projects(request, self.org) + + assert {p.id for p in result} == {self.project_1.id, self.project_2.id} + + def test_project_param_with_nonexistent_slug(self) -> None: + self.create_team_membership(user=self.user, team=self.team_1) + request = self.build_request(project=["nonexistent-slug"]) + + with pytest.raises(PermissionDenied): + self.endpoint.get_projects(request, self.org) + + def test_project_slug_param_takes_precedence_over_project_param(self) -> None: + self.create_team_membership(user=self.user, team=self.team_3) + request = self.build_request( + project=[str(self.project_1.id)], projectSlug=[self.project_2.slug] + ) + + result = self.endpoint.get_projects(request, self.org) + + assert {p.id for p in result} == {self.project_2.id} + + def test_empty_explicit_project_slugs_falls_back_to_project_param(self) -> None: + self.create_team_membership(user=self.user, team=self.team_1) + request = self.build_request(project=[str(self.project_1.id)]) + + result = self.endpoint.get_projects(request, self.org, project_slugs=set()) + + assert {p.id for p in result} == {self.project_1.id} + + def test_empty_explicit_project_ids_falls_back_to_project_param_slugs(self) -> None: + self.create_team_membership(user=self.user, team=self.team_3) + request = self.build_request(project=[self.project_1.slug]) + + result = self.endpoint.get_projects(request, self.org, project_ids=set()) + + assert {p.id for p in result} == {self.project_1.id} + + @mock.patch("sentry.api.bases.organization.cache") + def test_release_permission_cache_key_uses_project_slug_precedence( + self, mock_cache: mock.MagicMock + ) -> None: + self.create_team_membership(user=self.user, team=self.team_3) + mock_cache.get.return_value = None + endpoint = OrganizationReleasesBaseEndpoint() + + endpoint.has_release_permission( + self.build_request(project=[self.project_1.slug], projectSlug=[self.project_2.slug]), + self.org, + ) + first_cache_key = mock_cache.get.call_args.args[0] + + endpoint.has_release_permission( + self.build_request(project=[self.project_2.slug], projectSlug=[self.project_1.slug]), + self.org, + ) + second_cache_key = mock_cache.get.call_args.args[0] + + assert first_cache_key != second_cache_key + + @mock.patch("sentry.api.bases.organization.cache") + def test_release_permission_cache_key_uses_project_param_slugs_when_project_ids_empty( + self, mock_cache: mock.MagicMock + ) -> None: + self.create_team_membership(user=self.user, team=self.team_3) + mock_cache.get.return_value = None + endpoint = OrganizationReleasesBaseEndpoint() + + endpoint.has_release_permission( + self.build_request(project=[self.project_1.slug]), self.org, project_ids=set() + ) + first_cache_key = mock_cache.get.call_args.args[0] + + endpoint.has_release_permission( + self.build_request(project=[self.project_2.slug]), self.org, project_ids=set() + ) + second_cache_key = mock_cache.get.call_args.args[0] + + assert first_cache_key != second_cache_key + + def test_get_requested_project_ids_unchecked_ignores_slugs(self) -> None: + request = self.build_request(project=["1", "my-slug", "42"]) + + result = self.endpoint.get_requested_project_ids_unchecked(request) + + assert result == {1, 42} + + def test_get_requested_project_ids_and_slugs_unchecked(self) -> None: + request = self.build_request(project=["1", "my-slug", "42"]) + + result = self.endpoint.get_requested_project_ids_and_slugs_unchecked(request) + + assert result.ids == {1, 42} + assert result.slugs == {"my-slug"} + + def test_query_params_with_project_slug_precedence(self) -> None: + request = self.build_request(project=["1", ""], projectSlug=["", "my-slug"]) + + result = self.endpoint.get_query_params_with_project_slug_precedence(request) + + assert result.getlist("projectSlug") == ["my-slug"] + assert "project" not in result + assert request.GET.getlist("project") == ["1", ""] + + def test_query_params_without_empty_project_params(self) -> None: + request = self.build_request(project=["", "1"]) + + result = self.endpoint.get_query_params_without_empty_project_params(request) + + assert result.getlist("project") == ["1"] + assert request.GET.getlist("project") == ["", "1"] + + def test_query_params_with_empty_project_slug_keeps_project(self) -> None: + request = self.build_request(project=["", "1"], projectSlug=[""]) + + result = self.endpoint.get_query_params_with_project_slug_precedence(request) + + assert result.getlist("projectSlug") == [] + assert result.getlist("project") == ["1"] + class GetEnvironmentsTest(BaseOrganizationEndpointTest): def setUp(self) -> None: diff --git a/tests/sentry/incidents/endpoints/test_organization_combined_rule_index_endpoint.py b/tests/sentry/incidents/endpoints/test_organization_combined_rule_index_endpoint.py index fc0145152ad391..43106716da081c 100644 --- a/tests/sentry/incidents/endpoints/test_organization_combined_rule_index_endpoint.py +++ b/tests/sentry/incidents/endpoints/test_organization_combined_rule_index_endpoint.py @@ -1486,6 +1486,33 @@ def test_workflow_engine_dual_written_rules(self) -> None: for rule in response.data: assert rule["type"] == "alert_rule" + def test_workflow_engine_filters_by_project_slug(self) -> None: + alert_rule1 = self.create_alert_rule(name="Project Slug Rule 1", projects=[self.project]) + alert_rule2 = self.create_alert_rule(name="Project Slug Rule 2", projects=[self.project2]) + + migrate_alert_rule(alert_rule1) + migrate_alert_rule(alert_rule2) + + response = self.get_success_response(self.organization.slug, project=[self.project.slug]) + + assert response.status_code == 200 + assert len(response.data) == 1 + assert response.data[0]["name"] == "Project Slug Rule 1" + + def test_workflow_engine_filters_by_project_id_and_slug(self) -> None: + alert_rule1 = self.create_alert_rule(name="Project ID Rule", projects=[self.project]) + alert_rule2 = self.create_alert_rule(name="Project Slug Rule", projects=[self.project2]) + + migrate_alert_rule(alert_rule1) + migrate_alert_rule(alert_rule2) + + response = self.get_success_response( + self.organization.slug, project=[self.project.id, self.project2.slug] + ) + + assert response.status_code == 200 + assert {rule["name"] for rule in response.data} == {"Project ID Rule", "Project Slug Rule"} + def test_workflow_engine_filtering_by_team(self) -> None: # Create second team for testing team2 = self.create_team(organization=self.organization, name="Team 2") diff --git a/tests/sentry/notifications/api/endpoints/test_notification_actions_index.py b/tests/sentry/notifications/api/endpoints/test_notification_actions_index.py index 7c7e1589dff857..b0fcb7b07b8c09 100644 --- a/tests/sentry/notifications/api/endpoints/test_notification_actions_index.py +++ b/tests/sentry/notifications/api/endpoints/test_notification_actions_index.py @@ -6,6 +6,7 @@ from rest_framework import serializers, status from sentry.api.serializers.base import serialize +from sentry.constants import ALL_ACCESS_PROJECTS_SLUG from sentry.integrations.pagerduty.utils import add_service from sentry.models.organizationmemberteam import OrganizationMemberTeam from sentry.notifications.models.notificationaction import ( @@ -91,6 +92,25 @@ def test_get_simple(self) -> None: for action in notif_actions: assert serialize(action) in response.data + def test_get_project_slug_all_includes_org_actions(self) -> None: + project_notif_action = self.create_notification_action( + organization=self.organization, + projects=self.projects, + ) + org_notif_action = self.create_notification_action(organization=self.organization) + other_notif_action = self.create_notification_action(organization=self.other_organization) + + response = self.get_success_response( + self.organization.slug, + status_code=status.HTTP_200_OK, + qs_params={"projectSlug": ALL_ACCESS_PROJECTS_SLUG}, + ) + + assert len(response.data) == 2 + assert serialize(project_notif_action) in response.data + assert serialize(org_notif_action) in response.data + assert serialize(other_notif_action) not in response.data + @patch.object( NotificationAction, "get_trigger_types", @@ -133,6 +153,10 @@ def test_get_with_queries(self, mock_trigger_types: MagicMock) -> None: "query": {"project": project.id}, "result": {na2, na3}, }, + "regular project slug": { + "query": {"project": project.slug}, + "result": {na2, na3}, + }, "regular trigger": { "query": {"triggerType": "teacher"}, "result": {na1, na2, na4}, diff --git a/tests/snuba/api/endpoints/test_organization_stats_summary.py b/tests/snuba/api/endpoints/test_organization_stats_summary.py index ed6ce6ee9b9427..1c6dfd9cca8171 100644 --- a/tests/snuba/api/endpoints/test_organization_stats_summary.py +++ b/tests/snuba/api/endpoints/test_organization_stats_summary.py @@ -605,6 +605,24 @@ def test_project_filter(self) -> None: ], } + @freeze_time(_now) + def test_project_filter_with_id_and_slug(self) -> None: + response = self.do_request( + { + "project": [self.project.id, self.project2.slug], + "statsPeriod": "1d", + "interval": "1d", + "field": ["sum(quantity)"], + "category": ["error", "transaction"], + } + ) + + assert response.status_code == 200, response.content + assert {project["id"] for project in response.data["projects"]} == { + self.project.id, + self.project2.id, + } + @freeze_time(_now) def test_reason_filter(self) -> None: make_request = functools.partial( diff --git a/tests/snuba/api/endpoints/test_organization_stats_v2.py b/tests/snuba/api/endpoints/test_organization_stats_v2.py index 9bf670cd9bf938..5f88952468283b 100644 --- a/tests/snuba/api/endpoints/test_organization_stats_v2.py +++ b/tests/snuba/api/endpoints/test_organization_stats_v2.py @@ -381,6 +381,27 @@ def test_user_no_proj_specific_access(self) -> None: "groups": [], } + @freeze_time(_now) + def test_user_no_proj_specific_access_with_multiple_groupbys(self) -> None: + response = self.do_request( + { + "project": [-1], + "statsPeriod": "1d", + "interval": "1d", + "field": ["sum(quantity)"], + "category": ["error", "transaction"], + "groupBy": ["category", "project"], + }, + user=self.user2, + status_code=200, + ) + + assert result_sorted(response.data) == { + "start": isoformat_z(floor_to_utc_day(self._now) - timedelta(days=1)), + "end": isoformat_z(floor_to_utc_day(self._now) + timedelta(days=1)), + "groups": [], + } + @freeze_time(_now) def test_no_project_access(self) -> None: user = self.create_user(is_superuser=False) @@ -700,6 +721,48 @@ def test_project_filter(self) -> None: ], } + @freeze_time(_now) + def test_project_slug_filter(self) -> None: + response = self.do_request( + { + "project": self.project.slug, + "statsPeriod": "1d", + "interval": "1d", + "field": ["sum(quantity)"], + "category": ["error", "transaction"], + }, + org=self.org, + status_code=200, + ) + + assert result_sorted(response.data) == { + "start": isoformat_z(floor_to_utc_day(self._now) - timedelta(days=1)), + "end": isoformat_z(floor_to_utc_day(self._now) + timedelta(days=1)), + "intervals": [ + isoformat_z(floor_to_utc_day(self._now) - timedelta(days=1)), + isoformat_z(floor_to_utc_day(self._now)), + ], + "groups": [ + {"by": {}, "totals": {"sum(quantity)": 6}, "series": {"sum(quantity)": [0, 6]}} + ], + } + + @freeze_time(_now) + def test_project_filter_with_id_and_slug(self) -> None: + response = self.do_request( + { + "project": [self.project.id, self.project2.slug], + "statsPeriod": "1d", + "interval": "1d", + "field": ["sum(quantity)"], + "category": ["error", "transaction"], + }, + org=self.org, + status_code=200, + ) + + assert response.data["groups"][0]["totals"]["sum(quantity)"] == 7 + @freeze_time(_now) def test_staff_project_filter(self) -> None: staff_user = self.create_user(is_staff=True, is_superuser=True)