Skip to content

Commit f5bd813

Browse files
committed
feat(onboarding): Dual-query SCM repo selector with pagination
Switch useScmRepoSearch to a dual-query approach: - Browse (no search): uses useInfiniteApiQuery with paginate=true, auto-fetches all pages in background via useFetchAllPages. First page appears instantly. - Search (user types): uses server-side search with accessibleOnly=true for accurate results. Also adds ScmVirtualizedMenuList for rendering performance with large repo lists. Refs VDY-46
1 parent 8fc7ad5 commit f5bd813

File tree

4 files changed

+250
-26
lines changed

4 files changed

+250
-26
lines changed

static/app/views/onboarding/components/scmRepoSelector.tsx

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {useMemo} from 'react';
1+
import {useMemo, useRef} from 'react';
22

33
import {Select} from '@sentry/scraps/select';
44

@@ -9,6 +9,7 @@ import {trackAnalytics} from 'sentry/utils/analytics';
99
import {useOrganization} from 'sentry/utils/useOrganization';
1010

1111
import {ScmSearchControl} from './scmSearchControl';
12+
import {makeVirtualizedMenuList} from './scmVirtualizedMenuList';
1213
import {useScmRepoSearch} from './useScmRepoSearch';
1314
import {useScmRepoSelection} from './useScmRepoSelection';
1415

@@ -24,7 +25,10 @@ export function ScmRepoSelector({integration}: ScmRepoSelectorProps) {
2425
reposByIdentifier,
2526
dropdownItems,
2627
isFetching,
28+
isFetchingNextPage,
2729
isError,
30+
hasNextPage,
31+
fetchNextPage,
2832
debouncedSearch,
2933
setSearch,
3034
} = useScmRepoSearch(integration.id, selectedRepository);
@@ -35,6 +39,21 @@ export function ScmRepoSelector({integration}: ScmRepoSelectorProps) {
3539
reposByIdentifier,
3640
});
3741

42+
const onNearBottomRef = useRef<(() => void) | undefined>(undefined);
43+
onNearBottomRef.current = () => {
44+
if (hasNextPage && !isFetchingNextPage) {
45+
fetchNextPage();
46+
}
47+
};
48+
49+
const components = useMemo(
50+
() => ({
51+
Control: ScmSearchControl,
52+
MenuList: makeVirtualizedMenuList(onNearBottomRef),
53+
}),
54+
[]
55+
);
56+
3857
// Prepend the selected repo so the Select can always resolve and display
3958
// it, even when search results no longer include it.
4059
const options = useMemo(() => {
@@ -82,7 +101,7 @@ export function ScmRepoSelector({integration}: ScmRepoSelectorProps) {
82101
'No repositories found. Check your installation permissions to ensure your integration has access.'
83102
);
84103
}
85-
return t('Type to search repositories');
104+
return t('No repositories available');
86105
}
87106

88107
return (
@@ -96,14 +115,12 @@ export function ScmRepoSelector({integration}: ScmRepoSelectorProps) {
96115
setSearch(value);
97116
}
98117
}}
99-
// Disable client-side filtering; search is handled server-side.
100-
filterOption={() => true}
101118
noOptionsMessage={noOptionsMessage}
102-
isLoading={isFetching}
119+
isLoading={isFetching || isFetchingNextPage}
103120
isDisabled={busy}
104121
clearable
105122
searchable
106-
components={{Control: ScmSearchControl}}
123+
components={components}
107124
/>
108125
);
109126
}

static/app/views/onboarding/components/scmVirtualizedMenuList.tsx

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,25 @@
44
* causes ~1s lag with 130+ platform options containing PlatformIcon SVGs.
55
* Virtualizing limits mounted components to the visible set.
66
*
7+
* Supports prefetching via `onNearBottomRef`: when the user scrolls
8+
* within PREFETCH_THRESHOLD px of the bottom, the callback fires —
9+
* well before react-select's built-in onMenuScrollToBottom.
10+
*
711
* Stopgap until a Combobox scraps component replaces this
812
* (see #discuss-design-engineering).
913
*
10-
* Usage: <Select components={{MenuList: ScmVirtualizedMenuList}} />
14+
* Usage:
15+
* const nearBottomRef = useRef(() => fetchNextPage());
16+
* <Select components={{MenuList: makeVirtualizedMenuList(nearBottomRef)}} />
1117
*/
1218

13-
import {type Ref, useRef} from 'react';
19+
import {type MutableRefObject, type Ref, useCallback, useRef} from 'react';
1420
import {mergeRefs} from '@react-aria/utils';
1521
import {useVirtualizer} from '@tanstack/react-virtual';
1622

1723
const OPTION_HEIGHT = 36;
1824
const MAX_MENU_HEIGHT = 300;
25+
const PREFETCH_THRESHOLD = 1000;
1926

2027
interface ScmVirtualizedMenuListProps {
2128
children: React.ReactNode;
@@ -25,17 +32,43 @@ interface ScmVirtualizedMenuListProps {
2532
optionHeight?: number;
2633
}
2734

35+
/**
36+
* Creates a VirtualizedMenuList component bound to an optional near-bottom
37+
* callback ref. This avoids the need to pass custom props through react-select.
38+
*/
39+
export function makeVirtualizedMenuList(
40+
onNearBottomRef?: MutableRefObject<(() => void) | undefined>
41+
) {
42+
return function VirtualizedMenuList(props: ScmVirtualizedMenuListProps) {
43+
return <ScmVirtualizedMenuList {...props} onNearBottomRef={onNearBottomRef} />;
44+
};
45+
}
46+
2847
export function ScmVirtualizedMenuList({
2948
children,
3049
maxHeight = MAX_MENU_HEIGHT,
3150
optionHeight = OPTION_HEIGHT,
3251
innerRef,
3352
innerProps,
34-
}: ScmVirtualizedMenuListProps) {
53+
onNearBottomRef,
54+
}: ScmVirtualizedMenuListProps & {
55+
onNearBottomRef?: MutableRefObject<(() => void) | undefined>;
56+
}) {
3557
const items = Array.isArray(children) ? children : [];
3658
const scrollRef = useRef<HTMLDivElement>(null);
3759
const combinedRef = mergeRefs(scrollRef, innerRef ?? null);
3860

61+
const handleScroll = useCallback(() => {
62+
const el = scrollRef.current;
63+
if (!el || !onNearBottomRef?.current) {
64+
return;
65+
}
66+
const remaining = el.scrollHeight - el.scrollTop - el.clientHeight;
67+
if (remaining < PREFETCH_THRESHOLD) {
68+
onNearBottomRef.current();
69+
}
70+
}, [onNearBottomRef]);
71+
3972
const virtualizer = useVirtualizer({
4073
count: items.length,
4174
getScrollElement: () => scrollRef.current,
@@ -56,7 +89,12 @@ export function ScmVirtualizedMenuList({
5689
}
5790

5891
return (
59-
<div ref={combinedRef} {...innerProps} style={{maxHeight, overflowY: 'auto'}}>
92+
<div
93+
ref={combinedRef}
94+
{...innerProps}
95+
onScroll={handleScroll}
96+
style={{maxHeight, overflowY: 'auto'}}
97+
>
6098
<div style={{height: virtualizer.getTotalSize(), position: 'relative'}}>
6199
{virtualItems.map(virtualRow => (
62100
<div
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
import {OrganizationFixture} from 'sentry-fixture/organization';
2+
import {RepositoryFixture} from 'sentry-fixture/repository';
3+
4+
import {act, renderHookWithProviders, waitFor} from 'sentry-test/reactTestingLibrary';
5+
6+
import {useScmRepoSearch} from './useScmRepoSearch';
7+
8+
function makeRepos(count: number, prefix = 'org/repo') {
9+
return Array.from({length: count}, (_, i) => ({
10+
identifier: `${prefix}-${i}`,
11+
name: `${prefix.split('/')[1]}-${i}`,
12+
defaultBranch: 'main',
13+
isInstalled: false,
14+
}));
15+
}
16+
17+
describe('useScmRepoSearch', () => {
18+
const organization = OrganizationFixture();
19+
const reposUrl = `/organizations/${organization.slug}/integrations/1/repos/`;
20+
21+
afterEach(() => {
22+
MockApiClient.clearMockResponses();
23+
});
24+
25+
it('fires browse request on mount without requiring search', async () => {
26+
const browseRequest = MockApiClient.addMockResponse({
27+
url: reposUrl,
28+
body: {repos: makeRepos(3), searchable: true},
29+
match: [MockApiClient.matchQuery({accessibleOnly: true, paginate: true})],
30+
});
31+
32+
const {result} = renderHookWithProviders(() => useScmRepoSearch('1'), {
33+
organization,
34+
});
35+
36+
await waitFor(() => expect(result.current.dropdownItems).toHaveLength(3));
37+
expect(browseRequest).toHaveBeenCalled();
38+
expect(result.current.dropdownItems[0]!.value).toBe('org/repo-0');
39+
});
40+
41+
it('uses server-side search when user types', async () => {
42+
MockApiClient.addMockResponse({
43+
url: reposUrl,
44+
body: {repos: makeRepos(5), searchable: true},
45+
match: [MockApiClient.matchQuery({accessibleOnly: true, paginate: true})],
46+
});
47+
48+
const searchRequest = MockApiClient.addMockResponse({
49+
url: reposUrl,
50+
body: {
51+
repos: [{identifier: 'org/match', name: 'match', isInstalled: false}],
52+
searchable: true,
53+
},
54+
match: [MockApiClient.matchQuery({search: 'match', accessibleOnly: true})],
55+
});
56+
57+
const {result} = renderHookWithProviders(() => useScmRepoSearch('1'), {
58+
organization,
59+
});
60+
61+
// Wait for browse results first
62+
await waitFor(() => expect(result.current.dropdownItems).toHaveLength(5));
63+
64+
// Type a search query
65+
act(() => {
66+
result.current.setSearch('match');
67+
});
68+
69+
await waitFor(() => expect(searchRequest).toHaveBeenCalled());
70+
await waitFor(() => expect(result.current.dropdownItems).toHaveLength(1));
71+
expect(result.current.dropdownItems[0]!.value).toBe('org/match');
72+
});
73+
74+
it('returns to browse results when search is cleared', async () => {
75+
MockApiClient.addMockResponse({
76+
url: reposUrl,
77+
body: {repos: makeRepos(3), searchable: true},
78+
match: [MockApiClient.matchQuery({accessibleOnly: true, paginate: true})],
79+
});
80+
81+
MockApiClient.addMockResponse({
82+
url: reposUrl,
83+
body: {
84+
repos: [{identifier: 'org/x', name: 'x', isInstalled: false}],
85+
searchable: true,
86+
},
87+
match: [MockApiClient.matchQuery({search: 'x', accessibleOnly: true})],
88+
});
89+
90+
const {result} = renderHookWithProviders(() => useScmRepoSearch('1'), {
91+
organization,
92+
});
93+
94+
await waitFor(() => expect(result.current.dropdownItems).toHaveLength(3));
95+
96+
// Search
97+
act(() => {
98+
result.current.setSearch('x');
99+
});
100+
await waitFor(() => expect(result.current.dropdownItems).toHaveLength(1));
101+
102+
// Clear search -- should return to browse results
103+
act(() => {
104+
result.current.setSearch('');
105+
});
106+
await waitFor(() => expect(result.current.dropdownItems).toHaveLength(3));
107+
});
108+
109+
it('marks the selected repo as disabled in dropdown items', async () => {
110+
MockApiClient.addMockResponse({
111+
url: reposUrl,
112+
body: {
113+
repos: [
114+
{identifier: 'org/selected', name: 'selected', isInstalled: false},
115+
{identifier: 'org/other', name: 'other', isInstalled: false},
116+
],
117+
searchable: true,
118+
},
119+
match: [MockApiClient.matchQuery({accessibleOnly: true, paginate: true})],
120+
});
121+
122+
const selectedRepo = RepositoryFixture({
123+
name: 'org/selected',
124+
externalSlug: 'org/selected',
125+
});
126+
127+
const {result} = renderHookWithProviders(() => useScmRepoSearch('1', selectedRepo), {
128+
organization,
129+
});
130+
131+
await waitFor(() => expect(result.current.dropdownItems).toHaveLength(2));
132+
133+
const selectedItem = result.current.dropdownItems.find(
134+
item => item.value === 'org/selected'
135+
);
136+
const otherItem = result.current.dropdownItems.find(
137+
item => item.value === 'org/other'
138+
);
139+
expect(selectedItem!.disabled).toBe(true);
140+
expect(otherItem!.disabled).toBe(false);
141+
});
142+
});

0 commit comments

Comments
 (0)