From 93111b6af49e28b7aee9fbb4732242b842c26787 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 11 Mar 2022 15:40:42 -0600 Subject: [PATCH] explorer: implement import action --- craco.config.js | 3 +- package.json | 1 + src/explorer/Explorer.test.tsx | 13 ++++++- src/explorer/Explorer.tsx | 4 +- src/explorer/actions.ts | 23 +++++++++++ src/explorer/sagas.test.ts | 53 ++++++++++++++++++++++++- src/explorer/sagas.ts | 68 ++++++++++++++++++++++++++++++++- src/notifications/i18n.en.json | 3 +- src/notifications/i18n.ts | 1 + src/notifications/sagas.test.ts | 4 +- src/notifications/sagas.ts | 14 ++++++- yarn.lock | 5 +++ 12 files changed, 182 insertions(+), 10 deletions(-) diff --git a/craco.config.js b/craco.config.js index 92f930e0..93d95733 100644 --- a/craco.config.js +++ b/craco.config.js @@ -276,8 +276,9 @@ module.exports = { '[/\\\\]node_modules[/\\\\].+\\.(js|jsx|mjs|cjs|ts|tsx)$', ); // https://github.com/react-monaco-editor/react-monaco-editor/issues/306#issuecomment-701025628 + // https://github.com/GoogleChromeLabs/browser-fs-access/issues/42 jestConfig.transformIgnorePatterns[index] = - '[/\\\\]node_modules[/\\\\](?!(monaco-editor|react-monaco-editor)[/\\\\]).+\\.(js|jsx|mjs|cjs|ts|tsx)$'; + '[/\\\\]node_modules[/\\\\](?!(monaco-editor|react-monaco-editor|browser-fs-access)[/\\\\]).+\\.(js|jsx|mjs|cjs|ts|tsx)$'; return jestConfig; }, }, diff --git a/package.json b/package.json index eea4068f..237c5e76 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ "@types/wicg-file-system-access": "^2020.9.5", "@types/zen-push": "^0.1.1", "babel-plugin-macros": "^3.0.1", + "browser-fs-access": "^0.25.0", "canvas": "^2.9.0", "copy-webpack-plugin": "^6.4.1", "file-saver": "^2.0.5", diff --git a/src/explorer/Explorer.test.tsx b/src/explorer/Explorer.test.tsx index 6d99075b..e226f6f6 100644 --- a/src/explorer/Explorer.test.tsx +++ b/src/explorer/Explorer.test.tsx @@ -10,7 +10,7 @@ import { fileStorageExportFile, } from '../fileStorage/actions'; import Explorer from './Explorer'; -import { explorerDeleteFile } from './actions'; +import { explorerDeleteFile, explorerImportFiles } from './actions'; describe('archive button', () => { it('should be enabled if there are files', () => { @@ -38,6 +38,17 @@ describe('archive button', () => { }); }); +describe('import file button', () => { + it('should dispatch action when clicked', async () => { + const [explorer, dispatch] = testRender(); + + const button = explorer.getByTitle('Import a file'); + + userEvent.click(button); + expect(dispatch).toHaveBeenCalledWith(explorerImportFiles()); + }); +}); + describe('new file button', () => { it('should show new file wizard', async () => { const [explorer] = testRender(); diff --git a/src/explorer/Explorer.tsx b/src/explorer/Explorer.tsx index c71af072..cbaa254f 100644 --- a/src/explorer/Explorer.tsx +++ b/src/explorer/Explorer.tsx @@ -20,7 +20,7 @@ import { } from '../fileStorage/actions'; import { useSelector } from '../reducers'; import NewFileWizard from './NewFileWizard'; -import { explorerDeleteFile } from './actions'; +import { explorerDeleteFile, explorerImportFiles } from './actions'; import { ExplorerStringId } from './i18n'; import en from './i18n.en.json'; @@ -122,7 +122,7 @@ const Header: React.VFC = () => { // even though this is the "import" action icon="export" toolTipId={ExplorerStringId.HeaderImportTooltip} - onClick={() => alert('not implemented')} + onClick={() => dispatch(explorerImportFiles())} /> ({ + type: 'explorer.action.importFiles', +})); + +/** + * Action that indicates that explorerImportFiles() succeeded. + */ +export const explorerDidImportFiles = createAction(() => ({ + type: 'explorer.action.didImportFiles', +})); + +/** + * Action that indicates that explorerImportFiles() failed. + * @param error The error. + */ +export const explorerDidFailToImportFiles = createAction((error: Error) => ({ + type: 'explorer.action.didFailToImportFiles', + error, +})); + /** * Action that requests to create a new file. * @param fileName The requested new file name (without file extension). diff --git a/src/explorer/sagas.test.ts b/src/explorer/sagas.test.ts index a131fa61..626124bb 100644 --- a/src/explorer/sagas.test.ts +++ b/src/explorer/sagas.test.ts @@ -1,11 +1,62 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2022 The Pybricks Authors +import * as browserFsAccess from 'browser-fs-access'; +import { FileWithHandle } from 'browser-fs-access'; +import { mock } from 'jest-mock-extended'; import { AsyncSaga } from '../../test'; +import { fileStorageWriteFile } from '../fileStorage/actions'; import { pythonFileExtension } from '../pybricksMicropython/lib'; -import { Hub, explorerCreateNewFile } from './actions'; +import { + Hub, + explorerCreateNewFile, + explorerDidFailToImportFiles, + explorerDidImportFiles, + explorerImportFiles, +} from './actions'; import explorer from './sagas'; +describe('handleExplorerImportFiles', () => { + it('should write file to storage', async () => { + const testFileName = 'test.py'; + const testFileContents = '# test'; + + const saga = new AsyncSaga(explorer, { fileStorage: { fileNames: [] } }); + + jest.spyOn(browserFsAccess, 'fileOpen').mockResolvedValueOnce([ + mock({ + name: testFileName, + text: () => Promise.resolve(testFileContents), + }), + ]); + + saga.put(explorerImportFiles()); + + const action = await saga.take(); + expect(action).toEqual(fileStorageWriteFile(testFileName, testFileContents)); + + const action2 = await saga.take(); + expect(action2).toEqual(explorerDidImportFiles()); + + await saga.end(); + }); + + it('should handle user cancellation', async () => { + const cancelError = new DOMException('test message', 'AbortError'); + + const saga = new AsyncSaga(explorer); + + jest.spyOn(browserFsAccess, 'fileOpen').mockRejectedValueOnce(cancelError); + + saga.put(explorerImportFiles()); + + const action = await saga.take(); + expect(action).toEqual(explorerDidFailToImportFiles(cancelError)); + + await saga.end(); + }); +}); + describe('handleExplorerCreateNewFile', () => { it('should dispatch fileStorage action', async () => { const saga = new AsyncSaga(explorer); diff --git a/src/explorer/sagas.ts b/src/explorer/sagas.ts index 919a3299..e73daac4 100644 --- a/src/explorer/sagas.ts +++ b/src/explorer/sagas.ts @@ -1,10 +1,73 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2022 The Pybricks Authors -import { put, takeEvery } from 'typed-redux-saga/macro'; +import { fileOpen } from 'browser-fs-access'; +import { call, put, select, takeEvery } from 'typed-redux-saga/macro'; import { getPybricksMicroPythonFileTemplate } from '../editor/pybricksMicroPython'; import { fileStorageWriteFile } from '../fileStorage/actions'; -import { explorerCreateNewFile } from './actions'; +import { + FileNameValidationResult, + pythonFileExtension, + pythonFileExtensionRegex, + pythonFileMimeType, + validateFileName, +} from '../pybricksMicropython/lib'; +import { RootState } from '../reducers'; +import { ensureError } from '../utils'; +import { + explorerCreateNewFile, + explorerDidFailToImportFiles, + explorerDidImportFiles, + explorerImportFiles, +} from './actions'; + +function* handleExplorerImportFiles(): Generator { + try { + const selectedFiles = yield* call(() => + fileOpen({ + id: 'pybricks-code-explorer-import', + mimeTypes: [pythonFileMimeType], + extensions: [pythonFileExtension], + // TODO: translate description + description: 'Python Files', + multiple: true, + excludeAcceptAllOption: true, + }), + ); + + for (const file of selectedFiles) { + // getting the text now to catch possible error *before* user interaction + const text = yield* call(() => file.text()); + + const [baseName] = file.name.split(pythonFileExtensionRegex); + const existingFiles = yield* select( + (s: RootState) => s.fileStorage.fileNames, + ); + + const result = validateFileName( + baseName, + pythonFileExtension, + existingFiles, + ); + + if (result != FileNameValidationResult.IsOk) { + // TODO: validate file name and allow user to rename or skip + console.error( + 'skipping file', + file.name, + FileNameValidationResult[result], + ); + continue; + } + + yield* put(fileStorageWriteFile(`${baseName}${pythonFileExtension}`, text)); + } + + yield* put(explorerDidImportFiles()); + } catch (err) { + yield* put(explorerDidFailToImportFiles(ensureError(err))); + } +} function* handleExplorerCreateNewFile( action: ReturnType, @@ -20,5 +83,6 @@ function* handleExplorerCreateNewFile( } export default function* (): Generator { + yield* takeEvery(explorerImportFiles, handleExplorerImportFiles); yield* takeEvery(explorerCreateNewFile, handleExplorerCreateNewFile); } diff --git a/src/notifications/i18n.en.json b/src/notifications/i18n.en.json index 063e8ec8..a3792ab8 100644 --- a/src/notifications/i18n.en.json +++ b/src/notifications/i18n.en.json @@ -18,7 +18,8 @@ "deleteFile": { "message": "The file {fileName} will be permanently deleted. This cannot be undone.", "action": "Delete" - } + }, + "failedToImportFiles": "Failed to import file(s)" }, "fileStorage": { "failedToInitialize": "Failed to initial file storage. Changes will not be automatically saved.", diff --git a/src/notifications/i18n.ts b/src/notifications/i18n.ts index e04f2388..7235dfc1 100644 --- a/src/notifications/i18n.ts +++ b/src/notifications/i18n.ts @@ -15,6 +15,7 @@ export enum MessageId { EditorFailedToSaveFile = 'editor.failedToSaveFile', ExplorerDeleteFileMessage = 'explorer.deleteFile.message', ExplorerDeleteFileAction = 'explorer.deleteFile.action', + ExplorerFailedToImportFiles = 'explorer.failedToImportFiles', FileStorageFailedToInitialize = 'fileStorage.failedToInitialize', FileStorageFailedToRead = 'fileStorage.failedToRead', FileStorageFailedToWrite = 'fileStorage.failedToWrite', diff --git a/src/notifications/sagas.test.ts b/src/notifications/sagas.test.ts index 4d69966d..c22e808b 100644 --- a/src/notifications/sagas.test.ts +++ b/src/notifications/sagas.test.ts @@ -17,7 +17,7 @@ import { didFailToConnect as bleDidFailToConnect, } from '../ble/actions'; import { didFailToSaveAs } from '../editor/actions'; -import { explorerDeleteFile } from '../explorer/actions'; +import { explorerDeleteFile, explorerDidFailToImportFiles } from '../explorer/actions'; import { fileStorageDeleteFile, fileStorageDidFailToArchiveAllFiles, @@ -118,6 +118,7 @@ test.each([ fileStorageDidFailToDeleteFile('test.file', new Error('test error')), fileStorageDidFailToExportFile('test.file', new Error('test error')), fileStorageDidFailToArchiveAllFiles(new Error('test error')), + explorerDidFailToImportFiles(new Error('test error')), ])('actions that should show notification: %o', async (action: AnyAction) => { const { toaster, saga } = createTestToasterSaga(); @@ -143,6 +144,7 @@ test.each([ new DOMException('test message', 'AbortError'), ), fileStorageDidFailToArchiveAllFiles(new DOMException('test message', 'AbortError')), + explorerDidFailToImportFiles(new DOMException('test message', 'AbortError')), ])('actions that should not show a notification: %o', async (action: AnyAction) => { const { toaster, saga } = createTestToasterSaga(); diff --git a/src/notifications/sagas.ts b/src/notifications/sagas.ts index f109b0c3..8fc40f1e 100644 --- a/src/notifications/sagas.ts +++ b/src/notifications/sagas.ts @@ -18,7 +18,7 @@ import { didFailToConnect as bleDeviceDidFailToConnect, } from '../ble/actions'; import { didFailToSaveAs } from '../editor/actions'; -import { explorerDeleteFile } from '../explorer/actions'; +import { explorerDeleteFile, explorerDidFailToImportFiles } from '../explorer/actions'; import { fileStorageDeleteFile, fileStorageDidFailToArchiveAllFiles, @@ -480,6 +480,17 @@ function* showDeleteFileWarning(action: ReturnType) { yield* put(fileStorageDeleteFile(action.fileName)); } +function* showExplorerFailToImportFiles( + action: ReturnType, +): Generator { + if (action.error.name === 'AbortError') { + // user clicked cancel button - not an error + return; + } + + yield* showUnexpectedError(MessageId.ExplorerFailedToImportFiles, action.error); +} + export default function* (): Generator { yield* takeEvery(bleDeviceDidFailToConnect, showBleDeviceDidFailToConnectError); yield* takeEvery(bootloaderDidFailToConnect, showBootloaderDidFailToConnectError); @@ -498,4 +509,5 @@ export default function* (): Generator { yield* takeEvery(fileStorageDidFailToExportFile, showFileStorageFailToExport); yield* takeEvery(fileStorageDidFailToArchiveAllFiles, showFileStorageFailToArchive); yield* takeEvery(explorerDeleteFile, showDeleteFileWarning); + yield* takeEvery(explorerDidFailToImportFiles, showExplorerFailToImportFiles); } diff --git a/yarn.lock b/yarn.lock index 1178ec04..0c0a6bf5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3369,6 +3369,11 @@ brorand@^1.0.1, brorand@^1.1.0: resolved "https://registry.yarnpkg.com/brorand/-/brorand-1.1.0.tgz#12c25efe40a45e3c323eb8675a0a0ce57b22371f" integrity sha1-EsJe/kCkXjwyPrhnWgoM5XsiNx8= +browser-fs-access@^0.25.0: + version "0.25.0" + resolved "https://registry.yarnpkg.com/browser-fs-access/-/browser-fs-access-0.25.0.tgz#3b32fd86cc6e3ae18136225acdcae932e3b3fb43" + integrity sha512-ovMv4bO/+dPXTqpBpzvrW3N66oJhhNkGZZpE+1vZhQDWmU0FOTc2lD2T0AOuaFOSV0LV71j4+UFnYE1RW2XY2Q== + browser-process-hrtime@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/browser-process-hrtime/-/browser-process-hrtime-1.0.0.tgz#3c9b4b7d782c8121e56f10106d84c0d0ffc94626"