diff --git a/superset-frontend/src/profile/components/RecentActivity.tsx b/superset-frontend/src/profile/components/RecentActivity.tsx index 4c1e81cb7..39d7b7d11 100644 --- a/superset-frontend/src/profile/components/RecentActivity.tsx +++ b/superset-frontend/src/profile/components/RecentActivity.tsx @@ -18,9 +18,10 @@ */ import React from 'react'; import moment from 'moment'; +import rison from 'rison'; import TableLoader from '../../components/TableLoader'; -import { Activity } from '../types'; +import { ActivityResult } from '../types'; import { BootstrapUser } from '../../types/bootstrapTypes'; interface RecentActivityProps { @@ -29,8 +30,8 @@ interface RecentActivityProps { export default function RecentActivity({ user }: RecentActivityProps) { const rowLimit = 50; - const mutator = function (data: Activity[]) { - return data + const mutator = function (data: ActivityResult) { + return data.result .filter(row => row.action === 'dashboard' || row.action === 'explore') .map(row => ({ name: {row.item_title}, @@ -39,13 +40,14 @@ export default function RecentActivity({ user }: RecentActivityProps) { _time: row.time, })); }; + const params = rison.encode({ page_size: rowLimit }); return (
); diff --git a/superset-frontend/src/profile/types.ts b/superset-frontend/src/profile/types.ts index a2433e082..29ca01605 100644 --- a/superset-frontend/src/profile/types.ts +++ b/superset-frontend/src/profile/types.ts @@ -32,3 +32,7 @@ export type Activity = { item_url: string; time: number; }; + +export type ActivityResult = { + result: Activity[]; +}; diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx index bbb985609..07794f960 100644 --- a/superset-frontend/src/views/CRUD/utils.tsx +++ b/superset-frontend/src/views/CRUD/utils.tsx @@ -197,7 +197,7 @@ export const getRecentAcitivtyObjs = ( return Promise.all(newBatch) .then(([chartRes, dashboardRes]) => { res.other = [...chartRes.json.result, ...dashboardRes.json.result]; - res.viewed = recentsRes.json; + res.viewed = recentsRes.json.result; return res; }) .catch(errMsg => diff --git a/superset-frontend/src/views/CRUD/welcome/ActivityTable.tsx b/superset-frontend/src/views/CRUD/welcome/ActivityTable.tsx index d6dc3858a..302a32e7f 100644 --- a/superset-frontend/src/views/CRUD/welcome/ActivityTable.tsx +++ b/superset-frontend/src/views/CRUD/welcome/ActivityTable.tsx @@ -38,7 +38,7 @@ import EmptyState from './EmptyState'; import { WelcomeTable } from './types'; /** - * Return result from /superset/recent_activity/{user_id} + * Return result from /api/v1/log/recent_activity/{user_id}/ */ interface RecentActivity { action: string; diff --git a/superset-frontend/src/views/CRUD/welcome/Welcome.test.tsx b/superset-frontend/src/views/CRUD/welcome/Welcome.test.tsx index 5c693ab26..dffe5acdd 100644 --- a/superset-frontend/src/views/CRUD/welcome/Welcome.test.tsx +++ b/superset-frontend/src/views/CRUD/welcome/Welcome.test.tsx @@ -43,7 +43,7 @@ const dashboardFavoriteStatusEndpoint = 'glob:*/api/v1/dashboard/favorite_status?*'; const savedQueryEndpoint = 'glob:*/api/v1/saved_query/?*'; const savedQueryInfoEndpoint = 'glob:*/api/v1/saved_query/_info?*'; -const recentActivityEndpoint = 'glob:*/superset/recent_activity/*'; +const recentActivityEndpoint = 'glob:*/api/v1/log/recent_activity/*'; fetchMock.get(chartsEndpoint, { result: [ @@ -142,7 +142,7 @@ describe('Welcome with sql role', () => { it('calls api methods in parallel on page load', () => { const chartCall = fetchMock.calls(/chart\/\?q/); const savedQueryCall = fetchMock.calls(/saved_query\/\?q/); - const recentCall = fetchMock.calls(/superset\/recent_activity\/*/); + const recentCall = fetchMock.calls(/api\/v1\/log\/recent_activity\/*/); const dashboardCall = fetchMock.calls(/dashboard\/\?q/); expect(chartCall).toHaveLength(2); expect(recentCall).toHaveLength(1); @@ -186,7 +186,7 @@ describe('Welcome without sql role', () => { it('calls api methods in parallel on page load', () => { const chartCall = fetchMock.calls(/chart\/\?q/); const savedQueryCall = fetchMock.calls(/saved_query\/\?q/); - const recentCall = fetchMock.calls(/superset\/recent_activity\/*/); + const recentCall = fetchMock.calls(/api\/v1\/log\/recent_activity\/*/); const dashboardCall = fetchMock.calls(/dashboard\/\?q/); expect(chartCall).toHaveLength(2); expect(recentCall).toHaveLength(1); diff --git a/superset-frontend/src/views/CRUD/welcome/Welcome.tsx b/superset-frontend/src/views/CRUD/welcome/Welcome.tsx index b3dd41125..bede59457 100644 --- a/superset-frontend/src/views/CRUD/welcome/Welcome.tsx +++ b/superset-frontend/src/views/CRUD/welcome/Welcome.tsx @@ -23,6 +23,7 @@ import { styled, t, } from '@superset-ui/core'; +import rison from 'rison'; import Collapse from 'src/components/Collapse'; import { User } from 'src/types/bootstrapTypes'; import { reject } from 'lodash'; @@ -165,7 +166,8 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { const canAccessSqlLab = canUserAccessSqlLab(user); const userid = user.userId; const id = userid!.toString(); // confident that user is not a guest user - const recent = `/superset/recent_activity/${user.userId}/?limit=6`; + const params = rison.encode({ page_size: 6 }); + const recent = `/api/v1/log/recent_activity/${user.userId}/?q=${params}`; const [activeChild, setActiveChild] = useState('Loading'); const userKey = dangerouslyGetItemDoNotUse(id, null); let defaultChecked = false; diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py index 106e92fe4..0e0bf56f5 100644 --- a/superset/models/dashboard.py +++ b/superset/models/dashboard.py @@ -20,7 +20,7 @@ import json import logging from collections import defaultdict from functools import partial -from typing import Any, Callable, Dict, List, Set, Tuple, Type, Union +from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Type, Union import sqlalchemy as sqla from flask_appbuilder import Model @@ -177,6 +177,11 @@ class Dashboard(Model, AuditMixinNullable, ImportExportMixin): def url(self) -> str: return f"/superset/dashboard/{self.slug or self.id}/" + @staticmethod + def get_url(id_: int, slug: Optional[str] = None) -> str: + # To be able to generate URL's without instanciating a Dashboard object + return f"/superset/dashboard/{slug or id_}/" + @property def datasources(self) -> Set[BaseDatasource]: # Verbose but efficient database enumeration of dashboard datasources. diff --git a/superset/models/slice.py b/superset/models/slice.py index 657ff7d38..e12f9a742 100644 --- a/superset/models/slice.py +++ b/superset/models/slice.py @@ -284,12 +284,18 @@ class Slice( # pylint: disable=too-many-public-methods self, base_url: str = "/explore", overrides: Optional[Dict[str, Any]] = None, + ) -> str: + return self.build_explore_url(self.id, base_url, overrides) + + @staticmethod + def build_explore_url( + id_: int, base_url: str = "/explore", overrides: Optional[Dict[str, Any]] = None ) -> str: overrides = overrides or {} - form_data = {"slice_id": self.id} + form_data = {"slice_id": id_} form_data.update(overrides) params = parse.quote(json.dumps(form_data)) - return f"{base_url}/?slice_id={self.id}&form_data={params}" + return f"{base_url}/?slice_id={id_}&form_data={params}" @property def slice_url(self) -> str: diff --git a/superset/views/core.py b/superset/views/core.py index b9e198102..d65023d60 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -20,12 +20,11 @@ from __future__ import annotations import logging import re from contextlib import closing -from datetime import datetime, timedelta +from datetime import datetime from typing import Any, Callable, cast, Dict, List, Optional, Union from urllib import parse import backoff -import humanize import pandas as pd import simplejson as json from flask import abort, flash, g, redirect, render_template, request, Response @@ -41,7 +40,6 @@ from flask_babel import gettext as __, lazy_gettext as _ from sqlalchemy import and_, or_ from sqlalchemy.exc import DBAPIError, NoSuchModuleError, SQLAlchemyError from sqlalchemy.orm.session import Session -from sqlalchemy.sql import functions as func from superset import ( app, @@ -98,7 +96,7 @@ from superset.explore.permalink.commands.get import GetExplorePermalinkCommand from superset.explore.permalink.exceptions import ExplorePermalinkGetFailedError from superset.extensions import async_query_manager, cache_manager from superset.jinja_context import get_template_processor -from superset.models.core import Database, FavStar, Log +from superset.models.core import Database, FavStar from superset.models.dashboard import Dashboard from superset.models.datasource_access_request import DatasourceAccessRequest from superset.models.slice import Slice @@ -155,6 +153,7 @@ from superset.views.base import ( json_success, validate_sqlatable, ) +from superset.views.log.dao import LogDAO from superset.views.sql_lab.schemas import SqlJsonPayloadSchema from superset.views.utils import ( _deserialize_results_payload, @@ -1438,9 +1437,8 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods @has_access_api @event_logger.log_this @expose("/recent_activity//", methods=["GET"]) - def recent_activity( # pylint: disable=too-many-locals - self, user_id: int - ) -> FlaskResponse: + @deprecated() + def recent_activity(self, user_id: int) -> FlaskResponse: """Recent activity (actions) for a given user""" error_obj = self.get_user_activity_access_error(user_id) if error_obj: @@ -1452,96 +1450,8 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods # whether to get distinct subjects distinct = request.args.get("distinct") != "false" - has_subject_title = or_( - and_( - Dashboard.dashboard_title is not None, - Dashboard.dashboard_title != "", - ), - and_(Slice.slice_name is not None, Slice.slice_name != ""), - ) + payload = LogDAO.get_recent_activity(user_id, actions, distinct, 0, limit) - if distinct: - one_year_ago = datetime.today() - timedelta(days=365) - subqry = ( - db.session.query( - Log.dashboard_id, - Log.slice_id, - Log.action, - func.max(Log.dttm).label("dttm"), - ) - .group_by(Log.dashboard_id, Log.slice_id, Log.action) - .filter( - and_( - Log.action.in_(actions), - Log.user_id == user_id, - # limit to one year of data to improve performance - Log.dttm > one_year_ago, - or_(Log.dashboard_id.isnot(None), Log.slice_id.isnot(None)), - ) - ) - .subquery() - ) - qry = ( - db.session.query( - subqry, - Dashboard.slug.label("dashboard_slug"), - Dashboard.dashboard_title, - Slice.slice_name, - ) - .outerjoin(Dashboard, Dashboard.id == subqry.c.dashboard_id) - .outerjoin( - Slice, - Slice.id == subqry.c.slice_id, - ) - .filter(has_subject_title) - .order_by(subqry.c.dttm.desc()) - .limit(limit) - ) - else: - qry = ( - db.session.query( - Log.dttm, - Log.action, - Log.dashboard_id, - Log.slice_id, - Dashboard.slug.label("dashboard_slug"), - Dashboard.dashboard_title, - Slice.slice_name, - ) - .outerjoin(Dashboard, Dashboard.id == Log.dashboard_id) - .outerjoin(Slice, Slice.id == Log.slice_id) - .filter(has_subject_title) - .order_by(Log.dttm.desc()) - .limit(limit) - ) - - payload = [] - for log in qry.all(): - item_url = None - item_title = None - item_type = None - if log.dashboard_id: - item_type = "dashboard" - item_url = Dashboard(id=log.dashboard_id, slug=log.dashboard_slug).url - item_title = log.dashboard_title - elif log.slice_id: - slc = Slice(id=log.slice_id, slice_name=log.slice_name) - item_type = "slice" - item_url = slc.slice_url - item_title = slc.chart - - payload.append( - { - "action": log.action, - "item_type": item_type, - "item_url": item_url, - "item_title": item_title, - "time": log.dttm, - "time_delta_humanized": humanize.naturaltime( - datetime.now() - log.dttm - ), - } - ) return json_success(json.dumps(payload, default=utils.json_int_dttm_ser)) @api diff --git a/superset/views/log/api.py b/superset/views/log/api.py index a92626df4..5c73dba4a 100644 --- a/superset/views/log/api.py +++ b/superset/views/log/api.py @@ -14,12 +14,24 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from typing import Any, Optional + from flask import current_app as app +from flask_appbuilder.api import expose, protect, rison, safe from flask_appbuilder.hooks import before_request from flask_appbuilder.models.sqla.interface import SQLAInterface import superset.models.core as models -from superset.views.base_api import BaseSupersetModelRestApi +from superset import event_logger, security_manager +from superset.exceptions import SupersetSecurityException +from superset.superset_typing import FlaskResponse +from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics +from superset.views.log.dao import LogDAO +from superset.views.log.schemas import ( + get_recent_activity_schema, + RecentActivityResponseSchema, + RecentActivitySchema, +) from ...constants import MODEL_API_RW_METHOD_PERMISSION_MAP from . import LogMixin @@ -27,7 +39,7 @@ from . import LogMixin class LogRestApi(LogMixin, BaseSupersetModelRestApi): datamodel = SQLAInterface(models.Log) - include_route_methods = {"get_list", "get", "post"} + include_route_methods = {"get_list", "get", "post", "recent_activity"} class_permission_name = "Log" method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP resource_name = "log" @@ -44,13 +56,86 @@ class LogRestApi(LogMixin, BaseSupersetModelRestApi): "referrer", ] show_columns = list_columns + page_size = 20 + apispec_parameter_schemas = { + "get_recent_activity_schema": get_recent_activity_schema, + } + openapi_spec_component_schemas = ( + RecentActivityResponseSchema, + RecentActivitySchema, + ) @staticmethod def is_enabled() -> bool: return app.config["FAB_ADD_SECURITY_VIEWS"] and app.config["SUPERSET_LOG_VIEW"] - @before_request + @before_request(only=["get_list", "get", "post"]) def ensure_enabled(self) -> None: if not self.is_enabled(): return self.response_404() return None + + def get_user_activity_access_error(self, user_id: int) -> Optional[FlaskResponse]: + try: + security_manager.raise_for_user_activity_access(user_id) + except SupersetSecurityException as ex: + return self.response(403, message=ex.message) + return None + + @expose("/recent_activity//", methods=["GET"]) + @protect() + @safe + @statsd_metrics + @rison(get_recent_activity_schema) + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" + f".recent_activity", + log_to_statsd=False, + ) + def recent_activity(self, user_id: int, **kwargs: Any) -> FlaskResponse: + """Get recent activity data for a user + --- + get: + summary: Get recent activity data for a user + parameters: + - in: path + schema: + type: integer + name: user_id + description: The id of the user + - in: query + name: q + content: + application/json: + schema: + $ref: '#/components/schemas/get_recent_activity_schema' + responses: + 200: + description: A List of recent activity objects + content: + application/json: + schema: + $ref: "#/components/schemas/RecentActivityResponseSchema" + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 403: + $ref: '#/components/responses/403' + 500: + $ref: '#/components/responses/500' + """ + error_obj = self.get_user_activity_access_error(user_id) + if error_obj: + return error_obj + + args = kwargs["rison"] + page, page_size = self._sanitize_page_args(*self._handle_page_args(args)) + actions = args.get("actions", ["explore", "dashboard"]) + distinct = args.get("distinct", True) + + payload = LogDAO.get_recent_activity( + user_id, actions, distinct, page, page_size + ) + + return self.response(200, result=payload) diff --git a/superset/views/log/dao.py b/superset/views/log/dao.py new file mode 100644 index 000000000..71d8a6234 --- /dev/null +++ b/superset/views/log/dao.py @@ -0,0 +1,131 @@ +# 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 datetime import datetime, timedelta +from typing import Any, Dict, List + +import humanize +from sqlalchemy import and_, or_ +from sqlalchemy.sql import functions as func + +from superset import db +from superset.dao.base import BaseDAO +from superset.models.core import Log +from superset.models.dashboard import Dashboard +from superset.models.slice import Slice +from superset.utils.dates import datetime_to_epoch + + +class LogDAO(BaseDAO): + model_cls = Log + + @staticmethod + def get_recent_activity( + user_id: int, actions: List[str], distinct: bool, page: int, page_size: int + ) -> List[Dict[str, Any]]: + has_subject_title = or_( + and_( + Dashboard.dashboard_title is not None, + Dashboard.dashboard_title != "", + ), + and_(Slice.slice_name is not None, Slice.slice_name != ""), + ) + + if distinct: + one_year_ago = datetime.today() - timedelta(days=365) + subqry = ( + db.session.query( + Log.dashboard_id, + Log.slice_id, + Log.action, + func.max(Log.dttm).label("dttm"), + ) + .group_by(Log.dashboard_id, Log.slice_id, Log.action) + .filter( + and_( + Log.action.in_(actions), + Log.user_id == user_id, + # limit to one year of data to improve performance + Log.dttm > one_year_ago, + or_(Log.dashboard_id.isnot(None), Log.slice_id.isnot(None)), + ) + ) + .subquery() + ) + qry = ( + db.session.query( + subqry, + Dashboard.slug.label("dashboard_slug"), + Dashboard.dashboard_title, + Slice.slice_name, + ) + .outerjoin(Dashboard, Dashboard.id == subqry.c.dashboard_id) + .outerjoin( + Slice, + Slice.id == subqry.c.slice_id, + ) + .filter(has_subject_title) + .order_by(subqry.c.dttm.desc()) + .limit(page_size) + .offset(page * page_size) + ) + else: + qry = ( + db.session.query( + Log.dttm, + Log.action, + Log.dashboard_id, + Log.slice_id, + Dashboard.slug.label("dashboard_slug"), + Dashboard.dashboard_title, + Slice.slice_name, + ) + .outerjoin(Dashboard, Dashboard.id == Log.dashboard_id) + .outerjoin(Slice, Slice.id == Log.slice_id) + .filter(has_subject_title) + .filter(Log.action.in_(actions), Log.user_id == user_id) + .order_by(Log.dttm.desc()) + .limit(page_size) + .offset(page * page_size) + ) + + payload = [] + for log in qry.all(): + item_url = None + item_title = None + item_type = None + if log.dashboard_id: + item_type = "dashboard" + item_url = Dashboard.get_url(log.dashboard_id, log.dashboard_slug) + item_title = log.dashboard_title + elif log.slice_id: + item_type = "slice" + item_url = Slice.build_explore_url(log.slice_id) + item_title = log.slice_name or "" + + payload.append( + { + "action": log.action, + "item_type": item_type, + "item_url": item_url, + "item_title": item_title, + "time": datetime_to_epoch(log.dttm), + "time_delta_humanized": humanize.naturaltime( + datetime.utcnow() - log.dttm + ), + } + ) + return payload diff --git a/superset/views/log/schemas.py b/superset/views/log/schemas.py new file mode 100644 index 000000000..bb4569893 --- /dev/null +++ b/superset/views/log/schemas.py @@ -0,0 +1,45 @@ +# 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 marshmallow import fields, Schema + +get_recent_activity_schema = { + "type": "object", + "properties": { + "page": {"type": "number"}, + "page_size": {"type": "number"}, + "actions": {"type": "array", "items": {"type": "string"}}, + "distinct": {"type": "boolean"}, + }, +} + + +class RecentActivitySchema(Schema): + action = fields.String(description="Action taken describing type of activity") + item_type = fields.String(description="Type of item, e.g. slice or dashboard") + item_url = fields.String(description="URL to item") + item_title = fields.String(description="Title of item") + time = fields.Float(description="Time of activity, in epoch milliseconds") + time_delta_humanized = fields.String( + description="Human-readable description of how long ago activity took place" + ) + + +class RecentActivityResponseSchema(Schema): + result = fields.List( + fields.Nested(RecentActivitySchema), + description="A list of recent activity objects", + ) diff --git a/tests/integration_tests/dashboard_tests.py b/tests/integration_tests/dashboard_tests.py index 973394a26..d54151db8 100644 --- a/tests/integration_tests/dashboard_tests.py +++ b/tests/integration_tests/dashboard_tests.py @@ -487,13 +487,18 @@ class TestDashboard(SupersetTestCase): hidden_dash.slices = [] hidden_dash.owners = [] - db.session.merge(dash) - db.session.merge(hidden_dash) + db.session.add(dash) + db.session.add(hidden_dash) db.session.commit() self.login(user.username) resp = self.get_resp("/api/v1/dashboard/") + + db.session.delete(dash) + db.session.delete(hidden_dash) + db.session.commit() + self.assertIn(f"/superset/dashboard/{my_dash_slug}/", resp) self.assertNotIn(f"/superset/dashboard/{not_my_dash_slug}/", resp) @@ -510,8 +515,8 @@ class TestDashboard(SupersetTestCase): regular_dash.dashboard_title = "A Plain Ol Dashboard" regular_dash.slug = regular_dash_slug - db.session.merge(favorite_dash) - db.session.merge(regular_dash) + db.session.add(favorite_dash) + db.session.add(regular_dash) db.session.commit() dash = db.session.query(Dashboard).filter_by(slug=fav_dash_slug).first() @@ -521,12 +526,18 @@ class TestDashboard(SupersetTestCase): favorites.class_name = "Dashboard" favorites.user_id = user.id - db.session.merge(favorites) + db.session.add(favorites) db.session.commit() self.login(user.username) resp = self.get_resp("/api/v1/dashboard/") + + db.session.delete(favorites) + db.session.delete(regular_dash) + db.session.delete(favorite_dash) + db.session.commit() + self.assertIn(f"/superset/dashboard/{fav_dash_slug}/", resp) def test_user_can_not_view_unpublished_dash(self): @@ -541,12 +552,16 @@ class TestDashboard(SupersetTestCase): dash.owners = [admin_user] dash.slices = [] dash.published = False - db.session.merge(dash) + db.session.add(dash) db.session.commit() # list dashboards as a gamma user self.login(gamma_user.username) resp = self.get_resp("/api/v1/dashboard/") + + db.session.delete(dash) + db.session.commit() + self.assertNotIn(f"/superset/dashboard/{slug}/", resp) diff --git a/tests/integration_tests/dashboard_utils.py b/tests/integration_tests/dashboard_utils.py index 115d3269f..bea724daf 100644 --- a/tests/integration_tests/dashboard_utils.py +++ b/tests/integration_tests/dashboard_utils.py @@ -80,8 +80,9 @@ def create_dashboard( slug: str, title: str, position: str, slices: List[Slice] ) -> Dashboard: dash = db.session.query(Dashboard).filter_by(slug=slug).one_or_none() - if not dash: - dash = Dashboard() + if dash: + return dash + dash = Dashboard() dash.dashboard_title = title if position is not None: js = position @@ -90,7 +91,7 @@ def create_dashboard( dash.slug = slug if slices is not None: dash.slices = slices - db.session.merge(dash) + db.session.add(dash) db.session.commit() return dash diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 7288889bf..e6ff9f583 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -756,7 +756,7 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi 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"], 5) + self.assertEqual(data["count"], 0) @pytest.mark.usefixtures("create_created_by_gamma_dashboards") def test_get_dashboards_created_by_me(self): diff --git a/tests/integration_tests/dashboards/security/security_dataset_tests.py b/tests/integration_tests/dashboards/security/security_dataset_tests.py index 34a5fedad..2eafc4b53 100644 --- a/tests/integration_tests/dashboards/security/security_dataset_tests.py +++ b/tests/integration_tests/dashboards/security/security_dataset_tests.py @@ -137,8 +137,8 @@ class TestDashboardDatasetSecurity(DashboardTestCase): regular_dash.dashboard_title = "A Plain Ol Dashboard" regular_dash.slug = regular_dash_slug - db.session.merge(favorite_dash) - db.session.merge(regular_dash) + db.session.add(favorite_dash) + db.session.add(regular_dash) db.session.commit() dash = db.session.query(Dashboard).filter_by(slug=fav_dash_slug).first() @@ -148,7 +148,7 @@ class TestDashboardDatasetSecurity(DashboardTestCase): favorites.class_name = "Dashboard" favorites.user_id = user.id - db.session.merge(favorites) + db.session.add(favorites) db.session.commit() self.login(user.username) @@ -156,6 +156,12 @@ class TestDashboardDatasetSecurity(DashboardTestCase): # act get_dashboards_response = self.get_resp(DASHBOARDS_API_URL) + # cleanup + db.session.delete(favorites) + db.session.delete(favorite_dash) + db.session.delete(regular_dash) + db.session.commit() + # assert self.assertIn(f"/superset/dashboard/{fav_dash_slug}/", get_dashboards_response) diff --git a/tests/integration_tests/log_api_tests.py b/tests/integration_tests/log_api_tests.py index 089ac0792..83a7f5fd8 100644 --- a/tests/integration_tests/log_api_tests.py +++ b/tests/integration_tests/log_api_tests.py @@ -16,16 +16,20 @@ # under the License. # isort:skip_file """Unit tests for Superset""" +from datetime import datetime, timedelta import json from typing import Optional +from unittest.mock import ANY -import prison from flask_appbuilder.security.sqla.models import User +import prison from unittest.mock import patch from superset import db from superset.models.core import Log from superset.views.log.api import LogRestApi +from tests.integration_tests.dashboard_utils import create_dashboard +from tests.integration_tests.test_app import app from .base_tests import SupersetTestCase @@ -106,6 +110,8 @@ class TestLogApi(SupersetTestCase): self.login(username="alpha") rv = self.client.get(uri) self.assertEqual(rv.status_code, 403) + db.session.delete(log) + db.session.commit() def test_get_item(self): """ @@ -152,3 +158,178 @@ class TestLogApi(SupersetTestCase): self.assertEqual(rv.status_code, 405) db.session.delete(log) db.session.commit() + + def test_get_recent_activity_no_broad_access(self): + """ + Log API: Test recent activity not visible for other users without + ENABLE_BROAD_ACTIVITY_ACCESS flag on + """ + admin_user = self.get_user("admin") + self.login(username="admin") + app.config["ENABLE_BROAD_ACTIVITY_ACCESS"] = False + + uri = f"api/v1/log/recent_activity/{admin_user.id + 1}/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 403) + app.config["ENABLE_BROAD_ACTIVITY_ACCESS"] = True + + def test_get_recent_activity(self): + """ + Log API: Test recent activity endpoint + """ + admin_user = self.get_user("admin") + self.login(username="admin") + dash = create_dashboard("dash_slug", "dash_title", "{}", []) + log1 = self.insert_log("dashboard", admin_user, dashboard_id=dash.id) + log2 = self.insert_log("dashboard", admin_user, dashboard_id=dash.id) + + uri = f"api/v1/log/recent_activity/{admin_user.id}/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 200) + response = json.loads(rv.data.decode("utf-8")) + + db.session.delete(log1) + db.session.delete(log2) + db.session.delete(dash) + db.session.commit() + + self.assertEqual( + response, + { + "result": [ + { + "action": "dashboard", + "item_type": "dashboard", + "item_url": "/superset/dashboard/dash_slug/", + "item_title": "dash_title", + "time": ANY, + "time_delta_humanized": ANY, + } + ] + }, + ) + + def test_get_recent_activity_actions_filter(self): + """ + Log API: Test recent activity actions argument + """ + admin_user = self.get_user("admin") + self.login(username="admin") + dash = create_dashboard("dash_slug", "dash_title", "{}", []) + log = self.insert_log("dashboard", admin_user, dashboard_id=dash.id) + log2 = self.insert_log("explore", admin_user, dashboard_id=dash.id) + + arguments = {"actions": ["dashboard"]} + uri = f"api/v1/log/recent_activity/{admin_user.id}/?q={prison.dumps(arguments)}" + rv = self.client.get(uri) + + db.session.delete(log) + db.session.delete(log2) + db.session.delete(dash) + db.session.commit() + + self.assertEqual(rv.status_code, 200) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(len(response["result"]), 1) + + def test_get_recent_activity_distinct_false(self): + """ + Log API: Test recent activity when distinct is false + """ + db.session.query(Log).delete(synchronize_session=False) + db.session.commit() + admin_user = self.get_user("admin") + self.login(username="admin") + dash = create_dashboard("dash_slug", "dash_title", "{}", []) + log = self.insert_log("dashboard", admin_user, dashboard_id=dash.id) + log2 = self.insert_log("dashboard", admin_user, dashboard_id=dash.id) + + arguments = {"distinct": False} + uri = f"api/v1/log/recent_activity/{admin_user.id}/?q={prison.dumps(arguments)}" + rv = self.client.get(uri) + + db.session.delete(log) + db.session.delete(log2) + db.session.delete(dash) + db.session.commit() + self.assertEqual(rv.status_code, 200) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(len(response["result"]), 2) + + def test_get_recent_activity_pagination(self): + """ + Log API: Test recent activity pagination arguments + """ + admin_user = self.get_user("admin") + self.login(username="admin") + dash = create_dashboard("dash_slug", "dash_title", "{}", []) + dash2 = create_dashboard("dash2_slug", "dash2_title", "{}", []) + dash3 = create_dashboard("dash3_slug", "dash3_title", "{}", []) + log = self.insert_log("dashboard", admin_user, dashboard_id=dash.id) + log2 = self.insert_log("dashboard", admin_user, dashboard_id=dash2.id) + log3 = self.insert_log("dashboard", admin_user, dashboard_id=dash3.id) + + now = datetime.now() + log3.dttm = now + log2.dttm = now - timedelta(days=1) + log.dttm = now - timedelta(days=2) + + arguments = {"page": 0, "page_size": 2} + uri = f"api/v1/log/recent_activity/{admin_user.id}/?q={prison.dumps(arguments)}" + rv = self.client.get(uri) + + self.assertEqual(rv.status_code, 200) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual( + response, + { + "result": [ + { + "action": "dashboard", + "item_type": "dashboard", + "item_url": "/superset/dashboard/dash3_slug/", + "item_title": "dash3_title", + "time": ANY, + "time_delta_humanized": ANY, + }, + { + "action": "dashboard", + "item_type": "dashboard", + "item_url": "/superset/dashboard/dash2_slug/", + "item_title": "dash2_title", + "time": ANY, + "time_delta_humanized": ANY, + }, + ] + }, + ) + + arguments = {"page": 1, "page_size": 2} + uri = f"api/v1/log/recent_activity/{admin_user.id}/?q={prison.dumps(arguments)}" + rv = self.client.get(uri) + + db.session.delete(log) + db.session.delete(log2) + db.session.delete(log3) + db.session.delete(dash) + db.session.delete(dash2) + db.session.delete(dash3) + db.session.commit() + + self.assertEqual(rv.status_code, 200) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual( + response, + { + "result": [ + { + "action": "dashboard", + "item_type": "dashboard", + "item_url": "/superset/dashboard/dash_slug/", + "item_title": "dash_title", + "time": ANY, + "time_delta_humanized": ANY, + } + ] + }, + )