Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show error page when accessing unavailable app #54656

Merged

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 1 addition & 11 deletions src/core/public/application/application_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -525,17 +525,7 @@ describe('#start()', () => {
const { getComponent } = await service.start(startDeps);

expect(() => shallow(createElement(getComponent))).not.toThrow();
expect(getComponent()).toMatchInlineSnapshot(`
<AppRouter
history={
Object {
"push": [MockFunction],
}
}
mounters={Map {}}
setAppLeaveHandler={[Function]}
/>
`);
expect(getComponent()).toMatchSnapshot();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AppRouter now have an observable as a property. This causes the snapshot to be incredibly long, as all inner properties of the observer are rendered. I would love to be able to exclude some properties from the snapshot, but that doesn't seem doable, so I moved from inline to file snapshot.

});

it('renders null when in legacy mode', async () => {
Expand Down
13 changes: 7 additions & 6 deletions src/core/public/application/application_service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import React from 'react';
import { BehaviorSubject, Observable, Subject, Subscription } from 'rxjs';
import { map, takeUntil } from 'rxjs/operators';
import { map, shareReplay, takeUntil } from 'rxjs/operators';
import { createBrowserHistory, History } from 'history';

import { InjectedMetadataSetup } from '../injected_metadata';
Expand Down Expand Up @@ -256,6 +256,11 @@ export class ApplicationService {
)
.subscribe(apps => applications$.next(apps));

const applicationStatuses$ = applications$.pipe(
map(apps => new Map([...apps.entries()].map(([id, app]) => [id, app.status!]))),
shareReplay(1)
);

return {
applications$,
capabilities,
Expand All @@ -264,11 +269,6 @@ export class ApplicationService {
getUrlForApp: (appId, { path }: { path?: string } = {}) =>
getAppUrl(availableMounters, appId, path),
navigateToApp: async (appId, { path, state }: { path?: string; state?: any } = {}) => {
const app = applications$.value.get(appId);
if (app && app.status !== AppStatus.accessible) {
// should probably redirect to the error page instead
throw new Error(`Trying to navigate to an inaccessible application: ${appId}`);
}
if (await this.shouldNavigate(overlays)) {
this.appLeaveHandlers.delete(this.currentAppId$.value!);
this.navigate!(getAppUrl(availableMounters, appId, path), state);
Expand All @@ -283,6 +283,7 @@ export class ApplicationService {
<AppRouter
history={this.history}
mounters={availableMounters}
appStatuses$={applicationStatuses$}
setAppLeaveHandler={this.setAppLeaveHandler}
/>
);
Expand Down
33 changes: 33 additions & 0 deletions src/core/public/application/integration_tests/router.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,18 @@
*/

import React from 'react';
import { BehaviorSubject } from 'rxjs';
import { createMemoryHistory, History, createHashHistory } from 'history';

import { AppRouter, AppNotFound } from '../ui';
import { EitherApp, MockedMounterMap, MockedMounterTuple } from '../test_types';
import { createRenderer, createAppMounter, createLegacyAppMounter } from './utils';
import { AppStatus } from '../types';

describe('AppContainer', () => {
let mounters: MockedMounterMap<EitherApp>;
let history: History;
let appStatuses$: BehaviorSubject<Map<string, AppStatus>>;
let update: ReturnType<typeof createRenderer>;

const navigate = (path: string) => {
Expand All @@ -38,19 +41,34 @@ describe('AppContainer', () => {
new Map([...mounters].map(([appId, { mounter }]) => [appId, mounter]));
const setAppLeaveHandlerMock = () => undefined;

const mountersToAppStatus$ = () => {
return new BehaviorSubject(
new Map(
[...mounters.keys()].map(id => [
id,
id.startsWith('disabled') ? AppStatus.inaccessible : AppStatus.accessible,
])
)
);
};

beforeEach(() => {
mounters = new Map([
createAppMounter('app1', '<span>App 1</span>'),
createLegacyAppMounter('legacyApp1', jest.fn()),
createAppMounter('app2', '<div>App 2</div>'),
createLegacyAppMounter('baseApp:legacyApp2', jest.fn()),
createAppMounter('app3', '<div>App 3</div>', '/custom/path'),
createAppMounter('disabledApp', '<div>Disabled app</div>'),
createLegacyAppMounter('disabledLegacyApp', jest.fn()),
] as Array<MockedMounterTuple<EitherApp>>);
history = createMemoryHistory();
appStatuses$ = mountersToAppStatus$();
update = createRenderer(
<AppRouter
history={history}
mounters={mockMountersToMounters()}
appStatuses$={appStatuses$}
setAppLeaveHandler={setAppLeaveHandlerMock}
/>
);
Expand Down Expand Up @@ -89,6 +107,7 @@ describe('AppContainer', () => {
<AppRouter
history={history}
mounters={mockMountersToMounters()}
appStatuses$={mountersToAppStatus$()}
setAppLeaveHandler={setAppLeaveHandlerMock}
/>
);
Expand All @@ -107,6 +126,7 @@ describe('AppContainer', () => {
<AppRouter
history={history}
mounters={mockMountersToMounters()}
appStatuses$={mountersToAppStatus$()}
setAppLeaveHandler={setAppLeaveHandlerMock}
/>
);
Expand Down Expand Up @@ -147,6 +167,7 @@ describe('AppContainer', () => {
<AppRouter
history={history}
mounters={mockMountersToMounters()}
appStatuses$={mountersToAppStatus$()}
setAppLeaveHandler={setAppLeaveHandlerMock}
/>
);
Expand Down Expand Up @@ -202,4 +223,16 @@ describe('AppContainer', () => {

expect(dom?.exists(AppNotFound)).toBe(true);
});

it('displays error page if app is inaccessible', async () => {
const dom = await navigate('/app/disabledApp');

expect(dom?.exists(AppNotFound)).toBe(true);
});

it('displays error page if legacy app is inaccessible', async () => {
const dom = await navigate('/app/disabledLegacyApp');

expect(dom?.exists(AppNotFound)).toBe(true);
});
});
9 changes: 6 additions & 3 deletions src/core/public/application/ui/app_container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,26 @@ import React, {
MutableRefObject,
} from 'react';

import { AppUnmount, Mounter, AppLeaveHandler } from '../types';
import { AppLeaveHandler, AppStatus, AppUnmount, Mounter } from '../types';
import { AppNotFound } from './app_not_found_screen';

interface Props {
appId: string;
mounter?: Mounter;
appStatus: AppStatus;
setAppLeaveHandler: (appId: string, handler: AppLeaveHandler) => void;
}

export const AppContainer: FunctionComponent<Props> = ({
mounter,
appId,
setAppLeaveHandler,
appStatus,
}: Props) => {
const [appNotFound, setAppNotFound] = useState(false);
const elementRef = useRef<HTMLDivElement>(null);
const unmountRef: MutableRefObject<AppUnmount | null> = useRef<AppUnmount>(null);
// const appStatus = useObservable(appStatus$);

useLayoutEffect(() => {
const unmount = () => {
Expand All @@ -52,7 +55,7 @@ export const AppContainer: FunctionComponent<Props> = ({
}
};
const mount = async () => {
if (!mounter) {
if (!mounter || appStatus !== AppStatus.accessible) {
return setAppNotFound(true);
}

Expand All @@ -71,7 +74,7 @@ export const AppContainer: FunctionComponent<Props> = ({

mount();
return unmount;
}, [appId, mounter, setAppLeaveHandler]);
}, [appId, appStatus, mounter, setAppLeaveHandler]);

return (
<Fragment>
Expand Down
2 changes: 1 addition & 1 deletion src/core/public/application/ui/app_not_found_screen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import React from 'react';
import { FormattedMessage } from '@kbn/i18n/react';

export const AppNotFound = () => (
<EuiPage style={{ minHeight: '100%' }}>
<EuiPage style={{ minHeight: '100%' }} data-test-subj="appNotFoundPageContent">
<EuiPageBody>
<EuiPageContent verticalPosition="center" horizontalPosition="center">
<EuiEmptyPrompt
Expand Down
Loading