Skip to content

feat(api-idorslug): Update Subset of Monitor & Integration Endpoints and to use organization_id_or_slug #70784

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 6 commits into from
May 13, 2024
Merged

feat(api-idorslug): Update Subset of Monitor & Integration Endpoints and to use organization_id_or_slug #70784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
714d366
update endpoints
iamrajjoshi May 13, 2024
a439082
fix tests
iamrajjoshi May 13, 2024
79a21ee
revert change
iamrajjoshi May 13, 2024
9a41963
updated tests
iamrajjoshi May 13, 2024
6040ddc
updated comments
iamrajjoshi May 13, 2024
6a7bef1
Merge branch 'master' into raj/api-idorslug/rename-path-params/organi…
iamrajjoshi May 13, 2024
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
34 changes: 21 additions & 13 deletions src/sentry/api/endpoints/organization_unsubscribe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class OrganizationUnsubscribeBase(Endpoint, Generic[T]):

object_type = "unknown"

def fetch_instance(self, request: Request, organization_slug: str, id: int) -> T:
def fetch_instance(self, request: Request, organization_id_or_slug: int | str, id: int) -> T:
raise NotImplementedError()

def unsubscribe(self, request: Request, instance: T):
Expand All @@ -46,10 +46,12 @@ def unsubscribe(self, request: Request, instance: T):
def add_instance_data(self, data: dict[str, Any], instance: T) -> dict[str, Any]:
return data

def get(self, request: Request, organization_slug: str, id: int, **kwargs) -> Response:
def get(
self, request: Request, organization_id_or_slug: int | str, id: int, **kwargs
) -> Response:
if not request.user_from_signed_request:
raise NotFound()
instance = self.fetch_instance(request, organization_slug, id)
instance = self.fetch_instance(request, organization_id_or_slug, id)
view_url = ""
if hasattr(instance, "get_absolute_url"):
view_url = str(instance.get_absolute_url())
Expand All @@ -65,10 +67,12 @@ def get(self, request: Request, organization_slug: str, id: int, **kwargs) -> Re
}
return Response(self.add_instance_data(data, instance), 200)

def post(self, request: Request, organization_slug: str, id: int, **kwargs) -> Response:
def post(
self, request: Request, organization_id_or_slug: int | str, id: int, **kwargs
) -> Response:
if not request.user_from_signed_request:
raise NotFound()
instance = self.fetch_instance(request, organization_slug, id)
instance = self.fetch_instance(request, organization_id_or_slug, id)

if request.data.get("cancel"):
self.unsubscribe(request, instance)
Expand All @@ -79,16 +83,18 @@ def post(self, request: Request, organization_slug: str, id: int, **kwargs) -> R
class OrganizationUnsubscribeProject(OrganizationUnsubscribeBase[Project]):
object_type = "project"

def fetch_instance(self, request: Request, organization_slug: str, id: int) -> Project:
def fetch_instance(
self, request: Request, organization_id_or_slug: int | str, id: int
) -> Project:
try:
project = Project.objects.select_related("organization").get(id=id)
except Project.DoesNotExist:
raise NotFound()
if str(organization_slug).isdecimal():
if project.organization.id != int(organization_slug):
if str(organization_id_or_slug).isdecimal():
if project.organization.id != int(organization_id_or_slug):
raise NotFound()
else:
if project.organization.slug != organization_slug:
if project.organization.slug != organization_id_or_slug:
raise NotFound()
if not OrganizationMember.objects.filter(
user_id=request.user.pk, organization_id=project.organization_id
Expand All @@ -115,16 +121,18 @@ def unsubscribe(self, request: Request, instance: Project):
class OrganizationUnsubscribeIssue(OrganizationUnsubscribeBase[Group]):
object_type = "issue"

def fetch_instance(self, request: Request, organization_slug: str, issue_id: int) -> Group:
def fetch_instance(
self, request: Request, organization_id_or_slug: int | str, issue_id: int
) -> Group:
try:
issue = Group.objects.get_from_cache(id=issue_id)
except Group.DoesNotExist:
raise NotFound()
if str(organization_slug).isdecimal():
if issue.organization.id != int(organization_slug):
if str(organization_id_or_slug).isdecimal():
if issue.organization.id != int(organization_id_or_slug):
raise NotFound()
else:
if issue.organization.slug != organization_slug:
if issue.organization.slug != organization_id_or_slug:
raise NotFound()

if not OrganizationMember.objects.filter(
Expand Down
34 changes: 17 additions & 17 deletions src/sentry/api/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1643,47 +1643,47 @@ def create_group_urls(name_prefix: str) -> list[URLPattern | URLResolver]:
),
# Monitors
re_path(
r"^(?P<organization_slug>[^\/]+)/monitors/$",
r"^(?P<organization_id_or_slug>[^\/]+)/monitors/$",
OrganizationMonitorIndexEndpoint.as_view(),
name="sentry-api-0-organization-monitor-index",
),
re_path(
r"^(?P<organization_slug>[^\/]+)/monitors-stats/$",
r"^(?P<organization_id_or_slug>[^\/]+)/monitors-stats/$",
OrganizationMonitorIndexStatsEndpoint.as_view(),
name="sentry-api-0-organization-monitor-index-stats",
),
re_path(
r"^(?P<organization_slug>[^\/]+)/processing-errors/$",
r"^(?P<organization_id_or_slug>[^\/]+)/processing-errors/$",
OrganizationMonitorProcessingErrorsIndexEndpoint.as_view(),
name="sentry-api-0-organization-monitor-processing-errors-index",
),
re_path(
r"^(?P<organization_slug>[^\/]+)/monitors-schedule-data/$",
r"^(?P<organization_id_or_slug>[^\/]+)/monitors-schedule-data/$",
OrganizationMonitorScheduleSampleDataEndpoint.as_view(),
name="sentry-api-0-organization-monitors-schedule-sample-data",
),
re_path(
r"^(?P<organization_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/$",
r"^(?P<organization_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/$",
OrganizationMonitorDetailsEndpoint.as_view(),
name="sentry-api-0-organization-monitor-details",
),
re_path(
r"^(?P<organization_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/environments/(?P<environment>[^\/]+)$",
r"^(?P<organization_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/environments/(?P<environment>[^\/]+)$",
OrganizationMonitorEnvironmentDetailsEndpoint.as_view(),
name="sentry-api-0-organization-monitor-environment-details",
),
re_path(
r"^(?P<organization_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/stats/$",
r"^(?P<organization_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/stats/$",
OrganizationMonitorStatsEndpoint.as_view(),
name="sentry-api-0-organization-monitor-stats",
),
re_path(
r"^(?P<organization_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/checkins/$",
r"^(?P<organization_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/checkins/$",
OrganizationMonitorCheckInIndexEndpoint.as_view(),
name="sentry-api-0-organization-monitor-check-in-index",
),
re_path(
r"^(?P<organization_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/checkins/(?P<checkin_id>[^\/]+)/attachment/$",
r"^(?P<organization_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/checkins/(?P<checkin_id>[^\/]+)/attachment/$",
method_dispatch(
GET=OrganizationMonitorCheckInAttachmentEndpoint.as_view(),
OPTIONS=OrganizationMonitorCheckInAttachmentEndpoint.as_view(),
Expand Down Expand Up @@ -2120,12 +2120,12 @@ def create_group_urls(name_prefix: str) -> list[URLPattern | URLResolver]:
),
# Unsubscribe from organization notifications
re_path(
r"^(?P<organization_slug>[^/]+)/unsubscribe/project/(?P<id>\d+)/$",
r"^(?P<organization_id_or_slug>[^/]+)/unsubscribe/project/(?P<id>\d+)/$",
OrganizationUnsubscribeProject.as_view(),
name="sentry-api-0-organization-unsubscribe-project",
),
re_path(
r"^(?P<organization_slug>[^/]+)/unsubscribe/issue/(?P<id>\d+)/$",
r"^(?P<organization_id_or_slug>[^/]+)/unsubscribe/issue/(?P<id>\d+)/$",
OrganizationUnsubscribeIssue.as_view(),
name="sentry-api-0-organization-unsubscribe-issue",
),
Expand Down Expand Up @@ -2743,22 +2743,22 @@ def create_group_urls(name_prefix: str) -> list[URLPattern | URLResolver]:
name="sentry-api-0-project-statistical-detector",
),
re_path(
r"^(?P<organization_slug>[^\/]+)/(?P<project_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/checkins/(?P<checkin_id>[^\/]+)/attachment/$",
r"^(?P<organization_id_or_slug>[^\/]+)/(?P<project_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/checkins/(?P<checkin_id>[^\/]+)/attachment/$",
ProjectMonitorCheckInAttachmentEndpoint.as_view(),
name="sentry-api-0-project-monitor-check-in-attachment",
),
re_path(
r"^(?P<organization_slug>[^\/]+)/(?P<project_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/$",
r"^(?P<organization_id_or_slug>[^\/]+)/(?P<project_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/$",
ProjectMonitorDetailsEndpoint.as_view(),
name="sentry-api-0-project-monitor-details",
),
re_path(
r"^(?P<organization_slug>[^\/]+)/(?P<project_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/checkins/$",
r"^(?P<organization_id_or_slug>[^\/]+)/(?P<project_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/checkins/$",
ProjectMonitorCheckInIndexEndpoint.as_view(),
name="sentry-api-0-project-monitor-check-in-index",
),
re_path(
r"^(?P<organization_slug>[^\/]+)/(?P<project_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/environments/(?P<environment>[^\/]+)$",
r"^(?P<organization_id_or_slug>[^\/]+)/(?P<project_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/environments/(?P<environment>[^\/]+)$",
ProjectMonitorEnvironmentDetailsEndpoint.as_view(),
name="sentry-api-0-project-monitor-environment-details",
),
Expand All @@ -2768,12 +2768,12 @@ def create_group_urls(name_prefix: str) -> list[URLPattern | URLResolver]:
name="sentry-api-0-project-processing-errors-details",
),
re_path(
r"^(?P<organization_slug>[^\/]+)/(?P<project_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/processing-errors/$",
r"^(?P<organization_id_or_slug>[^\/]+)/(?P<project_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/processing-errors/$",
ProjectMonitorProcessingErrorsIndexEndpoint.as_view(),
name="sentry-api-0-project-monitor-processing-errors-index",
),
re_path(
r"^(?P<organization_slug>[^\/]+)/(?P<project_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/stats/$",
r"^(?P<organization_id_or_slug>[^\/]+)/(?P<project_id_or_slug>[^\/]+)/monitors/(?P<monitor_id_or_slug>[^\/]+)/stats/$",
ProjectMonitorStatsEndpoint.as_view(),
name="sentry-api-0-project-monitor-stats",
),
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/integrations/bitbucket/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
name="sentry-extensions-bitbucket-webhook",
),
re_path(
r"^search/(?P<organization_slug>[^\/]+)/(?P<integration_id>\d+)/$",
r"^search/(?P<organization_id_or_slug>[^\/]+)/(?P<integration_id>\d+)/$",
BitbucketSearchEndpoint.as_view(),
name="sentry-extensions-bitbucket-search",
),
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/integrations/github/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
name="sentry-integration-github-installation",
),
re_path(
r"^search/(?P<organization_slug>[^\/]+)/(?P<integration_id>\d+)/$",
r"^search/(?P<organization_id_or_slug>[^\/]+)/(?P<integration_id>\d+)/$",
GithubSharedSearchEndpoint.as_view(),
name="sentry-integration-github-search",
),
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/integrations/gitlab/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

urlpatterns = [
re_path(
r"^search/(?P<organization_slug>[^\/]+)/(?P<integration_id>\d+)/$",
r"^search/(?P<organization_id_or_slug>[^\/]+)/(?P<integration_id>\d+)/$",
GitlabIssueSearchEndpoint.as_view(),
name="sentry-extensions-gitlab-search",
),
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/integrations/jira/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
name="sentry-extensions-jira-issue-updated",
),
re_path(
r"^search/(?P<organization_slug>[^\/]+)/(?P<integration_id>\d+)/$",
r"^search/(?P<organization_id_or_slug>[^\/]+)/(?P<integration_id>\d+)/$",
JiraSearchEndpoint.as_view(),
name="sentry-extensions-jira-search",
),
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/integrations/jira_server/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
name="sentry-extensions-jiraserver-issue-updated",
),
re_path(
r"^search/(?P<organization_slug>[^\/]+)/(?P<integration_id>\d+)/$",
r"^search/(?P<organization_id_or_slug>[^\/]+)/(?P<integration_id>\d+)/$",
JiraServerSearchEndpoint.as_view(),
name="sentry-extensions-jiraserver-search",
),
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/integrations/vsts/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
name="sentry-extensions-vsts-issue-updated",
),
re_path(
r"^search/(?P<organization_slug>[^\/]+)/(?P<integration_id>\d+)/$",
r"^search/(?P<organization_id_or_slug>[^\/]+)/(?P<integration_id>\d+)/$",
VstsSearchEndpoint.as_view(),
name="sentry-extensions-vsts-search",
),
Expand Down
10 changes: 5 additions & 5 deletions src/sentry/monitors/endpoints/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class MonitorEndpoint(Endpoint):
def convert_args(
self,
request: Request,
organization_slug: str,
organization_id_or_slug: int | str,
monitor_id_or_slug: int | str,
environment: str | None = None,
checkin_id: str | None = None,
Expand All @@ -67,13 +67,13 @@ def convert_args(
try:
if (
id_or_slug_path_params_enabled(
self.convert_args.__qualname__, str(organization_slug)
self.convert_args.__qualname__, str(organization_id_or_slug)
)
and str(organization_slug).isdigit()
and str(organization_id_or_slug).isdigit()
):
organization = Organization.objects.get_from_cache(id=organization_slug)
organization = Organization.objects.get_from_cache(id=organization_id_or_slug)
else:
organization = Organization.objects.get_from_cache(slug=organization_slug)
organization = Organization.objects.get_from_cache(slug=organization_id_or_slug)
except Organization.DoesNotExist:
raise ResourceDoesNotExist

Expand Down
34 changes: 19 additions & 15 deletions src/sentry/monitors/endpoints/monitor_ingest_checkin_attachment.py
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

we should update the comments here too to be accurate to what is happening (ie. all the references to slug)

Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def convert_args(
request: Request,
monitor_id_or_slug: int | str,
checkin_id: str,
organization_slug: str | int | None = None,
organization_id_or_slug: int | str | None = None,
*args,
**kwargs,
):
Expand All @@ -83,24 +83,28 @@ def convert_args(
raise ResourceDoesNotExist
else:

# When using DSN auth we're able to infer the organization slug
if not organization_slug and using_dsn_auth:
organization_slug = request.auth.project.organization.slug
# When using DSN auth we're able to infer the organization slug (organization_id_or_slug is slug in this case)
if not organization_id_or_slug and using_dsn_auth:
organization_id_or_slug = request.auth.project.organization.slug

# The only monitor endpoints that do not have the org slug in their
# The only monitor endpoints that do not have the org id or slug in their
# parameters are the GUID-style checkin endpoints
if organization_slug:
if organization_id_or_slug:
try:
# Try lookup by slug first. This requires organization context.
# Try lookup by id or slug first. This requires organization context.
if (
id_or_slug_path_params_enabled(
self.convert_args.__qualname__, str(organization_slug)
self.convert_args.__qualname__, str(organization_id_or_slug)
)
and str(organization_slug).isdecimal()
and str(organization_id_or_slug).isdecimal()
):
organization = Organization.objects.get_from_cache(id=organization_slug)
organization = Organization.objects.get_from_cache(
id=organization_id_or_slug
)
else:
organization = Organization.objects.get_from_cache(slug=organization_slug)
organization = Organization.objects.get_from_cache(
slug=organization_id_or_slug
)

monitor = get_monitor_by_org_id_or_slug(organization, monitor_id_or_slug)
except (Organization.DoesNotExist, Monitor.DoesNotExist):
Expand Down Expand Up @@ -140,12 +144,12 @@ def convert_args(
# When looking up via GUID we do not check the organization slug,
# validate that the slug matches the org of the monitors project

# We only raise if the organization_slug was set and it doesn't match.
# We only raise if the organization_id_or_slug was set and it doesn't match.
# We don't check the api.id-or-slug-enabled option here because slug and id are unique
if (
organization_slug
and project.organization.slug != organization_slug
and project.organization.id != organization_slug
organization_id_or_slug
and project.organization.slug != organization_id_or_slug
and project.organization.id != organization_id_or_slug
):
raise ResourceDoesNotExist

Expand Down
2 changes: 1 addition & 1 deletion src/sentry/monitors/endpoints/organization_monitor_checkin_index.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class OrganizationMonitorCheckInIndexEndpoint(MonitorEndpoint, MonitorCheckInMix
@extend_schema(
operation_id="Retrieve Check-Ins for a Monitor",
parameters=[
GlobalParams.ORG_SLUG,
GlobalParams.ORG_ID_OR_SLUG,
MonitorParams.MONITOR_ID_OR_SLUG,
],
responses={
Expand Down
6 changes: 3 additions & 3 deletions src/sentry/monitors/endpoints/organization_monitor_details.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class OrganizationMonitorDetailsEndpoint(MonitorEndpoint, MonitorDetailsMixin):
@extend_schema(
operation_id="Retrieve a Monitor",
parameters=[
GlobalParams.ORG_SLUG,
GlobalParams.ORG_ID_OR_SLUG,
MonitorParams.MONITOR_ID_OR_SLUG,
GlobalParams.ENVIRONMENT,
],
Expand All @@ -56,7 +56,7 @@ def get(self, request: Request, organization, project, monitor) -> Response:
@extend_schema(
operation_id="Update a Monitor",
parameters=[
GlobalParams.ORG_SLUG,
GlobalParams.ORG_ID_OR_SLUG,
MonitorParams.MONITOR_ID_OR_SLUG,
],
request=MonitorValidator,
Expand All @@ -77,7 +77,7 @@ def put(self, request: AuthenticatedHttpRequest, organization, project, monitor)
@extend_schema(
operation_id="Delete a Monitor or Monitor Environments",
parameters=[
GlobalParams.ORG_SLUG,
GlobalParams.ORG_ID_OR_SLUG,
MonitorParams.MONITOR_ID_OR_SLUG,
GlobalParams.ENVIRONMENT,
],
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/monitors/endpoints/organization_monitor_environment_details.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class OrganizationMonitorEnvironmentDetailsEndpoint(
@extend_schema(
operation_id="Update a Monitor Environment",
parameters=[
GlobalParams.ORG_SLUG,
GlobalParams.ORG_ID_OR_SLUG,
MonitorParams.MONITOR_ID_OR_SLUG,
MonitorParams.ENVIRONMENT,
],
Expand All @@ -58,7 +58,7 @@ def put(
@extend_schema(
operation_id="Delete a Monitor Environments",
parameters=[
GlobalParams.ORG_SLUG,
GlobalParams.ORG_ID_OR_SLUG,
MonitorParams.MONITOR_ID_OR_SLUG,
MonitorParams.ENVIRONMENT,
],
Expand Down
Loading
Loading