From 468463ccacc0e2042c151cd3ddd69696d3b5e863 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 30 Mar 2022 14:33:23 +0100 Subject: [PATCH 01/14] feat: deprecate old API and create new API for dashes created by me --- superset/dashboards/api.py | 48 +++++++++++++++++++++++++++------- superset/dashboards/dao.py | 20 ++++++++++++++ superset/dashboards/schemas.py | 8 ++++++ superset/views/core.py | 16 +++++++++--- 4 files changed, 79 insertions(+), 13 deletions(-) diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 5e761cdd1f092..d65ef12ad9a56 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -23,7 +23,7 @@ from zipfile import is_zipfile, ZipFile from flask import g, make_response, redirect, request, Response, send_file, url_for -from flask_appbuilder.api import expose, protect, rison, safe +from flask_appbuilder.api import expose, permission_name, protect, rison, safe from flask_appbuilder.hooks import before_request from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_babel import ngettext @@ -61,6 +61,7 @@ FilterRelatedRoles, ) from superset.dashboards.schemas import ( + DashboardCreatedByMeResponseSchema, DashboardDatasetSchema, DashboardGetResponseSchema, DashboardPostSchema, @@ -212,6 +213,7 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: """ Override the name set for this collection of endpoints """ openapi_spec_component_schemas = ( ChartEntityResponseSchema, + DashboardCreatedByMeResponseSchema, DashboardGetResponseSchema, DashboardDatasetSchema, GetFavStarIdsSchema, @@ -272,8 +274,6 @@ def get(self, id_or_slug: str) -> Response: properties: result: $ref: '#/components/schemas/DashboardGetResponseSchema' - 302: - description: Redirects to the current digest 400: $ref: '#/components/responses/400' 401: @@ -336,8 +336,6 @@ def get_datasets(self, id_or_slug: str) -> Response: type: array items: $ref: '#/components/schemas/DashboardDatasetSchema' - 302: - description: Redirects to the current digest 400: $ref: '#/components/responses/400' 401: @@ -399,8 +397,6 @@ def get_charts(self, id_or_slug: str) -> Response: type: array items: $ref: '#/components/schemas/ChartEntityResponseSchema' - 302: - description: Redirects to the current digest 400: $ref: '#/components/responses/400' 401: @@ -427,6 +423,42 @@ def get_charts(self, id_or_slug: str) -> Response: except DashboardNotFoundError: return self.response_404() + @expose("/created_by_me/", 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, # pylint: disable=arguments-renamed + ) + @permission_name("can_read") + def created_by_me(self, id_or_slug: str) -> Response: + """Gets all dashboards created by the current user + --- + get: + description: >- + Gets all dashboards created by the current user + responses: + 200: + description: Dashboard + content: + application/json: + schema: + type: object + properties: + result: + $ref: '#/components/schemas/DashboardCreatedByMeResponseSchema' + 401: + $ref: '#/components/responses/401' + 403: + $ref: '#/components/responses/403' + 404: + $ref: '#/components/responses/404' + """ + dashboards = DashboardDAO.get_dashboards_created_changed_by_user(g.user.id) + result = self.dashboard_get_response_schema.dump(dashboards) + return self.response(200, result=result) + @expose("/", methods=["POST"]) @protect() @safe @@ -461,8 +493,6 @@ def post(self) -> Response: type: number result: $ref: '#/components/schemas/{{self.__class__.__name__}}.post' - 302: - description: Redirects to the current digest 400: $ref: '#/components/responses/400' 401: diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index ce6e30f8d4beb..d64bcb75b289e 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -19,6 +19,7 @@ from datetime import datetime from typing import Any, Dict, List, Optional, Union +from sqlalchemy import or_ from sqlalchemy.exc import SQLAlchemyError from superset import security_manager @@ -100,6 +101,25 @@ def get_dashboard_and_slices_changed_on( # pylint: disable=invalid-name # drop microseconds in datetime to match with last_modified header return max(dashboard_changed_on, slices_changed_on).replace(microsecond=0) + @staticmethod + def get_dashboards_created_changed_by_user(user_id: int) -> List[Dashboard]: + """ + Gets a list of dashboards that were created or changed by a certain user + :param user_id: The user id + :return: List of dashboards + """ + qry = ( + db.session.query(Dashboard) + .filter( # pylint: disable=comparison-with-callable + or_( + Dashboard.created_by_fk == user_id, + Dashboard.changed_by_fk == user_id, + ) + ) + .order_by(Dashboard.changed_on.desc()) + ) + return qry.all() + @staticmethod def get_dashboard_and_datasets_changed_on( # pylint: disable=invalid-name id_or_slug_or_dashboard: Union[str, Dashboard] diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 9b668df7212d0..a4c72e60e84bb 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -227,6 +227,14 @@ def post_load(self, data: Dict[str, Any], **kwargs: Any) -> Dict[str, Any]: return data +class DashboardCreatedByMeResponseSchema(Schema): + id = fields.Int() + dashboard = fields.Str() + title = fields.Str() + url = fields.Str() + dttm = fields.DateTime() + + class DashboardPostSchema(BaseDashboardSchema): dashboard_title = fields.String( description=dashboard_title_description, diff --git a/superset/views/core.py b/superset/views/core.py index c806470e8f1de..e57aeee814696 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1586,16 +1586,24 @@ def fave_dashboards(self, user_id: int) -> FlaskResponse: @event_logger.log_this @expose("/created_dashboards//", methods=["GET"]) def created_dashboards(self, user_id: int) -> FlaskResponse: + logging.warning( + "%s.select_star " + "This API endpoint is deprecated and will be removed in version 3.0.0", + self.__class__.__name__, + ) + error_obj = self.get_user_activity_access_error(user_id) if error_obj: return error_obj - Dash = Dashboard qry = ( - db.session.query(Dash) + db.session.query(Dashboard) .filter( # pylint: disable=comparison-with-callable - or_(Dash.created_by_fk == user_id, Dash.changed_by_fk == user_id) + or_( + Dashboard.created_by_fk == user_id, + Dashboard.changed_by_fk == user_id, + ) ) - .order_by(Dash.changed_on.desc()) + .order_by(Dashboard.changed_on.desc()) ) payload = [ { From e59507e1df6f1c6aca4c356f13e8d33208876131 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 31 Mar 2022 13:32:49 +0100 Subject: [PATCH 02/14] add tests --- .../src/profile/components/CreatedContent.tsx | 8 ++-- superset-frontend/src/types/bootstrapTypes.ts | 4 ++ superset/constants.py | 1 + superset/dashboards/api.py | 13 +++-- superset/dashboards/schemas.py | 15 ++++-- .../integration_tests/dashboards/api_tests.py | 47 +++++++++++++++++++ 6 files changed, 76 insertions(+), 12 deletions(-) diff --git a/superset-frontend/src/profile/components/CreatedContent.tsx b/superset-frontend/src/profile/components/CreatedContent.tsx index b097dee5e1448..df2feeee76b2f 100644 --- a/superset-frontend/src/profile/components/CreatedContent.tsx +++ b/superset-frontend/src/profile/components/CreatedContent.tsx @@ -22,7 +22,7 @@ import { t } from '@superset-ui/core'; import TableLoader from '../../components/TableLoader'; import { Slice } from '../types'; -import { User, Dashboard } from '../../types/bootstrapTypes'; +import { User, DashboardResponse } from '../../types/bootstrapTypes'; interface CreatedContentProps { user: User; @@ -49,8 +49,8 @@ class CreatedContent extends React.PureComponent { } renderDashboardTable() { - const mutator = (data: Dashboard[]) => - data.map(dash => ({ + const mutator = (data: DashboardResponse) => + data.result.map(dash => ({ dashboard: {dash.title}, created: moment.utc(dash.dttm).fromNow(), _created: dash.dttm, @@ -59,7 +59,7 @@ class CreatedContent extends React.PureComponent { Optional[Response]: "set_embedded", "delete_embedded", "thumbnail", + "created_by_me", } resource_name = "dashboard" allow_browser_login = True @@ -226,6 +227,7 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: dashboard_dataset_schema = DashboardDatasetSchema() embedded_response_schema = EmbeddedDashboardResponseSchema() embedded_config_schema = EmbeddedDashboardConfigSchema() + dashboard_created_by_schema = DashboardCreatedByMeResponseSchema() base_filters = [["id", DashboardAccessFilter, lambda: []]] @@ -457,10 +459,9 @@ def get_charts(self, id_or_slug: str) -> Response: @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get", - log_to_statsd=False, # pylint: disable=arguments-renamed + log_to_statsd=False, ) - @permission_name("can_read") - def created_by_me(self, id_or_slug: str) -> Response: + def created_by_me(self) -> Response: """Gets all dashboards created by the current user --- get: @@ -475,7 +476,9 @@ def created_by_me(self, id_or_slug: str) -> Response: type: object properties: result: - $ref: '#/components/schemas/DashboardCreatedByMeResponseSchema' + type: array + items: + $ref: '#/components/schemas/DashboardCreatedByMeResponseSchema' 401: $ref: '#/components/responses/401' 403: @@ -484,7 +487,7 @@ def created_by_me(self, id_or_slug: str) -> Response: $ref: '#/components/responses/404' """ dashboards = DashboardDAO.get_dashboards_created_changed_by_user(g.user.id) - result = self.dashboard_get_response_schema.dump(dashboards) + result = self.dashboard_created_by_schema.dump(dashboards, many=True) return self.response(200, result=result) @expose("/", methods=["POST"]) diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 1d2d8089d0445..655d04674b906 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -18,10 +18,11 @@ import re from typing import Any, Dict, Union -from marshmallow import fields, post_load, Schema +from marshmallow import fields, post_dump, post_load, Schema from marshmallow.validate import Length, ValidationError from superset.exceptions import SupersetException +from superset.models.dashboard import Dashboard from superset.utils import core as utils get_delete_ids_schema = {"type": "array", "items": {"type": "integer"}} @@ -229,10 +230,18 @@ def post_load(self, data: Dict[str, Any], **kwargs: Any) -> Dict[str, Any]: class DashboardCreatedByMeResponseSchema(Schema): id = fields.Int() - dashboard = fields.Str() title = fields.Str() url = fields.Str() - dttm = fields.DateTime() + changed_on = fields.DateTime() + dttm = fields.Int() + + @post_dump(pass_original=True) + def post_dump( + self, data: Dict[str, Any], obj: Dashboard, many: bool = True + ) -> Dict[str, Any]: + data["dttm"] = utils.json_int_dttm_ser(obj.changed_on) + data["title"] = obj.dashboard_title + return data class DashboardPostSchema(BaseDashboardSchema): diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index a179fa7c7e453..c5298abf89e8f 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -160,6 +160,26 @@ def create_dashboards(self): db.session.delete(fav_dashboard) db.session.commit() + @pytest.fixture() + def create_created_by_admin_dashboards(self): + with self.create_app().app_context(): + dashboards = [] + admin = self.get_user("admin") + for cx in range(2): + dashboard = self.insert_dashboard( + f"create_title{cx}", + f"create_slug{cx}", + [admin.id], + created_by=admin, + ) + dashboards.append(dashboard) + + yield dashboards + + for dashboard in dashboards: + db.session.delete(dashboard) + db.session.commit() + @pytest.fixture() def create_dashboard_with_report(self): with self.create_app().app_context(): @@ -676,6 +696,33 @@ def test_gets_not_certified_dashboards_filter(self): data = json.loads(rv.data.decode("utf-8")) self.assertEqual(data["count"], 6) + @pytest.mark.usefixtures("create_created_by_admin_dashboards") + @pytest.mark.usefixtures("create_dashboards") + def test_get_dashboards_created_by_me(self): + """ + Dashboard API: Test get dashboards created by current user + """ + uri = f"api/v1/dashboard/created_by_me/" + self.login(username="admin") + rv = self.client.get(uri) + data = json.loads(rv.data.decode("utf-8")) + assert rv.status_code == 200 + assert len(data["result"]) == 2 + assert list(data["result"][0].keys()) == [ + "changed_on", + "dttm", + "id", + "title", + "url", + ] + expected_results = [ + {"title": "create_title1", "url": "/superset/dashboard/create_slug1/"}, + {"title": "create_title0", "url": "/superset/dashboard/create_slug0/"}, + ] + for idx, response_item in enumerate(data["result"]): + for key, value in expected_results[idx].items(): + assert response_item[key] == value + def create_dashboard_import(self): buf = BytesIO() with ZipFile(buf, "w") as bundle: From 4bdef8b7098e77401f1a99fe78b65b939989f52b Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Fri, 1 Apr 2022 15:54:16 +0100 Subject: [PATCH 03/14] fix previous test --- tests/integration_tests/dashboard_tests.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/integration_tests/dashboard_tests.py b/tests/integration_tests/dashboard_tests.py index 63453d85ee4fc..3ad9b07e29c14 100644 --- a/tests/integration_tests/dashboard_tests.py +++ b/tests/integration_tests/dashboard_tests.py @@ -18,6 +18,7 @@ """Unit tests for Superset""" from datetime import datetime import json +import re import unittest from random import random @@ -139,9 +140,20 @@ def test_new_dashboard(self): self.login(username="admin") dash_count_before = db.session.query(func.count(Dashboard.id)).first()[0] url = "/dashboard/new/" - resp = self.get_resp(url) + response = self.client.get(url, follow_redirects=False) dash_count_after = db.session.query(func.count(Dashboard.id)).first()[0] self.assertEqual(dash_count_before + 1, dash_count_after) + group = re.match( + r"http:\/\/localhost\/superset\/dashboard\/([0-9]*)\/\?edit=true", + response.headers["Location"], + ) + assert group is not None + + # Cleanup + created_dashboard_id = int(group[1]) + created_dashboard = db.session.query(Dashboard).get(created_dashboard_id) + db.session.delete(created_dashboard) + db.session.commit() @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_save_dash(self, username="admin"): From e69778234e5658b9acffbef3ace596eb50c0cb2f Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Fri, 1 Apr 2022 17:00:41 +0100 Subject: [PATCH 04/14] fix test and lint --- superset-frontend/package-lock.json | 10 ++-------- superset/dashboards/api.py | 2 +- superset/dashboards/dao.py | 2 +- tests/integration_tests/dashboards/api_tests.py | 2 +- 4 files changed, 5 insertions(+), 11 deletions(-) diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 59582524ea16a..ef850561fe51b 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -275,7 +275,7 @@ }, "engines": { "node": "^16.9.1", - "npm": "^7.5.4" + "npm": "^7.5.4 || ^8.1.2" } }, "buildtools/eslint-plugin-theme-colors": { @@ -59782,9 +59782,6 @@ "version": "1.0.0", "dev": true, "license": "Apache-2.0", - "dependencies": { - "lodash": "^4.17.21" - }, "engines": { "node": "^16.9.1", "npm": "^7.5.4" @@ -86468,10 +86465,7 @@ } }, "eslint-plugin-theme-colors": { - "version": "file:tools/eslint-plugin-theme-colors", - "requires": { - "lodash": "^4.17.21" - } + "version": "file:tools/eslint-plugin-theme-colors" }, "eslint-scope": { "version": "5.1.1", diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index f4b09d9c58474..8a81d8d4b2b67 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -486,7 +486,7 @@ def created_by_me(self) -> Response: 404: $ref: '#/components/responses/404' """ - dashboards = DashboardDAO.get_dashboards_created_changed_by_user(g.user.id) + dashboards = DashboardDAO.get_dashboards_created_by(g.user.id) result = self.dashboard_created_by_schema.dump(dashboards, many=True) return self.response(200, result=result) diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index d64bcb75b289e..aedf9f8cd9a51 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -102,7 +102,7 @@ def get_dashboard_and_slices_changed_on( # pylint: disable=invalid-name return max(dashboard_changed_on, slices_changed_on).replace(microsecond=0) @staticmethod - def get_dashboards_created_changed_by_user(user_id: int) -> List[Dashboard]: + def get_dashboards_created_by(user_id: int) -> List[Dashboard]: """ Gets a list of dashboards that were created or changed by a certain user :param user_id: The user id diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index c5298abf89e8f..d2a501f832a49 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -716,8 +716,8 @@ def test_get_dashboards_created_by_me(self): "url", ] expected_results = [ - {"title": "create_title1", "url": "/superset/dashboard/create_slug1/"}, {"title": "create_title0", "url": "/superset/dashboard/create_slug0/"}, + {"title": "create_title1", "url": "/superset/dashboard/create_slug1/"}, ] for idx, response_item in enumerate(data["result"]): for key, value in expected_results[idx].items(): From 00b16811314d188cacf1cd5413c187ab7116c5ea Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 5 Apr 2022 14:17:16 +0100 Subject: [PATCH 05/14] fix sqlite test --- tests/integration_tests/dashboards/api_tests.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index d2a501f832a49..9cfe82122e46c 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -715,6 +715,9 @@ def test_get_dashboards_created_by_me(self): "title", "url", ] + if backend() == "sqlite": + # sqlite doesn't support ms timestamps not possible to infer proper ordering + return expected_results = [ {"title": "create_title0", "url": "/superset/dashboard/create_slug0/"}, {"title": "create_title1", "url": "/superset/dashboard/create_slug1/"}, From 7948e836c0df388067d2a84ed5cd5777fa4f192d Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 5 Apr 2022 14:39:19 +0100 Subject: [PATCH 06/14] fix lint --- superset/dashboards/schemas.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 655d04674b906..8b9a6ffd950be 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -235,9 +235,10 @@ class DashboardCreatedByMeResponseSchema(Schema): changed_on = fields.DateTime() dttm = fields.Int() + @staticmethod @post_dump(pass_original=True) def post_dump( - self, data: Dict[str, Any], obj: Dashboard, many: bool = True + data: Dict[str, Any], obj: Dashboard, many: bool = True # pylint: disable=unused-argument ) -> Dict[str, Any]: data["dttm"] = utils.json_int_dttm_ser(obj.changed_on) data["title"] = obj.dashboard_title From 963bf96f056827e44f41905799d49927af511baf Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 5 Apr 2022 15:11:47 +0100 Subject: [PATCH 07/14] fix lint --- superset/dashboards/schemas.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 8b9a6ffd950be..ae732fe44fcfd 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -235,10 +235,9 @@ class DashboardCreatedByMeResponseSchema(Schema): changed_on = fields.DateTime() dttm = fields.Int() - @staticmethod @post_dump(pass_original=True) def post_dump( - data: Dict[str, Any], obj: Dashboard, many: bool = True # pylint: disable=unused-argument + self, data: Dict[str, Any], obj: Dashboard, many: bool = True # pylint: disable=unused-argument,no-self-use ) -> Dict[str, Any]: data["dttm"] = utils.json_int_dttm_ser(obj.changed_on) data["title"] = obj.dashboard_title From 03267d3bdbb171affb9369965205c1858160bbcb Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 5 Apr 2022 15:56:45 +0100 Subject: [PATCH 08/14] lint --- superset/dashboards/schemas.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index ae732fe44fcfd..2c3b89b268abb 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -235,9 +235,10 @@ class DashboardCreatedByMeResponseSchema(Schema): changed_on = fields.DateTime() dttm = fields.Int() + # pylint: disable=unused-argument,no-self-use @post_dump(pass_original=True) def post_dump( - self, data: Dict[str, Any], obj: Dashboard, many: bool = True # pylint: disable=unused-argument,no-self-use + self, data: Dict[str, Any], obj: Dashboard, many: bool = True ) -> Dict[str, Any]: data["dttm"] = utils.json_int_dttm_ser(obj.changed_on) data["title"] = obj.dashboard_title From f9c7f74920f5ef72201cfcb512022699a3ef020a Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 6 Apr 2022 10:22:56 +0100 Subject: [PATCH 09/14] fix tests --- tests/integration_tests/dashboards/api_tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 9cfe82122e46c..204545d5e9119 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -17,6 +17,7 @@ # isort:skip_file """Unit tests for Superset""" import json +from datetime import datetime from io import BytesIO from typing import List, Optional from unittest.mock import patch @@ -697,7 +698,6 @@ def test_gets_not_certified_dashboards_filter(self): self.assertEqual(data["count"], 6) @pytest.mark.usefixtures("create_created_by_admin_dashboards") - @pytest.mark.usefixtures("create_dashboards") def test_get_dashboards_created_by_me(self): """ Dashboard API: Test get dashboards created by current user @@ -715,12 +715,12 @@ def test_get_dashboards_created_by_me(self): "title", "url", ] - if backend() == "sqlite": + if backend() == "postgres": # sqlite doesn't support ms timestamps not possible to infer proper ordering return expected_results = [ - {"title": "create_title0", "url": "/superset/dashboard/create_slug0/"}, {"title": "create_title1", "url": "/superset/dashboard/create_slug1/"}, + {"title": "create_title0", "url": "/superset/dashboard/create_slug0/"}, ] for idx, response_item in enumerate(data["result"]): for key, value in expected_results[idx].items(): From 5f1ba5f093f707cecd70127f4cc4c2b2df8eb1a6 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 6 Apr 2022 12:56:03 +0100 Subject: [PATCH 10/14] fix tests --- tests/integration_tests/dashboards/api_tests.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 204545d5e9119..c9aed8f2a25cd 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -17,8 +17,8 @@ # isort:skip_file """Unit tests for Superset""" import json -from datetime import datetime from io import BytesIO +from time import sleep from typing import List, Optional from unittest.mock import patch from zipfile import is_zipfile, ZipFile @@ -28,7 +28,6 @@ import pytest import prison import yaml -from sqlalchemy.sql import func from freezegun import freeze_time from sqlalchemy import and_ @@ -173,6 +172,7 @@ def create_created_by_admin_dashboards(self): [admin.id], created_by=admin, ) + sleep(1) dashboards.append(dashboard) yield dashboards @@ -695,7 +695,7 @@ def test_gets_not_certified_dashboards_filter(self): rv = self.get_assert_metric(uri, "get_list") self.assertEqual(rv.status_code, 200) data = json.loads(rv.data.decode("utf-8")) - self.assertEqual(data["count"], 6) + self.assertEqual(data["count"], 5) @pytest.mark.usefixtures("create_created_by_admin_dashboards") def test_get_dashboards_created_by_me(self): @@ -715,9 +715,6 @@ def test_get_dashboards_created_by_me(self): "title", "url", ] - if backend() == "postgres": - # sqlite doesn't support ms timestamps not possible to infer proper ordering - return expected_results = [ {"title": "create_title1", "url": "/superset/dashboard/create_slug1/"}, {"title": "create_title0", "url": "/superset/dashboard/create_slug0/"}, From 58d3df7b2d6bee4d700183d0226d58e34b27e506 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 7 Apr 2022 11:09:09 +0100 Subject: [PATCH 11/14] use dashboards get list instead --- .../src/profile/components/CreatedContent.tsx | 19 ++++++-- superset-frontend/src/types/bootstrapTypes.ts | 8 +++- superset/dashboards/api.py | 47 ++++--------------- superset/dashboards/dao.py | 19 -------- superset/dashboards/filters.py | 13 +++++ superset/models/helpers.py | 8 ++++ .../integration_tests/dashboards/api_tests.py | 28 +++++++---- 7 files changed, 69 insertions(+), 73 deletions(-) diff --git a/superset-frontend/src/profile/components/CreatedContent.tsx b/superset-frontend/src/profile/components/CreatedContent.tsx index df2feeee76b2f..61dfb418f09c3 100644 --- a/superset-frontend/src/profile/components/CreatedContent.tsx +++ b/superset-frontend/src/profile/components/CreatedContent.tsx @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +import rison from 'rison'; import React from 'react'; import moment from 'moment'; import { t } from '@superset-ui/core'; @@ -49,17 +50,27 @@ class CreatedContent extends React.PureComponent { } renderDashboardTable() { + const search = [{ col: 'created_by', opr: 'created_by_me', value: 'me' }]; + const query = rison.encode({ + keys: ['none'], + columns: ['created_on_delta_humanized', 'dashboard_title', 'url'], + filters: search, + order_column: 'changed_on', + order_direction: 'desc', + page: 0, + page_size: 100, + }); const mutator = (data: DashboardResponse) => data.result.map(dash => ({ - dashboard: {dash.title}, - created: moment.utc(dash.dttm).fromNow(), - _created: dash.dttm, + dashboard: {dash.dashboard_title}, + created: dash.created_on_delta_humanized, + _created: dash.created_on_delta_humanized, })); return ( Optional[Response]: "changed_by_url", "changed_on_utc", "changed_on_delta_humanized", + "created_on_delta_humanized", "created_by.first_name", "created_by.id", "created_by.last_name", @@ -181,13 +183,14 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "roles.name", "is_managed_externally", ] - list_select_columns = list_columns + ["changed_on", "changed_by_fk"] + list_select_columns = list_columns + ["changed_on", "created_on", "changed_by_fk"] order_columns = [ "changed_by.first_name", "changed_on_delta_humanized", "created_by.first_name", "dashboard_title", "published", + "changed_on", ] add_columns = [ @@ -217,6 +220,7 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: search_filters = { "dashboard_title": [DashboardTitleOrSlugFilter], "id": [DashboardFavoriteFilter, DashboardCertifiedFilter], + "created_by": [DashboardCreatedByMeFilter], } base_order = ("changed_on", "desc") @@ -229,7 +233,9 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: embedded_config_schema = EmbeddedDashboardConfigSchema() dashboard_created_by_schema = DashboardCreatedByMeResponseSchema() - base_filters = [["id", DashboardAccessFilter, lambda: []]] + base_filters = [ + ["id", DashboardAccessFilter, lambda: []], + ] order_rel_fields = { "slices": ("slice_name", "asc"), @@ -453,43 +459,6 @@ def get_charts(self, id_or_slug: str) -> Response: except DashboardNotFoundError: return self.response_404() - @expose("/created_by_me/", 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 created_by_me(self) -> Response: - """Gets all dashboards created by the current user - --- - get: - description: >- - Gets all dashboards created by the current user - responses: - 200: - description: Dashboard - content: - application/json: - schema: - type: object - properties: - result: - type: array - items: - $ref: '#/components/schemas/DashboardCreatedByMeResponseSchema' - 401: - $ref: '#/components/responses/401' - 403: - $ref: '#/components/responses/403' - 404: - $ref: '#/components/responses/404' - """ - dashboards = DashboardDAO.get_dashboards_created_by(g.user.id) - result = self.dashboard_created_by_schema.dump(dashboards, many=True) - return self.response(200, result=result) - @expose("/", methods=["POST"]) @protect() @safe diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index aedf9f8cd9a51..e2ff155fbc62c 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -101,25 +101,6 @@ def get_dashboard_and_slices_changed_on( # pylint: disable=invalid-name # drop microseconds in datetime to match with last_modified header return max(dashboard_changed_on, slices_changed_on).replace(microsecond=0) - @staticmethod - def get_dashboards_created_by(user_id: int) -> List[Dashboard]: - """ - Gets a list of dashboards that were created or changed by a certain user - :param user_id: The user id - :return: List of dashboards - """ - qry = ( - db.session.query(Dashboard) - .filter( # pylint: disable=comparison-with-callable - or_( - Dashboard.created_by_fk == user_id, - Dashboard.changed_by_fk == user_id, - ) - ) - .order_by(Dashboard.changed_on.desc()) - ) - return qry.all() - @staticmethod def get_dashboard_and_datasets_changed_on( # pylint: disable=invalid-name id_or_slug_or_dashboard: Union[str, Dashboard] diff --git a/superset/dashboards/filters.py b/superset/dashboards/filters.py index 52a945ca41a62..4805783dd9540 100644 --- a/superset/dashboards/filters.py +++ b/superset/dashboards/filters.py @@ -49,6 +49,19 @@ def apply(self, query: Query, value: Any) -> Query: ) +class DashboardCreatedByMeFilter(BaseFilter): # pylint: disable=too-few-public-methods + name = _("Created by me") + arg_name = "created_by_me" + + def apply(self, query: Query, value: Any) -> Query: + return query.filter( # pylint: disable=comparison-with-callable + or_( + Dashboard.created_by_fk == g.user.get_user_id(), + Dashboard.changed_by_fk == g.user.get_user_id(), + ) + ) + + class DashboardFavoriteFilter( # pylint: disable=too-few-public-methods BaseFavoriteFilter ): diff --git a/superset/models/helpers.py b/superset/models/helpers.py index 86ac2c1a98717..baa0566c01119 100644 --- a/superset/models/helpers.py +++ b/superset/models/helpers.py @@ -420,6 +420,10 @@ def changed_on_(self) -> Markup: def changed_on_delta_humanized(self) -> str: return self.changed_on_humanized + @renders("created_on") + def created_on_delta_humanized(self) -> str: + return self.created_on_humanized + @renders("changed_on") def changed_on_utc(self) -> str: # Convert naive datetime to UTC @@ -429,6 +433,10 @@ def changed_on_utc(self) -> str: def changed_on_humanized(self) -> str: return humanize.naturaltime(datetime.now() - self.changed_on) + @property + def created_on_humanized(self) -> str: + return humanize.naturaltime(datetime.now() - self.created_on) + @renders("changed_on") def modified(self) -> Markup: return Markup(f'{self.changed_on_humanized}') diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index c9aed8f2a25cd..66b498eb35136 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -702,22 +702,30 @@ def test_get_dashboards_created_by_me(self): """ Dashboard API: Test get dashboards created by current user """ - uri = f"api/v1/dashboard/created_by_me/" + query = { + "columns": ["created_on_delta_humanized", "dashboard_title", "url"], + "filters": [{"col": "created_by", "opr": "created_by_me", "value": "me"}], + "order_column": "changed_on", + "order_direction": "desc", + "page": 0, + "page_size": 100, + } + uri = f"api/v1/dashboard/?q={prison.dumps(query)}" self.login(username="admin") rv = self.client.get(uri) data = json.loads(rv.data.decode("utf-8")) assert rv.status_code == 200 assert len(data["result"]) == 2 - assert list(data["result"][0].keys()) == [ - "changed_on", - "dttm", - "id", - "title", - "url", - ] + assert list(data["result"][0].keys()) == query["columns"] expected_results = [ - {"title": "create_title1", "url": "/superset/dashboard/create_slug1/"}, - {"title": "create_title0", "url": "/superset/dashboard/create_slug0/"}, + { + "dashboard_title": "create_title1", + "url": "/superset/dashboard/create_slug1/", + }, + { + "dashboard_title": "create_title0", + "url": "/superset/dashboard/create_slug0/", + }, ] for idx, response_item in enumerate(data["result"]): for key, value in expected_results[idx].items(): From 94b70738532e404722cd7321a2d4ef3ad25d0cb1 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 7 Apr 2022 11:12:33 +0100 Subject: [PATCH 12/14] clean unnecessary marshmallow schema --- superset/dashboards/api.py | 3 --- superset/dashboards/schemas.py | 17 ----------------- 2 files changed, 20 deletions(-) diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index e2e917df60edd..fb9c36ca033b8 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -64,7 +64,6 @@ FilterRelatedRoles, ) from superset.dashboards.schemas import ( - DashboardCreatedByMeResponseSchema, DashboardDatasetSchema, DashboardGetResponseSchema, DashboardPostSchema, @@ -231,7 +230,6 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: dashboard_dataset_schema = DashboardDatasetSchema() embedded_response_schema = EmbeddedDashboardResponseSchema() embedded_config_schema = EmbeddedDashboardConfigSchema() - dashboard_created_by_schema = DashboardCreatedByMeResponseSchema() base_filters = [ ["id", DashboardAccessFilter, lambda: []], @@ -253,7 +251,6 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: """ Override the name set for this collection of endpoints """ openapi_spec_component_schemas = ( ChartEntityResponseSchema, - DashboardCreatedByMeResponseSchema, DashboardGetResponseSchema, DashboardDatasetSchema, GetFavStarIdsSchema, diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 2c3b89b268abb..4adef24e9f0d2 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -228,23 +228,6 @@ def post_load(self, data: Dict[str, Any], **kwargs: Any) -> Dict[str, Any]: return data -class DashboardCreatedByMeResponseSchema(Schema): - id = fields.Int() - title = fields.Str() - url = fields.Str() - changed_on = fields.DateTime() - dttm = fields.Int() - - # pylint: disable=unused-argument,no-self-use - @post_dump(pass_original=True) - def post_dump( - self, data: Dict[str, Any], obj: Dashboard, many: bool = True - ) -> Dict[str, Any]: - data["dttm"] = utils.json_int_dttm_ser(obj.changed_on) - data["title"] = obj.dashboard_title - return data - - class DashboardPostSchema(BaseDashboardSchema): dashboard_title = fields.String( description=dashboard_title_description, From fa473f7ac926f8050f8cad2dbf470afaea6bc768 Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Thu, 7 Apr 2022 11:14:02 +0100 Subject: [PATCH 13/14] Update superset/views/core.py Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> --- superset/views/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/views/core.py b/superset/views/core.py index ac111ea642d9c..0f5b88887cf9d 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1588,7 +1588,7 @@ def fave_dashboards(self, user_id: int) -> FlaskResponse: @expose("/created_dashboards//", methods=["GET"]) def created_dashboards(self, user_id: int) -> FlaskResponse: logging.warning( - "%s.select_star " + "%s.created_dashboards " "This API endpoint is deprecated and will be removed in version 3.0.0", self.__class__.__name__, ) From 585e47dddc0045f75b23549dd373d17448bd5d31 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 7 Apr 2022 13:10:15 +0100 Subject: [PATCH 14/14] lint --- superset/dashboards/dao.py | 1 - superset/dashboards/filters.py | 8 +++++--- superset/dashboards/schemas.py | 3 +-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index e2ff155fbc62c..ce6e30f8d4beb 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -19,7 +19,6 @@ from datetime import datetime from typing import Any, Dict, List, Optional, Union -from sqlalchemy import or_ from sqlalchemy.exc import SQLAlchemyError from superset import security_manager diff --git a/superset/dashboards/filters.py b/superset/dashboards/filters.py index 4805783dd9540..3bbef14f4cb0e 100644 --- a/superset/dashboards/filters.py +++ b/superset/dashboards/filters.py @@ -54,10 +54,12 @@ class DashboardCreatedByMeFilter(BaseFilter): # pylint: disable=too-few-public- arg_name = "created_by_me" def apply(self, query: Query, value: Any) -> Query: - return query.filter( # pylint: disable=comparison-with-callable + return query.filter( or_( - Dashboard.created_by_fk == g.user.get_user_id(), - Dashboard.changed_by_fk == g.user.get_user_id(), + Dashboard.created_by_fk # pylint: disable=comparison-with-callable + == g.user.get_user_id(), + Dashboard.changed_by_fk # pylint: disable=comparison-with-callable + == g.user.get_user_id(), ) ) diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 4adef24e9f0d2..d91879f0d88b3 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -18,11 +18,10 @@ import re from typing import Any, Dict, Union -from marshmallow import fields, post_dump, post_load, Schema +from marshmallow import fields, post_load, Schema from marshmallow.validate import Length, ValidationError from superset.exceptions import SupersetException -from superset.models.dashboard import Dashboard from superset.utils import core as utils get_delete_ids_schema = {"type": "array", "items": {"type": "integer"}}