codyml commented on code in PR #21497:
URL: https://github.com/apache/superset/pull/21497#discussion_r974199540
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -215,3 +218,23 @@ export const getDashboard = dashboardId => async dispatch
=> {
throw error;
}
};
+
+// Get dashboards the slice is added to
+export const getSliceDashboards = slice => async dispatch => {
+ if (slice) {
Review Comment:
My intention was for the `try...catch` clause to just handle request errors
or other unexpected errors. The `if` clause is there because this function
might be called with no `slice` param, in the case of a chart that hasn't yet
been saved, and I wanted it to return `[]` in that case as part of normal,
non-exception behavior.
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -215,3 +218,23 @@ export const getDashboard = dashboardId => async dispatch
=> {
throw error;
}
};
+
+// Get dashboards the slice is added to
+export const getSliceDashboards = slice => async dispatch => {
+ if (slice) {
+ try {
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/chart/${slice.slice_id}?q=${rison.encode({
Review Comment:
I check above that `slice` is defined, so I don't think this check is
necessary.
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -215,3 +218,23 @@ export const getDashboard = dashboardId => async dispatch
=> {
throw error;
}
};
+
+// Get dashboards the slice is added to
+export const getSliceDashboards = slice => async dispatch => {
+ if (slice) {
+ try {
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/chart/${slice.slice_id}?q=${rison.encode({
+ columns: ['dashboards.id'],
+ })}`,
+ });
+
+ return response.json.result.dashboards.map(({ id }) => id);
+ } catch (error) {
+ dispatch(saveSliceFailed());
+ throw error;
+ }
+ }
+
+ return [];
Review Comment:
I don't want to remove the `if` clause; returning `[]` isn't exception
behavior (see above).
##########
superset-frontend/src/explore/components/SaveModal.tsx:
##########
@@ -200,12 +207,14 @@ class SaveModal extends React.Component<SaveModalProps,
SaveModalState> {
.then(() => this.props.actions.getDashboard(saveToDashboardId))
.then((response: { result: DashboardGetResponse }) => {
dashboard = response.result;
- const dashboards = new Set<number>(this.props.sliceDashboards);
Review Comment:
Much cleaner, thanks.
##########
superset-frontend/src/explore/components/SaveModal.tsx:
##########
@@ -183,6 +182,14 @@ class SaveModal extends React.Component<SaveModalProps,
SaveModalState> {
}
}
+ // Get chart dashboards
+ let sliceDashboards: number[];
+ promise = promise
+ .then(() => this.props.actions.getSliceDashboards(this.props.slice))
+ .then(dashboards => {
+ sliceDashboards = dashboards;
+ });
+
Review Comment:
Oops, I didn't realize this `saveOrOverwrite` was now an `async` function!
I can clean up a lot of these promises now, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
