diff --git a/static/app/views/onboarding/components/useScmRepoSelection.spec.tsx b/static/app/views/onboarding/components/useScmRepoSelection.spec.tsx
index edd6aa20cfba24..f8e827339c66a0 100644
--- a/static/app/views/onboarding/components/useScmRepoSelection.spec.tsx
+++ b/static/app/views/onboarding/components/useScmRepoSelection.spec.tsx
@@ -1,27 +1,12 @@
import {OrganizationFixture} from 'sentry-fixture/organization';
import {OrganizationIntegrationsFixture} from 'sentry-fixture/organizationIntegrations';
-import {act, renderHookWithProviders, waitFor} from 'sentry-test/reactTestingLibrary';
+import {act, renderHookWithProviders} from 'sentry-test/reactTestingLibrary';
-import {
- OnboardingContextProvider,
- type OnboardingSessionState,
-} from 'sentry/components/onboarding/onboardingContext';
import type {IntegrationRepository} from 'sentry/types/integrations';
-import {sessionStorageWrapper} from 'sentry/utils/sessionStorage';
import {useScmRepoSelection} from './useScmRepoSelection';
-function makeOnboardingWrapper(initialState?: OnboardingSessionState) {
- return function OnboardingWrapper({children}: {children?: React.ReactNode}) {
- return (
-
- {children}
-
- );
- };
-}
-
describe('useScmRepoSelection', () => {
const organization = OrganizationFixture();
@@ -49,29 +34,56 @@ describe('useScmRepoSelection', () => {
isInstalled: false,
};
- const mockInstalledRepo: IntegrationRepository = {
- identifier: 'getsentry/sentry',
- name: 'sentry',
- isInstalled: true,
- };
-
beforeEach(() => {
- sessionStorageWrapper.clear();
onSelect = jest.fn();
reposByIdentifier = new Map([['getsentry/sentry', mockRepo]]);
+ });
- // Default: no existing repos
+ afterEach(() => {
+ MockApiClient.clearMockResponses();
+ });
+
+ it('uses existing repo when GET finds it, skips POST', async () => {
MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/repos/`,
- body: [],
+ body: [
+ {
+ id: '99',
+ name: 'getsentry/sentry',
+ externalSlug: 'getsentry/sentry',
+ status: 'active',
+ },
+ ],
});
- });
- afterEach(() => {
- MockApiClient.clearMockResponses();
+ const addRequest = MockApiClient.addMockResponse({
+ url: `/organizations/${organization.slug}/repos/`,
+ method: 'POST',
+ body: {},
+ });
+
+ const {result} = renderHookWithProviders(
+ () =>
+ useScmRepoSelection({integration: mockIntegration, onSelect, reposByIdentifier}),
+ {organization}
+ );
+
+ await act(async () => {
+ await result.current.handleSelect({value: 'getsentry/sentry'});
+ });
+
+ expect(addRequest).not.toHaveBeenCalled();
+ expect(onSelect).toHaveBeenCalledWith(
+ expect.objectContaining({id: '99', name: 'getsentry/sentry'})
+ );
});
- it('calls POST to add new repo and updates onSelect with server ID', async () => {
+ it('creates repo via POST when GET finds nothing', async () => {
+ MockApiClient.addMockResponse({
+ url: `/organizations/${organization.slug}/repos/`,
+ body: [],
+ });
+
const addRequest = MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/repos/`,
method: 'POST',
@@ -86,10 +98,7 @@ describe('useScmRepoSelection', () => {
const {result} = renderHookWithProviders(
() =>
useScmRepoSelection({integration: mockIntegration, onSelect, reposByIdentifier}),
- {
- organization,
- additionalWrapper: makeOnboardingWrapper({}),
- }
+ {organization}
);
await act(async () => {
@@ -101,61 +110,45 @@ describe('useScmRepoSelection', () => {
expect(onSelect).toHaveBeenCalledWith(
expect.objectContaining({id: '', name: 'sentry'})
);
- // Then real call with server response spread over optimistic
+ // Then real call with server response
expect(onSelect).toHaveBeenCalledWith(
expect.objectContaining({id: '42', name: 'getsentry/sentry'})
);
});
- it('does not POST for already-installed repos, uses existing repo ID', async () => {
- reposByIdentifier = new Map([['getsentry/sentry', mockInstalledRepo]]);
-
- // Override repos response with an existing repo
- MockApiClient.clearMockResponses();
+ it('reverts onSelect on POST failure', async () => {
MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/repos/`,
- body: [
- {
- id: '99',
- name: 'getsentry/sentry',
- externalSlug: 'getsentry/sentry',
- status: 'active',
- },
- ],
+ body: [],
});
- const addRequest = MockApiClient.addMockResponse({
+ MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/repos/`,
method: 'POST',
- body: {},
+ statusCode: 500,
+ body: {detail: 'Internal Error'},
});
const {result} = renderHookWithProviders(
() =>
useScmRepoSelection({integration: mockIntegration, onSelect, reposByIdentifier}),
- {
- organization,
- additionalWrapper: makeOnboardingWrapper({}),
- }
+ {organization}
);
- // Wait for existing repos query to resolve
- await waitFor(() => expect(result.current.busy).toBe(false));
-
await act(async () => {
await result.current.handleSelect({value: 'getsentry/sentry'});
});
- expect(addRequest).not.toHaveBeenCalled();
+ // Optimistic, then revert
expect(onSelect).toHaveBeenCalledWith(
- expect.objectContaining({id: '99', name: 'getsentry/sentry'})
+ expect.objectContaining({id: '', name: 'sentry'})
);
+ expect(onSelect).toHaveBeenCalledWith(undefined);
});
- it('reverts onSelect on addRepository failure', async () => {
+ it('reverts onSelect on GET failure', async () => {
MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/repos/`,
- method: 'POST',
statusCode: 500,
body: {detail: 'Internal Error'},
});
@@ -163,109 +156,58 @@ describe('useScmRepoSelection', () => {
const {result} = renderHookWithProviders(
() =>
useScmRepoSelection({integration: mockIntegration, onSelect, reposByIdentifier}),
- {
- organization,
- additionalWrapper: makeOnboardingWrapper({}),
- }
+ {organization}
);
await act(async () => {
await result.current.handleSelect({value: 'getsentry/sentry'});
});
- // Optimistic, then revert
expect(onSelect).toHaveBeenCalledWith(
expect.objectContaining({id: '', name: 'sentry'})
);
expect(onSelect).toHaveBeenCalledWith(undefined);
});
- it('cleans up previously added repo when selecting a new one', async () => {
- MockApiClient.addMockResponse({
- url: `/organizations/${organization.slug}/repos/`,
- method: 'POST',
- body: {
- id: '42',
- name: 'getsentry/sentry',
- externalSlug: 'getsentry/sentry',
- status: 'active',
- },
- });
-
- const hideRequest = MockApiClient.addMockResponse({
- url: `/organizations/${organization.slug}/repos/42/`,
- method: 'PUT',
- body: {},
- });
-
+ it('handleRemove calls onSelect with undefined', () => {
const {result} = renderHookWithProviders(
() =>
useScmRepoSelection({integration: mockIntegration, onSelect, reposByIdentifier}),
- {
- organization,
- additionalWrapper: makeOnboardingWrapper({}),
- }
+ {organization}
);
- // First selection
- await act(async () => {
- await result.current.handleSelect({value: 'getsentry/sentry'});
+ act(() => {
+ result.current.handleRemove();
});
- // Add a second repo
- const secondRepo: IntegrationRepository = {
- identifier: 'getsentry/relay',
- name: 'relay',
- isInstalled: false,
- };
- reposByIdentifier.set('getsentry/relay', secondRepo);
+ expect(onSelect).toHaveBeenCalledWith(undefined);
+ });
+ it('clears busy state after selection completes', async () => {
MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/repos/`,
- method: 'POST',
- body: {
- id: '43',
- name: 'getsentry/relay',
- externalSlug: 'getsentry/relay',
- status: 'active',
- },
- });
-
- // Second selection should clean up the first
- await act(async () => {
- await result.current.handleSelect({value: 'getsentry/relay'});
- });
-
- expect(hideRequest).toHaveBeenCalled();
- });
-
- it('does not call hideRepository on remove if repo was pre-existing', async () => {
- const hideRequest = MockApiClient.addMockResponse({
- url: `/organizations/${organization.slug}/repos/99/`,
- method: 'PUT',
- body: {},
+ body: [
+ {
+ id: '99',
+ name: 'getsentry/sentry',
+ externalSlug: 'getsentry/sentry',
+ status: 'active',
+ },
+ ],
});
const {result} = renderHookWithProviders(
() =>
useScmRepoSelection({integration: mockIntegration, onSelect, reposByIdentifier}),
- {
- organization,
- additionalWrapper: makeOnboardingWrapper({
- selectedRepository: {
- id: '99',
- name: 'sentry',
- externalSlug: 'getsentry/sentry',
- } as any,
- }),
- }
+ {organization}
);
+ expect(result.current.busy).toBe(false);
+
await act(async () => {
- await result.current.handleRemove();
+ await result.current.handleSelect({value: 'getsentry/sentry'});
});
- expect(onSelect).toHaveBeenCalledWith(undefined);
- expect(hideRequest).not.toHaveBeenCalled();
+ expect(result.current.busy).toBe(false);
});
});
diff --git a/static/app/views/onboarding/components/useScmRepoSelection.ts b/static/app/views/onboarding/components/useScmRepoSelection.ts
index 807ddf295deee7..080661b063d958 100644
--- a/static/app/views/onboarding/components/useScmRepoSelection.ts
+++ b/static/app/views/onboarding/components/useScmRepoSelection.ts
@@ -1,6 +1,8 @@
-import {useMemo, useRef, useState} from 'react';
+import {useState} from 'react';
+import * as Sentry from '@sentry/react';
-import {useOnboardingContext} from 'sentry/components/onboarding/onboardingContext';
+import {addErrorMessage} from 'sentry/actionCreators/indicator';
+import {t} from 'sentry/locale';
import type {
Integration,
IntegrationRepository,
@@ -8,7 +10,8 @@ import type {
} from 'sentry/types/integrations';
import {RepositoryStatus} from 'sentry/types/integrations';
import {getApiUrl} from 'sentry/utils/api/getApiUrl';
-import {fetchMutation, useApiQuery} from 'sentry/utils/queryClient';
+import type {ApiQueryKey} from 'sentry/utils/queryClient';
+import {fetchDataQuery, fetchMutation, useQueryClient} from 'sentry/utils/queryClient';
import {useOrganization} from 'sentry/utils/useOrganization';
interface UseScmRepoSelectionOptions {
@@ -43,67 +46,55 @@ export function useScmRepoSelection({
reposByIdentifier,
}: UseScmRepoSelectionOptions) {
const organization = useOrganization();
- const {selectedRepository} = useOnboardingContext();
+ const queryClient = useQueryClient();
const [busy, setBusy] = useState(false);
- // Fetch repos already registered in Sentry for this integration, so we
- // can look up the real Repository (with Sentry ID) for "Already Added" repos.
- const {data: existingRepos, isPending: existingReposPending} = useApiQuery<
- Repository[]
- >(
- [
- getApiUrl('/organizations/$organizationIdOrSlug/repos/', {
- path: {organizationIdOrSlug: organization.slug},
- }),
- {query: {status: 'active', integration_id: integration.id}},
- ],
- {staleTime: 0}
- );
-
- const existingReposBySlug = useMemo(
- () => new Map((existingRepos ?? []).map(r => [r.externalSlug, r])),
- [existingRepos]
- );
-
- // Track the ID of a repo we added during this session so we can clean
- // it up if the user switches to a different repo.
- const addedRepoIdRef = useRef(null);
-
- // Best-effort cleanup: fire-and-forget the hide request. If it fails the
- // repo stays registered in Sentry but is non-critical for the onboarding flow.
- const cleanupPreviousAdd = () => {
- if (addedRepoIdRef.current) {
- fetchMutation({
- url: `/organizations/${organization.slug}/repos/${addedRepoIdRef.current}/`,
- method: 'PUT',
- data: {status: 'hidden'},
- }).catch(() => {});
- addedRepoIdRef.current = null;
- }
- };
-
const handleSelect = async (selection: {value: string}) => {
const repo = reposByIdentifier.get(selection.value);
if (!repo) {
return;
}
- cleanupPreviousAdd();
-
const optimistic = buildOptimisticRepo(repo, integration);
onSelect(optimistic);
- if (repo.isInstalled) {
- const existing = existingReposBySlug.get(repo.identifier);
+ // Look up the repo in Sentry. The background link_all_repos task
+ // registers all repos after integration install, so most repos will
+ // already exist. Use a targeted query filtered by name to avoid
+ // pagination issues with the full list.
+ setBusy(true);
+ try {
+ const queryKey: ApiQueryKey = [
+ getApiUrl('/organizations/$organizationIdOrSlug/repos/', {
+ path: {organizationIdOrSlug: organization.slug},
+ }),
+ {
+ query: {
+ status: 'active',
+ integration_id: integration.id,
+ query: repo.identifier,
+ },
+ },
+ ];
+ const [matches] = await queryClient.fetchQuery({
+ queryKey,
+ queryFn: fetchDataQuery,
+ staleTime: 0,
+ });
+ // The query param above is an icontains filter to narrow results
+ // and avoid pagination. The exact match here uses Repository.name
+ // against IntegrationRepository.identifier — the same comparison the
+ // backend uses in organization_integration_repos.py:61,80 to determine
+ // isInstalled. Can't use externalSlug because it varies by provider
+ // (e.g. GitLab returns a numeric project ID).
+ const existing = matches?.find(r => r.name === repo.identifier);
+
if (existing) {
onSelect({...optimistic, ...existing});
return;
}
- // Lookup missed (e.g., repo was hidden). Fall through to re-add it.
- }
- setBusy(true);
- try {
+ // Repo not yet registered (link_all_repos may still be running).
const created = await fetchMutation({
url: `/organizations/${organization.slug}/repos/`,
method: 'POST',
@@ -114,43 +105,21 @@ export function useScmRepoSelection({
},
});
onSelect({...optimistic, ...created});
- addedRepoIdRef.current = created.id;
- } catch {
+ } catch (error) {
+ Sentry.captureException(error);
+ addErrorMessage(t('Failed to select repository'));
onSelect(undefined);
} finally {
setBusy(false);
}
};
- const handleRemove = async () => {
- if (!selectedRepository) {
- return;
- }
-
- const previous = selectedRepository;
+ const handleRemove = () => {
onSelect(undefined);
-
- if (addedRepoIdRef.current && addedRepoIdRef.current === previous.id) {
- setBusy(true);
- try {
- await fetchMutation({
- url: `/organizations/${organization.slug}/repos/${previous.id}/`,
- method: 'PUT',
- data: {status: 'hidden'},
- });
- addedRepoIdRef.current = null;
- } catch {
- onSelect(previous);
- } finally {
- setBusy(false);
- }
- }
};
return {
- // Busy while adding/removing a repo or while existing repos are still
- // loading. The UI disables the Select and remove button when true.
- busy: busy || existingReposPending,
+ busy,
handleSelect,
handleRemove,
};