Skip to content

Commit

Permalink
[CORE-249] Add Copy Functionality for Folders in File Browser (#5236)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kevinmarete
kevinmarete authored Jan 28, 2025
1 parent 84c6873 commit 859c5a2
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 23 deletions.
23 changes: 23 additions & 0 deletions src/components/file-browser/DirectoryTree.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe('Directory', () => {
level: 0,
id: 'node-0',
path: 'path/to/directory/',
url: 'gs://path/to/directory/',
provider: mockFileBrowserProvider,
reloadRequests: subscribable(),
rootLabel: 'Workspace bucket',
Expand All @@ -45,12 +46,15 @@ describe('Directory', () => {
const directories = [
{
path: 'directory1',
url: 'gs://directory1/',
},
{
path: 'directory2',
url: 'gs://directory3/',
},
{
path: 'directory3',
url: 'gs://directory3/',
},
];

Expand All @@ -72,6 +76,7 @@ describe('Directory', () => {
id: 'node-0',
level: 0,
path: '',
url: 'gs://',
provider: mockFileBrowserProvider,
reloadRequests: subscribable(),
rootLabel: 'Workspace bucket',
Expand Down Expand Up @@ -99,18 +104,22 @@ describe('Directory', () => {
return [
{
path: 'directory1/test1',
url: 'gs://directory1/test1/',
},
{
path: 'directory1/test2',
url: 'gs://directory1/test2/',
},
];
case 'directory1/test1/':
return [
{
path: 'directory1/test1/abc/',
url: 'gs://directory1/test1/abc/',
},
{
path: 'directory1/test1/xyz/',
url: 'gs://directory1/test1/xyz/',
},
];
default:
Expand All @@ -135,6 +144,7 @@ describe('Directory', () => {
id: 'node-0',
level: 0,
path: 'directory1/',
url: 'gs://directory1/',
provider: mockFileBrowserProvider,
reloadRequests: subscribable(),
rootLabel: 'Workspace bucket',
Expand Down Expand Up @@ -162,12 +172,15 @@ describe('Directory', () => {
const directories = [
{
path: 'path/to/directory/subdirectory1',
url: 'gs://path/to/directory/subdirectory1/',
},
{
path: 'path/to/directory/subdirectory2',
url: 'gs://path/to/directory/subdirectory2/',
},
{
path: 'path/to/directory/subdirectory3',
url: 'gs://path/to/directory/subdirectory3/',
},
];

Expand All @@ -188,6 +201,7 @@ describe('Directory', () => {
id: 'node-0',
level: 0,
path: 'path/to/directory/',
url: 'gs://path/to/directory/',
provider: mockFileBrowserProvider,
reloadRequests: subscribable(),
rootLabel: 'Workspace bucket',
Expand Down Expand Up @@ -226,12 +240,15 @@ describe('Directory', () => {
const directories = [
{
path: 'path/to/directory/subdirectory1',
url: 'gs://path/to/directory/subdirectory1/',
},
{
path: 'path/to/directory/subdirectory2',
url: 'gs://path/to/directory/subdirectory2/',
},
{
path: 'path/to/directory/subdirectory3',
url: 'gs://path/to/directory/subdirectory3/',
},
];

Expand All @@ -252,6 +269,7 @@ describe('Directory', () => {
id: 'node-0',
level: 0,
path: 'path/to/directory/',
url: 'gs://path/to/directory/',
provider: mockFileBrowserProvider,
reloadRequests: subscribable(),
rootLabel: 'Workspace bucket',
Expand Down Expand Up @@ -303,6 +321,7 @@ describe('Directory', () => {
id: 'node-0',
level: 0,
path: 'path/to/directory/',
url: 'gs://path/to/directory/',
provider: mockFileBrowserProvider,
reloadRequests: subscribable(),
rootLabel: 'Workspace bucket',
Expand Down Expand Up @@ -348,6 +367,7 @@ describe('Directory', () => {
id: 'node-0',
level: 0,
path: 'path/to/directory/',
url: 'gs://path/to/directory/',
provider: mockFileBrowserProvider,
reloadRequests: subscribable(),
rootLabel: 'Workspace bucket',
Expand Down Expand Up @@ -394,6 +414,7 @@ describe('Directory', () => {
id: 'node-0',
level: 0,
path: 'path/to/directory/',
url: 'gs://path/to/directory/',
provider: mockFileBrowserProvider,
reloadRequests: subscribable(),
rootLabel: 'Workspace bucket',
Expand All @@ -420,6 +441,7 @@ describe('Directory', () => {
const directories = [
{
path: 'path/to/directory/subdirectory1',
url: 'gs://path/to/directory/subdirectory1/',
},
];

Expand All @@ -445,6 +467,7 @@ describe('Directory', () => {
id: 'node-0',
level: 0,
path: 'path/to/directory/',
url: 'gs://path/to/directory/',
provider: mockFileBrowserProvider,
reloadRequests: subscribable(),
rootLabel: 'Workspace bucket',
Expand Down
9 changes: 7 additions & 2 deletions src/components/file-browser/DirectoryTree.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ export const Subdirectories = (props: SubdirectoriesProps) => {
id: `${parentId}-${index}`,
level: level + 1,
path: directory.path,
url: directory.url,
provider,
reloadRequests,
rootLabel,
Expand Down Expand Up @@ -194,6 +195,7 @@ interface DirectoryProps {
provider: FileBrowserProvider;
level: number;
path: string;
url?: string;
reloadRequests: ReloadRequestSubscribable;
rootLabel: string;
selectedDirectory: string;
Expand All @@ -208,6 +210,7 @@ export const Directory = (props: DirectoryProps) => {
id,
level,
path,
url,
provider,
reloadRequests,
rootLabel,
Expand Down Expand Up @@ -289,8 +292,9 @@ export const Directory = (props: DirectoryProps) => {
Link,
{
id: `${id}-link`,
role: 'presentation',
role: 'link',
tabIndex: -1,
href: url,
style: {
display: 'inline-block',
overflow: 'hidden',
Expand All @@ -308,7 +312,8 @@ export const Directory = (props: DirectoryProps) => {
...(isSelected && {
hover: { color: selectedDirectoryColor },
}),
onClick: () => {
onClick: (e) => {
e.preventDefault();
onSelectDirectory(path);
setIsExpanded(true);
},
Expand Down
1 change: 1 addition & 0 deletions src/components/file-browser/FileBrowser.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe('FileBrowser', () => {
const directories: FileBrowserDirectory[] = [
{
path: 'path/to/folder/',
url: 'gs://test-bucket/path/to/folder/',
},
];

Expand Down
6 changes: 5 additions & 1 deletion src/components/file-browser/FilesTable.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@ describe('FilesTable', () => {
const directories: FileBrowserDirectory[] = [
{
path: 'path/to/folder1/',
url: 'gs://test-bucket/path/to/folder1/',
},
{
path: 'path/to/folder2/',
url: 'gs://test-bucket/path/to/folder2/',
},
];

Expand Down Expand Up @@ -154,7 +156,7 @@ describe('FilesTable', () => {

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'));
const links = nameCells.map((cell) => getByRole(cell, 'link'));

// Act
await user.click(links[0]);
Expand All @@ -165,9 +167,11 @@ describe('FilesTable', () => {
expect(onClickDirectory.mock.calls.map((call) => call[0])).toEqual([
{
path: 'path/to/folder1/',
url: 'gs://test-bucket/path/to/folder1/',
},
{
path: 'path/to/folder2/',
url: 'gs://test-bucket/path/to/folder2/',
},
]);
});
Expand Down
37 changes: 24 additions & 13 deletions src/components/file-browser/FilesTable.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,20 +109,31 @@ const FilesTable = (props: FilesTableProps) => {
cellRenderer: ({ rowIndex }) => {
if (rowIndex < directories.length) {
const directory = directories[rowIndex];
return h(
Link,
{
style: {
...(Style.noWrapEllipsis as React.CSSProperties),
textDecoration: 'underline',
return h(Fragment, [
h(
Link,
{
href: directory.url ?? directory.path,
style: {
...(Style.noWrapEllipsis as React.CSSProperties),
textDecoration: 'underline',
},
onClick: (e) => {
e.preventDefault();
onClickDirectory(directory);
},
},
onClick: (e) => {
e.preventDefault();
onClickDirectory(directory);
},
},
[basename(directory.path)]
);
[basename(directory.path)]
),
h(ClipboardButton, {
'aria-label': 'Copy folder URL to clipboard',
className: 'cell-hover-only',
iconSize: 14,
text: directory.url ?? directory.path,
tooltip: 'Copy folder URL to clipboard',
style: { marginLeft: '1ch' },
}),
]);
}
const file = files[rowIndex - directories.length];
return h(Fragment, [
Expand Down
1 change: 1 addition & 0 deletions src/libs/ajax/file-browser-providers/FileBrowserProvider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export interface FileBrowserFile {

export interface FileBrowserDirectory {
path: string;
url?: string;
}

interface FileBrowserProvider {
Expand Down
33 changes: 26 additions & 7 deletions src/libs/ajax/file-browser-providers/GCSFileBrowserProvider.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,19 +119,19 @@ describe('GCSFileBrowserProvider', () => {

// Assert
const expectedFirstPageDirectories: FileBrowserDirectory[] = [
{ path: 'a-prefix/' },
{ path: 'b-prefix/' },
{ path: 'c-prefix/' },
{ path: 'a-prefix/', url: 'gs://test-bucket/a-prefix/' },
{ path: 'b-prefix/', url: 'gs://test-bucket/b-prefix/' },
{ path: 'c-prefix/', url: 'gs://test-bucket/c-prefix/' },
];
expect(firstResponse.items).toEqual(expectedFirstPageDirectories);
expect(firstResponse.hasNextPage).toBe(true);
expect(numGCSRequestsAfterFirstResponse).toBe(1);

const expectedSecondPageDirectories: FileBrowserDirectory[] = [
{ path: 'a-prefix/' },
{ path: 'b-prefix/' },
{ path: 'c-prefix/' },
{ path: 'd-prefix/' },
{ path: 'a-prefix/', url: 'gs://test-bucket/a-prefix/' },
{ path: 'b-prefix/', url: 'gs://test-bucket/b-prefix/' },
{ path: 'c-prefix/', url: 'gs://test-bucket/c-prefix/' },
{ path: 'd-prefix/', url: 'gs://test-bucket/d-prefix/' },
];
expect(secondResponse.items).toEqual(expectedSecondPageDirectories);
expect(secondResponse.hasNextPage).toBe(false);
Expand Down Expand Up @@ -281,4 +281,23 @@ describe('GCSFileBrowserProvider', () => {
// Assert
expect(del).toHaveBeenCalledWith('test-project', 'test-bucket', 'foo/bar/baz/');
});

it('retrieves directories in a directory', async () => {
// Arrange
const provider = GCSFileBrowserProvider({ bucket: 'test-bucket', project: 'test-project', pageSize: 3 });

// Act
const response = await provider.getDirectoriesInDirectory('some/path/');
const numGCSRequests = list.mock.calls.length;

// Assert
const expectedDirectories: FileBrowserDirectory[] = [
{ path: 'a-prefix/', url: 'gs://test-bucket/a-prefix/' },
{ path: 'b-prefix/', url: 'gs://test-bucket/b-prefix/' },
{ path: 'c-prefix/', url: 'gs://test-bucket/c-prefix/' },
];
expect(response.items).toEqual(expectedDirectories);
expect(response.hasNextPage).toBe(true);
expect(numGCSRequests).toBe(1);
});
});
1 change: 1 addition & 0 deletions src/libs/ajax/file-browser-providers/GCSFileBrowserProvider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ const GCSFileBrowserProvider = ({
itemsOrPrefixes: 'prefixes',
mapItemOrPrefix: (prefix) => ({
path: `${prefix}`,
url: `gs://${bucket}/${prefix}`,
}),
// This glob pattern matches prefixes (directories) excluding the given path
matchGlob: `${path}**?/`,
Expand Down

0 comments on commit 859c5a2

Please sign in to comment.