michael-s-molina commented on code in PR #20498:
URL: https://github.com/apache/superset/pull/20498#discussion_r907333232
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -60,33 +61,128 @@ export function removeSaveModalAlert() {
return { type: REMOVE_SAVE_MODAL_ALERT };
}
-export function saveSlice(formData, requestParams) {
- return dispatch => {
- const url = getExploreUrl({
- formData,
- endpointType: 'base',
- force: false,
- curUrl: null,
- requestParams,
- });
-
- // Save the query context so we can re-generate the data from Python
- // for alerts and reports
- const queryContext = buildV1ChartDataPayload({
- formData,
- force: false,
- resultFormat: 'json',
- resultType: 'full',
- });
-
- return SupersetClient.post({
- url,
- postPayload: { form_data: formData, query_context: queryContext },
- })
- .then(response => {
- dispatch(saveSliceSuccess(response.json));
- return response.json;
- })
- .catch(() => dispatch(saveSliceFailed()));
+const getSlicePayload = (sliceName, formData) => {
+ const [datasourceId, datasourceType] = formData.datasource.split('__');
+ const payload = {
+ params: JSON.stringify(formData),
+ slice_name: sliceName,
+ viz_type: formData.viz_type,
+ datasource_id: +datasourceId,
+ datasource_type: datasourceType,
+ dashboards: formData.dashboards,
+ query_context: JSON.stringify(
+ buildV1ChartDataPayload({
+ formData,
+ force: false,
+ resultFormat: 'json',
+ resultType: 'full',
+ setDataMask: null,
+ ownState: null,
+ }),
+ ),
};
-}
+ return payload;
+};
+
+const addDashboardSuccessToast = (addedToDashboard, sliceName) => {
+ if (addedToDashboard) {
+ if (addedToDashboard.new) {
+ addSuccessToast(
+ `${t('Chart')} [${sliceName}] ${t('was added to dashboard')} [${
+ addedToDashboard.title
+ }]`,
+ );
+ } else {
+ addSuccessToast(
+ `${t('Dashboard')} ${addedToDashboard.title}] ${t(
+ 'just got created and chart',
+ )} [${sliceName}] ${t('was added to it')}`,
+ );
+ }
+ }
+};
+
+// Update existing slice
+export const updateSlice =
+ (sliceId, sliceName, formData, addedToDashboard) => async dispatch => {
+ let response;
+ try {
+ response = (
+ await SupersetClient.put({
+ endpoint: `/api/v1/chart/${sliceId}`,
+ headers: { 'Content-Type': 'application/json' },
+ body: JSON.stringify(getSlicePayload(sliceName, formData)),
+ })
+ ).json;
+ } catch (error) {
+ dispatch(saveSliceFailed());
+ throw error;
+ }
+
+ dispatch(saveSliceSuccess());
+ addSuccessToast(
+ `${t('Chart')} [${sliceName}] ${t('has been overwritten')}`,
+ );
+
+ addDashboardSuccessToast(addedToDashboard, sliceName);
+ return response;
+ };
+
+// Create new slice
+export const createSlice =
+ (sliceName, formData, addedToDashboard) => async dispatch => {
+ let response;
+ try {
+ response = (
+ await SupersetClient.post({
+ endpoint: `/api/v1/chart/`,
+ headers: { 'Content-Type': 'application/json' },
+ body: JSON.stringify(getSlicePayload(sliceName, formData)),
+ })
+ ).json;
+ } catch (error) {
+ dispatch(saveSliceFailed());
+ throw error;
+ }
+
+ dispatch(saveSliceSuccess());
+ addSuccessToast(`${t('Chart')} ${sliceName} ${t('has been saved')}`);
+ addDashboardSuccessToast(addedToDashboard, sliceName);
+ return response;
+ };
+
+// Create new dashboard
+export const createDashboard = dashboardName => async dispatch => {
+ let response;
+ try {
+ response = (
+ await SupersetClient.post({
+ endpoint: `/api/v1/dashboard/`,
+ headers: { 'Content-Type': 'application/json' },
+ body: JSON.stringify({ dashboard_title: dashboardName }),
+ })
+ ).json;
+ } catch (error) {
+ dispatch(saveSliceFailed());
+ throw error;
+ }
+
+ return response;
+};
+
+// Get existing dashboard from ID
+export const getDashboard = dashboardId => async dispatch => {
+ let response;
+ try {
+ response = (
+ await SupersetClient.get({
+ endpoint: `/api/v1/dashboard/${dashboardId}`,
+ })
+ ).json;
+ } catch (error) {
+ dispatch(saveSliceFailed());
+ throw error;
+ }
+
+ return response;
Review Comment:
```suggestion
try {
const response = await SupersetClient.get({
endpoint: `/api/v1/dashboard/${dashboardId}`,
});
return response.json;
} catch (error) {
dispatch(saveSliceFailed());
throw error;
}
```
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -60,33 +61,128 @@ export function removeSaveModalAlert() {
return { type: REMOVE_SAVE_MODAL_ALERT };
}
-export function saveSlice(formData, requestParams) {
- return dispatch => {
- const url = getExploreUrl({
- formData,
- endpointType: 'base',
- force: false,
- curUrl: null,
- requestParams,
- });
-
- // Save the query context so we can re-generate the data from Python
- // for alerts and reports
- const queryContext = buildV1ChartDataPayload({
- formData,
- force: false,
- resultFormat: 'json',
- resultType: 'full',
- });
-
- return SupersetClient.post({
- url,
- postPayload: { form_data: formData, query_context: queryContext },
- })
- .then(response => {
- dispatch(saveSliceSuccess(response.json));
- return response.json;
- })
- .catch(() => dispatch(saveSliceFailed()));
+const getSlicePayload = (sliceName, formData) => {
+ const [datasourceId, datasourceType] = formData.datasource.split('__');
+ const payload = {
+ params: JSON.stringify(formData),
+ slice_name: sliceName,
+ viz_type: formData.viz_type,
+ datasource_id: +datasourceId,
+ datasource_type: datasourceType,
+ dashboards: formData.dashboards,
+ query_context: JSON.stringify(
+ buildV1ChartDataPayload({
+ formData,
+ force: false,
+ resultFormat: 'json',
+ resultType: 'full',
+ setDataMask: null,
+ ownState: null,
+ }),
+ ),
};
-}
+ return payload;
+};
+
+const addDashboardSuccessToast = (addedToDashboard, sliceName) => {
+ if (addedToDashboard) {
+ if (addedToDashboard.new) {
+ addSuccessToast(
+ `${t('Chart')} [${sliceName}] ${t('was added to dashboard')} [${
+ addedToDashboard.title
+ }]`,
+ );
+ } else {
+ addSuccessToast(
+ `${t('Dashboard')} ${addedToDashboard.title}] ${t(
+ 'just got created and chart',
+ )} [${sliceName}] ${t('was added to it')}`,
+ );
+ }
+ }
+};
+
+// Update existing slice
+export const updateSlice =
+ (sliceId, sliceName, formData, addedToDashboard) => async dispatch => {
+ let response;
+ try {
+ response = (
+ await SupersetClient.put({
+ endpoint: `/api/v1/chart/${sliceId}`,
+ headers: { 'Content-Type': 'application/json' },
+ body: JSON.stringify(getSlicePayload(sliceName, formData)),
+ })
+ ).json;
+ } catch (error) {
+ dispatch(saveSliceFailed());
+ throw error;
+ }
+
+ dispatch(saveSliceSuccess());
+ addSuccessToast(
+ `${t('Chart')} [${sliceName}] ${t('has been overwritten')}`,
+ );
+
+ addDashboardSuccessToast(addedToDashboard, sliceName);
+ return response;
+ };
+
+// Create new slice
+export const createSlice =
+ (sliceName, formData, addedToDashboard) => async dispatch => {
+ let response;
+ try {
+ response = (
+ await SupersetClient.post({
+ endpoint: `/api/v1/chart/`,
+ headers: { 'Content-Type': 'application/json' },
+ body: JSON.stringify(getSlicePayload(sliceName, formData)),
+ })
+ ).json;
+ } catch (error) {
+ dispatch(saveSliceFailed());
+ throw error;
+ }
+
+ dispatch(saveSliceSuccess());
+ addSuccessToast(`${t('Chart')} ${sliceName} ${t('has been saved')}`);
+ addDashboardSuccessToast(addedToDashboard, sliceName);
+ return response;
+ };
+
+// Create new dashboard
+export const createDashboard = dashboardName => async dispatch => {
+ let response;
+ try {
+ response = (
+ await SupersetClient.post({
+ endpoint: `/api/v1/dashboard/`,
+ headers: { 'Content-Type': 'application/json' },
+ body: JSON.stringify({ dashboard_title: dashboardName }),
+ })
+ ).json;
+ } catch (error) {
+ dispatch(saveSliceFailed());
+ throw error;
+ }
+
+ return response;
Review Comment:
```suggestion
try {
const response = await SupersetClient.post({
endpoint: `/api/v1/dashboard/`,
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ dashboard_title: dashboardName }),
});
return response.json;
} catch (error) {
dispatch(saveSliceFailed());
throw error;
}
```
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -60,33 +61,128 @@ export function removeSaveModalAlert() {
return { type: REMOVE_SAVE_MODAL_ALERT };
}
-export function saveSlice(formData, requestParams) {
- return dispatch => {
- const url = getExploreUrl({
- formData,
- endpointType: 'base',
- force: false,
- curUrl: null,
- requestParams,
- });
-
- // Save the query context so we can re-generate the data from Python
- // for alerts and reports
- const queryContext = buildV1ChartDataPayload({
- formData,
- force: false,
- resultFormat: 'json',
- resultType: 'full',
- });
-
- return SupersetClient.post({
- url,
- postPayload: { form_data: formData, query_context: queryContext },
- })
- .then(response => {
- dispatch(saveSliceSuccess(response.json));
- return response.json;
- })
- .catch(() => dispatch(saveSliceFailed()));
+const getSlicePayload = (sliceName, formData) => {
+ const [datasourceId, datasourceType] = formData.datasource.split('__');
+ const payload = {
+ params: JSON.stringify(formData),
+ slice_name: sliceName,
+ viz_type: formData.viz_type,
+ datasource_id: +datasourceId,
Review Comment:
This syntax is not very familiar to some developers. Maybe just add a
comment:
`datasource_id: +datasourceId, // converting to number`
##########
superset-frontend/src/explore/actions/saveModalActions.js:
##########
@@ -60,33 +61,128 @@ export function removeSaveModalAlert() {
return { type: REMOVE_SAVE_MODAL_ALERT };
}
-export function saveSlice(formData, requestParams) {
- return dispatch => {
- const url = getExploreUrl({
- formData,
- endpointType: 'base',
- force: false,
- curUrl: null,
- requestParams,
- });
-
- // Save the query context so we can re-generate the data from Python
- // for alerts and reports
- const queryContext = buildV1ChartDataPayload({
- formData,
- force: false,
- resultFormat: 'json',
- resultType: 'full',
- });
-
- return SupersetClient.post({
- url,
- postPayload: { form_data: formData, query_context: queryContext },
- })
- .then(response => {
- dispatch(saveSliceSuccess(response.json));
- return response.json;
- })
- .catch(() => dispatch(saveSliceFailed()));
+const getSlicePayload = (sliceName, formData) => {
+ const [datasourceId, datasourceType] = formData.datasource.split('__');
+ const payload = {
+ params: JSON.stringify(formData),
+ slice_name: sliceName,
+ viz_type: formData.viz_type,
+ datasource_id: +datasourceId,
+ datasource_type: datasourceType,
+ dashboards: formData.dashboards,
+ query_context: JSON.stringify(
+ buildV1ChartDataPayload({
+ formData,
+ force: false,
+ resultFormat: 'json',
+ resultType: 'full',
+ setDataMask: null,
+ ownState: null,
+ }),
+ ),
};
-}
+ return payload;
+};
+
+const addDashboardSuccessToast = (addedToDashboard, sliceName) => {
+ if (addedToDashboard) {
+ if (addedToDashboard.new) {
+ addSuccessToast(
+ `${t('Chart')} [${sliceName}] ${t('was added to dashboard')} [${
+ addedToDashboard.title
+ }]`,
+ );
+ } else {
+ addSuccessToast(
+ `${t('Dashboard')} ${addedToDashboard.title}] ${t(
+ 'just got created and chart',
+ )} [${sliceName}] ${t('was added to it')}`,
+ );
+ }
+ }
+};
+
+// Update existing slice
+export const updateSlice =
Review Comment:
Here's a matter of preference but I like to include all the steps to update
the slice under the `try` block so in case one fails we can present an error to
the user. If I look at the `saveSliceSuccess` action we can see that is just a
simple operation, but from the perspective of the `updateSlice` action, it does
not know the complexity involved, and if `saveSliceSuccess` fails, we should
present an error to the user. On example would be an error when updating the
Redux state. Even though the slice was saved on the backend, the client state
would be inconsistent. I think the `try` overhead is not a blocker. So I would
write this piece like this:
```
try {
const response = await SupersetClient.post({
endpoint: `/api/v1/chart/`,
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(getSlicePayload(sliceName, formData)),
});
dispatch(saveSliceSuccess());
addSuccessToast(`${t('Chart')} ${sliceName} ${t('has been saved')}`);
addDashboardSuccessToast(addedToDashboard, sliceName);
return response.json;
} catch (error) {
dispatch(saveSliceFailed());
throw error;
}
```
This is totally optional though. If you agree, then you can also change
`createSlice`.
--
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]
