From c34df6b7b356fc28e3c3a80a975ecdf2de7c0681 Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Fri, 20 Mar 2020 16:32:03 +0000 Subject: [PATCH] [dashboard] Refactor API using SIP-35 (#9315) * [dashboard] Refactor API using SIP-35 * [dashboard] Fix, import * [dashboard] more tests * [dashboards] a misc of improvements * [charts] Fix, DAO and tests * [dashboards] small exceptions refactor * [dashboards] lint * [dashboards] Improves comments on base classes * [dashboards] lint --- superset/app.py | 2 +- superset/commands/base.py | 3 +- superset/commands/exceptions.py | 32 +- .../commands/base.py => commands/utils.py} | 7 +- superset/dao/__init__.py | 16 + superset/dao/base.py | 111 +++++ superset/dao/exceptions.py | 57 +++ superset/dashboards/__init__.py | 16 + superset/dashboards/api.py | 391 ++++++++++++++++++ superset/dashboards/commands/__init__.py | 16 + superset/dashboards/commands/bulk_delete.py | 61 +++ superset/dashboards/commands/create.py | 67 +++ superset/dashboards/commands/delete.py | 61 +++ superset/dashboards/commands/exceptions.py | 64 +++ superset/dashboards/commands/update.py | 87 ++++ superset/dashboards/dao.py | 74 ++++ superset/dashboards/filters.py | 84 ++++ superset/dashboards/schemas.py | 87 ++++ superset/datasets/commands/create.py | 6 +- superset/datasets/commands/delete.py | 4 +- superset/datasets/commands/update.py | 6 +- superset/exceptions.py | 12 + superset/views/api.py | 5 - superset/views/dashboard/api.py | 319 -------------- tests/dashboards/__init__.py | 16 + .../api_tests.py} | 254 ++++++++++-- tests/dataset_api_tests.py | 16 +- 27 files changed, 1473 insertions(+), 401 deletions(-) rename superset/{datasets/commands/base.py => commands/utils.py} (88%) create mode 100644 superset/dao/__init__.py create mode 100644 superset/dao/base.py create mode 100644 superset/dao/exceptions.py create mode 100644 superset/dashboards/__init__.py create mode 100644 superset/dashboards/api.py create mode 100644 superset/dashboards/commands/__init__.py create mode 100644 superset/dashboards/commands/bulk_delete.py create mode 100644 superset/dashboards/commands/create.py create mode 100644 superset/dashboards/commands/delete.py create mode 100644 superset/dashboards/commands/exceptions.py create mode 100644 superset/dashboards/commands/update.py create mode 100644 superset/dashboards/dao.py create mode 100644 superset/dashboards/filters.py create mode 100644 superset/dashboards/schemas.py delete mode 100644 superset/views/dashboard/api.py create mode 100644 tests/dashboards/__init__.py rename tests/{dashboard_api_tests.py => dashboards/api_tests.py} (70%) diff --git a/superset/app.py b/superset/app.py index dc706fdc03c07..f1eace3e5d78f 100644 --- a/superset/app.py +++ b/superset/app.py @@ -152,7 +152,7 @@ def init_views(self) -> None: ) from superset.views.chart.api import ChartRestApi from superset.views.chart.views import SliceModelView, SliceAsync - from superset.views.dashboard.api import DashboardRestApi + from superset.dashboards.api import DashboardRestApi from superset.views.dashboard.views import ( DashboardModelView, Dashboard, diff --git a/superset/commands/base.py b/superset/commands/base.py index 9889d6f0cb3d8..44f46eb742215 100644 --- a/superset/commands/base.py +++ b/superset/commands/base.py @@ -26,7 +26,7 @@ class BaseCommand(ABC): def run(self): """ Run executes the command. Can raise command exceptions - :return: + :raises: CommandException """ pass @@ -35,5 +35,6 @@ def validate(self) -> None: """ Validate is normally called by run to validate data. Will raise exception if validation fails + :raises: CommandException """ pass diff --git a/superset/commands/exceptions.py b/superset/commands/exceptions.py index 83b3e1df4e502..61a18ebdc33f6 100644 --- a/superset/commands/exceptions.py +++ b/superset/commands/exceptions.py @@ -14,30 +14,25 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from typing import List, Optional +from typing import List +from flask_babel import lazy_gettext as _ from marshmallow import ValidationError +from superset.exceptions import SupersetException -class CommandException(Exception): - """ Common base class for Command exceptions. """ - - message = "" - def __init__(self, message: str = "", exception: Optional[Exception] = None): - if message: - self.message = message - self._exception = exception - super().__init__(self.message) +class CommandException(SupersetException): + """ Common base class for Command exceptions. """ - @property - def exception(self): - return self._exception + pass class CommandInvalidError(CommandException): """ Common base class for Command Invalid errors. """ + status = 422 + def __init__(self, message=""): self._invalid_exceptions = list() super().__init__(self.message) @@ -56,16 +51,27 @@ def normalized_messages(self): class UpdateFailedError(CommandException): + status = 500 message = "Command update failed" class CreateFailedError(CommandException): + status = 500 message = "Command create failed" class DeleteFailedError(CommandException): + status = 500 message = "Command delete failed" class ForbiddenError(CommandException): + status = 500 message = "Action is forbidden" + + +class OwnersNotFoundValidationError(ValidationError): + status = 422 + + def __init__(self): + super().__init__(_("Owners are invalid"), field_names=["owners"]) diff --git a/superset/datasets/commands/base.py b/superset/commands/utils.py similarity index 88% rename from superset/datasets/commands/base.py rename to superset/commands/utils.py index 646dfc328241a..9865549cfb325 100644 --- a/superset/datasets/commands/base.py +++ b/superset/commands/utils.py @@ -18,15 +18,14 @@ from flask_appbuilder.security.sqla.models import User -from superset.datasets.commands.exceptions import OwnersNotFoundValidationError -from superset.datasets.dao import DatasetDAO +from superset.commands.exceptions import OwnersNotFoundValidationError +from superset.extensions import security_manager def populate_owners(user: User, owners_ids: Optional[List[int]] = None) -> List[User]: """ Helper function for commands, will fetch all users from owners id's Can raise ValidationError - :param user: The current user :param owners_ids: A List of owners by id's """ @@ -36,7 +35,7 @@ def populate_owners(user: User, owners_ids: Optional[List[int]] = None) -> List[ if user.id not in owners_ids: owners.append(user) for owner_id in owners_ids: - owner = DatasetDAO.get_owner_by_id(owner_id) + owner = security_manager.get_user_by_id(owner_id) if not owner: raise OwnersNotFoundValidationError() owners.append(owner) diff --git a/superset/dao/__init__.py b/superset/dao/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/superset/dao/__init__.py @@ -0,0 +1,16 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. diff --git a/superset/dao/base.py b/superset/dao/base.py new file mode 100644 index 0000000000000..4f19efc4f8fb7 --- /dev/null +++ b/superset/dao/base.py @@ -0,0 +1,111 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from typing import Dict, Optional + +from flask_appbuilder.models.filters import BaseFilter +from flask_appbuilder.models.sqla import Model +from flask_appbuilder.models.sqla.interface import SQLAInterface +from sqlalchemy.exc import SQLAlchemyError + +from superset.dao.exceptions import ( + DAOConfigError, + DAOCreateFailedError, + DAODeleteFailedError, + DAOUpdateFailedError, +) +from superset.extensions import db + + +class BaseDAO: + """ + Base DAO, implement base CRUD sqlalchemy operations + """ + + model_cls: Optional[Model] = None + """ + Child classes need to state the Model class so they don't need to implement basic + create, update and delete methods + """ # pylint: disable=pointless-string-statement + base_filter: Optional[BaseFilter] = None + """ + Child classes can register base filtering to be aplied to all filter methods + """ # pylint: disable=pointless-string-statement + + @classmethod + def find_by_id(cls, model_id: int) -> Model: + """ + Retrives a model by id, if defined applies `base_filter` + """ + query = db.session.query(cls.model_cls) + if cls.base_filter: + data_model = SQLAInterface(cls.model_cls, db.session) + query = cls.base_filter( # pylint: disable=not-callable + "id", data_model + ).apply(query, None) + return query.filter_by(id=model_id).one_or_none() + + @classmethod + def create(cls, properties: Dict, commit=True) -> Optional[Model]: + """ + Generic for creating models + :raises: DAOCreateFailedError + """ + if cls.model_cls is None: + raise DAOConfigError() + model = cls.model_cls() # pylint: disable=not-callable + for key, value in properties.items(): + setattr(model, key, value) + try: + db.session.add(model) + if commit: + db.session.commit() + except SQLAlchemyError as e: # pragma: no cover + db.session.rollback() + raise DAOCreateFailedError(exception=e) + return model + + @classmethod + def update(cls, model: Model, properties: Dict, commit=True) -> Optional[Model]: + """ + Generic update a model + :raises: DAOCreateFailedError + """ + for key, value in properties.items(): + setattr(model, key, value) + try: + db.session.merge(model) + if commit: + db.session.commit() + except SQLAlchemyError as e: # pragma: no cover + db.session.rollback() + raise DAOUpdateFailedError(exception=e) + return model + + @classmethod + def delete(cls, model: Model, commit=True): + """ + Generic delete a model + :raises: DAOCreateFailedError + """ + try: + db.session.delete(model) + if commit: + db.session.commit() + except SQLAlchemyError as e: # pragma: no cover + db.session.rollback() + raise DAODeleteFailedError(exception=e) + return model diff --git a/superset/dao/exceptions.py b/superset/dao/exceptions.py new file mode 100644 index 0000000000000..5a6bbbe222bfe --- /dev/null +++ b/superset/dao/exceptions.py @@ -0,0 +1,57 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from superset.exceptions import SupersetException + + +class DAOException(SupersetException): + """ + Base DAO exception class + """ + + pass + + +class DAOCreateFailedError(DAOException): + """ + DAO Create failed + """ + + message = "Create failed" + + +class DAOUpdateFailedError(DAOException): + """ + DAO Update failed + """ + + message = "Updated failed" + + +class DAODeleteFailedError(DAOException): + """ + DAO Delete failed + """ + + message = "Delete failed" + + +class DAOConfigError(DAOException): + """ + DAO is miss configured + """ + + message = "DAO is not configured correctly missing model definition" diff --git a/superset/dashboards/__init__.py b/superset/dashboards/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/superset/dashboards/__init__.py @@ -0,0 +1,16 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py new file mode 100644 index 0000000000000..e960c4cbd2b48 --- /dev/null +++ b/superset/dashboards/api.py @@ -0,0 +1,391 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import logging + +from flask import g, make_response, request, Response +from flask_appbuilder.api import expose, protect, rison, safe +from flask_appbuilder.models.sqla.interface import SQLAInterface +from flask_babel import ngettext + +from superset.constants import RouteMethod +from superset.dashboards.commands.bulk_delete import BulkDeleteDashboardCommand +from superset.dashboards.commands.create import CreateDashboardCommand +from superset.dashboards.commands.delete import DeleteDashboardCommand +from superset.dashboards.commands.exceptions import ( + DashboardBulkDeleteFailedError, + DashboardCreateFailedError, + DashboardDeleteFailedError, + DashboardForbiddenError, + DashboardInvalidError, + DashboardNotFoundError, + DashboardUpdateFailedError, +) +from superset.dashboards.commands.update import UpdateDashboardCommand +from superset.dashboards.filters import DashboardFilter +from superset.dashboards.schemas import ( + DashboardPostSchema, + DashboardPutSchema, + get_delete_ids_schema, + get_export_ids_schema, +) +from superset.models.dashboard import Dashboard +from superset.views.base import generate_download_headers +from superset.views.base_api import BaseSupersetModelRestApi + +logger = logging.getLogger(__name__) + + +class DashboardRestApi(BaseSupersetModelRestApi): + datamodel = SQLAInterface(Dashboard) + include_route_methods = RouteMethod.REST_MODEL_VIEW_CRUD_SET | { + RouteMethod.EXPORT, + RouteMethod.RELATED, + "bulk_delete", # not using RouteMethod since locally defined + } + resource_name = "dashboard" + allow_browser_login = True + + class_permission_name = "DashboardModelView" + show_columns = [ + "id", + "charts", + "css", + "dashboard_title", + "json_metadata", + "owners.id", + "owners.username", + "changed_by_name", + "changed_by_url", + "changed_by.username", + "changed_on", + "position_json", + "published", + "url", + "slug", + "table_names", + ] + order_columns = ["dashboard_title", "changed_on", "published", "changed_by_fk"] + list_columns = [ + "changed_by_name", + "changed_by_url", + "changed_by.username", + "changed_on", + "dashboard_title", + "id", + "published", + "slug", + "url", + ] + edit_columns = [ + "dashboard_title", + "slug", + "owners", + "position_json", + "css", + "json_metadata", + "published", + ] + search_columns = ("dashboard_title", "slug", "owners", "published") + add_columns = edit_columns + base_order = ("changed_on", "desc") + + add_model_schema = DashboardPostSchema() + edit_model_schema = DashboardPutSchema() + + base_filters = [["slice", DashboardFilter, lambda: []]] + + openapi_spec_tag = "Dashboards" + order_rel_fields = { + "slices": ("slice_name", "asc"), + "owners": ("first_name", "asc"), + } + filter_rel_fields_field = {"owners": "first_name"} + allowed_rel_fields = {"owners"} + + @expose("/", methods=["POST"]) + @protect() + @safe + def post(self) -> Response: + """Creates a new Dashboard + --- + post: + description: >- + Create a new Dashboard + requestBody: + description: Dashboard schema + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/{{self.__class__.__name__}}.post' + responses: + 201: + description: Dashboard added + content: + application/json: + schema: + type: object + properties: + id: + type: number + result: + $ref: '#/components/schemas/{{self.__class__.__name__}}.post' + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + if not request.is_json: + return self.response_400(message="Request is not JSON") + item = self.add_model_schema.load(request.json) + # This validates custom Schema with custom validations + if item.errors: + return self.response_400(message=item.errors) + try: + new_model = CreateDashboardCommand(g.user, item.data).run() + return self.response(201, id=new_model.id, result=item.data) + except DashboardInvalidError as e: + return self.response_422(message=e.normalized_messages()) + except DashboardCreateFailedError as e: + logger.error(f"Error creating model {self.__class__.__name__}: {e}") + return self.response_422(message=str(e)) + + @expose("/", methods=["PUT"]) + @protect() + @safe + def put( # pylint: disable=too-many-return-statements, arguments-differ + self, pk: int + ) -> Response: + """Changes a Dashboard + --- + put: + description: >- + Changes a Dashboard + parameters: + - in: path + schema: + type: integer + name: pk + requestBody: + description: Dashboard schema + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/{{self.__class__.__name__}}.put' + responses: + 200: + description: Dashboard changed + content: + application/json: + schema: + type: object + properties: + id: + type: number + result: + $ref: '#/components/schemas/{{self.__class__.__name__}}.put' + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 403: + $ref: '#/components/responses/403' + 404: + $ref: '#/components/responses/404' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + if not request.is_json: + return self.response_400(message="Request is not JSON") + item = self.edit_model_schema.load(request.json) + # This validates custom Schema with custom validations + if item.errors: + return self.response_400(message=item.errors) + try: + changed_model = UpdateDashboardCommand(g.user, pk, item.data).run() + return self.response(200, id=changed_model.id, result=item.data) + except DashboardNotFoundError: + return self.response_404() + except DashboardForbiddenError: + return self.response_403() + except DashboardInvalidError as e: + return self.response_422(message=e.normalized_messages()) + except DashboardUpdateFailedError as e: + logger.error(f"Error updating model {self.__class__.__name__}: {e}") + return self.response_422(message=str(e)) + + @expose("/", methods=["DELETE"]) + @protect() + @safe + def delete(self, pk: int) -> Response: # pylint: disable=arguments-differ + """Deletes a Dashboard + --- + delete: + description: >- + Deletes a Dashboard + parameters: + - in: path + schema: + type: integer + name: pk + responses: + 200: + description: Dashboard deleted + content: + application/json: + schema: + type: object + properties: + message: + type: string + 401: + $ref: '#/components/responses/401' + 403: + $ref: '#/components/responses/403' + 404: + $ref: '#/components/responses/404' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + try: + DeleteDashboardCommand(g.user, pk).run() + return self.response(200, message="OK") + except DashboardNotFoundError: + return self.response_404() + except DashboardForbiddenError: + return self.response_403() + except DashboardDeleteFailedError as e: + logger.error(f"Error deleting model {self.__class__.__name__}: {e}") + return self.response_422(message=str(e)) + + @expose("/", methods=["DELETE"]) + @protect() + @safe + @rison(get_delete_ids_schema) + def bulk_delete(self, **kwargs) -> Response: # pylint: disable=arguments-differ + """Delete bulk Dashboards + --- + delete: + description: >- + Deletes multiple Dashboards in a bulk operation + 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/403' + 404: + $ref: '#/components/responses/404' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + item_ids = kwargs["rison"] + try: + BulkDeleteDashboardCommand(g.user, item_ids).run() + return self.response( + 200, + message=ngettext( + f"Deleted %(num)d dashboard", + f"Deleted %(num)d dashboards", + num=len(item_ids), + ), + ) + except DashboardNotFoundError: + return self.response_404() + except DashboardForbiddenError: + return self.response_403() + except DashboardBulkDeleteFailedError as e: + return self.response_422(message=str(e)) + + @expose("/export/", methods=["GET"]) + @protect() + @safe + @rison(get_export_ids_schema) + def export(self, **kwargs): + """Export dashboards + --- + get: + description: >- + Exports multiple Dashboards and downloads them as YAML files + parameters: + - in: query + name: q + content: + application/json: + schema: + type: array + items: + type: integer + responses: + 200: + description: Dashboard export + content: + text/plain: + schema: + type: string + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + query = self.datamodel.session.query(Dashboard).filter( + Dashboard.id.in_(kwargs["rison"]) + ) + query = self._base_filters.apply_all(query) + ids = [item.id for item in query.all()] + if not ids: + return self.response_404() + export = Dashboard.export_dashboards(ids) + resp = make_response(export, 200) + resp.headers["Content-Disposition"] = generate_download_headers("json")[ + "Content-Disposition" + ] + return resp diff --git a/superset/dashboards/commands/__init__.py b/superset/dashboards/commands/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/superset/dashboards/commands/__init__.py @@ -0,0 +1,16 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. diff --git a/superset/dashboards/commands/bulk_delete.py b/superset/dashboards/commands/bulk_delete.py new file mode 100644 index 0000000000000..07f2bef78742b --- /dev/null +++ b/superset/dashboards/commands/bulk_delete.py @@ -0,0 +1,61 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import logging +from typing import List, Optional + +from flask_appbuilder.security.sqla.models import User + +from superset.commands.base import BaseCommand +from superset.commands.exceptions import DeleteFailedError +from superset.dashboards.commands.exceptions import ( + DashboardBulkDeleteFailedError, + DashboardForbiddenError, + DashboardNotFoundError, +) +from superset.dashboards.dao import DashboardDAO +from superset.exceptions import SupersetSecurityException +from superset.models.dashboard import Dashboard +from superset.views.base import check_ownership + +logger = logging.getLogger(__name__) + + +class BulkDeleteDashboardCommand(BaseCommand): + def __init__(self, user: User, model_ids: List[int]): + self._actor = user + self._model_ids = model_ids + self._models: Optional[List[Dashboard]] = None + + def run(self): + self.validate() + try: + DashboardDAO.bulk_delete(self._models) + except DeleteFailedError as e: + logger.exception(e.exception) + raise DashboardBulkDeleteFailedError() + + def validate(self) -> None: + # Validate/populate model exists + self._models = DashboardDAO.find_by_ids(self._model_ids) + if not self._models or len(self._models) != len(self._model_ids): + raise DashboardNotFoundError() + # Check ownership + for model in self._models: + try: + check_ownership(model) + except SupersetSecurityException: + raise DashboardForbiddenError() diff --git a/superset/dashboards/commands/create.py b/superset/dashboards/commands/create.py new file mode 100644 index 0000000000000..da79677465e91 --- /dev/null +++ b/superset/dashboards/commands/create.py @@ -0,0 +1,67 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import logging +from typing import Dict, List, Optional + +from flask_appbuilder.security.sqla.models import User +from marshmallow import ValidationError + +from superset.commands.base import BaseCommand +from superset.commands.utils import populate_owners +from superset.dao.exceptions import DAOCreateFailedError +from superset.dashboards.commands.exceptions import ( + DashboardCreateFailedError, + DashboardInvalidError, + DashboardSlugExistsValidationError, +) +from superset.dashboards.dao import DashboardDAO + +logger = logging.getLogger(__name__) + + +class CreateDashboardCommand(BaseCommand): + def __init__(self, user: User, data: Dict): + self._actor = user + self._properties = data.copy() + + def run(self): + self.validate() + try: + dashboard = DashboardDAO.create(self._properties) + except DAOCreateFailedError as e: + logger.exception(e.exception) + raise DashboardCreateFailedError() + return dashboard + + def validate(self) -> None: + exceptions = list() + owner_ids: Optional[List[int]] = self._properties.get("owners") + slug: str = self._properties.get("slug", "") + + # Validate slug uniqueness + if not DashboardDAO.validate_slug_uniqueness(slug): + exceptions.append(DashboardSlugExistsValidationError()) + + try: + owners = populate_owners(self._actor, owner_ids) + self._properties["owners"] = owners + except ValidationError as e: + exceptions.append(e) + if exceptions: + exception = DashboardInvalidError() + exception.add_list(exceptions) + raise exception diff --git a/superset/dashboards/commands/delete.py b/superset/dashboards/commands/delete.py new file mode 100644 index 0000000000000..bc19b7cae7f78 --- /dev/null +++ b/superset/dashboards/commands/delete.py @@ -0,0 +1,61 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import logging +from typing import Optional + +from flask_appbuilder.security.sqla.models import User + +from superset.commands.base import BaseCommand +from superset.dao.exceptions import DAODeleteFailedError +from superset.dashboards.commands.exceptions import ( + DashboardDeleteFailedError, + DashboardForbiddenError, + DashboardNotFoundError, +) +from superset.dashboards.dao import DashboardDAO +from superset.exceptions import SupersetSecurityException +from superset.models.dashboard import Dashboard +from superset.views.base import check_ownership + +logger = logging.getLogger(__name__) + + +class DeleteDashboardCommand(BaseCommand): + def __init__(self, user: User, model_id: int): + self._actor = user + self._model_id = model_id + self._model: Optional[Dashboard] = None + + def run(self): + self.validate() + try: + dashboard = DashboardDAO.delete(self._model) + except DAODeleteFailedError as e: + logger.exception(e.exception) + raise DashboardDeleteFailedError() + return dashboard + + def validate(self) -> None: + # Validate/populate model exists + self._model = DashboardDAO.find_by_id(self._model_id) + if not self._model: + raise DashboardNotFoundError() + # Check ownership + try: + check_ownership(self._model) + except SupersetSecurityException: + raise DashboardForbiddenError() diff --git a/superset/dashboards/commands/exceptions.py b/superset/dashboards/commands/exceptions.py new file mode 100644 index 0000000000000..76d7237bb6a4a --- /dev/null +++ b/superset/dashboards/commands/exceptions.py @@ -0,0 +1,64 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from flask_babel import lazy_gettext as _ +from marshmallow.validate import ValidationError + +from superset.commands.exceptions import ( + CommandException, + CommandInvalidError, + CreateFailedError, + DeleteFailedError, + ForbiddenError, + UpdateFailedError, +) + + +class DashboardSlugExistsValidationError(ValidationError): + """ + Marshmallow validation error for dashboard slug already exists + """ + + def __init__(self): + super().__init__(_("Must be unique"), field_names=["slug"]) + + +class DashboardInvalidError(CommandInvalidError): + message = _("Dashboard parameters are invalid.") + + +class DashboardNotFoundError(CommandException): + message = _("Dashboard not found.") + + +class DashboardCreateFailedError(CreateFailedError): + message = _("Dashboard could not be created.") + + +class DashboardBulkDeleteFailedError(CreateFailedError): + message = _("Dashboards could not be deleted.") + + +class DashboardUpdateFailedError(UpdateFailedError): + message = _("Dashboard could not be updated.") + + +class DashboardDeleteFailedError(DeleteFailedError): + message = _("Dashboard could not be deleted.") + + +class DashboardForbiddenError(ForbiddenError): + message = _("Changing this Dashboard is forbidden") diff --git a/superset/dashboards/commands/update.py b/superset/dashboards/commands/update.py new file mode 100644 index 0000000000000..27cd13dd4a82a --- /dev/null +++ b/superset/dashboards/commands/update.py @@ -0,0 +1,87 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import logging +from typing import Dict, List, Optional + +from flask_appbuilder.security.sqla.models import User +from marshmallow import ValidationError + +from superset.commands.base import BaseCommand +from superset.commands.utils import populate_owners +from superset.connectors.sqla.models import SqlaTable +from superset.dao.exceptions import DAOUpdateFailedError +from superset.dashboards.commands.exceptions import ( + DashboardForbiddenError, + DashboardInvalidError, + DashboardNotFoundError, + DashboardSlugExistsValidationError, + DashboardUpdateFailedError, +) +from superset.dashboards.dao import DashboardDAO +from superset.exceptions import SupersetSecurityException +from superset.views.base import check_ownership + +logger = logging.getLogger(__name__) + + +class UpdateDashboardCommand(BaseCommand): + def __init__(self, user: User, model_id: int, data: Dict): + self._actor = user + self._model_id = model_id + self._properties = data.copy() + self._model: Optional[SqlaTable] = None + + def run(self): + self.validate() + try: + dashboard = DashboardDAO.update(self._model, self._properties) + except DAOUpdateFailedError as e: + logger.exception(e.exception) + raise DashboardUpdateFailedError() + return dashboard + + def validate(self) -> None: + exceptions: List[ValidationError] = [] + owner_ids: Optional[List[int]] = self._properties.get("owners") + slug: str = self._properties.get("slug", "") + + # Validate/populate model exists + self._model = DashboardDAO.find_by_id(self._model_id) + if not self._model: + raise DashboardNotFoundError() + # Check ownership + try: + check_ownership(self._model) + except SupersetSecurityException: + raise DashboardForbiddenError() + + # Validate slug uniqueness + if not DashboardDAO.validate_update_slug_uniqueness(self._model_id, slug): + exceptions.append(DashboardSlugExistsValidationError()) + + # Validate/Populate owner + if owner_ids is None: + owner_ids = [owner.id for owner in self._model.owners] + try: + owners = populate_owners(self._actor, owner_ids) + self._properties["owners"] = owners + except ValidationError as e: + exceptions.append(e) + if exceptions: + exception = DashboardInvalidError() + exception.add_list(exceptions) + raise exception diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py new file mode 100644 index 0000000000000..e635750bec696 --- /dev/null +++ b/superset/dashboards/dao.py @@ -0,0 +1,74 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import logging +from typing import List + +from flask_appbuilder.models.sqla.interface import SQLAInterface +from sqlalchemy.exc import SQLAlchemyError + +from superset.dao.base import BaseDAO +from superset.dashboards.filters import DashboardFilter +from superset.extensions import db +from superset.models.dashboard import Dashboard + +logger = logging.getLogger(__name__) + + +class DashboardDAO(BaseDAO): + model_cls = Dashboard + base_filter = DashboardFilter + + @staticmethod + def find_by_ids(model_ids: List[int]) -> List[Dashboard]: + query = db.session.query(Dashboard).filter(Dashboard.id.in_(model_ids)) + data_model = SQLAInterface(Dashboard, db.session) + query = DashboardFilter("id", data_model).apply(query, None) + return query.all() + + @staticmethod + def validate_slug_uniqueness(slug: str) -> bool: + if not slug: + return True + dashboard_query = db.session.query(Dashboard).filter(Dashboard.slug == slug) + return not db.session.query(dashboard_query.exists()).scalar() + + @staticmethod + def validate_update_slug_uniqueness(dashboard_id: int, slug: str) -> bool: + dashboard_query = db.session.query(Dashboard).filter( + Dashboard.slug == slug, Dashboard.id != dashboard_id + ) + return not db.session.query(dashboard_query.exists()).scalar() + + @staticmethod + def bulk_delete(models: List[Dashboard], commit=True): + item_ids = [model.id for model in models] + # bulk delete, first delete related data + for model in models: + model.slices = [] + model.owners = [] + db.session.merge(model) + # bulk delete itself + try: + db.session.query(Dashboard).filter(Dashboard.id.in_(item_ids)).delete( + synchronize_session="fetch" + ) + if commit: + db.session.commit() + except SQLAlchemyError as e: + if commit: + db.session.rollback() + raise e diff --git a/superset/dashboards/filters.py b/superset/dashboards/filters.py new file mode 100644 index 0000000000000..0b4338dfdbad5 --- /dev/null +++ b/superset/dashboards/filters.py @@ -0,0 +1,84 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from sqlalchemy import and_, or_ + +from superset import db, security_manager +from superset.models.core import FavStar +from superset.models.dashboard import Dashboard +from superset.models.slice import Slice +from superset.views.base import BaseFilter, get_user_roles + + +class DashboardFilter(BaseFilter): # pylint: disable=too-few-public-methods + """ + List dashboards with the following criteria: + 1. Those which the user owns + 2. Those which the user has favorited + 3. Those which have been published (if they have access to at least one slice) + + If the user is an admin show them all dashboards. + This means they do not get curation but can still sort by "published" + if they wish to see those dashboards which are published first + """ + + def apply(self, query, value): + user_roles = [role.name.lower() for role in list(get_user_roles())] + if "admin" in user_roles: + return query + + datasource_perms = security_manager.user_view_menu_names("datasource_access") + schema_perms = security_manager.user_view_menu_names("schema_access") + all_datasource_access = security_manager.all_datasource_access() + published_dash_query = ( + db.session.query(Dashboard.id) + .join(Dashboard.slices) + .filter( + and_( + Dashboard.published == True, # pylint: disable=singleton-comparison + or_( + Slice.perm.in_(datasource_perms), + Slice.schema_perm.in_(schema_perms), + all_datasource_access, + ), + ) + ) + ) + + users_favorite_dash_query = db.session.query(FavStar.obj_id).filter( + and_( + FavStar.user_id == security_manager.user_model.get_user_id(), + FavStar.class_name == "Dashboard", + ) + ) + owner_ids_query = ( + db.session.query(Dashboard.id) + .join(Dashboard.owners) + .filter( + security_manager.user_model.id + == security_manager.user_model.get_user_id() + ) + ) + + query = query.filter( + or_( + Dashboard.id.in_(owner_ids_query), + Dashboard.id.in_(published_dash_query), + Dashboard.id.in_(users_favorite_dash_query), + ) + ) + + return query diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py new file mode 100644 index 0000000000000..a6b7cd4e27f55 --- /dev/null +++ b/superset/dashboards/schemas.py @@ -0,0 +1,87 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import json +import re + +from marshmallow import fields, pre_load, Schema +from marshmallow.validate import Length, ValidationError + +from superset.exceptions import SupersetException +from superset.utils import core as utils + +get_delete_ids_schema = {"type": "array", "items": {"type": "integer"}} +get_export_ids_schema = {"type": "array", "items": {"type": "integer"}} + + +def validate_json(value): + try: + utils.validate_json(value) + except SupersetException: + raise ValidationError("JSON not valid") + + +def validate_json_metadata(value): + if not value: + return + try: + value_obj = json.loads(value) + except json.decoder.JSONDecodeError: + raise ValidationError("JSON not valid") + errors = DashboardJSONMetadataSchema(strict=True).validate(value_obj, partial=False) + if errors: + raise ValidationError(errors) + + +class DashboardJSONMetadataSchema(Schema): + timed_refresh_immune_slices = fields.List(fields.Integer()) + filter_scopes = fields.Dict() + expanded_slices = fields.Dict() + refresh_frequency = fields.Integer() + default_filters = fields.Str() + stagger_refresh = fields.Boolean() + stagger_time = fields.Integer() + color_scheme = fields.Str() + label_colors = fields.Dict() + + +class BaseDashboardSchema(Schema): + @pre_load + def pre_load(self, data): # pylint: disable=no-self-use + if data.get("slug"): + data["slug"] = data["slug"].strip() + data["slug"] = data["slug"].replace(" ", "-") + data["slug"] = re.sub(r"[^\w\-]+", "", data["slug"]) + + +class DashboardPostSchema(BaseDashboardSchema): + dashboard_title = fields.String(allow_none=True, validate=Length(0, 500)) + slug = fields.String(allow_none=True, validate=[Length(1, 255)]) + owners = fields.List(fields.Integer()) + position_json = fields.String(validate=validate_json) + css = fields.String() + json_metadata = fields.String(validate=validate_json_metadata) + published = fields.Boolean() + + +class DashboardPutSchema(BaseDashboardSchema): + dashboard_title = fields.String(allow_none=True, validate=Length(0, 500)) + slug = fields.String(allow_none=True, validate=Length(0, 255)) + owners = fields.List(fields.Integer(allow_none=True)) + position_json = fields.String(allow_none=True, validate=validate_json) + css = fields.String(allow_none=True) + json_metadata = fields.String(allow_none=True, validate=validate_json_metadata) + published = fields.Boolean(allow_none=True) diff --git a/superset/datasets/commands/create.py b/superset/datasets/commands/create.py index 344770b951d07..466e35dd7f813 100644 --- a/superset/datasets/commands/create.py +++ b/superset/datasets/commands/create.py @@ -21,8 +21,8 @@ from marshmallow import ValidationError from superset.commands.base import BaseCommand -from superset.commands.exceptions import CreateFailedError -from superset.datasets.commands.base import populate_owners +from superset.commands.utils import populate_owners +from superset.dao.exceptions import DAOCreateFailedError from superset.datasets.commands.exceptions import ( DatabaseNotFoundValidationError, DatasetCreateFailedError, @@ -44,7 +44,7 @@ def run(self): self.validate() try: dataset = DatasetDAO.create(self._properties) - except CreateFailedError as e: + except DAOCreateFailedError as e: logger.exception(e.exception) raise DatasetCreateFailedError() return dataset diff --git a/superset/datasets/commands/delete.py b/superset/datasets/commands/delete.py index d61c56a0e06d6..85837f96f7fbe 100644 --- a/superset/datasets/commands/delete.py +++ b/superset/datasets/commands/delete.py @@ -20,8 +20,8 @@ from flask_appbuilder.security.sqla.models import User from superset.commands.base import BaseCommand -from superset.commands.exceptions import DeleteFailedError from superset.connectors.sqla.models import SqlaTable +from superset.dao.exceptions import DAODeleteFailedError from superset.datasets.commands.exceptions import ( DatasetDeleteFailedError, DatasetForbiddenError, @@ -44,7 +44,7 @@ def run(self): self.validate() try: dataset = DatasetDAO.delete(self._model) - except DeleteFailedError as e: + except DAODeleteFailedError as e: logger.exception(e.exception) raise DatasetDeleteFailedError() return dataset diff --git a/superset/datasets/commands/update.py b/superset/datasets/commands/update.py index b3deeab2ebd9a..05a0b96664bd9 100644 --- a/superset/datasets/commands/update.py +++ b/superset/datasets/commands/update.py @@ -21,9 +21,9 @@ from marshmallow import ValidationError from superset.commands.base import BaseCommand -from superset.commands.exceptions import UpdateFailedError +from superset.commands.utils import populate_owners from superset.connectors.sqla.models import SqlaTable -from superset.datasets.commands.base import populate_owners +from superset.dao.exceptions import DAOUpdateFailedError from superset.datasets.commands.exceptions import ( DatabaseChangeValidationError, DatasetExistsValidationError, @@ -50,7 +50,7 @@ def run(self): self.validate() try: dataset = DatasetDAO.update(self._model, self._properties) - except UpdateFailedError as e: + except DAOUpdateFailedError as e: logger.exception(e.exception) raise DatasetUpdateFailedError() return dataset diff --git a/superset/exceptions.py b/superset/exceptions.py index c13c7004b1ab4..605627c0efbaf 100644 --- a/superset/exceptions.py +++ b/superset/exceptions.py @@ -14,10 +14,22 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from typing import Optional class SupersetException(Exception): status = 500 + message = "" + + def __init__(self, message: str = "", exception: Optional[Exception] = None): + if message: + self.message = message + self._exception = exception + super().__init__(self.message) + + @property + def exception(self): + return self._exception class SupersetTimeoutException(SupersetException): diff --git a/superset/views/api.py b/superset/views/api.py index ba0281eeda847..e82aa86dbc91a 100644 --- a/superset/views/api.py +++ b/superset/views/api.py @@ -26,11 +26,6 @@ from superset.models.slice import Slice from superset.utils import core as utils from superset.views.base import api, BaseSupersetView, handle_api_exception -from superset.views.chart import api as chart_api # pylint: disable=unused-import -from superset.views.dashboard import ( # pylint: disable=unused-import - api as dashboard_api, -) -from superset.views.database import api as database_api # pylint: disable=unused-import class Api(BaseSupersetView): diff --git a/superset/views/dashboard/api.py b/superset/views/dashboard/api.py deleted file mode 100644 index f547cbf873809..0000000000000 --- a/superset/views/dashboard/api.py +++ /dev/null @@ -1,319 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -import json -import logging -import re -from typing import Dict, List, Optional - -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.constants import RouteMethod -from superset.exceptions import SupersetException, SupersetSecurityException -from superset.models.dashboard import Dashboard -from superset.utils import core as utils -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()) - filter_scopes = fields.Dict() - expanded_slices = fields.Dict() - refresh_frequency = fields.Integer() - default_filters = fields.Str() - stagger_refresh = fields.Boolean() - stagger_time = fields.Integer() - color_scheme = fields.Str() - label_colors = fields.Dict() - - -def validate_json(value): - try: - utils.validate_json(value) - except SupersetException: - raise ValidationError("JSON not valid") - - -def validate_json_metadata(value): - if not value: - return - try: - value_obj = json.loads(value) - except json.decoder.JSONDecodeError: - raise ValidationError("JSON not valid") - errors = DashboardJSONMetadataSchema(strict=True).validate(value_obj, partial=False) - if errors: - raise ValidationError(errors) - - -def validate_slug_uniqueness(value): - # slug is not required but must be unique - if value: - item = ( - current_app.appbuilder.get_session.query(Dashboard.id) - .filter_by(slug=value) - .one_or_none() - ) - if item: - raise ValidationError("Must be unique") - - -class BaseDashboardSchema(BaseOwnedSchema): - @pre_load - def pre_load(self, data): # pylint: disable=no-self-use - super().pre_load(data) - if data.get("slug"): - data["slug"] = data["slug"].strip() - data["slug"] = data["slug"].replace(" ", "-") - data["slug"] = re.sub(r"[^\w\-]+", "", data["slug"]) - if "owners" in data and data["owners"] is None: - data["owners"] = [] - - -class DashboardPostSchema(BaseDashboardSchema): - __class_model__ = Dashboard - - dashboard_title = fields.String(allow_none=True, validate=Length(0, 500)) - slug = fields.String( - allow_none=True, validate=[Length(1, 255), validate_slug_uniqueness] - ) - owners = fields.List(fields.Integer(validate=validate_owner)) - position_json = fields.String(validate=validate_json) - css = fields.String() - json_metadata = fields.String(validate=validate_json_metadata) - published = fields.Boolean() - - -class DashboardPutSchema(BaseDashboardSchema): - dashboard_title = fields.String(allow_none=True, validate=Length(0, 500)) - slug = fields.String(allow_none=True, validate=Length(0, 255)) - owners = fields.List(fields.Integer(validate=validate_owner)) - position_json = fields.String(validate=validate_json) - css = fields.String() - json_metadata = fields.String(allow_none=True, validate=validate_json_metadata) - published = fields.Boolean() - - @post_load - def make_object(self, data: Dict, discard: Optional[List[str]] = None) -> Dashboard: - self.instance = super().make_object(data, []) - for slc in self.instance.slices: - slc.owners = list(set(self.instance.owners) | set(slc.owners)) - return self.instance - - -get_export_ids_schema = {"type": "array", "items": {"type": "integer"}} - - -class DashboardRestApi(DashboardMixin, BaseOwnedModelRestApi): - datamodel = SQLAInterface(Dashboard) - include_route_methods = RouteMethod.REST_MODEL_VIEW_CRUD_SET | { - RouteMethod.EXPORT, - RouteMethod.RELATED, - "bulk_delete", # not using RouteMethod since locally defined - } - resource_name = "dashboard" - allow_browser_login = True - - class_permission_name = "DashboardModelView" - show_columns = [ - "id", - "charts", - "css", - "dashboard_title", - "json_metadata", - "owners.id", - "owners.username", - "changed_by_name", - "changed_by_url", - "changed_by.username", - "changed_on", - "position_json", - "published", - "url", - "slug", - "table_names", - ] - order_columns = ["dashboard_title", "changed_on", "published", "changed_by_fk"] - list_columns = [ - "changed_by_name", - "changed_by_url", - "changed_by.username", - "changed_on", - "dashboard_title", - "id", - "published", - "slug", - "url", - ] - - add_model_schema = DashboardPostSchema() - edit_model_schema = DashboardPutSchema() - - order_rel_fields = { - "slices": ("slice_name", "asc"), - "owners": ("first_name", "asc"), - } - filter_rel_fields_field = {"owners": "first_name"} - allowed_rel_fields = {"owners"} - - @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 - @rison(get_export_ids_schema) - def export(self, **kwargs): - """Export dashboards - --- - get: - parameters: - - in: query - name: q - content: - application/json: - schema: - type: array - items: - type: integer - responses: - 200: - description: Dashboard export - content: - text/plain: - schema: - type: string - 400: - $ref: '#/components/responses/400' - 401: - $ref: '#/components/responses/401' - 404: - $ref: '#/components/responses/404' - 422: - $ref: '#/components/responses/422' - 500: - $ref: '#/components/responses/500' - """ - query = self.datamodel.session.query(Dashboard).filter( - Dashboard.id.in_(kwargs["rison"]) - ) - query = self._base_filters.apply_all(query) - ids = [item.id for item in query.all()] - if not ids: - return self.response_404() - export = Dashboard.export_dashboards(ids) - resp = make_response(export, 200) - resp.headers["Content-Disposition"] = generate_download_headers("json")[ - "Content-Disposition" - ] - return resp diff --git a/tests/dashboards/__init__.py b/tests/dashboards/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/dashboards/__init__.py @@ -0,0 +1,16 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. diff --git a/tests/dashboard_api_tests.py b/tests/dashboards/api_tests.py similarity index 70% rename from tests/dashboard_api_tests.py rename to tests/dashboards/api_tests.py index 83881f4fef6d2..1c8a050f2a77e 100644 --- a/tests/dashboard_api_tests.py +++ b/tests/dashboards/api_tests.py @@ -20,20 +20,30 @@ from typing import List, Optional import prison +from sqlalchemy.sql import func import tests.test_app from superset import db, security_manager -from superset.models import core as models +from superset.models.dashboard import Dashboard from superset.models.slice import Slice from superset.views.base import generate_download_headers -from .base_api_tests import ApiOwnersTestCaseMixin -from .base_tests import SupersetTestCase +from tests.base_api_tests import ApiOwnersTestCaseMixin +from tests.base_tests import SupersetTestCase class DashboardApiTests(SupersetTestCase, ApiOwnersTestCaseMixin): resource_name = "dashboard" + dashboard_data = { + "dashboard_title": "title1_changed", + "slug": "slug1_changed", + "position_json": '{"b": "B"}', + "css": "css_changed", + "json_metadata": '{"a": "A"}', + "published": False, + } + def __init__(self, *args, **kwargs): super(DashboardApiTests, self).__init__(*args, **kwargs) @@ -47,13 +57,13 @@ def insert_dashboard( css: str = "", json_metadata: str = "", published: bool = False, - ) -> models.Dashboard: + ) -> Dashboard: obj_owners = list() slices = slices or [] for owner in owners: user = db.session.query(security_manager.user_model).get(owner) obj_owners.append(user) - dashboard = models.Dashboard( + dashboard = Dashboard( dashboard_title=dashboard_title, slug=slug, owners=obj_owners, @@ -67,6 +77,122 @@ def insert_dashboard( db.session.commit() return dashboard + def test_get_dashboard(self): + """ + Dashboard API: Test get dashboard + """ + admin = self.get_user("admin") + dashboard = self.insert_dashboard("title", "slug1", [admin.id]) + self.login(username="admin") + uri = f"api/v1/dashboard/{dashboard.id}" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 200) + expected_result = { + "changed_by": None, + "changed_by_name": "", + "changed_by_url": "", + "charts": [], + "id": dashboard.id, + "css": "", + "dashboard_title": "title", + "json_metadata": "", + "owners": [{"id": 1, "username": "admin"}], + "position_json": "", + "published": False, + "url": f"/superset/dashboard/slug1/", + "slug": "slug1", + "table_names": "", + } + data = json.loads(rv.data.decode("utf-8")) + self.assertIn("changed_on", data["result"]) + for key, value in data["result"].items(): + # We can't assert timestamp + if key != "changed_on": + self.assertEqual(value, expected_result[key]) + # rollback changes + db.session.delete(dashboard) + db.session.commit() + + def test_get_dashboard_not_found(self): + """ + Dashboard API: Test get dashboard not found + """ + max_id = db.session.query(func.max(Dashboard.id)).scalar() + self.login(username="admin") + uri = f"api/v1/dashboard/{max_id + 1}" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + + def test_get_dashboard_no_data_access(self): + """ + Dashboard API: Test get dashboard without data access + """ + admin = self.get_user("admin") + dashboard = self.insert_dashboard("title", "slug1", [admin.id]) + + self.login(username="gamma") + uri = f"api/v1/dashboard/{dashboard.id}" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + # rollback changes + db.session.delete(dashboard) + db.session.commit() + + def test_get_dashboards_filter(self): + """ + Dashboard API: Test get dashboards filter + """ + admin = self.get_user("admin") + gamma = self.get_user("gamma") + dashboard = self.insert_dashboard("title", "slug1", [admin.id, gamma.id]) + + self.login(username="admin") + + arguments = { + "filters": [{"col": "dashboard_title", "opr": "sw", "value": "ti"}] + } + uri = f"api/v1/dashboard/?q={prison.dumps(arguments)}" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 200) + data = json.loads(rv.data.decode("utf-8")) + self.assertEqual(data["count"], 1) + + arguments = { + "filters": [ + {"col": "owners", "opr": "rel_m_m", "value": [admin.id, gamma.id]} + ] + } + uri = f"api/v1/dashboard/?q={prison.dumps(arguments)}" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 200) + data = json.loads(rv.data.decode("utf-8")) + self.assertEqual(data["count"], 1) + + # rollback changes + db.session.delete(dashboard) + db.session.commit() + + def test_get_dashboards_no_data_access(self): + """ + Dashboard API: Test get dashboards no data access + """ + admin = self.get_user("admin") + dashboard = self.insert_dashboard("title", "slug1", [admin.id]) + + self.login(username="gamma") + arguments = { + "filters": [{"col": "dashboard_title", "opr": "sw", "value": "ti"}] + } + uri = f"api/v1/dashboard/?q={prison.dumps(arguments)}" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 200) + data = json.loads(rv.data.decode("utf-8")) + self.assertEqual(data["count"], 0) + + # rollback changes + db.session.delete(dashboard) + db.session.commit() + def test_delete_dashboard(self): """ Dashboard API: Test delete @@ -77,7 +203,7 @@ def test_delete_dashboard(self): uri = f"api/v1/dashboard/{dashboard_id}" rv = self.client.delete(uri) self.assertEqual(rv.status_code, 200) - model = db.session.query(models.Dashboard).get(dashboard_id) + model = db.session.query(Dashboard).get(dashboard_id) self.assertEqual(model, None) def test_delete_bulk_dashboards(self): @@ -104,7 +230,7 @@ def test_delete_bulk_dashboards(self): 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) + model = db.session.query(Dashboard).get(dashboard_id) self.assertEqual(model, None) def test_delete_bulk_dashboards_bad_request(self): @@ -150,7 +276,7 @@ def test_delete_dashboard_admin_not_owned(self): uri = f"api/v1/dashboard/{dashboard_id}" rv = self.client.delete(uri) self.assertEqual(rv.status_code, 200) - model = db.session.query(models.Dashboard).get(dashboard_id) + model = db.session.query(Dashboard).get(dashboard_id) self.assertEqual(model, None) def test_delete_bulk_dashboard_admin_not_owned(self): @@ -179,7 +305,7 @@ def test_delete_bulk_dashboard_admin_not_owned(self): self.assertEqual(response, expected_response) for dashboard_id in dashboard_ids: - model = db.session.query(models.Dashboard).get(dashboard_id) + model = db.session.query(Dashboard).get(dashboard_id) self.assertEqual(model, None) def test_delete_dashboard_not_owned(self): @@ -250,7 +376,7 @@ def test_delete_bulk_dashboard_not_owned(self): 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"} + expected_response = {"message": "Forbidden"} self.assertEqual(response, expected_response) # nothing is delete in bulk with a list of owned and not owned dashboards @@ -259,7 +385,7 @@ def test_delete_bulk_dashboard_not_owned(self): 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"} + expected_response = {"message": "Forbidden"} self.assertEqual(response, expected_response) for dashboard in dashboards: @@ -288,7 +414,7 @@ def test_create_dashboard(self): rv = self.client.post(uri, json=dashboard_data) self.assertEqual(rv.status_code, 201) data = json.loads(rv.data.decode("utf-8")) - model = db.session.query(models.Dashboard).get(data.get("id")) + model = db.session.query(Dashboard).get(data.get("id")) db.session.delete(model) db.session.commit() @@ -302,7 +428,7 @@ def test_create_simple_dashboard(self): rv = self.client.post(uri, json=dashboard_data) self.assertEqual(rv.status_code, 201) data = json.loads(rv.data.decode("utf-8")) - model = db.session.query(models.Dashboard).get(data.get("id")) + model = db.session.query(Dashboard).get(data.get("id")) db.session.delete(model) db.session.commit() @@ -316,7 +442,7 @@ def test_create_dashboard_empty(self): rv = self.client.post(uri, json=dashboard_data) self.assertEqual(rv.status_code, 201) data = json.loads(rv.data.decode("utf-8")) - model = db.session.query(models.Dashboard).get(data.get("id")) + model = db.session.query(Dashboard).get(data.get("id")) db.session.delete(model) db.session.commit() @@ -326,7 +452,7 @@ def test_create_dashboard_empty(self): rv = self.client.post(uri, json=dashboard_data) self.assertEqual(rv.status_code, 201) data = json.loads(rv.data.decode("utf-8")) - model = db.session.query(models.Dashboard).get(data.get("id")) + model = db.session.query(Dashboard).get(data.get("id")) db.session.delete(model) db.session.commit() @@ -338,7 +464,7 @@ def test_create_dashboard_validate_title(self): self.login(username="admin") uri = "api/v1/dashboard/" rv = self.client.post(uri, json=dashboard_data) - self.assertEqual(rv.status_code, 422) + self.assertEqual(rv.status_code, 400) response = json.loads(rv.data.decode("utf-8")) expected_response = { "message": {"dashboard_title": ["Length must be between 0 and 500."]} @@ -366,7 +492,7 @@ def test_create_dashboard_validate_slug(self): dashboard_data = {"dashboard_title": "title2", "slug": "a" * 256} uri = "api/v1/dashboard/" rv = self.client.post(uri, json=dashboard_data) - self.assertEqual(rv.status_code, 422) + self.assertEqual(rv.status_code, 400) response = json.loads(rv.data.decode("utf-8")) expected_response = {"message": {"slug": ["Length must be between 1 and 255."]}} self.assertEqual(response, expected_response) @@ -384,7 +510,7 @@ def test_create_dashboard_validate_owners(self): rv = self.client.post(uri, json=dashboard_data) self.assertEqual(rv.status_code, 422) response = json.loads(rv.data.decode("utf-8")) - expected_response = {"message": {"owners": {"0": ["User 1000 does not exist"]}}} + expected_response = {"message": {"owners": ["Owners are invalid"]}} self.assertEqual(response, expected_response) def test_create_dashboard_validate_json(self): @@ -395,13 +521,13 @@ def test_create_dashboard_validate_json(self): self.login(username="admin") uri = "api/v1/dashboard/" rv = self.client.post(uri, json=dashboard_data) - self.assertEqual(rv.status_code, 422) + self.assertEqual(rv.status_code, 400) dashboard_data = {"dashboard_title": "title1", "json_metadata": '{"A:"a"}'} self.login(username="admin") uri = "api/v1/dashboard/" rv = self.client.post(uri, json=dashboard_data) - self.assertEqual(rv.status_code, 422) + self.assertEqual(rv.status_code, 400) dashboard_data = { "dashboard_title": "title1", @@ -410,34 +536,56 @@ def test_create_dashboard_validate_json(self): self.login(username="admin") uri = "api/v1/dashboard/" rv = self.client.post(uri, json=dashboard_data) - self.assertEqual(rv.status_code, 422) + self.assertEqual(rv.status_code, 400) def test_update_dashboard(self): """ Dashboard API: Test update """ + admin = self.get_user("admin") + dashboard_id = self.insert_dashboard("title1", "slug1", [admin.id]).id + self.login(username="admin") + uri = f"api/v1/dashboard/{dashboard_id}" + rv = self.client.put(uri, json=self.dashboard_data) + self.assertEqual(rv.status_code, 200) + model = db.session.query(Dashboard).get(dashboard_id) + self.assertEqual(model.dashboard_title, self.dashboard_data["dashboard_title"]) + self.assertEqual(model.slug, self.dashboard_data["slug"]) + self.assertEqual(model.position_json, self.dashboard_data["position_json"]) + self.assertEqual(model.css, self.dashboard_data["css"]) + self.assertEqual(model.json_metadata, self.dashboard_data["json_metadata"]) + self.assertEqual(model.published, self.dashboard_data["published"]) + self.assertEqual(model.owners, [admin]) + + db.session.delete(model) + db.session.commit() + + def test_update_partial_dashboard(self): + """ + Dashboard API: Test update partial + """ admin_id = self.get_user("admin").id dashboard_id = self.insert_dashboard("title1", "slug1", [admin_id]).id - dashboard_data = { - "dashboard_title": "title1_changed", - "slug": "slug1_changed", - "owners": [admin_id], - "position_json": '{"b": "B"}', - "css": "css_changed", - "json_metadata": '{"a": "A"}', - "published": False, - } self.login(username="admin") uri = f"api/v1/dashboard/{dashboard_id}" - rv = self.client.put(uri, json=dashboard_data) + rv = self.client.put( + uri, json={"json_metadata": self.dashboard_data["json_metadata"]} + ) self.assertEqual(rv.status_code, 200) - model = db.session.query(models.Dashboard).get(dashboard_id) - self.assertEqual(model.dashboard_title, "title1_changed") - self.assertEqual(model.slug, "slug1_changed") - self.assertEqual(model.position_json, '{"b": "B"}') - self.assertEqual(model.css, "css_changed") - self.assertEqual(model.json_metadata, '{"a": "A"}') - self.assertEqual(model.published, False) + + rv = self.client.put( + uri, json={"dashboard_title": self.dashboard_data["dashboard_title"]} + ) + self.assertEqual(rv.status_code, 200) + + rv = self.client.put(uri, json={"slug": self.dashboard_data["slug"]}) + self.assertEqual(rv.status_code, 200) + + model = db.session.query(Dashboard).get(dashboard_id) + self.assertEqual(model.json_metadata, self.dashboard_data["json_metadata"]) + self.assertEqual(model.dashboard_title, self.dashboard_data["dashboard_title"]) + self.assertEqual(model.slug, self.dashboard_data["slug"]) + db.session.delete(model) db.session.commit() @@ -453,7 +601,7 @@ def test_update_dashboard_new_owner(self): uri = f"api/v1/dashboard/{dashboard_id}" rv = self.client.put(uri, json=dashboard_data) self.assertEqual(rv.status_code, 200) - model = db.session.query(models.Dashboard).get(dashboard_id) + model = db.session.query(Dashboard).get(dashboard_id) self.assertIn(admin, model.owners) for slc in model.slices: self.assertIn(admin, slc.owners) @@ -471,12 +619,34 @@ def test_update_dashboard_slug_formatting(self): uri = f"api/v1/dashboard/{dashboard_id}" rv = self.client.put(uri, json=dashboard_data) self.assertEqual(rv.status_code, 200) - model = db.session.query(models.Dashboard).get(dashboard_id) + model = db.session.query(Dashboard).get(dashboard_id) self.assertEqual(model.dashboard_title, "title1_changed") self.assertEqual(model.slug, "slug1-changed") db.session.delete(model) db.session.commit() + def test_update_dashboard_validate_slug(self): + """ + Dashboard API: Test update validate slug + """ + admin_id = self.get_user("admin").id + dashboard1 = self.insert_dashboard("title1", "slug-1", [admin_id]) + dashboard2 = self.insert_dashboard("title2", "slug-2", [admin_id]) + + self.login(username="admin") + # Check for slug uniqueness + dashboard_data = {"dashboard_title": "title2", "slug": "slug 1"} + uri = f"api/v1/dashboard/{dashboard2.id}" + rv = self.client.put(uri, json=dashboard_data) + self.assertEqual(rv.status_code, 422) + response = json.loads(rv.data.decode("utf-8")) + expected_response = {"message": {"slug": ["Must be unique"]}} + self.assertEqual(response, expected_response) + + db.session.delete(dashboard1) + db.session.delete(dashboard2) + db.session.commit() + def test_update_published(self): """ Dashboard API: Test update published patch @@ -491,7 +661,7 @@ def test_update_published(self): rv = self.client.put(uri, json=dashboard_data) self.assertEqual(rv.status_code, 200) - model = db.session.query(models.Dashboard).get(dashboard.id) + model = db.session.query(Dashboard).get(dashboard.id) self.assertEqual(model.published, True) self.assertEqual(model.slug, "slug1") self.assertIn(admin, model.owners) @@ -552,7 +722,7 @@ def test_export_not_found(self): def test_export_not_allowed(self): """ - Dashboard API: Test dashboard export not not allowed + Dashboard API: Test dashboard export not allowed """ admin_id = self.get_user("admin").id dashboard = self.insert_dashboard("title", "slug1", [admin_id], published=False) diff --git a/tests/dataset_api_tests.py b/tests/dataset_api_tests.py index 847419162c2e1..a55140a5dae7b 100644 --- a/tests/dataset_api_tests.py +++ b/tests/dataset_api_tests.py @@ -22,12 +22,12 @@ import prison from superset import db, security_manager -from superset.commands.exceptions import ( - CreateFailedError, - DeleteFailedError, - UpdateFailedError, -) from superset.connectors.sqla.models import SqlaTable +from superset.dao.exceptions import ( + DAOCreateFailedError, + DAODeleteFailedError, + DAOUpdateFailedError, +) from superset.models.core import Database from superset.utils.core import get_example_database @@ -279,7 +279,7 @@ def test_create_dataset_sqlalchemy_error(self, mock_dao_create): """ Dataset API: Test create dataset sqlalchemy error """ - mock_dao_create.side_effect = CreateFailedError() + mock_dao_create.side_effect = DAOCreateFailedError() self.login(username="admin") example_db = get_example_database() dataset_data = { @@ -379,7 +379,7 @@ def test_update_dataset_sqlalchemy_error(self, mock_dao_update): """ Dataset API: Test update dataset sqlalchemy error """ - mock_dao_update.side_effect = UpdateFailedError() + mock_dao_update.side_effect = DAOUpdateFailedError() table = self.insert_dataset("ab_permission", "", [], get_example_database()) self.login(username="admin") @@ -438,7 +438,7 @@ def test_delete_dataset_sqlalchemy_error(self, mock_dao_delete): """ Dataset API: Test delete dataset sqlalchemy error """ - mock_dao_delete.side_effect = DeleteFailedError() + mock_dao_delete.side_effect = DAODeleteFailedError() admin = self.get_user("admin") table = self.insert_dataset(