revert(#17570): fix dashboard editing for dashboards with non-ascii characters

superset-frontend/cypress-base/cypress/integration/dashboard/edit_properties.test.ts

@@ -97,13 +97,17 @@ describe('Dashboard edit action', () => {
       .get('[data-test="dashboard-title-input"]')
       .type(`{selectall}{backspace}${dashboardTitle}`);
 
+    cy.wait('@dashboardGet').then(() => {
+      selectColorScheme('d3Category20b');
+    });
+
     // save edit changes
     cy.get('.ant-modal-footer')
-      .contains('Apply')
+      .contains('Save')
       .click()
       .then(() => {
         // assert that modal edit window has closed
-        cy.get('.ant-modal-body').should('not.be.visible');
+        cy.get('.ant-modal-body').should('not.exist');
 
         // assert title has been updated
         cy.get('.editable-title input').should('have.value', dashboardTitle);

superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js

@@ -71,21 +71,21 @@ describe('Dashboard save action', () => {
     cy.get('[aria-label="edit-alt"]').click({ timeout: 5000 });
     cy.get('[data-test="dashboard-delete-component-button"]')
       .last()
-      .trigger('mouseenter')
+      .trigger('moustenter')
       .click();
 
     cy.get('[data-test="grid-container"]')
       .find('.box_plot')
       .should('not.exist');
 
-    cy.intercept('PUT', '/api/v1/dashboard/**').as('putDashboardRequest');
+    cy.intercept('POST', '/superset/save_dash/**/').as('saveRequest');
     cy.get('[data-test="dashboard-header"]')
       .find('[data-test="header-save-button"]')
       .contains('Save')
       .click();
 
     // go back to view mode
-    cy.wait('@putDashboardRequest');
+    cy.wait('@saveRequest');
     cy.get('[data-test="dashboard-header"]')
       .find('[aria-label="edit-alt"]')
       .click();

superset-frontend/spec/javascripts/dashboard/components/PropertiesModal_spec.jsx

@@ -59,8 +59,7 @@ fetchMock.get('glob:*/api/v1/dashboard/*', {
   },
 });
 
-// all these tests need to be moved to dashboard/components/PropertiesModal/PropertiesModal.test.tsx
-describe.skip('PropertiesModal', () => {
+describe('PropertiesModal', () => {
   afterEach(() => {
     jest.restoreAllMocks();
     jest.resetAllMocks();

superset-frontend/src/dashboard/actions/dashboardState.js

@@ -18,7 +18,7 @@
  */
 /* eslint camelcase: 0 */
 import { ActionCreators as UndoActionCreators } from 'redux-undo';
-import { ensureIsArray, t, SupersetClient } from '@superset-ui/core';
+import { t, SupersetClient } from '@superset-ui/core';
 import { addChart, removeChart, refreshChart } from 'src/chart/chartAction';
 import { chart as initChart } from 'src/chart/chartReducer';
 import { applyDefaultFormData } from 'src/explore/store';
@@ -35,11 +35,7 @@ import { getActiveFilters } from 'src/dashboard/util/activeDashboardFilters';
 import { safeStringify } from 'src/utils/safeStringify';
 import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
 import { UPDATE_COMPONENTS_PARENTS_LIST } from './dashboardLayout';
-import {
-  setChartConfiguration,
-  dashboardInfoChanged,
-  SET_CHART_CONFIG_COMPLETE,
-} from './dashboardInfo';
+import { setChartConfiguration } from './dashboardInfo';
 import { fetchDatasourceMetadata } from './datasources';
 import {
   addFilter,
@@ -180,6 +176,8 @@ export function saveDashboardRequestSuccess(lastModifiedTime) {
 }
 
 export function saveDashboardRequest(data, id, saveType) {
+  const path = saveType === SAVE_TYPE_OVERWRITE ? 'save_dash' : 'copy_dash';
+
   return (dispatch, getState) => {
     dispatch({ type: UPDATE_COMPONENTS_PARENTS_LIST });
 
@@ -196,178 +194,52 @@ export function saveDashboardRequest(data, id, saveType) {
     const serializedFilters = serializeActiveFilterValues(getActiveFilters());
     // serialize filter scope for each filter field, grouped by filter id
     const serializedFilterScopes = serializeFilterScopes(dashboardFilters);
-    const {
-      certified_by,
-      certification_details,
-      css,
-      dashboard_title,
-      owners,
-      roles,
-      slug,
-    } = data;
-
-    const hasId = item => item.id !== undefined;
-
-    // making sure the data is what the backend expects
-    const cleanedData = {
-      ...data,
-      certified_by: certified_by || '',
-      certification_details:
-        certified_by && certification_details ? certification_details : '',
-      css: css || '',
-      dashboard_title: dashboard_title || t('[ untitled dashboard ]'),
-      owners: ensureIsArray(owners).map(o => (hasId(o) ? o.id : o)),
-      roles: !isFeatureEnabled(FeatureFlag.DASHBOARD_RBAC)
-        ? undefined
-        : ensureIsArray(roles).map(r => (hasId(r) ? r.id : r)),
-      slug: slug || null,
-      metadata: {
-        ...data.metadata,
-        color_namespace: data.metadata?.color_namespace || undefined,
-        color_scheme: data.metadata?.color_scheme || '',
-        expanded_slices: data.metadata?.expanded_slices || {},
-        label_colors: data.metadata?.label_colors || {},
-        refresh_frequency: data.metadata?.refresh_frequency || 0,
-        timed_refresh_immune_slices:
-          data.metadata?.timed_refresh_immune_slices || [],
-      },
-    };
-
-    const handleChartConfiguration = () => {
-      const {
-        dashboardInfo: {
-          metadata: { chart_configuration = {} },
-        },
-      } = getState();
-      const chartConfiguration = Object.values(chart_configuration).reduce(
-        (prev, next) => {
-          // If chart removed from dashboard - remove it from metadata
-          if (
-            Object.values(layout).find(
-              layoutItem => layoutItem?.meta?.chartId === next.id,
-            )
-          ) {
-            return { ...prev, [next.id]: next };
-          }
-          return prev;
-        },
-        {},
-      );
-      return chartConfiguration;
-    };
-
-    const onCopySuccess = response => {
-      const lastModifiedTime = response.json.last_modified_time;
-      if (lastModifiedTime) {
-        dispatch(saveDashboardRequestSuccess(lastModifiedTime));
-      }
-      if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) {
-        const chartConfiguration = handleChartConfiguration();
-        dispatch(setChartConfiguration(chartConfiguration));
-      }
-      dispatch(addSuccessToast(t('This dashboard was saved successfully.')));
-      return response;
-    };
-
-    const onUpdateSuccess = response => {
-      const updatedDashboard = response.json.result;
-      const lastModifiedTime = response.json.last_modified_time;
-      // synching with the backend transformations of the metadata
-      if (updatedDashboard.json_metadata) {
-        const metadata = JSON.parse(updatedDashboard.json_metadata);
-        dispatch(
-          dashboardInfoChanged({
-            metadata,
-          }),
-        );
-        if (metadata.chart_configuration) {
-          dispatch({
-            type: SET_CHART_CONFIG_COMPLETE,
-            chartConfiguration: metadata.chart_configuration,
-          });
-        }
-      }
-      if (lastModifiedTime) {
-        dispatch(saveDashboardRequestSuccess(lastModifiedTime));
-      }
-      // redirect to the new slug or id
-      window.history.pushState(
-        { event: 'dashboard_properties_changed' },
-        '',
-        `/superset/dashboard/${slug || id}/`,
-      );
-
-      dispatch(addSuccessToast(t('This dashboard was saved successfully.')));
-      return response;
-    };
-
-    const onError = async response => {
-      const { error, message } = await getClientErrorObject(response);
-      let errorText = t('Sorry, an unknown error occured');
-
-      if (error) {
-        errorText = t(
-          'Sorry, there was an error saving this dashboard: %s',
-          error,
-        );
-      }
-      if (typeof message === 'string' && message === 'Forbidden') {
-        errorText = t('You do not have permission to edit this dashboard');
-      }
-      dispatch(addDangerToast(errorText));
-    };
-
-    if (saveType === SAVE_TYPE_OVERWRITE) {
-      let chartConfiguration = {};
-      if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) {
-        chartConfiguration = handleChartConfiguration();
-      }
-      const updatedDashboard = {
-        certified_by: cleanedData.certified_by,
-        certification_details: cleanedData.certification_details,
-        css: cleanedData.css,
-        dashboard_title: cleanedData.dashboard_title,
-        slug: cleanedData.slug,
-        owners: cleanedData.owners,
-        roles: cleanedData.roles,
-        json_metadata: safeStringify({
-          ...(cleanedData?.metadata || {}),
-          default_filters: safeStringify(serializedFilters),
-          filter_scopes: serializedFilterScopes,
-          chart_configuration: chartConfiguration,
-        }),
-      };
-
-      return SupersetClient.put({
-        endpoint: `/api/v1/dashboard/${id}`,
-        headers: { 'Content-Type': 'application/json' },
-        body: JSON.stringify(updatedDashboard),
-      })
-        .then(response => onUpdateSuccess(response))
-        .catch(response => onError(response));
-    }
-    // changing the data as the endpoint requires
-    const copyData = { ...cleanedData };
-    if (copyData.metadata) {
-      delete copyData.metadata;
-    }
-    const finalCopyData = {
-      ...copyData,
-      // the endpoint is expecting the metadata to be flat
-      ...(cleanedData?.metadata || {}),
-    };
     return SupersetClient.post({
-      endpoint: `/superset/copy_dash/${id}/`,
+      endpoint: `/superset/${path}/${id}/`,
       postPayload: {
         data: {
-          ...finalCopyData,
+          ...data,
           default_filters: safeStringify(serializedFilters),
           filter_scopes: safeStringify(serializedFilterScopes),
         },
       },
     })
-      .then(response => onCopySuccess(response))
-      .catch(response => onError(response));
+      .then(response => {
+        if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) {
+          const {
+            dashboardInfo: {
+              metadata: { chart_configuration = {} },
+            },
+          } = getState();
+          const chartConfiguration = Object.values(chart_configuration).reduce(
+            (prev, next) => {
+              // If chart removed from dashboard - remove it from metadata
+              if (
+                Object.values(layout).find(
+                  layoutItem => layoutItem?.meta?.chartId === next.id,
+                )
+              ) {
+                return { ...prev, [next.id]: next };
+              }
+              return prev;
+            },
+            {},
+          );
+          dispatch(setChartConfiguration(chartConfiguration));
+        }
+        dispatch(saveDashboardRequestSuccess(response.json.last_modified_time));
+        dispatch(addSuccessToast(t('This dashboard was saved successfully.')));
+        return response;
+      })
+      .catch(response =>
+        getClientErrorObject(response).then(({ error }) =>
+          dispatch(
+            addDangerToast(
+              t('Sorry, there was an error saving this dashboard: %s', error),
+            ),
+          ),
+        ),
+      );
   };
 }