diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py index de8168c3390..6105d1b3ed4 100644 --- a/superset/charts/schemas.py +++ b/superset/charts/schemas.py @@ -91,6 +91,13 @@ datasource_type_description = ( ) datasource_name_description = "The datasource name." dashboards_description = "A list of dashboards to include this new chart to." +changed_on_description = "The ISO date that the chart was last changed." +slice_url_description = "The URL of the chart." +form_data_description = ( + "Form data from the Explore controls used to form the chart's data query." +) +description_markeddown_description = "Sanitized HTML version of the chart description." +owners_name_description = "Name of an owner of the chart." # # OpenAPI method specification overrides @@ -138,6 +145,24 @@ TIME_GRAINS = ( ) +class ChartEntityResponseSchema(Schema): + """ + Schema for a chart object + """ + + slice_id = fields.Integer() + slice_name = fields.String(description=slice_name_description) + cache_timeout = fields.Integer(description=cache_timeout_description) + changed_on = fields.String(description=changed_on_description) + datasource = fields.String(description=datasource_name_description) + description = fields.String(description=description_description) + description_markeddown = fields.String( + description=description_markeddown_description + ) + form_data = fields.Dict(description=form_data_description) + slice_url = fields.String(description=slice_url_description) + + class ChartPostSchema(Schema): """ Schema to add a new chart. @@ -1175,6 +1200,7 @@ CHART_SCHEMAS = ( ChartDataGeohashDecodeOptionsSchema, ChartDataGeohashEncodeOptionsSchema, ChartDataGeodeticParseOptionsSchema, + ChartEntityResponseSchema, ChartGetDatasourceResponseSchema, ChartCacheScreenshotResponseSchema, GetFavStarIdsSchema, diff --git a/superset/constants.py b/superset/constants.py index 858f813cd56..b5f905e90d6 100644 --- a/superset/constants.py +++ b/superset/constants.py @@ -114,4 +114,5 @@ MODEL_API_RW_METHOD_PERMISSION_MAP = { "screenshot": "read", "data": "read", "data_from_cache": "read", + "get_charts": "read", } diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 9d148585fe9..76787117fd6 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -30,6 +30,7 @@ from werkzeug.wrappers import Response as WerkzeugResponse from werkzeug.wsgi import FileWrapper from superset import is_feature_enabled, thumbnail_cache +from superset.charts.schemas import ChartEntityResponseSchema from superset.commands.exceptions import CommandInvalidError from superset.commands.importers.v1.utils import get_contents_from_bundle from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod @@ -90,6 +91,7 @@ class DashboardRestApi(BaseSupersetModelRestApi): RouteMethod.RELATED, "bulk_delete", # not using RouteMethod since locally defined "favorite_status", + "get_charts", } resource_name = "dashboard" allow_browser_login = True @@ -187,6 +189,7 @@ class DashboardRestApi(BaseSupersetModelRestApi): add_model_schema = DashboardPostSchema() edit_model_schema = DashboardPutSchema() + chart_entity_response_schema = ChartEntityResponseSchema() base_filters = [["slice", DashboardFilter, lambda: []]] @@ -204,7 +207,7 @@ class DashboardRestApi(BaseSupersetModelRestApi): openapi_spec_tag = "Dashboards" """ Override the name set for this collection of endpoints """ - openapi_spec_component_schemas = (GetFavStarIdsSchema,) + openapi_spec_component_schemas = (ChartEntityResponseSchema, GetFavStarIdsSchema) apispec_parameter_schemas = { "get_delete_ids_schema": get_delete_ids_schema, "get_export_ids_schema": get_export_ids_schema, @@ -219,6 +222,53 @@ class DashboardRestApi(BaseSupersetModelRestApi): self.include_route_methods = self.include_route_methods | {"thumbnail"} super().__init__() + @expose("//charts", methods=["GET"]) + @protect() + @safe + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get_charts", + log_to_statsd=False, + ) + def get_charts(self, pk: int) -> Response: + """Gets the chart definitions for a given dashboard + --- + get: + description: >- + Get the chart definitions for a given dashboard + parameters: + - in: path + schema: + type: integer + name: pk + responses: + 200: + description: Dashboard chart definitions + content: + application/json: + schema: + type: object + properties: + result: + type: array + items: + $ref: '#/components/schemas/ChartEntityResponseSchema' + 302: + description: Redirects to the current digest + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + """ + try: + charts = DashboardDAO.get_charts_for_dashboard(pk) + result = [self.chart_entity_response_schema.dump(chart) for chart in charts] + return self.response(200, result=result) + except DashboardNotFoundError: + return self.response_404() + @expose("/", methods=["POST"]) @protect() @safe diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index 65bdc690b11..7a6d1b79b6e 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -19,8 +19,10 @@ import logging from typing import Any, Dict, List, Optional from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.orm import contains_eager from superset.dao.base import BaseDAO +from superset.dashboards.commands.exceptions import DashboardNotFoundError from superset.dashboards.filters import DashboardFilter from superset.extensions import db from superset.models.core import FavStar, FavStarClassName @@ -35,6 +37,20 @@ class DashboardDAO(BaseDAO): model_cls = Dashboard base_filter = DashboardFilter + @staticmethod + def get_charts_for_dashboard(dashboard_id: int) -> List[Slice]: + query = ( + db.session.query(Dashboard) + .outerjoin(Slice, Dashboard.slices) + .outerjoin(Slice.table) + .filter(Dashboard.id == dashboard_id) + .options(contains_eager(Dashboard.slices)) + ) + dashboard = query.one_or_none() + if not dashboard: + raise DashboardNotFoundError() + return dashboard.slices + @staticmethod def validate_slug_uniqueness(slug: str) -> bool: if not slug: diff --git a/tests/base_tests.py b/tests/base_tests.py index a55302d1c06..e39a55cee93 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -29,6 +29,7 @@ from flask_appbuilder.security.sqla import models as ab_models from flask_testing import TestCase from sqlalchemy.ext.declarative.api import DeclarativeMeta from sqlalchemy.orm import Session +from sqlalchemy.sql import func from tests.test_app import app from superset.sql_parse import CtasMethod @@ -124,6 +125,10 @@ class SupersetTestCase(TestCase): def create_app(self): return app + @staticmethod + def get_nonexistent_numeric_id(model): + return (db.session.query(func.max(model.id)).scalar() or 0) + 1 + @staticmethod def get_birth_names_dataset(): example_db = get_example_database() diff --git a/tests/charts/api_tests.py b/tests/charts/api_tests.py index 94e4d8f3e8c..fef99e0b881 100644 --- a/tests/charts/api_tests.py +++ b/tests/charts/api_tests.py @@ -17,13 +17,13 @@ # isort:skip_file """Unit tests for Superset""" import json -from typing import List, Optional from datetime import datetime, timedelta from io import BytesIO from unittest import mock from zipfile import is_zipfile, ZipFile from superset.models.sql_lab import Query +from tests.insert_chart_mixin import InsertChartMixin from tests.fixtures.birth_names_dashboard import load_birth_names_dashboard_with_slices import humanize @@ -36,9 +36,8 @@ from sqlalchemy.sql import func from tests.fixtures.world_bank_dashboard import load_world_bank_dashboard_with_slices from tests.test_app import app from superset.charts.commands.data import ChartDataCommand -from superset.connectors.connector_registry import ConnectorRegistry from superset.connectors.sqla.models import SqlaTable -from superset.extensions import async_query_manager, cache_manager, db, security_manager +from superset.extensions import async_query_manager, cache_manager, db from superset.models.annotations import AnnotationLayer from superset.models.core import Database, FavStar, FavStarClassName from superset.models.dashboard import Dashboard @@ -67,44 +66,9 @@ CHART_DATA_URI = "api/v1/chart/data" CHARTS_FIXTURE_COUNT = 10 -class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin): +class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin): resource_name = "chart" - def insert_chart( - self, - slice_name: str, - owners: List[int], - datasource_id: int, - created_by=None, - datasource_type: str = "table", - description: Optional[str] = None, - viz_type: Optional[str] = None, - params: Optional[str] = None, - cache_timeout: Optional[int] = None, - ) -> Slice: - obj_owners = list() - for owner in owners: - user = db.session.query(security_manager.user_model).get(owner) - obj_owners.append(user) - datasource = ConnectorRegistry.get_datasource( - datasource_type, datasource_id, db.session - ) - slice = Slice( - cache_timeout=cache_timeout, - created_by=created_by, - datasource_id=datasource.id, - datasource_name=datasource.name, - datasource_type=datasource.type, - description=description, - owners=obj_owners, - params=params, - slice_name=slice_name, - viz_type=viz_type, - ) - db.session.add(slice) - db.session.commit() - return slice - @pytest.fixture(autouse=True) def clear_data_cache(self): with app.app_context(): diff --git a/tests/dashboards/api_tests.py b/tests/dashboards/api_tests.py index 658963ed523..5f5504e3181 100644 --- a/tests/dashboards/api_tests.py +++ b/tests/dashboards/api_tests.py @@ -23,6 +23,8 @@ from typing import List, Optional from unittest.mock import patch from zipfile import is_zipfile, ZipFile +from tests.insert_chart_mixin import InsertChartMixin + import pytest import prison import yaml @@ -54,9 +56,10 @@ from tests.fixtures.world_bank_dashboard import load_world_bank_dashboard_with_s DASHBOARDS_FIXTURE_COUNT = 10 -class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin): +class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin): resource_name = "dashboard" + dashboards: List[Dashboard] = [] dashboard_data = { "dashboard_title": "title1_changed", "slug": "slug1_changed", @@ -109,21 +112,35 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin): with self.create_app().app_context(): dashboards = [] admin = self.get_user("admin") - for cx in range(DASHBOARDS_FIXTURE_COUNT - 1): - dashboards.append( - self.insert_dashboard(f"title{cx}", f"slug{cx}", [admin.id]) + charts = [] + half_dash_count = round(DASHBOARDS_FIXTURE_COUNT / 2) + for cx in range(DASHBOARDS_FIXTURE_COUNT): + dashboard = self.insert_dashboard( + f"title{cx}", + f"slug{cx}", + [admin.id], + slices=charts if cx < half_dash_count else [], ) + if cx < half_dash_count: + chart = self.insert_chart(f"slice{cx}", [admin.id], 1, params="{}") + charts.append(chart) + dashboard.slices = [chart] + db.session.add(dashboard) + dashboards.append(dashboard) fav_dashboards = [] - for cx in range(round(DASHBOARDS_FIXTURE_COUNT / 2)): + for cx in range(half_dash_count): fav_star = FavStar( user_id=admin.id, class_name="Dashboard", obj_id=dashboards[cx].id ) db.session.add(fav_star) db.session.commit() fav_dashboards.append(fav_star) + self.dashboards = dashboards yield dashboards # rollback changes + for chart in charts: + db.session.delete(chart) for dashboard in dashboards: db.session.delete(dashboard) for fav_dashboard in fav_dashboards: @@ -152,6 +169,45 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin): db.session.delete(dashboard) db.session.commit() + @pytest.mark.usefixtures("create_dashboards") + def test_get_dashboard_charts(self): + """ + Dashboard API: Test getting charts belonging to a dashboard + """ + dashboard = self.dashboards[0] + uri = f"api/v1/dashboard/{dashboard.id}/charts" + response = self.get_assert_metric(uri, "get_charts") + self.assertEqual(response.status_code, 200) + data = json.loads(response.data.decode("utf-8")) + self.assertEqual(len(data["result"]), 1) + self.assertEqual( + data["result"][0]["slice_name"], dashboard.slices[0].slice_name + ) + + @pytest.mark.usefixtures("create_dashboards") + def test_get_dashboard_charts_not_found(self): + """ + Dashboard API: Test getting charts belonging to a dashboard that does not exist + """ + self.login(username="admin") + bad_id = self.get_nonexistent_numeric_id(Dashboard) + uri = f"api/v1/dashboard/{bad_id}/charts" + response = self.get_assert_metric(uri, "get_charts") + self.assertEqual(response.status_code, 404) + + @pytest.mark.usefixtures("create_dashboards") + def test_get_dashboard_charts_empty(self): + """ + Dashboard API: Test getting charts belonging to a dashboard without any charts + """ + self.login(username="admin") + # the fixture setup assigns no charts to the second half of dashboards + uri = f"api/v1/dashboard/{self.dashboards[-1].id}/charts" + response = self.get_assert_metric(uri, "get_charts") + self.assertEqual(response.status_code, 200) + data = json.loads(response.data.decode("utf-8")) + self.assertEqual(data["result"], []) + def test_get_dashboard(self): """ Dashboard API: Test get dashboard @@ -228,9 +284,9 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin): """ Dashboard API: Test get dashboard not found """ - max_id = db.session.query(func.max(Dashboard.id)).scalar() + bad_id = self.get_nonexistent_numeric_id(Dashboard) self.login(username="admin") - uri = f"api/v1/dashboard/{max_id + 1}" + uri = f"api/v1/dashboard/{bad_id}" rv = self.get_assert_metric(uri, "get") self.assertEqual(rv.status_code, 404) @@ -575,7 +631,7 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin): self.assertEqual(rv.status_code, 404) @pytest.mark.usefixtures("create_dashboard_with_report", "create_dashboards") - def test_bulk_delete_dashboard_with_report(self): + def test_delete_bulk_dashboard_with_report(self): """ Dashboard API: Test bulk delete with associated report """ @@ -1308,7 +1364,7 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin): def test_import_dashboard_invalid(self): """ - Dataset API: Test import invalid dashboard + Dashboard API: Test import invalid dashboard """ self.login(username="admin") uri = "api/v1/dashboard/import/" diff --git a/tests/insert_chart_mixin.py b/tests/insert_chart_mixin.py new file mode 100644 index 00000000000..2aabc4b1a92 --- /dev/null +++ b/tests/insert_chart_mixin.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. +from typing import List, Optional + +from superset import ConnectorRegistry, db, security_manager +from superset.models.slice import Slice + + +class InsertChartMixin: + """ + Implements shared logic for tests to insert charts (slices) in the DB + """ + + def insert_chart( + self, + slice_name: str, + owners: List[int], + datasource_id: int, + created_by=None, + datasource_type: str = "table", + description: Optional[str] = None, + viz_type: Optional[str] = None, + params: Optional[str] = None, + cache_timeout: Optional[int] = None, + ) -> Slice: + obj_owners = list() + for owner in owners: + user = db.session.query(security_manager.user_model).get(owner) + obj_owners.append(user) + datasource = ConnectorRegistry.get_datasource( + datasource_type, datasource_id, db.session + ) + slice = Slice( + cache_timeout=cache_timeout, + created_by=created_by, + datasource_id=datasource.id, + datasource_name=datasource.name, + datasource_type=datasource.type, + description=description, + owners=obj_owners, + params=params, + slice_name=slice_name, + viz_type=viz_type, + ) + db.session.add(slice) + db.session.commit() + return slice