From 2db6fbf410035e4f1bf95f576a5c38d0d784f98e Mon Sep 17 00:00:00 2001 From: Francesco Sansalvadore Date: Wed, 16 Sep 2026 14:39:22 +0200 Subject: [PATCH] test(studio): add e2e coverage for the storage move picker (#50460) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Tests, plus one small test hook in Studio. ## What is the current behavior? The Storage file explorer's move dialog was recently reworked: the free-text "Path to new directory" input was replaced with an embedded folder picker (folder browsing, bucket-wide folder search, a responsive breadcrumb, and a confirm button that targets the folder currently open). That work shipped with unit and component tests, but nothing exercises it end to end against a real bucket. ## What is the new behavior? New `e2e/studio/features/storage-move.spec.ts` with seven tests: | Test | What it covers | | --- | --- | | moves a file into a folder picked from the explorer | The core path: open the picker, click a folder, confirm, and assert the file left the root and landed in the destination | | offers folders only, never files, as destinations | Files are excluded from the listing entirely | | blocks confirming a move into the folder the file already sits in | The confirm button reports `aria-disabled` when the destination matches the source | | finds a nested folder by search and moves into it | Bucket-wide folder search, including the "`` in ``" row label | | reports when a search matches no folders | The empty-search message instead of a blank list | | collapses the middle of a deep path into a breadcrumb dropdown | The responsive breadcrumb: bucket and the two deepest folders stay inline, the middle collapses, and picking a collapsed folder navigates to it | | walks back up the path with the up-one-level button | Disabled at the bucket root, and drops the deepest folder otherwise | Supporting changes: - `utils/storage/queries.ts` gains `uploadObject` and `seedBucket`. Storage has no standalone folders — a folder exists because an object sits under that prefix — so seeding a folder tree means uploading objects at the paths a test needs. Doing this through the API keeps setup off the UI, which is both faster and less flaky than clicking through "Create folder" for each level. - `utils/storage/client.ts` accepts a string body so object uploads can send raw content alongside the existing JSON requests. - `utils/storage-helpers.ts` gains `openMoveDialog` and `confirmMove`. - `MoveItemsFolderPicker.tsx` gains `data-testid="folder-picker-list"` on its list container. ## Additional context **Why the `data-testid`.** Once a path is deep enough for the breadcrumb to collapse, the breadcrumb renders crumb buttons whose accessible names are folder names — so `getByRole('button', { name: 'beta' })` scoped to the dialog can match either a folder row or a breadcrumb crumb depending on depth. Scoping row lookups to the list container removes that ambiguity. This follows the e2e guidance about adding explicit test hooks where a component lacks an unambiguous accessible name. **These tests have not been executed.** They were written against the merged implementation and verified as far as the environment allows: - `npx playwright test --list` collects all seven - `tsc --noEmit` is clean for the new spec and helpers (the pre-existing errors in `column-editor-types.spec.ts`, `table-editor.spec.ts`, and `wait-for-response-with-timeout.ts` are untouched) - Studio's unit and component tests (82) still pass, and typecheck, eslint, prettier, the lint ratchet, and knip are all clean The suite needs Docker to bring up the local Supabase stack, which wasn't available where this was authored, so a real run in CI is the first actual execution. Selectors were all read off the merged source rather than guessed, but timing assumptions in particular deserve attention on the first CI run. **One thing this surfaced, not fixed here.** The success toast reads `Successfully moved 1 files to docs` — it doesn't singularize. The tests assert on `/Successfully moved/` rather than the full string so they don't encode that, but it's worth a follow-up. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Q94G7pWso6vQn5FQz6TUns --- _Generated by [Claude Code](https://claude.ai/code/session_01Q94G7pWso6vQn5FQz6TUns)_ ## Summary by CodeRabbit - **Tests** - Expanded end-to-end coverage for moving files between folders in Storage. - Validated folder selection, nested-folder search, empty search results, collapsed breadcrumbs, and navigation to parent folders. - Confirmed files are excluded from destination choices and moving to the current folder is prevented. - Added coverage for creating isolated test buckets, uploading fixture files, and confirming successful move operations. --------- Co-authored-by: Claude --- .../StorageExplorer/MoveItemsFolderPicker.tsx | 2 +- e2e/studio/features/storage-move.spec.ts | 229 ++++++++++++++++++ e2e/studio/utils/storage-helpers.ts | 45 ++++ e2e/studio/utils/storage/client.ts | 21 +- e2e/studio/utils/storage/index.ts | 9 +- e2e/studio/utils/storage/queries.ts | 30 +++ 6 files changed, 328 insertions(+), 8 deletions(-) create mode 100644 e2e/studio/features/storage-move.spec.ts diff --git a/apps/studio/components/interfaces/Storage/StorageExplorer/MoveItemsFolderPicker.tsx b/apps/studio/components/interfaces/Storage/StorageExplorer/MoveItemsFolderPicker.tsx index 2520ee8638d..55b89dfc2a7 100644 --- a/apps/studio/components/interfaces/Storage/StorageExplorer/MoveItemsFolderPicker.tsx +++ b/apps/studio/components/interfaces/Storage/StorageExplorer/MoveItemsFolderPicker.tsx @@ -147,7 +147,7 @@ export const MoveItemsFolderPicker = ({

-
+
{isSearching && isPendingFolders && (
diff --git a/e2e/studio/features/storage-move.spec.ts b/e2e/studio/features/storage-move.spec.ts new file mode 100644 index 00000000000..de8da50f3e0 --- /dev/null +++ b/e2e/studio/features/storage-move.spec.ts @@ -0,0 +1,229 @@ +import { expect, type Page } from '@playwright/test' + +import { + confirmMove, + navigateToBucket, + navigateToStorageFiles, + openMoveDialog, +} from '../utils/storage-helpers.js' +import { deleteBucket as deleteBucketViaApi, seedBucket } from '../utils/storage/index.js' +import { test } from '../utils/test.js' + +const bucketNamePrefix = 'pw_move' + +/** + * Seeds a bucket from scratch and lands the explorer inside it. Each test uses its own bucket so + * the file can run in parallel with the rest of the suite. + */ +const setUpBucket = async (page: Page, ref: string, bucketName: string, objectPaths: string[]) => { + await deleteBucketViaApi(bucketName) + await seedBucket(bucketName, objectPaths) + await navigateToStorageFiles(page, ref) + await navigateToBucket(page, ref, bucketName) +} + +test.describe('Storage move file', () => { + test('moves a file into a folder picked from the explorer', async ({ page, ref }) => { + const bucketName = `${bucketNamePrefix}_basic` + const fileName = 'move-me.txt' + + await setUpBucket(page, ref, bucketName, [fileName, 'docs/seed.txt']) + + const dialog = await openMoveDialog(page, fileName) + const folderList = dialog.getByTestId('folder-picker-list') + + // The destination starts at the bucket root, so the file is already there + await expect( + dialog.getByText(`Moving to ${bucketName}`, { exact: true }), + 'Destination should start at the bucket root' + ).toBeVisible() + + await folderList.getByRole('button', { name: 'docs' }).click() + + await expect( + dialog.getByText(`Moving to ${bucketName}/docs`, { exact: true }), + 'Destination should follow the folder that was opened' + ).toBeVisible() + + await confirmMove(page, ref, 'docs') + + await expect( + page.getByText(/Successfully moved/), + 'A success toast should confirm the move' + ).toBeVisible() + await expect( + page.getByTitle(fileName), + 'File should no longer sit at the bucket root' + ).not.toBeVisible() + + // Open the destination folder and confirm the file landed there + await page.getByTitle('docs').click() + await expect( + page.getByTitle(fileName), + 'File should be inside the destination folder' + ).toBeVisible() + }) + + test('offers folders only, never files, as destinations', async ({ page, ref }) => { + const bucketName = `${bucketNamePrefix}_folders_only` + const fileName = 'picker-source.txt' + const siblingFileName = 'sibling.txt' + + await setUpBucket(page, ref, bucketName, [fileName, siblingFileName, 'docs/seed.txt']) + + const dialog = await openMoveDialog(page, fileName) + const folderList = dialog.getByTestId('folder-picker-list') + + await expect( + folderList.getByRole('button', { name: 'docs' }), + 'Folders should be listed as destinations' + ).toBeVisible() + await expect( + folderList.getByText(siblingFileName), + 'Files should not be listed in the picker at all' + ).not.toBeVisible() + await expect( + folderList.getByText(fileName, { exact: true }), + 'The file being moved should not be listed either' + ).not.toBeVisible() + }) + + test('blocks confirming a move into the folder the file already sits in', async ({ + page, + ref, + }) => { + const bucketName = `${bucketNamePrefix}_same_folder` + const fileName = 'already-here.txt' + + await setUpBucket(page, ref, bucketName, [fileName, 'docs/seed.txt']) + + await openMoveDialog(page, fileName) + + // The picker opens at the bucket root, which is where the file already is + await expect( + page.getByRole('button', { name: `Move to ${bucketName}` }), + 'Confirm button should be disabled while the destination matches the source' + ).toHaveAttribute('aria-disabled', 'true') + }) + + test('finds a nested folder by search and moves into it', async ({ page, ref }) => { + const bucketName = `${bucketNamePrefix}_search` + const fileName = 'needs-filing.txt' + + await setUpBucket(page, ref, bucketName, [fileName, 'reports/2024/q1/seed.txt']) + + const dialog = await openMoveDialog(page, fileName) + const folderList = dialog.getByTestId('folder-picker-list') + + await dialog.getByPlaceholder(`Search folders in ${bucketName}...`).fill('q1') + + // Search results name the folder and where it lives, since they span the whole bucket + const searchResult = folderList.getByRole('button', { name: 'q1 in reports/2024' }) + await expect(searchResult, 'Search should surface the deeply nested folder').toBeVisible() + await searchResult.click() + + await expect( + dialog.getByText(`Moving to ${bucketName}/reports/2024/q1`, { exact: true }), + 'Picking a search result should set it as the destination' + ).toBeVisible() + + await confirmMove(page, ref, 'q1') + + await expect( + page.getByText(/Successfully moved/), + 'A success toast should confirm the move' + ).toBeVisible() + await expect( + page.getByTitle(fileName), + 'File should no longer sit at the bucket root' + ).not.toBeVisible() + }) + + test('reports when a search matches no folders', async ({ page, ref }) => { + const bucketName = `${bucketNamePrefix}_no_results` + const fileName = 'stays-put.txt' + + await setUpBucket(page, ref, bucketName, [fileName, 'docs/seed.txt']) + + const dialog = await openMoveDialog(page, fileName) + const folderList = dialog.getByTestId('folder-picker-list') + + await dialog.getByPlaceholder(`Search folders in ${bucketName}...`).fill('nothing-matches-this') + + await expect( + folderList.getByText('No folders match "nothing-matches-this"'), + 'An empty search should say so rather than showing a blank list' + ).toBeVisible() + }) + + test('collapses the middle of a deep path into a breadcrumb dropdown', async ({ page, ref }) => { + const bucketName = `${bucketNamePrefix}_breadcrumb` + const fileName = 'deep-move.txt' + + await setUpBucket(page, ref, bucketName, [fileName, 'alpha/beta/gamma/seed.txt']) + + const dialog = await openMoveDialog(page, fileName) + const folderList = dialog.getByTestId('folder-picker-list') + + await folderList.getByRole('button', { name: 'alpha' }).click() + await folderList.getByRole('button', { name: 'beta' }).click() + await folderList.getByRole('button', { name: 'gamma' }).click() + + await expect( + dialog.getByText(`Moving to ${bucketName}/alpha/beta/gamma`, { exact: true }), + 'Destination should track the folders that were opened' + ).toBeVisible() + + // The bucket and the two deepest folders stay inline; "alpha" collapses + const breadcrumb = dialog.getByRole('navigation', { name: 'breadcrumb' }) + await expect( + breadcrumb.getByText('beta', { exact: true }), + 'The second-to-last folder should stay visible' + ).toBeVisible() + await expect( + breadcrumb.getByText('gamma', { exact: true }), + 'The current folder should stay visible' + ).toBeVisible() + await expect( + breadcrumb.getByText('alpha', { exact: true }), + 'The middle of the path should collapse out of the breadcrumb' + ).not.toBeVisible() + + await breadcrumb.getByRole('button', { name: 'Show the folders in between' }).click() + await page.getByRole('menuitem', { name: 'alpha' }).click() + + await expect( + dialog.getByText(`Moving to ${bucketName}/alpha`, { exact: true }), + 'Choosing a collapsed folder should navigate to it' + ).toBeVisible() + await expect( + folderList.getByRole('button', { name: 'beta' }), + 'Navigating back up should list the folder below it again' + ).toBeVisible() + }) + + test('walks back up the path with the up-one-level button', async ({ page, ref }) => { + const bucketName = `${bucketNamePrefix}_up_level` + const fileName = 'up-level.txt' + + await setUpBucket(page, ref, bucketName, [fileName, 'outer/inner/seed.txt']) + + const dialog = await openMoveDialog(page, fileName) + const folderList = dialog.getByTestId('folder-picker-list') + + const upOneLevel = dialog.getByRole('button', { name: 'Go up one level' }) + await expect(upOneLevel, 'Up-one-level should be disabled at the bucket root').toBeDisabled() + + await folderList.getByRole('button', { name: 'outer' }).click() + await folderList.getByRole('button', { name: 'inner' }).click() + await expect( + dialog.getByText(`Moving to ${bucketName}/outer/inner`, { exact: true }) + ).toBeVisible() + + await upOneLevel.click() + await expect( + dialog.getByText(`Moving to ${bucketName}/outer`, { exact: true }), + 'Going up one level should drop the deepest folder' + ).toBeVisible() + }) +}) diff --git a/e2e/studio/utils/storage-helpers.ts b/e2e/studio/utils/storage-helpers.ts index 1ab31eef214..5da6e9b56c6 100644 --- a/e2e/studio/utils/storage-helpers.ts +++ b/e2e/studio/utils/storage-helpers.ts @@ -278,3 +278,48 @@ export const deleteAllBuckets = async (page: Page, ref: string) => { } } } + +/** + * Opens the move dialog for a file from its row actions menu. + * + * @param page - Playwright page instance + * @param fileName - Name of the file to move + * @returns The move dialog locator, to scope assertions to the picker + */ +export const openMoveDialog = async (page: Page, fileName: string) => { + // Opened from the row's context menu rather than its actions button: the actions button sits at + // the row's right edge, where a top-right toast can cover it, while a right-click targets the + // row's center. Both menus are built from the same options. + const row = page.getByTitle(fileName) + await expect(row, `Row for ${fileName} should be visible`).toBeVisible() + await row.click({ button: 'right' }) + await page.getByRole('menuitem', { name: 'Move' }).click() + + const dialog = page.getByRole('dialog') + await expect(dialog, 'Move dialog should be visible').toBeVisible() + await expect(dialog.getByText(`Move ${fileName}`), 'Dialog should name the file').toBeVisible() + + return dialog +} + +/** + * Confirms the move dialog, waiting for the move request so the assertion that follows runs + * against a settled explorer. + * + * @param page - Playwright page instance + * @param ref - Project reference + * @param destinationName - Folder name shown on the confirm button (the bucket name at the root) + */ +export const confirmMove = async (page: Page, ref: string, destinationName: string) => { + // The confirm button stays focusable when disabled, so it reports aria-disabled rather than + // the native disabled property + const moveButton = page.getByRole('button', { name: `Move to ${destinationName}` }) + await expect(moveButton, `Move to ${destinationName} should be enabled`).not.toHaveAttribute( + 'aria-disabled', + 'true' + ) + + const movePromise = waitForApiResponse(page, 'storage', ref, 'objects/move', { method: 'POST' }) + await moveButton.click() + await movePromise +} diff --git a/e2e/studio/utils/storage/client.ts b/e2e/studio/utils/storage/client.ts index 208d6edfd0f..a5d4b0608c8 100644 --- a/e2e/studio/utils/storage/client.ts +++ b/e2e/studio/utils/storage/client.ts @@ -1,4 +1,4 @@ -import { env } from "../../env.config.js"; +import { env } from '../../env.config.js' /** * Make an HTTP request to the local Supabase Storage API. @@ -10,23 +10,32 @@ import { env } from "../../env.config.js"; */ export async function storageRequest( path: string, - options?: { method?: 'GET' | 'POST' | 'PUT' | 'DELETE'; body?: Record } + options?: { + method?: 'GET' | 'POST' | 'PUT' | 'DELETE' + /** A JSON payload, or a string to send as a raw `text/plain` body (for object uploads) */ + body?: Record | string + } ): Promise { const storageUrl = `${env.API_URL}/storage/v1` - + const headers: Record = { apikey: env.SERVICE_ROLE_KEY, Authorization: `Bearer ${env.SERVICE_ROLE_KEY}`, } - if (options?.body) { - headers['Content-Type'] = 'application/json' + const isRawBody = typeof options?.body === 'string' + if (options?.body !== undefined) { + headers['Content-Type'] = isRawBody ? 'text/plain' : 'application/json' } const response = await fetch(`${storageUrl}${path}`, { method: options?.method ?? 'GET', headers, - body: options?.body ? JSON.stringify(options.body) : undefined, + body: isRawBody + ? (options!.body as string) + : options?.body + ? JSON.stringify(options.body) + : undefined, }) if (!response.ok) { diff --git a/e2e/studio/utils/storage/index.ts b/e2e/studio/utils/storage/index.ts index cffa68bfd69..8cfe7877a06 100644 --- a/e2e/studio/utils/storage/index.ts +++ b/e2e/studio/utils/storage/index.ts @@ -1,2 +1,9 @@ export { storageRequest } from './client.js' -export { createBucket, deleteBucket, deleteAllBuckets, listBuckets } from './queries.js' +export { + createBucket, + deleteBucket, + deleteAllBuckets, + listBuckets, + seedBucket, + uploadObject, +} from './queries.js' diff --git a/e2e/studio/utils/storage/queries.ts b/e2e/studio/utils/storage/queries.ts index e518b8a3381..38e6742c647 100644 --- a/e2e/studio/utils/storage/queries.ts +++ b/e2e/studio/utils/storage/queries.ts @@ -52,3 +52,33 @@ export async function deleteAllBuckets(): Promise { await deleteBucket(bucket.id) } } + +/** + * Uploads an object to a bucket, creating every folder in its path along the way. Storage has no + * standalone folders — a folder exists because an object sits under that prefix — so this is how + * a folder tree gets seeded. + * + * @param bucket - Bucket name / id + * @param objectPath - Path within the bucket, e.g. `reports/2024/q1/seed.txt` + * @param content - File contents (default: a short placeholder) + */ +export async function uploadObject( + bucket: string, + objectPath: string, + content: string = 'e2e fixture' +): Promise { + await storageRequest(`/object/${bucket}/${objectPath}`, { method: 'POST', body: content }) +} + +/** + * Seeds a bucket with a set of object paths. Creates the bucket first when it does not exist. + * + * @param bucket - Bucket name / id + * @param objectPaths - Paths within the bucket to create + */ +export async function seedBucket(bucket: string, objectPaths: string[]): Promise { + await createBucket(bucket, false) + for (const objectPath of objectPaths) { + await uploadObject(bucket, objectPath) + } +}