-
Notifications
You must be signed in to change notification settings - Fork 13.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[dashboard][bugfix][save as] re-direct to copied dashboard upon saveas #6189
Conversation
t('Could not fetch all saved charts'), | ||
.catch(errorResponse => | ||
getClientErrorObject(errorResponse).then(({ error }) => | ||
Promise.all([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Promise.all
seems unnecessary.
Is anybody expecting the result from the dispatches?
Might be better to return the errorObject, or don't care about return value at all.
.catch(errorResponse =>
getClientErrorObject(errorResponse).then(({ error }) => {
dispatch(...);
dispatch(...);
});
.catch(errorResponse => {
const errorObjectPromise = getClientErrorObject(errorResponse);
errorObject.then(({ error }) => {
dispatch(...);
dispatch(...);
});
return errorObjectPromise;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return is necessary over here. redirecting is depends on new dash id in the response.
Codecov Report
@@ Coverage Diff @@
## master #6189 +/- ##
=======================================
Coverage 76.93% 76.93%
=======================================
Files 47 47
Lines 9386 9386
=======================================
Hits 7221 7221
Misses 2165 2165 Continue to review full report at Codecov.
|
apache#6189) * [dashboard][bugfix][save as] re-direct to copied dashboard upon saveas * [dashboard][save modal] safer destructuring * [dashboard][save as] simplify Promise resolution * [dashboard] simply fetch slices error (cherry picked from commit e7e8e9d)
apache#6189) * [dashboard][bugfix][save as] re-direct to copied dashboard upon saveas * [dashboard][save modal] safer destructuring * [dashboard][save as] simplify Promise resolution * [dashboard] simply fetch slices error
🐛 Fixes #6188 where you are not re-directed upon copying a dashboard. Introduced by this line in the SupersetClient refactor 🙄
after fix
@graceguo-supercat @kristw @michellethomas