Skip to content

ref(api): type fetchMutation urls with getApiUrl - #124862

Merged
ryan953 merged 3 commits into
masterfrom
ryan953/fetchMutation-ApiUrl
Sep 18, 2026
Merged

ryan953 merged 3 commits into
masterfrom
ryan953/fetchMutation-ApiUrl

Conversation

@ryan953

@ryan953 ryan953 commented Sep 17, 2026

Copy link
Copy Markdown
Member

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.
This narrows the field to the branded ApiUrl that getApiUrl() returns and
converts the remaining call sites, which turns a typo or a renamed route into a
type error instead of a 404 at runtime.

The brand is what does the work. It is not assignable from string, so the
compiler finds every call site that still interpolates — including the two that
carried a query parameter inside the path (?_=<signature> on the unsubscribe
pages, ?rerun=true on the preprod build comparison). Those move to
options.query, since appending to a branded url widens it back to string and
no longer compiles.

getExternalActorEndpointDetails is deleted. It took a collection endpoint and
appended the mapping id for PUT, which only works on an opaque string. Each
getBaseFormEndpoint now returns the finished url for its own resource and the
form only chooses the method.

Two entries are added to knownGetsentryApiUrls for the admin
billing-platform-migration and delete-billing-metric-history endpoints, which had
no typed path yet. The latter had also been requesting /api/0/customers/...,
which double-prefixes the client's own /api/0 base; it now uses the same
unprefixed path as every other admin view.

No feature flags are involved — this is a type change plus the call sites it
forces.

Review focus

The external actor mapping forms are where behaviour can actually move, because
url construction shifted out of the shared helper and into each caller. The
second commit is the interesting one: getBaseFormEndpoint must be called with
the resolved mutation data, not the original mapping row, since the team
endpoint derives its slug from whichever team was just selected and the callback
needs the id to address an existing mapping. Worth confirming that reading — the
existing suite covers both the create-via-modal and inline-edit paths.

IntegrationExternalUserMappings had no id-in-path branch at all, so it gained
the one the teams side already had. There is no spec file for that view, so that
particular path is covered by reading rather than by a test.

`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 - `?_=<signature>`
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.
…points

`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.
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Sep 17, 2026
@github-actions

github-actions Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

📊 Type Coverage Diff

Metric Before After Delta
Coverage 95.55% 95.55% ±0%
Typed 140,055 140,052 🔴 -3
Untyped 6,519 6,521 🔴 +2
🔍 2 new type safety issues introduced

Type assertions (as) (2 new)

File Line Detail
static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.tsx 188 as ExternalActorMappingOrSuggestionfullData as ExternalActorMappingOrSuggestion
static/app/views/settings/organizationIntegrations/integrationExternalMappingForm.tsx 273 as ExternalActorMappingOrSuggestionfullData as ExternalActorMappingOrSuggestion

This is informational only and does not block the PR.

Comment on lines -32 to 35
const {url} = parseQueryKey(props.intentDataQueryKey);
const {intentData, isLoading, isError, error} = useSetupIntentData({
endpoint: url,
queryKey: props.intentDataQueryKey,
});

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Parsing the queryKey moves into the useIntentData hook, so we can use ApiQueryKey as the prop type instead of the old string.

Co-authored-by: Ryan Albrecht <ryan@ryanalbrecht.ca>
type ApiMutationVariables = {
method: 'PUT' | 'POST' | 'PATCH' | 'DELETE';
url: string;
url: ReturnType<typeof getApiUrl>;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Here's where the type is enforced going forward!

Comment on lines -186 to 194
const {apiEndpoint, apiMethod} = getExternalActorEndpointDetails(
getBaseFormEndpoint(fullData as ExternalActorMappingOrSuggestion),
fullData as ExternalActorMappingOrSuggestion
);
return fetchMutation<ExternalActorMapping>({
url: apiEndpoint,
method: apiMethod,
url: getBaseFormEndpoint(fullData as ExternalActorMappingOrSuggestion),
method: isValidMapping ? 'PUT' : 'POST',
data: fullData,
});

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is the place with the 'most' change in the PR.

Before we were accepting mapping and getBaseFormEndpoint() and calling them ourselves. Then we'd call getExternalActorEndpointDetails() which could potentially append an id to the end of the url.

All that gets streamlined. Now it's the job of each getBaseFormEndpoint() function to return a correct url, with or without the id appended. Therefore we don't need getExternalActorEndpointDetails anymore either.

there's a only two real files that implement getBaseFormEndpoint. They each accept the mapping parameter and append an id if it's passed in.

@ryan953
ryan953 marked this pull request as ready for review September 18, 2026 16:20
@ryan953
ryan953 requested review from a team as code owners September 18, 2026 16:20
@ryan953
ryan953 merged commit 390463b into master Sep 18, 2026
80 checks passed
@ryan953
ryan953 deleted the ryan953/fetchMutation-ApiUrl branch September 18, 2026 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants