From d6d2777ada0768682fde7f32cd7e49ec6b0203f2 Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Fri, 8 Apr 2022 09:05:22 +0100 Subject: [PATCH] feat: deprecate old API and create new API for dashes created by me (#19434) * feat: deprecate old API and create new API for dashes created by me * add tests * fix previous test * fix test and lint * fix sqlite test * fix lint * fix lint * lint * fix tests * fix tests * use dashboards get list instead * clean unnecessary marshmallow schema * Update superset/views/core.py Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> * lint Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> --- .../src/profile/components/CreatedContent.tsx | 25 +++++--- superset-frontend/src/types/bootstrapTypes.ts | 10 ++++ superset/constants.py | 1 + superset/dashboards/api.py | 19 +++--- superset/dashboards/filters.py | 15 +++++ superset/models/helpers.py | 8 +++ superset/views/core.py | 16 +++-- tests/integration_tests/dashboard_tests.py | 14 ++++- .../integration_tests/dashboards/api_tests.py | 59 ++++++++++++++++++- 9 files changed, 143 insertions(+), 24 deletions(-) diff --git a/superset-frontend/src/profile/components/CreatedContent.tsx b/superset-frontend/src/profile/components/CreatedContent.tsx index b097dee5e1448..61dfb418f09c3 100644 --- a/superset-frontend/src/profile/components/CreatedContent.tsx +++ b/superset-frontend/src/profile/components/CreatedContent.tsx @@ -16,13 +16,14 @@ * 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'; 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,17 +50,27 @@ class CreatedContent extends React.PureComponent { } renderDashboardTable() { - const mutator = (data: Dashboard[]) => - data.map(dash => ({ - dashboard: {dash.title}, - created: moment.utc(dash.dttm).fromNow(), - _created: dash.dttm, + 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.dashboard_title}, + created: dash.created_on_delta_humanized, + _created: dash.created_on_delta_humanized, })); return ( Optional[Response]: "set_embedded", "delete_embedded", "thumbnail", + "created_by_me", } resource_name = "dashboard" allow_browser_login = True @@ -166,6 +168,7 @@ def ensure_thumbnails_enabled(self) -> 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", @@ -179,13 +182,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 = [ @@ -215,6 +219,7 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: search_filters = { "dashboard_title": [DashboardTitleOrSlugFilter], "id": [DashboardFavoriteFilter, DashboardCertifiedFilter], + "created_by": [DashboardCreatedByMeFilter], } base_order = ("changed_on", "desc") @@ -226,7 +231,9 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: embedded_response_schema = EmbeddedDashboardResponseSchema() embedded_config_schema = EmbeddedDashboardConfigSchema() - base_filters = [["id", DashboardAccessFilter, lambda: []]] + base_filters = [ + ["id", DashboardAccessFilter, lambda: []], + ] order_rel_fields = { "slices": ("slice_name", "asc"), @@ -307,8 +314,6 @@ def get(self, dash: Dashboard) -> Response: properties: result: $ref: '#/components/schemas/DashboardGetResponseSchema' - 302: - description: Redirects to the current digest 400: $ref: '#/components/responses/400' 401: @@ -364,8 +369,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: @@ -427,8 +430,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: @@ -489,8 +490,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/filters.py b/superset/dashboards/filters.py index 52a945ca41a62..3bbef14f4cb0e 100644 --- a/superset/dashboards/filters.py +++ b/superset/dashboards/filters.py @@ -49,6 +49,21 @@ 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( + or_( + 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(), + ) + ) + + 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/superset/views/core.py b/superset/views/core.py index f806ea7b0f466..0f5b88887cf9d 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1587,16 +1587,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.created_dashboards " + "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 = [ { 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"): diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index a179fa7c7e453..66b498eb35136 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -18,6 +18,7 @@ """Unit tests for Superset""" import json from io import BytesIO +from time import sleep from typing import List, Optional from unittest.mock import patch from zipfile import is_zipfile, ZipFile @@ -27,7 +28,6 @@ import pytest import prison import yaml -from sqlalchemy.sql import func from freezegun import freeze_time from sqlalchemy import and_ @@ -160,6 +160,27 @@ 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, + ) + sleep(1) + 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(): @@ -674,7 +695,41 @@ 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): + """ + Dashboard API: Test get dashboards created by current user + """ + 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()) == query["columns"] + expected_results = [ + { + "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(): + assert response_item[key] == value def create_dashboard_import(self): buf = BytesIO()