From c89b9d72361cc1e8f9ed1e80baf06b9c50ea2c18 Mon Sep 17 00:00:00 2001 From: trholdridge <37459148+trholdridge@users.noreply.github.com> Date: Fri, 20 Dec 2024 09:44:31 -0500 Subject: [PATCH] [CORE-112] Show folders in main file view in file browser (#5207) --- .../file-browser/DirectoryTree.test.ts | 61 ++++ src/components/file-browser/DirectoryTree.ts | 11 +- .../file-browser/FileBrowser.test.ts | 40 ++- src/components/file-browser/FileBrowser.ts | 3 + .../file-browser/FilesInDirectory.test.ts | 294 +++++++++++++++++- .../file-browser/FilesInDirectory.ts | 74 +++-- .../file-browser/FilesTable.test.ts | 93 ++++-- src/components/file-browser/FilesTable.ts | 56 +++- .../GCSFileBrowserProvider.test.ts | 24 +- .../GCSFileBrowserProvider.ts | 11 + src/workflows-app/SubmissionDetails.test.js | 35 ++- 11 files changed, 592 insertions(+), 110 deletions(-) diff --git a/src/components/file-browser/DirectoryTree.test.ts b/src/components/file-browser/DirectoryTree.test.ts index 82cc439008..a6588d3174 100644 --- a/src/components/file-browser/DirectoryTree.test.ts +++ b/src/components/file-browser/DirectoryTree.test.ts @@ -91,6 +91,67 @@ describe('Directory', () => { expect(renderedSubdirectories).toEqual(['directory1', 'directory2', 'directory3']); }); + it('fetches and renders contents for parent directory of selected directory', () => { + // Arrange + const directoriesIn = (path) => { + switch (path) { + case 'directory1/': + return [ + { + path: 'directory1/test1', + }, + { + path: 'directory1/test2', + }, + ]; + case 'directory1/test1/': + return [ + { + path: 'directory1/test1/abc/', + }, + { + path: 'directory1/test1/xyz/', + }, + ]; + default: + return []; + } + }; + + asMockedFn(useDirectoriesInDirectory).mockImplementation((_, path) => { + return { + state: { directories: directoriesIn(path), status: 'Ready' }, + hasNextPage: undefined, + loadNextPage: () => Promise.resolve(), + loadAllRemainingItems: () => Promise.resolve(), + reload: () => Promise.resolve(), + }; + }); + + render( + ul({ role: 'tree' }, [ + h(Directory, { + activeDescendant: 'node-0', + id: 'node-0', + level: 0, + path: 'directory1/', + provider: mockFileBrowserProvider, + reloadRequests: subscribable(), + rootLabel: 'Workspace bucket', + selectedDirectory: 'directory1/test1/abc/', + setActiveDescendant: () => {}, + onError: () => {}, + onSelectDirectory: jest.fn(), + }), + ]) + ); + + // Assert + const subdirectoryList = screen.getByLabelText('directory1 subdirectories'); + const renderedSubdirectories = Array.from(subdirectoryList.children).map((el) => el.children[1].textContent); + expect(renderedSubdirectories).toEqual(['test1', 'test2']); + }); + describe('when directory name is clicked', () => { let onSelectDirectory; diff --git a/src/components/file-browser/DirectoryTree.ts b/src/components/file-browser/DirectoryTree.ts index 5de90cece2..9e742d786c 100644 --- a/src/components/file-browser/DirectoryTree.ts +++ b/src/components/file-browser/DirectoryTree.ts @@ -218,10 +218,17 @@ export const Directory = (props: DirectoryProps) => { } = props; const isSelected = path === selectedDirectory; - // Automatically expand root directory. - const [isExpanded, setIsExpanded] = useState(path === ''); + const [isExpanded, setIsExpanded] = useState(false); const [hasLoadedContents, setHasLoadedContents] = useState(false); + useEffect(() => { + // Expand the selected directory and its parents + const isSelectedOrParent = path === selectedDirectory.substring(0, path.length); + if (isSelectedOrParent) { + setIsExpanded(true); + } + }, [path, selectedDirectory]); + return li( { 'aria-expanded': isExpanded, diff --git a/src/components/file-browser/FileBrowser.test.ts b/src/components/file-browser/FileBrowser.test.ts index 52c29b4122..a829f395ce 100644 --- a/src/components/file-browser/FileBrowser.test.ts +++ b/src/components/file-browser/FileBrowser.test.ts @@ -1,13 +1,17 @@ import { h } from 'react-hyperscript-helpers'; -import { useFilesInDirectory } from 'src/components/file-browser/file-browser-hooks'; +import { useDirectoriesInDirectory, useFilesInDirectory } from 'src/components/file-browser/file-browser-hooks'; import FileBrowser from 'src/components/file-browser/FileBrowser'; import FilesTable from 'src/components/file-browser/FilesTable'; -import FileBrowserProvider, { FileBrowserFile } from 'src/libs/ajax/file-browser-providers/FileBrowserProvider'; +import FileBrowserProvider, { + FileBrowserDirectory, + FileBrowserFile, +} from 'src/libs/ajax/file-browser-providers/FileBrowserProvider'; import { asMockedFn, renderWithAppContexts as render } from 'src/testing/test-utils'; import { RequesterPaysModal } from 'src/workspaces/common/requester-pays/RequesterPaysModal'; jest.mock('src/components/file-browser/file-browser-hooks', () => ({ ...jest.requireActual('src/components/file-browser/file-browser-hooks'), + useDirectoriesInDirectory: jest.fn(), useFilesInDirectory: jest.fn(), })); @@ -29,6 +33,7 @@ jest.mock('src/workspaces/common/requester-pays/RequesterPaysModal', (): Request }; }); +type UseDirectoriesInDirectoryResult = ReturnType; type UseFilesInDirectoryResult = ReturnType; describe('FileBrowser', () => { @@ -36,6 +41,12 @@ describe('FileBrowser', () => { it('renders files', () => { // Arrange + const directories: FileBrowserDirectory[] = [ + { + path: 'path/to/folder/', + }, + ]; + const files: FileBrowserFile[] = [ { path: 'file.txt', @@ -47,6 +58,14 @@ describe('FileBrowser', () => { }, ]; + const useDirectoriesInDirectoryResult: UseDirectoriesInDirectoryResult = { + state: { directories, status: 'Ready' }, + hasNextPage: false, + loadNextPage: () => Promise.resolve(), + loadAllRemainingItems: () => Promise.resolve(), + reload: () => Promise.resolve(), + }; + const useFilesInDirectoryResult: UseFilesInDirectoryResult = { state: { files, status: 'Ready' }, hasNextPage: false, @@ -55,6 +74,7 @@ describe('FileBrowser', () => { reload: () => Promise.resolve(), }; + asMockedFn(useDirectoriesInDirectory).mockReturnValue(useDirectoriesInDirectoryResult); asMockedFn(useFilesInDirectory).mockReturnValue(useFilesInDirectoryResult); // Act @@ -71,21 +91,7 @@ describe('FileBrowser', () => { ); // Assert - expect(FilesTable).toHaveBeenCalledWith( - expect.objectContaining({ - files: [ - { - path: 'file.txt', - url: 'gs://test-bucket/file.txt', - contentType: 'text/plain', - size: 1024, - createdAt: 1667408400000, - updatedAt: 1667408400000, - }, - ], - }), - expect.anything() - ); + expect(FilesTable).toHaveBeenCalledWith(expect.objectContaining({ directories, files }), expect.anything()); }); it('prompts to select workspace on requester pays errors', () => { diff --git a/src/components/file-browser/FileBrowser.ts b/src/components/file-browser/FileBrowser.ts index a2f211d913..801ae833d0 100644 --- a/src/components/file-browser/FileBrowser.ts +++ b/src/components/file-browser/FileBrowser.ts @@ -161,6 +161,9 @@ const FileBrowser = (props: FileBrowserProps) => { rootLabel, selectedFiles, setSelectedFiles, + onClickDirectory: (directory: FileBrowserDirectory) => { + setPath(directory.path); + }, onClickFile: setFocusedFile, onCreateDirectory: (directory: FileBrowserDirectory) => { setPath(directory.path); diff --git a/src/components/file-browser/FilesInDirectory.test.ts b/src/components/file-browser/FilesInDirectory.test.ts index df7f56824b..d84fe5e054 100644 --- a/src/components/file-browser/FilesInDirectory.test.ts +++ b/src/components/file-browser/FilesInDirectory.test.ts @@ -2,15 +2,19 @@ import { controlledPromise } from '@terra-ui-packages/core-utils'; import { act, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { h } from 'react-hyperscript-helpers'; -import { useFilesInDirectory } from 'src/components/file-browser/file-browser-hooks'; +import { useDirectoriesInDirectory, useFilesInDirectory } from 'src/components/file-browser/file-browser-hooks'; import FilesInDirectory from 'src/components/file-browser/FilesInDirectory'; import FilesTable from 'src/components/file-browser/FilesTable'; -import FileBrowserProvider, { FileBrowserFile } from 'src/libs/ajax/file-browser-providers/FileBrowserProvider'; +import FileBrowserProvider, { + FileBrowserDirectory, + FileBrowserFile, +} from 'src/libs/ajax/file-browser-providers/FileBrowserProvider'; import { notify } from 'src/libs/notifications'; import { asMockedFn, renderWithAppContexts as render } from 'src/testing/test-utils'; jest.mock('src/components/file-browser/file-browser-hooks', () => ({ ...jest.requireActual('src/components/file-browser/file-browser-hooks'), + useDirectoriesInDirectory: jest.fn(), useFilesInDirectory: jest.fn(), })); @@ -39,6 +43,7 @@ jest.mock('src/libs/notifications', (): NotificationsExports => { }; }); +type UseDirectoriesInDirectoryResult = ReturnType; type UseFilesInDirectoryResult = ReturnType; // modals, popups, tooltips, etc. render into this element. @@ -55,8 +60,16 @@ afterAll(() => { describe('FilesInDirectory', () => { const mockFileBrowserProvider: FileBrowserProvider = {} as FileBrowserProvider; - it('loads files in the given path', () => { + it('loads files and directories in the given path', () => { // Arrange + const useDirectoriesInDirectoryResult: UseDirectoriesInDirectoryResult = { + state: { directories: [], status: 'Ready' }, + hasNextPage: false, + loadNextPage: () => Promise.resolve(), + loadAllRemainingItems: () => Promise.resolve(), + reload: () => Promise.resolve(), + }; + const useFilesInDirectoryResult: UseFilesInDirectoryResult = { state: { files: [], status: 'Loading' }, hasNextPage: undefined, @@ -65,6 +78,7 @@ describe('FilesInDirectory', () => { reload: () => Promise.resolve(), }; + asMockedFn(useDirectoriesInDirectory).mockReturnValue(useDirectoriesInDirectoryResult); asMockedFn(useFilesInDirectory).mockReturnValue(useFilesInDirectoryResult); // Act @@ -74,6 +88,7 @@ describe('FilesInDirectory', () => { path: 'path/to/directory/', selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), onClickFile: jest.fn(), onCreateDirectory: () => {}, onDeleteDirectory: () => {}, @@ -82,11 +97,18 @@ describe('FilesInDirectory', () => { ); // Assert + expect(asMockedFn(useDirectoriesInDirectory)).toHaveBeenCalledWith(mockFileBrowserProvider, 'path/to/directory/'); expect(asMockedFn(useFilesInDirectory)).toHaveBeenCalledWith(mockFileBrowserProvider, 'path/to/directory/'); }); - it('renders FilesTable with loaded files', () => { + it('renders FilesTable with loaded files and directories', () => { // Arrange + const directories: FileBrowserDirectory[] = [ + { + path: 'path/to/folder/', + }, + ]; + const files: FileBrowserFile[] = [ { path: 'path/to/file.txt', @@ -98,6 +120,14 @@ describe('FilesInDirectory', () => { }, ]; + const useDirectoriesInDirectoryResult: UseDirectoriesInDirectoryResult = { + state: { directories, status: 'Ready' }, + hasNextPage: false, + loadNextPage: () => Promise.resolve(), + loadAllRemainingItems: () => Promise.resolve(), + reload: () => Promise.resolve(), + }; + const useFilesInDirectoryResult: UseFilesInDirectoryResult = { state: { files, status: 'Ready' }, hasNextPage: undefined, @@ -106,6 +136,7 @@ describe('FilesInDirectory', () => { reload: () => Promise.resolve(), }; + asMockedFn(useDirectoriesInDirectory).mockReturnValue(useDirectoriesInDirectoryResult); asMockedFn(useFilesInDirectory).mockReturnValue(useFilesInDirectoryResult); // Act @@ -115,6 +146,7 @@ describe('FilesInDirectory', () => { path: 'path/to/directory/', selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), onClickFile: jest.fn(), onCreateDirectory: () => {}, onDeleteDirectory: () => {}, @@ -123,20 +155,86 @@ describe('FilesInDirectory', () => { ); // Assert - expect(FilesTable).toHaveBeenCalledWith(expect.objectContaining({ files }), expect.anything()); + expect(FilesTable).toHaveBeenCalledWith(expect.objectContaining({ directories, files }), expect.anything()); + }); + + it('renders FilesTable with only directories if another directories page is available', () => { + // Arrange + const directories: FileBrowserDirectory[] = [ + { + path: 'path/to/folder/', + }, + ]; + + const files: FileBrowserFile[] = [ + { + path: 'path/to/file.txt', + url: 'gs://test-bucket/path/to/file.txt', + contentType: 'text/plain', + size: 1024, + createdAt: 1667408400000, + updatedAt: 1667408400000, + }, + ]; + + const useDirectoriesInDirectoryResult: UseDirectoriesInDirectoryResult = { + state: { directories, status: 'Ready' }, + hasNextPage: true, + loadNextPage: () => Promise.resolve(), + loadAllRemainingItems: () => Promise.resolve(), + reload: () => Promise.resolve(), + }; + + const useFilesInDirectoryResult: UseFilesInDirectoryResult = { + state: { files, status: 'Ready' }, + hasNextPage: false, + loadNextPage: () => Promise.resolve(), + loadAllRemainingItems: () => Promise.resolve(), + reload: () => Promise.resolve(), + }; + + asMockedFn(useDirectoriesInDirectory).mockReturnValue(useDirectoriesInDirectoryResult); + asMockedFn(useFilesInDirectory).mockReturnValue(useFilesInDirectoryResult); + + // Act + render( + h(FilesInDirectory, { + provider: mockFileBrowserProvider, + path: 'path/to/directory/', + selectedFiles: {}, + setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), + onClickFile: jest.fn(), + onCreateDirectory: () => {}, + onDeleteDirectory: () => {}, + onError: () => {}, + }) + ); + + // Assert + expect(FilesTable).toHaveBeenCalledWith(expect.objectContaining({ directories }), expect.anything()); + expect(FilesTable).not.toHaveBeenCalledWith(expect.objectContaining({ files }), expect.anything()); }); it.each([ - { state: { status: 'Loading', files: [] }, expectedMessage: 'Loading files...' }, + { state: { status: 'Loading', files: [] }, expectedMessage: 'Loading...' }, { state: { status: 'Ready', files: [] }, expectedMessage: 'No files have been uploaded yet' }, { state: { status: 'Error', error: new Error('Something went wrong'), files: [] }, - expectedMessage: 'Unable to load files', + expectedMessage: 'Unable to load', }, ] as { state: UseFilesInDirectoryResult['state']; expectedMessage: string }[])( - 'renders a message based on loading state ($state.status) when no files are present', + 'renders a message based on loading state ($state.status) when no files or directories are present', ({ state, expectedMessage }) => { // Arrange + const useDirectoriesInDirectoryResult: UseDirectoriesInDirectoryResult = { + state: { directories: [], status: 'Ready' }, + hasNextPage: false, + loadNextPage: () => Promise.resolve(), + loadAllRemainingItems: () => Promise.resolve(), + reload: () => Promise.resolve(), + }; + const useFilesInDirectoryResult: UseFilesInDirectoryResult = { state, hasNextPage: false, @@ -145,6 +243,7 @@ describe('FilesInDirectory', () => { reload: () => Promise.resolve(), }; + asMockedFn(useDirectoriesInDirectory).mockReturnValue(useDirectoriesInDirectoryResult); asMockedFn(useFilesInDirectory).mockReturnValue(useFilesInDirectoryResult); // Act @@ -154,6 +253,7 @@ describe('FilesInDirectory', () => { path: 'path/to/directory/', selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), onClickFile: jest.fn(), onCreateDirectory: () => {}, onDeleteDirectory: () => {}, @@ -187,6 +287,37 @@ describe('FilesInDirectory', () => { path: 'path/to/directory/', selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), + onClickFile: jest.fn(), + onCreateDirectory: () => {}, + onDeleteDirectory: () => {}, + onError, + }) + ); + }); + + it('calls onError callback on errors loading directories', () => { + // Arrange + const useDirectoriesInDirectoryResult: UseDirectoriesInDirectoryResult = { + state: { status: 'Error', error: new Error('Something went wrong'), directories: [] }, + hasNextPage: false, + loadNextPage: () => Promise.resolve(), + loadAllRemainingItems: () => Promise.resolve(), + reload: () => Promise.resolve(), + }; + + asMockedFn(useDirectoriesInDirectory).mockReturnValue(useDirectoriesInDirectoryResult); + + const onError = jest.fn(); + + // Act + render( + h(FilesInDirectory, { + provider: mockFileBrowserProvider, + path: 'path/to/directory/', + selectedFiles: {}, + setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), onClickFile: jest.fn(), onCreateDirectory: () => {}, onDeleteDirectory: () => {}, @@ -198,7 +329,105 @@ describe('FilesInDirectory', () => { expect(onError).toHaveBeenCalledWith(new Error('Something went wrong')); }); - describe('when next page is available', () => { + describe('when next directories page is available', () => { + // Arrange + const loadNextDirectoriesPage = jest.fn(); + const loadAllRemainingDirectoryItems = jest.fn(); + const loadNextFilesPage = jest.fn(); + const loadAllRemainingFileItems = jest.fn(); + + const directories: FileBrowserDirectory[] = [ + { + path: 'path/to/folder/', + }, + ]; + + const files: FileBrowserFile[] = [ + { + path: 'path/to/file.txt', + url: 'gs://test-bucket/path/to/file.txt', + contentType: 'text/plain', + size: 1024, + createdAt: 1667408400000, + updatedAt: 1667408400000, + }, + ]; + + const useDirectoriesInDirectoryResult: UseDirectoriesInDirectoryResult = { + state: { directories, status: 'Ready' }, + hasNextPage: true, + loadNextPage: loadNextDirectoriesPage, + loadAllRemainingItems: loadAllRemainingDirectoryItems, + reload: () => Promise.resolve(), + }; + + const useFilesInDirectoryResult: UseFilesInDirectoryResult = { + state: { files, status: 'Ready' }, + hasNextPage: false, + loadNextPage: loadNextFilesPage, + loadAllRemainingItems: loadAllRemainingFileItems, + reload: () => Promise.resolve(), + }; + + beforeEach(() => { + asMockedFn(useDirectoriesInDirectory).mockReturnValue(useDirectoriesInDirectoryResult); + asMockedFn(useFilesInDirectory).mockReturnValue(useFilesInDirectoryResult); + }); + + it('renders a button to load next page', async () => { + // Arrange + const user = userEvent.setup(); + + // Act + render( + h(FilesInDirectory, { + provider: mockFileBrowserProvider, + path: 'path/to/directory/', + selectedFiles: {}, + setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), + onClickFile: jest.fn(), + onCreateDirectory: () => {}, + onDeleteDirectory: () => {}, + onError: () => {}, + }) + ); + + // Assert + const loadNextPageButton = screen.getByText('Load next page'); + await user.click(loadNextPageButton); + expect(loadNextDirectoriesPage).toHaveBeenCalled(); + expect(loadNextFilesPage).not.toHaveBeenCalled(); + }); + + it('renders a button to load all remaining pages', async () => { + // Arrange + const user = userEvent.setup(); + + // Act + render( + h(FilesInDirectory, { + provider: mockFileBrowserProvider, + path: 'path/to/directory/', + selectedFiles: {}, + setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), + onClickFile: jest.fn(), + onCreateDirectory: () => {}, + onDeleteDirectory: () => {}, + onError: () => {}, + }) + ); + + // Assert + const loadAllPagesButton = screen.getByText('Load all'); + await user.click(loadAllPagesButton); + expect(loadAllRemainingDirectoryItems).toHaveBeenCalled(); + expect(loadAllRemainingFileItems).toHaveBeenCalled(); + }); + }); + + describe('when next files page is available', () => { // Arrange const loadNextPage = jest.fn(); const loadAllRemainingItems = jest.fn(); @@ -214,6 +443,14 @@ describe('FilesInDirectory', () => { }, ]; + const useDirectoriesInDirectoryResult: UseDirectoriesInDirectoryResult = { + state: { directories: [], status: 'Ready' }, + hasNextPage: false, + loadNextPage: () => Promise.resolve(), + loadAllRemainingItems: () => Promise.resolve(), + reload: () => Promise.resolve(), + }; + const useFilesInDirectoryResult: UseFilesInDirectoryResult = { state: { files, status: 'Ready' }, hasNextPage: true, @@ -223,6 +460,7 @@ describe('FilesInDirectory', () => { }; beforeEach(() => { + asMockedFn(useDirectoriesInDirectory).mockReturnValue(useDirectoriesInDirectoryResult); asMockedFn(useFilesInDirectory).mockReturnValue(useFilesInDirectoryResult); }); @@ -237,6 +475,7 @@ describe('FilesInDirectory', () => { path: 'path/to/directory/', selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), onClickFile: jest.fn(), onCreateDirectory: () => {}, onDeleteDirectory: () => {}, @@ -261,6 +500,7 @@ describe('FilesInDirectory', () => { path: 'path/to/directory/', selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), onClickFile: jest.fn(), onCreateDirectory: () => {}, onDeleteDirectory: () => {}, @@ -298,6 +538,7 @@ describe('FilesInDirectory', () => { path: 'path/to/directory/', selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), onClickFile: jest.fn(), onCreateDirectory: () => {}, onDeleteDirectory: () => {}, @@ -339,6 +580,7 @@ describe('FilesInDirectory', () => { path: 'path/to/directory/', selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), onClickFile: jest.fn(), onCreateDirectory: () => {}, onDeleteDirectory: () => {}, @@ -378,14 +620,23 @@ describe('FilesInDirectory', () => { const onDeleteDirectory = jest.fn(); + const useDirectoriesInDirectoryResult: UseDirectoriesInDirectoryResult = { + state: { directories: [], status: 'Ready' }, + hasNextPage: false, + loadNextPage: () => Promise.resolve(), + loadAllRemainingItems: () => Promise.resolve(), + reload: () => Promise.resolve(), + }; + const useFilesInDirectoryResult: UseFilesInDirectoryResult = { - state: { status: 'Ready', files: [] }, + state: { files: [], status: 'Ready' }, hasNextPage: false, loadNextPage: () => Promise.resolve(), loadAllRemainingItems: () => Promise.resolve(), reload: () => Promise.resolve(), }; + asMockedFn(useDirectoriesInDirectory).mockReturnValue(useDirectoriesInDirectoryResult); asMockedFn(useFilesInDirectory).mockReturnValue(useFilesInDirectoryResult); render( @@ -394,6 +645,7 @@ describe('FilesInDirectory', () => { path: 'path/to/directory/', selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), onClickFile: jest.fn(), onCreateDirectory: () => {}, onDeleteDirectory, @@ -428,6 +680,14 @@ describe('FilesInDirectory', () => { const moveFile = jest.fn(() => Promise.resolve()); const mockProvider = { moveFile } as Partial as FileBrowserProvider; + const useDirectoriesInDirectoryResult: UseDirectoriesInDirectoryResult = { + state: { directories: [], status: 'Ready' }, + hasNextPage: false, + loadNextPage: () => Promise.resolve(), + loadAllRemainingItems: () => Promise.resolve(), + reload: () => Promise.resolve(), + }; + const useFilesInDirectoryResult: UseFilesInDirectoryResult = { state: { status: 'Ready', files }, hasNextPage: false, @@ -436,6 +696,7 @@ describe('FilesInDirectory', () => { reload: () => Promise.resolve(), }; + asMockedFn(useDirectoriesInDirectory).mockReturnValue(useDirectoriesInDirectoryResult); asMockedFn(useFilesInDirectory).mockReturnValue(useFilesInDirectoryResult); render( @@ -444,6 +705,7 @@ describe('FilesInDirectory', () => { path: 'path/to/directory/', selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), onClickFile: jest.fn(), onCreateDirectory: () => {}, onDeleteDirectory: () => {}, @@ -474,6 +736,15 @@ describe('FilesInDirectory', () => { const reload = jest.fn().mockReturnValue(reloadPromise); const mockProvider = {} as Partial as FileBrowserProvider; + + const useDirectoriesInDirectoryResult: UseDirectoriesInDirectoryResult = { + state: { directories: [], status: 'Ready' }, + hasNextPage: false, + loadNextPage: () => Promise.resolve(), + loadAllRemainingItems: () => Promise.resolve(), + reload: () => Promise.resolve(), + }; + const useFilesInDirectoryResult: UseFilesInDirectoryResult = { state: { status: 'Ready', files: [] }, hasNextPage: false, @@ -481,6 +752,8 @@ describe('FilesInDirectory', () => { loadAllRemainingItems: () => Promise.resolve(), reload, }; + + asMockedFn(useDirectoriesInDirectory).mockReturnValue(useDirectoriesInDirectoryResult); asMockedFn(useFilesInDirectory).mockReturnValue(useFilesInDirectoryResult); render( @@ -489,6 +762,7 @@ describe('FilesInDirectory', () => { path: 'path/to/directory/', selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: () => {}, onClickFile: () => {}, onCreateDirectory: () => {}, onDeleteDirectory: () => {}, diff --git a/src/components/file-browser/FilesInDirectory.ts b/src/components/file-browser/FilesInDirectory.ts index d396c2e94e..7a332adbed 100644 --- a/src/components/file-browser/FilesInDirectory.ts +++ b/src/components/file-browser/FilesInDirectory.ts @@ -2,7 +2,7 @@ import { Dispatch, Fragment, ReactNode, SetStateAction, useEffect, useRef, useSt import { div, h, span } from 'react-hyperscript-helpers'; import { ButtonOutline, Link, topSpinnerOverlay } from 'src/components/common'; import Dropzone from 'src/components/Dropzone'; -import { useFilesInDirectory } from 'src/components/file-browser/file-browser-hooks'; +import { useDirectoriesInDirectory, useFilesInDirectory } from 'src/components/file-browser/file-browser-hooks'; import { basename, dirname } from 'src/components/file-browser/file-browser-utils'; import { FilesMenu } from 'src/components/file-browser/FilesMenu'; import FilesTable from 'src/components/file-browser/FilesTable'; @@ -29,6 +29,7 @@ interface FilesInDirectoryProps { rootLabel?: string; selectedFiles: { [path: string]: FileBrowserFile }; setSelectedFiles: Dispatch>; + onClickDirectory: (directory: FileBrowserDirectory) => void; onClickFile: (file: FileBrowserFile) => void; onCreateDirectory: (directory: FileBrowserDirectory) => void; onDeleteDirectory: () => void; @@ -36,6 +37,8 @@ interface FilesInDirectoryProps { extraMenuItems?: any; } +type FileObjectStatus = 'Loading' | 'Ready' | 'Error'; + const FilesInDirectory = (props: FilesInDirectoryProps) => { const { editDisabled = false, @@ -45,6 +48,7 @@ const FilesInDirectory = (props: FilesInDirectoryProps) => { rootLabel = 'Files', selectedFiles, setSelectedFiles, + onClickDirectory, onClickFile, onCreateDirectory, onDeleteDirectory, @@ -56,13 +60,19 @@ const FilesInDirectory = (props: FilesInDirectoryProps) => { const loadedAlertElementRef = useRef(null); - const { state, hasNextPage, loadAllRemainingItems, loadNextPage, reload } = useFilesInDirectory(provider, path); + const directoriesResults = useDirectoriesInDirectory(provider, path); + const filesResults = useFilesInDirectory(provider, path); + const loadNextPage = + directoriesResults.hasNextPage !== false ? directoriesResults.loadNextPage : filesResults.loadNextPage; useEffect(() => { - if (state.status === 'Error') { - onError(state.error); + if (filesResults.state.status === 'Error') { + onError(filesResults.state.error); + } + if (directoriesResults.state.status === 'Error') { + onError(directoriesResults.state.error); } - }, [state, onError]); + }, [filesResults.state, directoriesResults.state, onError]); const { uploadState, uploadFiles, cancelUpload } = useUploader((file) => { return provider.uploadFileToDirectory(path, file); @@ -77,16 +87,30 @@ const FilesInDirectory = (props: FilesInDirectoryProps) => { }); }); - const { status, files } = state; + const statusFrom = (fs: FileObjectStatus, ds: FileObjectStatus): FileObjectStatus => { + if (fs === ds) { + return fs; + } + if (fs === 'Error' || ds === 'Error') { + return 'Error'; + } + return 'Loading'; + }; + + const { status: directoriesStatus, directories } = directoriesResults.state; + const { status: filesStatus, files } = filesResults.state; + const status = statusFrom(filesStatus, directoriesStatus); + const filesToShow = directoriesResults.hasNextPage === false ? files.length : 0; + const isLoading = status === 'Loading'; useEffect(() => { loadedAlertElementRef.current!.innerHTML = { Loading: '', - Ready: `Loaded ${files.length} files in ${directoryLabel}`, - Error: `Error loading files in ${directoryLabel}`, + Ready: `Loaded ${directories.length} directories and ${filesToShow} files in ${directoryLabel}`, + Error: `Error loading files or directories in ${directoryLabel}`, }[status]; - }, [directoryLabel, files, status]); + }, [directories, directoryLabel, filesToShow, status]); const [renamingFile, setRenamingFile] = useState(); @@ -111,7 +135,7 @@ const FilesInDirectory = (props: FilesInDirectoryProps) => { maxFiles: 0, // no limit on number of files onDropAccepted: async (files) => { await uploadFiles(files); - reload(); + filesResults.reload(); }, }, [ @@ -127,9 +151,9 @@ const FilesInDirectory = (props: FilesInDirectoryProps) => { onCreateDirectory, onDeleteFiles: () => { setSelectedFiles({}); - reload(); + filesResults.reload(); }, - onRefresh: reload, + onRefresh: filesResults.reload, extraMenuItems, }), @@ -153,18 +177,20 @@ const FilesInDirectory = (props: FilesInDirectoryProps) => { className: 'sr-only', role: 'alert', }, - [`Loading files in ${directoryLabel}`] + [`Loading files and directories in ${directoryLabel}`] ), - files.length > 0 && + directories.length + filesToShow > 0 && h(Fragment, [ h(FilesTable, { 'aria-label': `Files in ${directoryLabel}`, editDisabled, editDisabledReason, - files, + files: files.slice(0, filesToShow), + directories, selectedFiles, setSelectedFiles, + onClickDirectory, onClickFile, onRenameFile: setRenamingFile, }), @@ -180,10 +206,10 @@ const FilesInDirectory = (props: FilesInDirectoryProps) => { }, [ div([ - `${files.length} files `, + `${directories.length} directories and ${filesToShow} files `, isLoading && h(Fragment, ['Loading more... ', icon('loadingSpinner', { size: 12 })]), ]), - hasNextPage !== false && + (directoriesResults.hasNextPage !== false || filesResults.hasNextPage !== false) && div([ h( Link, @@ -200,7 +226,10 @@ const FilesInDirectory = (props: FilesInDirectoryProps) => { disabled: isLoading, style: { marginLeft: '1ch' }, tooltip: 'This may take a long time for folders containing several thousand objects.', - onClick: () => loadAllRemainingItems(), + onClick: () => { + directoriesResults.loadAllRemainingItems(); + filesResults.loadAllRemainingItems(); + }, }, ['Load all'] ), @@ -208,7 +237,8 @@ const FilesInDirectory = (props: FilesInDirectoryProps) => { ] ), ]), - files.length === 0 && + + directories.length + filesToShow === 0 && div( { style: { @@ -219,8 +249,8 @@ const FilesInDirectory = (props: FilesInDirectoryProps) => { }, [ Utils.cond( - [status === 'Loading', () => 'Loading files...'], - [status === 'Error', () => 'Unable to load files'], + [status === 'Loading', () => 'Loading...'], + [status === 'Error', () => 'Unable to load'], () => h(Fragment, [ div(['No files have been uploaded yet']), @@ -270,7 +300,7 @@ const FilesInDirectory = (props: FilesInDirectoryProps) => { const destinationPath = `${dirname(renamingFile.path)}${name}`; try { await provider.moveFile(renamingFile.path, destinationPath); - reload(); + filesResults.reload(); } catch (error) { reportError('Error renaming file', error); } finally { diff --git a/src/components/file-browser/FilesTable.test.ts b/src/components/file-browser/FilesTable.test.ts index 408816c810..ab75da1f03 100644 --- a/src/components/file-browser/FilesTable.test.ts +++ b/src/components/file-browser/FilesTable.test.ts @@ -5,7 +5,7 @@ import { useState } from 'react'; import { h } from 'react-hyperscript-helpers'; import { basename } from 'src/components/file-browser/file-browser-utils'; import FilesTable, { FilesTableProps } from 'src/components/file-browser/FilesTable'; -import { FileBrowserFile } from 'src/libs/ajax/file-browser-providers/FileBrowserProvider'; +import { FileBrowserDirectory, FileBrowserFile } from 'src/libs/ajax/file-browser-providers/FileBrowserProvider'; import { renderWithAppContexts as render } from 'src/testing/test-utils'; type ClipboardPolyfillExports = typeof import('clipboard-polyfill/text'); @@ -25,6 +25,15 @@ jest.mock('react-virtualized', () => ({ })); describe('FilesTable', () => { + const directories: FileBrowserDirectory[] = [ + { + path: 'path/to/folder1/', + }, + { + path: 'path/to/folder2/', + }, + ]; + const files: FileBrowserFile[] = [ { path: 'path/to/file1.txt', @@ -52,48 +61,49 @@ describe('FilesTable', () => { }, ]; - it.each([ - { field: 'size', columnIndex: 2, expected: ['1 KB', '1 MB', '1 GB'] }, - // { field: 'last modified', columnIndex: 2, expected: [] } - ])('renders a column with file $field', ({ columnIndex, expected }) => { + it('renders a column with size information', () => { // Act render( h(FilesTable, { + directories, files, selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), onClickFile: jest.fn(), onRenameFile: () => {}, }) ); const tableRows = screen.getAllByRole('row').slice(1); // skip header row - const cellsInColumn = tableRows.map((row) => getAllByRole(row, 'cell')[columnIndex]); + const cellsInColumn = tableRows.map((row) => getAllByRole(row, 'cell')[2]); const valuesInColumn = cellsInColumn.map((cell) => cell.textContent); // Assert - expect(valuesInColumn).toEqual(expected); + expect(valuesInColumn).toEqual(['--', '--', '1 KB', '1 MB', '1 GB']); }); it('renders file names as links', () => { // Act render( h(FilesTable, { + directories, files, selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), onClickFile: jest.fn(), onRenameFile: () => {}, }) ); - const tableRows = screen.getAllByRole('row').slice(1); // skip header row - const fileNameCells = tableRows.map((row) => getAllByRole(row, 'cell')[1]); - const fileLinks = fileNameCells.map((cell) => getByRole(cell, 'link')); + const tableRows = screen.getAllByRole('row').slice(3); // skip header row and directories + const nameCells = tableRows.map((row) => getAllByRole(row, 'cell')[1]); + const links = nameCells.map((cell) => getByRole(cell, 'link')); // Assert - expect(fileLinks.map((link) => link.textContent)).toEqual(['file1.txt', 'file2.bam', 'file3.vcf']); - expect(fileLinks.map((link) => link.getAttribute('href'))).toEqual([ + expect(links.map((link) => link.textContent)).toEqual(['file1.txt', 'file2.bam', 'file3.vcf']); + expect(links.map((link) => link.getAttribute('href'))).toEqual([ 'gs://test-bucket/path/to/file1.txt', 'gs://test-bucket/path/to/file2.bam', 'gs://test-bucket/path/to/file3.vcf', @@ -106,23 +116,62 @@ describe('FilesTable', () => { render( h(FilesTable, { + directories, files, selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), onClickFile: jest.fn(), onRenameFile: () => {}, }) ); // Act - const firstRow = screen.getAllByRole('row')[1]; // skip header row - const copyUrlButton = within(firstRow).getByLabelText('Copy file URL to clipboard'); + const thirdRow = screen.getAllByRole('row')[3]; // skip header row and directories + const copyUrlButton = within(thirdRow).getByLabelText('Copy file URL to clipboard'); await user.click(copyUrlButton); // Assert expect(clipboard.writeText).toHaveBeenCalledWith('gs://test-bucket/path/to/file1.txt'); }); + it('calls onClickDirectory callback when a directory link is clicked', async () => { + // Arrange + const user = userEvent.setup(); + + const onClickDirectory = jest.fn(); + render( + h(FilesTable, { + directories, + files, + selectedFiles: {}, + setSelectedFiles: () => {}, + onClickDirectory, + onClickFile: jest.fn(), + onRenameFile: () => {}, + }) + ); + + const tableRows = screen.getAllByRole('row').slice(1, 3); // skip header row and files + const nameCells = tableRows.map((row) => getAllByRole(row, 'cell')[1]); + const links = nameCells.map((cell) => getByRole(cell, 'button')); + + // Act + await user.click(links[0]); + await user.click(links[1]); + + // Assert + expect(onClickDirectory).toHaveBeenCalledTimes(2); + expect(onClickDirectory.mock.calls.map((call) => call[0])).toEqual([ + { + path: 'path/to/folder1/', + }, + { + path: 'path/to/folder2/', + }, + ]); + }); + it('calls onClickFile callback when a file link is clicked', async () => { // Arrange const user = userEvent.setup(); @@ -130,21 +179,23 @@ describe('FilesTable', () => { const onClickFile = jest.fn(); render( h(FilesTable, { + directories, files, selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), onClickFile, onRenameFile: () => {}, }) ); - const tableRows = screen.getAllByRole('row').slice(1); // skip header row - const fileNameCells = tableRows.map((row) => getAllByRole(row, 'cell')[1]); - const fileLinks = fileNameCells.map((cell) => getByRole(cell, 'link')); + const tableRows = screen.getAllByRole('row').slice(3); // skip header row and directories + const nameCells = tableRows.map((row) => getAllByRole(row, 'cell')[1]); + const links = nameCells.map((cell) => getByRole(cell, 'link')); // Act - await user.click(fileLinks[0]); - await user.click(fileLinks[1]); + await user.click(links[0]); + await user.click(links[1]); // Assert expect(onClickFile).toHaveBeenCalledTimes(2); @@ -176,9 +227,11 @@ describe('FilesTable', () => { const onRenameFile = jest.fn(); render( h(FilesTable, { + directories, files, selectedFiles: {}, setSelectedFiles: () => {}, + onClickDirectory: jest.fn(), onClickFile: () => {}, onRenameFile, }) @@ -216,7 +269,9 @@ describe('FilesTable', () => { render( h(TestComponent, { initialSelectedFiles: { [files[0].path]: files[0] }, + directories, files, + onClickDirectory: jest.fn(), onClickFile: () => {}, onRenameFile: () => {}, }) diff --git a/src/components/file-browser/FilesTable.ts b/src/components/file-browser/FilesTable.ts index b22bb26b09..332ab57163 100644 --- a/src/components/file-browser/FilesTable.ts +++ b/src/components/file-browser/FilesTable.ts @@ -1,3 +1,4 @@ +import { icon } from '@terra-ui-packages/components'; import filesize from 'filesize'; import React, { Dispatch, Fragment, SetStateAction } from 'react'; import { div, h } from 'react-hyperscript-helpers'; @@ -7,7 +8,7 @@ import { Checkbox, Link } from 'src/components/common'; import { basename } from 'src/components/file-browser/file-browser-utils'; import { FileMenu } from 'src/components/file-browser/FileMenu'; import { FlexTable, HeaderCell, TextCell } from 'src/components/table'; -import { FileBrowserFile } from 'src/libs/ajax/file-browser-providers/FileBrowserProvider'; +import { FileBrowserDirectory, FileBrowserFile } from 'src/libs/ajax/file-browser-providers/FileBrowserProvider'; import * as Style from 'src/libs/style'; import * as Utils from 'src/libs/utils'; @@ -15,9 +16,11 @@ export interface FilesTableProps { 'aria-label'?: string; editDisabled?: boolean; editDisabledReason?: string; + directories: FileBrowserDirectory[]; files: FileBrowserFile[]; selectedFiles: { [path: string]: FileBrowserFile }; setSelectedFiles: Dispatch>; + onClickDirectory: (directory: FileBrowserDirectory) => void; onClickFile: (file: FileBrowserFile) => void; onRenameFile: (file: FileBrowserFile) => void; } @@ -27,9 +30,11 @@ const FilesTable = (props: FilesTableProps) => { 'aria-label': ariaLabel = 'Files', editDisabled = false, editDisabledReason, + directories, files, selectedFiles, setSelectedFiles, + onClickDirectory, onClickFile, onRenameFile, } = props; @@ -44,7 +49,7 @@ const FilesTable = (props: FilesTableProps) => { 'aria-label': ariaLabel, width, height, - rowCount: files.length, + rowCount: files.length + directories.length, noContentMessage: ' ', styleCell: () => ({ padding: '0.5em', borderRight: 'none', borderLeft: 'none' }), styleHeader: () => ({ padding: '0.5em', borderRight: 'none', borderLeft: 'none' }), @@ -67,7 +72,18 @@ const FilesTable = (props: FilesTableProps) => { ]); }, cellRenderer: ({ rowIndex }) => { - const file = files[rowIndex]; + if (rowIndex < directories.length) { + const directory = directories[rowIndex]; + return h( + Link, + { + style: { flex: 1, textAlign: 'center' }, + onClick: () => onClickDirectory(directory), + }, + [icon('folderSolid')] + ); + } + const file = files[rowIndex - directories.length]; const isSelected = file.path in selectedFiles; return div({ style: { flex: 1, textAlign: 'center' } }, [ h(Checkbox, { @@ -91,7 +107,24 @@ const FilesTable = (props: FilesTableProps) => { size: { min: 100, grow: 1 }, headerRenderer: () => h(HeaderCell, ['Name']), cellRenderer: ({ rowIndex }) => { - const file = files[rowIndex]; + if (rowIndex < directories.length) { + const directory = directories[rowIndex]; + return h( + Link, + { + style: { + ...(Style.noWrapEllipsis as React.CSSProperties), + textDecoration: 'underline', + }, + onClick: (e) => { + e.preventDefault(); + onClickDirectory(directory); + }, + }, + [basename(directory.path)] + ); + } + const file = files[rowIndex - directories.length]; return h(Fragment, [ h( Link, @@ -123,7 +156,10 @@ const FilesTable = (props: FilesTableProps) => { size: { min: 150, grow: 0 }, headerRenderer: () => h(HeaderCell, ['Size']), cellRenderer: ({ rowIndex }) => { - const file = files[rowIndex]; + if (rowIndex < directories.length) { + return h(TextCell, ['--']); + } + const file = files[rowIndex - directories.length]; return h(TextCell, [filesize(file.size, { round: 0 })]); }, }, @@ -131,7 +167,10 @@ const FilesTable = (props: FilesTableProps) => { size: { min: 200, grow: 0 }, headerRenderer: () => h(HeaderCell, ['Last modified']), cellRenderer: ({ rowIndex }) => { - const file = files[rowIndex]; + if (rowIndex < directories.length) { + return h(TextCell, ['--']); + } + const file = files[rowIndex - directories.length]; return h(TextCell, [Utils.makePrettyDate(file.updatedAt)]); }, }, @@ -139,7 +178,10 @@ const FilesTable = (props: FilesTableProps) => { size: { basis: 40, grow: 0, shrink: 0 }, headerRenderer: () => h(HeaderCell, { className: 'sr-only' }, ['Actions']), cellRenderer: ({ rowIndex }) => { - const file = files[rowIndex]; + if (rowIndex < directories.length) { + return h(TextCell); + } + const file = files[rowIndex - directories.length]; return h(TextCell, [ h(FileMenu, { editDisabled, diff --git a/src/libs/ajax/file-browser-providers/GCSFileBrowserProvider.test.ts b/src/libs/ajax/file-browser-providers/GCSFileBrowserProvider.test.ts index 6838869964..06339da516 100644 --- a/src/libs/ajax/file-browser-providers/GCSFileBrowserProvider.test.ts +++ b/src/libs/ajax/file-browser-providers/GCSFileBrowserProvider.test.ts @@ -49,8 +49,8 @@ describe('GCSFileBrowserProvider', () => { undefined, () => ({ kind: 'storage#objects', - items: [gcsObject('a-file.txt'), gcsObject('b-file.txt')], - prefixes: ['a-prefix/'], + items: [gcsObject('a-file.txt'), gcsObject('b-file.txt'), gcsObject('c-file.txt')], + prefixes: ['a-prefix/', 'b-prefix/', 'c-prefix/'], nextPageToken: '2', }), ], @@ -58,16 +58,8 @@ describe('GCSFileBrowserProvider', () => { '2', () => ({ kind: 'storage#objects', - items: [gcsObject('c-file.txt'), gcsObject('d-file.txt')], - prefixes: ['b-prefix/'], - nextPageToken: '3', - }), - ], - [ - '3', - () => ({ - kind: 'storage#objects', - prefixes: ['c-prefix/', 'd-prefix/'], + items: [gcsObject('d-file.txt')], + prefixes: ['d-prefix/'], }), ], [ @@ -102,7 +94,7 @@ describe('GCSFileBrowserProvider', () => { ]; expect(firstResponse.items).toEqual(expectedFirstPageFiles); expect(firstResponse.hasNextPage).toBe(true); - expect(numGCSRequestsAfterFirstResponse).toBe(2); + expect(numGCSRequestsAfterFirstResponse).toBe(1); const expectedSecondPageFiles: FileBrowserFile[] = [ expectedFile('a-file.txt'), @@ -112,7 +104,7 @@ describe('GCSFileBrowserProvider', () => { ]; expect(secondResponse.items).toEqual(expectedSecondPageFiles); expect(secondResponse.hasNextPage).toBe(false); - expect(numGCSRequestsAfterSecondResponse).toBe(3); + expect(numGCSRequestsAfterSecondResponse).toBe(2); }); it('pages through directories (prefixes)', async () => { @@ -133,7 +125,7 @@ describe('GCSFileBrowserProvider', () => { ]; expect(firstResponse.items).toEqual(expectedFirstPageDirectories); expect(firstResponse.hasNextPage).toBe(true); - expect(numGCSRequestsAfterFirstResponse).toBe(3); + expect(numGCSRequestsAfterFirstResponse).toBe(1); const expectedSecondPageDirectories: FileBrowserDirectory[] = [ { path: 'a-prefix/' }, @@ -143,7 +135,7 @@ describe('GCSFileBrowserProvider', () => { ]; expect(secondResponse.items).toEqual(expectedSecondPageDirectories); expect(secondResponse.hasNextPage).toBe(false); - expect(numGCSRequestsAfterSecondResponse).toBe(3); + expect(numGCSRequestsAfterSecondResponse).toBe(2); }); it('gets a signed URL for downloads', async () => { diff --git a/src/libs/ajax/file-browser-providers/GCSFileBrowserProvider.ts b/src/libs/ajax/file-browser-providers/GCSFileBrowserProvider.ts index 82a2004b5f..433b61702d 100644 --- a/src/libs/ajax/file-browser-providers/GCSFileBrowserProvider.ts +++ b/src/libs/ajax/file-browser-providers/GCSFileBrowserProvider.ts @@ -11,6 +11,7 @@ export interface GCSFileBrowserProviderParams { type GCSFileBrowserProviderGetPageParams = { isFirstPage: boolean; + matchGlob?: string; pageToken?: string; pendingItems?: T[]; prefix: string; @@ -28,6 +29,7 @@ type GCSFileBrowserProviderGetPageParams = { ); interface BucketListRequestOptions { + matchGlob?: string; maxResults: number; pageToken?: string; } @@ -42,6 +44,7 @@ const GCSFileBrowserProvider = ({ isFirstPage, itemsOrPrefixes, mapItemOrPrefix, + matchGlob, pageToken, pendingItems = [], prefix, @@ -60,6 +63,9 @@ const GCSFileBrowserProvider = ({ if (nextPageToken) { requestOptions.pageToken = nextPageToken; } + if (matchGlob) { + requestOptions.matchGlob = matchGlob; + } const response = await Ajax(signal).Buckets.list(project, bucket, prefix, requestOptions); const responseItems = (response[itemsOrPrefixes] || []).map((itemOrPrefix) => mapItemOrPrefix(itemOrPrefix)); @@ -87,6 +93,7 @@ const GCSFileBrowserProvider = ({ isFirstPage: false, itemsOrPrefixes, mapItemOrPrefix, + matchGlob, pageToken: nextPageToken, pendingItems: nextPendingItems, prefix, @@ -115,6 +122,8 @@ const GCSFileBrowserProvider = ({ createdAt: new Date(item.timeCreated).getTime(), updatedAt: new Date(item.updated).getTime(), }), + // This glob pattern matches objects which are not themselves prefixes (i.e. files) + matchGlob: '**?', prefix: path, signal, }), @@ -125,6 +134,8 @@ const GCSFileBrowserProvider = ({ mapItemOrPrefix: (prefix) => ({ path: `${prefix}`, }), + // This glob pattern matches prefixes (directories) excluding the given path + matchGlob: `${path}**?/`, prefix: path, signal, }), diff --git a/src/workflows-app/SubmissionDetails.test.js b/src/workflows-app/SubmissionDetails.test.js index a067863c85..2ec224e183 100644 --- a/src/workflows-app/SubmissionDetails.test.js +++ b/src/workflows-app/SubmissionDetails.test.js @@ -2,7 +2,7 @@ import { act, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import _ from 'lodash/fp'; import { h } from 'react-hyperscript-helpers'; -import { useFilesInDirectory } from 'src/components/file-browser/file-browser-hooks'; +import { useDirectoriesInDirectory, useFilesInDirectory } from 'src/components/file-browser/file-browser-hooks'; import FileBrowser from 'src/components/file-browser/FileBrowser'; import FilesTable from 'src/components/file-browser/FilesTable'; import { Ajax } from 'src/libs/ajax'; @@ -58,6 +58,7 @@ jest.mock('src/libs/config', () => ({ jest.mock('src/components/file-browser/file-browser-hooks', () => ({ ...jest.requireActual('src/components/file-browser/file-browser-hooks'), + useDirectoriesInDirectory: jest.fn(), useFilesInDirectory: jest.fn(), })); @@ -1602,6 +1603,11 @@ describe('Submission Details page', () => { it('navigates to a sub-directory of the root', () => { // Arrange const mockFileBrowserProvider = {}; + const directories = [ + { + path: 'workspace-services/cbas/terra-app-/fetch_sra_to_bam/d16721eb-8745-4aa2-b71e-9ade2d6575aa/folder/', + }, + ]; const files = [ { path: 'workspace-services/cbas/terra-app-/fetch_sra_to_bam/d16721eb-8745-4aa2-b71e-9ade2d6575aa/', @@ -1613,6 +1619,14 @@ describe('Submission Details page', () => { }, ]; + const useDirectoriesInDirectoryResult = { + state: { directories, status: 'Ready' }, + hasNextPage: false, + loadNextPage: () => Promise.resolve(), + loadAllRemainingItems: () => Promise.resolve(), + reload: () => Promise.resolve(), + }; + const useFilesInDirectoryResult = { state: { files, status: 'Ready' }, hasNextPage: false, @@ -1621,6 +1635,7 @@ describe('Submission Details page', () => { reload: () => Promise.resolve(), }; + asMockedFn(useDirectoriesInDirectory).mockReturnValue(useDirectoriesInDirectoryResult); asMockedFn(useFilesInDirectory).mockReturnValue(useFilesInDirectoryResult); // Act @@ -1638,22 +1653,8 @@ describe('Submission Details page', () => { ); // Assert - expect(FilesTable).toHaveBeenCalledWith( - expect.objectContaining({ - files: [ - { - path: 'workspace-services/cbas/terra-app-/fetch_sra_to_bam/d16721eb-8745-4aa2-b71e-9ade2d6575aa/', - url: 'gs://test-bucket/file.txt', - contentType: 'text/plain', - size: 1024, - createdAt: 1667408400000, - updatedAt: 1667408400000, - }, - ], - }), - expect.anything() - ); - screen.getByText('Loaded 1 files in d16721eb-8745-4aa2-b71e-9ade2d6575aa'); + expect(FilesTable).toHaveBeenCalledWith(expect.objectContaining({ directories, files }), expect.anything()); + screen.getByText('Loaded 1 directories and 1 files in d16721eb-8745-4aa2-b71e-9ade2d6575aa'); screen.getByText('d16721eb-8745-4aa2-b71e-9ade2d6575aa'); }); });