[App Search] Convert Schema pages to new page template (elastic#102846)

* Convert Schema page to new page template + update empty state - remove panel wrapper, add create schema field modal * Convert ReindexJob view to new page template + remove breadcrumb prop * Convert Meta Engine Schema view to new page template * Update routers * [Polish] Misc Davey Schema UI tweaks - see https://github.com/elastic/kibana/pull/101958/files + change color away from secondary, since that's going away in EUI at some point * [UX] Fix SchemaAddFieldModal stuttering on first new schema field add - With the new template, transitioning from the empty state to the filled schema state causes the modal to stutter due to the component rerender - Changing the page to not instantly react/update `hasSchema` when local schema state changes but instead to wait for the server call to finish and for cachedSchema to update fixes the UX problem * [UI polish] Revert button color change per Davey's feedback
kibanamachine · Jun 22, 2021 · 7d9d0e1 · 7d9d0e1
1 parent be8aea6
commit 7d9d0e1
Show file tree

Hide file tree

Showing 13 changed files with 172 additions and 197 deletions.
diff --git a/...gins/enterprise_search/public/applications/app_search/components/engine/engine_router.tsx b/...gins/enterprise_search/public/applications/app_search/components/engine/engine_router.tsx
@@ -109,18 +109,18 @@ export const EngineRouter: React.FC = () => {
           <Documents />
         </Route>
       )}
+      {canViewEngineSchema && (
+        <Route path={ENGINE_SCHEMA_PATH}>
+          <SchemaRouter />
+        </Route>
+      )}
       {canManageEngineSearchUi && (
         <Route path={ENGINE_SEARCH_UI_PATH}>
           <SearchUI />
         </Route>
       )}
       {/* TODO: Remove layout once page template migration is over */}
       <Layout navigation={<AppSearchNav />}>
-        {canViewEngineSchema && (
-          <Route path={ENGINE_SCHEMA_PATH}>
-            <SchemaRouter />
-          </Route>
-        )}
         {canManageEngineCurations && (
           <Route path={ENGINE_CURATIONS_PATH}>
             <CurationsRouter />

diff --git a/...e_search/public/applications/app_search/components/schema/components/empty_state.test.tsx b/...e_search/public/applications/app_search/components/schema/components/empty_state.test.tsx
@@ -5,12 +5,16 @@
  * 2.0.
  */
 
+import { setMockValues } from '../../../../__mocks__/kea_logic';
+
 import React from 'react';
 
 import { shallow } from 'enzyme';
 
 import { EuiEmptyPrompt, EuiButton } from '@elastic/eui';
 
+import { SchemaAddFieldModal } from '../../../../shared/schema';
+
 import { EmptyState } from './';
 
 describe('EmptyState', () => {
@@ -24,4 +28,11 @@ describe('EmptyState', () => {
       expect.stringContaining('#indexing-documents-guide-schema')
     );
   });
+
+  it('renders a modal that lets a user add a new schema field', () => {
+    setMockValues({ isModalOpen: true });
+    const wrapper = shallow(<EmptyState />);
+
+    expect(wrapper.find(SchemaAddFieldModal)).toHaveLength(1);
+  });
 });
diff --git a/...rprise_search/public/applications/app_search/components/schema/components/empty_state.tsx b/...rprise_search/public/applications/app_search/components/schema/components/empty_state.tsx
@@ -7,14 +7,21 @@
 
 import React from 'react';
 
-import { EuiPanel, EuiEmptyPrompt, EuiButton } from '@elastic/eui';
+import { useValues, useActions } from 'kea';
+
+import { EuiEmptyPrompt, EuiButton } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 
+import { SchemaAddFieldModal } from '../../../../shared/schema';
 import { DOCS_PREFIX } from '../../../routes';
+import { SchemaLogic } from '../schema_logic';
 
 export const EmptyState: React.FC = () => {
+  const { isModalOpen } = useValues(SchemaLogic);
+  const { addSchemaField, closeModal } = useActions(SchemaLogic);
+
   return (
-    <EuiPanel color="subdued">
+    <>
       <EuiEmptyPrompt
         iconType="database"
         title={
@@ -45,6 +52,9 @@ export const EmptyState: React.FC = () => {
           </EuiButton>
         }
       />
-    </EuiPanel>
+      {isModalOpen && (
+        <SchemaAddFieldModal addNewField={addSchemaField} closeAddFieldModal={closeModal} />
+      )}
+    </>
   );
 };
diff --git a/..._search/public/applications/app_search/components/schema/reindex_job/reindex_job.test.tsx b/..._search/public/applications/app_search/components/schema/reindex_job/reindex_job.test.tsx
@@ -14,15 +14,11 @@ import React from 'react';
 
 import { shallow } from 'enzyme';
 
-import { Loading } from '../../../../shared/loading';
 import { SchemaErrorsAccordion } from '../../../../shared/schema';
 
 import { ReindexJob } from './';
 
 describe('ReindexJob', () => {
-  const props = {
-    schemaBreadcrumb: ['Engines', 'some-engine', 'Schema'],
-  };
   const values = {
     dataLoading: false,
     fieldCoercionErrors: {},
@@ -43,27 +39,20 @@ describe('ReindexJob', () => {
   });
 
   it('renders', () => {
-    const wrapper = shallow(<ReindexJob {...props} />);
+    const wrapper = shallow(<ReindexJob />);
 
     expect(wrapper.find(SchemaErrorsAccordion)).toHaveLength(1);
     expect(wrapper.find(SchemaErrorsAccordion).prop('generateViewPath')).toHaveLength(1);
   });
 
   it('calls loadReindexJob on page load', () => {
-    shallow(<ReindexJob {...props} />);
+    shallow(<ReindexJob />);
 
     expect(actions.loadReindexJob).toHaveBeenCalledWith('abc1234567890');
   });
 
-  it('renders a loading state', () => {
-    setMockValues({ ...values, dataLoading: true });
-    const wrapper = shallow(<ReindexJob {...props} />);
-
-    expect(wrapper.find(Loading)).toHaveLength(1);
-  });
-
   it('renders schema errors with links to document pages', () => {
-    const wrapper = shallow(<ReindexJob {...props} />);
+    const wrapper = shallow(<ReindexJob />);
     const generateViewPath = wrapper
       .find(SchemaErrorsAccordion)
       .prop('generateViewPath') as Function;

diff --git a/...prise_search/public/applications/app_search/components/schema/reindex_job/reindex_job.tsx b/...prise_search/public/applications/app_search/components/schema/reindex_job/reindex_job.tsx
@@ -10,25 +10,17 @@ import { useParams } from 'react-router-dom';
 
 import { useActions, useValues } from 'kea';
 
-import { EuiPageHeader, EuiPageContentBody } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 
-import { FlashMessages } from '../../../../shared/flash_messages';
-import { SetAppSearchChrome as SetPageChrome } from '../../../../shared/kibana_chrome';
-import { BreadcrumbTrail } from '../../../../shared/kibana_chrome/generate_breadcrumbs';
-import { Loading } from '../../../../shared/loading';
 import { SchemaErrorsAccordion } from '../../../../shared/schema';
-
 import { ENGINE_DOCUMENT_DETAIL_PATH } from '../../../routes';
-import { EngineLogic, generateEnginePath } from '../../engine';
+import { EngineLogic, generateEnginePath, getEngineBreadcrumbs } from '../../engine';
+import { AppSearchPageTemplate } from '../../layout';
+import { SCHEMA_TITLE } from '../constants';
 
 import { ReindexJobLogic } from './reindex_job_logic';
 
-interface Props {
-  schemaBreadcrumb: BreadcrumbTrail;
-}
-
-export const ReindexJob: React.FC<Props> = ({ schemaBreadcrumb }) => {
+export const ReindexJob: React.FC = () => {
   const { reindexJobId } = useParams() as { reindexJobId: string };
   const { loadReindexJob } = useActions(ReindexJobLogic);
   const { dataLoading, fieldCoercionErrors } = useValues(ReindexJobLogic);
@@ -40,34 +32,29 @@ export const ReindexJob: React.FC<Props> = ({ schemaBreadcrumb }) => {
     loadReindexJob(reindexJobId);
   }, [reindexJobId]);
 
-  if (dataLoading) return <Loading />;
-
   return (
-    <>
-      <SetPageChrome
-        trail={[
-          ...schemaBreadcrumb,
-          i18n.translate('xpack.enterpriseSearch.appSearch.engine.schema.reindexErrorsBreadcrumb', {
-            defaultMessage: 'Reindex errors',
-          }),
-        ]}
-      />
-      <EuiPageHeader
-        pageTitle={i18n.translate(
+    <AppSearchPageTemplate
+      pageChrome={getEngineBreadcrumbs([
+        SCHEMA_TITLE,
+        i18n.translate('xpack.enterpriseSearch.appSearch.engine.schema.reindexErrorsBreadcrumb', {
+          defaultMessage: 'Reindex errors',
+        }),
+      ])}
+      pageHeader={{
+        pageTitle: i18n.translate(
           'xpack.enterpriseSearch.appSearch.engine.schema.reindexJob.title',
           { defaultMessage: 'Schema change errors' }
-        )}
+        ),
+      }}
+      isLoading={dataLoading}
+    >
+      <SchemaErrorsAccordion
+        fieldCoercionErrors={fieldCoercionErrors}
+        schema={schema!}
+        generateViewPath={(documentId: string) =>
+          generateEnginePath(ENGINE_DOCUMENT_DETAIL_PATH, { documentId })
+        }
       />
-      <FlashMessages />
-      <EuiPageContentBody>
-        <SchemaErrorsAccordion
-          fieldCoercionErrors={fieldCoercionErrors}
-          schema={schema!}
-          generateViewPath={(documentId: string) =>
-            generateEnginePath(ENGINE_DOCUMENT_DETAIL_PATH, { documentId })
-          }
-        />
-      </EuiPageContentBody>
-    </>
+    </AppSearchPageTemplate>
   );
 };
diff --git a/...s/enterprise_search/public/applications/app_search/components/schema/schema_logic.test.ts b/...s/enterprise_search/public/applications/app_search/components/schema/schema_logic.test.ts
@@ -140,13 +140,13 @@ describe('SchemaLogic', () => {
 
   describe('selectors', () => {
     describe('hasSchema', () => {
-      it('returns true when the schema obj has items', () => {
-        mountAndSetSchema({ schema: { test: SchemaType.Text } });
+      it('returns true when the cached server schema obj has items', () => {
+        mount({ cachedSchema: { test: SchemaType.Text } });
         expect(SchemaLogic.values.hasSchema).toEqual(true);
       });
 
-      it('returns false when the schema obj is empty', () => {
-        mountAndSetSchema({ schema: {} });
+      it('returns false when the cached server schema obj is empty', () => {
+        mount({ schema: {} });
         expect(SchemaLogic.values.hasSchema).toEqual(false);
       });
     });

diff --git a/...lugins/enterprise_search/public/applications/app_search/components/schema/schema_logic.ts b/...lugins/enterprise_search/public/applications/app_search/components/schema/schema_logic.ts
@@ -108,7 +108,10 @@ export const SchemaLogic = kea<MakeLogicType<SchemaValues, SchemaActions>>({
     ],
   },
   selectors: {
-    hasSchema: [(selectors) => [selectors.schema], (schema) => Object.keys(schema).length > 0],
+    hasSchema: [
+      (selectors) => [selectors.cachedSchema],
+      (cachedSchema) => Object.keys(cachedSchema).length > 0,
+    ],
     hasSchemaChanged: [
       (selectors) => [selectors.schema, selectors.cachedSchema],
       (schema, cachedSchema) => !isEqual(schema, cachedSchema),

diff --git a/...gins/enterprise_search/public/applications/app_search/components/schema/schema_router.tsx b/...gins/enterprise_search/public/applications/app_search/components/schema/schema_router.tsx
@@ -10,27 +10,21 @@ import { Route, Switch } from 'react-router-dom';
 
 import { useValues } from 'kea';
 
-import { SetAppSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome';
 import { ENGINE_REINDEX_JOB_PATH } from '../../routes';
-import { EngineLogic, getEngineBreadcrumbs } from '../engine';
+import { EngineLogic } from '../engine';
 
-import { SCHEMA_TITLE } from './constants';
 import { ReindexJob } from './reindex_job';
 import { Schema, MetaEngineSchema } from './views';
 
 export const SchemaRouter: React.FC = () => {
   const { isMetaEngine } = useValues(EngineLogic);
-  const schemaBreadcrumb = getEngineBreadcrumbs([SCHEMA_TITLE]);
 
   return (
     <Switch>
       <Route path={ENGINE_REINDEX_JOB_PATH}>
-        <ReindexJob schemaBreadcrumb={schemaBreadcrumb} />
-      </Route>
-      <Route>
-        <SetPageChrome trail={schemaBreadcrumb} />
-        {isMetaEngine ? <MetaEngineSchema /> : <Schema />}
+        <ReindexJob />
       </Route>
+      <Route>{isMetaEngine ? <MetaEngineSchema /> : <Schema />}</Route>
     </Switch>
   );
 };
diff --git a/...search/public/applications/app_search/components/schema/views/meta_engine_schema.test.tsx b/...search/public/applications/app_search/components/schema/views/meta_engine_schema.test.tsx
@@ -7,15 +7,14 @@
 
 import { setMockValues, setMockActions } from '../../../../__mocks__/kea_logic';
 import '../../../../__mocks__/shallow_useeffect.mock';
+import '../../../__mocks__/engine_logic.mock';
 
 import React from 'react';
 
 import { shallow } from 'enzyme';
 
 import { EuiCallOut } from '@elastic/eui';
 
-import { Loading } from '../../../../shared/loading';
-
 import { MetaEnginesSchemaTable, MetaEnginesConflictsTable } from '../components';
 
 import { MetaEngineSchema } from './';
@@ -46,13 +45,6 @@ describe('MetaEngineSchema', () => {
     expect(actions.loadSchema).toHaveBeenCalled();
   });
 
-  it('renders a loading state', () => {
-    setMockValues({ ...values, dataLoading: true });
-    const wrapper = shallow(<MetaEngineSchema />);
-
-    expect(wrapper.find(Loading)).toHaveLength(1);
-  });
-
   it('renders an inactive fields callout & table when source engines have schema conflicts', () => {
     setMockValues({ ...values, hasConflicts: true, conflictingFieldsCount: 5 });
     const wrapper = shallow(<MetaEngineSchema />);