From 7232bf615326316085c031146e1511cacd29d52e Mon Sep 17 00:00:00 2001 From: David Lechner Date: Wed, 6 Jul 2022 16:53:07 -0500 Subject: [PATCH] add better error message when no files to backup Instead of showing unexpected error, show message explaining that there are no files to be backed up and how to fix it. Fixes: https://github.com/pybricks/support/issues/681 --- CHANGELOG.md | 5 ++++ src/alerts/actions.ts | 4 ++- src/explorer/alerts/NoFilesToBackup.tsx | 27 ++++++++++++++++++++ src/explorer/alerts/i18n.ts | 1 + src/explorer/alerts/index.ts | 3 ++- src/explorer/alerts/translations/en.json | 3 +++ src/explorer/error.ts | 10 ++++++++ src/explorer/sagas.test.ts | 32 +++++++++++++++++++++++- src/explorer/sagas.ts | 16 ++++++++++-- src/notifications/sagas.test.ts | 3 --- src/notifications/sagas.ts | 13 ---------- 11 files changed, 96 insertions(+), 21 deletions(-) create mode 100644 src/explorer/alerts/NoFilesToBackup.tsx create mode 100644 src/explorer/error.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b07d63c..149020ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,14 @@ ## [Unreleased] +### Added +- Added better error message when no files to backup ([support#681]). + ### Fixed - Fixed deleting files that are not open in the editor. +[support#681]: https://github.com/pybricks/support/issues/681 + ## [2.0.0-beta.3] - 2022-07-06 ### Changed diff --git a/src/alerts/actions.ts b/src/alerts/actions.ts index 70c630fb..e50238e6 100644 --- a/src/alerts/actions.ts +++ b/src/alerts/actions.ts @@ -15,7 +15,9 @@ export const alertsShowAlert = createAction( >( domain: D, specific: S, - props: AlertProps, + ...props: AlertProps extends never + ? [props?: never] + : [props: AlertProps] ) => ({ type: 'alerts.action.showAlert', domain, diff --git a/src/explorer/alerts/NoFilesToBackup.tsx b/src/explorer/alerts/NoFilesToBackup.tsx new file mode 100644 index 00000000..66293783 --- /dev/null +++ b/src/explorer/alerts/NoFilesToBackup.tsx @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { Icon, Intent } from '@blueprintjs/core'; +import React from 'react'; +import { CreateToast } from '../../i18nToaster'; +import { I18nId, useI18n } from './i18n'; + +const NoFilesToBackup: React.VoidFunctionComponent = () => { + const i18n = useI18n(); + return ( + <> + {i18n.translate(I18nId.NoFilesToBackupMessage, { + icon: , + })} + + ); +}; + +export const noFilesToBackup: CreateToast = (onAction) => { + return { + message: , + icon: 'info-sign', + intent: Intent.PRIMARY, + onDismiss: () => onAction('dismiss'), + }; +}; diff --git a/src/explorer/alerts/i18n.ts b/src/explorer/alerts/i18n.ts index 40d331a8..ef87d766 100644 --- a/src/explorer/alerts/i18n.ts +++ b/src/explorer/alerts/i18n.ts @@ -11,4 +11,5 @@ export function useI18n(): I18n { export enum I18nId { FileInUseMessage = 'fileInUse.message', + NoFilesToBackupMessage = 'noFilesToBackup.message', } diff --git a/src/explorer/alerts/index.ts b/src/explorer/alerts/index.ts index f5545974..535c81e6 100644 --- a/src/explorer/alerts/index.ts +++ b/src/explorer/alerts/index.ts @@ -2,6 +2,7 @@ // Copyright (c) 2022 The Pybricks Authors import { fileInUse } from './FileInUseAlert'; +import { noFilesToBackup } from './NoFilesToBackup'; // gathers all of the alert creation functions for passing up to the top level -export default { fileInUse }; +export default { fileInUse, noFilesToBackup }; diff --git a/src/explorer/alerts/translations/en.json b/src/explorer/alerts/translations/en.json index 73fb7d65..61607e20 100644 --- a/src/explorer/alerts/translations/en.json +++ b/src/explorer/alerts/translations/en.json @@ -1,5 +1,8 @@ { "fileInUse": { "message": "The file '{fileName}' could not be opened. It is already open in another window." + }, + "noFilesToBackup": { + "message": "There are no files to backup. Create a new file first by clicking the {icon} icon." } } diff --git a/src/explorer/error.ts b/src/explorer/error.ts new file mode 100644 index 00000000..e449101b --- /dev/null +++ b/src/explorer/error.ts @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { CustomError } from '../utils/customError'; + +/** Specific errors for editor subsystem. */ +export type ExplorerErrorName = 'NoFiles'; + +/** Error class for editor subsystem. */ +export class ExplorerError extends CustomError {} diff --git a/src/explorer/sagas.test.ts b/src/explorer/sagas.test.ts index 19726eea..2cfdf7f7 100644 --- a/src/explorer/sagas.test.ts +++ b/src/explorer/sagas.test.ts @@ -70,6 +70,7 @@ import { duplicateFileDialogDidCancel, duplicateFileDialogShow, } from './duplicateFileDialog/actions'; +import { ExplorerError, ExplorerErrorName } from './error'; import { Hub, newFileWizardDidAccept, @@ -85,6 +86,23 @@ import explorer from './sagas'; jest.mock('browser-fs-access'); +/** + * Asymmetric matcher for matching errors by name while ignoring the message. + * @param name The name to match. + * @returns An asymmetric matcher cast to an ExplorerError so that it can be + * passed to action functions. + */ +function expectExplorerError(name: ExplorerErrorName): ExplorerError { + const matcher: jest.AsymmetricMatcher & Record = { + $$typeof: Symbol.for('jest.asymmetricMatcher'), + asymmetricMatch: (other) => + other instanceof ExplorerError && other.name === name, + toAsymmetricMatcher: () => `[ExplorerError: ${name}]`, + }; + + return matcher as unknown as ExplorerError; +} + describe('handleExplorerArchiveAllFiles', () => { let saga: AsyncSaga; @@ -104,6 +122,10 @@ describe('handleExplorerArchiveAllFiles', () => { saga.put(fileStorageDidFailToDumpAllFiles(testError)); + await expect(saga.take()).resolves.toEqual( + alertsShowAlert('alerts', 'unexpectedError', { error: testError }), + ); + await expect(saga.take()).resolves.toEqual( explorerDidFailToArchiveAllFiles(testError), ); @@ -113,7 +135,11 @@ describe('handleExplorerArchiveAllFiles', () => { saga.put(fileStorageDidDumpAllFiles([])); await expect(saga.take()).resolves.toEqual( - explorerDidFailToArchiveAllFiles(new Error('no files')), + alertsShowAlert('explorer', 'noFilesToBackup'), + ); + + await expect(saga.take()).resolves.toEqual( + explorerDidFailToArchiveAllFiles(expectExplorerError('NoFiles')), ); }); @@ -133,6 +159,10 @@ describe('handleExplorerArchiveAllFiles', () => { throw testError; }); + await expect(saga.take()).resolves.toEqual( + alertsShowAlert('alerts', 'unexpectedError', { error: testError }), + ); + await expect(saga.take()).resolves.toEqual( explorerDidFailToArchiveAllFiles(testError), ); diff --git a/src/explorer/sagas.ts b/src/explorer/sagas.ts index 84602c6c..a6460b6d 100644 --- a/src/explorer/sagas.ts +++ b/src/explorer/sagas.ts @@ -78,6 +78,7 @@ import { duplicateFileDialogDidCancel, duplicateFileDialogShow, } from './duplicateFileDialog/actions'; +import { ExplorerError } from './error'; import { newFileWizardDidAccept, newFileWizardDidCancel, @@ -105,7 +106,7 @@ function* handleExplorerArchiveAllFiles(): Generator { defined(didDump); if (didDump.files.length === 0) { - throw new Error('no files'); + throw new ExplorerError('NoFiles', 'no files in storage'); } const zip = new JSZip(); @@ -131,7 +132,18 @@ function* handleExplorerArchiveAllFiles(): Generator { yield* put(explorerDidArchiveAllFiles()); } catch (err) { - yield* put(explorerDidFailToArchiveAllFiles(ensureError(err))); + const error = ensureError(err); + + if (error instanceof ExplorerError && error.name === 'NoFiles') { + yield* put(alertsShowAlert('explorer', 'noFilesToBackup')); + } else { + yield* put( + alertsShowAlert('alerts', 'unexpectedError', { + error, + }), + ); + } + yield* put(explorerDidFailToArchiveAllFiles(error)); } } diff --git a/src/notifications/sagas.test.ts b/src/notifications/sagas.test.ts index 9b84ff99..c3d08c18 100644 --- a/src/notifications/sagas.test.ts +++ b/src/notifications/sagas.test.ts @@ -19,7 +19,6 @@ import { import { editorDidFailToOpenFile } from '../editor/actions'; import { EditorError } from '../editor/error'; import { - explorerDidFailToArchiveAllFiles, explorerDidFailToCreateNewFile, explorerDidFailToDeleteFile, explorerDidFailToDuplicateFile, @@ -111,7 +110,6 @@ test.each([ appDidCheckForUpdate(false), bleDIServiceDidReceiveFirmwareRevision('3.0.0'), fileStorageDidFailToInitialize(new Error('test error')), - explorerDidFailToArchiveAllFiles(new Error('test error')), explorerDidFailToImportFiles(new Error('test error')), explorerDidFailToCreateNewFile(new Error('test error')), explorerDidFailToDuplicateFile('test.file', new Error('test error')), @@ -137,7 +135,6 @@ test.each([ serviceWorkerDidSucceed(), appDidCheckForUpdate(true), bleDIServiceDidReceiveFirmwareRevision(firmwareVersion), - explorerDidFailToArchiveAllFiles(new DOMException('test message', 'AbortError')), explorerDidFailToImportFiles(new DOMException('test message', 'AbortError')), explorerDidFailToCreateNewFile(new DOMException('test message', 'AbortError')), explorerDidFailToDuplicateFile( diff --git a/src/notifications/sagas.ts b/src/notifications/sagas.ts index 92f26458..5b4470f1 100644 --- a/src/notifications/sagas.ts +++ b/src/notifications/sagas.ts @@ -21,7 +21,6 @@ import { import { editorDidFailToOpenFile } from '../editor/actions'; import { EditorError } from '../editor/error'; import { - explorerDidFailToArchiveAllFiles, explorerDidFailToCreateNewFile, explorerDidFailToDeleteFile, explorerDidFailToDuplicateFile, @@ -387,17 +386,6 @@ function* showFileStorageFailToInitialize( yield* showUnexpectedError(I18nId.FileStorageFailedToInitialize, action.error); } -function* showFileStorageFailToArchive( - action: ReturnType, -): Generator { - if (action.error.name === 'AbortError') { - // user clicked cancel button - not an error - return; - } - - yield* showUnexpectedError(I18nId.ExplorerFailedToArchive, action.error); -} - function* showExplorerFailToImportFiles( action: ReturnType, ): Generator { @@ -476,7 +464,6 @@ export default function* (): Generator { yield* takeEvery(appDidCheckForUpdate, showNoUpdateInfo); yield* takeEvery(bleDIServiceDidReceiveFirmwareRevision, checkVersion); yield* takeEvery(fileStorageDidFailToInitialize, showFileStorageFailToInitialize); - yield* takeEvery(explorerDidFailToArchiveAllFiles, showFileStorageFailToArchive); yield* takeEvery(explorerDidFailToImportFiles, showExplorerFailToImportFiles); yield* takeEvery(explorerDidFailToCreateNewFile, showExplorerFailToCreateFile); yield* takeEvery(explorerDidFailToDuplicateFile, showExplorerFailToDuplicate);