Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Enterprise Search] Telemetry: refactor to Kea logic file #81926

Merged
merged 5 commits into from
Nov 3, 2020
Merged

[Enterprise Search] Telemetry: refactor to Kea logic file #81926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3a574a5
Add new TelemetryLogic helpers
cee-chen Oct 27, 2020
d666707
Update all previous sendTelemetry fns to use new logic actions
cee-chen Oct 27, 2020
4a71269
Update unit tests for updated components
cee-chen Oct 27, 2020
580481e
Cleanup: Remove old sendTelemetry fn
cee-chen Oct 27, 2020
b1fb8eb
[PR feedback] Correctly assert the async thrown error
cee-chen Nov 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions x-pack/plugins/enterprise_search/public/applications/__mocks__/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export { mockHistory, mockLocation } from './react_router_history.mock';
export { mockKibanaValues } from './kibana_logic.mock';
export { mockLicensingValues } from './licensing_logic.mock';
export { mockHttpValues } from './http_logic.mock';
export { mockTelemetryActions } from './telemetry_logic.mock';
export { mockFlashMessagesValues, mockFlashMessagesActions } from './flash_messages_logic.mock';
export { mockAllValues, mockAllActions, setMockValues, setMockActions } from './kea.mock';

Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/enterprise_search/public/applications/__mocks__/kea.mock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import { mockKibanaValues } from './kibana_logic.mock';
import { mockLicensingValues } from './licensing_logic.mock';
import { mockHttpValues } from './http_logic.mock';
import { mockTelemetryActions } from './telemetry_logic.mock';
import { mockFlashMessagesValues, mockFlashMessagesActions } from './flash_messages_logic.mock';

export const mockAllValues = {
Expand All @@ -22,6 +23,7 @@ export const mockAllValues = {
...mockFlashMessagesValues,
};
export const mockAllActions = {
...mockTelemetryActions,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you add these here if you just end up importing telementry_logic.mock directly in all of the tests?

Copy link
Contributor Author

@cee-chen cee-chen Oct 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that way you import a single __mocks__/kea.mock.ts to mock all possible shared logic file values/actions at once instead of having to manually and tediously mock every logic file that you need.

So for example, in product_card.test.tsx:

https://github.com/elastic/kibana/blob/1db25d72bdfbc9782ee76675a5256593160acdc4/x-pack/plugins/enterprise_search/public/applications/enterprise_search/components/product_card/product_card.test.tsx#L7-L8

This file calls both KibanaLogic and TelemetryLogic. Because we have this shared kea.mock.ts helper, we only have to import it once and now the test file gracefully handles all instances of useValues and useActions, returning sane defaults that we can either spy on or override as needed (via our exported mock*Values/mock*Actions objs).

...mockFlashMessagesActions,
};

Expand Down
12 changes: 12 additions & 0 deletions x-pack/plugins/enterprise_search/public/applications/__mocks__/telemetry_logic.mock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

export const mockTelemetryActions = {
sendTelemetry: jest.fn(),
sendEnterpriseSearchTelemetry: jest.fn(),
sendAppSearchTelemetry: jest.fn(),
sendWorkplaceSearchTelemetry: jest.fn(),
};
10 changes: 2 additions & 8 deletions ...public/applications/app_search/components/engine_overview/components/empty_state.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,12 @@
*/

import '../../../../__mocks__/kea.mock';
import { mockTelemetryActions } from '../../../../__mocks__';

import React from 'react';
import { shallow } from 'enzyme';
import { EuiEmptyPrompt, EuiButton } from '@elastic/eui';

jest.mock('../../../../shared/telemetry', () => ({
sendTelemetry: jest.fn(),
SendAppSearchTelemetry: jest.fn(),
}));
import { sendTelemetry } from '../../../../shared/telemetry';

import { EmptyState } from './';

describe('EmptyState', () => {
Expand All @@ -31,7 +26,6 @@ describe('EmptyState', () => {
const button = prompt.find(EuiButton);

button.simulate('click');
expect(sendTelemetry).toHaveBeenCalled();
(sendTelemetry as jest.Mock).mockClear();
expect(mockTelemetryActions.sendAppSearchTelemetry).toHaveBeenCalled();
});
});
11 changes: 4 additions & 7 deletions ...arch/public/applications/app_search/components/engine_overview/components/empty_state.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@
*/

import React from 'react';
import { useValues } from 'kea';
import { useActions } from 'kea';
import { EuiPageContent, EuiEmptyPrompt, EuiButton } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';

import { sendTelemetry } from '../../../../shared/telemetry';
import { HttpLogic } from '../../../../shared/http';
import { TelemetryLogic } from '../../../../shared/telemetry';
import { getAppSearchUrl } from '../../../../shared/enterprise_search_url';
import { SetAppSearchChrome as SetPageChrome } from '../../../../shared/kibana_chrome';
import { CREATE_ENGINES_PATH } from '../../../routes';
Expand All @@ -20,15 +19,13 @@ import { EngineOverviewHeader } from './header';
import './empty_state.scss';

export const EmptyState: React.FC = () => {
const { http } = useValues(HttpLogic);
const { sendAppSearchTelemetry } = useActions(TelemetryLogic);

const buttonProps = {
href: getAppSearchUrl(CREATE_ENGINES_PATH),
target: '_blank',
onClick: () =>
sendTelemetry({
http,
product: 'app_search',
sendAppSearchTelemetry({
action: 'clicked',
metric: 'create_first_engine_button',
}),
Expand Down
6 changes: 2 additions & 4 deletions ...arch/public/applications/app_search/components/engine_overview/components/header.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,11 @@

import '../../../../__mocks__/kea.mock';
import '../../../../__mocks__/enterprise_search_url.mock';
import { mockTelemetryActions } from '../../../../__mocks__';

import React from 'react';
import { shallow } from 'enzyme';

jest.mock('../../../../shared/telemetry', () => ({ sendTelemetry: jest.fn() }));
import { sendTelemetry } from '../../../../shared/telemetry';

import { EngineOverviewHeader } from './';

describe('EngineOverviewHeader', () => {
Expand All @@ -29,6 +27,6 @@ describe('EngineOverviewHeader', () => {
expect(button.prop('isDisabled')).toBeFalsy();

button.simulate('click');
expect(sendTelemetry).toHaveBeenCalled();
expect(mockTelemetryActions.sendAppSearchTelemetry).toHaveBeenCalled();
});
});
11 changes: 4 additions & 7 deletions ...se_search/public/applications/app_search/components/engine_overview/components/header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import React from 'react';
import { useValues } from 'kea';
import { useActions } from 'kea';
import {
EuiPageHeader,
EuiPageHeaderSection,
Expand All @@ -16,12 +16,11 @@ import {
} from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';

import { sendTelemetry } from '../../../../shared/telemetry';
import { HttpLogic } from '../../../../shared/http';
import { TelemetryLogic } from '../../../../shared/telemetry';
import { getAppSearchUrl } from '../../../../shared/enterprise_search_url';

export const EngineOverviewHeader: React.FC = () => {
const { http } = useValues(HttpLogic);
const { sendAppSearchTelemetry } = useActions(TelemetryLogic);

const buttonProps = {
fill: true,
Expand All @@ -30,9 +29,7 @@ export const EngineOverviewHeader: React.FC = () => {
href: getAppSearchUrl(),
target: '_blank',
onClick: () =>
sendTelemetry({
http,
product: 'app_search',
sendAppSearchTelemetry({
action: 'clicked',
metric: 'header_launch_button',
}),
Expand Down
9 changes: 2 additions & 7 deletions ...se_search/public/applications/app_search/components/engine_overview/engine_table.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,11 @@

import '../../../__mocks__/kea.mock';
import '../../../__mocks__/enterprise_search_url.mock';
import { mockHttpValues, mountWithIntl } from '../../../__mocks__/';
import { mockTelemetryActions, mountWithIntl } from '../../../__mocks__/';

import React from 'react';
import { EuiBasicTable, EuiPagination, EuiButtonEmpty, EuiLink } from '@elastic/eui';

jest.mock('../../../shared/telemetry', () => ({ sendTelemetry: jest.fn() }));
import { sendTelemetry } from '../../../shared/telemetry';

import { EngineTable } from './engine_table';

describe('EngineTable', () => {
Expand Down Expand Up @@ -58,9 +55,7 @@ describe('EngineTable', () => {
expect(link.prop('href')).toEqual('http://localhost:3002/as/engines/test-engine');
link.simulate('click');

expect(sendTelemetry).toHaveBeenCalledWith({
http: mockHttpValues.http,
product: 'app_search',
expect(mockTelemetryActions.sendAppSearchTelemetry).toHaveBeenCalledWith({
action: 'clicked',
metric: 'engine_table_link',
});
Expand Down
11 changes: 4 additions & 7 deletions ...erprise_search/public/applications/app_search/components/engine_overview/engine_table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@
*/

import React from 'react';
import { useValues } from 'kea';
import { useActions } from 'kea';
import { EuiBasicTable, EuiBasicTableColumn, EuiLink } from '@elastic/eui';
import { FormattedMessage, FormattedDate, FormattedNumber } from '@kbn/i18n/react';
import { i18n } from '@kbn/i18n';

import { sendTelemetry } from '../../../shared/telemetry';
import { HttpLogic } from '../../../shared/http';
import { TelemetryLogic } from '../../../shared/telemetry';
import { getAppSearchUrl } from '../../../shared/enterprise_search_url';
import { getEngineRoute } from '../../routes';

Expand Down Expand Up @@ -42,15 +41,13 @@ export const EngineTable: React.FC<IEngineTableProps> = ({
data,
pagination: { totalEngines, pageIndex, onPaginate },
}) => {
const { http } = useValues(HttpLogic);
const { sendAppSearchTelemetry } = useActions(TelemetryLogic);

const engineLinkProps = (name: string) => ({
href: getAppSearchUrl(getEngineRoute(name)),
target: '_blank',
onClick: () =>
sendTelemetry({
http,
product: 'app_search',
sendAppSearchTelemetry({
action: 'clicked',
metric: 'engine_table_link',
}),
Expand Down
18 changes: 9 additions & 9 deletions ...earch/public/applications/enterprise_search/components/product_card/product_card.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import '../../../__mocks__/kea.mock';
import { mockTelemetryActions } from '../../../__mocks__';

import React from 'react';
import { useValues } from 'kea';
Expand All @@ -14,11 +15,6 @@ import { EuiCard } from '@elastic/eui';
import { EuiButton } from '../../../shared/react_router_helpers';
import { APP_SEARCH_PLUGIN, WORKPLACE_SEARCH_PLUGIN } from '../../../../../common/constants';

jest.mock('../../../shared/telemetry', () => ({
sendTelemetry: jest.fn(),
}));
import { sendTelemetry } from '../../../shared/telemetry';

import { ProductCard } from './';

describe('ProductCard', () => {
Expand All @@ -38,7 +34,10 @@ describe('ProductCard', () => {
expect(button.prop('children')).toEqual('Launch App Search');

button.simulate('click');
expect(sendTelemetry).toHaveBeenCalledWith(expect.objectContaining({ metric: 'app_search' }));
expect(mockTelemetryActions.sendEnterpriseSearchTelemetry).toHaveBeenCalledWith({
action: 'clicked',
metric: 'app_search',
});
});

it('renders a Workplace Search card', () => {
Expand All @@ -53,9 +52,10 @@ describe('ProductCard', () => {
expect(button.prop('children')).toEqual('Launch Workplace Search');

button.simulate('click');
expect(sendTelemetry).toHaveBeenCalledWith(
expect.objectContaining({ metric: 'workplace_search' })
);
expect(mockTelemetryActions.sendEnterpriseSearchTelemetry).toHaveBeenCalledWith({
action: 'clicked',
metric: 'workplace_search',
});
});

it('renders correct button text when host not present', () => {
Expand Down
11 changes: 4 additions & 7 deletions ...ise_search/public/applications/enterprise_search/components/product_card/product_card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@
*/

import React from 'react';
import { useValues } from 'kea';
import { useValues, useActions } from 'kea';
import { snakeCase } from 'lodash';
import { i18n } from '@kbn/i18n';
import { EuiCard, EuiTextColor } from '@elastic/eui';

import { EuiButton } from '../../../shared/react_router_helpers';
import { sendTelemetry } from '../../../shared/telemetry';
import { HttpLogic } from '../../../shared/http';
import { TelemetryLogic } from '../../../shared/telemetry';
import { KibanaLogic } from '../../../shared/kibana';

import './product_card.scss';
Expand All @@ -29,7 +28,7 @@ interface IProductCard {
}

export const ProductCard: React.FC<IProductCard> = ({ product, image }) => {
const { http } = useValues(HttpLogic);
const { sendEnterpriseSearchTelemetry } = useActions(TelemetryLogic);
const { config } = useValues(KibanaLogic);

const LAUNCH_BUTTON_TEXT = i18n.translate(
Expand Down Expand Up @@ -69,9 +68,7 @@ export const ProductCard: React.FC<IProductCard> = ({ product, image }) => {
to={product.URL}
shouldNotCreateHref={true}
onClick={() =>
sendTelemetry({
http,
product: 'enterprise_search',
sendEnterpriseSearchTelemetry({
action: 'clicked',
metric: snakeCase(product.ID),
})
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/enterprise_search/public/applications/shared/telemetry/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

export { sendTelemetry } from './send_telemetry';
export { TelemetryLogic } from './telemetry_logic';
export {
SendEnterpriseSearchTelemetry,
SendAppSearchTelemetry,
Expand Down
Loading