Merge branch 'main' into spaces/flaky-selection-test

.buildkite/scripts/steps/package_testing/test.sh

@@ -41,9 +41,9 @@ trap "echoKibanaLogs" EXIT
 
 vagrant provision "$TEST_PACKAGE"
 
-export TEST_BROWSER_HEADLESS=1
-export TEST_KIBANA_URL="http://elastic:changeme@$KIBANA_IP_ADDRESS:5601"
-export TEST_ES_URL=http://elastic:[email protected]:9200
+# export TEST_BROWSER_HEADLESS=1
+# export TEST_KIBANA_URL="http://elastic:changeme@$KIBANA_IP_ADDRESS:5601"
+# export TEST_ES_URL=http://elastic:[email protected]:9200
 
-cd x-pack
-node scripts/functional_test_runner.js --include-tag=smoke
+# cd x-pack
+# node scripts/functional_test_runner.js --include-tag=smoke

src/core/public/analytics/analytics_service.ts

@@ -9,6 +9,7 @@
 import type { AnalyticsClient } from '@kbn/analytics-client';
 import { createAnalytics } from '@kbn/analytics-client';
 import { of } from 'rxjs';
+import { trackClicks } from './track_clicks';
 import { InjectedMetadataSetup } from '../injected_metadata';
 import { CoreContext } from '../core_system';
 import { getSessionId } from './get_session_id';
@@ -53,6 +54,7 @@ export class AnalyticsService {
     // and can benefit other consumers of the client.
     this.registerSessionIdContext();
     this.registerBrowserInfoAnalyticsContext();
+    trackClicks(this.analyticsClient, core.env.mode.dev);
   }
 
   public setup({ injectedMetadata }: AnalyticsServiceSetupDeps): AnalyticsServiceSetup {

src/core/public/analytics/track_clicks.test.ts

@@ -0,0 +1,96 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { firstValueFrom, ReplaySubject } from 'rxjs';
+import { analyticsClientMock } from './analytics_service.test.mocks';
+import { trackClicks } from './track_clicks';
+import { take } from 'rxjs/operators';
+
+describe('trackClicks', () => {
+  const addEventListenerSpy = jest.spyOn(window, 'addEventListener');
+  const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  test('registers the analytics event type and a listener to the "click" events', () => {
+    trackClicks(analyticsClientMock, true);
+
+    expect(analyticsClientMock.registerEventType).toHaveBeenCalledTimes(1);
+    expect(analyticsClientMock.registerEventType).toHaveBeenCalledWith(
+      expect.objectContaining({
+        eventType: 'click',
+      })
+    );
+    expect(addEventListenerSpy).toHaveBeenCalledTimes(1);
+    expect(addEventListenerSpy).toHaveBeenCalledWith('click', expect.any(Function), undefined);
+  });
+
+  test('reports an analytics event when a click event occurs', async () => {
+    // Gather an actual "click" event
+    const event$ = new ReplaySubject<MouseEvent>(1);
+    const parent = document.createElement('div');
+    parent.setAttribute('data-test-subj', 'test-click-target-parent');
+    const element = document.createElement('button');
+    parent.appendChild(element);
+    element.setAttribute('data-test-subj', 'test-click-target');
+    element.innerText = 'test'; // Only to validate that it is not included in the event.
+    element.value = 'test'; // Only to validate that it is not included in the event.
+    element.addEventListener('click', (e) => event$.next(e));
+    element.click();
+    // Using an observable because the event might not be immediately available
+    const event = await firstValueFrom(event$.pipe(take(1)));
+    event$.complete(); // No longer needed
+
+    trackClicks(analyticsClientMock, true);
+    expect(addEventListenerSpy).toHaveBeenCalledTimes(1);
+
+    (addEventListenerSpy.mock.calls[0][1] as EventListener)(event);
+    expect(analyticsClientMock.reportEvent).toHaveBeenCalledTimes(1);
+    expect(analyticsClientMock.reportEvent).toHaveBeenCalledWith('click', {
+      target: [
+        'DIV',
+        'data-test-subj=test-click-target-parent',
+        'BUTTON',
+        'data-test-subj=test-click-target',
+      ],
+    });
+  });
+
+  test('handles any processing errors logging them in dev mode', async () => {
+    trackClicks(analyticsClientMock, true);
+    expect(addEventListenerSpy).toHaveBeenCalledTimes(1);
+
+    // A basic MouseEvent does not have a target and will fail the logic, making it go to the catch branch as intended.
+    (addEventListenerSpy.mock.calls[0][1] as EventListener)(new MouseEvent('click'));
+    expect(analyticsClientMock.reportEvent).toHaveBeenCalledTimes(0);
+    expect(consoleErrorSpy).toHaveBeenCalledTimes(1);
+    expect(consoleErrorSpy.mock.calls[0]).toMatchInlineSnapshot(`
+      Array [
+        "Failed to report the click event",
+        Object {
+          "error": [TypeError: Cannot read properties of null (reading 'parentElement')],
+          "event": MouseEvent {
+            "isTrusted": false,
+          },
+        },
+      ]
+    `);
+  });
+
+  test('swallows any processing errors when not in dev mode', async () => {
+    trackClicks(analyticsClientMock, false);
+    expect(addEventListenerSpy).toHaveBeenCalledTimes(1);
+
+    // A basic MouseEvent does not have a target and will fail the logic, making it go to the catch branch as intended.
+    (addEventListenerSpy.mock.calls[0][1] as EventListener)(new MouseEvent('click'));
+    expect(analyticsClientMock.reportEvent).toHaveBeenCalledTimes(0);
+    expect(consoleErrorSpy).toHaveBeenCalledTimes(0);
+  });
+});

src/core/public/analytics/track_clicks.ts

@@ -0,0 +1,77 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { fromEvent } from 'rxjs';
+import type { AnalyticsClient } from '@kbn/analytics-client';
+
+/** HTML attributes that should be skipped from reporting because they might contain user data */
+const POTENTIAL_PII_HTML_ATTRIBUTES = ['value'];
+
+/**
+ * Registers the event type "click" in the analytics client.
+ * Then it listens to all the "click" events in the UI and reports them with the `target` property being a
+ * full list of the element's and its parents' attributes. This allows
+ * @param analytics
+ */
+export function trackClicks(analytics: AnalyticsClient, isDevMode: boolean) {
+  analytics.registerEventType<{ target: string[] }>({
+    eventType: 'click',
+    schema: {
+      target: {
+        type: 'array',
+        items: {
+          type: 'keyword',
+          _meta: {
+            description:
+              'The attributes of the clicked element and all its parents in the form `{attr.name}={attr.value}`. It allows finding the clicked elements by looking up its attributes like "data-test-subj=my-button".',
+          },
+        },
+      },
+    },
+  });
+
+  // window or document?
+  // I tested it on multiple browsers and it seems to work the same.
+  // My assumption is that window captures other documents like iframes as well?
+  return fromEvent(window, 'click').subscribe((event) => {
+    try {
+      const target = event.target as HTMLElement;
+      analytics.reportEvent('click', { target: getTargetDefinition(target) });
+    } catch (error) {
+      if (isDevMode) {
+        // Defensively log the error in dev mode to catch any potential bugs.
+        // eslint-disable-next-line no-console
+        console.error(`Failed to report the click event`, { event, error });
+      }
+    }
+  });
+}
+
+/**
+ * Returns a list of strings consisting on the tag name and all the attributes of the element.
+ * Additionally, it recursively walks up the DOM tree to find all the parents' definitions and prepends them to the list.
+ *
+ * @example
+ * From
+ * ```html
+ * <div data-test-subj="my-parent">
+ *   <div data-test-subj="my-button" />
+ * </div>
+ * ```
+ * it returns ['DIV', 'data-test-subj=my-parent', 'DIV', 'data-test-subj=my-button']
+ * @param target The child node to start from.
+ */
+function getTargetDefinition(target: HTMLElement): string[] {
+  return [
+    ...(target.parentElement ? getTargetDefinition(target.parentElement) : []),
+    target.tagName,
+    ...[...target.attributes]
+      .filter((attr) => !POTENTIAL_PII_HTML_ATTRIBUTES.includes(attr.name))
+      .map((attr) => `${attr.name}=${attr.value}`),
+  ];
+}

src/plugins/chart_expressions/expression_heatmap/common/expression_functions/heatmap_legend.ts

@@ -8,6 +8,7 @@
 import { Position } from '@elastic/charts';
 import { i18n } from '@kbn/i18n';
 import type { ExpressionFunctionDefinition } from '@kbn/expressions-plugin/common';
+import { DEFAULT_LEGEND_SIZE, LegendSize } from '@kbn/visualizations-plugin/common/constants';
 import { EXPRESSION_HEATMAP_LEGEND_NAME } from '../constants';
 import { HeatmapLegendConfig, HeatmapLegendConfigResult } from '../types';
 
@@ -52,10 +53,19 @@ export const heatmapLegendConfig: ExpressionFunctionDefinition<
       }),
     },
     legendSize: {
-      types: ['number'],
+      types: ['string'],
+      default: DEFAULT_LEGEND_SIZE,
       help: i18n.translate('expressionHeatmap.function.args.legendSize.help', {
-        defaultMessage: 'Specifies the legend size in pixels.',
+        defaultMessage: 'Specifies the legend size.',
       }),
+      options: [
+        LegendSize.AUTO,
+        LegendSize.SMALL,
+        LegendSize.MEDIUM,
+        LegendSize.LARGE,
+        LegendSize.EXTRA_LARGE,
+      ],
+      strict: true,
     },
   },
   fn(input, args) {

src/plugins/chart_expressions/expression_heatmap/common/types/expression_functions.ts

@@ -15,6 +15,7 @@ import {
 import { ExpressionValueVisDimension } from '@kbn/visualizations-plugin/common';
 
 import { CustomPaletteState } from '@kbn/charts-plugin/common';
+import { LegendSize } from '@kbn/visualizations-plugin/public';
 import {
   EXPRESSION_HEATMAP_NAME,
   EXPRESSION_HEATMAP_LEGEND_NAME,
@@ -43,7 +44,7 @@ export interface HeatmapLegendConfig {
    * Exact legend width (vertical) or height (horizontal)
    * Limited to max of 70% of the chart container dimension Vertical legends limited to min of 30% of computed width
    */
-  legendSize?: number;
+  legendSize?: LegendSize;
 }
 
 export type HeatmapLegendConfigResult = HeatmapLegendConfig & {

src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx

@@ -17,6 +17,7 @@ import { findTestSubject } from '@elastic/eui/lib/test';
 import { act } from 'react-dom/test-utils';
 import { HeatmapRenderProps, HeatmapArguments } from '../../common';
 import HeatmapComponent from './heatmap_component';
+import { LegendSize } from '@kbn/visualizations-plugin/common';
 
 jest.mock('@elastic/charts', () => {
   const original = jest.requireActual('@elastic/charts');
@@ -47,6 +48,7 @@ const args: HeatmapArguments = {
     isVisible: true,
     position: 'top',
     type: 'heatmap_legend',
+    legendSize: LegendSize.SMALL,
   },
   gridConfig: {
     isCellLabelVisible: true,
@@ -119,6 +121,33 @@ describe('HeatmapComponent', function () {
     expect(component.find(Settings).prop('legendPosition')).toEqual('top');
   });
 
+  it('sets correct legend sizes', () => {
+    const component = shallowWithIntl(<HeatmapComponent {...wrapperProps} />);
+    expect(component.find(Settings).prop('legendSize')).toEqual(80);
+
+    component.setProps({
+      args: {
+        ...args,
+        legend: {
+          ...args.legend,
+          legendSize: LegendSize.AUTO,
+        },
+      },
+    });
+    expect(component.find(Settings).prop('legendSize')).toBeUndefined();
+
+    component.setProps({
+      args: {
+        ...args,
+        legend: {
+          ...args.legend,
+          legendSize: undefined,
+        },
+      },
+    });
+    expect(component.find(Settings).prop('legendSize')).toEqual(130);
+  });
+
   it('renders the legend toggle component if uiState is set', async () => {
     const component = mountWithIntl(<HeatmapComponent {...wrapperProps} />);
     await actWithTimeout(async () => {

src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx

@@ -29,6 +29,10 @@ import {
   getAccessorByDimension,
   getFormatByAccessor,
 } from '@kbn/visualizations-plugin/common/utils';
+import {
+  DEFAULT_LEGEND_SIZE,
+  LegendSizeToPixels,
+} from '@kbn/visualizations-plugin/common/constants';
 import type { HeatmapRenderProps, FilterEvent, BrushEvent } from '../../common';
 import { applyPaletteParams, findMinMaxByColumnId, getSortPredicate } from './helpers';
 import {
@@ -485,7 +489,7 @@ export const HeatmapComponent: FC<HeatmapRenderProps> = memo(
               onElementClick={interactive ? (onElementClick as ElementClickListener) : undefined}
               showLegend={showLegend ?? args.legend.isVisible}
               legendPosition={args.legend.position}
-              legendSize={args.legend.legendSize}
+              legendSize={LegendSizeToPixels[args.legend.legendSize ?? DEFAULT_LEGEND_SIZE]}
               legendColorPicker={uiState ? LegendColorPickerWrapper : undefined}
               debugState={window._echDebugStateFlag ?? false}
               tooltip={tooltip}

src/plugins/chart_expressions/expression_partition_vis/common/expression_functions/i18n.ts

@@ -47,7 +47,7 @@ export const strings = {
     }),
   getLegendSizeArgHelp: () =>
     i18n.translate('expressionPartitionVis.reusable.function.args.legendSizeHelpText', {
-      defaultMessage: 'Specifies the legend size in pixels',
+      defaultMessage: 'Specifies the legend size',
     }),
   getNestedLegendArgHelp: () =>
     i18n.translate('expressionPartitionVis.reusable.function.args.nestedLegendHelpText', {