[Telemetry] application usage views: allow tracking on any component and fix unmounting issues (#106507)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

packages/kbn-analytics/src/application_usage_tracker.ts

@@ -156,8 +156,9 @@ export class ApplicationUsageTracker {
     const appKey = this.createKey(this.currentAppId, viewId);
     const serializedKey = ApplicationUsageTracker.serializeKey(appKey);
     const appViewMetric = this.trackedApplicationViews[serializedKey];
-    this.sendMetricsToReporter([appViewMetric]);
-
-    delete this.trackedApplicationViews[serializedKey];
+    if (appViewMetric) {
+      this.sendMetricsToReporter([appViewMetric]);
+      delete this.trackedApplicationViews[serializedKey];
+    }
   }
 }

src/core/server/saved_objects/service/lib/repository.test.js

@@ -3814,6 +3814,25 @@ describe('SavedObjectsRepository', () => {
           expect.anything()
         );
       });
+
+      it('does not increment counter when incrementBy is 0', async () => {
+        await incrementCounterSuccess(type, id, [{ fieldName: counterFields[0], incrementBy: 0 }]);
+
+        expect(client.update).toBeCalledTimes(1);
+        expect(client.update).toBeCalledWith(
+          expect.objectContaining({
+            body: expect.objectContaining({
+              script: expect.objectContaining({
+                params: expect.objectContaining({
+                  counterFieldNames: [counterFields[0]],
+                  counts: [0],
+                }),
+              }),
+            }),
+          }),
+          expect.anything()
+        );
+      });
     });
   });
 

src/core/server/saved_objects/service/lib/repository.ts

@@ -1705,8 +1705,20 @@ export class SavedObjectsRepository {
     } = options;
 
     const normalizedCounterFields = counterFields.map((counterField) => {
-      const fieldName = typeof counterField === 'string' ? counterField : counterField.fieldName;
-      const incrementBy = typeof counterField === 'string' ? 1 : counterField.incrementBy || 1;
+      /**
+       * no counterField configs provided, instead a field name string was passed.
+       * ie `incrementCounter(so_type, id, ['my_field_name'])`
+       * Using the default of incrementing by 1
+       */
+      if (typeof counterField === 'string') {
+        return {
+          fieldName: counterField,
+          incrementBy: initialize ? 0 : 1,
+        };
+      }
+
+      const { incrementBy = 1, fieldName } = counterField;
+
       return {
         fieldName,
         incrementBy: initialize ? 0 : incrementBy,

src/plugins/usage_collection/public/components/track_application_view/track_application_view.test.tsx

@@ -7,22 +7,21 @@
  */
 
 import React from 'react';
-import { mountWithIntl } from '@kbn/test/jest';
 import { ApplicationUsageContext, TrackApplicationView } from './track_application_view';
 import { IApplicationUsageTracker } from '../../plugin';
-import { fireEvent } from '@testing-library/react';
+import { fireEvent, render } from '@testing-library/react';
 
 describe('TrackApplicationView', () => {
   test('it renders the internal component even when no tracker has been set', () => {
-    const component = mountWithIntl(
+    const { unmount } = render(
       <TrackApplicationView viewId={'testView'}>
         <h1>Hello</h1>
       </TrackApplicationView>
     );
-    component.unmount();
+    unmount();
   });
 
-  test('it tracks the component while it is rendered', () => {
+  test('it tracks the component while it is rendered', async () => {
     const applicationUsageTrackerMock: jest.Mocked<IApplicationUsageTracker> = {
       trackApplicationViewUsage: jest.fn(),
       flushTrackedView: jest.fn(),
@@ -30,7 +29,7 @@ describe('TrackApplicationView', () => {
     };
     expect(applicationUsageTrackerMock.trackApplicationViewUsage).not.toHaveBeenCalled();
     const viewId = 'testView';
-    const component = mountWithIntl(
+    const { findByText, unmount } = render(
       <ApplicationUsageContext.Provider value={applicationUsageTrackerMock}>
         <TrackApplicationView viewId={viewId}>
           <h1>Hello</h1>
@@ -39,10 +38,11 @@ describe('TrackApplicationView', () => {
     );
     expect(applicationUsageTrackerMock.trackApplicationViewUsage).toHaveBeenCalledWith(viewId);
     expect(applicationUsageTrackerMock.updateViewClickCounter).not.toHaveBeenCalled();
-    fireEvent.click(component.getDOMNode());
+    const element = await findByText('Hello');
+    fireEvent.click(element);
     expect(applicationUsageTrackerMock.updateViewClickCounter).toHaveBeenCalledWith(viewId);
     expect(applicationUsageTrackerMock.flushTrackedView).not.toHaveBeenCalled();
-    component.unmount();
+    unmount();
     expect(applicationUsageTrackerMock.flushTrackedView).toHaveBeenCalledWith(viewId);
   });
 });

src/plugins/usage_collection/public/components/track_application_view/track_application_view_component.test.tsx

@@ -7,22 +7,21 @@
  */
 
 import React from 'react';
-import { mountWithIntl } from '@kbn/test/jest';
 import { TrackApplicationViewComponent } from './track_application_view_component';
 import { IApplicationUsageTracker } from '../../plugin';
-import { fireEvent } from '@testing-library/react';
+import { fireEvent, render } from '@testing-library/react';
 
 describe('TrackApplicationViewComponent', () => {
   test('it renders the internal component even when no tracker is provided', () => {
-    const component = mountWithIntl(
+    const { unmount } = render(
       <TrackApplicationViewComponent viewId={'testView'}>
         <h1>Hello</h1>
       </TrackApplicationViewComponent>
     );
-    component.unmount();
+    unmount();
   });
 
-  test('it tracks the component while it is rendered', () => {
+  test('it tracks the component while it is rendered', async () => {
     const applicationUsageTrackerMock: jest.Mocked<IApplicationUsageTracker> = {
       trackApplicationViewUsage: jest.fn(),
       flushTrackedView: jest.fn(),
@@ -30,7 +29,7 @@ describe('TrackApplicationViewComponent', () => {
     };
     expect(applicationUsageTrackerMock.trackApplicationViewUsage).not.toHaveBeenCalled();
     const viewId = 'testView';
-    const component = mountWithIntl(
+    const { findByText, unmount } = render(
       <TrackApplicationViewComponent
         viewId={viewId}
         applicationUsageTracker={applicationUsageTrackerMock}
@@ -40,10 +39,11 @@ describe('TrackApplicationViewComponent', () => {
     );
     expect(applicationUsageTrackerMock.trackApplicationViewUsage).toHaveBeenCalledWith(viewId);
     expect(applicationUsageTrackerMock.updateViewClickCounter).not.toHaveBeenCalled();
-    fireEvent.click(component.getDOMNode());
+    const element = await findByText('Hello');
+    fireEvent.click(element);
     expect(applicationUsageTrackerMock.updateViewClickCounter).toHaveBeenCalledWith(viewId);
     expect(applicationUsageTrackerMock.flushTrackedView).not.toHaveBeenCalled();
-    component.unmount();
+    unmount();
     expect(applicationUsageTrackerMock.flushTrackedView).toHaveBeenCalledWith(viewId);
   });
 });

src/plugins/usage_collection/public/components/track_application_view/track_application_view_component.tsx

@@ -6,7 +6,7 @@
  * Side Public License, v 1.
  */
 
-import { Component } from 'react';
+import React from 'react';
 import ReactDOM from 'react-dom';
 import { IApplicationUsageTracker } from '../../plugin';
 import { TrackApplicationViewProps } from './types';
@@ -15,17 +15,22 @@ interface Props extends TrackApplicationViewProps {
   applicationUsageTracker?: IApplicationUsageTracker;
 }
 
-export class TrackApplicationViewComponent extends Component<Props> {
-  onClick = () => {
+export class TrackApplicationViewComponent extends React.Component<Props> {
+  private parentNode: (Node & ParentNode) | null | undefined;
+
+  onClick = (e: MouseEvent) => {
     const { applicationUsageTracker, viewId } = this.props;
-    applicationUsageTracker?.updateViewClickCounter(viewId);
+    this.parentNode = this.parentNode || ReactDOM.findDOMNode(this)?.parentNode;
+    if (this.parentNode === e.target || this.parentNode?.contains(e.target as Node | null)) {
+      applicationUsageTracker?.updateViewClickCounter(viewId);
+    }
   };
 
   componentDidMount() {
     const { applicationUsageTracker, viewId } = this.props;
     if (applicationUsageTracker) {
       applicationUsageTracker.trackApplicationViewUsage(viewId);
-      ReactDOM.findDOMNode(this)?.parentNode?.addEventListener('click', this.onClick);
+      document.addEventListener('click', this.onClick);
     }
   }
 
@@ -33,8 +38,8 @@ export class TrackApplicationViewComponent extends Component<Props> {
     const { applicationUsageTracker, viewId } = this.props;
     if (applicationUsageTracker) {
       applicationUsageTracker.flushTrackedView(viewId);
-      ReactDOM.findDOMNode(this)?.parentNode?.removeEventListener('click', this.onClick);
     }
+    document.removeEventListener('click', this.onClick);
   }
 
   render() {