This is an automated email from the ASF dual-hosted git repository. michaelsmolina pushed a commit to branch 4.1-airbnb in repository https://gitbox.apache.org/repos/asf/superset.git
commit 25e9840039e56b493a37d3ced01e4070e58ee7da Author: JUST.in DO IT <[email protected]> AuthorDate: Mon Mar 17 09:18:47 2025 -0700 fix(log): Update recent_activity by event name (#32681) (cherry picked from commit 449f51aed56b3b1f91b04476b20033efc3610c8b) --- .../components/ExploreViewContainer/index.jsx | 9 +- superset-frontend/src/middleware/logger.test.js | 5 +- .../src/middleware/loggerMiddleware.js | 11 +- superset/daos/log.py | 19 ++- superset/utils/log.py | 4 +- superset/views/log/api.py | 2 +- tests/integration_tests/log_api_tests.py | 162 +++++++++++++-------- 7 files changed, 138 insertions(+), 74 deletions(-) diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx index 3d263671c1..d6d11687be 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx +++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx @@ -356,7 +356,14 @@ function ExploreViewContainer(props) { } useComponentDidMount(() => { - props.actions.logEvent(LOG_ACTIONS_MOUNT_EXPLORER); + props.actions.logEvent( + LOG_ACTIONS_MOUNT_EXPLORER, + props.slice?.slice_id + ? { + slice_id: props.slice.slice_id, + } + : undefined, + ); }); useChangeEffect(tabId, (previous, current) => { diff --git a/superset-frontend/src/middleware/logger.test.js b/superset-frontend/src/middleware/logger.test.js index d67c681caa..5d51649e00 100644 --- a/superset-frontend/src/middleware/logger.test.js +++ b/superset-frontend/src/middleware/logger.test.js @@ -23,11 +23,12 @@ import { LOG_EVENT } from 'src/logger/actions'; import { LOG_ACTIONS_LOAD_CHART } from 'src/logger/LogUtils'; describe('logger middleware', () => { + const dashboardId = 123; const next = sinon.spy(); const mockStore = { getState: () => ({ dashboardInfo: { - id: 1, + id: dashboardId, }, impressionId: 'impression_id', }), @@ -39,6 +40,7 @@ describe('logger middleware', () => { eventData: { key: 'value', start_offset: 100, + path: `/dashboard/${dashboardId}/`, }, }, }; @@ -91,6 +93,7 @@ describe('logger middleware', () => { source: 'dashboard', source_id: mockStore.getState().dashboardInfo.id, event_type: 'timing', + dashboard_id: mockStore.getState().dashboardInfo.id, }); expect(typeof events[0].ts).toBe('number'); diff --git a/superset-frontend/src/middleware/loggerMiddleware.js b/superset-frontend/src/middleware/loggerMiddleware.js index 5408edb62c..ea4160f365 100644 --- a/superset-frontend/src/middleware/loggerMiddleware.js +++ b/superset-frontend/src/middleware/loggerMiddleware.js @@ -78,19 +78,24 @@ const loggerMiddleware = store => next => action => { impression_id: impressionId, version: 'v2', }; - if (dashboardInfo?.id) { + const { eventName } = action.payload; + let { eventData = {} } = action.payload; + + if (dashboardInfo?.id && eventData.path?.includes('/dashboard/')) { logMetadata = { source: 'dashboard', source_id: dashboardInfo.id, + dashboard_id: dashboardInfo.id, ...logMetadata, }; } else if (explore?.slice) { logMetadata = { source: 'explore', source_id: explore.slice ? explore.slice.slice_id : 0, + ...(explore.slice.slice_id && { slice_id: explore.slice.slice_id }), ...logMetadata, }; - } else if (sqlLab) { + } else if (eventData.path?.includes('/sqllab/')) { const editor = sqlLab.queryEditors.find( ({ id }) => id === sqlLab.tabHistory.slice(-1)[0], ); @@ -102,8 +107,6 @@ const loggerMiddleware = store => next => action => { }; } - const { eventName } = action.payload; - let { eventData = {} } = action.payload; eventData = { ...logMetadata, ts: new Date().getTime(), diff --git a/superset/daos/log.py b/superset/daos/log.py index 002c3f2307..fe32ef8169 100644 --- a/superset/daos/log.py +++ b/superset/daos/log.py @@ -59,8 +59,14 @@ class LogDAO(BaseDAO[Log]): .group_by(Log.dashboard_id, Log.slice_id, Log.action) .filter( and_( - Log.action.in_(actions), + Log.action == "log", Log.user_id == user_id, + or_( + *{ + Log.json.contains(f'"event_name": "{action}"') + for action in actions + }, + ), # 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)), @@ -99,7 +105,16 @@ class LogDAO(BaseDAO[Log]): .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) + .filter( + Log.action == "log", + Log.user_id == user_id, + or_( + *{ + Log.json.contains(f'"event_name": "{action}"') + for action in actions + }, + ), + ) .order_by(Log.dttm.desc()) .limit(page_size) .offset(page * page_size) diff --git a/superset/utils/log.py b/superset/utils/log.py index 71c5528833..70f6a1bda2 100644 --- a/superset/utils/log.py +++ b/superset/utils/log.py @@ -394,8 +394,8 @@ class DBEventLogger(AbstractEventLogger): log = Log( action=action, json=json_string, - dashboard_id=dashboard_id, - slice_id=slice_id, + dashboard_id=dashboard_id or record.get("dashboard_id"), + slice_id=slice_id or record.get("slice_id"), duration_ms=duration_ms, referrer=referrer, user_id=user_id, diff --git a/superset/views/log/api.py b/superset/views/log/api.py index 33f4ad51d4..ffa3a86006 100644 --- a/superset/views/log/api.py +++ b/superset/views/log/api.py @@ -130,7 +130,7 @@ class LogRestApi(LogMixin, BaseSupersetModelRestApi): """ args = kwargs["rison"] page, page_size = self._sanitize_page_args(*self._handle_page_args(args)) - actions = args.get("actions", ["explore", "dashboard"]) + actions = args.get("actions", ["mount_explorer", "mount_dashboard"]) distinct = args.get("distinct", True) payload = LogDAO.get_recent_activity(actions, distinct, page, page_size) diff --git a/tests/integration_tests/log_api_tests.py b/tests/integration_tests/log_api_tests.py index fae09754aa..0666f4e156 100644 --- a/tests/integration_tests/log_api_tests.py +++ b/tests/integration_tests/log_api_tests.py @@ -171,8 +171,18 @@ class TestLogApi(SupersetTestCase): admin_user = self.get_user("admin") self.login(ADMIN_USERNAME) 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) + log1 = self.insert_log( + "log", + admin_user, + dashboard_id=dash.id, + json='{"event_name": "mount_dashboard"}', + ) + log2 = self.insert_log( + "log", + admin_user, + dashboard_id=dash.id, + json='{"event_name": "mount_dashboard"}', + ) uri = f"api/v1/log/recent_activity/" # noqa: F541 rv = self.client.get(uri) @@ -184,21 +194,18 @@ class TestLogApi(SupersetTestCase): 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, - } - ] - }, - ) + assert response == { + "result": [ + { + "action": "log", + "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): """ @@ -207,10 +214,20 @@ class TestLogApi(SupersetTestCase): admin_user = self.get_user("admin") self.login(ADMIN_USERNAME) 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) + log = self.insert_log( + "log", + admin_user, + dashboard_id=dash.id, + json='{"event_name": "mount_dashboard"}', + ) + log2 = self.insert_log( + "log", + admin_user, + dashboard_id=dash.id, + json='{"event_name": "mount_explorer"}', + ) - arguments = {"actions": ["dashboard"]} + arguments = {"actions": ["mount_dashboard"]} uri = f"api/v1/log/recent_activity/?q={prison.dumps(arguments)}" rv = self.client.get(uri) @@ -232,8 +249,18 @@ class TestLogApi(SupersetTestCase): admin_user = self.get_user("admin") self.login(ADMIN_USERNAME) 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) + log = self.insert_log( + "log", + admin_user, + dashboard_id=dash.id, + json='{"event_name": "mount_dashboard"}', + ) + log2 = self.insert_log( + "log", + admin_user, + dashboard_id=dash.id, + json='{"event_name": "mount_dashboard"}', + ) arguments = {"distinct": False} uri = f"api/v1/log/recent_activity/?q={prison.dumps(arguments)}" @@ -256,9 +283,24 @@ class TestLogApi(SupersetTestCase): 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) + log = self.insert_log( + "log", + admin_user, + dashboard_id=dash.id, + json='{"event_name": "mount_dashboard"}', + ) + log2 = self.insert_log( + "log", + admin_user, + dashboard_id=dash2.id, + json='{"event_name": "mount_dashboard"}', + ) + log3 = self.insert_log( + "log", + admin_user, + dashboard_id=dash3.id, + json='{"event_name": "mount_dashboard"}', + ) now = datetime.now() log3.dttm = now @@ -271,29 +313,26 @@ class TestLogApi(SupersetTestCase): 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, - }, - ] - }, - ) + assert response == { + "result": [ + { + "action": "log", + "item_type": "dashboard", + "item_url": "/superset/dashboard/dash3_slug/", + "item_title": "dash3_title", + "time": ANY, + "time_delta_humanized": ANY, + }, + { + "action": "log", + "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/?q={prison.dumps(arguments)}" @@ -309,18 +348,15 @@ class TestLogApi(SupersetTestCase): 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, - } - ] - }, - ) + assert response == { + "result": [ + { + "action": "log", + "item_type": "dashboard", + "item_url": "/superset/dashboard/dash_slug/", + "item_title": "dash_title", + "time": ANY, + "time_delta_humanized": ANY, + } + ] + }
