From d6eb1d14cee6737f041452faf56821c27cfe5981 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Tue, 21 Feb 2023 21:14:40 +0100 Subject: [PATCH 1/4] dont toast --- .../common/data_views/data_views.ts | 44 ++++++------------- 1 file changed, 14 insertions(+), 30 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 65d23292486f3..b5e72f74974f2 100644 --- a/src/plugins/data_views/common/data_views/data_views.ts +++ b/src/plugins/data_views/common/data_views/data_views.ts @@ -569,8 +569,8 @@ export class DataViewsService { }; /** - * Refresh field list for a given index pattern. - * @param indexPattern + * Refresh field list for a given data view. + * @param dataView * @param displayErrors - If set false, API consumer is responsible for displaying and handling errors. */ refreshFields = async (dataView: DataView, displayErrors: boolean = true) => { @@ -582,22 +582,19 @@ export class DataViewsService { await this.refreshFieldsFn(dataView); } catch (err) { if (err instanceof DataViewMissingIndices) { - this.onNotification( - { title: err.message, color: 'danger', iconType: 'alert' }, - `refreshFields:${dataView.getIndexPattern()}` + // If the index pattern is missing indices, we still want to show the user the fields + } else { + this.onError( + err, + { + title: i18n.translate('dataViews.fetchFieldErrorTitle', { + defaultMessage: 'Error fetching fields for data view {title} (ID: {id})', + values: { id: dataView.id, title: dataView.getIndexPattern() }, + }), + }, + dataView.getIndexPattern() ); } - - this.onError( - err, - { - title: i18n.translate('dataViews.fetchFieldErrorTitle', { - defaultMessage: 'Error fetching fields for data view {title} (ID: {id})', - values: { id: dataView.id, title: dataView.getIndexPattern() }, - }), - }, - dataView.getIndexPattern() - ); } }; @@ -635,12 +632,6 @@ export class DataViewsService { return { fields: this.fieldArrayToMap(updatedFieldList, fieldAttrs), indices }; } catch (err) { if (err instanceof DataViewMissingIndices) { - if (displayErrors) { - this.onNotification( - { title: err.message, color: 'danger', iconType: 'alert' }, - `refreshFieldSpecMap:${title}` - ); - } return {}; } @@ -803,14 +794,7 @@ export class DataViewsService { indices = fieldsAndIndices.indices; } catch (err) { if (err instanceof DataViewMissingIndices) { - this.onNotification( - { - title: err.message, - color: 'danger', - iconType: 'alert', - }, - `initFromSavedObject:${spec.title}` - ); + // } else { this.onError( err, From 0fc1962d73417eed79c2355cbfe283167fac2d30 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Tue, 21 Feb 2023 22:10:45 +0100 Subject: [PATCH 2/4] Add onMissingIndices callback --- .../common/data_views/data_views.test.ts | 24 +++++++++++++++++++ .../common/data_views/data_views.ts | 16 +++++++++++-- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/plugins/data_views/common/data_views/data_views.test.ts b/src/plugins/data_views/common/data_views/data_views.test.ts index 01e2b9b1d95f3..73fe2b389433f 100644 --- a/src/plugins/data_views/common/data_views/data_views.test.ts +++ b/src/plugins/data_views/common/data_views/data_views.test.ts @@ -19,6 +19,7 @@ import { IDataViewsApiClient, } from '../types'; import { stubbedSavedObjectIndexPattern } from '../data_view.stub'; +import { DataViewMissingIndices } from '../lib'; const createFieldsFetcher = () => ({ @@ -69,8 +70,10 @@ describe('IndexPatterns', () => { remove: jest.fn(), } as any as UiSettingsCommon; const indexPatternObj = { id: 'id', version: 'a', attributes: { title: 'title' } }; + const onMissingIndices = jest.fn(); beforeEach(() => { + jest.clearAllMocks(); savedObjectsClient = {} as SavedObjectsClientCommon; savedObjectsClient.find = jest.fn( () => Promise.resolve([indexPatternObj]) as Promise>> @@ -111,6 +114,7 @@ describe('IndexPatterns', () => { apiClient, fieldFormats, onNotification: () => {}, + onMissingIndices, onError: () => {}, onRedirectNoIndexPattern: () => {}, getCanSave: () => Promise.resolve(true), @@ -123,6 +127,7 @@ describe('IndexPatterns', () => { apiClient, fieldFormats, onNotification: () => {}, + onMissingIndices, onError: () => {}, onRedirectNoIndexPattern: () => {}, getCanSave: () => Promise.resolve(false), @@ -207,6 +212,9 @@ describe('IndexPatterns', () => { test('allowNoIndex flag preserves existing fields when index is missing', async () => { const id = '2'; + apiClient.getFieldsForWildcard = jest.fn().mockImplementation(async () => { + throw new DataViewMissingIndices('Catch me if you can!'); + }); setDocsourcePayload(id, { id: 'foo', version: 'foo', @@ -220,6 +228,22 @@ describe('IndexPatterns', () => { expect((await indexPatterns.get(id)).fields.length).toBe(1); }); + test('missing indices triggering onMissingIndices fn', async () => { + const id = '1'; + setDocsourcePayload(id, { + id: 'foo', + version: 'foo', + attributes: { + title: 'something', + }, + }); + apiClient.getFieldsForWildcard = jest.fn().mockImplementation(async () => { + throw new DataViewMissingIndices('Catch me if you can!'); + }); + await indexPatterns.get(id); + expect(onMissingIndices).toHaveBeenCalled(); + }); + test('savedObjectCache pre-fetches title, type, typeMeta', async () => { expect(await indexPatterns.getIds()).toEqual(['id']); expect(savedObjectsClient.find).toHaveBeenCalledWith({ 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 b5e72f74974f2..6e730deaac26e 100644 --- a/src/plugins/data_views/common/data_views/data_views.ts +++ b/src/plugins/data_views/common/data_views/data_views.ts @@ -103,6 +103,10 @@ export interface DataViewsServiceDeps { * Handler for service notifications */ onNotification: OnNotification; + /** + * Handler when there are missing indices + */ + onMissingIndices?: (error: Error) => void; /** * Handler for service errors */ @@ -303,6 +307,11 @@ export class DataViewsService { * @param key used to indicate uniqueness of the notification */ private onNotification: OnNotification; + /** + * Handler triggered when there are no indices + * @param error Error object + */ + private onMissingIndices?: (error: Error) => void; /* * Handler for service errors * @param error notification content in toast format @@ -332,6 +341,7 @@ export class DataViewsService { onError, getCanSave = () => Promise.resolve(false), getCanSaveAdvancedSettings, + onMissingIndices, } = deps; this.apiClient = apiClient; this.config = uiSettings; @@ -339,6 +349,7 @@ export class DataViewsService { this.fieldFormats = fieldFormats; this.onNotification = onNotification; this.onError = onError; + this.onMissingIndices = onMissingIndices; this.getCanSave = getCanSave; this.getCanSaveAdvancedSettings = getCanSaveAdvancedSettings; @@ -582,7 +593,7 @@ export class DataViewsService { await this.refreshFieldsFn(dataView); } catch (err) { if (err instanceof DataViewMissingIndices) { - // If the index pattern is missing indices, we still want to show the user the fields + this.onMissingIndices?.(err); } else { this.onError( err, @@ -632,6 +643,7 @@ export class DataViewsService { return { fields: this.fieldArrayToMap(updatedFieldList, fieldAttrs), indices }; } catch (err) { if (err instanceof DataViewMissingIndices) { + this.onMissingIndices?.(err); return {}; } @@ -794,7 +806,7 @@ export class DataViewsService { indices = fieldsAndIndices.indices; } catch (err) { if (err instanceof DataViewMissingIndices) { - // + this.onMissingIndices?.(err); } else { this.onError( err, From 406803896bd88bdb0fa2a5ea96f8d9562d1d881d Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Wed, 22 Feb 2023 11:49:25 +0100 Subject: [PATCH 3/4] Cleanup --- .../data_views/common/data_views/data_views.test.ts | 3 --- src/plugins/data_views/common/data_views/data_views.ts | 8 ++++++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/plugins/data_views/common/data_views/data_views.test.ts b/src/plugins/data_views/common/data_views/data_views.test.ts index 73fe2b389433f..05ca95b1adfe2 100644 --- a/src/plugins/data_views/common/data_views/data_views.test.ts +++ b/src/plugins/data_views/common/data_views/data_views.test.ts @@ -212,9 +212,6 @@ describe('IndexPatterns', () => { test('allowNoIndex flag preserves existing fields when index is missing', async () => { const id = '2'; - apiClient.getFieldsForWildcard = jest.fn().mockImplementation(async () => { - throw new DataViewMissingIndices('Catch me if you can!'); - }); setDocsourcePayload(id, { id: 'foo', version: 'foo', 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 6e730deaac26e..abfcd5d5f83ac 100644 --- a/src/plugins/data_views/common/data_views/data_views.ts +++ b/src/plugins/data_views/common/data_views/data_views.ts @@ -104,7 +104,8 @@ export interface DataViewsServiceDeps { */ onNotification: OnNotification; /** - * Handler when there are missing indices + * Handler triggered when there are no indices + * @param error Error object */ onMissingIndices?: (error: Error) => void; /** @@ -338,10 +339,10 @@ export class DataViewsService { apiClient, fieldFormats, onNotification, + onMissingIndices, onError, getCanSave = () => Promise.resolve(false), getCanSaveAdvancedSettings, - onMissingIndices, } = deps; this.apiClient = apiClient; this.config = uiSettings; @@ -646,6 +647,9 @@ export class DataViewsService { this.onMissingIndices?.(err); return {}; } + if (!displayErrors) { + throw err; + } this.onError( err, From 23d41fcec091cb381fd191ee5b08340170538359 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Wed, 22 Feb 2023 13:21:20 +0100 Subject: [PATCH 4/4] Implement simpler approach --- .../common/data_views/data_views.test.ts | 24 +++++++++++++------ .../common/data_views/data_views.ts | 19 ++++----------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/plugins/data_views/common/data_views/data_views.test.ts b/src/plugins/data_views/common/data_views/data_views.test.ts index 05ca95b1adfe2..e5e9b2d83faf7 100644 --- a/src/plugins/data_views/common/data_views/data_views.test.ts +++ b/src/plugins/data_views/common/data_views/data_views.test.ts @@ -23,7 +23,7 @@ import { DataViewMissingIndices } from '../lib'; const createFieldsFetcher = () => ({ - getFieldsForWildcard: jest.fn(async () => ({ fields: [], indices: [] })), + getFieldsForWildcard: jest.fn(async () => ({ fields: [], indices: ['test'] })), } as any as IDataViewsApiClient); const fieldFormats = fieldFormatsMock; @@ -70,7 +70,6 @@ describe('IndexPatterns', () => { remove: jest.fn(), } as any as UiSettingsCommon; const indexPatternObj = { id: 'id', version: 'a', attributes: { title: 'title' } }; - const onMissingIndices = jest.fn(); beforeEach(() => { jest.clearAllMocks(); @@ -114,7 +113,6 @@ describe('IndexPatterns', () => { apiClient, fieldFormats, onNotification: () => {}, - onMissingIndices, onError: () => {}, onRedirectNoIndexPattern: () => {}, getCanSave: () => Promise.resolve(true), @@ -127,7 +125,6 @@ describe('IndexPatterns', () => { apiClient, fieldFormats, onNotification: () => {}, - onMissingIndices, onError: () => {}, onRedirectNoIndexPattern: () => {}, getCanSave: () => Promise.resolve(false), @@ -225,7 +222,20 @@ describe('IndexPatterns', () => { expect((await indexPatterns.get(id)).fields.length).toBe(1); }); - test('missing indices triggering onMissingIndices fn', async () => { + test('existing indices, so dataView.matchedIndices.length equals 1 ', async () => { + const id = '1'; + setDocsourcePayload(id, { + id: 'foo', + version: 'foo', + attributes: { + title: 'something', + }, + }); + const dataView = await indexPatterns.get(id); + expect(dataView.matchedIndices.length).toBe(1); + }); + + test('missing indices, so dataView.matchedIndices.length equals 0 ', async () => { const id = '1'; setDocsourcePayload(id, { id: 'foo', @@ -237,8 +247,8 @@ describe('IndexPatterns', () => { apiClient.getFieldsForWildcard = jest.fn().mockImplementation(async () => { throw new DataViewMissingIndices('Catch me if you can!'); }); - await indexPatterns.get(id); - expect(onMissingIndices).toHaveBeenCalled(); + const dataView = await indexPatterns.get(id); + expect(dataView.matchedIndices.length).toBe(0); }); test('savedObjectCache pre-fetches title, type, typeMeta', async () => { 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 abfcd5d5f83ac..afccfdcee79fc 100644 --- a/src/plugins/data_views/common/data_views/data_views.ts +++ b/src/plugins/data_views/common/data_views/data_views.ts @@ -103,11 +103,6 @@ export interface DataViewsServiceDeps { * Handler for service notifications */ onNotification: OnNotification; - /** - * Handler triggered when there are no indices - * @param error Error object - */ - onMissingIndices?: (error: Error) => void; /** * Handler for service errors */ @@ -308,11 +303,6 @@ export class DataViewsService { * @param key used to indicate uniqueness of the notification */ private onNotification: OnNotification; - /** - * Handler triggered when there are no indices - * @param error Error object - */ - private onMissingIndices?: (error: Error) => void; /* * Handler for service errors * @param error notification content in toast format @@ -339,7 +329,6 @@ export class DataViewsService { apiClient, fieldFormats, onNotification, - onMissingIndices, onError, getCanSave = () => Promise.resolve(false), getCanSaveAdvancedSettings, @@ -350,7 +339,6 @@ export class DataViewsService { this.fieldFormats = fieldFormats; this.onNotification = onNotification; this.onError = onError; - this.onMissingIndices = onMissingIndices; this.getCanSave = getCanSave; this.getCanSaveAdvancedSettings = getCanSaveAdvancedSettings; @@ -594,7 +582,7 @@ export class DataViewsService { await this.refreshFieldsFn(dataView); } catch (err) { if (err instanceof DataViewMissingIndices) { - this.onMissingIndices?.(err); + // not considered an error, check dataView.matchedIndices.length to be 0 } else { this.onError( err, @@ -644,7 +632,7 @@ export class DataViewsService { return { fields: this.fieldArrayToMap(updatedFieldList, fieldAttrs), indices }; } catch (err) { if (err instanceof DataViewMissingIndices) { - this.onMissingIndices?.(err); + // not considered an error, check dataView.matchedIndices.length to be 0 return {}; } if (!displayErrors) { @@ -805,12 +793,13 @@ export class DataViewsService { const fieldsAndIndices = await this.initFromSavedObjectLoadFields({ savedObjectId: savedObject.id, spec, + displayErrors, }); fields = fieldsAndIndices.fields; indices = fieldsAndIndices.indices; } catch (err) { if (err instanceof DataViewMissingIndices) { - this.onMissingIndices?.(err); + // not considered an error, check dataView.matchedIndices.length to be 0 } else { this.onError( err,