Skip to content

Commit

Permalink
fix: Dashboard import holding issue (apache#19112)
Browse files Browse the repository at this point in the history
* fix dashboard import holding issue

* resolve comment

(cherry picked from commit e118b4d)
  • Loading branch information
codemaster08240328 authored and sadpandajoe committed Apr 6, 2022
1 parent 08c0c49 commit 6c46953
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 10 deletions.
5 changes: 4 additions & 1 deletion superset-frontend/src/views/CRUD/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
t('Please re-export your file and try importing again'),
].join('\n'),
),
);
} else {
Expand Down
25 changes: 25 additions & 0 deletions superset-frontend/src/views/CRUD/utils.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
{
Expand Down Expand Up @@ -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 contains only the "issue_codes" key', () => {
expect(hasTerminalValidation(terminalErrorsWithOnlyIssuesCode.errors)).toBe(
true,
);
});

test('does not ask for password when the import type is wrong', () => {
const error = {
errors: [
Expand Down
20 changes: 11 additions & 9 deletions superset-frontend/src/views/CRUD/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -399,15 +399,17 @@ export const getAlreadyExists = (errors: Record<string, any>[]) =>
.flat();

export const hasTerminalValidation = (errors: Record<string, any>[]) =>
errors.some(
error =>
!Object.entries(error.extra)
.filter(([key, _]) => key !== 'issue_codes')
.every(
([_, payload]) =>
isNeedsPassword(payload) || isAlreadyExists(payload),
),
);
errors.some(error => {
const noIssuesCodes = Object.entries(error.extra).filter(
([key]) => key !== 'issue_codes',
);

if (noIssuesCodes.length === 0) return true;

return !noIssuesCodes.every(
([, payload]) => isNeedsPassword(payload) || isAlreadyExists(payload),
);
});

export const checkUploadExtensions = (
perm: Array<string>,
Expand Down

0 comments on commit 6c46953

Please sign in to comment.