dpgaspar commented on a change in pull request #13352:
URL: https://github.com/apache/superset/pull/13352#discussion_r586255569
##########
File path: superset/models/dashboard.py
##########
@@ -361,15 +364,16 @@ def export_dashboards( # pylint: disable=too-many-locals
@classmethod
def get(cls, id_or_slug: str) -> Dashboard:
session = db.session()
Review comment:
Unrelated, but seems strange that this is creating a new session here
##########
File path: superset/dashboards/dao.py
##########
@@ -38,6 +38,20 @@ class DashboardDAO(BaseDAO):
model_cls = Dashboard
base_filter = DashboardFilter
+ @staticmethod
+ def get_dashboard(id_or_slug: str) -> Dashboard:
Review comment:
nit: would prefer something like `find_by_id_or_slug` or
`get_by_id_slug` we already know it's a dashboard
##########
File path: superset/models/dashboard.py
##########
@@ -361,15 +364,16 @@ def export_dashboards( # pylint: disable=too-many-locals
@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:
Review comment:
nit: `def id_or_slug_filter(id_or_slug: Union[int, str]) ->
BinaryExpression:`
##########
File path: tests/dashboards/api_tests.py
##########
@@ -169,6 +169,32 @@ def create_dashboard_with_report(self):
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):
Review comment:
nice!
##########
File path: superset/dashboards/schemas.py
##########
@@ -121,6 +123,45 @@ class DashboardJSONMetadataSchema(Schema):
label_colors = fields.Dict()
+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 DatasourceSchema(Schema):
+ id = fields.Int()
+ name = fields.String()
+ description = fields.String()
+
+
+class DashboardResponseSchema(Schema):
Review comment:
maybe `DashboardGetResponseSchema` since responses vary from HTTP method
to HTTP method
##########
File path: superset/dashboards/api.py
##########
@@ -222,6 +205,52 @@ def __init__(self) -> None:
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
Review comment:
nit: we can add a small description here
##########
File path: superset/dashboards/dao.py
##########
@@ -38,6 +38,18 @@ class DashboardDAO(BaseDAO):
model_cls = Dashboard
base_filter = DashboardFilter
+ @staticmethod
+ def get_dashboard(id_or_slug: str) -> Dashboard:
+ query =
db.session.query(Dashboard).filter(id_or_slug_filter(id_or_slug))
Review comment:
evaluate if it's more efficient to join here with owners, charts and
roles
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
