mirror of
https://github.com/stablyai/orca.git
synced 2026-09-27 08:02:35 +00:00
fix: distinguish duplicate project names in picker (#6272)
* fix: distinguish duplicate project names in picker * review: disambiguate duplicate project picker details Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Jinwoo-H <jinwoo0825@gmail.com> Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
jeganathan I
co-authored by
Orca
Jinwoo-H
Orca
Jinwoo-H
parent
d6c96e77d2
commit
ec9efe4638
@@ -138,4 +138,34 @@ describe('ProjectCombobox', () => {
|
||||
|
||||
expect(onValueChange).toHaveBeenCalledWith('project-group:folder-group')
|
||||
})
|
||||
|
||||
it('renders directory details for duplicate project names', () => {
|
||||
const duplicateProjects: NewWorkspaceProjectOption[] = [
|
||||
{
|
||||
kind: 'project',
|
||||
id: 'project:merchant-a',
|
||||
projectId: 'project:merchant-a',
|
||||
displayName: 'merchant',
|
||||
badgeColor: '#111111',
|
||||
detail: '/workspace/storefront/merchant'
|
||||
},
|
||||
{
|
||||
kind: 'project',
|
||||
id: 'project:merchant-b',
|
||||
projectId: 'project:merchant-b',
|
||||
displayName: 'merchant',
|
||||
badgeColor: '#222222',
|
||||
detail: '/workspace/admin/merchant'
|
||||
}
|
||||
]
|
||||
|
||||
act(() => {
|
||||
root.render(
|
||||
<ProjectCombobox options={duplicateProjects} value={null} onValueChange={vi.fn()} />
|
||||
)
|
||||
})
|
||||
|
||||
expect(container.textContent).toContain('/workspace/storefront/merchant')
|
||||
expect(container.textContent).toContain('/workspace/admin/merchant')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -825,9 +825,10 @@ export function useComposerState(options: UseComposerStateOptions): UseComposerS
|
||||
projects,
|
||||
projectHostSetups,
|
||||
eligibleRepos,
|
||||
projectGroups
|
||||
projectGroups,
|
||||
hosts: hostOptions
|
||||
}),
|
||||
[eligibleRepos, projectGroups, projectHostSetups, projects]
|
||||
[eligibleRepos, hostOptions, projectGroups, projectHostSetups, projects]
|
||||
)
|
||||
const selectedRepoSettings = useMemo(() => {
|
||||
if (!settings) {
|
||||
|
||||
@@ -0,0 +1,147 @@
|
||||
import { getExecutionHostLabel, type ExecutionHostId } from '../../../shared/execution-host'
|
||||
import type { NewWorkspaceProjectOption } from './new-workspace-project-options'
|
||||
|
||||
export type ProjectSetupDirectory = {
|
||||
path: string
|
||||
hostId: ExecutionHostId
|
||||
}
|
||||
|
||||
export type ProjectOptionDraft = Extract<NewWorkspaceProjectOption, { kind: 'project' }> & {
|
||||
detailSource: 'provider' | 'generic'
|
||||
}
|
||||
|
||||
type ProjectDirectoryDetailMode = 'path' | 'host-label' | 'host-id'
|
||||
|
||||
function getRepeatedValues(values: readonly string[]): Set<string> {
|
||||
const seen = new Set<string>()
|
||||
const repeated = new Set<string>()
|
||||
for (const value of values) {
|
||||
if (seen.has(value)) {
|
||||
repeated.add(value)
|
||||
continue
|
||||
}
|
||||
seen.add(value)
|
||||
}
|
||||
return repeated
|
||||
}
|
||||
|
||||
function getProjectDirectoryDetail(
|
||||
directories: readonly ProjectSetupDirectory[],
|
||||
hostLabelById: ReadonlyMap<ExecutionHostId, string>,
|
||||
mode: ProjectDirectoryDetailMode
|
||||
): string | null {
|
||||
const detailByKey = new Map<string, string>()
|
||||
for (const directory of directories) {
|
||||
const path = directory.path.trim()
|
||||
if (!path) {
|
||||
continue
|
||||
}
|
||||
const hostLabel =
|
||||
hostLabelById.get(directory.hostId)?.trim() || getExecutionHostLabel(directory.hostId)
|
||||
const detail =
|
||||
mode === 'path'
|
||||
? path
|
||||
: mode === 'host-id'
|
||||
? `${hostLabel} (${directory.hostId}) · ${path}`
|
||||
: `${hostLabel} · ${path}`
|
||||
const key = mode === 'path' ? path : `${directory.hostId}\0${path}`
|
||||
detailByKey.set(key, detail)
|
||||
}
|
||||
|
||||
const distinctDetails = [...detailByKey.values()].sort()
|
||||
if (distinctDetails.length === 0) {
|
||||
return null
|
||||
}
|
||||
|
||||
const [firstDetail] = distinctDetails
|
||||
return distinctDetails.length === 1
|
||||
? firstDetail
|
||||
: `${firstDetail} (+${distinctDetails.length - 1} more)`
|
||||
}
|
||||
|
||||
export function getDuplicateProjectDetailsById(
|
||||
options: readonly ProjectOptionDraft[],
|
||||
setupDirectoriesByProjectId: ReadonlyMap<string, readonly ProjectSetupDirectory[]>,
|
||||
hostLabelById: ReadonlyMap<ExecutionHostId, string>
|
||||
): Map<string, string> {
|
||||
const optionsByName = new Map<string, ProjectOptionDraft[]>()
|
||||
for (const option of options) {
|
||||
optionsByName.set(option.displayName, [
|
||||
...(optionsByName.get(option.displayName) ?? []),
|
||||
option
|
||||
])
|
||||
}
|
||||
|
||||
const detailsById = new Map<string, string>()
|
||||
for (const sameNameOptions of optionsByName.values()) {
|
||||
if (sameNameOptions.length < 2) {
|
||||
continue
|
||||
}
|
||||
|
||||
const repeatedProviderDetails = getRepeatedValues(
|
||||
sameNameOptions
|
||||
.filter((option) => option.detailSource === 'provider')
|
||||
.map((option) => option.detail)
|
||||
)
|
||||
const ambiguousOptions = sameNameOptions.filter(
|
||||
(option) => option.detailSource === 'generic' || repeatedProviderDetails.has(option.detail)
|
||||
)
|
||||
if (ambiguousOptions.length === 0) {
|
||||
continue
|
||||
}
|
||||
|
||||
const pathOnlyDetailsById = new Map<string, string>()
|
||||
for (const option of ambiguousOptions) {
|
||||
const detail = getProjectDirectoryDetail(
|
||||
setupDirectoriesByProjectId.get(option.projectId) ?? [],
|
||||
hostLabelById,
|
||||
'path'
|
||||
)
|
||||
if (detail) {
|
||||
pathOnlyDetailsById.set(option.id, detail)
|
||||
}
|
||||
}
|
||||
|
||||
const repeatedPathOnlyDetails = getRepeatedValues([...pathOnlyDetailsById.values()])
|
||||
const hostLabelDetailsById = new Map<string, string>()
|
||||
for (const option of ambiguousOptions) {
|
||||
const pathOnlyDetail = pathOnlyDetailsById.get(option.id)
|
||||
if (!pathOnlyDetail || !repeatedPathOnlyDetails.has(pathOnlyDetail)) {
|
||||
continue
|
||||
}
|
||||
const detail = getProjectDirectoryDetail(
|
||||
setupDirectoriesByProjectId.get(option.projectId) ?? [],
|
||||
hostLabelById,
|
||||
'host-label'
|
||||
)
|
||||
if (detail) {
|
||||
hostLabelDetailsById.set(option.id, detail)
|
||||
}
|
||||
}
|
||||
|
||||
const repeatedHostLabelDetails = getRepeatedValues([...hostLabelDetailsById.values()])
|
||||
for (const option of ambiguousOptions) {
|
||||
const pathOnlyDetail = pathOnlyDetailsById.get(option.id)
|
||||
if (!pathOnlyDetail) {
|
||||
continue
|
||||
}
|
||||
let detail = pathOnlyDetail
|
||||
if (repeatedPathOnlyDetails.has(pathOnlyDetail)) {
|
||||
detail = hostLabelDetailsById.get(option.id) ?? pathOnlyDetail
|
||||
if (repeatedHostLabelDetails.has(detail)) {
|
||||
// Why: user-renamed hosts can share a label, so fall back to the
|
||||
// stable host id only when the final visible detail still repeats.
|
||||
detail =
|
||||
getProjectDirectoryDetail(
|
||||
setupDirectoriesByProjectId.get(option.projectId) ?? [],
|
||||
hostLabelById,
|
||||
'host-id'
|
||||
) ?? detail
|
||||
}
|
||||
}
|
||||
detailsById.set(option.id, detail)
|
||||
}
|
||||
}
|
||||
|
||||
return detailsById
|
||||
}
|
||||
@@ -108,6 +108,257 @@ describe('buildNewWorkspaceProjectOptions', () => {
|
||||
expect(options.map((option) => option.id)).toEqual(['github:stablyai/orca'])
|
||||
})
|
||||
|
||||
it('shows configured directories when project names are duplicated', () => {
|
||||
const options = buildNewWorkspaceProjectOptions({
|
||||
projects: [
|
||||
project({
|
||||
id: 'project:merchant-a',
|
||||
displayName: 'merchant',
|
||||
providerIdentity: undefined
|
||||
}),
|
||||
project({
|
||||
id: 'project:merchant-b',
|
||||
displayName: 'merchant',
|
||||
providerIdentity: undefined
|
||||
})
|
||||
],
|
||||
projectHostSetups: [
|
||||
setup({
|
||||
id: 'merchant-a-setup',
|
||||
projectId: 'project:merchant-a',
|
||||
repoId: 'merchant-a-repo',
|
||||
path: '/workspace/storefront/merchant'
|
||||
}),
|
||||
setup({
|
||||
id: 'merchant-b-setup',
|
||||
projectId: 'project:merchant-b',
|
||||
repoId: 'merchant-b-repo',
|
||||
path: '/workspace/admin/merchant'
|
||||
})
|
||||
],
|
||||
eligibleRepos: [repo('merchant-a-repo'), repo('merchant-b-repo')]
|
||||
})
|
||||
|
||||
expect(options).toEqual([
|
||||
expect.objectContaining({
|
||||
id: 'project:merchant-b',
|
||||
detail: '/workspace/admin/merchant'
|
||||
}),
|
||||
expect.objectContaining({
|
||||
id: 'project:merchant-a',
|
||||
detail: '/workspace/storefront/merchant'
|
||||
})
|
||||
])
|
||||
})
|
||||
|
||||
it('keeps provider details when duplicate project names are already distinguishable', () => {
|
||||
const options = buildNewWorkspaceProjectOptions({
|
||||
projects: [
|
||||
project({
|
||||
id: 'github:acme/merchant',
|
||||
displayName: 'merchant',
|
||||
providerIdentity: { provider: 'github', owner: 'acme', repo: 'merchant' }
|
||||
}),
|
||||
project({
|
||||
id: 'github:contoso/merchant',
|
||||
displayName: 'merchant',
|
||||
providerIdentity: { provider: 'github', owner: 'contoso', repo: 'merchant' }
|
||||
})
|
||||
],
|
||||
projectHostSetups: [
|
||||
setup({
|
||||
id: 'acme-setup',
|
||||
projectId: 'github:acme/merchant',
|
||||
repoId: 'acme-repo',
|
||||
path: '/workspace/acme/merchant'
|
||||
}),
|
||||
setup({
|
||||
id: 'contoso-setup',
|
||||
projectId: 'github:contoso/merchant',
|
||||
repoId: 'contoso-repo',
|
||||
path: '/workspace/contoso/merchant'
|
||||
})
|
||||
],
|
||||
eligibleRepos: [repo('acme-repo'), repo('contoso-repo')]
|
||||
})
|
||||
|
||||
expect(options.map((option) => option.detail).sort()).toEqual([
|
||||
'acme/merchant',
|
||||
'contoso/merchant'
|
||||
])
|
||||
})
|
||||
|
||||
it('shows directory details for non-provider duplicates with different setup counts', () => {
|
||||
const options = buildNewWorkspaceProjectOptions({
|
||||
projects: [
|
||||
project({
|
||||
id: 'project:merchant-single',
|
||||
displayName: 'merchant',
|
||||
providerIdentity: undefined
|
||||
}),
|
||||
project({
|
||||
id: 'project:merchant-multi',
|
||||
displayName: 'merchant',
|
||||
providerIdentity: undefined
|
||||
})
|
||||
],
|
||||
projectHostSetups: [
|
||||
setup({
|
||||
id: 'merchant-single-setup',
|
||||
projectId: 'project:merchant-single',
|
||||
repoId: 'merchant-single-repo',
|
||||
path: '/workspace/single/merchant'
|
||||
}),
|
||||
setup({
|
||||
id: 'merchant-multi-local-setup',
|
||||
projectId: 'project:merchant-multi',
|
||||
repoId: 'merchant-multi-local-repo',
|
||||
path: '/workspace/multi/local/merchant'
|
||||
}),
|
||||
setup({
|
||||
id: 'merchant-multi-remote-setup',
|
||||
projectId: 'project:merchant-multi',
|
||||
hostId: 'ssh:builder',
|
||||
repoId: 'merchant-multi-remote-repo',
|
||||
path: '/workspace/multi/remote/merchant'
|
||||
})
|
||||
],
|
||||
eligibleRepos: [
|
||||
repo('merchant-single-repo'),
|
||||
repo('merchant-multi-local-repo'),
|
||||
repo('merchant-multi-remote-repo')
|
||||
]
|
||||
})
|
||||
|
||||
expect(options.map((option) => option.detail).sort()).toEqual([
|
||||
'/workspace/multi/local/merchant (+1 more)',
|
||||
'/workspace/single/merchant'
|
||||
])
|
||||
})
|
||||
|
||||
it('keeps provider details while disambiguating non-provider duplicate names', () => {
|
||||
const options = buildNewWorkspaceProjectOptions({
|
||||
projects: [
|
||||
project({
|
||||
id: 'github:acme/merchant',
|
||||
displayName: 'merchant',
|
||||
providerIdentity: { provider: 'github', owner: 'acme', repo: 'merchant' }
|
||||
}),
|
||||
project({
|
||||
id: 'project:merchant-local',
|
||||
displayName: 'merchant',
|
||||
providerIdentity: undefined
|
||||
})
|
||||
],
|
||||
projectHostSetups: [
|
||||
setup({
|
||||
id: 'acme-setup',
|
||||
projectId: 'github:acme/merchant',
|
||||
repoId: 'acme-repo',
|
||||
path: '/workspace/acme/merchant'
|
||||
}),
|
||||
setup({
|
||||
id: 'merchant-local-setup',
|
||||
projectId: 'project:merchant-local',
|
||||
repoId: 'merchant-local-repo',
|
||||
path: '/workspace/local/merchant'
|
||||
})
|
||||
],
|
||||
eligibleRepos: [repo('acme-repo'), repo('merchant-local-repo')]
|
||||
})
|
||||
|
||||
expect(options.map((option) => option.detail).sort()).toEqual([
|
||||
'/workspace/local/merchant',
|
||||
'acme/merchant'
|
||||
])
|
||||
})
|
||||
|
||||
it('adds host labels when duplicate project directory details collide', () => {
|
||||
const options = buildNewWorkspaceProjectOptions({
|
||||
projects: [
|
||||
project({
|
||||
id: 'project:merchant-local',
|
||||
displayName: 'merchant',
|
||||
providerIdentity: undefined
|
||||
}),
|
||||
project({
|
||||
id: 'project:merchant-remote',
|
||||
displayName: 'merchant',
|
||||
providerIdentity: undefined
|
||||
})
|
||||
],
|
||||
projectHostSetups: [
|
||||
setup({
|
||||
id: 'merchant-local-setup',
|
||||
projectId: 'project:merchant-local',
|
||||
hostId: 'local',
|
||||
repoId: 'merchant-local-repo',
|
||||
path: '/workspace/merchant'
|
||||
}),
|
||||
setup({
|
||||
id: 'merchant-remote-setup',
|
||||
projectId: 'project:merchant-remote',
|
||||
hostId: 'ssh:builder',
|
||||
repoId: 'merchant-remote-repo',
|
||||
path: '/workspace/merchant'
|
||||
})
|
||||
],
|
||||
eligibleRepos: [repo('merchant-local-repo'), repo('merchant-remote-repo')],
|
||||
hosts: [
|
||||
{ id: 'local', label: 'Local Mac' },
|
||||
{ id: 'ssh:builder', label: 'Builder' }
|
||||
]
|
||||
})
|
||||
|
||||
expect(options.map((option) => option.detail).sort()).toEqual([
|
||||
'Builder · /workspace/merchant',
|
||||
'Local Mac · /workspace/merchant'
|
||||
])
|
||||
})
|
||||
|
||||
it('adds host ids when duplicate project host labels still collide', () => {
|
||||
const options = buildNewWorkspaceProjectOptions({
|
||||
projects: [
|
||||
project({
|
||||
id: 'project:merchant-builder-a',
|
||||
displayName: 'merchant',
|
||||
providerIdentity: undefined
|
||||
}),
|
||||
project({
|
||||
id: 'project:merchant-builder-b',
|
||||
displayName: 'merchant',
|
||||
providerIdentity: undefined
|
||||
})
|
||||
],
|
||||
projectHostSetups: [
|
||||
setup({
|
||||
id: 'merchant-builder-a-setup',
|
||||
projectId: 'project:merchant-builder-a',
|
||||
hostId: 'ssh:builder-a',
|
||||
repoId: 'merchant-builder-a-repo',
|
||||
path: '/workspace/merchant'
|
||||
}),
|
||||
setup({
|
||||
id: 'merchant-builder-b-setup',
|
||||
projectId: 'project:merchant-builder-b',
|
||||
hostId: 'ssh:builder-b',
|
||||
repoId: 'merchant-builder-b-repo',
|
||||
path: '/workspace/merchant'
|
||||
})
|
||||
],
|
||||
eligibleRepos: [repo('merchant-builder-a-repo'), repo('merchant-builder-b-repo')],
|
||||
hosts: [
|
||||
{ id: 'ssh:builder-a', label: 'Builder' },
|
||||
{ id: 'ssh:builder-b', label: 'Builder' }
|
||||
]
|
||||
})
|
||||
|
||||
expect(options.map((option) => option.detail).sort()).toEqual([
|
||||
'Builder (ssh:builder-a) · /workspace/merchant',
|
||||
'Builder (ssh:builder-b) · /workspace/merchant'
|
||||
])
|
||||
})
|
||||
|
||||
it('filters project options by display name and detail', () => {
|
||||
const options: NewWorkspaceProjectOption[] = [
|
||||
{
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import { projectHostSetupProjectionFromRepos } from '../../../shared/project-host-setup-projection'
|
||||
import type { Project, ProjectGroup, ProjectHostSetup, Repo } from '../../../shared/types'
|
||||
import { isClipboardTextByteLengthOverLimit } from '../../../shared/clipboard-text'
|
||||
import type { ExecutionHostRegistryEntry } from '../../../shared/execution-host-registry'
|
||||
import {
|
||||
getDuplicateProjectDetailsById,
|
||||
type ProjectSetupDirectory
|
||||
} from './new-workspace-duplicate-project-details'
|
||||
|
||||
export const NEW_WORKSPACE_PROJECT_GROUP_OPTION_PREFIX = 'project-group:'
|
||||
export const NEW_WORKSPACE_FOLDER_SOURCE_OPTION_PREFIX = 'folder-source:'
|
||||
@@ -45,6 +50,7 @@ type BuildNewWorkspaceProjectOptionsInput = {
|
||||
projects: readonly Project[]
|
||||
projectHostSetups: readonly ProjectHostSetup[]
|
||||
eligibleRepos: readonly Repo[]
|
||||
hosts?: readonly Pick<ExecutionHostRegistryEntry, 'id' | 'label'>[]
|
||||
}
|
||||
|
||||
type BuildNewWorkspaceCreateTargetOptionsInput = BuildNewWorkspaceProjectOptionsInput & {
|
||||
@@ -85,7 +91,9 @@ export function buildNewWorkspaceProjectOptions(
|
||||
const { eligibleRepos } = input
|
||||
const { projects, projectHostSetups } = getProjectModel(input)
|
||||
const eligibleRepoIds = new Set(eligibleRepos.map((repo) => repo.id))
|
||||
const hostLabelById = new Map((input.hosts ?? []).map((host) => [host.id, host.label]))
|
||||
const readySetupCountsByProjectId = new Map<string, number>()
|
||||
const setupDirectoriesByProjectId = new Map<string, ProjectSetupDirectory[]>()
|
||||
|
||||
for (const setup of projectHostSetups) {
|
||||
if (setup.setupState !== 'ready' || !eligibleRepoIds.has(setup.repoId)) {
|
||||
@@ -95,9 +103,12 @@ export function buildNewWorkspaceProjectOptions(
|
||||
setup.projectId,
|
||||
(readySetupCountsByProjectId.get(setup.projectId) ?? 0) + 1
|
||||
)
|
||||
const directories = setupDirectoriesByProjectId.get(setup.projectId) ?? []
|
||||
directories.push({ path: setup.path, hostId: setup.hostId })
|
||||
setupDirectoriesByProjectId.set(setup.projectId, directories)
|
||||
}
|
||||
|
||||
return projects
|
||||
const options = projects
|
||||
.filter((project) => (readySetupCountsByProjectId.get(project.id) ?? 0) > 0)
|
||||
.map((project) => ({
|
||||
kind: 'project' as const,
|
||||
@@ -105,8 +116,21 @@ export function buildNewWorkspaceProjectOptions(
|
||||
projectId: project.id,
|
||||
displayName: project.displayName,
|
||||
badgeColor: project.badgeColor,
|
||||
detail: getProjectDetail(project, readySetupCountsByProjectId.get(project.id) ?? 0)
|
||||
detail: getProjectDetail(project, readySetupCountsByProjectId.get(project.id) ?? 0),
|
||||
detailSource: project.providerIdentity ? ('provider' as const) : ('generic' as const)
|
||||
}))
|
||||
|
||||
const duplicateProjectDetailsById = getDuplicateProjectDetailsById(
|
||||
options,
|
||||
setupDirectoriesByProjectId,
|
||||
hostLabelById
|
||||
)
|
||||
|
||||
return options
|
||||
.map(({ detailSource: _detailSource, ...option }) => {
|
||||
const directoryDetail = duplicateProjectDetailsById.get(option.id)
|
||||
return directoryDetail ? { ...option, detail: directoryDetail } : option
|
||||
})
|
||||
.sort((a, b) => a.displayName.localeCompare(b.displayName) || a.detail.localeCompare(b.detail))
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user