chore: Migrate copy_dash endpoint to api v1 (#23112)

Co-authored-by: hughhhh <hughmil3s@gmail.com>

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

@@ -299,7 +299,7 @@ export function saveDashboardRequest(data, id, saveType) {
     };
 
     const onCopySuccess = response => {
-      const lastModifiedTime = response.json.last_modified_time;
+      const lastModifiedTime = response.json.result.last_modified_time;
       if (lastModifiedTime) {
         dispatch(saveDashboardRequestSuccess(lastModifiedTime));
       }
@@ -450,24 +450,21 @@ export function saveDashboardRequest(data, id, saveType) {
         });
     }
     // changing the data as the endpoint requires
-    const copyData = { ...cleanedData };
-    if (copyData.metadata) {
-      delete copyData.metadata;
+    if ('positions' in cleanedData && !('positions' in cleanedData.metadata)) {
+      cleanedData.metadata.positions = cleanedData.positions;
     }
-    const finalCopyData = {
-      ...copyData,
-      // the endpoint is expecting the metadata to be flat
-      ...(cleanedData?.metadata || {}),
+    cleanedData.metadata.default_filters = safeStringify(serializedFilters);
+    cleanedData.metadata.filter_scopes = serializedFilterScopes;
+    const copyPayload = {
+      dashboard_title: cleanedData.dashboard_title,
+      css: cleanedData.css,
+      duplicate_slices: cleanedData.duplicate_slices,
+      json_metadata: JSON.stringify(cleanedData.metadata),
     };
+
     return SupersetClient.post({
-      endpoint: `/superset/copy_dash/${id}/`,
-      postPayload: {
-        data: {
-          ...finalCopyData,
-          default_filters: safeStringify(serializedFilters),
-          filter_scopes: safeStringify(serializedFilterScopes),
-        },
-      },
+      endpoint: `/api/v1/dashboard/${id}/copy/`,
+      jsonPayload: copyPayload,
     })
       .then(response => onCopySuccess(response))
       .catch(response => onError(response));

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

@@ -134,10 +134,11 @@ describe('dashboardState actions', () => {
       const thunk = saveDashboardRequest(newDashboardData, 1, 'save_dash');
       thunk(dispatch, getState);
       expect(postStub.callCount).toBe(1);
-      const { postPayload } = postStub.getCall(0).args[0];
-      expect(postPayload.data.positions[DASHBOARD_GRID_ID].parents).toBe(
-        mockParentsList,
-      );
+      const { jsonPayload } = postStub.getCall(0).args[0];
+      const parsedJsonMetadata = JSON.parse(jsonPayload.json_metadata);
+      expect(
+        parsedJsonMetadata.positions[DASHBOARD_GRID_ID].parents,
+      ).toStrictEqual(mockParentsList);
     });
 
     describe('FeatureFlag.CONFIRM_DASHBOARD_DIFF', () => {

superset-frontend/src/dashboard/components/SaveModal.tsx

@@ -153,13 +153,8 @@ class SaveModal extends React.PureComponent<SaveModalProps, SaveModalState> {
       );
     } else {
       this.onSave(data, dashboardId, saveType).then((resp: JsonResponse) => {
-        if (
-          saveType === SAVE_TYPE_NEWDASHBOARD &&
-          resp &&
-          resp.json &&
-          resp.json.id
-        ) {
-          window.location.href = `/superset/dashboard/${resp.json.id}/`;
+        if (saveType === SAVE_TYPE_NEWDASHBOARD && resp.json?.result?.id) {
+          window.location.href = `/superset/dashboard/${resp.json.result.id}/`;
         }
       });
       this.modal?.current?.close?.();

superset/constants.py

@@ -153,6 +153,7 @@ class RouteMethod:  # pylint: disable=too-few-public-methods
     "get_all_objects": "read",
     "add_objects": "write",
     "delete_object": "write",
+    "copy_dash": "write",
 }
 
 EXTRA_FORM_DATA_APPEND_KEYS = {

superset/dashboards/api.py

@@ -66,6 +66,7 @@
     FilterRelatedRoles,
 )
 from superset.dashboards.schemas import (
+    DashboardCopySchema,
     DashboardDatasetSchema,
     DashboardGetResponseSchema,
     DashboardPostSchema,
@@ -149,6 +150,7 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]:
         "set_embedded",
         "delete_embedded",
         "thumbnail",
+        "copy_dash",
     }
     resource_name = "dashboard"
     allow_browser_login = True
@@ -284,6 +286,7 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]:
     """ Override the name set for this collection of endpoints """
     openapi_spec_component_schemas = (
         ChartEntityResponseSchema,
+        DashboardCopySchema,
         DashboardGetResponseSchema,
         DashboardDatasetSchema,
         GetFavStarIdsSchema,
@@ -1380,3 +1383,69 @@ def delete_embedded(self, dashboard: Dashboard) -> Response:
         for embedded in dashboard.embedded:
             DashboardDAO.delete(embedded)
         return self.response(200, message="OK")
+
+    @expose("/<id_or_slug>/copy/", methods=["POST"])
+    @protect()
+    @safe
+    @permission_name("write")
+    @statsd_metrics
+    @event_logger.log_this_with_context(
+        action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.copy_dash",
+        log_to_statsd=False,
+    )
+    @with_dashboard
+    def copy_dash(self, original_dash: Dashboard) -> Response:
+        """Makes a copy of an existing dashboard
+        ---
+        post:
+          summary: Makes a copy of an existing dashboard
+          parameters:
+          - in: path
+            schema:
+              type: string
+            name: id_or_slug
+            description: The dashboard id or slug
+          requestBody:
+            required: true
+            content:
+              application/json:
+                schema:
+                  $ref: '#/components/schemas/DashboardCopySchema'
+          responses:
+            200:
+              description: Id of new dashboard and last modified time
+              content:
+                application/json:
+                  schema:
+                    type: object
+                    properties:
+                      id:
+                        type: number
+                      last_modified_time:
+                        type: number
+            400:
+              $ref: '#/components/responses/400'
+            401:
+              $ref: '#/components/responses/401'
+            403:
+              $ref: '#/components/responses/403'
+            404:
+              $ref: '#/components/responses/404'
+            500:
+              $ref: '#/components/responses/500'
+        """
+        try:
+            data = DashboardCopySchema().load(request.json)
+        except ValidationError as error:
+            return self.response_400(message=error.messages)
+
+        dash = DashboardDAO.copy_dashboard(original_dash, data)
+        return self.response(
+            200,
+            result={
+                "id": dash.id,
+                "last_modified_time": dash.changed_on.replace(
+                    microsecond=0
+                ).timestamp(),
+            },
+        )

superset/dashboards/dao.py

@@ -19,6 +19,7 @@
 from datetime import datetime
 from typing import Any, Dict, List, Optional, Union
 
+from flask import g
 from flask_appbuilder.models.sqla.interface import SQLAInterface
 from sqlalchemy.exc import SQLAlchemyError
 
@@ -262,13 +263,6 @@ def set_dash_metadata(  # pylint: disable=too-many-locals
             # positions have its own column, no need to store it in metadata
             md.pop("positions", None)
 
-        # The css and dashboard_title properties are not part of the metadata
-        # TODO (geido): remove by refactoring/deprecating save_dash endpoint
-        if data.get("css") is not None:
-            dashboard.css = data.get("css")
-        if data.get("dashboard_title") is not None:
-            dashboard.dashboard_title = data.get("dashboard_title")
-
         if new_filter_scopes:
             md["filter_scopes"] = new_filter_scopes
         else:
@@ -308,6 +302,41 @@ def favorited_ids(dashboards: List[Dashboard]) -> List[FavStar]:
             .all()
         ]
 
+    @classmethod
+    def copy_dashboard(
+        cls, original_dash: Dashboard, data: Dict[str, Any]
+    ) -> Dashboard:
+        dash = Dashboard()
+        dash.owners = [g.user] if g.user else []
+        dash.dashboard_title = data["dashboard_title"]
+        dash.css = data.get("css")
+
+        metadata = json.loads(data["json_metadata"])
+        old_to_new_slice_ids: Dict[int, int] = {}
+        if data.get("duplicate_slices"):
+            # Duplicating slices as well, mapping old ids to new ones
+            for slc in original_dash.slices:
+                new_slice = slc.clone()
+                new_slice.owners = [g.user] if g.user else []
+                db.session.add(new_slice)
+                db.session.flush()
+                new_slice.dashboards.append(dash)
+                old_to_new_slice_ids[slc.id] = new_slice.id
+
+            # update chartId of layout entities
+            for value in metadata["positions"].values():
+                if isinstance(value, dict) and value.get("meta", {}).get("chartId"):
+                    old_id = value["meta"]["chartId"]
+                    new_id = old_to_new_slice_ids.get(old_id)
+                    value["meta"]["chartId"] = new_id
+        else:
+            dash.slices = original_dash.slices
+
+        cls.set_dash_metadata(dash, metadata, old_to_new_slice_ids)
+        db.session.add(dash)
+        db.session.commit()
+        return dash
+
     @staticmethod
     def add_favorite(dashboard: Dashboard) -> None:
         ids = DashboardDAO.favorited_ids([dashboard])

superset/dashboards/schemas.py

@@ -266,7 +266,7 @@ class DashboardPostSchema(BaseDashboardSchema):
     position_json = fields.String(
         description=position_json_description, validate=validate_json
     )
-    css = fields.String()
+    css = fields.String(description=css_description)
     json_metadata = fields.String(
         description=json_metadata_description,
         validate=validate_json_metadata,
@@ -280,6 +280,23 @@ class DashboardPostSchema(BaseDashboardSchema):
     external_url = fields.String(allow_none=True)
 
 
+class DashboardCopySchema(Schema):
+    dashboard_title = fields.String(
+        description=dashboard_title_description,
+        allow_none=True,
+        validate=Length(0, 500),
+    )
+    css = fields.String(description=css_description)
+    json_metadata = fields.String(
+        description=json_metadata_description,
+        validate=validate_json_metadata,
+        required=True,
+    )
+    duplicate_slices = fields.Boolean(
+        description="Whether or not to also copy all charts on the dashboard"
+    )
+
+
 class DashboardPutSchema(BaseDashboardSchema):
     dashboard_title = fields.String(
         description=dashboard_title_description,

superset/views/core.py

@@ -1243,6 +1243,7 @@ def tables(  # pylint: disable=no-self-use
     @has_access_api
     @event_logger.log_this
     @expose("/copy_dash/<int:dashboard_id>/", methods=["GET", "POST"])
+    @deprecated()
     def copy_dash(  # pylint: disable=no-self-use
         self, dashboard_id: int
     ) -> FlaskResponse:
@@ -1259,6 +1260,7 @@ def copy_dash(  # pylint: disable=no-self-use
 
         dash.owners = [g.user] if g.user else []
         dash.dashboard_title = data["dashboard_title"]
+        dash.css = data.get("css")
 
         old_to_new_slice_ids: Dict[int, int] = {}
         if data["duplicate_slices"]:
@@ -1293,6 +1295,7 @@ def copy_dash(  # pylint: disable=no-self-use
     @has_access_api
     @event_logger.log_this
     @expose("/save_dash/<int:dashboard_id>/", methods=["GET", "POST"])
+    @deprecated()
     def save_dash(  # pylint: disable=no-self-use
         self, dashboard_id: int
     ) -> FlaskResponse:
@@ -1319,6 +1322,10 @@ def save_dash(  # pylint: disable=no-self-use
         # remove to avoid confusion.
         data.pop("last_modified_time", None)
 
+        if data.get("css") is not None:
+            dash.css = data["css"]
+        if data.get("dashboard_title") is not None:
+            dash.dashboard_title = data["dashboard_title"]
         DashboardDAO.set_dash_metadata(dash, data)
         session.merge(dash)
         session.commit()