Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 0 deletions apps/web/src/components/auto-fix/AutoFixConfigForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ export function AutoFixConfigForm({ organizationId }: AutoFixConfigFormProps) {
name: repo.name,
full_name: repo.fullName,
private: repo.private,
fork: repo.fork,
})) as Repository[]
}
selectedIds={selectedRepositoryIds}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ export function AutoTriageConfigForm({ organizationId }: AutoTriageConfigFormPro
name: repo.name,
full_name: repo.fullName,
private: repo.private,
fork: repo.fork,
})) as Repository[]
}
selectedIds={selectedRepositoryIds}
Expand Down
58 changes: 50 additions & 8 deletions apps/web/src/components/code-reviews/RepositoryMultiSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ import { Input } from '@/components/ui/input';
import { Button } from '@/components/ui/button';
import { Lock, Unlock, Search } from 'lucide-react';
import { cn } from '@/lib/utils';
import { useLocalStorage } from '@/hooks/useLocalStorage';
import {
visibleRepositories,
withVisibleSelected,
withoutVisibleSelected,
} from './repository-multi-select-selection';

export type RepositoryId = string | number;

Expand All @@ -14,6 +20,7 @@ export type Repository<TId extends RepositoryId = number> = {
name: string;
full_name: string;
private: boolean;
fork?: boolean;
};

export type RepositoryMultiSelectProps<TId extends RepositoryId = number> = {
Expand All @@ -30,13 +37,22 @@ export function RepositoryMultiSelect<TId extends RepositoryId = number>({
renderRepositoryAccessory,
}: RepositoryMultiSelectProps<TId>) {
const [searchQuery, setSearchQuery] = useState('');
const [hideForks, setHideForks] = useLocalStorage<boolean>('repo-picker:hide-forks', false, {
initializeWithValue: false,
});

const forkCount = useMemo(() => repositories.filter(repo => repo.fork).length, [repositories]);
const visible = useMemo(
() => visibleRepositories(repositories, hideForks),
[repositories, hideForks]
);

const filteredRepositories = useMemo(() => {
if (!searchQuery.trim()) return repositories;
if (!searchQuery.trim()) return visible;

const query = searchQuery.toLowerCase();
return repositories.filter(repo => repo.full_name.toLowerCase().includes(query));
}, [repositories, searchQuery]);
return visible.filter(repo => repo.full_name.toLowerCase().includes(query));
}, [visible, searchQuery]);

const handleToggle = (repoId: TId) => {
const newSelection = selectedIds.includes(repoId)
Expand All @@ -47,15 +63,26 @@ export function RepositoryMultiSelect<TId extends RepositoryId = number>({
};

const handleSelectAll = () => {
onSelectionChange(repositories.map(repo => repo.id));
onSelectionChange(
withVisibleSelected(
selectedIds,
visible.map(repo => repo.id)
)
);
};

const handleDeselectAll = () => {
onSelectionChange([]);
onSelectionChange(
withoutVisibleSelected(
selectedIds,
visible.map(repo => repo.id)
)
);
};

const isAllSelected = selectedIds.length === repositories.length && repositories.length > 0;
const isNoneSelected = selectedIds.length === 0;
const visibleSelectedCount = visible.filter(repo => selectedIds.includes(repo.id)).length;
const isAllSelected = visible.length > 0 && visibleSelectedCount === visible.length;
const isNoneSelected = visibleSelectedCount === 0;

return (
<div className="space-y-3">
Expand Down Expand Up @@ -91,6 +118,18 @@ export function RepositoryMultiSelect<TId extends RepositoryId = number>({
>
Deselect All
</Button>
{forkCount > 0 && (
<div className="ml-auto flex items-center gap-2">
<Checkbox
id="hide-forks"
checked={hideForks}
onCheckedChange={checked => setHideForks(checked === true)}
/>
<label htmlFor="hide-forks" className="text-muted-foreground cursor-pointer text-xs">
Hide forks
</label>
</div>
)}
</div>

<div className="border-border bg-background h-64 overflow-y-auto rounded-md border">
Expand Down Expand Up @@ -136,7 +175,10 @@ export function RepositoryMultiSelect<TId extends RepositoryId = number>({
</div>

<div className="text-muted-foreground text-xs">
{selectedIds.length} of {repositories.length} repositories selected
{visibleSelectedCount} of {visible.length} repositories selected
{hideForks && forkCount > 0
? ` · ${forkCount} ${forkCount === 1 ? 'fork' : 'forks'} hidden`
: ''}
</div>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { describe, expect, it } from '@jest/globals';
import {
visibleRepositories,
withVisibleSelected,
withoutVisibleSelected,
} from './repository-multi-select-selection';

// `fork: undefined` is a repo cached before the flag existed — it must stay listed.
const repositories = [
{ id: 1, fork: false },
{ id: 2, fork: true },
{ id: 3, fork: undefined },
];

describe('visibleRepositories', () => {
it('lists every repository when forks are not hidden', () => {
expect(visibleRepositories(repositories, false).map(repo => repo.id)).toEqual([1, 2, 3]);
});

it('drops only forks when forks are hidden', () => {
expect(visibleRepositories(repositories, true).map(repo => repo.id)).toEqual([1, 3]);
});
});

describe('withVisibleSelected', () => {
it('adds the visible repositories without duplicating a selected one', () => {
expect(withVisibleSelected([1, 5], [1, 3])).toEqual([1, 5, 3]);
});
});

describe('withoutVisibleSelected', () => {
it('clears the visible repositories and keeps a hidden selection', () => {
expect(withoutVisibleSelected([1, 2, 3], [1, 3])).toEqual([2]);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/** The repositories the picker lists. Forks drop out only when the user hides them. */
export function visibleRepositories<T extends { fork?: boolean }>(
repositories: readonly T[],
hideForks: boolean
): T[] {
return hideForks ? repositories.filter(repository => !repository.fork) : [...repositories];
}

/** Select-all adds the listed repositories and keeps selections the fork filter hides. */
export function withVisibleSelected<TId>(
selectedIds: readonly TId[],
visibleIds: readonly TId[]
): TId[] {
return [...new Set([...selectedIds, ...visibleIds])];
}

/** Deselect-all clears the listed repositories and keeps selections the fork filter hides. */
export function withoutVisibleSelected<TId>(
selectedIds: readonly TId[],
visibleIds: readonly TId[]
): TId[] {
const visible = new Set(visibleIds);
return selectedIds.filter(id => !visible.has(id));
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export type SecurityRepository = {
fullName: string;
name: string;
private: boolean;
fork?: boolean;
dependabotAlerts: DependabotAlertsAvailability;
};

Expand Down Expand Up @@ -166,5 +167,6 @@ export function toRepositoryOptions(repositories: SecurityRepository[]): Reposit
name: repository.name,
full_name: repository.fullName,
private: repository.private,
fork: repository.fork,
}));
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type GitHubRepositoriesResult = {
name: string;
fullName: string;
private: boolean;
fork?: boolean;
}[];
errorMessage?: string;
};
Expand Down
45 changes: 40 additions & 5 deletions apps/web/src/lib/cloud-agent/github-integration-helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ jest.mock('@/components/cloud-agent/demo-config', () => ({
DEMO_SOURCE_REPO_NAME: 'demo-repo',
}));

const cachedRepositories = [{ id: 1, name: 'repo', full_name: 'org/repo', private: false }];
const cachedRepositories = [
{ id: 1, name: 'repo', full_name: 'org/repo', private: false, fork: false },
];

const buildIntegration = (overrides: Partial<PlatformIntegration> = {}): PlatformIntegration =>
({
Expand Down Expand Up @@ -78,7 +80,7 @@ describe('github-integration-helpers', () => {

expect(result.integrationInstalled).toBe(true);
expect(result.repositories).toEqual([
{ id: 1, name: 'repo', fullName: 'org/repo', private: false },
{ id: 1, name: 'repo', fullName: 'org/repo', private: false, fork: false },
]);
expect(mockFetchGitHubRepositories).not.toHaveBeenCalled();
});
Expand Down Expand Up @@ -137,6 +139,28 @@ describe('github-integration-helpers', () => {
expect(mockUpdateRepositoriesForIntegration).not.toHaveBeenCalled();
});

it('refetches when the cached repositories predate the fork flag', async () => {
mockGetIntegrationForOwner.mockResolvedValue(
buildIntegration({
repositories: [{ id: 1, name: 'repo', full_name: 'org/repo', private: false }],
})
);
mockFetchGitHubRepositories.mockResolvedValue([
{ id: 1, name: 'repo', full_name: 'org/repo', private: false, fork: true },
]);

const { fetchGitHubRepositoriesForUser } = await import('./github-integration-helpers');
const result = await fetchGitHubRepositoriesForUser('user-123');

expect(mockFetchGitHubRepositories).toHaveBeenCalled();
expect(mockUpdateRepositoriesForIntegration).toHaveBeenCalledWith('integration-1', [
{ id: 1, name: 'repo', full_name: 'org/repo', private: false, fork: true },
]);
expect(result.repositories).toEqual([
{ id: 1, name: 'repo', fullName: 'org/repo', private: false, fork: true },
]);
});

it('fetches fresh repositories when forceRefresh is true', async () => {
mockGetIntegrationForOwner.mockResolvedValue(buildIntegration());
mockFetchGitHubRepositories.mockResolvedValue([
Expand Down Expand Up @@ -171,6 +195,7 @@ describe('github-integration-helpers', () => {
name: 'repo',
fullName: 'org/repo',
private: false,
fork: false,
platformIntegrationId: 'integration-1',
},
]);
Expand All @@ -182,14 +207,22 @@ describe('github-integration-helpers', () => {
buildIntegration({
id: 'integration-1',
platform_account_login: 'acme-core',
repositories: [{ id: 1, name: 'api', full_name: 'acme-core/api', private: true }],
repositories: [
{ id: 1, name: 'api', full_name: 'acme-core/api', private: true, fork: false },
],
}),
buildIntegration({
id: 'integration-2',
platform_installation_id: 'installation-2',
platform_account_login: 'acme-security',
repositories: [
{ id: 2, name: 'scanner', full_name: 'acme-security/scanner', private: true },
{
id: 2,
name: 'scanner',
full_name: 'acme-security/scanner',
private: true,
fork: false,
},
],
}),
]);
Expand All @@ -216,7 +249,9 @@ describe('github-integration-helpers', () => {
mockGetIntegrationsByOrganization.mockResolvedValue([
buildIntegration({
id: 'integration-1',
repositories: [{ id: 1, name: 'api', full_name: 'acme-core/api', private: true }],
repositories: [
{ id: 1, name: 'api', full_name: 'acme-core/api', private: true, fork: false },
],
}),
buildIntegration({
id: 'integration-2',
Expand Down
13 changes: 10 additions & 3 deletions apps/web/src/lib/cloud-agent/github-integration-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
isPlatformIntegrationSuspended,
} from '@/lib/integrations/core/health';
import {
needsForkFlagBackfill,
requireNumericPlatformRepositories,
type PlatformRepository,
} from '@/lib/integrations/core/types';
Expand All @@ -29,6 +30,7 @@ type GitHubRepositoriesResult = {
name: string;
fullName: string;
private: boolean;
fork?: boolean;
platformIntegrationId?: string;
platformAccountLogin?: string;
}[];
Expand All @@ -45,6 +47,7 @@ const mapRepositories = (
name: repo.name,
fullName: repo.full_name,
private: repo.private,
fork: repo.fork,
...(integration
? {
platformIntegrationId: integration.id,
Expand Down Expand Up @@ -162,7 +165,7 @@ export async function fetchGitHubRepositoriesForOrganization(

try {
const cachedRepositories = requireNumericPlatformRepositories(integration.repositories);
if (forceRefresh || !cachedRepositories?.length) {
if (forceRefresh || !cachedRepositories?.length || needsForkFlagBackfill(cachedRepositories)) {
const repositories = await fetchGitHubRepositories(
integration.platform_installation_id,
integration.github_app_type || 'standard'
Expand Down Expand Up @@ -210,7 +213,11 @@ async function fetchRepositoriesForIntegrations(
integrations.map(async integration => {
if (!integration.platform_installation_id) return { repositories: [], syncedAt: null };
const cachedRepositories = requireNumericPlatformRepositories(integration.repositories);
if (forceRefresh || !cachedRepositories?.length) {
if (
forceRefresh ||
!cachedRepositories?.length ||
needsForkFlagBackfill(cachedRepositories)
) {
const repositories = await fetchGitHubRepositories(
integration.platform_installation_id,
integration.github_app_type || 'standard'
Expand Down Expand Up @@ -271,7 +278,7 @@ export async function fetchGitHubRepositoriesForUser(
try {
const cachedRepositories = requireNumericPlatformRepositories(integration.repositories);
// If forceRefresh or no cached repos, fetch from GitHub and update cache
if (forceRefresh || !cachedRepositories?.length) {
if (forceRefresh || !cachedRepositories?.length || needsForkFlagBackfill(cachedRepositories)) {
const appType = integration.github_app_type || 'standard';
const repositories = await fetchGitHubRepositories(
integration.platform_installation_id,
Expand Down
3 changes: 3 additions & 0 deletions apps/web/src/lib/code-reviews/core/selectable-repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export type FetchedRepository = {
name: string;
fullName: string;
private: boolean;
fork?: boolean;
};

export type ManuallyAddedRepository = {
Expand All @@ -32,6 +33,7 @@ export type SelectableRepository = {
name: string;
full_name: string;
private: boolean;
fork?: boolean;
};

/** The deduped repository list offered for review config / conversion (fetched + legacy manual). */
Expand All @@ -44,6 +46,7 @@ export function buildSelectableRepositories(
name: repo.name,
full_name: repo.fullName,
private: repo.private,
fork: repo.fork,
}));
const seenIds = new Set(canonical.map(repo => repo.id));
const legacy = manuallyAdded.filter(repo => !seenIds.has(repo.id));
Expand Down
5 changes: 5 additions & 0 deletions apps/web/src/lib/integrations/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ export function requireNumericPlatformRepositories(
return repositories;
}

/** True when a cached GitHub repository list predates the `fork` flag and must be refetched. */
export function needsForkFlagBackfill(repositories: PlatformRepository[] | null): boolean {
return repositories?.some(repository => repository.fork === undefined) ?? false;
}

/**
* Represents ownership of an integration
* Can be either a user or an organization
Expand Down
Loading