From 9638f3134d06fbe92dc3e7b565f34711e02970ad Mon Sep 17 00:00:00 2001 From: Lily Kuang Date: Thu, 25 May 2023 15:22:06 +0200 Subject: [PATCH] added save_dash back --- superset/views/core.py | 53 +++++ tests/integration_tests/dashboard_tests.py | 218 ++++++++++++++++++ .../integration_tests/dashboards/base_case.py | 34 +++ tests/integration_tests/dashboards/consts.py | 1 + tests/integration_tests/security_tests.py | 2 + 5 files changed, 308 insertions(+) diff --git a/superset/views/core.py b/superset/views/core.py index 5acb2779fe7a6..e672c97144205 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1118,6 +1118,59 @@ def copy_dash( # pylint: disable=no-self-use session.close() return json_success(dash_json) + @api + @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: + """Save a dashboard's metadata""" + session = db.session() + dash = session.query(Dashboard).get(dashboard_id) + security_manager.raise_for_ownership(dash) + data = json.loads(request.form["data"]) + # client-side send back last_modified_time which was set when + # the dashboard was open. it was use to avoid mid-air collision. + remote_last_modified_time = data.get("last_modified_time") + current_last_modified_time = dash.changed_on.replace(microsecond=0).timestamp() + if ( + remote_last_modified_time + and remote_last_modified_time < current_last_modified_time + ): + return json_error_response( + __( + "This dashboard was changed recently. " + "Please reload dashboard to get latest version." + ), + 412, + ) + # 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() + + # get updated changed_on + dash = session.query(Dashboard).get(dashboard_id) + last_modified_time = dash.changed_on.replace(microsecond=0).timestamp() + session.close() + return json_success( + json.dumps({"status": "SUCCESS", "last_modified_time": last_modified_time}) + ) + @api @has_access_api @event_logger.log_this diff --git a/tests/integration_tests/dashboard_tests.py b/tests/integration_tests/dashboard_tests.py index b532d3772b843..d54151db83c2d 100644 --- a/tests/integration_tests/dashboard_tests.py +++ b/tests/integration_tests/dashboard_tests.py @@ -16,6 +16,7 @@ # under the License. # isort:skip_file """Unit tests for Superset""" +from datetime import datetime import json import re import unittest @@ -153,6 +154,171 @@ def test_new_dashboard(self): db.session.delete(created_dashboard) db.session.commit() + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + def test_save_dash(self, username="admin"): + self.login(username=username) + dash = db.session.query(Dashboard).filter_by(slug="births").first() + positions = self.get_mock_positions(dash) + data = { + "css": "", + "expanded_slices": {}, + "positions": positions, + "dashboard_title": dash.dashboard_title, + # set a further modified_time for unit test + "last_modified_time": datetime.now().timestamp() + 1000, + } + url = "/superset/save_dash/{}/".format(dash.id) + resp = self.get_resp(url, data=dict(data=json.dumps(data))) + self.assertIn("SUCCESS", resp) + + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + def test_save_dash_with_filter(self, username="admin"): + self.login(username=username) + dash = db.session.query(Dashboard).filter_by(slug="world_health").first() + + positions = self.get_mock_positions(dash) + filters = {str(dash.slices[0].id): {"region": ["North America"]}} + default_filters = json.dumps(filters) + data = { + "css": "", + "expanded_slices": {}, + "positions": positions, + "dashboard_title": dash.dashboard_title, + "default_filters": default_filters, + # set a further modified_time for unit test + "last_modified_time": datetime.now().timestamp() + 1000, + } + + url = "/superset/save_dash/{}/".format(dash.id) + resp = self.get_resp(url, data=dict(data=json.dumps(data))) + self.assertIn("SUCCESS", resp) + + updatedDash = db.session.query(Dashboard).filter_by(slug="world_health").first() + new_url = updatedDash.url + self.assertIn("world_health", new_url) + self.assertNotIn("preselect_filters", new_url) + + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + def test_save_dash_with_invalid_filters(self, username="admin"): + self.login(username=username) + dash = db.session.query(Dashboard).filter_by(slug="world_health").first() + + # add an invalid filter slice + positions = self.get_mock_positions(dash) + filters = {str(99999): {"region": ["North America"]}} + default_filters = json.dumps(filters) + data = { + "css": "", + "expanded_slices": {}, + "positions": positions, + "dashboard_title": dash.dashboard_title, + "default_filters": default_filters, + # set a further modified_time for unit test + "last_modified_time": datetime.now().timestamp() + 1000, + } + + url = "/superset/save_dash/{}/".format(dash.id) + resp = self.get_resp(url, data=dict(data=json.dumps(data))) + self.assertIn("SUCCESS", resp) + + updatedDash = db.session.query(Dashboard).filter_by(slug="world_health").first() + new_url = updatedDash.url + self.assertNotIn("region", new_url) + + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + def test_save_dash_with_dashboard_title(self, username="admin"): + self.login(username=username) + dash = db.session.query(Dashboard).filter_by(slug="births").first() + origin_title = dash.dashboard_title + positions = self.get_mock_positions(dash) + data = { + "css": "", + "expanded_slices": {}, + "positions": positions, + "dashboard_title": "new title", + # set a further modified_time for unit test + "last_modified_time": datetime.now().timestamp() + 1000, + } + url = "/superset/save_dash/{}/".format(dash.id) + self.get_resp(url, data=dict(data=json.dumps(data))) + updatedDash = db.session.query(Dashboard).filter_by(slug="births").first() + self.assertEqual(updatedDash.dashboard_title, "new title") + # bring back dashboard original title + data["dashboard_title"] = origin_title + self.get_resp(url, data=dict(data=json.dumps(data))) + + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + def test_save_dash_with_colors(self, username="admin"): + self.login(username=username) + dash = db.session.query(Dashboard).filter_by(slug="births").first() + positions = self.get_mock_positions(dash) + new_label_colors = {"data value": "random color"} + data = { + "css": "", + "expanded_slices": {}, + "positions": positions, + "dashboard_title": dash.dashboard_title, + "color_namespace": "Color Namespace Test", + "color_scheme": "Color Scheme Test", + "label_colors": new_label_colors, + # set a further modified_time for unit test + "last_modified_time": datetime.now().timestamp() + 1000, + } + url = "/superset/save_dash/{}/".format(dash.id) + self.get_resp(url, data=dict(data=json.dumps(data))) + updatedDash = db.session.query(Dashboard).filter_by(slug="births").first() + self.assertIn("color_namespace", updatedDash.json_metadata) + self.assertIn("color_scheme", updatedDash.json_metadata) + self.assertIn("label_colors", updatedDash.json_metadata) + # bring back original dashboard + del data["color_namespace"] + del data["color_scheme"] + del data["label_colors"] + self.get_resp(url, data=dict(data=json.dumps(data))) + + @pytest.mark.usefixtures( + "load_birth_names_dashboard_with_slices", + "cleanup_copied_dash", + "load_unicode_dashboard_with_position", + ) + def test_copy_dash(self, username="admin"): + self.login(username=username) + dash = db.session.query(Dashboard).filter_by(slug="births").first() + positions = self.get_mock_positions(dash) + new_label_colors = {"data value": "random color"} + data = { + "css": "", + "duplicate_slices": False, + "expanded_slices": {}, + "positions": positions, + "dashboard_title": "Copy Of Births", + "color_namespace": "Color Namespace Test", + "color_scheme": "Color Scheme Test", + "label_colors": new_label_colors, + # set a further modified_time for unit test + "last_modified_time": datetime.now().timestamp() + 1000, + } + + # Save changes to Births dashboard and retrieve updated dash + dash_id = dash.id + url = "/superset/save_dash/{}/".format(dash_id) + self.client.post(url, data=dict(data=json.dumps(data))) + dash = db.session.query(Dashboard).filter_by(id=dash_id).first() + orig_json_data = dash.data + + # Verify that copy matches original + url = "/superset/copy_dash/{}/".format(dash_id) + resp = self.get_json_resp(url, data=dict(data=json.dumps(data))) + self.assertEqual(resp["dashboard_title"], "Copy Of Births") + self.assertEqual(resp["position_json"], orig_json_data["position_json"]) + self.assertEqual(resp["metadata"], orig_json_data["metadata"]) + # check every attribute in each dashboard's slices list, + # exclude modified and changed_on attribute + for index, slc in enumerate(orig_json_data["slices"]): + for key in slc: + if key not in ["modified", "changed_on", "changed_on_humanized"]: + self.assertEqual(slc[key], resp["slices"][index][key]) + @pytest.mark.usefixtures( "load_energy_table_with_slice", "load_birth_names_dashboard_with_slices" ) @@ -184,6 +350,39 @@ def test_add_slices(self, username="admin"): dash.slices = [o for o in dash.slices if o.slice_name != "Energy Force Layout"] db.session.commit() + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + def test_remove_slices(self, username="admin"): + self.login(username=username) + dash = db.session.query(Dashboard).filter_by(slug="births").first() + origin_slices_length = len(dash.slices) + + positions = self.get_mock_positions(dash) + # remove one chart + chart_keys = [] + for key in positions.keys(): + if key.startswith("DASHBOARD_CHART_TYPE"): + chart_keys.append(key) + positions.pop(chart_keys[0]) + + data = { + "css": "", + "expanded_slices": {}, + "positions": positions, + "dashboard_title": dash.dashboard_title, + # set a further modified_time for unit test + "last_modified_time": datetime.now().timestamp() + 1000, + } + + # save dash + dash_id = dash.id + url = "/superset/save_dash/{}/".format(dash_id) + self.client.post(url, data=dict(data=json.dumps(data))) + dash = db.session.query(Dashboard).filter_by(id=dash_id).first() + + # verify slices data + data = dash.data + self.assertEqual(len(data["slices"]), origin_slices_length - 1) + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") @pytest.mark.usefixtures("public_role_like_gamma") def test_public_user_dashboard_access(self): @@ -244,6 +443,25 @@ def test_dashboard_with_created_by_can_be_accessed_by_public_users(self): # Cleanup self.revoke_public_access_to_table(table) + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + def test_only_owners_can_save(self): + dash = db.session.query(Dashboard).filter_by(slug="births").first() + dash.owners = [] + db.session.merge(dash) + db.session.commit() + self.test_save_dash("admin") + + self.logout() + self.assertRaises(Exception, self.test_save_dash, "alpha") + + alpha = security_manager.find_user("alpha") + + dash = db.session.query(Dashboard).filter_by(slug="births").first() + dash.owners = [alpha] + db.session.merge(dash) + db.session.commit() + self.test_save_dash("alpha") + @pytest.mark.usefixtures("load_energy_table_with_slice", "load_dashboard") def test_users_can_list_published_dashboard(self): self.login("alpha") diff --git a/tests/integration_tests/dashboards/base_case.py b/tests/integration_tests/dashboards/base_case.py index 319639b16f023..a0a1ff630f08d 100644 --- a/tests/integration_tests/dashboards/base_case.py +++ b/tests/integration_tests/dashboards/base_case.py @@ -23,6 +23,9 @@ from superset import app, security_manager from tests.integration_tests.base_tests import SupersetTestCase from tests.integration_tests.dashboards.consts import * +from tests.integration_tests.dashboards.dashboard_test_utils import ( + build_save_dash_parts, +) from tests.integration_tests.dashboards.superset_factory_util import ( delete_all_inserted_objects, ) @@ -45,6 +48,17 @@ def get_dashboards_list_response(self) -> Response: def get_dashboards_api_response(self) -> Response: return self.client.get(DASHBOARDS_API_URL) + def save_dashboard_via_view( + self, dashboard_id: Union[str, int], dashboard_data: Dict[str, Any] + ) -> Response: + save_dash_url = SAVE_DASHBOARD_URL_FORMAT.format(dashboard_id) + return self.get_resp(save_dash_url, data=dict(data=json.dumps(dashboard_data))) + + def save_dashboard( + self, dashboard_id: Union[str, int], dashboard_data: Dict[str, Any] + ) -> Response: + return self.save_dashboard_via_view(dashboard_id, dashboard_data) + def delete_dashboard_via_view(self, dashboard_id: int) -> Response: delete_dashboard_url = DELETE_DASHBOARD_VIEW_URL_FORMAT.format(dashboard_id) return self.get_resp(delete_dashboard_url, {}) @@ -76,6 +90,26 @@ def assert_permissions_were_deleted(self, deleted_dashboard): view_menu = security_manager.find_view_menu(deleted_dashboard.view_name) self.assertIsNone(view_menu) + def save_dash_basic_case(self, username=ADMIN_USERNAME): + # arrange + self.login(username=username) + ( + dashboard_to_save, + data_before_change, + data_after_change, + ) = build_save_dash_parts() + + # act + save_dash_response = self.save_dashboard_via_view( + dashboard_to_save.id, data_after_change + ) + + # assert + self.assertIn("SUCCESS", save_dash_response) + + # post test + self.save_dashboard(dashboard_to_save.id, data_before_change) + def clean_created_objects(self): with app.test_request_context(): self.logout() diff --git a/tests/integration_tests/dashboards/consts.py b/tests/integration_tests/dashboards/consts.py index b3eb0937ce0e0..59d1c01fa9987 100644 --- a/tests/integration_tests/dashboards/consts.py +++ b/tests/integration_tests/dashboards/consts.py @@ -24,6 +24,7 @@ EXPORT_DASHBOARDS_API_URL_WITH_QUERY_FORMAT = EXPORT_DASHBOARDS_API_URL + QUERY_FORMAT GET_DASHBOARD_VIEW_URL_FORMAT = "/superset/dashboard/{}/" +SAVE_DASHBOARD_URL_FORMAT = "/superset/save_dash/{}/" ADD_SLICES_URL_FORMAT = "/superset/add_slices/{}/" DELETE_DASHBOARD_VIEW_URL_FORMAT = "/dashboard/delete/{}" diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index 32e1d9b351173..01ea96c9c4c00 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -1363,6 +1363,7 @@ def assert_can_gamma(self, perm_set): self.assertIn(("can_explore_json", "Superset"), perm_set) self.assertIn(("can_fave_dashboards", "Superset"), perm_set) self.assertIn(("can_fave_slices", "Superset"), perm_set) + self.assertIn(("can_save_dash", "Superset"), perm_set) self.assertIn(("can_slice", "Superset"), perm_set) self.assertIn(("can_explore_json", "Superset"), perm_set) self.assertIn(("can_userinfo", "UserDBModelView"), perm_set) @@ -1562,6 +1563,7 @@ def test_gamma_permissions(self): self.assertIn(("can_explore_json", "Superset"), gamma_perm_set) self.assertIn(("can_fave_dashboards", "Superset"), gamma_perm_set) self.assertIn(("can_fave_slices", "Superset"), gamma_perm_set) + self.assertIn(("can_save_dash", "Superset"), gamma_perm_set) self.assertIn(("can_slice", "Superset"), gamma_perm_set) self.assertIn(("can_userinfo", "UserDBModelView"), gamma_perm_set)