From ca884452ba2aae7e42435727ab9a14cccd64e3c0 Mon Sep 17 00:00:00 2001 From: Matt Kime Date: Sat, 7 May 2022 23:43:51 -0500 Subject: [PATCH 1/7] debounce data view toasts --- .../common/data_views/data_views.ts | 28 ++++++++++++----- src/plugins/data_views/common/types.ts | 2 +- src/plugins/data_views/public/plugin.ts | 9 ++++-- .../data_views/public/toasts_debounced.ts | 30 +++++++++++++++++++ 4 files changed, 58 insertions(+), 11 deletions(-) create mode 100644 src/plugins/data_views/public/toasts_debounced.ts diff --git a/src/plugins/data_views/common/data_views/data_views.ts b/src/plugins/data_views/common/data_views/data_views.ts index 9a59179ef4a35..a1fea7cafb411 100644 --- a/src/plugins/data_views/common/data_views/data_views.ts +++ b/src/plugins/data_views/common/data_views/data_views.ts @@ -333,7 +333,10 @@ export class DataViewsService { indexPattern.fields.replaceAll(fieldsWithSavedAttrs); } catch (err) { if (err instanceof DataViewMissingIndices) { - this.onNotification({ title: err.message, color: 'danger', iconType: 'alert' }); + this.onNotification( + { title: err.message, color: 'danger', iconType: 'alert' }, + `refreshFields:${indexPattern.title}` + ); } this.onError(err, { @@ -378,7 +381,10 @@ export class DataViewsService { return this.fieldArrayToMap(updatedFieldList, fieldAttrs); } catch (err) { if (err instanceof DataViewMissingIndices) { - this.onNotification({ title: err.message, color: 'danger', iconType: 'alert' }); + this.onNotification( + { title: err.message, color: 'danger', iconType: 'alert' }, + `refreshFieldSpecMap:${title}` + ); return {}; } @@ -530,11 +536,14 @@ export class DataViewsService { } } catch (err) { if (err instanceof DataViewMissingIndices) { - this.onNotification({ - title: err.message, - color: 'danger', - iconType: 'alert', - }); + this.onNotification( + { + title: err.message, + color: 'danger', + iconType: 'alert', + }, + `initFromSavedObject:${title}` + ); } else { this.onError(err, { title: i18n.translate('dataViews.fetchFieldErrorTitle', { @@ -718,7 +727,10 @@ export class DataViewsService { 'Unable to write data view! Refresh the page to get the most up to date changes for this data view.', }); - this.onNotification({ title, color: 'danger' }); + this.onNotification( + { title, color: 'danger' }, + `updateSavedObject:${indexPattern.title}` + ); throw err; } diff --git a/src/plugins/data_views/common/types.ts b/src/plugins/data_views/common/types.ts index 606128d484ddb..fe0e77831537a 100644 --- a/src/plugins/data_views/common/types.ts +++ b/src/plugins/data_views/common/types.ts @@ -123,7 +123,7 @@ export interface FieldAttrSet { count?: number; } -export type OnNotification = (toastInputFields: ToastInputFields) => void; +export type OnNotification = (toastInputFields: ToastInputFields, key: string) => void; export type OnError = (error: Error, toastInputFields: ErrorToastOptions) => void; export interface UiSettingsCommon { diff --git a/src/plugins/data_views/public/plugin.ts b/src/plugins/data_views/public/plugin.ts index eb05e2ab209fc..91a184240f766 100644 --- a/src/plugins/data_views/public/plugin.ts +++ b/src/plugins/data_views/public/plugin.ts @@ -25,6 +25,8 @@ import { import { DataViewsServicePublic } from './data_views_service_public'; import { HasData } from './services'; +import { toastsAddDebounced } from './toasts_debounced'; + export class DataViewsPublicPlugin implements Plugin< @@ -50,14 +52,17 @@ export class DataViewsPublicPlugin { fieldFormats }: DataViewsPublicStartDependencies ): DataViewsPublicPluginStart { const { uiSettings, http, notifications, savedObjects, theme, overlays, application } = core; + + const onNotifDebounced = toastsAddDebounced(notifications, uiSettings); + return new DataViewsServicePublic({ hasData: this.hasData.start(core), uiSettings: new UiSettingsPublicToCommon(uiSettings), savedObjectsClient: new SavedObjectsClientPublicToCommon(savedObjects.client), apiClient: new DataViewsApiClient(http), fieldFormats, - onNotification: (toastInputFields) => { - notifications.toasts.add(toastInputFields); + onNotification: (toastInputFields, key) => { + onNotifDebounced(toastInputFields, key); }, onError: notifications.toasts.addError.bind(notifications.toasts), onRedirectNoIndexPattern: onRedirectNoIndexPattern( diff --git a/src/plugins/data_views/public/toasts_debounced.ts b/src/plugins/data_views/public/toasts_debounced.ts new file mode 100644 index 0000000000000..b423a2335a12e --- /dev/null +++ b/src/plugins/data_views/public/toasts_debounced.ts @@ -0,0 +1,30 @@ +/* + * 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 { debounce } from 'lodash'; +import { CoreStart } from 'src/core/public'; +import type { OnNotification } from '../common/types'; + +export const toastsAddDebounced = ( + notifications: CoreStart['notifications'], + uiSettings: CoreStart['uiSettings'] +): OnNotification => { + const debouncerCollector: Record = {}; + return (toastInputFields, key) => { + if (!debouncerCollector[key]) { + debouncerCollector[key] = debounce( + notifications.toasts.add, + uiSettings.get('search:timeout') + 5000, + { + leading: true, + } + ); + } + return debouncerCollector[key](toastInputFields); + }; +}; From d28f8640529aa9adc475b36ac4e4074f314587fb Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Sun, 8 May 2022 05:33:03 +0000 Subject: [PATCH 2/7] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- src/plugins/data_views/public/toasts_debounced.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/data_views/public/toasts_debounced.ts b/src/plugins/data_views/public/toasts_debounced.ts index b423a2335a12e..5a15764b40f8e 100644 --- a/src/plugins/data_views/public/toasts_debounced.ts +++ b/src/plugins/data_views/public/toasts_debounced.ts @@ -7,7 +7,7 @@ */ import { debounce } from 'lodash'; -import { CoreStart } from 'src/core/public'; +import { CoreStart } from '@kbn/core/public'; import type { OnNotification } from '../common/types'; export const toastsAddDebounced = ( From 43a67c64b870a7f5cddc06afec45e8514b3787ff Mon Sep 17 00:00:00 2001 From: Matt Kime Date: Mon, 9 May 2022 14:11:33 -0500 Subject: [PATCH 3/7] more debounce --- .../common/data_views/data_views.ts | 48 ++++++++++++------- src/plugins/data_views/common/types.ts | 2 +- src/plugins/data_views/public/plugin.ts | 8 +++- .../data_views/public/toasts_debounced.ts | 23 ++++++++- 4 files changed, 58 insertions(+), 23 deletions(-) diff --git a/src/plugins/data_views/common/data_views/data_views.ts b/src/plugins/data_views/common/data_views/data_views.ts index a1fea7cafb411..e9c54320c497f 100644 --- a/src/plugins/data_views/common/data_views/data_views.ts +++ b/src/plugins/data_views/common/data_views/data_views.ts @@ -339,12 +339,16 @@ export class DataViewsService { ); } - this.onError(err, { - title: i18n.translate('dataViews.fetchFieldErrorTitle', { - defaultMessage: 'Error fetching fields for data view {title} (ID: {id})', - values: { id: indexPattern.id, title: indexPattern.title }, - }), - }); + this.onError( + err, + { + title: i18n.translate('dataViews.fetchFieldErrorTitle', { + defaultMessage: 'Error fetching fields for data view {title} (ID: {id})', + values: { id: indexPattern.id, title: indexPattern.title }, + }), + }, + indexPattern.title + ); } }; @@ -388,12 +392,16 @@ export class DataViewsService { return {}; } - this.onError(err, { - title: i18n.translate('dataViews.fetchFieldErrorTitle', { - defaultMessage: 'Error fetching fields for data view {title} (ID: {id})', - values: { id, title }, - }), - }); + this.onError( + err, + { + title: i18n.translate('dataViews.fetchFieldErrorTitle', { + defaultMessage: 'Error fetching fields for data view {title} (ID: {id})', + values: { id, title }, + }), + }, + title + ); throw err; } }; @@ -545,12 +553,16 @@ export class DataViewsService { `initFromSavedObject:${title}` ); } else { - this.onError(err, { - title: i18n.translate('dataViews.fetchFieldErrorTitle', { - defaultMessage: 'Error fetching fields for data view {title} (ID: {id})', - values: { id: savedObject.id, title }, - }), - }); + this.onError( + err, + { + title: i18n.translate('dataViews.fetchFieldErrorTitle', { + defaultMessage: 'Error fetching fields for data view {title} (ID: {id})', + values: { id: savedObject.id, title }, + }), + }, + title || '' + ); } } diff --git a/src/plugins/data_views/common/types.ts b/src/plugins/data_views/common/types.ts index fe0e77831537a..5897fbfc55777 100644 --- a/src/plugins/data_views/common/types.ts +++ b/src/plugins/data_views/common/types.ts @@ -124,7 +124,7 @@ export interface FieldAttrSet { } export type OnNotification = (toastInputFields: ToastInputFields, key: string) => void; -export type OnError = (error: Error, toastInputFields: ErrorToastOptions) => void; +export type OnError = (error: Error, toastInputFields: ErrorToastOptions, key: string) => void; export interface UiSettingsCommon { get: (key: string) => Promise; diff --git a/src/plugins/data_views/public/plugin.ts b/src/plugins/data_views/public/plugin.ts index 91a184240f766..da620c62d161a 100644 --- a/src/plugins/data_views/public/plugin.ts +++ b/src/plugins/data_views/public/plugin.ts @@ -25,7 +25,7 @@ import { import { DataViewsServicePublic } from './data_views_service_public'; import { HasData } from './services'; -import { toastsAddDebounced } from './toasts_debounced'; +import { toastsAddDebounced, toastsErrorDebounced } from './toasts_debounced'; export class DataViewsPublicPlugin implements @@ -54,6 +54,7 @@ export class DataViewsPublicPlugin const { uiSettings, http, notifications, savedObjects, theme, overlays, application } = core; const onNotifDebounced = toastsAddDebounced(notifications, uiSettings); + const onErrorDebounced = toastsErrorDebounced(notifications, uiSettings); return new DataViewsServicePublic({ hasData: this.hasData.start(core), @@ -64,7 +65,10 @@ export class DataViewsPublicPlugin onNotification: (toastInputFields, key) => { onNotifDebounced(toastInputFields, key); }, - onError: notifications.toasts.addError.bind(notifications.toasts), + onError: (error, toastInputFields, key) => { + onErrorDebounced(error, toastInputFields, key); + // notifications.toasts.addError.bind(notifications.toasts); + }, onRedirectNoIndexPattern: onRedirectNoIndexPattern( application.capabilities, application.navigateToApp, diff --git a/src/plugins/data_views/public/toasts_debounced.ts b/src/plugins/data_views/public/toasts_debounced.ts index b423a2335a12e..f948b9c65fb4c 100644 --- a/src/plugins/data_views/public/toasts_debounced.ts +++ b/src/plugins/data_views/public/toasts_debounced.ts @@ -8,7 +8,7 @@ import { debounce } from 'lodash'; import { CoreStart } from 'src/core/public'; -import type { OnNotification } from '../common/types'; +import type { OnNotification, OnError } from '../common/types'; export const toastsAddDebounced = ( notifications: CoreStart['notifications'], @@ -18,7 +18,7 @@ export const toastsAddDebounced = ( return (toastInputFields, key) => { if (!debouncerCollector[key]) { debouncerCollector[key] = debounce( - notifications.toasts.add, + notifications.toasts.add.bind(notifications.toasts.add), uiSettings.get('search:timeout') + 5000, { leading: true, @@ -28,3 +28,22 @@ export const toastsAddDebounced = ( return debouncerCollector[key](toastInputFields); }; }; + +export const toastsErrorDebounced = ( + notifications: CoreStart['notifications'], + uiSettings: CoreStart['uiSettings'] +): OnError => { + const debouncerCollector: Record = {}; + return (error, toastInputFields, key) => { + if (!debouncerCollector[key]) { + debouncerCollector[key] = debounce( + notifications.toasts.addError.bind(notifications.toasts), + uiSettings.get('search:timeout') + 5000, + { + leading: true, + } + ); + } + return debouncerCollector[key](error, toastInputFields); + }; +}; From 6c0154976ad8eff9506ab5f42e521b63029c8bcc Mon Sep 17 00:00:00 2001 From: Matt Kime Date: Mon, 9 May 2022 17:55:29 -0500 Subject: [PATCH 4/7] fix debounce --- src/plugins/data_views/public/toasts_debounced.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/data_views/public/toasts_debounced.ts b/src/plugins/data_views/public/toasts_debounced.ts index 008e4ad39c89c..5a031491c36a3 100644 --- a/src/plugins/data_views/public/toasts_debounced.ts +++ b/src/plugins/data_views/public/toasts_debounced.ts @@ -18,7 +18,7 @@ export const toastsAddDebounced = ( return (toastInputFields, key) => { if (!debouncerCollector[key]) { debouncerCollector[key] = debounce( - notifications.toasts.add.bind(notifications.toasts.add), + notifications.toasts.add.bind(notifications.toasts), uiSettings.get('search:timeout') + 5000, { leading: true, From 2624a423f8f4ccf8a75fcc641a055e835ce41cf4 Mon Sep 17 00:00:00 2001 From: Matt Kime Date: Thu, 12 May 2022 15:54:51 -0500 Subject: [PATCH 5/7] debounce tdata view toast usage --- .../data_views/public/debounce_by_key.test.ts | 31 ++++++++++++ .../data_views/public/debounce_by_key.ts | 24 +++++++++ src/plugins/data_views/public/plugin.ts | 17 ++++--- .../data_views/public/toasts_debounced.ts | 49 ------------------- 4 files changed, 66 insertions(+), 55 deletions(-) create mode 100644 src/plugins/data_views/public/debounce_by_key.test.ts create mode 100644 src/plugins/data_views/public/debounce_by_key.ts delete mode 100644 src/plugins/data_views/public/toasts_debounced.ts diff --git a/src/plugins/data_views/public/debounce_by_key.test.ts b/src/plugins/data_views/public/debounce_by_key.test.ts new file mode 100644 index 0000000000000..4ae23f1548d87 --- /dev/null +++ b/src/plugins/data_views/public/debounce_by_key.test.ts @@ -0,0 +1,31 @@ +/* + * 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 { debounceByKey } from './debounce_by_key'; + +describe('debounceByKey', () => { + test('debounce, confirm params', async () => { + const fn = jest.fn(); + const fn2 = jest.fn(); + + const debouncedFn = debounceByKey(fn, 100); + const debouncedFn2 = debounceByKey(fn2, 100); + + // debounces based on key, not params + debouncedFn('a')(1); + debouncedFn('a')(2); + + debouncedFn2('b')(2); + debouncedFn2('b')(1); + + expect(fn).toBeCalledTimes(1); + expect(fn).toBeCalledWith(1); + expect(fn2).toBeCalledTimes(1); + expect(fn2).toBeCalledWith(2); + }); +}); diff --git a/src/plugins/data_views/public/debounce_by_key.ts b/src/plugins/data_views/public/debounce_by_key.ts new file mode 100644 index 0000000000000..c8ae7094a6437 --- /dev/null +++ b/src/plugins/data_views/public/debounce_by_key.ts @@ -0,0 +1,24 @@ +/* + * 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 { debounce } from 'lodash'; + +export const debounceByKey = any>( + fn: F, + waitInMs: number +): ((key: string) => F) => { + const debouncerCollector: Record = {}; + return (key: string) => { + if (!debouncerCollector[key]) { + debouncerCollector[key] = debounce(fn, waitInMs, { + leading: true, + }); + } + return debouncerCollector[key]; + }; +}; diff --git a/src/plugins/data_views/public/plugin.ts b/src/plugins/data_views/public/plugin.ts index b2d03f3b7f36e..5c3ad2c33307d 100644 --- a/src/plugins/data_views/public/plugin.ts +++ b/src/plugins/data_views/public/plugin.ts @@ -25,7 +25,7 @@ import { import { DataViewsServicePublic } from './data_views_service_public'; import { HasData } from './services'; -import { toastsAddDebounced, toastsErrorDebounced } from './toasts_debounced'; +import { debounceByKey } from './debounce_by_key'; export class DataViewsPublicPlugin implements @@ -53,8 +53,14 @@ export class DataViewsPublicPlugin ): DataViewsPublicPluginStart { const { uiSettings, http, notifications, savedObjects, theme, overlays, application } = core; - const onNotifDebounced = toastsAddDebounced(notifications, uiSettings); - const onErrorDebounced = toastsErrorDebounced(notifications, uiSettings); + const onNotifDebounced = debounceByKey( + notifications.toasts.add.bind(notifications.toasts), + 10000 + ); + const onErrorDebounced = debounceByKey( + notifications.toasts.addError.bind(notifications.toasts), + 10000 + ); return new DataViewsServicePublic({ hasData: this.hasData.start(core), @@ -63,11 +69,10 @@ export class DataViewsPublicPlugin apiClient: new DataViewsApiClient(http), fieldFormats, onNotification: (toastInputFields, key) => { - onNotifDebounced(toastInputFields, key); + onNotifDebounced(key)(toastInputFields); }, onError: (error, toastInputFields, key) => { - onErrorDebounced(error, toastInputFields, key); - // notifications.toasts.addError.bind(notifications.toasts); + onErrorDebounced(key)(error, toastInputFields); }, onRedirectNoIndexPattern: onRedirectNoIndexPattern( application.capabilities, diff --git a/src/plugins/data_views/public/toasts_debounced.ts b/src/plugins/data_views/public/toasts_debounced.ts deleted file mode 100644 index 5a031491c36a3..0000000000000 --- a/src/plugins/data_views/public/toasts_debounced.ts +++ /dev/null @@ -1,49 +0,0 @@ -/* - * 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 { debounce } from 'lodash'; -import { CoreStart } from '@kbn/core/public'; -import type { OnNotification, OnError } from '../common/types'; - -export const toastsAddDebounced = ( - notifications: CoreStart['notifications'], - uiSettings: CoreStart['uiSettings'] -): OnNotification => { - const debouncerCollector: Record = {}; - return (toastInputFields, key) => { - if (!debouncerCollector[key]) { - debouncerCollector[key] = debounce( - notifications.toasts.add.bind(notifications.toasts), - uiSettings.get('search:timeout') + 5000, - { - leading: true, - } - ); - } - return debouncerCollector[key](toastInputFields); - }; -}; - -export const toastsErrorDebounced = ( - notifications: CoreStart['notifications'], - uiSettings: CoreStart['uiSettings'] -): OnError => { - const debouncerCollector: Record = {}; - return (error, toastInputFields, key) => { - if (!debouncerCollector[key]) { - debouncerCollector[key] = debounce( - notifications.toasts.addError.bind(notifications.toasts), - uiSettings.get('search:timeout') + 5000, - { - leading: true, - } - ); - } - return debouncerCollector[key](error, toastInputFields); - }; -}; From 99d5779b1d517d880d9617c5e8401f6824d1e822 Mon Sep 17 00:00:00 2001 From: Matt Kime Date: Thu, 12 May 2022 15:56:17 -0500 Subject: [PATCH 6/7] debounce tdata view toast u --- src/plugins/data_views/public/debounce_by_key.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/data_views/public/debounce_by_key.test.ts b/src/plugins/data_views/public/debounce_by_key.test.ts index 4ae23f1548d87..c5fba82fdcfdf 100644 --- a/src/plugins/data_views/public/debounce_by_key.test.ts +++ b/src/plugins/data_views/public/debounce_by_key.test.ts @@ -13,8 +13,8 @@ describe('debounceByKey', () => { const fn = jest.fn(); const fn2 = jest.fn(); - const debouncedFn = debounceByKey(fn, 100); - const debouncedFn2 = debounceByKey(fn2, 100); + const debouncedFn = debounceByKey(fn, 1000); + const debouncedFn2 = debounceByKey(fn2, 1000); // debounces based on key, not params debouncedFn('a')(1); From 6c3bd18a12b2888891d1b607f7cbccc6373abff5 Mon Sep 17 00:00:00 2001 From: Matt Kime Date: Thu, 12 May 2022 18:29:20 -0500 Subject: [PATCH 7/7] lets try adding some code comments --- src/plugins/data_views/common/data_views/data_views.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/plugins/data_views/common/data_views/data_views.ts b/src/plugins/data_views/common/data_views/data_views.ts index 99ba423eec6cb..b263c7d77fe3b 100644 --- a/src/plugins/data_views/common/data_views/data_views.ts +++ b/src/plugins/data_views/common/data_views/data_views.ts @@ -113,7 +113,17 @@ export class DataViewsService { private savedObjectsCache?: Array> | null; private apiClient: IDataViewsApiClient; private fieldFormats: FieldFormatsStartCommon; + /** + * Handler for service notifications + * @param toastInputFields notification content in toast format + * @param key used to indicate uniqueness of the notification + */ private onNotification: OnNotification; + /* + * Handler for service errors + * @param error notification content in toast format + * @param key used to indicate uniqueness of the error + */ private onError: OnError; private dataViewCache: ReturnType; public getCanSave: () => Promise;