From 5f97b877b0b5f8f3ef8fa3802bf4d192ffcc8c10 Mon Sep 17 00:00:00 2001 From: Constance Date: Tue, 22 Jun 2021 17:35:00 -0700 Subject: [PATCH] [App Search] Convert Curations pages to new page template (#102835) * Update CurationRouter - Remove breadcrumbs set in router (will get set by page template) - Set up a curation breadcrumb helper for DRYness - Remove NotFound route - curation ID 404 handling will be used instead * Convert Curations page to new page template + move Empty State from table to top level * Convert Curation creation page to new page template * Convert single Curation page to new page template + remove breadcrumb prop * Update router * [Polish] Copy changes from Davey - see https://github.com/elastic/kibana/pull/101958/files - Per https://elastic.github.io/eui/#/guidelines/writing we shouldn't be using "new", so I removed that also * [UI polish] Add plus icon to create button - To match other create buttons across app --- .../components/curations/constants.ts | 2 +- .../curations/curation/curation.test.tsx | 38 ++++++------------ .../curations/curation/curation.tsx | 38 +++++++----------- .../curation/documents/hidden_documents.tsx | 2 +- .../curations/curations_router.test.tsx | 2 +- .../components/curations/curations_router.tsx | 14 +------ .../components/curations/utils.test.ts | 16 +++++++- .../app_search/components/curations/utils.ts | 8 ++++ .../views/curation_creation.test.tsx | 1 + .../curations/views/curation_creation.tsx | 18 +++++---- .../curations/views/curations.test.tsx | 35 ++++++++-------- .../components/curations/views/curations.tsx | 40 ++++++++++--------- .../components/engine/engine_router.tsx | 10 ++--- 13 files changed, 110 insertions(+), 114 deletions(-) diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/constants.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/constants.ts index 37c1e9a7a1a2e6..c490910184a693 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/constants.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/constants.ts @@ -18,7 +18,7 @@ export const CURATIONS_OVERVIEW_TITLE = i18n.translate( ); export const CREATE_NEW_CURATION_TITLE = i18n.translate( 'xpack.enterpriseSearch.appSearch.engine.curations.create.title', - { defaultMessage: 'Create new curation' } + { defaultMessage: 'Create a curation' } ); export const MANAGE_CURATION_TITLE = i18n.translate( 'xpack.enterpriseSearch.appSearch.engine.curations.manage.title', diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/curation.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/curation.test.tsx index 937acfd84ce83d..2efe1f2ffe86fc 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/curation.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/curation.test.tsx @@ -8,16 +8,13 @@ import '../../../../__mocks__/shallow_useeffect.mock'; import { setMockActions, setMockValues } from '../../../../__mocks__/kea_logic'; import { mockUseParams } from '../../../../__mocks__/react_router'; +import '../../../__mocks__/engine_logic.mock'; import React from 'react'; import { shallow, ShallowWrapper } from 'enzyme'; -import { EuiPageHeader } from '@elastic/eui'; - -import { SetAppSearchChrome as SetPageChrome } from '../../../../shared/kibana_chrome'; -import { Loading } from '../../../../shared/loading'; -import { rerender } from '../../../../test_helpers'; +import { rerender, getPageTitle, getPageHeaderActions } from '../../../../test_helpers'; jest.mock('./curation_logic', () => ({ CurationLogic: jest.fn() })); import { CurationLogic } from './curation_logic'; @@ -27,9 +24,6 @@ import { AddResultFlyout } from './results'; import { Curation } from './'; describe('Curation', () => { - const props = { - curationsBreadcrumb: ['Engines', 'some-engine', 'Curations'], - }; const values = { dataLoading: false, queries: ['query A', 'query B'], @@ -47,39 +41,34 @@ describe('Curation', () => { }); it('renders', () => { - const wrapper = shallow(); + const wrapper = shallow(); - expect(wrapper.find(EuiPageHeader).prop('pageTitle')).toEqual('Manage curation'); - expect(wrapper.find(SetPageChrome).prop('trail')).toEqual([ - ...props.curationsBreadcrumb, + expect(getPageTitle(wrapper)).toEqual('Manage curation'); + expect(wrapper.prop('pageChrome')).toEqual([ + 'Engines', + 'some-engine', + 'Curations', 'query A, query B', ]); }); - it('renders a loading component on page load', () => { - setMockValues({ ...values, dataLoading: true }); - const wrapper = shallow(); - - expect(wrapper.find(Loading)).toHaveLength(1); - }); - it('renders the add result flyout when open', () => { setMockValues({ ...values, isFlyoutOpen: true }); - const wrapper = shallow(); + const wrapper = shallow(); expect(wrapper.find(AddResultFlyout)).toHaveLength(1); }); it('initializes CurationLogic with a curationId prop from URL param', () => { mockUseParams.mockReturnValueOnce({ curationId: 'hello-world' }); - shallow(); + shallow(); expect(CurationLogic).toHaveBeenCalledWith({ curationId: 'hello-world' }); }); it('calls loadCuration on page load & whenever the curationId URL param changes', () => { mockUseParams.mockReturnValueOnce({ curationId: 'cur-123456789' }); - const wrapper = shallow(); + const wrapper = shallow(); expect(actions.loadCuration).toHaveBeenCalledTimes(1); mockUseParams.mockReturnValueOnce({ curationId: 'cur-987654321' }); @@ -92,9 +81,8 @@ describe('Curation', () => { let confirmSpy: jest.SpyInstance; beforeAll(() => { - const wrapper = shallow(); - const headerActions = wrapper.find(EuiPageHeader).prop('rightSideItems'); - restoreDefaultsButton = shallow(headerActions![0] as React.ReactElement); + const wrapper = shallow(); + restoreDefaultsButton = getPageHeaderActions(wrapper).childAt(0); confirmSpy = jest.spyOn(window, 'confirm'); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/curation.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/curation.tsx index ffa9fd8422a1bc..2a01c0db049ab1 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/curation.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/curation.tsx @@ -10,26 +10,19 @@ import { useParams } from 'react-router-dom'; import { useValues, useActions } from 'kea'; -import { EuiPageHeader, EuiSpacer, EuiFlexGroup, EuiFlexItem, EuiButton } from '@elastic/eui'; - -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 { EuiSpacer, EuiFlexGroup, EuiFlexItem, EuiButton } from '@elastic/eui'; import { RESTORE_DEFAULTS_BUTTON_LABEL } from '../../../constants'; +import { AppSearchPageTemplate } from '../../layout'; import { MANAGE_CURATION_TITLE, RESTORE_CONFIRMATION } from '../constants'; +import { getCurationsBreadcrumbs } from '../utils'; import { CurationLogic } from './curation_logic'; import { PromotedDocuments, OrganicDocuments, HiddenDocuments } from './documents'; import { ActiveQuerySelect, ManageQueriesModal } from './queries'; import { AddResultLogic, AddResultFlyout } from './results'; -interface Props { - curationsBreadcrumb: BreadcrumbTrail; -} - -export const Curation: React.FC = ({ curationsBreadcrumb }) => { +export const Curation: React.FC = () => { const { curationId } = useParams() as { curationId: string }; const { loadCuration, resetCuration } = useActions(CurationLogic({ curationId })); const { dataLoading, queries } = useValues(CurationLogic({ curationId })); @@ -39,14 +32,12 @@ export const Curation: React.FC = ({ curationsBreadcrumb }) => { loadCuration(); }, [curationId]); - if (dataLoading) return ; - return ( - <> - - { @@ -55,10 +46,10 @@ export const Curation: React.FC = ({ curationsBreadcrumb }) => { > {RESTORE_DEFAULTS_BUTTON_LABEL} , - ]} - responsive={false} - /> - + ], + }} + isLoading={dataLoading} + > @@ -69,7 +60,6 @@ export const Curation: React.FC = ({ curationsBreadcrumb }) => { - @@ -78,6 +68,6 @@ export const Curation: React.FC = ({ curationsBreadcrumb }) => { {isFlyoutOpen && } - + ); }; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/documents/hidden_documents.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/documents/hidden_documents.tsx index f2bc416b00341d..8cb06f32d9e4ef 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/documents/hidden_documents.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/documents/hidden_documents.tsx @@ -80,7 +80,7 @@ export const HiddenDocuments: React.FC = () => {

{i18n.translate( 'xpack.enterpriseSearch.appSearch.engine.curations.hiddenDocuments.emptyTitle', - { defaultMessage: 'No documents are being hidden for this query' } + { defaultMessage: "You haven't hidden any documents yet" } )}

} diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curations_router.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curations_router.test.tsx index 9598212d3e0c98..a241edb8020a4c 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curations_router.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curations_router.test.tsx @@ -19,6 +19,6 @@ describe('CurationsRouter', () => { const wrapper = shallow(); expect(wrapper.find(Switch)).toHaveLength(1); - expect(wrapper.find(Route)).toHaveLength(4); + expect(wrapper.find(Route)).toHaveLength(3); }); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curations_router.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curations_router.tsx index 28ce311b438875..40f2d07ab61ab6 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curations_router.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curations_router.tsx @@ -8,38 +8,26 @@ import React from 'react'; import { Route, Switch } from 'react-router-dom'; -import { APP_SEARCH_PLUGIN } from '../../../../../common/constants'; -import { SetAppSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome'; -import { NotFound } from '../../../shared/not_found'; import { ENGINE_CURATIONS_PATH, ENGINE_CURATIONS_NEW_PATH, ENGINE_CURATION_PATH, } from '../../routes'; -import { getEngineBreadcrumbs } from '../engine'; -import { CURATIONS_TITLE, CREATE_NEW_CURATION_TITLE } from './constants'; import { Curation } from './curation'; import { Curations, CurationCreation } from './views'; export const CurationsRouter: React.FC = () => { - const CURATIONS_BREADCRUMB = getEngineBreadcrumbs([CURATIONS_TITLE]); - return ( - - - - - - + ); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/utils.test.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/utils.test.ts index 51618ed4e37419..02641b09255e53 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/utils.test.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/utils.test.ts @@ -5,7 +5,21 @@ * 2.0. */ -import { convertToDate, addDocument, removeDocument } from './utils'; +import '../../__mocks__/engine_logic.mock'; + +import { getCurationsBreadcrumbs, convertToDate, addDocument, removeDocument } from './utils'; + +describe('getCurationsBreadcrumbs', () => { + it('generates curation-prefixed breadcrumbs', () => { + expect(getCurationsBreadcrumbs()).toEqual(['Engines', 'some-engine', 'Curations']); + expect(getCurationsBreadcrumbs(['Some page'])).toEqual([ + 'Engines', + 'some-engine', + 'Curations', + 'Some page', + ]); + }); +}); describe('convertToDate', () => { it('converts the English-only server timestamps to a parseable Date', () => { diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/utils.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/utils.ts index 8af2636128304b..978b63885fbdd3 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/utils.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/utils.ts @@ -5,6 +5,14 @@ * 2.0. */ +import { BreadcrumbTrail } from '../../../shared/kibana_chrome/generate_breadcrumbs'; +import { getEngineBreadcrumbs } from '../engine'; + +import { CURATIONS_TITLE } from './constants'; + +export const getCurationsBreadcrumbs = (breadcrumbs: BreadcrumbTrail = []) => + getEngineBreadcrumbs([CURATIONS_TITLE, ...breadcrumbs]); + // The server API feels us an English datestring, but we want to convert // it to an actual Date() instance so that we can localize date formats. export const convertToDate = (serverDateString: string): Date => { diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.test.tsx index ad306dfc730802..33aab9943cc832 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.test.tsx @@ -6,6 +6,7 @@ */ import { setMockActions } from '../../../../__mocks__/kea_logic'; +import '../../../__mocks__/engine_logic.mock'; import React from 'react'; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.tsx index 32d46775a2125e..9aa1759cec5c07 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.tsx @@ -9,10 +9,10 @@ import React from 'react'; import { useActions } from 'kea'; -import { EuiPageHeader, EuiPageContent, EuiTitle, EuiText, EuiSpacer } from '@elastic/eui'; +import { EuiPanel, EuiTitle, EuiText, EuiSpacer } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import { FlashMessages } from '../../../../shared/flash_messages'; +import { AppSearchPageTemplate } from '../../layout'; import { MultiInputRows } from '../../multi_input_rows'; import { @@ -21,15 +21,17 @@ import { QUERY_INPUTS_PLACEHOLDER, } from '../constants'; import { CurationsLogic } from '../index'; +import { getCurationsBreadcrumbs } from '../utils'; export const CurationCreation: React.FC = () => { const { createCuration } = useActions(CurationsLogic); return ( - <> - - - + +

{i18n.translate( @@ -56,7 +58,7 @@ export const CurationCreation: React.FC = () => { inputPlaceholder={QUERY_INPUTS_PLACEHOLDER} onSubmit={(queries) => createCuration(queries)} /> - - + + ); }; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curations.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curations.test.tsx index bcc402d6eea273..85827d53741793 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curations.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curations.test.tsx @@ -6,17 +6,16 @@ */ import { mockKibanaValues, setMockActions, setMockValues } from '../../../../__mocks__/kea_logic'; +import '../../../../__mocks__/react_router'; import '../../../__mocks__/engine_logic.mock'; import React from 'react'; import { shallow, ReactWrapper } from 'enzyme'; -import { EuiPageHeader, EuiBasicTable } from '@elastic/eui'; +import { EuiBasicTable } from '@elastic/eui'; -import { Loading } from '../../../../shared/loading'; -import { mountWithIntl } from '../../../../test_helpers'; -import { EmptyState } from '../components'; +import { mountWithIntl, getPageTitle } from '../../../../test_helpers'; import { Curations, CurationsTable } from './curations'; @@ -61,32 +60,34 @@ describe('Curations', () => { it('renders', () => { const wrapper = shallow(); - expect(wrapper.find(EuiPageHeader).prop('pageTitle')).toEqual('Curated results'); + expect(getPageTitle(wrapper)).toEqual('Curated results'); expect(wrapper.find(CurationsTable)).toHaveLength(1); }); - it('renders a loading component on page load', () => { - setMockValues({ ...values, dataLoading: true, curations: [] }); - const wrapper = shallow(); + describe('loading state', () => { + it('renders a full-page loading state on initial page load', () => { + setMockValues({ ...values, dataLoading: true, curations: [] }); + const wrapper = shallow(); + + expect(wrapper.prop('isLoading')).toEqual(true); + }); + + it('does not re-render a full-page loading state after initial page load (uses component-level loading state instead)', () => { + setMockValues({ ...values, dataLoading: true, curations: [{}] }); + const wrapper = shallow(); - expect(wrapper.find(Loading)).toHaveLength(1); + expect(wrapper.prop('isLoading')).toEqual(false); + }); }); it('calls loadCurations on page load', () => { + setMockValues({ ...values, myRole: {} }); // Required for AppSearchPageTemplate to load mountWithIntl(); expect(actions.loadCurations).toHaveBeenCalledTimes(1); }); describe('CurationsTable', () => { - it('renders an empty state', () => { - setMockValues({ ...values, curations: [] }); - const table = shallow().find(EuiBasicTable); - const noItemsMessage = table.prop('noItemsMessage') as React.ReactElement; - - expect(noItemsMessage.type).toEqual(EmptyState); - }); - it('passes loading prop based on dataLoading', () => { setMockValues({ ...values, dataLoading: true }); const wrapper = shallow(); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curations.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curations.tsx index 80de9aba772585..12497ab52baf6e 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curations.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curations.tsx @@ -9,25 +9,24 @@ import React, { useEffect } from 'react'; import { useValues, useActions } from 'kea'; -import { EuiBasicTable, EuiBasicTableColumn, EuiPageContent, EuiPageHeader } from '@elastic/eui'; +import { EuiBasicTable, EuiBasicTableColumn, EuiPanel } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { EDIT_BUTTON_LABEL, DELETE_BUTTON_LABEL } from '../../../../shared/constants'; -import { FlashMessages } from '../../../../shared/flash_messages'; import { KibanaLogic } from '../../../../shared/kibana'; -import { Loading } from '../../../../shared/loading'; import { EuiButtonTo, EuiLinkTo } from '../../../../shared/react_router_helpers'; import { convertMetaToPagination, handlePageChange } from '../../../../shared/table_pagination'; import { ENGINE_CURATIONS_NEW_PATH, ENGINE_CURATION_PATH } from '../../../routes'; import { FormattedDateTime } from '../../../utils/formatted_date_time'; import { generateEnginePath } from '../../engine'; +import { AppSearchPageTemplate } from '../../layout'; import { EmptyState } from '../components'; import { CURATIONS_OVERVIEW_TITLE, CREATE_NEW_CURATION_TITLE } from '../constants'; import { CurationsLogic } from '../curations_logic'; import { Curation } from '../types'; -import { convertToDate } from '../utils'; +import { getCurationsBreadcrumbs, convertToDate } from '../utils'; export const Curations: React.FC = () => { const { dataLoading, curations, meta } = useValues(CurationsLogic); @@ -37,23 +36,29 @@ export const Curations: React.FC = () => { loadCurations(); }, [meta.page.current]); - if (dataLoading && !curations.length) return ; - return ( - <> - + {CREATE_NEW_CURATION_TITLE} , - ]} - /> - - + ], + }} + isLoading={dataLoading && !curations.length} + isEmptyState={!curations.length} + emptyState={} + > + - - + + ); }; @@ -139,7 +144,6 @@ export const CurationsTable: React.FC = () => { responsive hasActions loading={dataLoading} - noItemsMessage={} pagination={{ ...convertMetaToPagination(meta), hidePerPageOptions: true, diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_router.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_router.tsx index 04e252e44270bf..f7698d8c5b5c2f 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_router.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_router.tsx @@ -119,6 +119,11 @@ export const EngineRouter: React.FC = () => { )} + {canManageEngineCurations && ( + + + + )} {canManageEngineResultSettings && ( @@ -136,11 +141,6 @@ export const EngineRouter: React.FC = () => { )} {/* TODO: Remove layout once page template migration is over */} }> - {canManageEngineCurations && ( - - - - )} {canManageEngineSynonyms && (