fix(dashboard): Get dashboard by slug (#13352)

* refactor out id_or_slug filter logic

* fix(dashboard): accept slug in place of id in url

* remove unnecessary show fields

* fixes and tests

* linting

* linter compliance

* change requests

* names

superset/dashboards/api.py

@@ -58,6 +58,7 @@ from superset.dashboards.filters import (
     FilterRelatedRoles,
 )
 from superset.dashboards.schemas import (
+    DashboardGetResponseSchema,
     DashboardPostSchema,
     DashboardPutSchema,
     get_delete_ids_schema,
@@ -99,29 +100,6 @@ class DashboardRestApi(BaseSupersetModelRestApi):
     class_permission_name = "Dashboard"
     method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
 
-    show_columns = [
-        "id",
-        "charts",
-        "css",
-        "dashboard_title",
-        "json_metadata",
-        "owners.id",
-        "owners.username",
-        "owners.first_name",
-        "owners.last_name",
-        "roles.id",
-        "roles.name",
-        "changed_by_name",
-        "changed_by_url",
-        "changed_by.username",
-        "changed_on",
-        "position_json",
-        "published",
-        "url",
-        "slug",
-        "table_names",
-        "thumbnail_url",
-    ]
     list_columns = [
         "id",
         "published",
@@ -190,6 +168,7 @@ class DashboardRestApi(BaseSupersetModelRestApi):
     add_model_schema = DashboardPostSchema()
     edit_model_schema = DashboardPutSchema()
     chart_entity_response_schema = ChartEntityResponseSchema()
+    dashboard_get_response_schema = DashboardGetResponseSchema()
 
     base_filters = [["slice", DashboardFilter, lambda: []]]
 
@@ -207,7 +186,11 @@ class DashboardRestApi(BaseSupersetModelRestApi):
 
     openapi_spec_tag = "Dashboards"
     """ Override the name set for this collection of endpoints """
-    openapi_spec_component_schemas = (ChartEntityResponseSchema, GetFavStarIdsSchema)
+    openapi_spec_component_schemas = (
+        ChartEntityResponseSchema,
+        DashboardGetResponseSchema,
+        GetFavStarIdsSchema,
+    )
     apispec_parameter_schemas = {
         "get_delete_ids_schema": get_delete_ids_schema,
         "get_export_ids_schema": get_export_ids_schema,
@@ -222,6 +205,53 @@ class DashboardRestApi(BaseSupersetModelRestApi):
             self.include_route_methods = self.include_route_methods | {"thumbnail"}
         super().__init__()
 
+    @expose("/<id_or_slug>", methods=["GET"])
+    @protect()
+    @safe
+    @statsd_metrics
+    @event_logger.log_this_with_context(
+        action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get",
+        log_to_statsd=False,
+    )
+    def get(self, id_or_slug: str) -> Response:
+        """Gets a dashboard
+        ---
+        get:
+          description: >-
+            Get a dashboard
+          parameters:
+          - in: path
+            schema:
+              type: string
+            name: id_or_slug
+            description: Either the id of the dashboard, or its slug
+          responses:
+            200:
+              description: Dashboard
+              content:
+                application/json:
+                  schema:
+                    type: object
+                    properties:
+                      result:
+                        $ref: '#/components/schemas/DashboardGetResponseSchema'
+            302:
+              description: Redirects to the current digest
+            400:
+              $ref: '#/components/responses/400'
+            401:
+              $ref: '#/components/responses/401'
+            404:
+              $ref: '#/components/responses/404'
+        """
+        # pylint: disable=arguments-differ
+        try:
+            dash = DashboardDAO.get_by_id_or_slug(id_or_slug)
+            result = self.dashboard_get_response_schema.dump(dash)
+            return self.response(200, result=result)
+        except DashboardNotFoundError:
+            return self.response_404()
+
     @expose("/<pk>/charts", methods=["GET"])
     @protect()
     @safe

superset/dashboards/dao.py

@@ -27,7 +27,7 @@ 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
-from superset.models.dashboard import Dashboard
+from superset.models.dashboard import Dashboard, id_or_slug_filter
 from superset.models.slice import Slice
 from superset.utils.dashboard_filter_scopes_converter import copy_filter_scopes
 
@@ -38,6 +38,25 @@ class DashboardDAO(BaseDAO):
     model_cls = Dashboard
     base_filter = DashboardFilter
 
+    @staticmethod
+    def get_by_id_or_slug(id_or_slug: str) -> Dashboard:
+        query = (
+            db.session.query(Dashboard)
+            .filter(id_or_slug_filter(id_or_slug))
+            .outerjoin(Slice, Dashboard.slices)
+            .outerjoin(Slice.table)
+            .outerjoin(Dashboard.owners)
+            .outerjoin(Dashboard.roles)
+        )
+        # Apply dashboard base filters
+        query = DashboardFilter("id", SQLAInterface(Dashboard, db.session)).apply(
+            query, None
+        )
+        dashboard = query.one_or_none()
+        if not dashboard:
+            raise DashboardNotFoundError()
+        return dashboard
+
     @staticmethod
     def get_charts_for_dashboard(dashboard_id: int) -> List[Slice]:
         query = (

superset/dashboards/schemas.py

@@ -61,7 +61,9 @@ published_description = (
     "Determines whether or not this dashboard is visible in "
     "the list of all dashboards."
 )
-
+charts_description = (
+    "The names of the dashboard's charts. Names are used for legacy reasons."
+)
 
 openapi_spec_methods_override = {
     "get": {"get": {"description": "Get a dashboard detail information."}},
@@ -124,6 +126,38 @@ class DashboardJSONMetadataSchema(Schema):
     remote_id = fields.Integer()
 
 
+class UserSchema(Schema):
+    id = fields.Int()
+    username = fields.String()
+    first_name = fields.String()
+    last_name = fields.String()
+
+
+class RolesSchema(Schema):
+    id = fields.Int()
+    name = fields.String()
+
+
+class DashboardGetResponseSchema(Schema):
+    id = fields.Int()
+    slug = fields.String()
+    url = fields.String()
+    dashboard_title = fields.String(description=dashboard_title_description)
+    thumbnail_url = fields.String()
+    published = fields.Boolean()
+    css = fields.String(description=css_description)
+    json_metadata = fields.String(description=json_metadata_description)
+    position_json = fields.String(description=position_json_description)
+    changed_by_name = fields.String()
+    changed_by_url = fields.String()
+    changed_by = fields.Nested(UserSchema)
+    changed_on = fields.DateTime()
+    charts = fields.List(fields.String(description=charts_description))
+    owners = fields.List(fields.Nested(UserSchema))
+    roles = fields.List(fields.Nested(RolesSchema))
+    table_names = fields.String()  # legacy nonsense
+
+
 class BaseDashboardSchema(Schema):
     # pylint: disable=no-self-use,unused-argument
     @post_load

superset/models/dashboard.py

@@ -42,6 +42,7 @@ from sqlalchemy.orm import relationship, sessionmaker, subqueryload
 from sqlalchemy.orm.mapper import Mapper
 from sqlalchemy.orm.session import object_session
 from sqlalchemy.sql import join, select
+from sqlalchemy.sql.elements import BinaryExpression
 
 from superset import app, ConnectorRegistry, db, is_feature_enabled, security_manager
 from superset.connectors.base.models import BaseDatasource
@@ -57,6 +58,8 @@ from superset.utils import core as utils
 from superset.utils.decorators import debounce
 from superset.utils.urls import get_url_path
 
+# pylint: disable=too-many-public-methods
+
 metadata = Model.metadata  # pylint: disable=no-member
 config = app.config
 logger = logging.getLogger(__name__)
@@ -361,15 +364,16 @@ class Dashboard(  # pylint: disable=too-many-instance-attributes
     @classmethod
     def get(cls, id_or_slug: str) -> Dashboard:
         session = db.session()
-        qry = session.query(Dashboard)
-        if id_or_slug.isdigit():
-            qry = qry.filter_by(id=int(id_or_slug))
-        else:
-            qry = qry.filter_by(slug=id_or_slug)
-
+        qry = session.query(Dashboard).filter(id_or_slug_filter(id_or_slug))
         return qry.one_or_none()
 
 
+def id_or_slug_filter(id_or_slug: str) -> BinaryExpression:
+    if id_or_slug.isdigit():
+        return Dashboard.id == int(id_or_slug)
+    return Dashboard.slug == id_or_slug
+
+
 OnDashboardChange = Callable[[Mapper, Connection, Dashboard], Any]
 
 # events for updating tags

tests/dashboards/api_tests.py

@@ -170,6 +170,32 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi
             db.session.delete(dashboard)
             db.session.commit()
 
+    @pytest.mark.usefixtures("create_dashboards")
+    def get_dashboard_by_slug(self):
+        self.login(username="admin")
+        dashboard = self.dashboards[0]
+        uri = f"api/v1/dashboard/{dashboard.slug}"
+        response = self.get_assert_metric(uri, "get")
+        self.assertEqual(response.status_code, 200)
+        data = json.loads(response.data.decode("utf-8"))
+        self.assertEqual(data["id"], dashboard.id)
+
+    @pytest.mark.usefixtures("create_dashboards")
+    def get_dashboard_by_bad_slug(self):
+        self.login(username="admin")
+        dashboard = self.dashboards[0]
+        uri = f"api/v1/dashboard/{dashboard.slug}-bad-slug"
+        response = self.get_assert_metric(uri, "get")
+        self.assertEqual(response.status_code, 404)
+
+    @pytest.mark.usefixtures("create_dashboards")
+    def get_dashboard_by_slug_not_allowed(self):
+        self.login(username="gamma")
+        dashboard = self.dashboards[0]
+        uri = f"api/v1/dashboard/{dashboard.slug}"
+        response = self.get_assert_metric(uri, "get")
+        self.assertEqual(response.status_code, 404)
+
     @pytest.mark.usefixtures("create_dashboards")
     def test_get_dashboard_charts(self):
         """
@@ -242,6 +268,7 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi
             "id": dashboard.id,
             "css": "",
             "dashboard_title": "title",
+            "datasources": [],
             "json_metadata": "",
             "owners": [
                 {