From 1353939da538a3a4ec84427676e25653c49a93f1 Mon Sep 17 00:00:00 2001 From: Alexander Tarasov Date: Wed, 15 Jan 2025 17:52:06 +0100 Subject: [PATCH] fix(issues): inherit ShortIdLookupEndpoint from GroupEndpoint (#83502) --- api-docs/openapi.json | 2 +- api-docs/paths/organizations/shortid.json | 6 +-- src/sentry/api/urls.py | 2 +- ...pi_publish_status_allowlist_dont_modify.py | 1 - .../issues/endpoints/organization_shortid.py | 23 +++------- .../endpoints/organizations/test_shortid.py | 2 +- .../endpoints/test_organization_shortid.py | 44 +++++++++++++------ 7 files changed, 44 insertions(+), 36 deletions(-) diff --git a/api-docs/openapi.json b/api-docs/openapi.json index a40ec189185cb3..90a7809f2e4080 100644 --- a/api-docs/openapi.json +++ b/api-docs/openapi.json @@ -106,7 +106,7 @@ "/api/0/organizations/{organization_id_or_slug}/repos/{repo_id}/commits/": { "$ref": "paths/organizations/repo-commits.json" }, - "/api/0/organizations/{organization_id_or_slug}/shortids/{short_id}/": { + "/api/0/organizations/{organization_id_or_slug}/shortids/{issue_id}/": { "$ref": "paths/organizations/shortid.json" }, "/api/0/projects/": { diff --git a/api-docs/paths/organizations/shortid.json b/api-docs/paths/organizations/shortid.json index 15a3291d1d7a9c..404cb67e09e526 100644 --- a/api-docs/paths/organizations/shortid.json +++ b/api-docs/paths/organizations/shortid.json @@ -1,7 +1,7 @@ { "get": { "tags": ["Organizations"], - "description": "This resolves a short ID to the project slug and internal issue ID.", + "description": "This resolves a short ID or internal issue ID to the project slug and group details.", "operationId": "Resolve a Short ID", "parameters": [ { @@ -14,9 +14,9 @@ } }, { - "name": "short_id", + "name": "issue_id", "in": "path", - "description": "The short ID to look up.", + "description": "The short ID or issue ID to look up.", "required": true, "schema": { "type": "string" diff --git a/src/sentry/api/urls.py b/src/sentry/api/urls.py index 0b868c3d5b8186..57b32c88d59aef 100644 --- a/src/sentry/api/urls.py +++ b/src/sentry/api/urls.py @@ -1286,7 +1286,7 @@ def create_group_urls(name_prefix: str) -> list[URLPattern | URLResolver]: name="sentry-api-0-organization-dashboard-favorite", ), re_path( - r"^(?P[^\/]+)/shortids/(?P[^\/]+)/$", + r"^(?P[^\/]+)/shortids/(?P[^\/]+)/$", ShortIdLookupEndpoint.as_view(), name="sentry-api-0-short-id-lookup", ), diff --git a/src/sentry/apidocs/api_publish_status_allowlist_dont_modify.py b/src/sentry/apidocs/api_publish_status_allowlist_dont_modify.py index ccfe6fc5831f24..412ab0356990d9 100644 --- a/src/sentry/apidocs/api_publish_status_allowlist_dont_modify.py +++ b/src/sentry/apidocs/api_publish_status_allowlist_dont_modify.py @@ -303,7 +303,6 @@ "PUT", }, "/api/0/organizations/{organization_id_or_slug}/dashboards/{dashboard_id}/visit/": {"POST"}, - "/api/0/organizations/{organization_id_or_slug}/shortids/{short_id}/": {"GET"}, "/api/0/organizations/{organization_id_or_slug}/eventids/{event_id}/": {"GET"}, "/api/0/organizations/{organization_id_or_slug}/data-scrubbing-selector-suggestions/": {"GET"}, "/api/0/organizations/{organization_id_or_slug}/access-requests/": {"GET", "PUT"}, diff --git a/src/sentry/issues/endpoints/organization_shortid.py b/src/sentry/issues/endpoints/organization_shortid.py index 0f08a7e6d867f8..741be3dd7698eb 100644 --- a/src/sentry/issues/endpoints/organization_shortid.py +++ b/src/sentry/issues/endpoints/organization_shortid.py @@ -4,40 +4,31 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint -from sentry.api.bases.organization import OrganizationEndpoint -from sentry.api.exceptions import ResourceDoesNotExist +from sentry.api.bases import GroupEndpoint from sentry.api.serializers import serialize from sentry.models.group import Group -from sentry.models.organization import Organization @region_silo_endpoint -class ShortIdLookupEndpoint(OrganizationEndpoint): +class ShortIdLookupEndpoint(GroupEndpoint): owner = ApiOwner.ISSUES publish_status = { - "GET": ApiPublishStatus.UNKNOWN, + "GET": ApiPublishStatus.PRIVATE, } - def get(self, request: Request, organization: Organization, short_id: str) -> Response: + def get(self, request: Request, group: Group) -> Response: """ Resolve a Short ID `````````````````` - This resolves a short ID to the project slug and internal issue ID. + This resolves a short ID or internal issue ID to the project slug and group details. - :pparam string organization_id_or_slug: the id or slug of the organization the - short ID should be looked up in. - :pparam string short_id: the short ID to look up. + :pparam string issue_id: the short ID or issue ID to look up. :auth: required """ - try: - group = Group.objects.by_qualified_short_id(organization.id, short_id) - except Group.DoesNotExist: - raise ResourceDoesNotExist() - return Response( { - "organizationSlug": organization.slug, + "organizationSlug": group.project.organization.slug, "projectSlug": group.project.slug, "groupId": str(group.id), "group": serialize(group, request.user), diff --git a/tests/apidocs/endpoints/organizations/test_shortid.py b/tests/apidocs/endpoints/organizations/test_shortid.py index ad8895cabc1d8a..00933a8bc91f3e 100644 --- a/tests/apidocs/endpoints/organizations/test_shortid.py +++ b/tests/apidocs/endpoints/organizations/test_shortid.py @@ -12,7 +12,7 @@ def setUp(self): "sentry-api-0-short-id-lookup", kwargs={ "organization_id_or_slug": self.organization.slug, - "short_id": group.qualified_short_id, + "issue_id": group.qualified_short_id, }, ) diff --git a/tests/sentry/issues/endpoints/test_organization_shortid.py b/tests/sentry/issues/endpoints/test_organization_shortid.py index 06b6cf1571a8de..1c8eeaa6190fb4 100644 --- a/tests/sentry/issues/endpoints/test_organization_shortid.py +++ b/tests/sentry/issues/endpoints/test_organization_shortid.py @@ -7,20 +7,38 @@ class ShortIdLookupEndpointTest(APITestCase): - def test_simple(self) -> None: - org = self.create_organization(owner=self.user) - project = self.create_project(organization=org) - group = self.create_group(project=project, short_id=project.next_short_id()) - - self.login_as(user=self.user) - url = reverse( + def setUp(self) -> None: + self.group = self.create_group(project=self.project, short_id=self.project.next_short_id()) + self.url = reverse( "sentry-api-0-short-id-lookup", - kwargs={"organization_id_or_slug": org.slug, "short_id": group.qualified_short_id}, + kwargs={ + "organization_id_or_slug": self.organization.slug, + "issue_id": self.group.qualified_short_id, + }, ) - response = self.client.get(url, format="json") + + def test_simple(self) -> None: + self.login_as(user=self.user) + response = self.client.get(self.url, format="json") assert response.status_code == 200, response.content - assert response.data["organizationSlug"] == org.slug - assert response.data["projectSlug"] == project.slug - assert response.data["groupId"] == str(group.id) - assert response.data["group"]["id"] == str(group.id) + assert response.data["organizationSlug"] == self.organization.slug + assert response.data["projectSlug"] == self.project.slug + assert response.data["groupId"] == str(self.group.id) + assert response.data["group"]["id"] == str(self.group.id) + + def test_access_non_member_project(self) -> None: + # disable Open Membership + self.organization.flags.allow_joinleave = False + self.organization.save() + + # user has no access to the first project + user_no_team = self.create_user(is_superuser=False) + self.create_member( + user=user_no_team, organization=self.organization, role="member", teams=[] + ) + self.login_as(user_no_team) + + response = self.client.get(self.url, format="json") + assert response.status_code == 403, response.content + assert response.data["detail"] == "You do not have permission to perform this action."