Skip to content

Commit

Permalink
ref: improve links inside AddCodeOwnerModal (#83672)
Browse files Browse the repository at this point in the history
Link should have a trailing `/` and should not have "undefined" in any
path segment.


The way these links are constructed it's possible to have `/undefined/`
inside the url because of the optional types involved. This change makes
the types slightly better in the `*MappingLink` fields, and will return
empty string instead of `"undefined"` if needed.

The url that these links point to is:
`/settings/${organization.slug}/integrations/` which should have a
trailing slash in all cases.
If the `provider.key` or `integrationId` segments are missing then a url
of `/settings/${organization.slug}/integrations///` is perfectly fine,
and will take the user to the same page as
`/settings/${organization.slug}/integrations/`. However a url of
`/settings/${organization.slug}/integrations/undefined//` will show an
error on the screen. With the defaults added the error screen should not
be possible anymore.
  • Loading branch information
ryan953 authored and andrewshie-sentry committed Jan 22, 2025
1 parent 8595a61 commit c1fa01c
Showing 1 changed file with 4 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export default class AddCodeOwnerModal extends DeprecatedAsyncComponent<Props, S
const {Header, Body, Footer} = this.props;
const {codeownersFile, error, errorJSON, codeMappings, integrations} = this.state;
const {organization} = this.props;
const baseUrl = `/settings/${organization.slug}/integrations`;
const baseUrl = `/settings/${organization.slug}/integrations/`;

return (
<Fragment>
Expand Down Expand Up @@ -168,7 +168,7 @@ export default class AddCodeOwnerModal extends DeprecatedAsyncComponent<Props, S
{integrations.map(integration => (
<LinkButton
key={integration.id}
to={`${baseUrl}/${integration.provider.key}/${integration.id}/?tab=codeMappings&referrer=add-codeowners`}
to={`${baseUrl}${integration.provider.key}/${integration.id}/?tab=codeMappings&referrer=add-codeowners`}
>
{getIntegrationIcon(integration.provider.key)}
<IntegrationName>{integration.name}</IntegrationName>
Expand Down Expand Up @@ -292,7 +292,6 @@ function ErrorMessage({
errorJSON: {raw?: string} | null;
}) {
const codeMapping = codeMappings.find(mapping => mapping.id === codeMappingId);
const {integrationId, provider} = codeMapping as RepositoryProjectPathConfig;
const errActors = errorJSON?.raw?.[0]!.split('\n').map((el, i) => <p key={i}>{el}</p>);
return (
<Alert type="error" showIcon>
Expand All @@ -304,12 +303,12 @@ function ErrorMessage({
{
userMappingsLink: (
<Link
to={`${baseUrl}/${provider?.key}/${integrationId}/?tab=userMappings&referrer=add-codeowners`}
to={`${baseUrl}${codeMapping.provider?.key ?? ''}/${codeMapping.integrationId ?? ''}/?tab=userMappings&referrer=add-codeowners`}
/>
),
teamMappingsLink: (
<Link
to={`${baseUrl}/${provider?.key}/${integrationId}/?tab=teamMappings&referrer=add-codeowners`}
to={`${baseUrl}${codeMapping.provider?.key ?? ''}/${codeMapping.integrationId ?? ''}/?tab=teamMappings&referrer=add-codeowners`}
/>
),
}
Expand Down

0 comments on commit c1fa01c

Please sign in to comment.