diff --git a/superset/views/dashboard/api.py b/superset/views/dashboard/api.py index 8704904db5f..857f99ebd32 100644 --- a/superset/views/dashboard/api.py +++ b/superset/views/dashboard/api.py @@ -15,24 +15,30 @@ # specific language governing permissions and limitations # under the License. import json +import logging import re from typing import Dict, List -from flask import current_app, make_response +from flask import current_app, g, make_response from flask_appbuilder.api import expose, protect, rison, safe from flask_appbuilder.models.sqla.interface import SQLAInterface +from flask_babel import lazy_gettext as _, ngettext from marshmallow import fields, post_load, pre_load, Schema, ValidationError from marshmallow.validate import Length +from sqlalchemy.exc import SQLAlchemyError -from superset.exceptions import SupersetException +from superset.exceptions import SupersetException, SupersetSecurityException from superset.models.dashboard import Dashboard from superset.utils import core as utils -from superset.views.base import generate_download_headers +from superset.views.base import check_ownership, generate_download_headers from superset.views.base_api import BaseOwnedModelRestApi from superset.views.base_schemas import BaseOwnedSchema, validate_owner from .mixin import DashboardMixin +logger = logging.getLogger(__name__) +get_delete_ids_schema = {"type": "array", "items": {"type": "integer"}} + class DashboardJSONMetadataSchema(Schema): timed_refresh_immune_slices = fields.List(fields.Integer()) @@ -136,6 +142,7 @@ class DashboardRestApi(DashboardMixin, BaseOwnedModelRestApi): "post": "add", "put": "edit", "delete": "delete", + "bulk_delete": "delete", "info": "list", "related": "list", } @@ -172,6 +179,93 @@ class DashboardRestApi(DashboardMixin, BaseOwnedModelRestApi): } filter_rel_fields_field = {"owners": "first_name", "slices": "slice_name"} + @expose("/", methods=["DELETE"]) + @protect() + @safe + @rison(get_delete_ids_schema) + def bulk_delete(self, **kwargs): # pylint: disable=arguments-differ + """Delete bulk Dashboards + --- + delete: + parameters: + - in: query + name: q + content: + application/json: + schema: + type: array + items: + type: integer + responses: + 200: + description: Dashboard bulk delete + content: + application/json: + schema: + type: object + properties: + message: + type: string + 401: + $ref: '#/components/responses/401' + 403: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + item_ids = kwargs["rison"] + query = self.datamodel.session.query(Dashboard).filter( + Dashboard.id.in_(item_ids) + ) + items = self._base_filters.apply_all(query).all() + if not items: + return self.response_404() + # Check user ownership over the items + for item in items: + try: + check_ownership(item) + except SupersetSecurityException as e: + logger.warning( + f"Dashboard {item} was not deleted, " + f"because the user ({g.user}) does not own it" + ) + return self.response(403, message=_("No dashboards deleted")) + except SQLAlchemyError as e: + logger.error(f"Error checking dashboard ownership {e}") + return self.response_422(message=str(e)) + # bulk delete, first delete related data + for item in items: + try: + item.slices = [] + item.owners = [] + self.datamodel.session.merge(item) + except SQLAlchemyError as e: + logger.error(f"Error bulk deleting related data on dashboards {e}") + self.datamodel.session.rollback() + return self.response_422(message=str(e)) + # bulk delete itself + try: + self.datamodel.session.query(Dashboard).filter( + Dashboard.id.in_(item_ids) + ).delete(synchronize_session="fetch") + except SQLAlchemyError as e: + logger.error(f"Error bulk deleting dashboards {e}") + self.datamodel.session.rollback() + return self.response_422(message=str(e)) + self.datamodel.session.commit() + return self.response( + 200, + message=ngettext( + f"Deleted %(num)d dashboard", + f"Deleted %(num)d dashboards", + num=len(items), + ), + ) + @expose("/export/", methods=["GET"]) @protect() @safe diff --git a/tests/dashboard_api_tests.py b/tests/dashboard_api_tests.py index 6f037217795..5b48bb852a3 100644 --- a/tests/dashboard_api_tests.py +++ b/tests/dashboard_api_tests.py @@ -78,6 +78,44 @@ class DashboardApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): model = db.session.query(models.Dashboard).get(dashboard_id) self.assertEqual(model, None) + def test_delete_bulk_dashboards(self): + """ + Dashboard API: Test delete bulk + """ + admin_id = self.get_user("admin").id + dashboard_count = 4 + dashboard_ids = list() + for dashboard_name_index in range(dashboard_count): + dashboard_ids.append( + self.insert_dashboard( + f"title{dashboard_name_index}", + f"slug{dashboard_name_index}", + [admin_id], + ).id + ) + self.login(username="admin") + argument = dashboard_ids + uri = f"api/v1/dashboard/?q={prison.dumps(argument)}" + rv = self.client.delete(uri) + self.assertEqual(rv.status_code, 200) + response = json.loads(rv.data.decode("utf-8")) + expected_response = {"message": f"Deleted {dashboard_count} dashboards"} + self.assertEqual(response, expected_response) + for dashboard_id in dashboard_ids: + model = db.session.query(models.Dashboard).get(dashboard_id) + self.assertEqual(model, None) + + def test_delete_bulk_dashboards_bad_request(self): + """ + Dashboard API: Test delete bulk bad request + """ + dashboard_ids = [1, "a"] + self.login(username="admin") + argument = dashboard_ids + uri = f"api/v1/dashboard/?q={prison.dumps(argument)}" + rv = self.client.delete(uri) + self.assertEqual(rv.status_code, 400) + def test_delete_not_found_dashboard(self): """ Dashboard API: Test not found delete @@ -88,6 +126,17 @@ class DashboardApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): rv = self.client.delete(uri) self.assertEqual(rv.status_code, 404) + def test_delete_bulk_dashboards_not_found(self): + """ + Dashboard API: Test delete bulk not found + """ + dashboard_ids = [1001, 1002] + self.login(username="admin") + argument = dashboard_ids + uri = f"api/v1/dashboard/?q={prison.dumps(argument)}" + rv = self.client.delete(uri) + self.assertEqual(rv.status_code, 404) + def test_delete_dashboard_admin_not_owned(self): """ Dashboard API: Test admin delete not owned @@ -102,6 +151,35 @@ class DashboardApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): model = db.session.query(models.Dashboard).get(dashboard_id) self.assertEqual(model, None) + def test_delete_bulk_dashboard_admin_not_owned(self): + """ + Dashboard API: Test admin delete bulk not owned + """ + gamma_id = self.get_user("gamma").id + dashboard_count = 4 + dashboard_ids = list() + for dashboard_name_index in range(dashboard_count): + dashboard_ids.append( + self.insert_dashboard( + f"title{dashboard_name_index}", + f"slug{dashboard_name_index}", + [gamma_id], + ).id + ) + + self.login(username="admin") + argument = dashboard_ids + uri = f"api/v1/dashboard/?q={prison.dumps(argument)}" + rv = self.client.delete(uri) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(rv.status_code, 200) + expected_response = {"message": f"Deleted {dashboard_count} dashboards"} + self.assertEqual(response, expected_response) + + for dashboard_id in dashboard_ids: + model = db.session.query(models.Dashboard).get(dashboard_id) + self.assertEqual(model, None) + def test_delete_dashboard_not_owned(self): """ Dashboard API: Test delete try not owned @@ -127,6 +205,68 @@ class DashboardApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): db.session.delete(user_alpha2) db.session.commit() + def test_delete_bulk_dashboard_not_owned(self): + """ + Dashboard API: Test delete bulk try not owned + """ + user_alpha1 = self.create_user( + "alpha1", "password", "Alpha", email="alpha1@superset.org" + ) + user_alpha2 = self.create_user( + "alpha2", "password", "Alpha", email="alpha2@superset.org" + ) + existing_slice = ( + db.session.query(Slice).filter_by(slice_name="Girl Name Cloud").first() + ) + + dashboard_count = 4 + dashboards = list() + for dashboard_name_index in range(dashboard_count): + dashboards.append( + self.insert_dashboard( + f"title{dashboard_name_index}", + f"slug{dashboard_name_index}", + [user_alpha1.id], + slices=[existing_slice], + published=True, + ) + ) + + owned_dashboard = self.insert_dashboard( + "title_owned", + "slug_owned", + [user_alpha2.id], + slices=[existing_slice], + published=True, + ) + + self.login(username="alpha2", password="password") + + # verify we can't delete not owned dashboards + arguments = [dashboard.id for dashboard in dashboards] + uri = f"api/v1/dashboard/?q={prison.dumps(arguments)}" + rv = self.client.delete(uri) + self.assertEqual(rv.status_code, 403) + response = json.loads(rv.data.decode("utf-8")) + expected_response = {"message": "No dashboards deleted"} + self.assertEqual(response, expected_response) + + # nothing is delete in bulk with a list of owned and not owned dashboards + arguments = [dashboard.id for dashboard in dashboards] + [owned_dashboard.id] + uri = f"api/v1/dashboard/?q={prison.dumps(arguments)}" + rv = self.client.delete(uri) + self.assertEqual(rv.status_code, 403) + response = json.loads(rv.data.decode("utf-8")) + expected_response = {"message": "No dashboards deleted"} + self.assertEqual(response, expected_response) + + for dashboard in dashboards: + db.session.delete(dashboard) + db.session.delete(owned_dashboard) + db.session.delete(user_alpha1) + db.session.delete(user_alpha2) + db.session.commit() + def test_create_dashboard(self): """ Dashboard API: Test create dashboard