This is an automated email from the ASF dual-hosted git repository. beto pushed a commit to branch confirm_overwrite_dialog in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
commit bc29d0223dee77edb555ce8051f5b0e3f1fbcaf2 Author: Beto Dealmeida <[email protected]> AuthorDate: Tue Dec 8 21:54:59 2020 -0800 feat: add confirmation dialog for imports --- .../src/components/ImportModal/index.tsx | 55 +++++++++++-- .../src/views/CRUD/chart/ChartList.tsx | 3 + .../src/views/CRUD/dashboard/DashboardList.tsx | 3 + .../src/views/CRUD/data/database/DatabaseList.tsx | 3 + .../src/views/CRUD/data/dataset/DatasetList.tsx | 3 + superset-frontend/src/views/CRUD/hooks.ts | 89 ++++++++++++++++------ 6 files changed, 127 insertions(+), 29 deletions(-) diff --git a/superset-frontend/src/components/ImportModal/index.tsx b/superset-frontend/src/components/ImportModal/index.tsx index 1c26623..5beeefc 100644 --- a/superset-frontend/src/components/ImportModal/index.tsx +++ b/superset-frontend/src/components/ImportModal/index.tsx @@ -101,6 +101,7 @@ export interface ImportModelsModalProps { resourceLabel: string; icon: React.ReactNode; passwordsNeededMessage: string; + confirmOverwriteMessage: string; addDangerToast: (msg: string) => void; addSuccessToast: (msg: string) => void; onModelImport: () => void; @@ -115,6 +116,7 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({ resourceLabel, icon, passwordsNeededMessage, + confirmOverwriteMessage, addDangerToast, addSuccessToast, onModelImport, @@ -123,9 +125,14 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({ passwordFields = [], setPasswordFields = () => {}, }) => { - const [uploadFile, setUploadFile] = useState<File | null>(null); const [isHidden, setIsHidden] = useState<boolean>(true); + const [uploadFile, setUploadFile] = useState<File | null>(null); const [passwords, setPasswords] = useState<Record<string, string>>({}); + const [needsOverwriteConfirm, setNeedsOverwriteConfirm] = useState<boolean>( + false, + ); + const [confirmedOverwrite, setConfirmedOverwrite] = useState<boolean>(false); + const fileInputRef = useRef<HTMLInputElement>(null); const clearModal = () => { @@ -143,7 +150,7 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({ }; const { - state: { passwordsNeeded }, + state: { alreadyExists, passwordsNeeded }, importResource, } = useImportResource<any>(resourceName, resourceLabel, handleErrorMsg); @@ -151,6 +158,10 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({ setPasswordFields(passwordsNeeded); }, [passwordsNeeded, setPasswordFields]); + useEffect(() => { + setNeedsOverwriteConfirm(alreadyExists.length > 0); + }, [alreadyExists]); + // Functions const hide = () => { setIsHidden(true); @@ -162,7 +173,7 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({ return; } - importResource(uploadFile, passwords).then(result => { + importResource(uploadFile, passwords, confirmedOverwrite).then(result => { if (result) { addSuccessToast(t('The import was successful')); clearModal(); @@ -176,6 +187,11 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({ setUploadFile((files && files[0]) || null); }; + const confirmOverwrite = (event: React.ChangeEvent<HTMLInputElement>) => { + const targetValue = (event.currentTarget?.value as string) ?? ''; + setConfirmedOverwrite(targetValue.toUpperCase() === t('OVERWRITE')); + }; + const renderPasswordFields = () => { if (passwordFields.length === 0) { return null; @@ -208,6 +224,31 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({ ); }; + const renderOverwriteConfirmation = () => { + if (alreadyExists.length === 0) { + return null; + } + + return ( + <> + <StyledInputContainer> + <div>{confirmOverwriteMessage}</div> + <div className="control-label"> + <label htmlFor="overwrite"> + {t('Type "OVERWRITE" to confirm')} + </label> + </div> + <input + data-test="overwrite-modal-input" + id="overwrite" + type="text" + onChange={confirmOverwrite} + /> + </StyledInputContainer> + </> + ); + }; + // Show/hide if (isHidden && show) { setIsHidden(false); @@ -217,10 +258,13 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({ <Modal name="model" className="import-model-modal" - disablePrimaryButton={uploadFile === null} + disablePrimaryButton={ + uploadFile === null || (needsOverwriteConfirm && !confirmedOverwrite) + } onHandledPrimaryAction={onUpload} onHide={hide} - primaryButtonName={t('Import')} + primaryButtonName={needsOverwriteConfirm ? t('Overwrite') : t('Import')} + primaryButtonType={needsOverwriteConfirm ? 'danger' : 'primary'} width="750px" show={show} title={ @@ -248,6 +292,7 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({ /> </StyledInputContainer> {renderPasswordFields()} + {renderOverwriteConfirmation()} </Modal> ); }; diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index 4687ed6..51f2586 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -581,6 +581,9 @@ function ChartList(props: ChartListProps) { 'the database configuration are not present in export files, and ' + 'should be added manually after the import if they are needed.', )} + confirmOverwriteMessage={t( + 'One or more charts to be imported already exist.', + )} addDangerToast={addDangerToast} addSuccessToast={addSuccessToast} onModelImport={handleChartImport} diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index e3fe98a..381569d 100644 --- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -542,6 +542,9 @@ function DashboardList(props: DashboardListProps) { 'the database configuration are not present in export files, and ' + 'should be added manually after the import if they are needed.', )} + confirmOverwriteMessage={t( + 'One or more dashboards to be imported already exist.', + )} addDangerToast={addDangerToast} addSuccessToast={addSuccessToast} onModelImport={handleDashboardImport} diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx index f00fd8c..099108f 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx @@ -437,6 +437,9 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) { 'sections of the database configuration are not present in export ' + 'files, and should be added manually after the import if they are needed.', )} + confirmOverwriteMessage={t( + 'One or more databases to be imported already exist.', + )} addDangerToast={addDangerToast} addSuccessToast={addSuccessToast} onModelImport={handleDatabaseImport} diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx index 82bbefb8..773ec01 100644 --- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx +++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx @@ -660,6 +660,9 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({ 'the database configuration are not present in export files, and ' + 'should be added manually after the import if they are needed.', )} + confirmOverwriteMessage={t( + 'One or more datasets to be imported already exist.', + )} addDangerToast={addDangerToast} addSuccessToast={addSuccessToast} onModelImport={handleDatasetImport} diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index e209815..18f47b5 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -316,6 +316,7 @@ export function useSingleViewResource<D extends object = any>( interface ImportResourceState<D extends object = any> { loading: boolean; passwordsNeeded: string[]; + alreadyExists: string[]; } export function useImportResource<D extends object = any>( @@ -326,24 +327,51 @@ export function useImportResource<D extends object = any>( const [state, setState] = useState<ImportResourceState<D>>({ loading: false, passwordsNeeded: [], + alreadyExists: [], }); function updateState(update: Partial<ImportResourceState<D>>) { setState(currentState => ({ ...currentState, ...update })); } - const needsPassword = (errMsg: Record<string, Record<string, string[]>>) => - Object.values(errMsg).every(validationErrors => - Object.entries(validationErrors as Object).every( - ([field, messages]) => - field === '_schema' && - messages.length === 1 && - messages[0] === 'Must provide a password for the database', - ), + /* eslint-disable no-underscore-dangle */ + const isNeedsPassword = (payload: any) => + typeof payload === 'object' && + Array.isArray(payload._schema) && + payload._schema.length === 1 && + payload._schema[0] === 'Must provide a password for the database'; + + const isAlreadyExists = (payload: any) => + typeof payload === 'string' && + payload.includes('already exists and `overwrite=true` was not passed'); + + const getPasswordsNeeded = ( + errMsg: Record<string, Record<string, string[]>>, + ) => + Object.entries(errMsg) + .filter(([, validationErrors]) => isNeedsPassword(validationErrors)) + .map(([fileName]) => fileName); + + const getAlreadyExists = (errMsg: Record<string, Record<string, string[]>>) => + Object.entries(errMsg) + .filter(([, validationErrors]) => isAlreadyExists(validationErrors)) + .map(([fileName]) => fileName); + + const hasTerminalValidation = ( + errMsg: Record<string, Record<string, string[]>>, + ) => + Object.values(errMsg).some( + validationErrors => + !isNeedsPassword(validationErrors) && + !isAlreadyExists(validationErrors), ); const importResource = useCallback( - (bundle: File, databasePasswords: Record<string, string> = {}) => { + ( + bundle: File, + databasePasswords: Record<string, string> = {}, + overwrite = false, + ) => { // Set loading state updateState({ loading: true, @@ -358,6 +386,12 @@ export function useImportResource<D extends object = any>( if (databasePasswords) { formData.append('passwords', JSON.stringify(databasePasswords)); } + /* If the imported model already exists the user needs to confirm + * that they want to overwrite it. + */ + if (overwrite) { + formData.append('overwrite', 'true'); + } return SupersetClient.post({ endpoint: `/api/v1/${resourceName}/import/`, @@ -366,25 +400,31 @@ export function useImportResource<D extends object = any>( .then(() => true) .catch(response => getClientErrorObject(response).then(error => { - /* When importing a bundle, if all validation errors are because - * the databases need passwords we return a list of the database - * files so that the user can type in the passwords and resubmit - * the file. - */ const errMsg = error.message || error.error; - if (typeof errMsg !== 'string' && needsPassword(errMsg)) { + if (typeof errMsg === 'string') { + handleErrorMsg( + t( + 'An error occurred while importing %s: %s', + resourceLabel, + errMsg, + ), + ); + return false; + } + if (hasTerminalValidation(errMsg)) { + handleErrorMsg( + t( + 'An error occurred while importing %s: %s', + resourceLabel, + JSON.stringify(errMsg), + ), + ); + } else { updateState({ - passwordsNeeded: Object.keys(errMsg), + passwordsNeeded: getPasswordsNeeded(errMsg), + alreadyExists: getAlreadyExists(errMsg), }); - return false; } - handleErrorMsg( - t( - 'An error occurred while importing %s: %s', - resourceLabel, - JSON.stringify(errMsg), - ), - ); return false; }), ) @@ -399,6 +439,7 @@ export function useImportResource<D extends object = any>( state: { loading: state.loading, passwordsNeeded: state.passwordsNeeded, + alreadyExists: state.alreadyExists, }, importResource, };
