fix: confirm overwrite and password on import (#15056)

* fix: confirm overwrite and password on import

* Add tests

superset-frontend/src/views/CRUD/hooks.ts

@@ -20,7 +20,12 @@ import rison from 'rison';
 import { useState, useEffect, useCallback } from 'react';
 import { makeApi, SupersetClient, t, JsonObject } from '@superset-ui/core';
 
-import { createErrorHandler } from 'src/views/CRUD/utils';
+import {
+  createErrorHandler,
+  getAlreadyExists,
+  getPasswordsNeeded,
+  hasTerminalValidation,
+} from 'src/views/CRUD/utils';
 import { FetchDataConfig } from 'src/components/ListView';
 import { FilterValue } from 'src/components/ListView/types';
 import Chart, { Slice } from 'src/types/Chart';
@@ -384,40 +389,6 @@ export function useImportResource(
     setState(currentState => ({ ...currentState, ...update }));
   }
 
-  /* 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[] | string>>,
-  ) =>
-    Object.entries(errMsg)
-      .filter(([, validationErrors]) => isNeedsPassword(validationErrors))
-      .map(([fileName]) => fileName);
-
-  const getAlreadyExists = (
-    errMsg: Record<string, Record<string, string[] | string>>,
-  ) =>
-    Object.entries(errMsg)
-      .filter(([, validationErrors]) => isAlreadyExists(validationErrors))
-      .map(([fileName]) => fileName);
-
-  const hasTerminalValidation = (
-    errMsg: Record<string, Record<string, string[] | string>>,
-  ) =>
-    Object.values(errMsg).some(
-      validationErrors =>
-        !isNeedsPassword(validationErrors) &&
-        !isAlreadyExists(validationErrors),
-    );
-
   const importResource = useCallback(
     (
       bundle: File,
@@ -452,29 +423,28 @@ export function useImportResource(
         .then(() => true)
         .catch(response =>
           getClientErrorObject(response).then(error => {
-            const errMsg = error.message || error.error;
-            if (typeof errMsg === 'string') {
+            if (!error.errors) {
               handleErrorMsg(
                 t(
                   'An error occurred while importing %s: %s',
                   resourceLabel,
-                  parsedErrorMessage(errMsg),
+                  error.message || error.error,
                 ),
               );
               return false;
             }
-            if (hasTerminalValidation(errMsg)) {
+            if (hasTerminalValidation(error.errors)) {
               handleErrorMsg(
                 t(
                   'An error occurred while importing %s: %s',
                   resourceLabel,
-                  parsedErrorMessage(errMsg),
+                  error.errors.map(payload => payload.message).join('\n'),
                 ),
               );
             } else {
               updateState({
-                passwordsNeeded: getPasswordsNeeded(errMsg),
-                alreadyExists: getAlreadyExists(errMsg),
+                passwordsNeeded: getPasswordsNeeded(error.errors),
+                alreadyExists: getAlreadyExists(error.errors),
               });
             }
             return false;

superset-frontend/src/views/CRUD/utils.test.tsx

@@ -0,0 +1,145 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+  isNeedsPassword,
+  isAlreadyExists,
+  getPasswordsNeeded,
+  getAlreadyExists,
+  hasTerminalValidation,
+} from 'src/views/CRUD/utils';
+
+const terminalErrors = {
+  errors: [
+    {
+      message: 'Error importing database',
+      error_type: 'GENERIC_COMMAND_ERROR',
+      level: 'warning',
+      extra: {
+        'metadata.yaml': { type: ['Must be equal to Database.'] },
+        issue_codes: [
+          {
+            code: 1010,
+            message:
+              'Issue 1010 - Superset encountered an error while running a command.',
+          },
+        ],
+      },
+    },
+  ],
+};
+
+const overwriteNeededErrors = {
+  errors: [
+    {
+      message: 'Error importing database',
+      error_type: 'GENERIC_COMMAND_ERROR',
+      level: 'warning',
+      extra: {
+        'databases/imported_database.yaml':
+          'Database already exists and `overwrite=true` was not passed',
+        issue_codes: [
+          {
+            code: 1010,
+            message:
+              'Issue 1010 - Superset encountered an error while running a command.',
+          },
+        ],
+      },
+    },
+  ],
+};
+
+const passwordNeededErrors = {
+  errors: [
+    {
+      message: 'Error importing database',
+      error_type: 'GENERIC_COMMAND_ERROR',
+      level: 'warning',
+      extra: {
+        'databases/imported_database.yaml': {
+          _schema: ['Must provide a password for the database'],
+        },
+        issue_codes: [
+          {
+            code: 1010,
+            message:
+              'Issue 1010 - Superset encountered an error while running a command.',
+          },
+        ],
+      },
+    },
+  ],
+};
+
+test('identifies error payloads indicating that password is needed', () => {
+  let needsPassword;
+
+  needsPassword = isNeedsPassword({
+    _schema: ['Must provide a password for the database'],
+  });
+  expect(needsPassword).toBe(true);
+
+  needsPassword = isNeedsPassword(
+    'Database already exists and `overwrite=true` was not passed',
+  );
+  expect(needsPassword).toBe(false);
+
+  needsPassword = isNeedsPassword({ type: ['Must be equal to Database.'] });
+  expect(needsPassword).toBe(false);
+});
+
+test('identifies error payloads indicating that overwrite confirmation is needed', () => {
+  let alreadyExists;
+
+  alreadyExists = isAlreadyExists(
+    'Database already exists and `overwrite=true` was not passed',
+  );
+  expect(alreadyExists).toBe(true);
+
+  alreadyExists = isAlreadyExists({
+    _schema: ['Must provide a password for the database'],
+  });
+  expect(alreadyExists).toBe(false);
+
+  alreadyExists = isAlreadyExists({ type: ['Must be equal to Database.'] });
+  expect(alreadyExists).toBe(false);
+});
+
+test('extracts DB configuration files that need passwords', () => {
+  const passwordsNeeded = getPasswordsNeeded(passwordNeededErrors.errors);
+  expect(passwordsNeeded).toEqual(['databases/imported_database.yaml']);
+});
+
+test('extracts files that need overwrite confirmation', () => {
+  const alreadyExists = getAlreadyExists(overwriteNeededErrors.errors);
+  expect(alreadyExists).toEqual(['databases/imported_database.yaml']);
+});
+
+test('detects if the error message is terminal or if it requires uses intervention', () => {
+  let isTerminal;
+
+  isTerminal = hasTerminalValidation(terminalErrors.errors);
+  expect(isTerminal).toBe(true);
+
+  isTerminal = hasTerminalValidation(overwriteNeededErrors.errors);
+  expect(isTerminal).toBe(false);
+
+  isTerminal = hasTerminalValidation(passwordNeededErrors.errors);
+  expect(isTerminal).toBe(false);
+});

superset-frontend/src/views/CRUD/utils.tsx

@@ -322,3 +322,40 @@ export const CardStyles = styled.div`
     text-decoration: none;
   }
 `;
+
+export /* 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';
+
+export const isAlreadyExists = (payload: any) =>
+  typeof payload === 'string' &&
+  payload.includes('already exists and `overwrite=true` was not passed');
+
+export const getPasswordsNeeded = (errors: Record<string, any>[]) =>
+  errors
+    .map(error =>
+      Object.entries(error.extra)
+        .filter(([, payload]) => isNeedsPassword(payload))
+        .map(([fileName]) => fileName),
+    )
+    .flat();
+
+export const getAlreadyExists = (errors: Record<string, any>[]) =>
+  errors
+    .map(error =>
+      Object.entries(error.extra)
+        .filter(([, payload]) => isAlreadyExists(payload))
+        .map(([fileName]) => fileName),
+    )
+    .flat();
+
+export const hasTerminalValidation = (errors: Record<string, any>[]) =>
+  errors.some(
+    error =>
+      !Object.values(error.extra).some(
+        payload => isNeedsPassword(payload) || isAlreadyExists(payload),
+      ),
+  );