[Security Solution] fix alerts page infinite loading state due to data view error

x-pack/solutions/security/plugins/security_solution/public/data_view_manager/components/data_view_picker/index.test.tsx

@@ -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());
@@ -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) },
@@ -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={PageScope.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

x-pack/solutions/security/plugins/security_solution/public/data_view_manager/components/data_view_picker/index.tsx

@@ -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(
@@ -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'}`,
+        });
+      }
     },
-    [dataViewId, data.dataViews, dataViewFieldEditor, handleChangeDataView]
+    [dataViewId, data.dataViews, dataViewFieldEditor, handleChangeDataView, notifications]
   );
 
   const getDataViewHelpText = useCallback(

x-pack/solutions/security/plugins/security_solution/public/data_view_manager/hooks/use_init_data_view_manager.ts

@@ -98,6 +98,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,
@@ -122,6 +123,7 @@ export const useInitDataViewManager = () => {
       createDataViewSelectedListener({
         scope,
         dataViews: services.dataViews,
+        notifications: services.notifications,
         storage: services.storage,
         logger: createDataViewSelectedListenerLogger,
       })
@@ -151,6 +153,7 @@ export const useInitDataViewManager = () => {
     services.application,
     services.dataViews,
     services.http,
+    services.notifications,
     createInitListenerLogger,
     services.spaces,
     services.storage,

x-pack/solutions/security/plugins/security_solution/public/data_view_manager/redux/listeners/data_view_selected.test.ts

@@ -14,6 +14,7 @@ import type { RootState } from '../reducer';
 import { DEFAULT_SECURITY_SOLUTION_DATA_VIEW_ID, PageScope } 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(),
@@ -92,6 +93,7 @@ const mockStorage = {
   remove: jest.fn(),
   clear: jest.fn(),
 } as unknown as Storage;
+const mockToastsDanger = jest.fn();
 
 const mockListenerApi = {
   dispatch: mockDispatch,
@@ -107,6 +109,11 @@ describe('createDataViewSelectedListener', () => {
     jest.clearAllMocks();
     listener = createDataViewSelectedListener({
       dataViews: mockDataViewsService,
+      notifications: {
+        toasts: {
+          addDanger: mockToastsDanger,
+        },
+      } as unknown as CoreStart['notifications'],
       logger: mockLogger,
       scope: PageScope.default,
       storage: mockStorage,
@@ -185,6 +192,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: PageScope.analyzer,
         logger: mockLogger,
         storage: mockStorage,
@@ -213,6 +225,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: PageScope.analyzer,
         logger: mockLogger,
         storage: mockStorage,
@@ -229,4 +246,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: PageScope.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: PageScope.analyzer,
+      logger: mockLogger,
+      storage: mockStorage,
+    });
+    jest.mocked(mockDataViewsService.getDataViewLazy).mockRejectedValue(new Error('conflict'));
+
+    await analyzerListener.effect(
+      selectDataViewAsync({ id: 'conflicted-id', scope: PageScope.analyzer }),
+      mockListenerApi
+    );
+
+    expect(mockStorage.remove).toHaveBeenCalledWith(
+      'securitySolution.dataViewManager.selectedDataView.analyzer'
+    );
+  });
 });

x-pack/solutions/security/plugins/security_solution/public/data_view_manager/redux/listeners/data_view_selected.ts

@@ -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';
@@ -37,6 +38,7 @@ import { PageScope } from '../../constants';
 export const createDataViewSelectedListener = (dependencies: {
   scope: PageScope;
   dataViews: DataViewsServicePublic;
+  notifications: CoreStart['notifications'];
   storage: Storage;
   logger: Logger;
 }) => {
@@ -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.`,
+          });
+
+          // This cleans local storage for the analyzer data view to prevent the error from happening again on page refresh.
+          if (action.payload.scope === PageScope.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(',') ?? '';
@@ -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 =

x-pack/solutions/security/plugins/security_solution/public/data_view_manager/redux/listeners/init_listener.test.ts

@@ -34,13 +34,14 @@ const mockDataViewsService = {
     isPersisted: () => false,
     toSpec: () => ({ id: 'adhoc_test-*', title: 'test-*' }),
   }),
-  getAllDataViewLazy: jest.fn().mockReturnValue([]),
+  getIdsWithTitle: jest.fn().mockReturnValue([]),
 } as unknown as DataViewsServicePublic;
 
 const http = {} as unknown as CoreStart['http'];
 const application = {} as unknown as CoreStart['application'];
 const uiSettings = {} as unknown as CoreStart['uiSettings'];
 const spaces = { getActiveSpace: async () => ({ id: 'default' }) } as unknown as SpacesPluginStart;
+const mockToastsDanger = jest.fn();
 
 const mockDispatch = jest.fn();
 const mockGetState = jest.fn(() => {
@@ -82,6 +83,11 @@ describe('createInitListener', () => {
         http,
         application,
         uiSettings,
+        notifications: {
+          toasts: {
+            addDanger: mockToastsDanger,
+          },
+        } as unknown as CoreStart['notifications'],
         spaces,
         storage: {
           get: jest.fn(),
@@ -94,15 +100,34 @@ describe('createInitListener', () => {
     );
   });
 
-  it('should load the data views and dispatch further actions', async () => {
+  it('should load the data views from getIdsWithTitle and dispatch further actions', async () => {
+    jest.mocked(mockDataViewsService.getIdsWithTitle).mockResolvedValue([
+      {
+        id: 'logs-*',
+        title: 'logs-*',
+        name: 'logs',
+        managed: false,
+      },
+    ]);
+
     await listener.effect(sharedDataViewManagerSlice.actions.init([]), mockListenerApi);
 
     expect(jest.mocked(createDefaultDataView)).toHaveBeenCalled();
 
-    expect(jest.mocked(mockDataViewsService.getAllDataViewLazy)).toHaveBeenCalled();
+    expect(jest.mocked(mockDataViewsService.getIdsWithTitle)).toHaveBeenCalled();
 
     expect(jest.mocked(mockListenerApi.dispatch)).toBeCalledWith(
-      sharedDataViewManagerSlice.actions.setDataViews([])
+      sharedDataViewManagerSlice.actions.setDataViews([
+        {
+          id: 'logs-*',
+          title: 'logs-*',
+          name: 'logs',
+          managed: false,
+          timeFieldName: undefined,
+          type: undefined,
+          typeMeta: undefined,
+        },
+      ])
     );
     expect(jest.mocked(mockListenerApi.dispatch)).toBeCalledWith(
       sharedDataViewManagerSlice.actions.setDataViewId({
@@ -141,12 +166,13 @@ describe('createInitListener', () => {
         scope: PageScope.analyzer,
       })
     );
+    expect(mockToastsDanger).not.toHaveBeenCalled();
   });
 
-  describe('when data views fetch returns an error', () => {
+  describe('when getIdsWithTitle fetch returns an error', () => {
     beforeEach(() => {
       jest
-        .mocked(mockDataViewsService.getAllDataViewLazy)
+        .mocked(mockDataViewsService.getIdsWithTitle)
         .mockRejectedValue(new Error('some loading error'));
     });
 
@@ -156,6 +182,10 @@ describe('createInitListener', () => {
       expect(jest.mocked(mockListenerApi.dispatch)).toBeCalledWith(
         sharedDataViewManagerSlice.actions.error()
       );
+      expect(mockToastsDanger).toHaveBeenCalledWith({
+        title: 'Error initializing data views',
+        text: 'Error: some loading error',
+      });
     });
   });
 });