Skip to content

Commit

Permalink
[Enterprise Search] Product 404 polish pass (#103198) (#103350)
Browse files Browse the repository at this point in the history
* Refactor NotFound component

- shared NotFound becomes NotFoundPrompt - returns only an EuiEmptyPrompt, and individual products/plugins are in charge of their own layout, rather than NotFound doing a bunch of arduous switch handling (also closer to how errorConnecting is a component set per-plugin)

- This is both due to the recent page template refactor and the fact that WS has extra complex logic of needing to switch between its kibana layout and personal dashboard layout

- logos are still hosted in shared/ since they need extra custom CSS to work correctly sizing wise and in dark mode. I renamed its folder from `assets`->`logos` for extra clarity

* [AS] Update current AS routers using NotFound

+ update EngineRouter to use NotFound

* [WS] Update app router

- Handle errorConnecting at the topmost level, instead of in WorkplaceSearchConfigured (to simplify various logic/expectations & match App Search)

- Simplify isOrganization check to use `useRouteMatch` instead of a regex

- Use new NotFound component
- Add NotFound component for the personal dashboard router

* [WS] Improve Source 404 UX

- Add NotFound to SourceRouter + add breadcrumbs for organization views

- When an actual source ID 404s, fix blanket redirect to a dashboard aware redirect - personal dashboard 404s should send the user back to personal sources, not organization sources
+ add a flash message error (similar to how App Search behaves for engine 404s)
+ harden error status checks (gracefully allow for non-http errors to fall back flashAPIErrors

* [WS] Improve Settings 404 UX

- This was the only remaining WS route I found that either did not have a 404 or a fallback to some overview page, so I tweaked the redirect order for a graceful redirect (vs a blank page)

* Fix settings router test

* Move away from custom product logos to OOTB Enterprise Search logo

Keeping it simple, etc. RIP in peace fancy logos

* [PR feedback] toContain over stringContaining

Co-authored-by: Constance <constancecchen@users.noreply.github.com>
  • Loading branch information
kibanamachine and Constance authored Jun 24, 2021
1 parent ca36d0c commit 4d7b3af
Show file tree
Hide file tree
Showing 25 changed files with 378 additions and 394 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
import React from 'react';
import { Route, Switch, Redirect } from 'react-router-dom';

import { APP_SEARCH_PLUGIN } from '../../../../../common/constants';
import { NotFound } from '../../../shared/not_found';
import {
ENGINE_ANALYTICS_PATH,
ENGINE_ANALYTICS_TOP_QUERIES_PATH,
Expand All @@ -21,6 +19,7 @@ import {
ENGINE_ANALYTICS_QUERY_DETAIL_PATH,
} from '../../routes';
import { generateEnginePath, getEngineBreadcrumbs } from '../engine';
import { NotFound } from '../not_found';

import { ANALYTICS_TITLE } from './constants';
import {
Expand Down Expand Up @@ -61,10 +60,7 @@ export const AnalyticsRouter: React.FC = () => {
<Redirect to={generateEnginePath(ENGINE_ANALYTICS_PATH)} />
</Route>
<Route>
<NotFound
breadcrumbs={getEngineBreadcrumbs([ANALYTICS_TITLE])}
product={APP_SEARCH_PLUGIN}
/>
<NotFound pageChrome={getEngineBreadcrumbs([ANALYTICS_TITLE])} />
</Route>
</Switch>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,15 @@ import { CurationsRouter } from '../curations';
import { DocumentDetail, Documents } from '../documents';
import { EngineOverview } from '../engine_overview';
import { AppSearchPageTemplate } from '../layout';
import { NotFound } from '../not_found';
import { RelevanceTuning } from '../relevance_tuning';
import { ResultSettings } from '../result_settings';
import { SchemaRouter } from '../schema';
import { SearchUI } from '../search_ui';
import { SourceEngines } from '../source_engines';
import { Synonyms } from '../synonyms';

import { EngineLogic } from './';
import { EngineLogic, getEngineBreadcrumbs } from './';

export const EngineRouter: React.FC = () => {
const {
Expand Down Expand Up @@ -159,6 +160,9 @@ export const EngineRouter: React.FC = () => {
<ApiLogs />
</Route>
)}
<Route>
<NotFound pageChrome={getEngineBreadcrumbs()} />
</Route>
</Switch>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,4 @@
* 2.0.
*/

.logo404 {
width: $euiSize * 8;
height: $euiSize * 8;

fill: $euiColorEmptyShade;
stroke: $euiColorLightShade;

&__light {
fill: $euiColorLightShade;
}

&__dark {
fill: $euiColorMediumShade;
}
}
export { NotFound } from './not_found';
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';

import { shallow } from 'enzyme';

import { NotFoundPrompt } from '../../../shared/not_found';
import { SendAppSearchTelemetry } from '../../../shared/telemetry';
import { AppSearchPageTemplate } from '../layout';

import { NotFound } from './';

describe('NotFound', () => {
const wrapper = shallow(<NotFound />);

it('renders the shared not found prompt', () => {
expect(wrapper.find(NotFoundPrompt)).toHaveLength(1);
});

it('renders a telemetry error event', () => {
expect(wrapper.find(SendAppSearchTelemetry).prop('action')).toEqual('error');
});

it('passes optional preceding page chrome', () => {
wrapper.setProps({ pageChrome: ['Engines', 'some-engine'] });

expect(wrapper.find(AppSearchPageTemplate).prop('pageChrome')).toEqual([
'Engines',
'some-engine',
'404',
]);
});
});
Original file line number Diff line number Diff line change
@@ -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
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';

import { APP_SEARCH_PLUGIN } from '../../../../../common/constants';
import { PageTemplateProps } from '../../../shared/layout';
import { NotFoundPrompt } from '../../../shared/not_found';
import { SendAppSearchTelemetry } from '../../../shared/telemetry';
import { AppSearchPageTemplate } from '../layout';

export const NotFound: React.FC<PageTemplateProps> = ({ pageChrome = [] }) => {
return (
<AppSearchPageTemplate pageChrome={[...pageChrome, '404']} template="centeredContent">
<SendAppSearchTelemetry action="error" metric="not_found" />
<NotFoundPrompt productSupportUrl={APP_SEARCH_PLUGIN.SUPPORT_URL} />
</AppSearchPageTemplate>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { Redirect } from 'react-router-dom';

import { shallow, ShallowWrapper } from 'enzyme';

import { Layout, SideNav, SideNavLink } from '../shared/layout';
import { SideNav, SideNavLink } from '../shared/layout';

import { rerender } from '../test_helpers';

Expand Down Expand Up @@ -83,13 +83,6 @@ describe('AppSearchConfigured', () => {
wrapper = shallow(<AppSearchConfigured {...DEFAULT_INITIAL_APP_DATA} />);
});

it('renders with layout', () => {
expect(wrapper.find(Layout)).toHaveLength(1);
expect(wrapper.find(Layout).prop('readOnlyMode')).toBeFalsy();
expect(wrapper.find(EnginesOverview)).toHaveLength(1);
expect(wrapper.find(EngineRouter)).toHaveLength(1);
});

it('renders header actions', () => {
expect(renderHeaderActions).toHaveBeenCalled();
});
Expand All @@ -98,11 +91,9 @@ describe('AppSearchConfigured', () => {
expect(AppLogic).toHaveBeenCalledWith(DEFAULT_INITIAL_APP_DATA);
});

it('passes readOnlyMode state', () => {
setMockValues({ myRole: {}, readOnlyMode: true });
rerender(wrapper);

expect(wrapper.find(Layout).first().prop('readOnlyMode')).toEqual(true);
it('renders engine routes', () => {
expect(wrapper.find(EnginesOverview)).toHaveLength(1);
expect(wrapper.find(EngineRouter)).toHaveLength(1);
});

describe('routes with ability checks', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ import { APP_SEARCH_PLUGIN } from '../../../common/constants';
import { InitialAppData } from '../../../common/types';
import { HttpLogic } from '../shared/http';
import { KibanaLogic } from '../shared/kibana';
import { Layout, SideNav, SideNavLink } from '../shared/layout';
import { NotFound } from '../shared/not_found';
import { SideNav, SideNavLink } from '../shared/layout';

import { ROLE_MAPPINGS_TITLE } from '../shared/role_mapping/constants';

Expand All @@ -28,6 +27,7 @@ import { ErrorConnecting } from './components/error_connecting';
import { KibanaHeaderActions } from './components/layout';
import { Library } from './components/library';
import { MetaEngineCreation } from './components/meta_engine_creation';
import { NotFound } from './components/not_found';
import { RoleMappings } from './components/role_mappings';
import { Settings, SETTINGS_TITLE } from './components/settings';
import { SetupGuide } from './components/setup_guide';
Expand Down Expand Up @@ -85,7 +85,6 @@ export const AppSearchConfigured: React.FC<Required<InitialAppData>> = (props) =
},
} = useValues(AppLogic(props));
const { renderHeaderActions } = useValues(KibanaLogic);
const { readOnlyMode } = useValues(HttpLogic);

useEffect(() => {
renderHeaderActions(KibanaHeaderActions);
Expand Down Expand Up @@ -133,13 +132,7 @@ export const AppSearchConfigured: React.FC<Required<InitialAppData>> = (props) =
</Route>
)}
<Route>
<Layout navigation={<AppSearchNav />} readOnlyMode={readOnlyMode}>
<Switch>
<Route>
<NotFound product={APP_SEARCH_PLUGIN} />
</Route>
</Switch>
</Layout>
<NotFound />
</Route>
</Switch>
);
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
* 2.0.
*/

export { NotFound } from './not_found';
export { NotFoundPrompt } from './not_found_prompt';

This file was deleted.

Loading

0 comments on commit 4d7b3af

Please sign in to comment.