Skip to content

Commit

Permalink
[ML] Fixes handling of unrecognised URLs (#133157)
Browse files Browse the repository at this point in the history
* not found page

* replace not found page with a warning banner

* add check for non-empty pathname

* functional test

* disabled anomaly detection section

* update message

(cherry picked from commit 6d290df)
  • Loading branch information
darnautov committed Jun 3, 2022
1 parent 49ee611 commit 093652e
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 74 deletions.
43 changes: 23 additions & 20 deletions x-pack/plugins/ml/public/application/components/ml_page/ml_page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import React, { createContext, FC, useCallback, useMemo, useReducer } from 'react';
import { EuiLoadingContent, EuiPageContentBody } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { Route } from 'react-router-dom';
import { Redirect, Route, Switch } from 'react-router-dom';
import type { AppMountParameters } from '@kbn/core/public';
import { KibanaPageTemplate, RedirectAppLinks } from '@kbn/kibana-react-plugin/public';
import { useSideNavItems } from './side_nav';
Expand Down Expand Up @@ -137,25 +137,28 @@ const CommonPageWrapper: FC<CommonPageWrapperProps> = React.memo(
value={{ setPageTitle, setHeaderActionMenu: pageDeps.setHeaderActionMenu }}
>
<EuiPageContentBody restrictWidth={false}>
{routeList.map((route) => {
return (
<Route
key={route.id}
path={route.path}
exact
render={(props) => {
window.setTimeout(() => {
pageDeps.setBreadcrumbs(route.breadcrumbs);
});
return (
<MlPageWrapper path={route.path}>
{route.render(props, pageDeps)}
</MlPageWrapper>
);
}}
/>
);
})}
<Switch>
{routeList.map((route) => {
return (
<Route
key={route.id}
path={route.path}
exact
render={(props) => {
window.setTimeout(() => {
pageDeps.setBreadcrumbs(route.breadcrumbs);
});
return (
<MlPageWrapper path={route.path}>
{route.render(props, pageDeps)}
</MlPageWrapper>
);
}}
/>
);
})}
<Redirect to="/overview" />
</Switch>
</EuiPageContentBody>
</MlPageControlsContext.Provider>
</RedirectAppLinks>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ export function useSideNavItems(activeRoute: MlRoute | undefined) {
name: i18n.translate('xpack.ml.navMenu.anomalyDetectionTabLinkText', {
defaultMessage: 'Anomaly Detection',
}),
disabled: disableLinks,
items: [
{
id: 'anomaly_detection',
Expand Down
53 changes: 0 additions & 53 deletions x-pack/plugins/ml/public/application/routing/use_active_route.ts

This file was deleted.

99 changes: 99 additions & 0 deletions x-pack/plugins/ml/public/application/routing/use_active_route.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* 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 { useLocation, useRouteMatch } from 'react-router-dom';
import { keyBy } from 'lodash';
import React, { useEffect, useMemo, useRef } from 'react';
import { toMountPoint, useExecutionContext } from '@kbn/kibana-react-plugin/public';
import { EuiCallOut } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n-react';
import { useMlKibana } from '../contexts/kibana';
import type { MlRoute } from './router';

/**
* Provides an active route of the ML app.
* @param routesList
*/
export const useActiveRoute = (routesList: MlRoute[]): MlRoute => {
const { pathname } = useLocation();

const {
services: { executionContext, overlays, theme },
} = useMlKibana();

/**
* Temp fix for routes with params.
*/
const editCalendarMatch = useRouteMatch('/settings/calendars_list/edit_calendar/:calendarId');
const editFilterMatch = useRouteMatch('/settings/filter_lists/edit_filter_list/:filterId');

const routesMap = useMemo(() => keyBy(routesList, 'path'), []);

const activeRoute = useMemo(() => {
if (editCalendarMatch) {
return routesMap[editCalendarMatch.path];
}
if (editFilterMatch) {
return routesMap[editFilterMatch.path];
}
// Remove trailing slash from the pathname
const pathnameKey = pathname.replace(/\/$/, '');
return routesMap[pathnameKey];
}, [pathname]);

const bannerId = useRef<string | undefined>();

useEffect(
function handleNotFoundRoute() {
if (!activeRoute && !!pathname) {
bannerId.current = overlays.banners.replace(
bannerId.current,
toMountPoint(
<EuiCallOut
color="warning"
iconType="iInCircle"
title={
<FormattedMessage
id="xpack.ml.notFoundPage.title"
defaultMessage="Page Not Found"
/>
}
data-test-subj={'mlPageNotFoundBanner'}
>
<p data-test-subj={'mlPageNotFoundBannerText'}>
<FormattedMessage
id="xpack.ml.notFoundPage.bannerText"
defaultMessage="The Machine Learning application doesn't recognize this route: {route}. You've been redirected to the Overview page."
values={{
route: pathname,
}}
/>
</p>
</EuiCallOut>,
{ theme$: theme.theme$ }
)
);

// hide the message after the user has had a chance to acknowledge it -- so it doesn't permanently stick around
setTimeout(() => {
if (bannerId.current) {
overlays.banners.remove(bannerId.current);
}
}, 15000);
}
},
[activeRoute, overlays, theme, pathname]
);

useExecutionContext(executionContext, {
name: 'Machine Learning',
type: 'application',
page: activeRoute?.path ?? '/overview',
});

return activeRoute ?? routesMap['/overview'];
};
10 changes: 9 additions & 1 deletion x-pack/test/functional/apps/ml/permissions/read_ml_access.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import { FtrProviderContext } from '../../../ftr_provider_context';

import { USER } from '../../../services/ml/security_common';

export default function ({ getService }: FtrProviderContext) {
export default function ({ getService, getPageObjects }: FtrProviderContext) {
const esArchiver = getService('esArchiver');
const ml = getService('ml');
const PageObjects = getPageObjects(['common', 'error']);

const testUsers = [
{ user: USER.ML_VIEWER, discoverAvailable: true },
Expand Down Expand Up @@ -100,6 +101,13 @@ export default function ({ getService }: FtrProviderContext) {
await ml.overviewPage.assertDFACreateJobButtonExists();
await ml.overviewPage.assertDFACreateJobButtonEnabled(false);
});

it('should redirect to the Overview page from the unrecognized routes', async () => {
await PageObjects.common.navigateToUrl('ml', 'magic-ai');

await ml.testExecution.logTestStep('should display a warning banner');
await ml.overviewPage.assertPageNotFoundBannerText('magic-ai');
});
});
}
});
Expand Down
12 changes: 12 additions & 0 deletions x-pack/test/functional/services/ml/overview_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,17 @@ export function MachineLearningOverviewPageProvider({ getService }: FtrProviderC
async assertJobSyncRequiredWarningNotExists() {
await testSubjects.missingOrFail('mlJobSyncRequiredWarning', { timeout: 5000 });
},

async assertPageNotFoundBannerExists() {
await testSubjects.existOrFail('mlPageNotFoundBanner', { timeout: 5000 });
},

async assertPageNotFoundBannerText(pathname: string) {
await this.assertPageNotFoundBannerExists();
const text = await testSubjects.getVisibleText('mlPageNotFoundBannerText');
expect(text).to.eql(
`The Machine Learning application doesn't recognize this route: /ml/${pathname}. You've been redirected to the Overview page.`
);
},
};
}

0 comments on commit 093652e

Please sign in to comment.