From e860e53ebc3e6f8c518b53718ae2be0f2f12b069 Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Thu, 17 Sep 2026 16:00:54 -0700 Subject: [PATCH 1/3] ref(api): type fetchMutation urls with getApiUrl `fetchMutation` took its `url` as a plain `string`, so every call site built a path by interpolation and nothing checked that the result was a real endpoint. Narrow the field to the branded `ApiUrl` that `getApiUrl()` returns and convert the remaining call sites, so a typo or a renamed route is a type error rather than a 404 at runtime. Two call sites carried a query parameter inside the path string - `?_=` on the unsubscribe pages and `?rerun=true` on the preprod build comparison. Those move to `options.query`, since appending to a branded url widens it back to `string`. Delete `getExternalActorEndpointDetails`, which took a collection endpoint and appended the mapping id for `PUT`. That only works on an opaque string, so each `getBaseFormEndpoint` now returns the finished url for its own resource and the form only chooses the method. The admin delete-billing-metric-history call previously requested `/api/0/customers/...`, which double-prefixed the client's own base url; it now uses the same unprefixed path as the rest of the admin views. --- .../components/feedback/useMutateFeedback.tsx | 9 ++++-- .../onboarding/createSampleEventButton.tsx | 8 ++++- .../replays/table/replayBulkViewedActions.tsx | 12 +++++++- .../askSeerCombobox/askSeerComboBox.spec.tsx | 5 +++- .../searchQueryBuilder/index.spec.tsx | 8 ++++- static/app/utils/api/knownGetsentryApiUrls.ts | 2 ++ static/app/utils/integrationUtil.tsx | 17 ----------- static/app/utils/queryClient.tsx | 3 +- .../replays/hooks/useMarkReplayViewed.tsx | 23 ++++++++++++-- .../discardIssueMutationOptions.ts | 8 ++++- .../buildComparison/buildComparison.tsx | 6 +++- .../organizationApiKeyDetails.tsx | 5 +++- .../integrationExternalMappingForm.spec.tsx | 6 +++- .../integrationExternalMappingForm.tsx | 28 +++++++---------- .../integrationExternalMappings.spec.tsx | 26 +++++++++++++--- .../integrationExternalTeamMappings.tsx | 30 ++++++++++++++----- static/app/views/unsubscribe/issue.spec.tsx | 9 ++++-- static/app/views/unsubscribe/issue.tsx | 7 ++++- static/app/views/unsubscribe/project.spec.tsx | 9 ++++-- static/app/views/unsubscribe/project.tsx | 7 ++++- .../deleteBillingMetricHistory.spec.tsx | 6 ++-- .../components/deleteBillingMetricHistory.tsx | 8 ++++- static/gsAdmin/views/customerDetails.tsx | 7 ++++- .../intentForms/setupIntentForm.tsx | 4 +-- static/gsApp/hooks/useIntentData.tsx | 7 +++-- 25 files changed, 182 insertions(+), 78 deletions(-) diff --git a/static/app/components/feedback/useMutateFeedback.tsx b/static/app/components/feedback/useMutateFeedback.tsx index 16c72547ad3a..5ffc925eccf6 100644 --- a/static/app/components/feedback/useMutateFeedback.tsx +++ b/static/app/components/feedback/useMutateFeedback.tsx @@ -7,6 +7,7 @@ import type {Actor} from 'sentry/types/core'; import type {GroupStatus} from 'sentry/types/group'; import type {Organization} from 'sentry/types/organization'; import {parseQueryKey} from 'sentry/utils/api/apiQueryKey'; +import {getApiUrl} from 'sentry/utils/api/getApiUrl'; import {fetchMutation} from 'sentry/utils/queryClient'; type TFeedbackIds = 'all' | string[]; @@ -35,8 +36,12 @@ export function useMutateFeedback({feedbackIds, organization, projectIds}: Props mutationFn: ([ids, payload]) => { const isSingleId = ids !== 'all' && ids.length === 1; const url = isSingleId - ? `/organizations/${organization.slug}/issues/${ids[0]}/` - : `/organizations/${organization.slug}/issues/`; + ? getApiUrl('/organizations/$organizationIdOrSlug/issues/$issueId/', { + path: {organizationIdOrSlug: organization.slug, issueId: String(ids[0])}, + }) + : getApiUrl('/organizations/$organizationIdOrSlug/issues/', { + path: {organizationIdOrSlug: organization.slug}, + }); // TODO: it would be excellent if `PUT /issues/` could return the same data // as `GET /issues/` when query params are set. IE: it should expand inbox & owners diff --git a/static/app/components/onboarding/createSampleEventButton.tsx b/static/app/components/onboarding/createSampleEventButton.tsx index a16e7c72a373..37bdf48d665e 100644 --- a/static/app/components/onboarding/createSampleEventButton.tsx +++ b/static/app/components/onboarding/createSampleEventButton.tsx @@ -14,6 +14,7 @@ import {t} from 'sentry/locale'; import type {Project} from 'sentry/types/project'; import {trackAnalytics} from 'sentry/utils/analytics'; import {apiOptions} from 'sentry/utils/api/apiOptions'; +import {getApiUrl} from 'sentry/utils/api/getApiUrl'; import {fetchMutation} from 'sentry/utils/queryClient'; import {normalizeUrl} from 'sentry/utils/url/normalizeUrl'; import {useNavigate} from 'sentry/utils/useNavigate'; @@ -57,7 +58,12 @@ export function CreateSampleEventButton({ const {mutate: createSampleGroup, isPending} = useMutation({ mutationFn: () => { - const url = `/projects/${organization.slug}/${project!.slug}/create-sample/`; + const url = getApiUrl( + '/projects/$organizationIdOrSlug/$projectIdOrSlug/create-sample/', + { + path: {organizationIdOrSlug: organization.slug, projectIdOrSlug: project!.slug}, + } + ); return fetchMutation<{groupID: string}>({method: 'POST', url}); }, onMutate() { diff --git a/static/app/components/replays/table/replayBulkViewedActions.tsx b/static/app/components/replays/table/replayBulkViewedActions.tsx index e4b1be7c387a..f26bd4ab55b1 100644 --- a/static/app/components/replays/table/replayBulkViewedActions.tsx +++ b/static/app/components/replays/table/replayBulkViewedActions.tsx @@ -8,6 +8,7 @@ import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {IconCheckmark} from 'sentry/icons'; import {t, tn} from 'sentry/locale'; import {trackAnalytics} from 'sentry/utils/analytics'; +import {getApiUrl} from 'sentry/utils/api/getApiUrl'; import type {ListCheckboxQueryKeyRef} from 'sentry/utils/list/useListItemCheckboxState'; import {fetchMutation} from 'sentry/utils/queryClient'; import {replayListApiOptions} from 'sentry/utils/replays/replayListApiOptions'; @@ -40,7 +41,16 @@ export function ReplayBulkViewedActions({ const results = await Promise.allSettled( selectedRows.map(replay => { - const url = `/projects/${organization.slug}/${replay.project_id}/replays/${replay.id}/viewed-by/`; + const url = getApiUrl( + '/projects/$organizationIdOrSlug/$projectIdOrSlug/replays/$replayId/viewed-by/', + { + path: { + organizationIdOrSlug: organization.slug, + projectIdOrSlug: String(replay.project_id), + replayId: replay.id, + }, + } + ); return fetchMutation({method: 'POST', url}).then(() => replay.id); }) diff --git a/static/app/components/searchQueryBuilder/askSeerCombobox/askSeerComboBox.spec.tsx b/static/app/components/searchQueryBuilder/askSeerCombobox/askSeerComboBox.spec.tsx index f15584474edc..c5892126fa39 100644 --- a/static/app/components/searchQueryBuilder/askSeerCombobox/askSeerComboBox.spec.tsx +++ b/static/app/components/searchQueryBuilder/askSeerCombobox/askSeerComboBox.spec.tsx @@ -14,6 +14,7 @@ import { useSearchQueryBuilderAI, } from 'sentry/components/searchQueryBuilder/context'; import * as analytics from 'sentry/utils/analytics'; +import {getApiUrl} from 'sentry/utils/api/getApiUrl'; import {fetchMutation} from 'sentry/utils/queryClient'; import {GlobalFeedbackForm} from 'sentry/utils/useFeedbackForm'; import { @@ -36,7 +37,9 @@ const askSeerMutationOptions = mutationOptions({ status: string; unsupported_reason: string | null; }>({ - url: '/organizations/org-slug/trace-explorer-ai/query/', + url: getApiUrl('/organizations/$organizationIdOrSlug/trace-explorer-ai/query/', { + path: {organizationIdOrSlug: 'org-slug'}, + }), method: 'POST', data: {}, }); diff --git a/static/app/components/searchQueryBuilder/index.spec.tsx b/static/app/components/searchQueryBuilder/index.spec.tsx index f6771c6393f8..8953517fe9f5 100644 --- a/static/app/components/searchQueryBuilder/index.spec.tsx +++ b/static/app/components/searchQueryBuilder/index.spec.tsx @@ -36,6 +36,7 @@ import { import {InvalidReason, WildcardOperators} from 'sentry/components/searchSyntax/parser'; import {SavedSearchType, type TagCollection} from 'sentry/types/group'; import * as analytics from 'sentry/utils/analytics'; +import {getApiUrl} from 'sentry/utils/api/getApiUrl'; import { FieldKey, FieldKind, @@ -7409,7 +7410,12 @@ describe('SearchQueryBuilder', () => { status: string; unsupported_reason: string | null; }>({ - url: '/organizations/org-slug/trace-explorer-ai/query/', + url: getApiUrl( + '/organizations/$organizationIdOrSlug/trace-explorer-ai/query/', + { + path: {organizationIdOrSlug: 'org-slug'}, + } + ), method: 'POST', data: {}, }); diff --git a/static/app/utils/api/knownGetsentryApiUrls.ts b/static/app/utils/api/knownGetsentryApiUrls.ts index eb2b6008af5d..802631be31ef 100644 --- a/static/app/utils/api/knownGetsentryApiUrls.ts +++ b/static/app/utils/api/knownGetsentryApiUrls.ts @@ -11,6 +11,7 @@ export type KnownGetsentryApiUrls = | '/_admin/cells/$region/invoice-comparison/' | '/_admin/cells/$region/queue-spike-projection-batch/' | '/_admin/customers/$organizationIdOrSlug/balance-changes/' + | '/_admin/customers/$organizationIdOrSlug/billing-platform-migration/' | '/_admin/customers/$organizationIdOrSlug/queue-spike-projection/' | '/_admin/instance-level-oauth/' | '/_admin/users/$userId/suspend/' @@ -33,6 +34,7 @@ export type KnownGetsentryApiUrls = | '/customers/$organizationIdOrSlug/billing-details/' | '/customers/$organizationIdOrSlug/billing-seats/current/' | '/customers/$organizationIdOrSlug/charges/' + | '/customers/$organizationIdOrSlug/delete-billing-metric-history/' | '/customers/$organizationIdOrSlug/history/' | '/customers/$organizationIdOrSlug/history/current/' | '/customers/$organizationIdOrSlug/invoices/' diff --git a/static/app/utils/integrationUtil.tsx b/static/app/utils/integrationUtil.tsx index 88c7b64a417a..7ee20bb0479d 100644 --- a/static/app/utils/integrationUtil.tsx +++ b/static/app/utils/integrationUtil.tsx @@ -325,23 +325,6 @@ export const getAlertText = (integrations?: Integration[]): string | undefined = } }; -/** - * Uses the mapping and baseEndpoint to derive the details for the mappings request. - * @param baseEndpoint Must have a trailing slash, since the id is appended for PUT requests! - * @param mapping The mapping or suggestion being sent to the endpoint - * @returns An object containing the request method (apiMethod), and final endpoint (apiEndpoint) - */ -export const getExternalActorEndpointDetails = ( - baseEndpoint: string, - mapping?: ExternalActorMappingOrSuggestion -): {apiEndpoint: string; apiMethod: 'POST' | 'PUT'} => { - const isValidMapping = mapping && isExternalActorMapping(mapping); - return { - apiMethod: isValidMapping ? 'PUT' : 'POST', - apiEndpoint: isValidMapping ? `${baseEndpoint}${mapping.id}/` : baseEndpoint, - }; -}; - export function getIntegrationStatus(integration: Integration) { // there are multiple status fields for an integration we consider const statusList = [integration.organizationIntegrationStatus, integration.status]; diff --git a/static/app/utils/queryClient.tsx b/static/app/utils/queryClient.tsx index 1b366a9bd88f..ed66279561ff 100644 --- a/static/app/utils/queryClient.tsx +++ b/static/app/utils/queryClient.tsx @@ -13,6 +13,7 @@ import {apiFetch} from 'sentry/utils/api/apiFetch'; import {selectJson} from 'sentry/utils/api/apiOptions'; import {normalizeQueryKey} from 'sentry/utils/api/apiQueryKey'; import type {ApiQueryKey, QueryKeyEndpointOptions} from 'sentry/utils/api/apiQueryKey'; +import type {getApiUrl} from 'sentry/utils/api/getApiUrl'; import {RequestError} from 'sentry/utils/requestError/requestError'; const nonRetryCodes = new Set([400, 401, 402, 403, 404]); @@ -155,7 +156,7 @@ export function setApiQueryData( type ApiMutationVariables = { method: 'PUT' | 'POST' | 'PATCH' | 'DELETE'; - url: string; + url: ReturnType; data?: Record; options?: Pick< QueryKeyEndpointOptions, diff --git a/static/app/utils/replays/hooks/useMarkReplayViewed.tsx b/static/app/utils/replays/hooks/useMarkReplayViewed.tsx index 3781fa2fc427..e1be174c4d62 100644 --- a/static/app/utils/replays/hooks/useMarkReplayViewed.tsx +++ b/static/app/utils/replays/hooks/useMarkReplayViewed.tsx @@ -1,5 +1,6 @@ import {useMutation, useQueryClient} from '@tanstack/react-query'; +import {getApiUrl} from 'sentry/utils/api/getApiUrl'; import {fetchMutation} from 'sentry/utils/queryClient'; import {useOrganization} from 'sentry/utils/useOrganization'; @@ -13,11 +14,29 @@ export function useMarkReplayViewed() { return useMutation({ mutationFn: ({projectSlug, replayId}) => { - const url = `/projects/${organization.slug}/${projectSlug}/replays/${replayId}/viewed-by/`; + const url = getApiUrl( + '/projects/$organizationIdOrSlug/$projectIdOrSlug/replays/$replayId/viewed-by/', + { + path: { + organizationIdOrSlug: organization.slug, + projectIdOrSlug: projectSlug, + replayId, + }, + } + ); return fetchMutation({method: 'POST', url}); }, onSuccess(_data, {projectSlug, replayId}) { - const url = `/projects/${organization.slug}/${projectSlug}/replays/${replayId}/viewed-by/`; + const url = getApiUrl( + '/projects/$organizationIdOrSlug/$projectIdOrSlug/replays/$replayId/viewed-by/', + { + path: { + organizationIdOrSlug: organization.slug, + projectIdOrSlug: projectSlug, + replayId, + }, + } + ); queryClient.refetchQueries({queryKey: [url]}); }, retry: false, diff --git a/static/app/views/issueDetails/discardIssueMutationOptions.ts b/static/app/views/issueDetails/discardIssueMutationOptions.ts index c3a8995ff4f7..776d1981ceea 100644 --- a/static/app/views/issueDetails/discardIssueMutationOptions.ts +++ b/static/app/views/issueDetails/discardIssueMutationOptions.ts @@ -4,6 +4,7 @@ import {addLoadingMessage, clearIndicators} from 'sentry/actionCreators/indicato import {t} from 'sentry/locale'; import {GroupStore} from 'sentry/stores/groupStore'; import {IssueListCacheStore} from 'sentry/stores/IssueListCacheStore'; +import {getApiUrl} from 'sentry/utils/api/getApiUrl'; import {uniqueId} from 'sentry/utils/guid'; import {fetchMutation} from 'sentry/utils/queryClient'; import type {useNavigate} from 'sentry/utils/useNavigate'; @@ -23,7 +24,12 @@ export function discardIssueMutationOptions({ mutationFn: (variables: DiscardIssueVariables) => fetchMutation({ method: 'PUT', - url: `/issues/${variables.groupId}/`, + url: getApiUrl('/organizations/$organizationIdOrSlug/issues/$issueId/', { + path: { + organizationIdOrSlug: variables.orgSlug, + issueId: String(variables.groupId), + }, + }), data: {discard: true}, }), onMutate: variables => { diff --git a/static/app/views/preprod/buildComparison/buildComparison.tsx b/static/app/views/preprod/buildComparison/buildComparison.tsx index 75c9333803c1..059149913d76 100644 --- a/static/app/views/preprod/buildComparison/buildComparison.tsx +++ b/static/app/views/preprod/buildComparison/buildComparison.tsx @@ -60,7 +60,11 @@ export default function BuildComparison() { RequestError >({ mutationFn: () => { - return fetchMutation({url: `${compareUrl}?rerun=true`, method: 'POST'}); + return fetchMutation({ + url: compareUrl, + method: 'POST', + options: {query: {rerun: 'true'}}, + }); }, onSuccess: response => { if (response?.status === 'exists') { diff --git a/static/app/views/settings/organizationApiKeys/organizationApiKeyDetails.tsx b/static/app/views/settings/organizationApiKeys/organizationApiKeyDetails.tsx index 6661d0d40ec6..55102a74f2ff 100644 --- a/static/app/views/settings/organizationApiKeys/organizationApiKeyDetails.tsx +++ b/static/app/views/settings/organizationApiKeys/organizationApiKeyDetails.tsx @@ -13,6 +13,7 @@ import {SentryDocumentTitle} from 'sentry/components/sentryDocumentTitle'; import {API_ACCESS_SCOPES} from 'sentry/constants'; import {t} from 'sentry/locale'; import {apiOptions} from 'sentry/utils/api/apiOptions'; +import {getApiUrl} from 'sentry/utils/api/getApiUrl'; import {fetchMutation} from 'sentry/utils/queryClient'; import {useNavigate} from 'sentry/utils/useNavigate'; import {useOrganization} from 'sentry/utils/useOrganization'; @@ -82,7 +83,9 @@ function OrganizationApiKeyForm({ const mutation = useMutation({ mutationFn: (data: ApiKeyFormValues) => fetchMutation({ - url: `/organizations/${organizationSlug}/api-keys/${apiKey.id}/`, + url: getApiUrl('/organizations/$organizationIdOrSlug/api-keys/$apiKeyId/', { + path: {organizationIdOrSlug: organizationSlug, apiKeyId: apiKey.id}, + }), method: 'PUT', data, }), diff --git a/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.spec.tsx b/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.spec.tsx index b2aac3f6832c..33a79da4f9e2 100644 --- a/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.spec.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.spec.tsx @@ -10,10 +10,14 @@ import { ModalFooter, } from '@sentry/scraps/modal'; +import {getApiUrl} from 'sentry/utils/api/getApiUrl'; + import {IntegrationExternalMappingForm} from './integrationExternalMappingForm'; describe('IntegrationExternalMappingForm', () => { - const membersEndpoint = '/organizations/org-slug/members/'; + const membersEndpoint = getApiUrl('/organizations/$organizationIdOrSlug/members/', { + path: {organizationIdOrSlug: 'org-slug'}, + }); const teamsEndpoint = '/organizations/org-slug/teams/'; const baseProps = { integration: GitHubIntegrationFixture(), diff --git a/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.tsx b/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.tsx index dab539e22ce6..e6372ed2f6c3 100644 --- a/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.tsx @@ -21,10 +21,8 @@ import type { } from 'sentry/types/integrations'; import type {Member, Team} from 'sentry/types/organization'; import {apiOptions} from 'sentry/utils/api/apiOptions'; -import { - getExternalActorEndpointDetails, - isExternalActorMapping, -} from 'sentry/utils/integrationUtil'; +import type {getApiUrl} from 'sentry/utils/api/getApiUrl'; +import {isExternalActorMapping} from 'sentry/utils/integrationUtil'; import {fetchMutation} from 'sentry/utils/queryClient'; import {RequestError} from 'sentry/utils/requestError/requestError'; import {requestErrorToFieldErrors} from 'sentry/utils/requestError/requestErrorToFieldErrors'; @@ -34,7 +32,9 @@ import {useOrganization} from 'sentry/utils/useOrganization'; type SentrySelection = {id: string; name: string}; type BaseProps = { - getBaseFormEndpoint: (mapping?: ExternalActorMappingOrSuggestion) => string; + getBaseFormEndpoint: ( + mapping?: ExternalActorMappingOrSuggestion + ) => ReturnType; integration: Integration; type: 'user' | 'team'; defaultOptions?: Array<{label: React.ReactNode; value: SentrySelection}>; @@ -182,14 +182,11 @@ function InlineMappingForm({ initialValue={initialValue} mutationOptions={{ mutationFn: ({sentryId}: {sentryId: SentrySelection}) => { + const isValidMapping = Object.hasOwn(mapping || {}, 'id'); const fullData = buildMutationData(mapping, integration, type, sentryId); - const {apiEndpoint, apiMethod} = getExternalActorEndpointDetails( - getBaseFormEndpoint(fullData as ExternalActorMappingOrSuggestion), - fullData as ExternalActorMappingOrSuggestion - ); return fetchMutation({ - url: apiEndpoint, - method: apiMethod, + url: getBaseFormEndpoint(mapping), + method: isValidMapping ? 'PUT' : 'POST', data: fullData, }); }, @@ -264,6 +261,7 @@ function ModalMappingForm({ externalName: string; sentryId: SentrySelection; }) => { + const isValidMapping = mapping && Object.hasOwn(mapping || {}, 'id'); const fullData = buildMutationData( mapping, integration, @@ -271,13 +269,9 @@ function ModalMappingForm({ sentryId, externalName ); - const {apiEndpoint, apiMethod} = getExternalActorEndpointDetails( - getBaseFormEndpoint(fullData as ExternalActorMappingOrSuggestion), - fullData as ExternalActorMappingOrSuggestion - ); return fetchMutation({ - url: apiEndpoint, - method: apiMethod, + url: getBaseFormEndpoint(mapping), + method: isValidMapping ? 'PUT' : 'POST', data: fullData, }); }, diff --git a/static/app/views/settings/organizationIntegrations/integrationExternalMappings.spec.tsx b/static/app/views/settings/organizationIntegrations/integrationExternalMappings.spec.tsx index 2c6ab5380401..aebbec91b584 100644 --- a/static/app/views/settings/organizationIntegrations/integrationExternalMappings.spec.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationExternalMappings.spec.tsx @@ -9,6 +9,8 @@ import { waitForElementToBeRemoved, } from 'sentry-test/reactTestingLibrary'; +import {getApiUrl} from 'sentry/utils/api/getApiUrl'; + import {IntegrationExternalMappings} from './integrationExternalMappings'; describe('IntegrationExternalMappings', () => { @@ -105,7 +107,11 @@ describe('IntegrationExternalMappings', () => { onCreate={onCreateMock} onDelete={onDeleteMock} defaultOptions={[]} - getBaseFormEndpoint={() => '/organizations/org-slug/codeowners-associations/'} + getBaseFormEndpoint={() => + getApiUrl('/organizations/$organizationIdOrSlug/codeowners-associations/', { + path: {organizationIdOrSlug: organization.slug}, + }) + } /> ); @@ -123,7 +129,11 @@ describe('IntegrationExternalMappings', () => { onCreate={onCreateMock} onDelete={onDeleteMock} defaultOptions={[]} - getBaseFormEndpoint={() => '/organizations/org-slug/codeowners-associations/'} + getBaseFormEndpoint={() => + getApiUrl('/organizations/$organizationIdOrSlug/codeowners-associations/', { + path: {organizationIdOrSlug: organization.slug}, + }) + } /> ); @@ -144,7 +154,11 @@ describe('IntegrationExternalMappings', () => { onCreate={onCreateMock} onDelete={onDeleteMock} defaultOptions={[]} - getBaseFormEndpoint={() => '/organizations/org-slug/codeowners-associations/'} + getBaseFormEndpoint={() => + getApiUrl('/organizations/$organizationIdOrSlug/codeowners-associations/', { + path: {organizationIdOrSlug: organization.slug}, + }) + } /> ); @@ -174,7 +188,11 @@ describe('IntegrationExternalMappings', () => { onCreate={onCreateMock} onDelete={onDeleteMock} defaultOptions={[]} - getBaseFormEndpoint={() => '/organizations/org-slug/codeowners-associations/'} + getBaseFormEndpoint={() => + getApiUrl('/organizations/$organizationIdOrSlug/codeowners-associations/', { + path: {organizationIdOrSlug: organization.slug}, + }) + } /> ); renderGlobalModal(); diff --git a/static/app/views/settings/organizationIntegrations/integrationExternalTeamMappings.tsx b/static/app/views/settings/organizationIntegrations/integrationExternalTeamMappings.tsx index c680a2be57e9..97eee1cfa0fc 100644 --- a/static/app/views/settings/organizationIntegrations/integrationExternalTeamMappings.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationExternalTeamMappings.tsx @@ -1,4 +1,4 @@ -import {useQuery, useMutation} from '@tanstack/react-query'; +import {skipToken, useQuery, useMutation} from '@tanstack/react-query'; import {useModal} from '@sentry/scraps/modal'; @@ -126,7 +126,9 @@ export function IntegrationExternalTeamMappings(props: Props) { const getBaseFormEndpoint = (mapping?: ExternalActorMappingOrSuggestion) => { if (!mapping) { - return ''; + return getApiUrl('/teams/$organizationIdOrSlug/$teamIdOrSlug/external-teams/', { + path: skipToken, + }); } // Search both initialResults and teams (filtered by hasExternalTeams). // Fall back to sentryName from the mutation data for teams found via search @@ -135,12 +137,24 @@ export function IntegrationExternalTeamMappings(props: Props) { initialResults?.find(item => item.id === mapping.teamId) ?? teams.find(item => item.id === mapping.teamId); const teamSlug = team?.slug ?? ('sentryName' in mapping ? mapping.sentryName : ''); - return getApiUrl('/teams/$organizationIdOrSlug/$teamIdOrSlug/external-teams/', { - path: { - organizationIdOrSlug: organization.slug, - teamIdOrSlug: teamSlug, - }, - }); + const externalTeamId = 'id' in mapping ? mapping.id : null; + return externalTeamId + ? getApiUrl( + '/teams/$organizationIdOrSlug/$teamIdOrSlug/external-teams/$externalTeamId/', + { + path: { + organizationIdOrSlug: organization.slug, + teamIdOrSlug: teamSlug, + externalTeamId, + }, + } + ) + : getApiUrl('/teams/$organizationIdOrSlug/$teamIdOrSlug/external-teams/', { + path: { + organizationIdOrSlug: organization.slug, + teamIdOrSlug: teamSlug, + }, + }); }; const onCreate = () => { diff --git a/static/app/views/unsubscribe/issue.spec.tsx b/static/app/views/unsubscribe/issue.spec.tsx index 511ea8c85417..f0197c8f3f67 100644 --- a/static/app/views/unsubscribe/issue.spec.tsx +++ b/static/app/views/unsubscribe/issue.spec.tsx @@ -8,7 +8,7 @@ describe('UnsubscribeIssue', () => { beforeEach(() => { mockUpdate = MockApiClient.addMockResponse({ - url: '/organizations/acme/unsubscribe/issue/9876/?_=signature-value', + url: '/organizations/acme/unsubscribe/issue/9876/', method: 'POST', status: 201, }); @@ -57,8 +57,11 @@ describe('UnsubscribeIssue', () => { await userEvent.click(button); expect(mockUpdate).toHaveBeenCalledWith( - '/organizations/acme/unsubscribe/issue/9876/?_=signature-value', - expect.objectContaining({data: {cancel: 1}}) + '/organizations/acme/unsubscribe/issue/9876/', + expect.objectContaining({ + data: {cancel: 1}, + query: {_: 'signature-value'}, + }) ); }); }); diff --git a/static/app/views/unsubscribe/issue.tsx b/static/app/views/unsubscribe/issue.tsx index 7e688cf020bc..3420fbe93427 100644 --- a/static/app/views/unsubscribe/issue.tsx +++ b/static/app/views/unsubscribe/issue.tsx @@ -68,7 +68,12 @@ function UnsubscribeBody({orgSlug, issueId, signature}: BodyProps) { ); const mutation = useMutation({ mutationFn: (value: {cancel: number}) => - fetchMutation({url: `${endpoint}?_=${signature}`, method: 'POST', data: value}), + fetchMutation({ + url: endpoint, + method: 'POST', + options: {query: {_: signature}}, + data: value, + }), onSuccess: () => { testableWindowLocation.assign('/auth/login/'); }, diff --git a/static/app/views/unsubscribe/project.spec.tsx b/static/app/views/unsubscribe/project.spec.tsx index ad2b76f0ee55..3dac779d7965 100644 --- a/static/app/views/unsubscribe/project.spec.tsx +++ b/static/app/views/unsubscribe/project.spec.tsx @@ -7,7 +7,7 @@ describe('UnsubscribeProject', () => { let mockGet: jest.Mock; beforeEach(() => { mockUpdate = MockApiClient.addMockResponse({ - url: '/organizations/acme/unsubscribe/project/9876/?_=signature-value', + url: '/organizations/acme/unsubscribe/project/9876/', method: 'POST', status: 201, }); @@ -60,8 +60,11 @@ describe('UnsubscribeProject', () => { await userEvent.click(button); expect(mockUpdate).toHaveBeenCalledWith( - '/organizations/acme/unsubscribe/project/9876/?_=signature-value', - expect.objectContaining({data: {cancel: 1}}) + '/organizations/acme/unsubscribe/project/9876/', + expect.objectContaining({ + data: {cancel: 1}, + query: {_: 'signature-value'}, + }) ); }); }); diff --git a/static/app/views/unsubscribe/project.tsx b/static/app/views/unsubscribe/project.tsx index 6609ec5f2f1e..bcd758766e13 100644 --- a/static/app/views/unsubscribe/project.tsx +++ b/static/app/views/unsubscribe/project.tsx @@ -68,7 +68,12 @@ function UnsubscribeBody({orgSlug, issueId, signature}: BodyProps) { ); const mutation = useMutation({ mutationFn: (value: {cancel: number}) => - fetchMutation({url: `${endpoint}?_=${signature}`, method: 'POST', data: value}), + fetchMutation({ + url: endpoint, + method: 'POST', + options: {query: {_: signature}}, + data: value, + }), onSuccess: () => { testableWindowLocation.assign('/auth/login/'); }, diff --git a/static/gsAdmin/components/deleteBillingMetricHistory.spec.tsx b/static/gsAdmin/components/deleteBillingMetricHistory.spec.tsx index 3331a4b72f52..95608bcbacaa 100644 --- a/static/gsAdmin/components/deleteBillingMetricHistory.spec.tsx +++ b/static/gsAdmin/components/deleteBillingMetricHistory.spec.tsx @@ -141,7 +141,7 @@ describe('DeleteBillingMetricHistory', () => { // Mock the API endpoint for deleting billing metric history const deleteBillingMetricHistoryMock = MockApiClient.addMockResponse({ - url: `/api/0/customers/${organization.slug}/delete-billing-metric-history/`, + url: `/customers/${organization.slug}/delete-billing-metric-history/`, method: 'POST', body: {}, }); @@ -170,7 +170,7 @@ describe('DeleteBillingMetricHistory', () => { // Check that the API call was made with the correct parameters expect(deleteBillingMetricHistoryMock).toHaveBeenCalledWith( - `/api/0/customers/${organization.slug}/delete-billing-metric-history/`, + `/customers/${organization.slug}/delete-billing-metric-history/`, expect.objectContaining({ method: 'POST', data: { @@ -219,7 +219,7 @@ describe('DeleteBillingMetricHistory', () => { // Mock the API endpoint to return an error const deleteBillingMetricHistoryMock = MockApiClient.addMockResponse({ - url: `/api/0/customers/${organization.slug}/delete-billing-metric-history/`, + url: `/customers/${organization.slug}/delete-billing-metric-history/`, method: 'POST', statusCode: 400, body: { diff --git a/static/gsAdmin/components/deleteBillingMetricHistory.tsx b/static/gsAdmin/components/deleteBillingMetricHistory.tsx index 4023f01aee31..104584d7cbae 100644 --- a/static/gsAdmin/components/deleteBillingMetricHistory.tsx +++ b/static/gsAdmin/components/deleteBillingMetricHistory.tsx @@ -13,6 +13,7 @@ import {openModal} from 'sentry/actionCreators/modal'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import type {Organization} from 'sentry/types/organization'; import {apiOptions} from 'sentry/utils/api/apiOptions'; +import {getApiUrl} from 'sentry/utils/api/getApiUrl'; import {fetchMutation} from 'sentry/utils/queryClient'; import {RequestError} from 'sentry/utils/requestError/requestError'; @@ -70,7 +71,12 @@ function DeleteBillingMetricHistoryModal({ const mutation = useMutation({ mutationFn: (dataCategory: number) => fetchMutation({ - url: `/api/0/customers/${orgSlug}/delete-billing-metric-history/`, + url: getApiUrl( + `/customers/$organizationIdOrSlug/delete-billing-metric-history/`, + { + path: {organizationIdOrSlug: orgSlug}, + } + ), method: 'POST', data: {data_category: dataCategory}, }), diff --git a/static/gsAdmin/views/customerDetails.tsx b/static/gsAdmin/views/customerDetails.tsx index 6466bceabba2..2f91879a4570 100644 --- a/static/gsAdmin/views/customerDetails.tsx +++ b/static/gsAdmin/views/customerDetails.tsx @@ -196,7 +196,12 @@ export function CustomerDetails() { const onToggleBillingPlatformMigrationMutation = useMutation({ mutationFn: (params: Record) => fetchMutation({ - url: `/_admin/customers/${orgId}/billing-platform-migration/`, + url: getApiUrl( + '/_admin/customers/$organizationIdOrSlug/billing-platform-migration/', + { + path: {organizationIdOrSlug: orgId}, + } + ), method: 'POST', data: params, }), diff --git a/static/gsApp/components/creditCardEdit/intentForms/setupIntentForm.tsx b/static/gsApp/components/creditCardEdit/intentForms/setupIntentForm.tsx index f0b2d0290221..930479d453ca 100644 --- a/static/gsApp/components/creditCardEdit/intentForms/setupIntentForm.tsx +++ b/static/gsApp/components/creditCardEdit/intentForms/setupIntentForm.tsx @@ -10,7 +10,6 @@ import {useMutation} from '@tanstack/react-query'; import {addSuccessMessage} from 'sentry/actionCreators/indicator'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {t} from 'sentry/locale'; -import {parseQueryKey} from 'sentry/utils/api/apiQueryKey'; import {getApiUrl} from 'sentry/utils/api/getApiUrl'; import {fetchMutation} from 'sentry/utils/queryClient'; @@ -29,9 +28,8 @@ export function SetupIntentForm(props: IntentFormProps) { const [errorMessage, setErrorMessage] = useState(undefined); const [isSubmitting, setIsSubmitting] = useState(false); - const {url} = parseQueryKey(props.intentDataQueryKey); const {intentData, isLoading, isError, error} = useSetupIntentData({ - endpoint: url, + queryKey: props.intentDataQueryKey, }); const {mutateAsync: updateSubscription} = useMutation({ diff --git a/static/gsApp/hooks/useIntentData.tsx b/static/gsApp/hooks/useIntentData.tsx index e2ff98f4474d..fe34ca15e32d 100644 --- a/static/gsApp/hooks/useIntentData.tsx +++ b/static/gsApp/hooks/useIntentData.tsx @@ -1,7 +1,7 @@ import {useEffect, useState} from 'react'; import {useMutation} from '@tanstack/react-query'; -import type {ApiQueryKey} from 'sentry/utils/api/apiQueryKey'; +import {parseQueryKey, type ApiQueryKey} from 'sentry/utils/api/apiQueryKey'; import {fetchMutation, useApiQuery} from 'sentry/utils/queryClient'; import type {RequestError} from 'sentry/utils/requestError/requestError'; @@ -17,17 +17,18 @@ interface HookResult { /** * Get payment method setup intent data. */ -export function useSetupIntentData({endpoint}: {endpoint: string}): HookResult { +export function useSetupIntentData({queryKey}: {queryKey: ApiQueryKey}): HookResult { const [setupIntentData, setSetupIntentData] = useState< PaymentSetupCreateResponse | undefined >(undefined); + const {url} = parseQueryKey(queryKey); const [isLoading, setIsLoading] = useState(false); const [error, setError] = useState(undefined); const {mutate: loadSetupIntentData} = useMutation< PaymentSetupCreateResponse, RequestError >({ - mutationFn: () => fetchMutation({url: endpoint, method: 'POST'}), + mutationFn: () => fetchMutation({url, method: 'POST'}), onSuccess: data => { setSetupIntentData(data); setIsLoading(false); From 0a7a35580df7b4766decbf6f715c85566389581d Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Thu, 17 Sep 2026 16:40:50 -0700 Subject: [PATCH 2/3] fix(integrations): keep the selected actor when resolving mapping endpoints `getBaseFormEndpoint` was being handed the original mapping row instead of the resolved form data, so it could not see which team or user had just been picked. The team endpoint derives its slug from that selection, so creating a mapping from the modal resolved against an unsubstituted url template, and the callback also lost the id it needs to address an existing mapping. Pass the built mutation data back in, as the endpoint callbacks expect, and give the external-users callback the same id-in-path branch the teams one already has, so editing a user mapping targets its own resource rather than the collection. --- .../integrationExternalMappingForm.spec.tsx | 11 ++++++-- .../integrationExternalMappingForm.tsx | 4 +-- .../integrationExternalUserMappings.tsx | 25 +++++++++++++------ 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.spec.tsx b/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.spec.tsx index 33a79da4f9e2..2f69011fe4f6 100644 --- a/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.spec.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.spec.tsx @@ -19,9 +19,16 @@ describe('IntegrationExternalMappingForm', () => { path: {organizationIdOrSlug: 'org-slug'}, }); const teamsEndpoint = '/organizations/org-slug/teams/'; + const memberEndpoint = (memberId: string) => + getApiUrl('/organizations/$organizationIdOrSlug/members/$memberId/', { + path: {organizationIdOrSlug: 'org-slug', memberId}, + }); const baseProps = { integration: GitHubIntegrationFixture(), - getBaseFormEndpoint: jest.fn(_mapping => membersEndpoint), + // Callers own the whole url, so an existing mapping resolves to its own resource. + getBaseFormEndpoint: jest.fn(mapping => + mapping && 'id' in mapping ? memberEndpoint(mapping.id) : membersEndpoint + ), } satisfies Partial>; const closeModal = jest.fn(); @@ -78,7 +85,7 @@ describe('IntegrationExternalMappingForm', () => { body: {}, }); putResponse = MockApiClient.addMockResponse({ - url: `${membersEndpoint}1/`, + url: memberEndpoint('1'), method: 'PUT', body: {}, }); diff --git a/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.tsx b/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.tsx index e6372ed2f6c3..1a09f3b611cb 100644 --- a/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.tsx @@ -185,7 +185,7 @@ function InlineMappingForm({ const isValidMapping = Object.hasOwn(mapping || {}, 'id'); const fullData = buildMutationData(mapping, integration, type, sentryId); return fetchMutation({ - url: getBaseFormEndpoint(mapping), + url: getBaseFormEndpoint(fullData as ExternalActorMappingOrSuggestion), method: isValidMapping ? 'PUT' : 'POST', data: fullData, }); @@ -270,7 +270,7 @@ function ModalMappingForm({ externalName ); return fetchMutation({ - url: getBaseFormEndpoint(mapping), + url: getBaseFormEndpoint(fullData as ExternalActorMappingOrSuggestion), method: isValidMapping ? 'PUT' : 'POST', data: fullData, }); diff --git a/static/app/views/settings/organizationIntegrations/integrationExternalUserMappings.tsx b/static/app/views/settings/organizationIntegrations/integrationExternalUserMappings.tsx index e119cf48b653..20ef6dfb11e9 100644 --- a/static/app/views/settings/organizationIntegrations/integrationExternalUserMappings.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationExternalUserMappings.tsx @@ -9,6 +9,7 @@ import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {t} from 'sentry/locale'; import type { ExternalActorMapping, + ExternalActorMappingOrSuggestion, ExternalUser, Integration, } from 'sentry/types/integrations'; @@ -34,12 +35,20 @@ export function IntegrationExternalUserMappings(props: Props) { const organization = useOrganization(); const location = useLocation(); - const BASE_FORM_ENDPOINT = getApiUrl( - '/organizations/$organizationIdOrSlug/external-users/', - { - path: {organizationIdOrSlug: organization.slug}, - } - ); + // An existing mapping is updated in place, so the id belongs in the path. + const getBaseFormEndpoint = (mapping?: ExternalActorMappingOrSuggestion) => { + const externalUserId = mapping && 'id' in mapping ? mapping.id : null; + return externalUserId + ? getApiUrl( + '/organizations/$organizationIdOrSlug/external-users/$externalUserId/', + { + path: {organizationIdOrSlug: organization.slug, externalUserId}, + } + ) + : getApiUrl('/organizations/$organizationIdOrSlug/external-users/', { + path: {organizationIdOrSlug: organization.slug}, + }); + }; // We paginate on this query, since we're filtering by hasExternalTeams:true const { data, @@ -141,7 +150,7 @@ export function IntegrationExternalUserMappings(props: Props) { {...modalProps} type="user" integration={integration} - getBaseFormEndpoint={() => BASE_FORM_ENDPOINT} + getBaseFormEndpoint={mapping => getBaseFormEndpoint(mapping)} defaultOptions={defaultUserOptions} onSubmitSuccess={handleSubmitSuccess} /> @@ -154,7 +163,7 @@ export function IntegrationExternalUserMappings(props: Props) { type="user" integration={integration} mappings={mappings()} - getBaseFormEndpoint={() => BASE_FORM_ENDPOINT} + getBaseFormEndpoint={mapping => getBaseFormEndpoint(mapping)} defaultOptions={defaultUserOptions} onCreate={openMembersModal} onDelete={deleteMutation.mutate} From 692d226eedf342637a8db53049e257fc630cf932 Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Fri, 18 Sep 2026 09:11:22 -0700 Subject: [PATCH 3/3] Apply batched suggestions from code review Co-authored-by: Ryan Albrecht --- .../integrationExternalUserMappings.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/static/app/views/settings/organizationIntegrations/integrationExternalUserMappings.tsx b/static/app/views/settings/organizationIntegrations/integrationExternalUserMappings.tsx index 20ef6dfb11e9..c77dc0758e6d 100644 --- a/static/app/views/settings/organizationIntegrations/integrationExternalUserMappings.tsx +++ b/static/app/views/settings/organizationIntegrations/integrationExternalUserMappings.tsx @@ -150,7 +150,7 @@ export function IntegrationExternalUserMappings(props: Props) { {...modalProps} type="user" integration={integration} - getBaseFormEndpoint={mapping => getBaseFormEndpoint(mapping)} + getBaseFormEndpoint={getBaseFormEndpoint} defaultOptions={defaultUserOptions} onSubmitSuccess={handleSubmitSuccess} /> @@ -163,7 +163,7 @@ export function IntegrationExternalUserMappings(props: Props) { type="user" integration={integration} mappings={mappings()} - getBaseFormEndpoint={mapping => getBaseFormEndpoint(mapping)} + getBaseFormEndpoint={getBaseFormEndpoint} defaultOptions={defaultUserOptions} onCreate={openMembersModal} onDelete={deleteMutation.mutate}