diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index ad59ac378c591..d3216411ebf7b 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/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)); diff --git a/superset-frontend/src/dashboard/actions/dashboardState.test.js b/superset-frontend/src/dashboard/actions/dashboardState.test.js index 00b358bc43972..5fbe28cb1fc2c 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.test.js +++ b/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', () => { diff --git a/superset-frontend/src/dashboard/components/SaveModal.tsx b/superset-frontend/src/dashboard/components/SaveModal.tsx index dbdcb7b55200e..d024ffc914666 100644 --- a/superset-frontend/src/dashboard/components/SaveModal.tsx +++ b/superset-frontend/src/dashboard/components/SaveModal.tsx @@ -153,13 +153,8 @@ class SaveModal extends React.PureComponent { ); } 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?.(); diff --git a/superset/constants.py b/superset/constants.py index f41cd47ee2b77..958592b62fb59 100644 --- a/superset/constants.py +++ b/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 = { diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index a252cff000118..e2744e38b6dd5 100644 --- a/superset/dashboards/api.py +++ b/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("//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(), + }, + ) diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index a51ddbb92c230..89fca4619a0bf 100644 --- a/superset/dashboards/dao.py +++ b/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]) diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 17e9509f06224..014f5dafd53ba 100644 --- a/superset/dashboards/schemas.py +++ b/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, diff --git a/superset/views/core.py b/superset/views/core.py index 0f888454b01f3..6559db125422e 100755 --- a/superset/views/core.py +++ b/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//", 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//", 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() diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 29be38bc1b5b8..2ba00506d7628 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -20,7 +20,7 @@ from io import BytesIO from time import sleep from typing import List, Optional -from unittest.mock import patch +from unittest.mock import ANY, patch from zipfile import is_zipfile, ZipFile from tests.integration_tests.insert_chart_mixin import InsertChartMixin @@ -2171,3 +2171,95 @@ def test_gets_not_created_by_user_dashboards_filter(self): self.assertEqual(data["count"], len(expected_models)) db.session.delete(dashboard) db.session.commit() + + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + def test_copy_dashboard(self): + self.login(username="admin") + original_dash = ( + db.session.query(Dashboard).filter_by(slug="world_health").first() + ) + + data = { + "dashboard_title": "copied dash", + "css": "", + "duplicate_slices": False, + "json_metadata": json.dumps( + { + "positions": original_dash.position, + "color_namespace": "Color Namespace Test", + "color_scheme": "Color Scheme Test", + } + ), + } + pk = original_dash.id + uri = f"api/v1/dashboard/{pk}/copy/" + rv = self.client.post(uri, json=data) + self.assertEqual(rv.status_code, 200) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(response, {"result": {"id": ANY, "last_modified_time": ANY}}) + + dash = ( + db.session.query(Dashboard) + .filter(Dashboard.id == response["result"]["id"]) + .one() + ) + self.assertNotEqual(dash.id, original_dash.id) + self.assertEqual(len(dash.position), len(original_dash.position)) + self.assertEqual(dash.dashboard_title, "copied dash") + self.assertEqual(dash.css, "") + self.assertEqual(dash.owners, [security_manager.find_user("admin")]) + self.assertCountEqual(dash.slices, original_dash.slices) + self.assertEqual(dash.params_dict["color_namespace"], "Color Namespace Test") + self.assertEqual(dash.params_dict["color_scheme"], "Color Scheme Test") + + db.session.delete(dash) + db.session.commit() + + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + def test_copy_dashboard_duplicate_slices(self): + self.login(username="admin") + original_dash = ( + db.session.query(Dashboard).filter_by(slug="world_health").first() + ) + + data = { + "dashboard_title": "copied dash", + "css": "", + "duplicate_slices": True, + "json_metadata": json.dumps( + { + "positions": original_dash.position, + "color_namespace": "Color Namespace Test", + "color_scheme": "Color Scheme Test", + } + ), + } + pk = original_dash.id + uri = f"api/v1/dashboard/{pk}/copy/" + rv = self.client.post(uri, json=data) + self.assertEqual(rv.status_code, 200) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(response, {"result": {"id": ANY, "last_modified_time": ANY}}) + + dash = ( + db.session.query(Dashboard) + .filter(Dashboard.id == response["result"]["id"]) + .one() + ) + self.assertNotEqual(dash.id, original_dash.id) + self.assertEqual(len(dash.position), len(original_dash.position)) + self.assertEqual(dash.dashboard_title, "copied dash") + self.assertEqual(dash.css, "") + self.assertEqual(dash.owners, [security_manager.find_user("admin")]) + self.assertEqual(dash.params_dict["color_namespace"], "Color Namespace Test") + self.assertEqual(dash.params_dict["color_scheme"], "Color Scheme Test") + self.assertEqual(len(dash.slices), len(original_dash.slices)) + for original_slc in original_dash.slices: + for slc in dash.slices: + self.assertNotEqual(slc.id, original_slc.id) + + for slc in dash.slices: + db.session.delete(slc) + + db.session.delete(dash) + db.session.commit() diff --git a/tests/integration_tests/dashboards/consts.py b/tests/integration_tests/dashboards/consts.py index a6e36839be9ed..59d1c01fa9987 100644 --- a/tests/integration_tests/dashboards/consts.py +++ b/tests/integration_tests/dashboards/consts.py @@ -25,7 +25,6 @@ GET_DASHBOARD_VIEW_URL_FORMAT = "/superset/dashboard/{}/" SAVE_DASHBOARD_URL_FORMAT = "/superset/save_dash/{}/" -COPY_DASHBOARD_URL_FORMAT = "/superset/copy_dash/{}/" ADD_SLICES_URL_FORMAT = "/superset/add_slices/{}/" DELETE_DASHBOARD_VIEW_URL_FORMAT = "/dashboard/delete/{}" diff --git a/tests/integration_tests/dashboards/dao_tests.py b/tests/integration_tests/dashboards/dao_tests.py index 672e930364f2a..f252dbe5c6352 100644 --- a/tests/integration_tests/dashboards/dao_tests.py +++ b/tests/integration_tests/dashboards/dao_tests.py @@ -127,3 +127,60 @@ def test_get_dashboard_changed_on(self, mock_sm_g, mock_g): DashboardDAO.set_dash_metadata(dashboard, original_data) db.session.merge(dashboard) db.session.commit() + + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + @patch("superset.dashboards.dao.g") + def test_copy_dashboard(self, mock_g): + mock_g.user = security_manager.find_user("admin") + original_dash = ( + db.session.query(Dashboard).filter_by(slug="world_health").first() + ) + metadata = json.loads(original_dash.json_metadata) + metadata["positions"] = original_dash.position + dash_data = { + "dashboard_title": "copied dash", + "json_metadata": json.dumps(metadata), + "css": "", + "duplicate_slices": False, + } + dash = DashboardDAO.copy_dashboard(original_dash, dash_data) + self.assertNotEqual(dash.id, original_dash.id) + self.assertEqual(len(dash.position), len(original_dash.position)) + self.assertEqual(dash.dashboard_title, "copied dash") + self.assertEqual(dash.css, "") + self.assertEqual(dash.owners, [security_manager.find_user("admin")]) + self.assertCountEqual(dash.slices, original_dash.slices) + + db.session.delete(dash) + db.session.commit() + + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + @patch("superset.dashboards.dao.g") + def test_copy_dashboard_duplicate_slices(self, mock_g): + mock_g.user = security_manager.find_user("admin") + original_dash = ( + db.session.query(Dashboard).filter_by(slug="world_health").first() + ) + metadata = json.loads(original_dash.json_metadata) + metadata["positions"] = original_dash.position + dash_data = { + "dashboard_title": "copied dash", + "json_metadata": json.dumps(metadata), + "css": "", + "duplicate_slices": True, + } + dash = DashboardDAO.copy_dashboard(original_dash, dash_data) + self.assertNotEqual(dash.id, original_dash.id) + self.assertEqual(len(dash.position), len(original_dash.position)) + self.assertEqual(dash.dashboard_title, "copied dash") + self.assertEqual(dash.css, "") + self.assertEqual(dash.owners, [security_manager.find_user("admin")]) + self.assertEqual(len(dash.slices), len(original_dash.slices)) + for original_slc in original_dash.slices: + for slc in dash.slices: + self.assertNotEqual(slc.id, original_slc.id) + + for slc in dash.slices: + db.session.delete(slc) + db.session.delete(dash) + db.session.commit()