fix: draft folder/file handling (#777)

packages/core/dev-test/backends/github/config.yml

@@ -58,6 +58,8 @@ collections:
         name: image
         widget: image
         required: false
+        media_library:
+          folder_support: true
       - label: Body
         name: body
         widget: markdown

packages/core/src/actions/entries.ts

@@ -556,18 +556,20 @@ export function retrieveLocalBackup(collection: Collection, slug: string) {
       // load assets from backup
       const mediaFiles = entry.mediaFiles || [];
       const assetProxies: AssetProxy[] = await Promise.all(
-        mediaFiles.map(file => {
-          if (file.file || file.url) {
-            return createAssetProxy({
-              path: file.path,
-              file: file.file,
-              url: file.url,
-              field: file.field,
-            });
-          } else {
-            return getAsset(collection, entry, file.path, file.field)(dispatch, getState);
-          }
-        }),
+        mediaFiles
+          .filter(file => !file.isDirectory)
+          .map(file => {
+            if (file.file || file.url) {
+              return createAssetProxy({
+                path: file.path,
+                file: file.file,
+                url: file.url,
+                field: file.field,
+              });
+            } else {
+              return getAsset(collection, entry, file.path, file.field)(dispatch, getState);
+            }
+          }),
       );
       dispatch(addAssets(assetProxies));
 

packages/core/src/actions/mediaLibrary.ts

@@ -270,7 +270,7 @@ export function persistMedia(
 
       let mediaFile: ImplementationMediaFile;
       if (editingDraft) {
-        const id = await getBlobSHA(file);
+        const id = `${assetProxy.path}/${await getBlobSHA(file)}`;
         mediaFile = createMediaFileFromAsset({
           id,
           file,

packages/core/src/components/media-library/common/FolderCreationDialog.tsx

@@ -33,12 +33,18 @@ const FolderCreationDialog: FC<TranslatedProps<FolderCreationDialogProps>> = ({
     }
 
     onCreate(folderName);
+    setFolderName('');
   }, [folderName, onCreate]);
 
+  const handleClose = useCallback(() => {
+    onClose();
+    setFolderName('');
+  }, [onClose]);
+
   return (
     <Modal
       open={open}
-      onClose={onClose}
+      onClose={handleClose}
       className="
         w-[50%]
         min-w-[300px]
@@ -65,7 +71,7 @@ const FolderCreationDialog: FC<TranslatedProps<FolderCreationDialogProps>> = ({
         >
           {t('mediaLibrary.folderSupport.createNewFolder')}
         </h3>
-        <IconButton variant="text" aria-label="add" onClick={onClose}>
+        <IconButton variant="text" aria-label="add" onClick={handleClose}>
           <CloseIcon className="w-5 h-5" />
         </IconButton>
       </div>
@@ -96,7 +102,7 @@ const FolderCreationDialog: FC<TranslatedProps<FolderCreationDialogProps>> = ({
           space-x-2
         "
       >
-        <Button variant="text" aria-label="cancel" onClick={onClose}>
+        <Button variant="text" aria-label="cancel" onClick={handleClose}>
           Cancel
         </Button>
         <Button

packages/core/src/components/media-library/common/MediaLibraryCard.tsx

@@ -62,7 +62,7 @@ const MediaLibraryCard = <T extends MediaField, EF extends BaseField = UnknownFi
   t,
 }: TranslatedProps<MediaLibraryCardProps<T, EF>>) => {
   const entry = useAppSelector(selectEditingDraft);
-  const url = useMediaAsset(path, collection, field, entry, currentFolder);
+  const url = useMediaAsset(path, collection, field, entry, currentFolder, isDirectory);
 
   const handleDownload = useCallback(() => {
     const url = displayURL.url;

packages/core/src/lib/hooks/__tests__/useMediaFiles.spec.tsx

@@ -0,0 +1,427 @@
+/**
+ * @jest-environment jsdom
+ */
+import '@testing-library/jest-dom';
+import { act, render } from '@testing-library/react';
+import React from 'react';
+
+import { currentBackend } from '@staticcms/core/backend';
+import { selectCollection } from '@staticcms/core/reducers/selectors/collections';
+import { selectConfig } from '@staticcms/core/reducers/selectors/config';
+import { selectEditingDraft } from '@staticcms/core/reducers/selectors/entryDraft';
+import { selectMediaLibraryFiles } from '@staticcms/core/reducers/selectors/mediaLibrary';
+import { useAppSelector } from '@staticcms/core/store/hooks';
+import { createMockCollection } from '@staticcms/test/data/collections.mock';
+import { createMockConfig } from '@staticcms/test/data/config.mock';
+import { createMockEntry } from '@staticcms/test/data/entry.mock';
+import useMediaFiles from '../useMediaFiles';
+import { mockFileField } from '@staticcms/test/data/fields.mock';
+
+import type { MediaField, MediaFile } from '@staticcms/core/interface';
+import type { FC } from 'react';
+
+interface MockWidgetProps {
+  field?: MediaField;
+  currentFolder?: string;
+}
+
+jest.mock('@staticcms/core/reducers/selectors/collections');
+jest.mock('@staticcms/core/reducers/selectors/config');
+jest.mock('@staticcms/core/reducers/selectors/entryDraft');
+jest.mock('@staticcms/core/reducers/selectors/mediaLibrary');
+jest.mock('@staticcms/core/backend');
+jest.mock('@staticcms/core/store/hooks');
+
+const MockWidget: FC<MockWidgetProps> = ({ field, currentFolder }) => {
+  const files = useMediaFiles(field, currentFolder);
+
+  return (
+    <div data-testid="files">
+      <div data-testid="file-count">{files.length}</div>
+      {files.map(file => (
+        <div key={file.path} data-testid={file.path}>
+          {file.path}
+        </div>
+      ))}
+    </div>
+  );
+};
+
+const testMediaFiles: MediaFile[] = [
+  {
+    name: 'file.txt',
+    id: 'file.txt',
+    path: 'path/to/file.txt',
+  },
+  {
+    name: 'other-file.png',
+    id: 'other-file.png',
+    path: 'path/to/other-file.png',
+  },
+  {
+    name: '.gitkeep',
+    id: '.gitkeep',
+    path: 'path/to/.gitkeep',
+  },
+  {
+    name: 'A Directory',
+    id: 'A Directory',
+    path: 'path/to/A Directory',
+    isDirectory: true,
+  },
+];
+
+const testEntryMediaFiles: Record<string, MediaFile[]> = {
+  'path/to': [
+    {
+      name: 'file-entry.txt',
+      id: 'file-entry.txt',
+      path: 'path/to/file-entry.txt',
+    },
+    {
+      name: 'other-entry-file.png',
+      id: 'other-entry-file.png',
+      path: 'path/to/other-entry-file.png',
+    },
+    {
+      name: '.gitkeep',
+      id: '.gitkeep',
+      path: 'path/to/.gitkeep',
+    },
+    {
+      name: 'An Entry Directory',
+      id: 'An Entry Directory',
+      path: 'path/to/An Entry Directory',
+      isDirectory: true,
+    },
+    {
+      name: 'An Empty Directory',
+      id: 'An Empty Directory',
+      path: 'path/to/An Empty Entry Directory',
+      isDirectory: true,
+    },
+  ],
+  'path/to/An Entry Directory': [
+    {
+      name: '.gitkeep',
+      id: '.gitkeep',
+      path: 'path/to/An Entry Directory/.gitkeep',
+    },
+    {
+      name: 'sub-folder-file.jpg',
+      id: 'sub-folder-file.jpg',
+      path: 'path/to/An Entry Directory/sub-folder-file.jpg',
+    },
+  ],
+  'path/to/An Empty Entry Directory': [
+    {
+      name: '.gitkeep',
+      id: '.gitkeep',
+      path: 'path/to/An Empty Entry Directory/.gitkeep',
+    },
+  ],
+};
+
+describe('useMediaFiles', () => {
+  const mockSelectCollection = selectCollection as jest.Mock;
+  const mockSelectConfig = selectConfig as jest.Mock;
+  const mockSelectEditingDraft = selectEditingDraft as jest.Mock;
+  const mockSelectMediaLibraryFiles = selectMediaLibraryFiles as jest.Mock;
+  const mockCurrentBackend = currentBackend as jest.Mock;
+  const mockUseAppSelector = useAppSelector as jest.Mock;
+
+  const mockGetMedia = jest.fn();
+
+  const mockCollection = createMockCollection();
+
+  const createMockComponent = async (props: MockWidgetProps = {}) => {
+    const { rerender, ...result } = render(<MockWidget {...props} />);
+
+    await act(async () => {
+      await Promise.resolve();
+    });
+
+    const rerenderMockComponent = async (rerenderProps: Partial<MockWidgetProps> = {}) => {
+      rerender(<MockWidget {...props} {...rerenderProps} />);
+
+      await act(async () => {
+        await Promise.resolve();
+      });
+    };
+
+    return { ...result, rerender: rerenderMockComponent };
+  };
+
+  beforeEach(() => {
+    jest.useFakeTimers();
+    mockUseAppSelector.mockImplementation((fn: () => unknown) => {
+      if (typeof fn !== 'function') {
+        return undefined;
+      }
+
+      return fn();
+    });
+
+    mockSelectCollection.mockReturnValue(() => undefined);
+    mockSelectConfig.mockReturnValue(
+      createMockConfig({
+        collections: [mockCollection],
+        media_folder: 'path/to',
+        public_folder: 'public/path',
+      }),
+    );
+    mockSelectEditingDraft.mockReturnValue(undefined);
+    mockSelectMediaLibraryFiles.mockReturnValue(testMediaFiles);
+    mockCurrentBackend.mockReturnValue({
+      getMedia: mockGetMedia,
+    });
+    mockGetMedia.mockImplementation(path => Promise.resolve(testEntryMediaFiles[path] ?? []));
+  });
+
+  afterAll(() => {
+    jest.useRealTimers();
+  });
+
+  describe('top level', () => {
+    it('should retrieve media files, ignoring .gitkeep files', async () => {
+      const { getByTestId } = await createMockComponent();
+
+      expect(getByTestId('file-count').textContent).toBe('2');
+      expect(getByTestId('path/to/file.txt')).toBeInTheDocument();
+      expect(getByTestId('path/to/other-file.png')).toBeInTheDocument();
+
+      expect(mockGetMedia).not.toHaveBeenCalled();
+    });
+
+    it('shows folders when folder support is on', async () => {
+      mockSelectConfig.mockReturnValue(
+        createMockConfig({
+          collections: [mockCollection],
+          media_library: { folder_support: true },
+        }),
+      );
+
+      const { getByTestId } = await createMockComponent();
+
+      expect(getByTestId('file-count').textContent).toBe('3');
+      expect(getByTestId('path/to/file.txt')).toBeInTheDocument();
+      expect(getByTestId('path/to/other-file.png')).toBeInTheDocument();
+      expect(getByTestId('path/to/A Directory')).toBeInTheDocument();
+
+      expect(mockGetMedia).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('entry', () => {
+    beforeEach(() => {
+      mockSelectEditingDraft.mockReturnValue(createMockEntry({ data: {} }));
+      mockSelectCollection.mockReturnValue(() => mockCollection);
+    });
+
+    it('should retrieve media files, ignoring .gitkeep files', async () => {
+      const { getByTestId } = await createMockComponent({
+        field: mockFileField,
+        currentFolder: 'path/to',
+      });
+
+      expect(getByTestId('file-count').textContent).toBe('2');
+      expect(getByTestId('path/to/file-entry.txt')).toBeInTheDocument();
+      expect(getByTestId('path/to/other-entry-file.png')).toBeInTheDocument();
+
+      expect(mockGetMedia).toHaveBeenCalledWith('path/to', false, 'public/path');
+    });
+
+    it('shows folders when folder support is on', async () => {
+      const { getByTestId } = await createMockComponent({
+        field: { ...mockFileField, media_library: { folder_support: true } },
+        currentFolder: 'path/to',
+      });
+
+      expect(getByTestId('file-count').textContent).toBe('4');
+      expect(getByTestId('path/to/file-entry.txt')).toBeInTheDocument();
+      expect(getByTestId('path/to/other-entry-file.png')).toBeInTheDocument();
+      expect(getByTestId('path/to/An Entry Directory')).toBeInTheDocument();
+      expect(getByTestId('path/to/An Empty Entry Directory')).toBeInTheDocument();
+
+      expect(mockGetMedia).toHaveBeenCalledWith('path/to', true, 'public/path');
+    });
+
+    it('should retrieve sub folder media files, ignoring .gitkeep files', async () => {
+      const { getByTestId } = await createMockComponent({
+        field: mockFileField,
+        currentFolder: 'path/to/An Entry Directory',
+      });
+
+      expect(getByTestId('file-count').textContent).toBe('1');
+      expect(getByTestId('path/to/An Entry Directory/sub-folder-file.jpg')).toBeInTheDocument();
+
+      expect(mockGetMedia).toHaveBeenCalledWith(
+        'path/to/An Entry Directory',
+        false,
+        'public/path/An Entry Directory',
+      );
+    });
+
+    it('should return no files for empty directory, ignoring .gitkeep files', async () => {
+      const { getByTestId } = await createMockComponent({
+        field: mockFileField,
+        currentFolder: 'path/to/An Empty Entry Directory',
+      });
+
+      expect(getByTestId('file-count').textContent).toBe('0');
+
+      expect(mockGetMedia).toHaveBeenCalledWith(
+        'path/to/An Empty Entry Directory',
+        false,
+        'public/path/An Empty Entry Directory',
+      );
+    });
+
+    it('should retrieve media as user transitions through folders', async () => {
+      const { getByTestId, rerender } = await createMockComponent({
+        field: { ...mockFileField, media_library: { folder_support: true } },
+        currentFolder: 'path/to',
+      });
+
+      expect(getByTestId('file-count').textContent).toBe('4');
+      expect(getByTestId('path/to/file-entry.txt')).toBeInTheDocument();
+      expect(getByTestId('path/to/other-entry-file.png')).toBeInTheDocument();
+      expect(getByTestId('path/to/An Entry Directory')).toBeInTheDocument();
+      expect(getByTestId('path/to/An Empty Entry Directory')).toBeInTheDocument();
+
+      expect(mockGetMedia).toHaveBeenLastCalledWith('path/to', true, 'public/path');
+
+      const promise = rerender({
+        currentFolder: 'path/to/An Entry Directory',
+      });
+
+      expect(getByTestId('file-count').textContent).toBe('0');
+
+      await promise;
+
+      expect(getByTestId('file-count').textContent).toBe('1');
+      expect(getByTestId('path/to/An Entry Directory/sub-folder-file.jpg')).toBeInTheDocument();
+
+      expect(mockGetMedia).toHaveBeenCalledWith(
+        'path/to/An Entry Directory',
+        true,
+        'public/path/An Entry Directory',
+      );
+
+      const promise2 = await rerender({
+        currentFolder: 'path/to/An Empty Entry Directory',
+      });
+
+      expect(getByTestId('file-count').textContent).toBe('0');
+
+      await promise2;
+
+      expect(getByTestId('file-count').textContent).toBe('0');
+
+      expect(mockGetMedia).toHaveBeenCalledWith(
+        'path/to/An Empty Entry Directory',
+        true,
+        'public/path/An Empty Entry Directory',
+      );
+    });
+
+    it('should retrieve media as user transitions through draft folders', async () => {
+      mockSelectEditingDraft.mockReturnValue(
+        createMockEntry({
+          data: {},
+          mediaFiles: [
+            {
+              name: '.gitkeep',
+              id: '.gitkeep',
+              path: 'path/to/Draft Folder/.gitkeep',
+              draft: true,
+            },
+            {
+              name: '.gitkeep',
+              id: '.gitkeep',
+              path: 'path/to/Draft Folder/Sub Folder/.gitkeep',
+              draft: true,
+            },
+            {
+              name: 'image.gif',
+              id: 'image.gif',
+              path: 'path/to/Draft Folder/Sub Folder/image.gif',
+              draft: true,
+            },
+          ],
+        }),
+      );
+
+      const { getByTestId, rerender } = await createMockComponent({
+        field: { ...mockFileField, media_library: { folder_support: true } },
+        currentFolder: 'path/to',
+      });
+
+      expect(getByTestId('file-count').textContent).toBe('5');
+      expect(getByTestId('path/to/file-entry.txt')).toBeInTheDocument();
+      expect(getByTestId('path/to/other-entry-file.png')).toBeInTheDocument();
+      expect(getByTestId('path/to/An Entry Directory')).toBeInTheDocument();
+      expect(getByTestId('path/to/An Empty Entry Directory')).toBeInTheDocument();
+      expect(getByTestId('path/to/Draft Folder')).toBeInTheDocument();
+
+      expect(mockGetMedia).toHaveBeenLastCalledWith('path/to', true, 'public/path');
+
+      const promise = rerender({
+        currentFolder: 'path/to/Draft Folder',
+      });
+
+      // Draft files/folders should appear immediately
+      expect(getByTestId('file-count').textContent).toBe('1');
+      expect(getByTestId('path/to/Draft Folder/Sub Folder')).toBeInTheDocument();
+
+      await promise;
+
+      expect(getByTestId('file-count').textContent).toBe('1');
+      expect(getByTestId('path/to/Draft Folder/Sub Folder')).toBeInTheDocument();
+
+      expect(mockGetMedia).toHaveBeenCalledWith(
+        'path/to/Draft Folder',
+        true,
+        'public/path/Draft Folder',
+      );
+
+      const promise2 = await rerender({
+        currentFolder: 'path/to/Draft Folder/Sub Folder',
+      });
+
+      // Draft files/folders should appear immediately
+      expect(getByTestId('file-count').textContent).toBe('1');
+      expect(getByTestId('path/to/Draft Folder/Sub Folder/image.gif')).toBeInTheDocument();
+
+      await promise2;
+
+      expect(getByTestId('file-count').textContent).toBe('1');
+      expect(getByTestId('path/to/Draft Folder/Sub Folder/image.gif')).toBeInTheDocument();
+
+      expect(mockGetMedia).toHaveBeenCalledWith(
+        'path/to/Draft Folder/Sub Folder',
+        true,
+        'public/path/Draft Folder/Sub Folder',
+      );
+
+      const promise3 = await rerender({
+        currentFolder: 'path/to/Draft Folder',
+      });
+
+      // Draft files/folders should appear immediately
+      expect(getByTestId('file-count').textContent).toBe('1');
+      expect(getByTestId('path/to/Draft Folder/Sub Folder')).toBeInTheDocument();
+
+      await promise3;
+
+      expect(getByTestId('file-count').textContent).toBe('1');
+      expect(getByTestId('path/to/Draft Folder/Sub Folder')).toBeInTheDocument();
+
+      expect(mockGetMedia).toHaveBeenCalledWith(
+        'path/to/Draft Folder',
+        true,
+        'public/path/Draft Folder',
+      );
+    });
+  });
+});

packages/core/src/lib/hooks/useMediaAsset.ts

@@ -19,6 +19,7 @@ export default function useMediaAsset<T extends MediaField, EF extends BaseField
   field?: T,
   entry?: Entry,
   currentFolder?: string,
+  isDirectory?: boolean,
 ): string {
   const isAbsolute = useMemo(
     () => (isNotEmpty(url) ? /^(?:[a-z+]+:)?\/\//g.test(url) : false),
@@ -30,7 +31,7 @@ export default function useMediaAsset<T extends MediaField, EF extends BaseField
   const debouncedUrl = useDebounce(url, 200);
 
   useEffect(() => {
-    if (!debouncedUrl || isAbsolute || debouncedUrl.startsWith('blob:')) {
+    if (!debouncedUrl || isAbsolute || debouncedUrl.startsWith('blob:') || isDirectory) {
       return;
     }
 

packages/core/src/lib/hooks/useMediaFiles.ts

@@ -41,10 +41,6 @@ export default function useMediaFiles(field?: MediaField, currentFolder?: string
     let alive = true;
 
     const getMediaFiles = async () => {
-      if (entry.mediaFiles.find(f => dirname(f.path) == currentFolder)?.draft) {
-        setCurrentFolderMediaFiles([]);
-        return;
-      }
       const { media_folder, public_folder } = config ?? {};
       const backend = currentBackend(config);
       const files = await backend.getMedia(
@@ -61,6 +57,7 @@ export default function useMediaFiles(field?: MediaField, currentFolder?: string
     };
 
     getMediaFiles();
+    setCurrentFolderMediaFiles([]);
 
     return () => {
       alive = false;
@@ -68,46 +65,47 @@ export default function useMediaFiles(field?: MediaField, currentFolder?: string
   }, [currentFolder, config, entry, folderSupport]);
 
   const files = useMemo(() => {
-    if (entry) {
-      const entryFiles = entry.mediaFiles ?? [];
-      if (config) {
-        const mediaFolder = selectMediaFolder(config, collection, entry, field, currentFolder);
-        const entryFolderFiles = entryFiles
-          .filter(f => {
-            if (f.name === '.gitkeep') {
-              const folder = dirname(f.path);
-              return dirname(folder) === mediaFolder;
-            }
-
-            return dirname(f.path) === mediaFolder;
-          })
-          .map(file => {
-            if (file.name === '.gitkeep') {
-              const folder = dirname(file.path);
-              return {
-                key: folder,
-                id: folder,
-                name: basename(folder),
-                path: folder,
-                isDirectory: true,
-                draft: true,
-              } as MediaFile;
-            }
-            return { key: file.id, ...file };
-          });
-
-        if (currentFolderMediaFiles) {
-          if (entryFiles.length > 0) {
-            const draftFiles = entryFolderFiles.filter(file => file.draft == true);
-            currentFolderMediaFiles.unshift(...draftFiles);
-          }
-          return currentFolderMediaFiles.map(file => ({ key: file.id, ...file }));
-        }
-        return entryFolderFiles;
-      }
+    if (!entry || !config) {
+      return mediaLibraryFiles ?? [];
     }
 
-    return mediaLibraryFiles ?? [];
+    const entryFiles = entry.mediaFiles ?? [];
+    const mediaFolder = selectMediaFolder(config, collection, entry, field, currentFolder);
+    const entryFolderFiles = entryFiles
+      .filter(f => {
+        if (f.name === '.gitkeep') {
+          const folder = dirname(f.path);
+          return dirname(folder) === mediaFolder;
+        }
+
+        return dirname(f.path) === mediaFolder;
+      })
+      .map(file => {
+        if (file.name === '.gitkeep') {
+          const folder = dirname(file.path);
+          return {
+            key: folder,
+            id: folder,
+            name: basename(folder),
+            path: folder,
+            isDirectory: true,
+            draft: true,
+          } as MediaFile;
+        }
+        return { key: file.id, ...file };
+      });
+
+    if (currentFolderMediaFiles) {
+      const files = [...currentFolderMediaFiles];
+      if (entryFiles.length > 0) {
+        const draftFiles = entryFolderFiles.filter(
+          file => file.draft == true && !files.find(f => f.id === file.id),
+        );
+        files.unshift(...draftFiles);
+      }
+      return files.map(file => ({ key: file.id, ...file }));
+    }
+    return entryFolderFiles;
   }, [collection, config, currentFolderMediaFiles, entry, field, mediaLibraryFiles, currentFolder]);
 
   return useMemo(

packages/core/src/lib/util/getBlobSHA.ts

@@ -1,12 +1,12 @@
 import { sha256 } from 'js-sha256';
 
 export default (blob: Blob): Promise<string> =>
-  new Promise((resolve, reject) => {
+  new Promise(resolve => {
     const fr = new FileReader();
     fr.onload = ({ target }) => resolve(sha256(target?.result || ''));
-    fr.onerror = err => {
+    fr.onerror = () => {
       fr.abort();
-      reject(err);
+      resolve('');
     };
     fr.readAsArrayBuffer(blob);
   });

packages/core/test/data/fields.mock.ts

@@ -49,6 +49,12 @@ export const mockFileField: FileOrImageField = {
   widget: 'file',
 };
 
+export const mockImageField: FileOrImageField = {
+  label: 'Image',
+  name: 'mock_image',
+  widget: 'image',
+};
+
 export const mockMarkdownField: MarkdownField = {
   label: 'Body',
   name: 'body',