diff --git a/src/explorer/Explorer.test.tsx b/src/explorer/Explorer.test.tsx index c94c6b4d..533f86b1 100644 --- a/src/explorer/Explorer.test.tsx +++ b/src/explorer/Explorer.test.tsx @@ -6,7 +6,6 @@ import { cleanup } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; import { testRender, uuid } from '../../test'; -import { FileMetadata } from '../fileStorage/actions'; import Explorer from './Explorer'; import { explorerArchiveAllFiles, @@ -15,6 +14,7 @@ import { explorerImportFiles, explorerRenameFile, } from './actions'; +import { ExplorerFileInfo } from './reducers'; afterEach(async () => { jest.restoreAllMocks(); @@ -22,10 +22,9 @@ afterEach(async () => { localStorage.clear(); }); -const testFile: FileMetadata = { - uuid: uuid(0), - path: 'test.file', - sha256: '', +const testFile: ExplorerFileInfo = { + id: uuid(0), + name: 'test.file', }; describe('archive button', () => { diff --git a/src/explorer/Explorer.tsx b/src/explorer/Explorer.tsx index 59a15701..38cda4d5 100644 --- a/src/explorer/Explorer.tsx +++ b/src/explorer/Explorer.tsx @@ -290,12 +290,12 @@ const FileTree: React.VoidFunctionComponent = ({ i18n }) => { () => files.reduce( (obj, file) => { - const index = file.uuid; + const index = file.id; obj[index] = { index, data: { - fileName: file.path, + fileName: file.name, icon: 'document', secondaryLabel: ( @@ -319,8 +319,8 @@ const FileTree: React.VoidFunctionComponent = ({ i18n }) => { hasChildren: true, children: [...files] // REVISIT: consider using Intl.Collator() for i18n.locale - .sort((a, b) => a.path.localeCompare(b.path)) - .map((n) => n.uuid), + .sort((a, b) => a.name.localeCompare(b.name)) + .map((n) => n.id), }, } as Record, ), diff --git a/src/explorer/newFileWizard/NewFileWizard.tsx b/src/explorer/newFileWizard/NewFileWizard.tsx index d09b5bd0..3b47487e 100644 --- a/src/explorer/newFileWizard/NewFileWizard.tsx +++ b/src/explorer/newFileWizard/NewFileWizard.tsx @@ -45,7 +45,7 @@ const NewFileWizard: React.VoidFunctionComponent = ({ const fileNameValidation = validateFileName( fileName, pythonFileExtension, - files.map((f) => f.path), + files.map((f) => f.name), ); const [hubType, setHubType] = useState(defaultHub); diff --git a/src/explorer/reducers.test.ts b/src/explorer/reducers.test.ts index f65e2c8c..ebc1ed7a 100644 --- a/src/explorer/reducers.test.ts +++ b/src/explorer/reducers.test.ts @@ -9,50 +9,84 @@ import { fileStorageDidInitialize, fileStorageDidRemoveItem, } from '../fileStorage/actions'; -import reducers from './reducers'; +import reducers, { ExplorerFileInfo } from './reducers'; type State = ReturnType; -test('files', () => { - const testFile: FileMetadata = { +describe('files', () => { + const testFile: ExplorerFileInfo = { + id: uuid(0), + name: 'test.file', + }; + + const testFileMetadata: FileMetadata = { uuid: uuid(0), path: 'test.file', sha256: '', }; - const modifiedFile: FileMetadata = { ...testFile, path: 'modified.file' }; + const modifiedFile: ExplorerFileInfo = { + ...testFile, + name: 'modified.file', + }; - expect(testFile).not.toEqual(modifiedFile); + const modifiedFileMetadata: FileMetadata = { + ...testFileMetadata, + path: 'modified.file', + }; - // initialization populates file list - expect( - reducers( - { files: [] as readonly FileMetadata[] } as State, - fileStorageDidInitialize([testFile]), - ).files, - ).toEqual([testFile]); + beforeAll(() => { + // check validity of test data before starting tests + expect(testFile).not.toEqual(modifiedFile); + expect(testFileMetadata).not.toEqual(modifiedFileMetadata); + }); - // adding appends an item - expect( - reducers( - { files: [] as readonly FileMetadata[] } as State, - fileStorageDidAddItem(testFile), - ).files, - ).toEqual([testFile]); + it('should get a list when file storage is initialized', () => { + expect( + reducers( + { files: [] as readonly ExplorerFileInfo[] } as State, + fileStorageDidInitialize([testFileMetadata]), + ).files, + ).toEqual([testFile]); + }); - // changing replaces an item - expect( - reducers( - { files: [testFile] as readonly FileMetadata[] } as State, - fileStorageDidChangeItem(testFile, modifiedFile), - ).files, - ).toEqual([modifiedFile]); + it('should modify the list when a file is added to storage', () => { + expect( + reducers( + { files: [] as readonly ExplorerFileInfo[] } as State, + fileStorageDidAddItem(testFileMetadata), + ).files, + ).toEqual([testFile]); + }); - // removing deletes an item - expect( - reducers( - { files: [testFile] as readonly FileMetadata[] } as State, - fileStorageDidRemoveItem(testFile), - ).files, - ).not.toContain(testFile); + it('should modify the list when an item is renamed in storage', () => { + expect( + reducers( + { files: [testFile] as readonly ExplorerFileInfo[] } as State, + fileStorageDidChangeItem(testFileMetadata, modifiedFileMetadata), + ).files, + ).toEqual([modifiedFile]); + }); + + it('should not modify the list if a change is made other than renaming', () => { + const originalList: readonly ExplorerFileInfo[] = [testFile]; + expect( + reducers( + { files: originalList } as State, + fileStorageDidChangeItem(testFileMetadata, { + ...testFileMetadata, + sha256: 'changed', + }), + ).files, + ).toBe(originalList); + }); + + it('should modify the list when a file is removed from storage', () => { + expect( + reducers( + { files: [testFile] as readonly ExplorerFileInfo[] } as State, + fileStorageDidRemoveItem(testFileMetadata), + ).files, + ).not.toContain(testFile); + }); }); diff --git a/src/explorer/reducers.ts b/src/explorer/reducers.ts index d820af8c..cda4c2e7 100644 --- a/src/explorer/reducers.ts +++ b/src/explorer/reducers.ts @@ -12,21 +12,40 @@ import { import renameFileDialog from './renameFileDialog/reducers'; -const files: Reducer = (state = [], action) => { +export type ExplorerFileInfo = Readonly<{ + /** A unique identifier for this file (not the path, which can change). */ + id: string; + /** The file name (including extension - without directory). */ + name: string; +}>; + +function metadataToInfo(file: FileMetadata): ExplorerFileInfo { + return { id: file.uuid, name: file.path }; +} + +const files: Reducer = (state = [], action) => { if (fileStorageDidInitialize.matches(action)) { - return [...action.files]; + return action.files.map(metadataToInfo); } if (fileStorageDidAddItem.matches(action)) { - return [...state, action.file]; + return [...state, metadataToInfo(action.file)]; } if (fileStorageDidChangeItem.matches(action)) { - return [...state].map((f) => (f.uuid === action.file.uuid ? action.file : f)); + // We only care about UUID and the path. UUID can't change so if the + // path didn't change, there is nothing to do. + if (action.oldFile.path === action.file.path) { + return state; + } + + return state.map((f) => + f.id === action.file.uuid ? metadataToInfo(action.file) : f, + ); } if (fileStorageDidRemoveItem.matches(action)) { - return [...state].filter((value) => value.uuid !== action.file.uuid); + return state.filter((value) => value.id !== action.file.uuid); } return state; diff --git a/src/explorer/renameFileDialog/RenameFileDialog.tsx b/src/explorer/renameFileDialog/RenameFileDialog.tsx index 34ba7672..bfabcf0a 100644 --- a/src/explorer/renameFileDialog/RenameFileDialog.tsx +++ b/src/explorer/renameFileDialog/RenameFileDialog.tsx @@ -28,7 +28,7 @@ const RenameFileDialog: React.VFC = () => { const result = validateFileName( newName, extension, - files.map((f) => f.path), + files.map((f) => f.name), ); const inputRef = useRef(null); diff --git a/src/explorer/sagas.ts b/src/explorer/sagas.ts index bcfc7e47..bff37894 100644 --- a/src/explorer/sagas.ts +++ b/src/explorer/sagas.ts @@ -125,7 +125,7 @@ function* handleExplorerImportFiles(): Generator { const result = validateFileName( baseName, pythonFileExtension, - existingFiles.map((f) => f.path), + existingFiles.map((f) => f.name), ); if (result != FileNameValidationResult.IsOk) {