Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import React from 'react';
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { DataViewPicker } from '.';
import { useDispatch } from 'react-redux';
import { useKibana } from '../../../common/lib/kibana';
Expand Down Expand Up @@ -72,6 +72,7 @@ jest.mock('@kbn/unified-search-plugin/public', () => ({

describe('DataViewPicker', () => {
let mockDispatch = jest.fn();
const mockAddDanger = jest.fn();

beforeEach(() => {
jest.mocked(useUpdateUrlParam).mockReturnValue(jest.fn());
Expand All @@ -83,6 +84,11 @@ describe('DataViewPicker', () => {
jest.mocked(useKibana).mockReturnValue({
services: {
...mockUseKibana().services,
notifications: {
toasts: {
addDanger: mockAddDanger,
},
},
dataViewFieldEditor: { openEditor: jest.fn() },
dataViewEditor: {
userPermissions: { editDataView: jest.fn().mockReturnValue(true) },
Expand Down Expand Up @@ -177,6 +183,27 @@ describe('DataViewPicker', () => {
});
});

it('shows a danger toast when adding field fails to load data view', async () => {
jest
.mocked(useKibana().services.data.dataViews.get)
.mockRejectedValue(new Error('conflict loading data view'));

render(
<TestProviders>
<DataViewPicker scope={DataViewManagerScopeName.default} />
</TestProviders>
);

fireEvent.click(screen.getByTestId('addField'));

await waitFor(() => {
expect(mockAddDanger).toHaveBeenCalledWith({
title: 'Error retrieving data view',
text: 'Error: conflict loading data view',
});
});
});

describe('when user does not have editDataView permission', () => {
it('does not render edit data view button', () => {
jest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export const DataViewPicker = memo(({ scope, onClosePopover, disabled }: DataVie
const selectDataView = useSelectDataView();

const {
services: { dataViewEditor, data, dataViewFieldEditor, fieldFormats },
services: { dataViewEditor, data, dataViewFieldEditor, fieldFormats, notifications },
} = useKibana();

const canEditDataView = useMemo(
Expand Down Expand Up @@ -117,27 +117,35 @@ export const DataViewPicker = memo(({ scope, onClosePopover, disabled }: DataVie
return;
}

const dataViewInstance = await data.dataViews.get(dataViewId);
// Modifications to the fields do not trigger cache invalidation, but should as `fields` will be stale.
if (dataViewInstance.isPersisted?.()) {
data.dataViews.clearInstanceCache(dataViewId);
}
// We wrap dataViews.get within a try catch because we've seen errors happening with conflicting ids in the saved object api
try {
const dataViewInstance = await data.dataViews.get(dataViewId);
// Modifications to the fields do not trigger cache invalidation, but should as `fields` will be stale.
if (dataViewInstance.isPersisted?.()) {
data.dataViews.clearInstanceCache(dataViewId);
}

closeFieldEditor.current = await dataViewFieldEditor.openEditor({
ctx: {
dataView: dataViewInstance,
},
fieldName,
onSave: async () => {
if (!dataViewInstance.id) {
return;
}

closeFieldEditor.current = await dataViewFieldEditor.openEditor({
ctx: {
dataView: dataViewInstance,
},
fieldName,
onSave: async () => {
if (!dataViewInstance.id) {
return;
}

handleChangeDataView(dataViewInstance.id, dataViewInstance.getIndexPattern());
},
});
handleChangeDataView(dataViewInstance.id, dataViewInstance.getIndexPattern());
},
});
} catch (error) {
notifications.toasts.addDanger({
title: 'Error retrieving data view',
text: `Error: ${error instanceof Error ? error.message : 'unknown'}`,
Comment thread
PhilippeOberti marked this conversation as resolved.
});
}
},
[dataViewId, data.dataViews, dataViewFieldEditor, handleChangeDataView]
[dataViewId, data.dataViews, dataViewFieldEditor, handleChangeDataView, notifications]
);

const getDataViewHelpText = useCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export const useInitDataViewManager = () => {
dataViews: services.dataViews,
http: services.http,
uiSettings: services.uiSettings,
notifications: services.notifications,
application: services.application,
spaces: services.spaces,
storage: services.storage,
Expand All @@ -113,6 +114,7 @@ export const useInitDataViewManager = () => {
createDataViewSelectedListener({
scope,
dataViews: services.dataViews,
notifications: services.notifications,
storage: services.storage,
logger: createDataViewSelectedListenerLogger,
})
Expand Down Expand Up @@ -141,6 +143,7 @@ export const useInitDataViewManager = () => {
services.application,
services.dataViews,
services.http,
services.notifications,
createInitListenerLogger,
services.spaces,
services.storage,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type { RootState } from '../reducer';
import { DataViewManagerScopeName, DEFAULT_SECURITY_SOLUTION_DATA_VIEW_ID } from '../../constants';
import { DEFAULT_ALERT_DATA_VIEW_ID } from '../../../../common/constants';
import type { Storage } from '@kbn/kibana-utils-plugin/public';
import type { CoreStart } from '@kbn/core/public';

const mockDataViewsService = {
getDataViewLazy: jest.fn(),
Expand Down Expand Up @@ -88,6 +89,7 @@ const mockStorage = {
remove: jest.fn(),
clear: jest.fn(),
} as unknown as Storage;
const mockToastsDanger = jest.fn();

const mockListenerApi = {
dispatch: mockDispatch,
Expand All @@ -103,6 +105,11 @@ describe('createDataViewSelectedListener', () => {
jest.clearAllMocks();
listener = createDataViewSelectedListener({
dataViews: mockDataViewsService,
notifications: {
toasts: {
addDanger: mockToastsDanger,
},
} as unknown as CoreStart['notifications'],
scope: DataViewManagerScopeName.default,
logger: mockLogger,
storage: mockStorage,
Expand Down Expand Up @@ -181,6 +188,11 @@ describe('createDataViewSelectedListener', () => {
it('should store data view ID in storage when scope is analyzer', async () => {
const analyzerListener = createDataViewSelectedListener({
dataViews: mockDataViewsService,
notifications: {
toasts: {
addDanger: mockToastsDanger,
},
} as unknown as CoreStart['notifications'],
scope: DataViewManagerScopeName.analyzer,
logger: mockLogger,
storage: mockStorage,
Expand Down Expand Up @@ -209,6 +221,11 @@ describe('createDataViewSelectedListener', () => {
it('should store resolved data view ID from cached view when scope is analyzer', async () => {
const analyzerListener = createDataViewSelectedListener({
dataViews: mockDataViewsService,
notifications: {
toasts: {
addDanger: mockToastsDanger,
},
} as unknown as CoreStart['notifications'],
scope: DataViewManagerScopeName.analyzer,
logger: mockLogger,
storage: mockStorage,
Expand All @@ -225,4 +242,49 @@ describe('createDataViewSelectedListener', () => {
);
});
});

it('should show toast and fallback to default when getDataViewLazy fails without fallback patterns', async () => {
jest.mocked(mockDataViewsService.getDataViewLazy).mockRejectedValue(new Error('conflict'));

await listener.effect(
selectDataViewAsync({ id: 'conflicted-id', scope: DataViewManagerScopeName.default }),
mockListenerApi
);

expect(mockToastsDanger).toHaveBeenCalledWith({
title: 'Selected data view is unavailable',
text: 'Unable to load data view "conflicted-id". Using fallback selection when possible.',
});
expect(mockDataViewsService.create).not.toHaveBeenCalled();
expect(mockDispatch).toHaveBeenCalledWith(
expect.objectContaining({
payload: DEFAULT_SECURITY_SOLUTION_DATA_VIEW_ID,
type: 'x-pack/security_solution/dataViewManager/default/setSelectedDataView',
})
);
});

it('should clear stale analyzer storage value when getDataViewLazy fails for analyzer scope', async () => {
const analyzerListener = createDataViewSelectedListener({
dataViews: mockDataViewsService,
notifications: {
toasts: {
addDanger: mockToastsDanger,
},
} as unknown as CoreStart['notifications'],
scope: DataViewManagerScopeName.analyzer,
logger: mockLogger,
storage: mockStorage,
});
jest.mocked(mockDataViewsService.getDataViewLazy).mockRejectedValue(new Error('conflict'));

await analyzerListener.effect(
selectDataViewAsync({ id: 'conflicted-id', scope: DataViewManagerScopeName.analyzer }),
mockListenerApi
);

expect(mockStorage.remove).toHaveBeenCalledWith(
'securitySolution.dataViewManager.selectedDataView.analyzer'
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type { AnyAction, Dispatch, ListenerEffectAPI } from '@reduxjs/toolkit';
import type { Storage } from '@kbn/kibana-utils-plugin/public';
import { isEmpty } from 'lodash';
import type { Logger } from '@kbn/logging';
import type { CoreStart } from '@kbn/core/public';
import type { RootState } from '../reducer';
import { scopes } from '../reducer';
import { selectDataViewAsync } from '../actions';
Expand All @@ -37,6 +38,7 @@ import { DataViewManagerScopeName } from '../../constants';
export const createDataViewSelectedListener = (dependencies: {
scope: DataViewManagerScopeName;
dataViews: DataViewsServicePublic;
notifications: CoreStart['notifications'];
storage: Storage;
logger: Logger;
}) => {
Expand Down Expand Up @@ -103,18 +105,34 @@ export const createDataViewSelectedListener = (dependencies: {
}`
);

// If the data view is not found in the cache, attempt to fetch it by ID from the DataViews service.
// We wrap the dataViews.getDataViewLazy within a try catch because we've seen errors happening with conflicting ids in the saved object api
if (!cachedDataViewSpec) {
try {
if (action.payload.id) {
dataViewById = await dependencies.dataViews.getDataViewLazy(action.payload.id);
}
} catch (error: unknown) {
logger.error(`Error fetching data view by id ${action.payload.id}: ${error}`);
dependencies.notifications.toasts.addDanger({
title: 'Selected data view is unavailable',
text: `Unable to load data view ${
action.payload.id ? `"${action.payload.id}"` : ''
}. Using fallback selection when possible.`,
Comment thread
PhilippeOberti marked this conversation as resolved.
});

// This cleans local storage for the analyzer data view to prevent the error from happening again on page refresh.
if (action.payload.scope === DataViewManagerScopeName.analyzer && action.payload.id) {
dependencies.storage.remove(
`securitySolution.dataViewManager.selectedDataView.${action.payload.scope}`
);
}
dataViewByIdError = error;
}
}

if (!dataViewById) {
// We attempt to create an ad-hoc data view if the data view by id lookup fails, and fallback patterns are provided.
if (!dataViewById && action.payload.fallbackPatterns?.length) {
logger.debug(`Data view by id lookup failed for id: ${action.payload.id}`);
try {
const title = action.payload.fallbackPatterns?.join(',') ?? '';
Expand All @@ -134,6 +152,11 @@ export const createDataViewSelectedListener = (dependencies: {
logger.error(`Error creating ad-hoc data view: ${error}`);
adhocDataViewCreationError = error;
}
} else if (!dataViewById && !action.payload.fallbackPatterns?.length) {
// No need to create an ad-hoc data view if there are no fallback patterns
logger.debug(
`Skipping ad-hoc data view creation for scope ${dependencies.scope} because no fallback patterns were provided`
);
}

const resolvedIdToUse =
Expand Down
Loading