Skip to content

feat(settings): Migrate configure integration from deprecatedRouteProps #103128

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 1 commit into from
Nov 12, 2025
+30 −21
Merged

feat(settings): Migrate configure integration from deprecatedRouteProps #103128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion static/app/router/routes.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,6 @@ function buildRoutes(): RouteObject[] {
'sentry/views/settings/organizationIntegrations/configureIntegration'
)
),
deprecatedRouteProps: true,
},
],
},
Expand Down
2 changes: 2 additions & 0 deletions static/app/utils/useParams.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@ type ParamKeys =
| 'groupId'
| 'id'
| 'installationId'
| 'integrationId'
| 'integrationSlug'
| 'issueId'
| 'memberId'
| 'notificationSource'
| 'orgId'
| 'projectId'
| 'providerKey'
| 'regionName'
| 'release'
| 'relocationUuid'
Expand Down
18 changes: 10 additions & 8 deletions static/app/views/settings/organizationIntegrations/configureIntegration.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {OpsgenieIntegrationFixture} from 'sentry-fixture/opsgenieIntegration';
import {OpsgenieIntegrationProviderFixture} from 'sentry-fixture/opsgenieIntegrationProvider';
import {OrganizationFixture} from 'sentry-fixture/organization';
import {RouteComponentPropsFixture} from 'sentry-fixture/routeComponentPropsFixture';

import {
render,
Expand Down Expand Up @@ -56,13 +55,16 @@ describe('OpsgenieMigrationButton', () => {
method: 'PUT',
});

render(
<ConfigureIntegration
{...RouteComponentPropsFixture()}
params={{integrationId, providerKey: 'opsgenie'}}
/>,
{organization: org}
);
render(<ConfigureIntegration />, {
organization: org,
initialRouterConfig: {
location: {
pathname: `/settings/${org.slug}/integrations/opsgenie/${integrationId}/`,
query: {},
},
route: '/settings/:orgId/integrations/:providerKey/:integrationId/',
},
});
renderGlobalModal();
expect(await screen.findByRole('button', {name: 'Migrate Plugin'})).toBeEnabled();

Expand Down
30 changes: 18 additions & 12 deletions static/app/views/settings/organizationIntegrations/configureIntegration.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,22 @@ import type {
OrganizationIntegration,
PluginWithProjectList,
} from 'sentry/types/integrations';
import type {RouteComponentProps} from 'sentry/types/legacyReactRouter';
import type {Organization} from 'sentry/types/organization';
import {isActiveSuperuser} from 'sentry/utils/isActiveSuperuser';
import {singleLineRenderer} from 'sentry/utils/marked/marked';
import type {ApiQueryKey} from 'sentry/utils/queryClient';
import {setApiQueryData, useApiQuery, useQueryClient} from 'sentry/utils/queryClient';
import {decodeScalar} from 'sentry/utils/queryString';
import useRouteAnalyticsEventNames from 'sentry/utils/routeAnalytics/useRouteAnalyticsEventNames';
import useRouteAnalyticsParams from 'sentry/utils/routeAnalytics/useRouteAnalyticsParams';
import normalizeUrl from 'sentry/utils/url/normalizeUrl';
import useApi from 'sentry/utils/useApi';
import {useLocation} from 'sentry/utils/useLocation';
import {useNavigate} from 'sentry/utils/useNavigate';
import useOrganization from 'sentry/utils/useOrganization';
import {useParams} from 'sentry/utils/useParams';
import useProjects from 'sentry/utils/useProjects';
import {useRoutes} from 'sentry/utils/useRoutes';
import BreadcrumbTitle from 'sentry/views/settings/components/settingsBreadcrumb/breadcrumbTitle';
import SettingsPageHeader from 'sentry/views/settings/components/settingsPageHeader';

Expand All @@ -48,11 +52,6 @@ import IntegrationMainSettings from './integrationMainSettings';
import IntegrationRepos from './integrationRepos';
import {IntegrationServerlessFunctions} from './integrationServerlessFunctions';

type Props = RouteComponentProps<{
integrationId: string;
providerKey: string;
}>;

const TABS = [
'repos',
'codeMappings',
Expand All @@ -73,12 +72,19 @@ const makePluginQuery = (organization: Organization): ApiQueryKey => {
return [`/organizations/${organization.slug}/plugins/configs/`];
};

function ConfigureIntegration({params, router, routes, location}: Props) {
function ConfigureIntegration() {
const routes = useRoutes();
const location = useLocation();
const navigate = useNavigate();
const api = useApi();
const queryClient = useQueryClient();
const organization = useOrganization();
const tab: Tab = TABS.includes(location.query.tab) ? location.query.tab : 'repos';
const {integrationId, providerKey} = params;
const tabParam = decodeScalar(location.query.tab) as Tab | undefined;
const tab: Tab = tabParam && TABS.includes(tabParam) ? tabParam : 'repos';
const {integrationId, providerKey} = useParams<{
integrationId: string;
Comment on lines +82 to +85
Copy link

@sentry sentry bot Nov 12, 2025

Choose a reason for hiding this comment

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

The location.query.tab is being read and type-cast to Tab | undefined. However, after the cast, it's being validated against TABS.includes(tabParam). Consider using proper type narrowing to avoid potential issues. Currently, if decodeScalar returns undefined, the condition will short-circuit correctly, but making this more explicit would improve code clarity.
Severity: MEDIUM

💡 Suggested Fix

Suggested change
const tabParam = decodeScalar(location.query.tab) as Tab | undefined;
const tab: Tab = tabParam && TABS.includes(tabParam) ? tabParam : 'repos';
const {integrationId, providerKey} = useParams<{
integrationId: string;
const tabParam = (decodeScalar(location.query.tab) ?? '') as Tab | undefined;
const tab: Tab = (tabParam && TABS.includes(tabParam as Tab)) ? (tabParam as Tab) : 'repos';

Did we get this right? 👍 / 👎 to inform future reviews.

Reference_id: 2624811

providerKey: string;
}>();
const {
data: config = {providers: []},
isPending: isLoadingConfig,
Expand Down Expand Up @@ -133,13 +139,13 @@ function ConfigureIntegration({params, router, routes, location}: Props) {
!organization.access.includes('org:integrations') &&
!isActiveSuperuser()
) {
router.push(
navigate(
normalizeUrl({
pathname: `/settings/${organization.slug}/integrations/${providerKey}/`,
})
);
}
}, [router, organization, providerKey]);
}, [navigate, organization, providerKey]);

Comment on lines 145 to 149
Copy link

@sentry sentry bot Nov 12, 2025

Choose a reason for hiding this comment

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

The component now uses navigate() from useNavigate() hook instead of router.push(). Ensure that all navigation calls are using the correct object format. At lines 145-149 and 164-167, navigate() is being called with location objects containing pathname and query properties. Verify that the navigate() function correctly handles the query property (as it's a React Router v6 pattern, which typically uses search parameters instead). This might need adjustment depending on how the router handles query strings.
Severity: MEDIUM

🤖 Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
static/app/views/settings/organizationIntegrations/configureIntegration.tsx#L145-L149

Potential issue: The component now uses `navigate()` from `useNavigate()` hook instead
of `router.push()`. Ensure that all navigation calls are using the correct object
format. At lines 145-149 and 164-167, `navigate()` is being called with location objects
containing `pathname` and `query` properties. Verify that the `navigate()` function
correctly handles the `query` property (as it's a React Router v6 pattern, which
typically uses search parameters instead). This might need adjustment depending on how
the router handles query strings.

Did we get this right? 👍 / 👎 to inform future reviews.

Reference_id: 2624811

if (isLoadingConfig || isLoadingIntegration || isLoadingPlugins) {
return <LoadingIndicator />;
Expand All @@ -156,7 +162,7 @@ function ConfigureIntegration({params, router, routes, location}: Props) {
const onTabChange = (value: Tab) => {
// XXX: Omit the cursor to prevent paginating the next tab's queries.
const {cursor: _, ...query} = location.query;
router.push({
navigate({
pathname: location.pathname,
query: {...query, tab: value},
});
Expand Down
Loading