Merge branch 'master' into 70582-direct-package-upload-part-2

.github/PULL_REQUEST_TEMPLATE.md

@@ -9,8 +9,9 @@ Delete any items that are not applicable to this PR.
 - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
 - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials
 - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
-- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)
-- [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)
+- [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
+- [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
+- [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
 - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers)
 
 ### For maintainers

src/plugins/dashboard/public/index.ts

@@ -31,7 +31,12 @@ export {
 } from './application';
 export { DashboardConstants, createDashboardEditUrl } from './dashboard_constants';
 
-export { DashboardStart, DashboardUrlGenerator, DashboardFeatureFlagConfig } from './plugin';
+export {
+  DashboardSetup,
+  DashboardStart,
+  DashboardUrlGenerator,
+  DashboardFeatureFlagConfig,
+} from './plugin';
 export {
   DASHBOARD_APP_URL_GENERATOR,
   createDashboardUrlGenerator,

src/plugins/dashboard/public/plugin.tsx

@@ -145,7 +145,7 @@ interface StartDependencies {
   savedObjects: SavedObjectsStart;
 }
 
-export type Setup = void;
+export type DashboardSetup = void;
 
 export interface DashboardStart {
   getSavedDashboardLoader: () => SavedObjectLoader;
@@ -180,7 +180,7 @@ declare module '../../../plugins/ui_actions/public' {
 }
 
 export class DashboardPlugin
-  implements Plugin<Setup, DashboardStart, SetupDependencies, StartDependencies> {
+  implements Plugin<DashboardSetup, DashboardStart, SetupDependencies, StartDependencies> {
   constructor(private initializerContext: PluginInitializerContext) {}
 
   private appStateUpdater = new BehaviorSubject<AppUpdater>(() => ({}));
@@ -193,17 +193,8 @@ export class DashboardPlugin
 
   public setup(
     core: CoreSetup<StartDependencies, DashboardStart>,
-    {
-      share,
-      uiActions,
-      embeddable,
-      home,
-      kibanaLegacy,
-      urlForwarding,
-      data,
-      usageCollection,
-    }: SetupDependencies
-  ): Setup {
+    { share, uiActions, embeddable, home, urlForwarding, data, usageCollection }: SetupDependencies
+  ): DashboardSetup {
     this.dashboardFeatureFlagConfig = this.initializerContext.config.get<
       DashboardFeatureFlagConfig
     >();

src/plugins/discover/public/application/angular/doc_table/components/table_row.ts

@@ -18,14 +18,15 @@
  */
 
 import { find, template } from 'lodash';
+import { stringify } from 'query-string';
 import $ from 'jquery';
 import rison from 'rison-node';
 import '../../doc_viewer';
 
 import openRowHtml from './table_row/open.html';
 import detailsHtml from './table_row/details.html';
 
-import { dispatchRenderComplete } from '../../../../../../kibana_utils/public';
+import { dispatchRenderComplete, url } from '../../../../../../kibana_utils/public';
 import { DOC_HIDE_TIME_COLUMN_SETTING } from '../../../../../common';
 import cellTemplateHtml from '../components/table_row/cell.html';
 import truncateByHeightTemplateHtml from '../components/table_row/truncate_by_height.html';
@@ -49,7 +50,7 @@ interface LazyScope extends ng.IScope {
   [key: string]: any;
 }
 
-export function createTableRowDirective($compile: ng.ICompileService, $httpParamSerializer: any) {
+export function createTableRowDirective($compile: ng.ICompileService) {
   const cellTemplate = template(noWhiteSpace(cellTemplateHtml));
   const truncateByHeightTemplate = template(noWhiteSpace(truncateByHeightTemplateHtml));
 
@@ -114,26 +115,25 @@ export function createTableRowDirective($compile: ng.ICompileService, $httpParam
       };
 
       $scope.getContextAppHref = () => {
-        const path = `#/context/${encodeURIComponent($scope.indexPattern.id)}/${encodeURIComponent(
-          $scope.row._id
-        )}`;
         const globalFilters: any = getServices().filterManager.getGlobalFilters();
         const appFilters: any = getServices().filterManager.getAppFilters();
-        const hash = $httpParamSerializer({
-          _g: encodeURI(
-            rison.encode({
+
+        const hash = stringify(
+          url.encodeQuery({
+            _g: rison.encode({
               filters: globalFilters || [],
-            })
-          ),
-          _a: encodeURI(
-            rison.encode({
+            }),
+            _a: rison.encode({
               columns: $scope.columns,
               filters: (appFilters || []).map(esFilters.disableFilter),
-            })
-          ),
-        });
+            }),
+          }),
+          { encode: false, sort: false }
+        );
 
-        return `${path}?${hash}`;
+        return `#/context/${encodeURIComponent($scope.indexPattern.id)}/${encodeURIComponent(
+          $scope.row._id
+        )}?${hash}`;
       };
 
       // create a tr element that lists the value for each *column*

src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.ts

@@ -464,24 +464,58 @@ export const useField = <T>(
     [errors]
   );
 
+  /**
+   * Handler to update the state and make sure the component is still mounted.
+   * When resetting the form, some field might get unmounted (e.g. a toggle on "true" becomes "false" and now certain fields should not be in the DOM).
+   * In that scenario there is a race condition in the "reset" method below, because the useState() hook is not synchronous.
+   *
+   * A better approach would be to have the state in a reducer and being able to update all values in a single dispatch action.
+   */
+  const updateStateIfMounted = useCallback(
+    (
+      state: 'isPristine' | 'isValidating' | 'isChangingValue' | 'isValidated' | 'errors' | 'value',
+      nextValue: any
+    ) => {
+      if (isMounted.current === false) {
+        return;
+      }
+
+      switch (state) {
+        case 'value':
+          return setValue(nextValue);
+        case 'errors':
+          return setErrors(nextValue);
+        case 'isChangingValue':
+          return setIsChangingValue(nextValue);
+        case 'isPristine':
+          return setPristine(nextValue);
+        case 'isValidated':
+          return setIsValidated(nextValue);
+        case 'isValidating':
+          return setValidating(nextValue);
+      }
+    },
+    [setValue]
+  );
+
   const reset: FieldHook<T>['reset'] = useCallback(
     (resetOptions = { resetValue: true }) => {
       const { resetValue = true, defaultValue: updatedDefaultValue } = resetOptions;
 
-      setPristine(true);
-      setValidating(false);
-      setIsChangingValue(false);
-      setIsValidated(false);
-      setErrors([]);
+      updateStateIfMounted('isPristine', true);
+      updateStateIfMounted('isValidating', false);
+      updateStateIfMounted('isChangingValue', false);
+      updateStateIfMounted('isValidated', false);
+      updateStateIfMounted('errors', []);
 
       if (resetValue) {
         hasBeenReset.current = true;
         const newValue = deserializeValue(updatedDefaultValue ?? defaultValue);
-        setValue(newValue);
+        updateStateIfMounted('value', newValue);
         return newValue;
       }
     },
-    [setValue, deserializeValue, defaultValue]
+    [updateStateIfMounted, deserializeValue, defaultValue]
   );
 
   // Don't take into account non blocker validation. Some are just warning (like trying to add a wrong ComboBox item)

src/plugins/share/public/index.ts

@@ -40,4 +40,6 @@ export {
 
 import { SharePlugin } from './plugin';
 
+export { KibanaURL } from './kibana_url';
+
 export const plugin = () => new SharePlugin();

src/plugins/share/public/kibana_url.ts

@@ -0,0 +1,44 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// TODO: Replace this logic with KibanaURL once it is available.
+// https://github.com/elastic/kibana/issues/64497
+export class KibanaURL {
+  public readonly path: string;
+  public readonly appName: string;
+  public readonly appPath: string;
+
+  constructor(path: string) {
+    const match = path.match(/^.*\/app\/([^\/#]+)(.+)$/);
+
+    if (!match) {
+      throw new Error('Unexpected URL path.');
+    }
+
+    const [, appName, appPath] = match;
+
+    if (!appName || !appPath) {
+      throw new Error('Could not parse URL path.');
+    }
+
+    this.path = path;
+    this.appName = appName;
+    this.appPath = appPath;
+  }
+}

src/plugins/telemetry/server/fetcher.test.ts

@@ -23,19 +23,93 @@ import { coreMock } from '../../../core/server/mocks';
 
 describe('FetcherTask', () => {
   describe('sendIfDue', () => {
-    it('returns undefined and warns when it fails to get telemetry configs', async () => {
+    it('stops when it fails to get telemetry configs', async () => {
       const initializerContext = coreMock.createPluginInitializerContext({});
       const fetcherTask = new FetcherTask(initializerContext);
       const mockError = new Error('Some message.');
-      fetcherTask['getCurrentConfigs'] = async () => {
-        throw mockError;
-      };
+      const getCurrentConfigs = jest.fn().mockRejectedValue(mockError);
+      const fetchTelemetry = jest.fn();
+      const sendTelemetry = jest.fn();
+      Object.assign(fetcherTask, {
+        getCurrentConfigs,
+        fetchTelemetry,
+        sendTelemetry,
+      });
       const result = await fetcherTask['sendIfDue']();
       expect(result).toBe(undefined);
+      expect(getCurrentConfigs).toBeCalledTimes(1);
+      expect(fetchTelemetry).toBeCalledTimes(0);
+      expect(sendTelemetry).toBeCalledTimes(0);
       expect(fetcherTask['logger'].warn).toBeCalledTimes(1);
       expect(fetcherTask['logger'].warn).toHaveBeenCalledWith(
-        `Error fetching telemetry configs: ${mockError}`
+        `Error getting telemetry configs. (${mockError})`
       );
     });
+
+    it('stops when all collectors are not ready', async () => {
+      const initializerContext = coreMock.createPluginInitializerContext({});
+      const fetcherTask = new FetcherTask(initializerContext);
+      const getCurrentConfigs = jest.fn().mockResolvedValue({});
+      const areAllCollectorsReady = jest.fn().mockResolvedValue(false);
+      const shouldSendReport = jest.fn().mockReturnValue(true);
+      const fetchTelemetry = jest.fn();
+      const sendTelemetry = jest.fn();
+      const updateReportFailure = jest.fn();
+
+      Object.assign(fetcherTask, {
+        getCurrentConfigs,
+        areAllCollectorsReady,
+        shouldSendReport,
+        fetchTelemetry,
+        updateReportFailure,
+        sendTelemetry,
+      });
+
+      await fetcherTask['sendIfDue']();
+
+      expect(fetchTelemetry).toBeCalledTimes(0);
+      expect(sendTelemetry).toBeCalledTimes(0);
+
+      expect(areAllCollectorsReady).toBeCalledTimes(1);
+      expect(updateReportFailure).toBeCalledTimes(0);
+      expect(fetcherTask['logger'].warn).toBeCalledTimes(1);
+      expect(fetcherTask['logger'].warn).toHaveBeenCalledWith(
+        `Error fetching usage. (Error: Not all collectors are ready.)`
+      );
+    });
+
+    it('fetches usage and send telemetry', async () => {
+      const initializerContext = coreMock.createPluginInitializerContext({});
+      const fetcherTask = new FetcherTask(initializerContext);
+      const mockTelemetryUrl = 'mock_telemetry_url';
+      const mockClusters = ['cluster_1', 'cluster_2'];
+      const getCurrentConfigs = jest.fn().mockResolvedValue({
+        telemetryUrl: mockTelemetryUrl,
+      });
+      const areAllCollectorsReady = jest.fn().mockResolvedValue(true);
+      const shouldSendReport = jest.fn().mockReturnValue(true);
+
+      const fetchTelemetry = jest.fn().mockResolvedValue(mockClusters);
+      const sendTelemetry = jest.fn();
+      const updateReportFailure = jest.fn();
+
+      Object.assign(fetcherTask, {
+        getCurrentConfigs,
+        areAllCollectorsReady,
+        shouldSendReport,
+        fetchTelemetry,
+        updateReportFailure,
+        sendTelemetry,
+      });
+
+      await fetcherTask['sendIfDue']();
+
+      expect(areAllCollectorsReady).toBeCalledTimes(1);
+      expect(fetchTelemetry).toBeCalledTimes(1);
+      expect(sendTelemetry).toBeCalledTimes(2);
+      expect(sendTelemetry).toHaveBeenNthCalledWith(1, mockTelemetryUrl, mockClusters[0]);
+      expect(sendTelemetry).toHaveBeenNthCalledWith(2, mockTelemetryUrl, mockClusters[1]);
+      expect(updateReportFailure).toBeCalledTimes(0);
+    });
   });
 });