From d8be9a6b4b48618bad75aa3c33561aeb7da823ed Mon Sep 17 00:00:00 2001 From: SpiderMan Date: Thu, 10 Mar 2022 18:08:45 -0500 Subject: [PATCH 1/2] fix dashboard import holding issue --- superset-frontend/src/views/CRUD/hooks.ts | 7 ++++-- .../src/views/CRUD/utils.test.tsx | 25 +++++++++++++++++++ superset-frontend/src/views/CRUD/utils.tsx | 20 ++++++++------- 3 files changed, 41 insertions(+), 11 deletions(-) diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index ae58aef0d9e0f..84555e24de657 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -448,7 +448,10 @@ export function useImportResource( t( 'An error occurred while importing %s: %s', resourceLabel, - error.errors.map(payload => payload.message).join('\n'), + [ + ...error.errors.map(payload => payload.message), + 'Please re-export your file and try importing again', + ].join('\n'), ), ); } else { @@ -464,7 +467,7 @@ export function useImportResource( updateState({ loading: false }); }); }, - [], + [handleErrorMsg, resourceLabel, resourceName], ); return { state, importResource }; diff --git a/superset-frontend/src/views/CRUD/utils.test.tsx b/superset-frontend/src/views/CRUD/utils.test.tsx index 68d377e3102cf..c6a28d46c406b 100644 --- a/superset-frontend/src/views/CRUD/utils.test.tsx +++ b/superset-frontend/src/views/CRUD/utils.test.tsx @@ -46,6 +46,25 @@ const terminalErrors = { ], }; +const terminalErrorsWithOnlyIssuesCode = { + errors: [ + { + message: 'Error importing database', + error_type: 'GENERIC_COMMAND_ERROR', + level: 'warning', + extra: { + issue_codes: [ + { + code: 1010, + message: + 'Issue 1010 - Superset encountered an error while running a command.', + }, + ], + }, + }, + ], +}; + const overwriteNeededErrors = { errors: [ { @@ -146,6 +165,12 @@ test('detects if the error message is terminal or if it requires uses interventi expect(isTerminal).toBe(false); }); +test('error message is terminal when the extra field not contains other keys excepting issue_codes', () => { + expect(hasTerminalValidation(terminalErrorsWithOnlyIssuesCode.errors)).toBe( + true, + ); +}); + test('does not ask for password when the import type is wrong', () => { const error = { errors: [ diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx index d9d21c8565d17..7275350daf5d3 100644 --- a/superset-frontend/src/views/CRUD/utils.tsx +++ b/superset-frontend/src/views/CRUD/utils.tsx @@ -401,15 +401,17 @@ export const getAlreadyExists = (errors: Record[]) => .flat(); export const hasTerminalValidation = (errors: Record[]) => - errors.some( - error => - !Object.entries(error.extra) - .filter(([key, _]) => key !== 'issue_codes') - .every( - ([_, payload]) => - isNeedsPassword(payload) || isAlreadyExists(payload), - ), - ); + errors.some(error => { + const not_issues_codes = Object.entries(error.extra).filter( + ([key, _]) => key !== 'issue_codes', + ); + + if (not_issues_codes.length === 0) return true; + + return !not_issues_codes.every( + ([_, payload]) => isNeedsPassword(payload) || isAlreadyExists(payload), + ); + }); export const checkUploadExtensions = ( perm: Array | string | undefined | boolean, From a66d6faa98f30c831ad4c77239f13c7ab25c27d3 Mon Sep 17 00:00:00 2001 From: SpiderMan Date: Fri, 1 Apr 2022 15:18:25 -0400 Subject: [PATCH 2/2] resolve comment --- superset-frontend/src/views/CRUD/hooks.ts | 4 ++-- superset-frontend/src/views/CRUD/utils.test.tsx | 2 +- superset-frontend/src/views/CRUD/utils.tsx | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index 931c5036f9c3c..70b5df2bf6912 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -450,7 +450,7 @@ export function useImportResource( resourceLabel, [ ...error.errors.map(payload => payload.message), - 'Please re-export your file and try importing again', + t('Please re-export your file and try importing again'), ].join('\n'), ), ); @@ -467,7 +467,7 @@ export function useImportResource( updateState({ loading: false }); }); }, - [handleErrorMsg, resourceLabel, resourceName], + [], ); return { state, importResource }; diff --git a/superset-frontend/src/views/CRUD/utils.test.tsx b/superset-frontend/src/views/CRUD/utils.test.tsx index c6a28d46c406b..dcce0b83697ed 100644 --- a/superset-frontend/src/views/CRUD/utils.test.tsx +++ b/superset-frontend/src/views/CRUD/utils.test.tsx @@ -165,7 +165,7 @@ test('detects if the error message is terminal or if it requires uses interventi expect(isTerminal).toBe(false); }); -test('error message is terminal when the extra field not contains other keys excepting issue_codes', () => { +test('error message is terminal when the "extra" field contains only the "issue_codes" key', () => { expect(hasTerminalValidation(terminalErrorsWithOnlyIssuesCode.errors)).toBe( true, ); diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx index 6a5fcf1dceba0..43c98163c0f74 100644 --- a/superset-frontend/src/views/CRUD/utils.tsx +++ b/superset-frontend/src/views/CRUD/utils.tsx @@ -400,14 +400,14 @@ export const getAlreadyExists = (errors: Record[]) => export const hasTerminalValidation = (errors: Record[]) => errors.some(error => { - const not_issues_codes = Object.entries(error.extra).filter( - ([key, _]) => key !== 'issue_codes', + const noIssuesCodes = Object.entries(error.extra).filter( + ([key]) => key !== 'issue_codes', ); - if (not_issues_codes.length === 0) return true; + if (noIssuesCodes.length === 0) return true; - return !not_issues_codes.every( - ([_, payload]) => isNeedsPassword(payload) || isAlreadyExists(payload), + return !noIssuesCodes.every( + ([, payload]) => isNeedsPassword(payload) || isAlreadyExists(payload), ); });