diff --git a/app/src/components/Workspace/GoogleDriveResourcePickerDialog.test.tsx b/app/src/components/Workspace/GoogleDriveResourcePickerDialog.test.tsx index c80fa5f2..abcbcc13 100644 --- a/app/src/components/Workspace/GoogleDriveResourcePickerDialog.test.tsx +++ b/app/src/components/Workspace/GoogleDriveResourcePickerDialog.test.tsx @@ -1,5 +1,5 @@ // @vitest-environment jsdom -import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest' import { GoogleDriveResourcePickerDialog } from './GoogleDriveResourcePickerDialog' @@ -9,6 +9,7 @@ const mocks = vi.hoisted(() => ({ listChildren: vi.fn(), listRoots: vi.fn(), searchResources: vi.fn(), + resolvePaths: vi.fn(), })) vi.mock('./googleDriveBrowser', async () => { @@ -23,8 +24,14 @@ vi.mock('./googleDriveBrowser', async () => { } }) +vi.mock('./googleDrivePaths', () => ({ + resolveGoogleDriveResourcePaths: mocks.resolvePaths, +})) + describe('GoogleDriveResourcePickerDialog', () => { beforeEach(() => { + mocks.resolvePaths.mockReset() + mocks.resolvePaths.mockResolvedValue(undefined) mocks.listRoots.mockReset() mocks.listRoots.mockResolvedValue([ { id: 'my-drive-root-id', name: 'My Drive' }, @@ -118,9 +125,7 @@ describe('GoogleDriveResourcePickerDialog', () => { it('keeps an actionable error open and retries root listing', async () => { mocks.listRoots .mockRejectedValueOnce(new Error('Drive API disabled')) - .mockResolvedValueOnce([ - { id: 'my-drive-root-id', name: 'My Drive' }, - ]) + .mockResolvedValueOnce([{ id: 'my-drive-root-id', name: 'My Drive' }]) render( { trigger.remove() }) }) + +/** Starts a search only after root discovery has completed. */ +async function searchNotebooks() { + await screen.findByRole('button', { name: 'Open My Drive' }) + fireEvent.change(screen.getByRole('searchbox'), { + target: { value: 'Notebooks' }, + }) + fireEvent.click(screen.getByRole('button', { name: 'Search' })) +} + +it('shows duplicate names immediately and progressively adds accessible full paths', async () => { + mocks.listRoots.mockResolvedValue([{ id: 'my-root', name: 'My Drive' }]) + mocks.searchResources.mockResolvedValue([ + { + id: 'a', + name: 'Notebooks', + mimeType: 'application/vnd.google-apps.folder', + }, + { + id: 'b', + name: 'Notebooks', + mimeType: 'application/vnd.google-apps.folder', + }, + ]) + let publish!: (id: string, path: { label: string; complete: boolean }) => void + let finish!: () => void + mocks.resolvePaths.mockImplementation((_token, _matches, _roots, onPath) => { + publish = onPath + return new Promise((resolve) => { + finish = resolve + }) + }) + render( + + ) + await searchNotebooks() + expect( + await screen.findAllByRole('button', { name: 'Open folder Notebooks' }) + ).toHaveLength(2) + expect(screen.getAllByText('Loading path…')).toHaveLength(2) + await act(async () => { + publish('a', { label: 'My Drive / Work / Notebooks', complete: true }) + publish('b', { label: 'Engineering / Team / Notebooks', complete: true }) + finish() + }) + const rows = screen.getAllByRole('button', { name: 'Open folder Notebooks' }) + expect( + document.getElementById(rows[0].getAttribute('aria-describedby')!) + ?.textContent + ).toBe('My Drive / Work / Notebooks') + expect(screen.getByTitle('Engineering / Team / Notebooks')).toBeTruthy() +}) + +it.each(['navigation', 'new search', 'credentials', 'unmount'])( + 'aborts paths and ignores late callbacks after %s', + async (action) => { + mocks.listRoots.mockResolvedValue([{ id: 'my-root', name: 'My Drive' }]) + mocks.listChildren.mockResolvedValue([]) + mocks.searchResources.mockResolvedValue([ + { + id: 'a', + name: 'Notebooks', + mimeType: 'application/vnd.google-apps.folder', + }, + ]) + const pending: Array<{ + publish: (id: string, path: { label: string; complete: boolean }) => void + signal: AbortSignal + finish: () => void + }> = [] + mocks.resolvePaths.mockImplementation( + (_token, _matches, _roots, publish, signal) => + new Promise((finish) => pending.push({ publish, signal, finish })) + ) + const props = { + accessToken: 'token', + mode: 'folder' as const, + onCancel: vi.fn(), + onSelect: vi.fn(), + } + const view = render() + await searchNotebooks() + await screen.findByRole('button', { name: 'Open folder Notebooks' }) + const first = pending[0] + if (action === 'navigation') + fireEvent.click( + screen.getByRole('button', { name: 'Open folder Notebooks' }) + ) + if (action === 'new search') { + fireEvent.change(screen.getByRole('searchbox'), { + target: { value: 'Other' }, + }) + fireEvent.click(screen.getByRole('button', { name: 'Search' })) + await waitFor(() => expect(pending).toHaveLength(2)) + } + if (action === 'credentials') + view.rerender( + + ) + if (action === 'unmount') view.unmount() + expect(first.signal.aborted).toBe(true) + await act(async () => { + first.publish('a', { label: 'Stale path', complete: true }) + first.finish() + pending[1]?.finish() + }) + expect(screen.queryByText('Stale path')).toBeNull() + } +) + +it('keeps file selection usable while a partial path is published', async () => { + mocks.listRoots.mockResolvedValue([{ id: 'my-root', name: 'My Drive' }]) + mocks.searchResources.mockResolvedValue([ + { + id: 'file-id', + name: 'notes.json', + mimeType: 'application/json', + resourceKey: 'key', + }, + ]) + mocks.resolvePaths.mockImplementation( + async (_token, _matches, _roots, publish) => { + publish('file-id', { + label: '… / notes.json (partial path)', + complete: false, + }) + } + ) + const onSelect = vi.fn() + render( + + ) + await searchNotebooks() + fireEvent.click( + await screen.findByRole('button', { name: 'Select file notes.json' }) + ) + expect(screen.getByText('… / notes.json (partial path)')).toBeTruthy() + fireEvent.click( + screen.getByRole('button', { name: 'Select file', exact: true }) + ) + expect(onSelect).toHaveBeenCalledWith( + expect.objectContaining({ id: 'file-id', resourceKey: 'key' }) + ) +}) diff --git a/app/src/components/Workspace/GoogleDriveResourcePickerDialog.tsx b/app/src/components/Workspace/GoogleDriveResourcePickerDialog.tsx index b7331e5b..58ed753c 100644 --- a/app/src/components/Workspace/GoogleDriveResourcePickerDialog.tsx +++ b/app/src/components/Workspace/GoogleDriveResourcePickerDialog.tsx @@ -14,6 +14,11 @@ import type { GoogleDriveResource, } from './googleDriveBrowser' +import { + resolveGoogleDriveResourcePaths, + type GoogleDriveResourcePath, +} from './googleDrivePaths' + export type GoogleDrivePickerMode = 'file' | 'folder' export type PickedGoogleDriveResource = { @@ -54,6 +59,10 @@ export function GoogleDriveResourcePickerDialog({ }: GoogleDriveResourcePickerDialogProps) { const dialogRef = useRef(null) const requestIdRef = useRef(0) + const pathsAbortRef = useRef(null) + const [paths, setPaths] = useState>( + {} + ) const [roots, setRoots] = useState([]) const [breadcrumbs, setBreadcrumbs] = useState([]) const [resources, setResources] = useState([]) @@ -69,6 +78,8 @@ export function GoogleDriveResourcePickerDialog({ const loadRoots = useCallback(async () => { const requestId = ++requestIdRef.current + pathsAbortRef.current?.abort() + setPaths({}) setLoading(true) setErrorMessage('') setBreadcrumbs([]) @@ -107,6 +118,8 @@ export function GoogleDriveResourcePickerDialog({ nextBreadcrumbs: GoogleDriveLocation[] ) => { const requestId = ++requestIdRef.current + pathsAbortRef.current?.abort() + setPaths({}) setLoading(true) setErrorMessage('') setBreadcrumbs(nextBreadcrumbs) @@ -155,6 +168,8 @@ export function GoogleDriveResourcePickerDialog({ return } const requestId = ++requestIdRef.current + pathsAbortRef.current?.abort() + setPaths({}) setLoading(true) setErrorMessage('') setActiveSearch(query) @@ -169,6 +184,20 @@ export function GoogleDriveResourcePickerDialog({ ) if (requestId === requestIdRef.current) { setResources(matches) + setLoading(false) + const controller = new AbortController() + pathsAbortRef.current = controller + await resolveGoogleDriveResourcePaths( + accessToken, + matches, + roots, + (id, path) => { + if (requestId === requestIdRef.current) { + setPaths((previous) => ({ ...previous, [id]: path })) + } + }, + controller.signal + ) } } catch (error) { if (requestId !== requestIdRef.current) { @@ -192,13 +221,14 @@ export function GoogleDriveResourcePickerDialog({ } } }, - [accessToken, loadRoots, mode] + [accessToken, loadRoots, mode, roots] ) useEffect(() => { void loadRoots() return () => { requestIdRef.current += 1 + pathsAbortRef.current?.abort() } }, [loadRoots]) @@ -445,10 +475,12 @@ export function GoogleDriveResourcePickerDialog({

) : (
- {visibleResources.map((resource) => { + {visibleResources.map((resource, index) => { const isFolder = resource.mimeType === GOOGLE_DRIVE_FOLDER_MIME_TYPE const selected = selectedFile?.id === resource.id + const pathLabel = paths[resource.id]?.label ?? 'Loading path…' + const pathId = `google-drive-resource-picker-path-${index}` return ( +

{selected}

+ {open && ( + setOpen(false)} + onSelect={(resource) => { + setSelected(`Selected ID: ${resource.id}`) + setOpen(false) + }} + /> + )} + + ) +} + +createRoot(document.getElementById('root')!).render() diff --git a/docs-dev/CUJs/assets/drive-search-paths.png b/docs-dev/CUJs/assets/drive-search-paths.png new file mode 100644 index 00000000..a7da6be1 Binary files /dev/null and b/docs-dev/CUJs/assets/drive-search-paths.png differ diff --git a/docs-dev/CUJs/drive-search-paths.md b/docs-dev/CUJs/drive-search-paths.md new file mode 100644 index 00000000..22922627 --- /dev/null +++ b/docs-dev/CUJs/drive-search-paths.md @@ -0,0 +1,41 @@ +# Disambiguate Google Drive search results + +The production folder and file picker displays a full path below each search +match. Paths load progressively; inaccessible ancestry is labeled partial. +Selection uses Drive IDs, including when full paths themselves collide. + +## Reproduce with synthetic data + +1. Run `go run testing/drive-paths/main.go` from the repository root. +2. Run `pnpm -C app dev --host 127.0.0.1 --port 5186`. +3. Open `http://127.0.0.1:5186/test/fixtures/drive-paths.html`. +4. Search for `Notebooks`. +5. Verify three independently selectable rows with these descriptions: + - `My Drive / Projects / Notebooks` + - `Engineering / Runme / Notebooks` + - `… / Notebooks (partial path)` +6. Open the Engineering result, then choose **Select this folder**. +7. Verify the page reports `Selected ID: team-notebooks`. +8. Reopen the picker and use keyboard focus to inspect path descriptions. + +The fixture imports the production React dialog and resolver and sends real +HTTP requests to a read-only Go fixture. Its token and data are synthetic. +The fixture is a Vite development entry point, not a production app route. +Save screenshots and assertion transcripts under `app/test/browser/test-output/`. + +## Verified UI + +![Full and partial paths in the production picker with synthetic data](assets/drive-search-paths.png) + +## Regression coverage + +- `googleDriveBrowser.test.ts`: search requests and preserves parent IDs; + shortcut source parents are not attached to target IDs. +- `googleDrivePaths.test.ts`: root naming, shared-ancestor request deduplication, + result-metadata reuse, target resource keys, inaccessible/malformed ancestors, + cycle/depth limits, bounded concurrency, timeout and cancellation. +- `GoogleDriveResourcePickerDialog.test.tsx`: progressive path descriptions and + stale callback rejection after search, navigation, token changes and unmount. + +The design and implementation record is in +[20261003_google_drive_full_paths.runme](https://web.runme.dev/?doc=https%3A%2F%2Fdrive.google.com%2Ffile%2Fd%2F100i8UJfmZyvqw-1UxFiIs5pxM2Nt79oC%2Fview). diff --git a/testing/drive-paths/main.go b/testing/drive-paths/main.go new file mode 100644 index 00000000..56e782ac --- /dev/null +++ b/testing/drive-paths/main.go @@ -0,0 +1,58 @@ +// A read-only Drive fixture for the full-path picker CUJ. All data is synthetic. +package main + +import ( + "encoding/json" + "flag" + "log" + "net/http" + "strings" +) + +func main() { + addr := flag.String("addr", "127.0.0.1:9098", "listen address") + flag.Parse() + folders := []map[string]any{ + {"id": "personal-notebooks", "name": "Notebooks", "mimeType": "application/vnd.google-apps.folder", "parents": []string{"personal-projects"}}, + {"id": "team-notebooks", "name": "Notebooks", "mimeType": "application/vnd.google-apps.folder", "parents": []string{"team-projects"}, "driveId": "engineering"}, + {"id": "archive-notebooks", "name": "Notebooks", "mimeType": "application/vnd.google-apps.folder", "parents": []string{"restricted"}}, + } + metadata := map[string]any{ + "root": map[string]any{"id": "my-root"}, + "personal-projects": map[string]any{"id": "personal-projects", "name": "Projects", "parents": []string{"my-root"}}, + "team-projects": map[string]any{"id": "team-projects", "name": "Runme", "parents": []string{"engineering"}, "driveId": "engineering"}, + } + http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Access-Control-Allow-Origin", "*") + w.Header().Set("Access-Control-Allow-Headers", "Authorization, X-Goog-Drive-Resource-Keys") + w.Header().Set("Content-Type", "application/json") + if r.Method == http.MethodOptions { + w.WriteHeader(http.StatusNoContent) + return + } + if r.Method != http.MethodGet { + w.WriteHeader(http.StatusMethodNotAllowed) + return + } + var response any + switch { + case r.URL.Path == "/drive/v3/drives": + response = map[string]any{"drives": []map[string]string{{"id": "engineering", "name": "Engineering"}}} + case r.URL.Path == "/drive/v3/files": + if strings.Contains(r.URL.Query().Get("q"), "name contains") { + response = map[string]any{"files": folders} + } else { + response = map[string]any{"files": []any{}} + } + default: + var ok bool + response, ok = metadata[strings.TrimPrefix(r.URL.Path, "/drive/v3/files/")] + if !ok { + w.WriteHeader(http.StatusNotFound) + response = map[string]string{"error": "unavailable"} + } + } + json.NewEncoder(w).Encode(response) + }) + log.Fatal(http.ListenAndServe(*addr, nil)) +}