diff --git a/CHANGELOG.md b/CHANGELOG.md index 04c90597..f7acf664 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,10 +8,12 @@ - Fixed run button enabled when no file open ([support#691]). - Fixed flash firmware dialog not showing when settings not open ([support#694]). - Fixed errors not handled while flashing firmware via USB ([pybricks-code#1011]). +- Fixed imports with invalid file name silently ignored ([support#717]). [pybricks-code#1011]: https://github.com/pybricks/pybricks-code/issues/1011 [support#691]: https://github.com/pybricks/support/issues/691 [support#694]: https://github.com/pybricks/support/issues/694 +[support#717]: https://github.com/pybricks/support/issues/717 ## [2.0.0-beta.5] - 2022-07-28 diff --git a/src/explorer/Explorer.tsx b/src/explorer/Explorer.tsx index 0c33fe54..b453bb1b 100644 --- a/src/explorer/Explorer.tsx +++ b/src/explorer/Explorer.tsx @@ -45,6 +45,7 @@ import DuplicateFileDialog from './duplicateFileDialog/DuplicateFileDialog'; import { useI18n } from './i18n'; import NewFileWizard from './newFileWizard/NewFileWizard'; import RenameFileDialog from './renameFileDialog/RenameFileDialog'; +import RenameImportDialog from './renameImportDialog/RenameImportDialog'; type ActionButtonProps = { /** The DOM id for this instance. */ @@ -417,6 +418,7 @@ const Explorer: React.VFC = () => { + diff --git a/src/explorer/reducers.ts b/src/explorer/reducers.ts index ea8599cd..b4395b91 100644 --- a/src/explorer/reducers.ts +++ b/src/explorer/reducers.ts @@ -7,10 +7,12 @@ import deleteFileAlert from './deleteFileAlert/reducers'; import duplicateFileDialog from './duplicateFileDialog/reducers'; import newFileWizard from './newFileWizard/reducers'; import renameFileDialog from './renameFileDialog/reducers'; +import renameImportDialog from './renameImportDialog/reducers'; export default combineReducers({ duplicateFileDialog, deleteFileAlert, newFileWizard, renameFileDialog, + renameImportDialog, }); diff --git a/src/explorer/renameImportDialog/RenameImportDialog.test.tsx b/src/explorer/renameImportDialog/RenameImportDialog.test.tsx new file mode 100644 index 00000000..08712c4e --- /dev/null +++ b/src/explorer/renameImportDialog/RenameImportDialog.test.tsx @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { waitFor } from '@testing-library/dom'; +import React from 'react'; +import { testRender } from '../../../test'; +import RenameImportDialog from './RenameImportDialog'; +import { renameImportDialogDidAccept, renameImportDialogDidCancel } from './actions'; + +describe('rename button', () => { + it('should accept the dialog Rename is clicked', async () => { + const [user, dialog, dispatch] = testRender(, { + explorer: { renameImportDialog: { isOpen: true, fileName: 'old.file' } }, + }); + + const button = dialog.getByRole('button', { name: 'Rename' }); + + // have to type a new file name before Rename button is enabled + const input = dialog.getByLabelText('File name'); + await waitFor(() => expect(input).toHaveFocus()); + await user.type(input, 'new', { skipClick: true }); + await waitFor(() => expect(button).not.toBeDisabled()); + + await user.click(button); + expect(dispatch).toHaveBeenCalledWith( + renameImportDialogDidAccept('old.file', 'new.file'), + ); + }); + + it('should accept the dialog when enter is pressed in the text input', async () => { + const [user, dialog, dispatch] = testRender(, { + explorer: { renameImportDialog: { isOpen: true, fileName: 'old.file' } }, + }); + + // have to type a new file name before Rename button is enabled + const input = dialog.getByLabelText('File name'); + await waitFor(() => expect(input).toHaveFocus()); + await user.type(input, 'new{Enter}', { skipClick: true }); + + expect(dispatch).toHaveBeenCalledWith( + renameImportDialogDidAccept('old.file', 'new.file'), + ); + }); + + it('should cancel when close button is clicked', async () => { + const [user, dialog, dispatch] = testRender(, { + explorer: { renameImportDialog: { isOpen: true } }, + }); + + const button = dialog.getByRole('button', { name: 'Close' }); + + await waitFor(() => expect(button).toBeVisible()); + + await user.click(button); + expect(dispatch).toHaveBeenCalledWith(renameImportDialogDidCancel()); + }); + + it('should cancel when skip button is clicked', async () => { + const [user, dialog, dispatch] = testRender(, { + explorer: { renameImportDialog: { isOpen: true } }, + }); + + const button = dialog.getByRole('button', { name: 'Skip importing this file' }); + + await waitFor(() => expect(button).toBeVisible()); + + await user.click(button); + expect(dispatch).toHaveBeenCalledWith(renameImportDialogDidCancel()); + }); +}); diff --git a/src/explorer/renameImportDialog/RenameImportDialog.tsx b/src/explorer/renameImportDialog/RenameImportDialog.tsx new file mode 100644 index 00000000..9800923d --- /dev/null +++ b/src/explorer/renameImportDialog/RenameImportDialog.tsx @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { Button, Classes, Dialog } from '@blueprintjs/core'; +import React, { useCallback, useRef, useState } from 'react'; +import { useDispatch } from 'react-redux'; +import { appName } from '../../app/constants'; +import { useFileStorageMetadata } from '../../fileStorage/hooks'; +import { + FileNameValidationResult, + validateFileName, +} from '../../pybricksMicropython/lib'; +import { useSelector } from '../../reducers'; +import FileNameFormGroup from '../fileNameFormGroup/FileNameFormGroup'; +import { renameImportDialogDidAccept, renameImportDialogDidCancel } from './actions'; +import { useI18n } from './i18n'; + +const RenameImportDialog: React.VFC = () => { + const i18n = useI18n(); + const dispatch = useDispatch(); + const isOpen = useSelector((s) => s.explorer.renameImportDialog.isOpen); + const oldName = useSelector((s) => s.explorer.renameImportDialog.fileName); + + const [baseName, extension] = oldName.split(/(\.\w+)$/); + + const [newName, setNewName] = useState(baseName); + const files = useFileStorageMetadata() ?? []; + const result = validateFileName( + newName, + extension, + files.map((f) => f.path), + ); + + const inputRef = useRef(null); + + const handleSubmit = useCallback( + (e) => { + e.preventDefault(); + dispatch(renameImportDialogDidAccept(oldName, `${newName}${extension}`)); + }, + [dispatch, oldName, newName, extension], + ); + + const handleClose = useCallback(() => { + dispatch(renameImportDialogDidCancel()); + }, [dispatch]); + + return ( + setNewName(baseName)} + onOpened={() => { + inputRef.current?.select(); + inputRef.current?.focus(); + }} + onClose={handleClose} + > +
+
+

{i18n.translate('message', { fileName: oldName, appName })}

+ +
+
+
+ + +
+
+
+ + ); +}; + +export default RenameImportDialog; diff --git a/src/explorer/renameImportDialog/actions.ts b/src/explorer/renameImportDialog/actions.ts new file mode 100644 index 00000000..2d4dd818 --- /dev/null +++ b/src/explorer/renameImportDialog/actions.ts @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { createAction } from '../../actions'; + +/** + * Action that requests to show the rename file dialog. + * @param oldName The old file name. + */ +export const renameImportDialogShow = createAction((oldName: string) => ({ + type: 'explorer.renameImportDialog.action.show', + oldName, +})); + +/** + * Action that indicates the rename file dialog was accepted. + * @param oldName The old file name. + * @param newName The new file name. + */ +export const renameImportDialogDidAccept = createAction( + (oldName: string, newName: string) => ({ + type: 'explorer.renameImportDialog.action.didAccept', + oldName, + newName, + }), +); + +/** + * Action that indicates the rename file dialog was canceled. + */ +export const renameImportDialogDidCancel = createAction(() => ({ + type: 'explorer.renameImportDialog.action.didCancel', +})); diff --git a/src/explorer/renameImportDialog/i18n.ts b/src/explorer/renameImportDialog/i18n.ts new file mode 100644 index 00000000..eb8dc486 --- /dev/null +++ b/src/explorer/renameImportDialog/i18n.ts @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { useI18n as useShopifyI18n } from '@shopify/react-i18n'; +import type { TypedI18n } from '../../i18n'; +import type translations from './translations/en.json'; + +export function useI18n(): TypedI18n { + // istanbul ignore next: babel-loader rewrites this line + const [i18n] = useShopifyI18n(); + return i18n; +} diff --git a/src/explorer/renameImportDialog/reducers.ts b/src/explorer/renameImportDialog/reducers.ts new file mode 100644 index 00000000..faf79100 --- /dev/null +++ b/src/explorer/renameImportDialog/reducers.ts @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { Reducer, combineReducers } from 'redux'; +import { + renameImportDialogDidAccept, + renameImportDialogDidCancel, + renameImportDialogShow, +} from './actions'; + +/** Controls the rename file dialog isOpen state. */ +const isOpen: Reducer = (state = false, action) => { + if (renameImportDialogShow.matches(action)) { + return true; + } + + if ( + renameImportDialogDidAccept.matches(action) || + renameImportDialogDidCancel.matches(action) + ) { + return false; + } + + return state; +}; + +/** Controls the rename file dialog file name input box text. */ +const fileName: Reducer = (state = '', action) => { + if (renameImportDialogShow.matches(action)) { + return action.oldName; + } + + return state; +}; + +export default combineReducers({ isOpen, fileName }); diff --git a/src/explorer/renameImportDialog/translations/en.json b/src/explorer/renameImportDialog/translations/en.json new file mode 100644 index 00000000..3f63720c --- /dev/null +++ b/src/explorer/renameImportDialog/translations/en.json @@ -0,0 +1,8 @@ +{ + "title": "Rename imported file", + "message": "The name of the imported file '{fileName}' is not allowed in {appName}. Please change the name below.", + "action": { + "skip": "Skip importing this file", + "rename": "Rename" + } +} diff --git a/src/explorer/sagas.test.ts b/src/explorer/sagas.test.ts index 528618d7..f58afd86 100644 --- a/src/explorer/sagas.test.ts +++ b/src/explorer/sagas.test.ts @@ -82,6 +82,10 @@ import { renameFileDialogDidCancel, renameFileDialogShow, } from './renameFileDialog/actions'; +import { + renameImportDialogDidAccept, + renameImportDialogShow, +} from './renameImportDialog/actions'; import explorer from './sagas'; jest.mock('browser-fs-access'); @@ -226,6 +230,40 @@ describe('handleExplorerImportFiles', () => { await saga.end(); }); + + it('should handle invalid file name', async () => { + const testFileName = 'bad#name.py'; + const testFileContents = '# test'; + + const saga = new AsyncSaga(explorer); + + jest.spyOn(browserFsAccess, 'fileOpen').mockResolvedValueOnce([ + mock({ + name: testFileName, + text: () => Promise.resolve(testFileContents), + }), + ]); + + saga.put(explorerImportFiles()); + + await expect(saga.take()).resolves.toEqual( + renameImportDialogShow(testFileName), + ); + + const renamedFileName = 'good_name.py'; + + saga.put(renameImportDialogDidAccept(testFileName, renamedFileName)); + + await expect(saga.take()).resolves.toEqual( + fileStorageWriteFile(renamedFileName, testFileContents), + ); + + saga.put(fileStorageDidWriteFile(renamedFileName, uuid(0))); + + await expect(saga.take()).resolves.toEqual(explorerDidImportFiles()); + + await saga.end(); + }); }); describe('handleExplorerCreateNewFile', () => { diff --git a/src/explorer/sagas.ts b/src/explorer/sagas.ts index a6460b6d..40278e4a 100644 --- a/src/explorer/sagas.ts +++ b/src/explorer/sagas.ts @@ -89,6 +89,11 @@ import { renameFileDialogDidCancel, renameFileDialogShow, } from './renameFileDialog/actions'; +import { + renameImportDialogDidAccept, + renameImportDialogDidCancel, + renameImportDialogShow, +} from './renameImportDialog/actions'; function* handleExplorerArchiveAllFiles(): Generator { try { @@ -166,20 +171,26 @@ function* handleExplorerImportFiles(): Generator { const text = yield* call(() => file.text()); const [baseName] = file.name.split(pythonFileExtensionRegex); + let fileName = `${baseName}${pythonFileExtension}`; const result = validateFileName(baseName, pythonFileExtension, []); 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(renameImportDialogShow(file.name)); - const fileName = `${baseName}${pythonFileExtension}`; + const { accepted, cancelled } = yield* race({ + accepted: take(renameImportDialogDidAccept), + cancelled: take(renameImportDialogDidCancel), + }); + + if (cancelled) { + continue; + } + + defined(accepted); + + fileName = accepted.newName; + } yield* put(fileStorageWriteFile(fileName, text));