From 1d01f19ef7fea9b24632f2d4ccc3044639ca9d0e Mon Sep 17 00:00:00 2001 From: Jackson Goerner <37640160+glipR@users.noreply.github.com> Date: Thu, 19 Dec 2024 11:22:45 +1100 Subject: [PATCH] feat: Enhance search logging (#2280) * Support custom fields in logging. Signed-off-by: Jackson Goerner * Add search logSearchEvent function. Signed-off-by: Jackson Goerner * Add utility for retrieving search state elements. Signed-off-by: Jackson Goerner * Call logSearchEvent in search item classes. Signed-off-by: Jackson Goerner * Fix issue where search_term not being updated for inline results. Signed-off-by: Jackson Goerner * Add tests for log ducks. Signed-off-by: Jackson Goerner * Add header to new scripts. Signed-off-by: Jackson Goerner * Be explicit with kwargs for log generic action. Signed-off-by: Jackson Goerner * Provide default case and forced return for getResults. Signed-off-by: Jackson Goerner * Fix eslint error default case. Signed-off-by: Jackson Goerner * Use python 3.9 type annotations. Signed-off-by: Jackson Goerner * Update betterer results file. Signed-off-by: Jackson Goerner * Fix tests related to tests using the connect wrapper. Signed-off-by: Jackson Goerner * Fix test related to inlineresults fix. Signed-off-by: Jackson Goerner * Fix + Add log tests. Signed-off-by: Jackson Goerner --------- Signed-off-by: Jackson Goerner --- frontend/amundsen_application/api/log/v0.py | 15 +- .../static/.betterer.results | 8 +- .../DashboardListItem/index.spec.tsx | 3 +- .../DashboardListItem/index.tsx | 57 ++++- .../FeatureListItem/index.spec.tsx | 4 +- .../FeatureListItem/index.tsx | 54 ++++- .../TableListItem/index.spec.tsx | 4 +- .../ResourceListItem/TableListItem/index.tsx | 56 ++++- .../UserListItem/index.spec.tsx | 3 +- .../ResourceListItem/UserListItem/index.tsx | 55 ++++- .../static/js/ducks/log/api/v0.ts | 5 + .../static/js/ducks/log/index.spec.ts | 201 ++++++++++++++++++ .../static/js/ducks/log/reducer.ts | 28 +++ .../static/js/ducks/log/sagas.ts | 66 ++++++ .../static/js/ducks/log/types.ts | 29 +++ .../static/js/ducks/rootSaga.ts | 2 + .../static/js/ducks/search/reducer.spec.ts | 2 + .../static/js/ducks/search/reducer.ts | 3 + .../static/js/ducks/search/utils.ts | 22 +- .../ResultItemList/index.tsx | 50 ++++- .../ResultItemList/tests/index.spec.tsx | 3 +- 21 files changed, 600 insertions(+), 70 deletions(-) create mode 100644 frontend/amundsen_application/static/js/ducks/log/index.spec.ts create mode 100644 frontend/amundsen_application/static/js/ducks/log/reducer.ts create mode 100644 frontend/amundsen_application/static/js/ducks/log/sagas.ts create mode 100644 frontend/amundsen_application/static/js/ducks/log/types.ts diff --git a/frontend/amundsen_application/api/log/v0.py b/frontend/amundsen_application/api/log/v0.py index 05888ec3b2..6e65a93a4b 100644 --- a/frontend/amundsen_application/api/log/v0.py +++ b/frontend/amundsen_application/api/log/v0.py @@ -3,6 +3,7 @@ import logging +from typing import List from http import HTTPStatus from flask import Response, jsonify, make_response, request @@ -38,7 +39,12 @@ def _log_generic_action(*, label: str, location: str, value: str, - position: str) -> None: + position: str, + resource_href: str, + resource_type: str, + search_term: str, + search_page_index: int, + search_results: List[str]) -> None: pass # pragma: no cover try: @@ -52,7 +58,12 @@ def _log_generic_action(*, label=args.get('label', None), location=args.get('location', None), value=args.get('value', None), - position=args.get('position', None) + position=args.get('position', None), + resource_href=args.get('resource_href', None), + resource_type=args.get('resource_type', None), + search_term=args.get('search_term', None), + search_page_index=args.get('search_page_index', None), + search_results=args.get('search_results', None), ) message = 'Logging of {} action successful'.format(command) return make_response(jsonify({'msg': message}), HTTPStatus.OK) diff --git a/frontend/amundsen_application/static/.betterer.results b/frontend/amundsen_application/static/.betterer.results index a3036a576d..037655ccd2 100644 --- a/frontend/amundsen_application/static/.betterer.results +++ b/frontend/amundsen_application/static/.betterer.results @@ -44,15 +44,13 @@ exports[`eslint`] = { [112, 6, 191, "Unexpected lexical declaration in case block.", "1433271452"], [118, 6, 59, "Unexpected lexical declaration in case block.", "3993269111"] ], - "js/ducks/search/reducer.ts:1964208432": [ + "js/ducks/search/reducer.ts:4202738843": [ [310, 6, 27, "Unexpected lexical declaration in case block.", "3336779920"], [364, 6, 53, "Unexpected lexical declaration in case block.", "2684420572"], [387, 6, 66, "Unexpected lexical declaration in case block.", "2160263677"], [402, 6, 123, "Unexpected lexical declaration in case block.", "2271601490"], - [417, 6, 61, "Unexpected lexical declaration in case block.", "2053236172"] - ], - "js/ducks/search/utils.ts:923002554": [ - [20, 2, 248, "Expected a default case.", "1034339850"] + [417, 6, 61, "Unexpected lexical declaration in case block.", "2053236172"], + [444, 6, 60, "Unexpected lexical declaration in case block.", "3101482005"] ], "js/ducks/tableMetadata/api/v0.ts:3333048528": [ [141, 23, 2, "Expected to return a value at the end of arrow function.", "5859494"] diff --git a/frontend/amundsen_application/static/js/components/ResourceListItem/DashboardListItem/index.spec.tsx b/frontend/amundsen_application/static/js/components/ResourceListItem/DashboardListItem/index.spec.tsx index e99961cda0..fcbbd87f87 100644 --- a/frontend/amundsen_application/static/js/components/ResourceListItem/DashboardListItem/index.spec.tsx +++ b/frontend/amundsen_application/static/js/components/ResourceListItem/DashboardListItem/index.spec.tsx @@ -15,7 +15,7 @@ import { dashboardSummary } from 'fixtures/metadata/dashboard'; import { NO_TIMESTAMP_TEXT } from '../../../constants'; import * as Constants from './constants'; -import DashboardListItem, { DashboardListItemProps } from './index'; +import { DashboardListItem, DashboardListItemProps } from './index'; const MOCK_DISPLAY_NAME = 'displayName'; const MOCK_ICON_CLASS = 'test-class'; @@ -37,6 +37,7 @@ const setup = (propOverrides?: Partial) => { name: dashboardSummary.name, description: dashboardSummary.description, }, + logSearchEvent: jest.fn(), ...propOverrides, }; const wrapper = shallow( diff --git a/frontend/amundsen_application/static/js/components/ResourceListItem/DashboardListItem/index.tsx b/frontend/amundsen_application/static/js/components/ResourceListItem/DashboardListItem/index.tsx index 02977987fe..ae3b552636 100644 --- a/frontend/amundsen_application/static/js/components/ResourceListItem/DashboardListItem/index.tsx +++ b/frontend/amundsen_application/static/js/components/ResourceListItem/DashboardListItem/index.tsx @@ -7,25 +7,45 @@ import { Link } from 'react-router-dom'; import BookmarkIcon from 'components/Bookmark/BookmarkIcon'; import { getSourceDisplayName, getSourceIconClass } from 'config/config-utils'; -import { logClick } from 'utils/analytics'; import { buildDashboardURL } from 'utils/navigation'; import { formatDate } from 'utils/date'; import { ResourceType, DashboardResource } from 'interfaces'; +import { LogSearchEventRequest } from 'ducks/log/types'; +import { connect } from 'react-redux'; +import { logSearchEvent } from 'ducks/log/reducer'; +import { bindActionCreators } from 'redux'; import { NO_TIMESTAMP_TEXT } from '../../../constants'; import { LoggingParams } from '../types'; import { HighlightedDashboard } from '../MetadataHighlightList/utils'; import MetadataHighlightList from '../MetadataHighlightList'; import * as Constants from './constants'; -export interface DashboardListItemProps { +export interface OwnProps { dashboard: DashboardResource; logging: LoggingParams; dashboardHighlights: HighlightedDashboard; } -class DashboardListItem extends React.Component { +export interface DispatchFromProps { + logSearchEvent: ( + resourceLink: string, + resourceType: ResourceType, + source: string, + index: number, + event: any, + inline: boolean, + extra?: { [key: string]: any } + ) => LogSearchEventRequest; +} + +export type DashboardListItemProps = OwnProps & DispatchFromProps; + +export class DashboardListItem extends React.Component< + DashboardListItemProps, + {} +> { getLink = () => { const { dashboard, logging } = this.props; @@ -41,7 +61,8 @@ class DashboardListItem extends React.Component { `icon resource-icon ${getSourceIconClass(dashboardId, dashboardType)}`; render() { - const { dashboard, logging, dashboardHighlights } = this.props; + const { dashboard, logging, dashboardHighlights, logSearchEvent } = + this.props; return (
  • @@ -49,11 +70,14 @@ class DashboardListItem extends React.Component { className="resource-list-item table-list-item" to={this.getLink()} onClick={(e) => - logClick(e, { - target_id: 'dashboard_list_item', - value: logging.source, - position: logging.index.toString(), - }) + logSearchEvent( + this.getLink(), + ResourceType.dashboard, + logging.source, + logging.index, + e, + false + ) } >
    @@ -120,4 +144,17 @@ class DashboardListItem extends React.Component { } } -export default DashboardListItem; +export const mapDispatchToProps = (dispatch: any): DispatchFromProps => { + const dispatchableActions: DispatchFromProps = bindActionCreators( + { + logSearchEvent, + }, + dispatch + ); + + return dispatchableActions; +}; +export default connect<{}, DispatchFromProps, OwnProps>( + null, + mapDispatchToProps +)(DashboardListItem); diff --git a/frontend/amundsen_application/static/js/components/ResourceListItem/FeatureListItem/index.spec.tsx b/frontend/amundsen_application/static/js/components/ResourceListItem/FeatureListItem/index.spec.tsx index 2f390e924f..fcfb212757 100644 --- a/frontend/amundsen_application/static/js/components/ResourceListItem/FeatureListItem/index.spec.tsx +++ b/frontend/amundsen_application/static/js/components/ResourceListItem/FeatureListItem/index.spec.tsx @@ -10,7 +10,8 @@ import * as ConfigUtils from 'config/config-utils'; import { featureSummary } from 'fixtures/metadata/feature'; -import FeatureListItem, { +import { + FeatureListItem, FeatureListItemProps, } from 'components/ResourceListItem/FeatureListItem'; import { RightIcon } from 'components/SVGIcons'; @@ -33,6 +34,7 @@ describe('FeatureListItem', () => { name: MOCK_DISPLAY_NAME, description: 'I am an ML feature', }, + logSearchEvent: jest.fn(), ...propOverrides, }; const wrapper = shallow( diff --git a/frontend/amundsen_application/static/js/components/ResourceListItem/FeatureListItem/index.tsx b/frontend/amundsen_application/static/js/components/ResourceListItem/FeatureListItem/index.tsx index 47ea209620..8ceb5cc4a6 100644 --- a/frontend/amundsen_application/static/js/components/ResourceListItem/FeatureListItem/index.tsx +++ b/frontend/amundsen_application/static/js/components/ResourceListItem/FeatureListItem/index.tsx @@ -5,21 +5,38 @@ import * as React from 'react'; import { Link } from 'react-router-dom'; import { getSourceDisplayName, getSourceIconClass } from 'config/config-utils'; -import { logClick } from 'utils/analytics'; import BadgeList from 'features/BadgeList'; import { ResourceType, FeatureResource } from 'interfaces'; import { RightIcon } from 'components/SVGIcons'; -import { LoggingParams } from '../types'; +import { LogSearchEventRequest } from 'ducks/log/types'; +import { connect } from 'react-redux'; +import { bindActionCreators } from 'redux'; +import { logSearchEvent } from 'ducks/log/reducer'; import { HighlightedResource } from '../MetadataHighlightList/utils'; +import { LoggingParams } from '../types'; -export interface FeatureListItemProps { +export interface OwnProps { feature: FeatureResource; logging: LoggingParams; featureHighlights: HighlightedResource; } +export interface DispatchFromProps { + logSearchEvent: ( + resourceLink: string, + resourceType: ResourceType, + source: string, + index: number, + event: any, + inline: boolean, + extra?: { [key: string]: any } + ) => LogSearchEventRequest; +} + +export type FeatureListItemProps = OwnProps & DispatchFromProps; + const getLink = (feature: FeatureResource, logging: LoggingParams) => `/feature/${feature.key}?index=${logging.index}&source=${logging.source}`; @@ -28,10 +45,11 @@ const generateResourceIconClass = ( featureType: ResourceType ): string => `icon resource-icon ${getSourceIconClass(featureId, featureType)}`; -const FeatureListItem: React.FC = ({ +export const FeatureListItem: React.FC = ({ feature, logging, featureHighlights, + logSearchEvent, }: FeatureListItemProps) => { const source = feature.availability?.length > 0 ? feature.availability[0] : ''; @@ -43,11 +61,14 @@ const FeatureListItem: React.FC = ({ to={getLink(feature, logging)} data-type="feature_list_item" onClick={(e: React.MouseEvent) => - logClick(e, { - target_id: 'feature_list_item', - value: logging.source, - position: logging.index.toString(), - }) + logSearchEvent( + getLink(feature, logging), + ResourceType.feature, + logging.source, + logging.index, + e, + false + ) } >
    @@ -87,4 +108,17 @@ const FeatureListItem: React.FC = ({ ); }; -export default FeatureListItem; +export const mapDispatchToProps = (dispatch: any): DispatchFromProps => { + const dispatchableActions: DispatchFromProps = bindActionCreators( + { + logSearchEvent, + }, + dispatch + ); + + return dispatchableActions; +}; +export default connect<{}, DispatchFromProps, OwnProps>( + null, + mapDispatchToProps +)(FeatureListItem); diff --git a/frontend/amundsen_application/static/js/components/ResourceListItem/TableListItem/index.spec.tsx b/frontend/amundsen_application/static/js/components/ResourceListItem/TableListItem/index.spec.tsx index fd55137a66..b7fd52761a 100644 --- a/frontend/amundsen_application/static/js/components/ResourceListItem/TableListItem/index.spec.tsx +++ b/frontend/amundsen_application/static/js/components/ResourceListItem/TableListItem/index.spec.tsx @@ -11,7 +11,8 @@ import { ResourceType } from 'interfaces'; import * as ConfigUtils from 'config/config-utils'; import BadgeList from 'features/BadgeList'; -import TableListItem, { +import { + TableListItem, TableListItemProps, getLink, generateResourceIconClass, @@ -56,6 +57,7 @@ describe('TableListItem', () => { name: 'tableName', description: 'I am the description', }, + logSearchEvent: jest.fn(), ...propOverrides, }; // eslint-disable-next-line react/jsx-props-no-spreading diff --git a/frontend/amundsen_application/static/js/components/ResourceListItem/TableListItem/index.tsx b/frontend/amundsen_application/static/js/components/ResourceListItem/TableListItem/index.tsx index 3c69e03a34..48d33620f6 100644 --- a/frontend/amundsen_application/static/js/components/ResourceListItem/TableListItem/index.tsx +++ b/frontend/amundsen_application/static/js/components/ResourceListItem/TableListItem/index.tsx @@ -12,18 +12,34 @@ import { getSourceDisplayName, getSourceIconClass } from 'config/config-utils'; import BadgeList from 'features/BadgeList'; import SchemaInfo from 'components/ResourceListItem/SchemaInfo'; -import { logClick } from 'utils/analytics'; -import { LoggingParams } from '../types'; -import MetadataHighlightList from '../MetadataHighlightList'; +import { LogSearchEventRequest } from 'ducks/log/types'; +import { bindActionCreators } from 'redux'; +import { logSearchEvent } from 'ducks/log/reducer'; +import { connect } from 'react-redux'; import { HighlightedTable } from '../MetadataHighlightList/utils'; +import MetadataHighlightList from '../MetadataHighlightList'; +import { LoggingParams } from '../types'; -export interface TableListItemProps { +export interface OwnProps { table: TableResource; logging: LoggingParams; tableHighlights: HighlightedTable; disabled?: boolean; } +export interface DispatchFromProps { + logSearchEvent: ( + resourceLink: string, + resourceType: ResourceType, + source: string, + index: number, + event: any, + inline: boolean, + extra?: { [key: string]: any } + ) => LogSearchEventRequest; +} + +export type TableListItemProps = OwnProps & DispatchFromProps; /* this function get's the table name from the key to preserve original capitalization since search needs the names to be lowercase for analysis @@ -53,11 +69,12 @@ export const getLink = (table, logging) => { export const generateResourceIconClass = (databaseId: string): string => `icon resource-icon ${getSourceIconClass(databaseId, ResourceType.table)}`; -const TableListItem: React.FC = ({ +export const TableListItem: React.FC = ({ table, logging, tableHighlights, disabled, + logSearchEvent, }) => (
  • = ({ }`} to={getLink(table, logging)} onClick={(e) => - logClick(e, { - target_id: 'table_list_item', - value: logging.source, - position: logging.index.toString(), - }) + logSearchEvent( + getLink(table, logging), + ResourceType.table, + logging.source, + logging.index, + e, + false + ) } >
    @@ -132,4 +152,18 @@ const TableListItem: React.FC = ({
    • ); -export default TableListItem; + +export const mapDispatchToProps = (dispatch: any): DispatchFromProps => { + const dispatchableActions: DispatchFromProps = bindActionCreators( + { + logSearchEvent, + }, + dispatch + ); + + return dispatchableActions; +}; +export default connect<{}, DispatchFromProps, OwnProps>( + null, + mapDispatchToProps +)(TableListItem); diff --git a/frontend/amundsen_application/static/js/components/ResourceListItem/UserListItem/index.spec.tsx b/frontend/amundsen_application/static/js/components/ResourceListItem/UserListItem/index.spec.tsx index 55fb781542..40b47a6fa1 100644 --- a/frontend/amundsen_application/static/js/components/ResourceListItem/UserListItem/index.spec.tsx +++ b/frontend/amundsen_application/static/js/components/ResourceListItem/UserListItem/index.spec.tsx @@ -9,7 +9,7 @@ import * as Avatar from 'react-avatar'; import { Link } from 'react-router-dom'; import { ResourceType } from 'interfaces'; -import UserListItem, { UserListItemProps } from '.'; +import { UserListItem, UserListItemProps } from '.'; const setup = (propOverrides?: Partial) => { const props: UserListItemProps = { @@ -34,6 +34,7 @@ const setup = (propOverrides?: Partial) => { team_name: 'QA', user_id: 'test0', }, + logSearchEvent: jest.fn(), ...propOverrides, }; // eslint-disable-next-line react/jsx-props-no-spreading diff --git a/frontend/amundsen_application/static/js/components/ResourceListItem/UserListItem/index.tsx b/frontend/amundsen_application/static/js/components/ResourceListItem/UserListItem/index.tsx index 7ac2dc03ec..d36481a0a5 100644 --- a/frontend/amundsen_application/static/js/components/ResourceListItem/UserListItem/index.tsx +++ b/frontend/amundsen_application/static/js/components/ResourceListItem/UserListItem/index.tsx @@ -5,16 +5,33 @@ import * as React from 'react'; import * as Avatar from 'react-avatar'; import { Link } from 'react-router-dom'; -import { UserResource } from 'interfaces'; +import { ResourceType, UserResource } from 'interfaces'; +import { LogSearchEventRequest } from 'ducks/log/types'; +import { connect } from 'react-redux'; +import { bindActionCreators } from 'redux'; +import { logSearchEvent } from 'ducks/log/reducer'; import { LoggingParams } from '../types'; -import { logClick } from '../../../utils/analytics'; -export interface UserListItemProps { +export interface OwnProps { user: UserResource; logging: LoggingParams; } -class UserListItem extends React.Component { +export interface DispatchFromProps { + logSearchEvent: ( + resourceLink: string, + resourceType: ResourceType, + source: string, + index: number, + event: any, + inline: boolean, + extra?: { [key: string]: any } + ) => LogSearchEventRequest; +} + +export type UserListItemProps = OwnProps & DispatchFromProps; + +export class UserListItem extends React.Component { getLink = () => { const { user, logging } = this.props; @@ -41,7 +58,7 @@ class UserListItem extends React.Component { }; render() { - const { user, logging } = this.props; + const { user, logging, logSearchEvent } = this.props; const userInfo = this.renderUserInfo(user); return ( @@ -50,11 +67,14 @@ class UserListItem extends React.Component { className="resource-list-item user-list-item" to={this.getLink()} onClick={(e) => - logClick(e, { - target_id: 'user_list_item', - value: logging.source, - position: logging.index.toString(), - }) + logSearchEvent( + this.getLink(), + ResourceType.user, + logging.source, + logging.index, + e, + false + ) } >
    @@ -78,4 +98,17 @@ class UserListItem extends React.Component { } } -export default UserListItem; +export const mapDispatchToProps = (dispatch: any): DispatchFromProps => { + const dispatchableActions: DispatchFromProps = bindActionCreators( + { + logSearchEvent, + }, + dispatch + ); + + return dispatchableActions; +}; +export default connect<{}, DispatchFromProps, OwnProps>( + null, + mapDispatchToProps +)(UserListItem); diff --git a/frontend/amundsen_application/static/js/ducks/log/api/v0.ts b/frontend/amundsen_application/static/js/ducks/log/api/v0.ts index 65c5300e37..8afc7783ea 100644 --- a/frontend/amundsen_application/static/js/ducks/log/api/v0.ts +++ b/frontend/amundsen_application/static/js/ducks/log/api/v0.ts @@ -16,6 +16,11 @@ export interface ClickLogParams { label?: string; value?: string; position?: string; + resource_href?: string; + resource_type?: string; + search_term?: string; + search_results?: string[]; + search_page_index?: number; } export const BASE_URL = '/api/log/v0/log_event'; diff --git a/frontend/amundsen_application/static/js/ducks/log/index.spec.ts b/frontend/amundsen_application/static/js/ducks/log/index.spec.ts new file mode 100644 index 0000000000..8bd679d74c --- /dev/null +++ b/frontend/amundsen_application/static/js/ducks/log/index.spec.ts @@ -0,0 +1,201 @@ +// Copyright Contributors to the Amundsen project. +// SPDX-License-Identifier: Apache-2.0 + +import { testSaga } from 'redux-saga-test-plan'; + +import { ResourceType } from 'interfaces/Resources'; +import * as Analytics from 'utils/analytics'; +import globalState from 'fixtures/globalState'; +import { logSearchEvent } from './reducer'; + +import { logSearchEventWatcher, logSearchEventWorker } from './sagas'; + +import { LogSearchEvent } from './types'; + +const searchState = globalState.search; + +describe('log ducks', () => { + const resourceLink = '/some/resource/link'; + const resourceType = ResourceType.table; + const source = 'source'; + const index = 1; + const event = 'some_event'; + const inline = false; + const extra = { key: 'value', other_key: 42 }; + + const mockState = { + search: { + ...searchState, + }, + }; + + describe('actions', () => { + it('logSearchEvent - returns the action to log search events', () => { + const action = logSearchEvent( + resourceLink, + resourceType, + source, + index, + event, + inline, + extra + ); + const { payload } = action; + + expect(action.type).toBe(LogSearchEvent.REQUEST); + expect(payload.resourceLink).toBe(resourceLink); + expect(payload.resourceType).toBe(resourceType); + expect(payload.source).toBe(source); + expect(payload.index).toBe(index); + expect(payload.event).toBe(event); + expect(payload.inline).toBe(inline); + expect(payload.extra).toBe(extra); + }); + }); + + describe('sagas', () => { + describe('logSearchEventWatcher', () => { + it('takes every LogSearchEvent.REQUEST with logSearchEventWorker', () => { + testSaga(logSearchEventWatcher) + .next() + .takeEvery(LogSearchEvent.REQUEST, logSearchEventWorker) + .next() + .isDone(); + }); + }); + + describe('logSearchEventWorker', () => { + it('executes flow for logging search events', () => { + testSaga( + logSearchEventWorker, + logSearchEvent( + resourceLink, + resourceType, + source, + index, + event, + inline, + extra + ) + ) + .next() + .select() + .next(mockState) + .call(Analytics.logClick, event, { + value: source, + position: index.toString(), + resource_href: resourceLink, + resource_type: resourceType, + search_page_index: mockState.search.tables.page_index, + search_term: mockState.search.search_term, + search_results: mockState.search.tables.results.map((t) => t.key), + ...extra, + }) + .next() + .put({ type: LogSearchEvent.SUCCESS, payload: { completed: true } }) + .next() + .isDone(); + }); + + it('handles request error', () => { + testSaga( + logSearchEventWorker, + logSearchEvent( + resourceLink, + resourceType, + source, + index, + event, + inline, + extra + ) + ) + .next() + .select() + .next(mockState) + .call(Analytics.logClick, event, { + value: source, + position: index.toString(), + resource_href: resourceLink, + resource_type: resourceType, + search_page_index: mockState.search.tables.page_index, + search_term: mockState.search.search_term, + search_results: mockState.search.tables.results.map((t) => t.key), + ...extra, + }) + .throw(new Error('error')) + .put({ type: LogSearchEvent.FAILURE, payload: { completed: false } }) + .next() + .isDone(); + }); + + it('handles resource types correctly', () => { + testSaga( + logSearchEventWorker, + logSearchEvent( + resourceLink, + ResourceType.dashboard, + source, + index, + event, + inline, + extra + ) + ) + .next() + .select() + .next(mockState) + .call(Analytics.logClick, event, { + value: source, + position: index.toString(), + resource_href: resourceLink, + resource_type: ResourceType.dashboard, + search_page_index: mockState.search.dashboards.page_index, + search_term: mockState.search.search_term, + search_results: mockState.search.dashboards.results.map( + (d) => d.uri + ), + ...extra, + }) + .next() + .put({ type: LogSearchEvent.SUCCESS, payload: { completed: true } }) + .next() + .isDone(); + }); + + it('handles inline search events correctly', () => { + testSaga( + logSearchEventWorker, + logSearchEvent( + resourceLink, + resourceType, + source, + index, + event, + true, + extra + ) + ) + .next() + .select() + .next(mockState) + .call(Analytics.logClick, event, { + value: source, + position: index.toString(), + resource_href: resourceLink, + resource_type: resourceType, + search_page_index: mockState.search.inlineResults.tables.page_index, + search_term: mockState.search.search_term, + search_results: mockState.search.inlineResults.tables.results.map( + (t) => t.key + ), + ...extra, + }) + .next() + .put({ type: LogSearchEvent.SUCCESS, payload: { completed: true } }) + .next() + .isDone(); + }); + }); + }); +}); diff --git a/frontend/amundsen_application/static/js/ducks/log/reducer.ts b/frontend/amundsen_application/static/js/ducks/log/reducer.ts new file mode 100644 index 0000000000..ff5a5ebe55 --- /dev/null +++ b/frontend/amundsen_application/static/js/ducks/log/reducer.ts @@ -0,0 +1,28 @@ +// Copyright Contributors to the Amundsen project. +// SPDX-License-Identifier: Apache-2.0 + +import { ResourceType } from 'interfaces/Resources'; +import { LogSearchEvent, LogSearchEventRequest } from './types'; + +export function logSearchEvent( + resourceLink: string, + resourceType: ResourceType, + source: string, + index: number, + event: any, + inline: boolean, + extra?: { [key: string]: any } +): LogSearchEventRequest { + return { + type: LogSearchEvent.REQUEST, + payload: { + resourceLink, + resourceType, + event, + source, + index, + inline, + extra, + }, + }; +} diff --git a/frontend/amundsen_application/static/js/ducks/log/sagas.ts b/frontend/amundsen_application/static/js/ducks/log/sagas.ts new file mode 100644 index 0000000000..513c9653e8 --- /dev/null +++ b/frontend/amundsen_application/static/js/ducks/log/sagas.ts @@ -0,0 +1,66 @@ +// Copyright Contributors to the Amundsen project. +// SPDX-License-Identifier: Apache-2.0 + +import { SagaIterator } from 'redux-saga'; +import { call, put, select, takeEvery } from 'redux-saga/effects'; +import { logClick } from 'utils/analytics'; +import { GlobalState } from 'ducks/rootReducer'; +import { getPageIndex, getResults } from 'ducks/search/utils'; +import { ResourceType } from 'interfaces/Resources'; +import { LogSearchEvent, LogSearchEventRequest } from './types'; + +export function* logSearchEventWorker( + action: LogSearchEventRequest +): SagaIterator { + const { resourceLink, resourceType, source, index, event, inline, extra } = + action.payload; + const state: GlobalState = yield select(); + + try { + const makeKeyMap = (resourceType: ResourceType) => { + switch (resourceType) { + case ResourceType.table: + return (table) => table.key; + case ResourceType.feature: + return (feature) => feature.key; + case ResourceType.user: + return (user) => user.user_id; + case ResourceType.dashboard: + return (dashboard) => dashboard.uri; + default: + return (a) => a; + } + }; + + // logClick isn't an async, it just kicks off the axios post, so this doesn't need call. + yield call(logClick, event, { + value: source, + position: index.toString(), + resource_href: resourceLink, + resource_type: resourceType, + search_page_index: inline ? 0 : getPageIndex(state.search, resourceType), + search_term: state.search.search_term, + search_results: getResults( + inline ? state.search.inlineResults : state.search, + resourceType + )?.results.map(makeKeyMap(resourceType)), + ...extra, + }); + yield put({ + type: LogSearchEvent.SUCCESS, + payload: { + completed: true, + }, + }); + } catch (error) { + yield put({ + type: LogSearchEvent.FAILURE, + payload: { + completed: false, + }, + }); + } +} +export function* logSearchEventWatcher(): SagaIterator { + yield takeEvery(LogSearchEvent.REQUEST, logSearchEventWorker); +} diff --git a/frontend/amundsen_application/static/js/ducks/log/types.ts b/frontend/amundsen_application/static/js/ducks/log/types.ts new file mode 100644 index 0000000000..6ecbf47e13 --- /dev/null +++ b/frontend/amundsen_application/static/js/ducks/log/types.ts @@ -0,0 +1,29 @@ +// Copyright Contributors to the Amundsen project. +// SPDX-License-Identifier: Apache-2.0 + +import { ResourceType } from 'interfaces/Resources'; + +export enum LogSearchEvent { + REQUEST = 'amundsen/search/LOG_SEARCH_REQUEST', + SUCCESS = 'amundsen/search/LOG_SEARCH_SUCCESS', + FAILURE = 'amundsen/search/LOG_SEARCH_FAILURE', +} + +export interface LogSearchEventRequest { + type: LogSearchEvent.REQUEST; + payload: { + source: string; + index: number; + resourceLink: string; + resourceType: ResourceType; + event: any; + inline: boolean; + extra?: { [key: string]: any }; + }; +} +export interface LogSearchEventResponse { + type: LogSearchEvent.SUCCESS | LogSearchEvent.FAILURE; + payload: { + completed: boolean; + }; +} diff --git a/frontend/amundsen_application/static/js/ducks/rootSaga.ts b/frontend/amundsen_application/static/js/ducks/rootSaga.ts index 1a564b6c43..85d8b91c73 100644 --- a/frontend/amundsen_application/static/js/ducks/rootSaga.ts +++ b/frontend/amundsen_application/static/js/ducks/rootSaga.ts @@ -36,6 +36,7 @@ import { urlDidUpdateWatcher, } from './search/sagas'; import { filterWatcher } from './search/filters/sagas'; +import { logSearchEventWatcher } from './log/sagas'; import { updateTableOwnerWatcher } from './tableMetadata/owners/sagas'; import { getColumnDescriptionWatcher, @@ -82,6 +83,7 @@ export default function* rootSaga() { getFeatureWatcher(), getIssuesWatcher(), getLastIndexedWatcher(), + logSearchEventWatcher(), getLoggedInUserWatcher(), getNoticesWatcher(), getPopularResourcesWatcher(), diff --git a/frontend/amundsen_application/static/js/ducks/search/reducer.spec.ts b/frontend/amundsen_application/static/js/ducks/search/reducer.spec.ts index cdb759b421..1b3ecaf654 100644 --- a/frontend/amundsen_application/static/js/ducks/search/reducer.spec.ts +++ b/frontend/amundsen_application/static/js/ducks/search/reducer.spec.ts @@ -475,6 +475,7 @@ describe('search reducer', () => { expect(reducer(testState, getInlineResults(term))).toEqual({ ...testState, + search_term: term, inlineResults: { dashboards: initialInlineResultsState.dashboards, features: initialInlineResultsState.features, @@ -490,6 +491,7 @@ describe('search reducer', () => { expect(reducer(testState, getInlineResultsDebounce(term))).toEqual({ ...testState, + search_term: term, inlineResults: { dashboards: initialInlineResultsState.dashboards, features: initialInlineResultsState.features, diff --git a/frontend/amundsen_application/static/js/ducks/search/reducer.ts b/frontend/amundsen_application/static/js/ducks/search/reducer.ts index a3d7e65f04..fabc4a9e7e 100644 --- a/frontend/amundsen_application/static/js/ducks/search/reducer.ts +++ b/frontend/amundsen_application/static/js/ducks/search/reducer.ts @@ -442,8 +442,11 @@ export default function reducer( }; case InlineSearch.REQUEST: case InlineSearch.REQUEST_DEBOUNCE: + const inlineRequest = (action).payload; + return { ...state, + search_term: inlineRequest.term, inlineResults: { ...initialInlineResultsState, isLoading: true, diff --git a/frontend/amundsen_application/static/js/ducks/search/utils.ts b/frontend/amundsen_application/static/js/ducks/search/utils.ts index 499ca1112b..9547c7a1a3 100644 --- a/frontend/amundsen_application/static/js/ducks/search/utils.ts +++ b/frontend/amundsen_application/static/js/ducks/search/utils.ts @@ -1,6 +1,7 @@ import { GlobalState } from 'ducks/rootReducer'; import { SearchReducerState } from 'ducks/search/reducer'; import { DEFAULT_RESOURCE_TYPE, ResourceType } from 'interfaces/Resources'; +import { SearchResults } from './types'; export const getSearchState = (state: GlobalState): SearchReducerState => state.search; @@ -13,23 +14,30 @@ and the shape of the state happend to be the same because a piece of application can be the combination of multiple responses. */ -export const getPageIndex = ( +export const getResults = ( state: Partial, resource?: ResourceType -) => { +): SearchResults | undefined => { resource = resource || state.resource; switch (resource) { case ResourceType.table: - return state.tables?.page_index || 0; + return state.tables; case ResourceType.user: - return state.users?.page_index || 0; + return state.users; case ResourceType.dashboard: - return state.dashboards?.page_index || 0; + return state.dashboards; + case ResourceType.feature: + return state.features; + default: + return undefined; } - - return 0; }; +export const getPageIndex = ( + state: Partial, + resource?: ResourceType +) => getResults(state, resource)?.page_index || 0; + export const autoSelectResource = (state: Partial) => { if (state.tables && state.tables.total_results > 0) { return ResourceType.table; diff --git a/frontend/amundsen_application/static/js/features/SearchBar/InlineSearchResults/ResultItemList/index.tsx b/frontend/amundsen_application/static/js/features/SearchBar/InlineSearchResults/ResultItemList/index.tsx index 99800d4b90..0df1aa5240 100644 --- a/frontend/amundsen_application/static/js/features/SearchBar/InlineSearchResults/ResultItemList/index.tsx +++ b/frontend/amundsen_application/static/js/features/SearchBar/InlineSearchResults/ResultItemList/index.tsx @@ -5,13 +5,16 @@ import * as React from 'react'; import { ResourceType } from 'interfaces'; import { logClick } from 'utils/analytics'; +import { LogSearchEventRequest } from 'ducks/log/types'; +import { bindActionCreators } from 'redux'; +import { logSearchEvent } from 'ducks/log/reducer'; +import { connect } from 'react-redux'; +import ResultItem from './ResultItem'; import { RESULT_LIST_FOOTER_PREFIX, RESULT_LIST_FOOTER_SUFFIX, } from '../constants'; -import ResultItem from './ResultItem'; - export interface SuggestedResult { href: string; iconClass: string; @@ -20,7 +23,7 @@ export interface SuggestedResult { type: string; } -export interface ResultItemListProps { +export interface OwnProps { onItemSelect: (resourceType: ResourceType, updateUrl?: boolean) => void; resourceType: ResourceType; searchTerm: string; @@ -29,7 +32,21 @@ export interface ResultItemListProps { totalResults: number; } -class ResultItemList extends React.Component { +export interface DispatchFromProps { + logSearchEvent: ( + resourceLink: string, + resourceType: ResourceType, + source: string, + index: number, + event: any, + inline: boolean, + extra?: { [key: string]: any } + ) => LogSearchEventRequest; +} + +export interface ResultItemListProps extends OwnProps, DispatchFromProps {} + +export class ResultItemList extends React.Component { generateFooterLinkText = () => { const { totalResults, title } = this.props; @@ -44,9 +61,10 @@ class ResultItemList extends React.Component { }; renderResultItems = (results: SuggestedResult[]) => { - const onResultItemSelect = (e) => { - logClick(e); - const { resourceType, onItemSelect } = this.props; + const onResultItemSelect = (e, item, index) => { + const { resourceType, onItemSelect, logSearchEvent } = this.props; + + logSearchEvent(item.href, resourceType, resourceType, index, e, true); onItemSelect(resourceType, true); }; @@ -61,7 +79,7 @@ class ResultItemList extends React.Component { key={id} id={id} href={href} - onItemSelect={onResultItemSelect} + onItemSelect={(e) => onResultItemSelect(e, item, index)} iconClass={`icon icon-dark ${iconClass}`} subtitle={subtitle} titleNode={titleNode} @@ -97,4 +115,18 @@ class ResultItemList extends React.Component { }; } -export default ResultItemList; +export const mapDispatchToProps = (dispatch: any): DispatchFromProps => { + const dispatchableActions: DispatchFromProps = bindActionCreators( + { + logSearchEvent, + }, + dispatch + ); + + return dispatchableActions; +}; + +export default connect<{}, DispatchFromProps, OwnProps>( + null, + mapDispatchToProps +)(ResultItemList); diff --git a/frontend/amundsen_application/static/js/features/SearchBar/InlineSearchResults/ResultItemList/tests/index.spec.tsx b/frontend/amundsen_application/static/js/features/SearchBar/InlineSearchResults/ResultItemList/tests/index.spec.tsx index 405bc51833..8ab5a94b0f 100644 --- a/frontend/amundsen_application/static/js/features/SearchBar/InlineSearchResults/ResultItemList/tests/index.spec.tsx +++ b/frontend/amundsen_application/static/js/features/SearchBar/InlineSearchResults/ResultItemList/tests/index.spec.tsx @@ -6,7 +6,7 @@ import * as React from 'react'; import { shallow } from 'enzyme'; import { ResourceType } from 'interfaces'; -import ResultItemList, { ResultItemListProps } from '..'; +import { ResultItemList, ResultItemListProps } from '..'; import { RESULT_LIST_FOOTER_PREFIX, @@ -41,6 +41,7 @@ describe('ResultItemList', () => { ], title: 'Datasets', totalResults: 10, + logSearchEvent: jest.fn(), ...propOverrides, }; const wrapper = shallow();