[App Search] EnginesLogic + minor UX fix (#87561)

* [Misc cleanup] DRY out type def

EnginesTableData was made before we started importing more types over from ent-search; we should reuse the more complete EngineDetails instead of re-declaring our own

* Add EnginesLogic file + tests
- based on current state inside EnginesOverview

- Not a 1:1 translation from ent-search's EnginesLogic b/c the table component/view is different
- also missing engine creation which will be a separate PR

* Update EnginesOverview to use EnginesLogic

- should be significantly simpler
- tests no longer need mountAsync

* [Extra] Make up for lost icon.tsx coverage

- because we no longer use mount() in the engines overview tests, I'm adding an extra set of quick shallow render tests to cover the icon .tsx lines

* [Misc] Rename fetchX to loadY (copying Kea)

x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/assets/icons.test.tsx

@@ -0,0 +1,23 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import React from 'react';
+import { shallow } from 'enzyme';
+
+import { EngineIcon } from './engine_icon';
+import { MetaEngineIcon } from './meta_engine_icon';
+
+describe('Engines icons', () => {
+  it('renders an engine icon', () => {
+    const wrapper = shallow(<EngineIcon />);
+    expect(wrapper.hasClass('engineIcon')).toBe(true);
+  });
+
+  it('renders a meta engine icon', () => {
+    const wrapper = shallow(<MetaEngineIcon />);
+    expect(wrapper.hasClass('engineIcon')).toBe(true);
+  });
+});

x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_logic.test.ts

@@ -0,0 +1,169 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { LogicMounter } from '../../../__mocks__/kea.mock';
+
+jest.mock('../../../shared/http', () => ({
+  HttpLogic: { values: { http: { get: jest.fn() } } },
+}));
+import { HttpLogic } from '../../../shared/http';
+
+import { EngineDetails } from '../engine/types';
+import { EnginesLogic } from './';
+
+describe('EnginesLogic', () => {
+  const DEFAULT_VALUES = {
+    dataLoading: true,
+    engines: [],
+    enginesTotal: 0,
+    enginesPage: 1,
+    metaEngines: [],
+    metaEnginesTotal: 0,
+    metaEnginesPage: 1,
+  };
+
+  const MOCK_ENGINE = {
+    name: 'hello-world',
+    created_at: 'Fri, 1 Jan 1970 12:00:00 +0000',
+    document_count: 50,
+    field_count: 10,
+  } as EngineDetails;
+  const MOCK_ENGINES_API_RESPONSE = {
+    results: [MOCK_ENGINE],
+    meta: {
+      page: {
+        current: 1,
+        total_pages: 10,
+        total_results: 100,
+        size: 10,
+      },
+    },
+  };
+
+  const { mount } = new LogicMounter(EnginesLogic);
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('has expected default values', () => {
+    mount();
+    expect(EnginesLogic.values).toEqual(DEFAULT_VALUES);
+  });
+
+  describe('actions', () => {
+    describe('onEnginesLoad', () => {
+      describe('dataLoading', () => {
+        it('should be set to false', () => {
+          mount();
+          EnginesLogic.actions.onEnginesLoad({ engines: [], total: 0 });
+
+          expect(EnginesLogic.values).toEqual({
+            ...DEFAULT_VALUES,
+            dataLoading: false,
+          });
+        });
+      });
+
+      describe('engines & enginesTotal', () => {
+        it('should be set to the provided value', () => {
+          mount();
+          EnginesLogic.actions.onEnginesLoad({ engines: [MOCK_ENGINE], total: 100 });
+
+          expect(EnginesLogic.values).toEqual({
+            ...DEFAULT_VALUES,
+            dataLoading: false,
+            engines: [MOCK_ENGINE],
+            enginesTotal: 100,
+          });
+        });
+      });
+    });
+
+    describe('onMetaEnginesLoad', () => {
+      describe('engines & enginesTotal', () => {
+        it('should be set to the provided value', () => {
+          mount();
+          EnginesLogic.actions.onMetaEnginesLoad({ engines: [MOCK_ENGINE], total: 1 });
+
+          expect(EnginesLogic.values).toEqual({
+            ...DEFAULT_VALUES,
+            metaEngines: [MOCK_ENGINE],
+            metaEnginesTotal: 1,
+          });
+        });
+      });
+    });
+
+    describe('onEnginesPagination', () => {
+      describe('enginesPage', () => {
+        it('should be set to the provided value', () => {
+          mount();
+          EnginesLogic.actions.onEnginesPagination(2);
+
+          expect(EnginesLogic.values).toEqual({
+            ...DEFAULT_VALUES,
+            enginesPage: 2,
+          });
+        });
+      });
+    });
+
+    describe('onMetaEnginesPagination', () => {
+      describe('metaEnginesPage', () => {
+        it('should be set to the provided value', () => {
+          mount();
+          EnginesLogic.actions.onMetaEnginesPagination(99);
+
+          expect(EnginesLogic.values).toEqual({
+            ...DEFAULT_VALUES,
+            metaEnginesPage: 99,
+          });
+        });
+      });
+    });
+
+    describe('loadEngines', () => {
+      it('should call the engines API endpoint and set state based on the results', async () => {
+        const promise = Promise.resolve(MOCK_ENGINES_API_RESPONSE);
+        (HttpLogic.values.http.get as jest.Mock).mockReturnValueOnce(promise);
+        mount({ enginesPage: 10 });
+        jest.spyOn(EnginesLogic.actions, 'onEnginesLoad');
+
+        EnginesLogic.actions.loadEngines();
+        await promise;
+
+        expect(HttpLogic.values.http.get).toHaveBeenCalledWith('/api/app_search/engines', {
+          query: { type: 'indexed', pageIndex: 10 },
+        });
+        expect(EnginesLogic.actions.onEnginesLoad).toHaveBeenCalledWith({
+          engines: [MOCK_ENGINE],
+          total: 100,
+        });
+      });
+    });
+
+    describe('loadMetaEngines', () => {
+      it('should call the engines API endpoint and set state based on the results', async () => {
+        const promise = Promise.resolve(MOCK_ENGINES_API_RESPONSE);
+        (HttpLogic.values.http.get as jest.Mock).mockReturnValueOnce(promise);
+        mount({ metaEnginesPage: 99 });
+        jest.spyOn(EnginesLogic.actions, 'onMetaEnginesLoad');
+
+        EnginesLogic.actions.loadMetaEngines();
+        await promise;
+
+        expect(HttpLogic.values.http.get).toHaveBeenCalledWith('/api/app_search/engines', {
+          query: { type: 'meta', pageIndex: 99 },
+        });
+        expect(EnginesLogic.actions.onMetaEnginesLoad).toHaveBeenCalledWith({
+          engines: [MOCK_ENGINE],
+          total: 100,
+        });
+      });
+    });
+  });
+});

x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_logic.ts

@@ -0,0 +1,116 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { kea, MakeLogicType } from 'kea';
+
+import { HttpLogic } from '../../../shared/http';
+
+import { EngineDetails } from '../engine/types';
+
+interface EnginesValues {
+  dataLoading: boolean;
+  engines: EngineDetails[];
+  enginesTotal: number;
+  enginesPage: number;
+  metaEngines: EngineDetails[];
+  metaEnginesTotal: number;
+  metaEnginesPage: number;
+}
+
+interface OnEnginesLoad {
+  engines: EngineDetails[];
+  total: number;
+}
+interface EnginesActions {
+  onEnginesLoad({ engines, total }: OnEnginesLoad): OnEnginesLoad;
+  onMetaEnginesLoad({ engines, total }: OnEnginesLoad): OnEnginesLoad;
+  onEnginesPagination(page: number): { page: number };
+  onMetaEnginesPagination(page: number): { page: number };
+  loadEngines(): void;
+  loadMetaEngines(): void;
+}
+
+export const EnginesLogic = kea<MakeLogicType<EnginesValues, EnginesActions>>({
+  path: ['enterprise_search', 'app_search', 'engines_logic'],
+  actions: {
+    onEnginesLoad: ({ engines, total }) => ({ engines, total }),
+    onMetaEnginesLoad: ({ engines, total }) => ({ engines, total }),
+    onEnginesPagination: (page) => ({ page }),
+    onMetaEnginesPagination: (page) => ({ page }),
+    loadEngines: true,
+    loadMetaEngines: true,
+  },
+  reducers: {
+    dataLoading: [
+      true,
+      {
+        onEnginesLoad: () => false,
+      },
+    ],
+    engines: [
+      [],
+      {
+        onEnginesLoad: (_, { engines }) => engines,
+      },
+    ],
+    enginesTotal: [
+      0,
+      {
+        onEnginesLoad: (_, { total }) => total,
+      },
+    ],
+    enginesPage: [
+      1,
+      {
+        onEnginesPagination: (_, { page }) => page,
+      },
+    ],
+    metaEngines: [
+      [],
+      {
+        onMetaEnginesLoad: (_, { engines }) => engines,
+      },
+    ],
+    metaEnginesTotal: [
+      0,
+      {
+        onMetaEnginesLoad: (_, { total }) => total,
+      },
+    ],
+    metaEnginesPage: [
+      1,
+      {
+        onMetaEnginesPagination: (_, { page }) => page,
+      },
+    ],
+  },
+  listeners: ({ actions, values }) => ({
+    loadEngines: async () => {
+      const { http } = HttpLogic.values;
+      const { enginesPage } = values;
+
+      const response = await http.get('/api/app_search/engines', {
+        query: { type: 'indexed', pageIndex: enginesPage },
+      });
+      actions.onEnginesLoad({
+        engines: response.results,
+        total: response.meta.page.total_results,
+      });
+    },
+    loadMetaEngines: async () => {
+      const { http } = HttpLogic.values;
+      const { metaEnginesPage } = values;
+
+      const response = await http.get('/api/app_search/engines', {
+        query: { type: 'meta', pageIndex: metaEnginesPage },
+      });
+      actions.onMetaEnginesLoad({
+        engines: response.results,
+        total: response.meta.page.total_results,
+      });
+    },
+  }),
+});

x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_overview.test.tsx

@@ -4,14 +4,12 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import '../../../__mocks__/kea.mock';
-import '../../../__mocks__/react_router_history.mock';
+import '../../../__mocks__/shallow_useeffect.mock';
+import { rerender } from '../../../__mocks__';
+import { setMockValues, setMockActions } from '../../../__mocks__/kea.mock';
 
 import React from 'react';
-import { act } from 'react-dom/test-utils';
-import { shallow, ReactWrapper } from 'enzyme';
-
-import { mountAsync, mockHttpValues, setMockValues } from '../../../__mocks__';
+import { shallow, ShallowWrapper } from 'enzyme';
 
 import { LoadingState, EmptyState } from './components';
 import { EnginesTable } from './engines_table';
@@ -19,91 +17,85 @@ import { EnginesTable } from './engines_table';
 import { EnginesOverview } from './';
 
 describe('EnginesOverview', () => {
+  const values = {
+    hasPlatinumLicense: false,
+    dataLoading: false,
+    engines: [],
+    enginesTotal: 0,
+    enginesPage: 1,
+    metaEngines: [],
+    metaEnginesTotal: 0,
+    metaEnginesPage: 1,
+  };
+  const actions = {
+    loadEngines: jest.fn(),
+    loadMetaEngines: jest.fn(),
+    onEnginesPagination: jest.fn(),
+    onMetaEnginesPagination: jest.fn(),
+  };
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    setMockValues(values);
+    setMockActions(actions);
+  });
+
   describe('non-happy-path states', () => {
     it('isLoading', () => {
+      setMockValues({ ...values, dataLoading: true });
       const wrapper = shallow(<EnginesOverview />);
 
       expect(wrapper.find(LoadingState)).toHaveLength(1);
     });
 
-    it('isEmpty', async () => {
-      setMockValues({
-        http: {
-          ...mockHttpValues.http,
-          get: () => ({
-            results: [],
-            meta: { page: { total_results: 0 } },
-          }),
-        },
-      });
-      const wrapper = await mountAsync(<EnginesOverview />, { i18n: true });
+    it('isEmpty', () => {
+      setMockValues({ ...values, engines: [] });
+      const wrapper = shallow(<EnginesOverview />);
 
       expect(wrapper.find(EmptyState)).toHaveLength(1);
     });
   });
 
   describe('happy-path states', () => {
-    const mockedApiResponse = {
-      results: [
-        {
-          name: 'hello-world',
-          created_at: 'Fri, 1 Jan 1970 12:00:00 +0000',
-          document_count: 50,
-          field_count: 10,
-        },
-      ],
-      meta: {
-        page: {
-          current: 1,
-          total_pages: 10,
-          total_results: 100,
-          size: 10,
-        },
-      },
+    const valuesWithEngines = {
+      ...values,
+      dataLoading: false,
+      engines: ['dummy-engine'],
+      enginesTotal: 100,
+      enginesPage: 1,
     };
-    const mockApi = jest.fn(() => mockedApiResponse);
 
     beforeEach(() => {
-      jest.clearAllMocks();
-      setMockValues({ http: { ...mockHttpValues.http, get: mockApi } });
+      setMockValues(valuesWithEngines);
     });
 
     it('renders and calls the engines API', async () => {
-      const wrapper = await mountAsync(<EnginesOverview />, { i18n: true });
+      const wrapper = shallow(<EnginesOverview />);
 
       expect(wrapper.find(EnginesTable)).toHaveLength(1);
-      expect(mockApi).toHaveBeenNthCalledWith(1, '/api/app_search/engines', {
-        query: {
-          type: 'indexed',
-          pageIndex: 1,
-        },
-      });
+      expect(actions.loadEngines).toHaveBeenCalled();
     });
 
     describe('when on a platinum license', () => {
       it('renders a 2nd meta engines table & makes a 2nd meta engines API call', async () => {
         setMockValues({
+          ...valuesWithEngines,
           hasPlatinumLicense: true,
-          http: { ...mockHttpValues.http, get: mockApi },
+          metaEngines: ['dummy-meta-engine'],
         });
-        const wrapper = await mountAsync(<EnginesOverview />, { i18n: true });
+        const wrapper = shallow(<EnginesOverview />);
 
         expect(wrapper.find(EnginesTable)).toHaveLength(2);
-        expect(mockApi).toHaveBeenNthCalledWith(2, '/api/app_search/engines', {
-          query: {
-            type: 'meta',
-            pageIndex: 1,
-          },
-        });
+        expect(actions.loadMetaEngines).toHaveBeenCalled();
       });
     });
 
     describe('pagination', () => {
-      const getTablePagination = (wrapper: ReactWrapper) =>
+      const getTablePagination = (wrapper: ShallowWrapper) =>
         wrapper.find(EnginesTable).prop('pagination');
 
       it('passes down page data from the API', async () => {
-        const wrapper = await mountAsync(<EnginesOverview />, { i18n: true });
+        const wrapper = shallow(<EnginesOverview />);
         const pagination = getTablePagination(wrapper);
 
         expect(pagination.totalEngines).toEqual(100);
@@ -111,17 +103,13 @@ describe('EnginesOverview', () => {
       });
 
       it('re-polls the API on page change', async () => {
-        const wrapper = await mountAsync(<EnginesOverview />, { i18n: true });
-        await act(async () => getTablePagination(wrapper).onPaginate(5));
-        wrapper.update();
+        const wrapper = shallow(<EnginesOverview />);
 
-        expect(mockApi).toHaveBeenLastCalledWith('/api/app_search/engines', {
-          query: {
-            type: 'indexed',
-            pageIndex: 5,
-          },
-        });
-        expect(getTablePagination(wrapper).pageIndex).toEqual(4);
+        setMockValues({ ...valuesWithEngines, enginesPage: 51 });
+        rerender(wrapper);
+
+        expect(actions.loadEngines).toHaveBeenCalledTimes(2);
+        expect(getTablePagination(wrapper).pageIndex).toEqual(50);
       });
     });
   });

x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_overview.tsx

@@ -4,8 +4,8 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React, { useEffect, useState } from 'react';
-import { useValues } from 'kea';
+import React, { useEffect } from 'react';
+import { useValues, useActions } from 'kea';
 import {
   EuiPageContent,
   EuiPageContentHeader,
@@ -17,7 +17,6 @@ import {
 import { SetAppSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome';
 import { SendAppSearchTelemetry as SendTelemetry } from '../../../shared/telemetry';
 import { FlashMessages } from '../../../shared/flash_messages';
-import { HttpLogic } from '../../../shared/http';
 import { LicensingLogic } from '../../../shared/licensing';
 
 import { EngineIcon } from './assets/engine_icon';
@@ -25,61 +24,34 @@ import { MetaEngineIcon } from './assets/meta_engine_icon';
 import { ENGINES_TITLE, META_ENGINES_TITLE } from './constants';
 import { EnginesOverviewHeader, LoadingState, EmptyState } from './components';
 import { EnginesTable } from './engines_table';
+import { EnginesLogic } from './engines_logic';
 
 import './engines_overview.scss';
 
-interface GetEnginesParams {
-  type: string;
-  pageIndex: number;
-}
-interface SetEnginesCallbacks {
-  setResults: React.Dispatch<React.SetStateAction<never[]>>;
-  setResultsTotal: React.Dispatch<React.SetStateAction<number>>;
-}
-
 export const EnginesOverview: React.FC = () => {
-  const { http } = useValues(HttpLogic);
   const { hasPlatinumLicense } = useValues(LicensingLogic);
-
-  const [isLoading, setIsLoading] = useState(true);
-  const [engines, setEngines] = useState([]);
-  const [enginesPage, setEnginesPage] = useState(1);
-  const [enginesTotal, setEnginesTotal] = useState(0);
-  const [metaEngines, setMetaEngines] = useState([]);
-  const [metaEnginesPage, setMetaEnginesPage] = useState(1);
-  const [metaEnginesTotal, setMetaEnginesTotal] = useState(0);
-
-  const getEnginesData = async ({ type, pageIndex }: GetEnginesParams) => {
-    return await http.get('/api/app_search/engines', {
-      query: { type, pageIndex },
-    });
-  };
-  const setEnginesData = async (params: GetEnginesParams, callbacks: SetEnginesCallbacks) => {
-    const response = await getEnginesData(params);
-
-    callbacks.setResults(response.results);
-    callbacks.setResultsTotal(response.meta.page.total_results);
-
-    setIsLoading(false);
-  };
+  const {
+    dataLoading,
+    engines,
+    enginesTotal,
+    enginesPage,
+    metaEngines,
+    metaEnginesTotal,
+    metaEnginesPage,
+  } = useValues(EnginesLogic);
+  const { loadEngines, loadMetaEngines, onEnginesPagination, onMetaEnginesPagination } = useActions(
+    EnginesLogic
+  );
 
   useEffect(() => {
-    const params = { type: 'indexed', pageIndex: enginesPage };
-    const callbacks = { setResults: setEngines, setResultsTotal: setEnginesTotal };
-
-    setEnginesData(params, callbacks);
+    loadEngines();
   }, [enginesPage]);
 
   useEffect(() => {
-    if (hasPlatinumLicense) {
-      const params = { type: 'meta', pageIndex: metaEnginesPage };
-      const callbacks = { setResults: setMetaEngines, setResultsTotal: setMetaEnginesTotal };
-
-      setEnginesData(params, callbacks);
-    }
+    if (hasPlatinumLicense) loadMetaEngines();
   }, [hasPlatinumLicense, metaEnginesPage]);
 
-  if (isLoading) return <LoadingState />;
+  if (dataLoading) return <LoadingState />;
   if (!engines.length) return <EmptyState />;
 
   return (
@@ -103,7 +75,7 @@ export const EnginesOverview: React.FC = () => {
             pagination={{
               totalEngines: enginesTotal,
               pageIndex: enginesPage - 1,
-              onPaginate: setEnginesPage,
+              onPaginate: onEnginesPagination,
             }}
           />
         </EuiPageContentBody>
@@ -124,7 +96,7 @@ export const EnginesOverview: React.FC = () => {
                 pagination={{
                   totalEngines: metaEnginesTotal,
                   pageIndex: metaEnginesPage - 1,
-                  onPaginate: setMetaEnginesPage,
+                  onPaginate: onMetaEnginesPagination,
                 }}
               />
             </EuiPageContentBody>

x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_table.test.tsx

@@ -12,6 +12,7 @@ import React from 'react';
 import { EuiBasicTable, EuiPagination, EuiButtonEmpty } from '@elastic/eui';
 import { EuiLinkTo } from '../../../shared/react_router_helpers';
 
+import { EngineDetails } from '../engine/types';
 import { EnginesTable } from './engines_table';
 
 describe('EnginesTable', () => {
@@ -25,7 +26,7 @@ describe('EnginesTable', () => {
       isMeta: false,
       document_count: 99999,
       field_count: 10,
-    },
+    } as EngineDetails,
   ];
   const pagination = {
     totalEngines: 50,

x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_table.tsx

@@ -16,22 +16,15 @@ import { getEngineRoute } from '../../routes';
 
 import { ENGINES_PAGE_SIZE } from '../../../../../common/constants';
 import { UNIVERSAL_LANGUAGE } from '../../constants';
+import { EngineDetails } from '../engine/types';
 
-interface EnginesTableData {
-  name: string;
-  created_at: string;
-  document_count: number;
-  field_count: number;
-  language: string | null;
-  isMeta: boolean;
-}
 interface EnginesTablePagination {
   totalEngines: number;
   pageIndex: number;
   onPaginate(pageIndex: number): void;
 }
 interface EnginesTableProps {
-  data: EnginesTableData[];
+  data: EngineDetails[];
   pagination: EnginesTablePagination;
 }
 interface OnChange {
@@ -55,7 +48,7 @@ export const EnginesTable: React.FC<EnginesTableProps> = ({
       }),
   });
 
-  const columns: Array<EuiBasicTableColumn<EnginesTableData>> = [
+  const columns: Array<EuiBasicTableColumn<EngineDetails>> = [
     {
       field: 'name',
       name: i18n.translate('xpack.enterpriseSearch.appSearch.enginesOverview.table.column.name', {
@@ -100,7 +93,7 @@ export const EnginesTable: React.FC<EnginesTableProps> = ({
         }
       ),
       dataType: 'string',
-      render: (language: string, engine: EnginesTableData) =>
+      render: (language: string, engine: EngineDetails) =>
         engine.isMeta ? '' : language || UNIVERSAL_LANGUAGE,
     },
     {

x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/index.ts

@@ -4,5 +4,6 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+export { EnginesLogic } from './engines_logic';
 export { EnginesOverview } from './engines_overview';
 export { ENGINES_TITLE, META_ENGINES_TITLE } from './constants';