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

[Endpoint] add react router to endpoint app #53808

Merged
merged 12 commits into from
Jan 2, 2020
85 changes: 70 additions & 15 deletions test/functional/page_objects/common_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ export function CommonPageProvider({ getService, getPageObjects }: FtrProviderCo
const defaultTryTimeout = config.get('timeouts.try');
const defaultFindTimeout = config.get('timeouts.find');

interface NavigateProps {
appConfig: {};
ensureCurrentUrl: boolean;
shouldLoginIfPrompted: boolean;
shouldAcceptAlert: boolean;
useActualUrl: boolean;
}

class CommonPage {
/**
* Navigates the browser window to provided URL
Expand Down Expand Up @@ -115,6 +123,34 @@ export function CommonPageProvider({ getService, getPageObjects }: FtrProviderCo
return currentUrl;
}

private async navigate(navigateProps: NavigateProps) {
const {
appConfig,
ensureCurrentUrl,
shouldLoginIfPrompted,
shouldAcceptAlert,
useActualUrl,
} = navigateProps;
const appUrl = getUrl.noAuth(config.get('servers.kibana'), appConfig);

await retry.try(async () => {
if (useActualUrl) {
log.debug(`navigateToActualUrl ${appUrl}`);
await browser.get(appUrl);
} else {
await CommonPage.navigateToUrlAndHandleAlert(appUrl, shouldAcceptAlert);
}

const currentUrl = shouldLoginIfPrompted
? await this.loginIfPrompted(appUrl)
: await browser.getCurrentUrl();

if (ensureCurrentUrl && !currentUrl.includes(appUrl)) {
throw new Error(`expected ${currentUrl}.includes(${appUrl})`);
}
});
}

/**
* Navigates browser using the pathname from the appConfig and subUrl as the hash
* @param appName As defined in the apps config, e.g. 'home'
Expand All @@ -137,23 +173,42 @@ export function CommonPageProvider({ getService, getPageObjects }: FtrProviderCo
hash: useActualUrl ? subUrl : `/${appName}/${subUrl}`,
};

const appUrl = getUrl.noAuth(config.get('servers.kibana'), appConfig);

await retry.try(async () => {
if (useActualUrl) {
log.debug(`navigateToActualUrl ${appUrl}`);
await browser.get(appUrl);
} else {
await CommonPage.navigateToUrlAndHandleAlert(appUrl, shouldAcceptAlert);
}
await this.navigate({
appConfig,
ensureCurrentUrl,
shouldLoginIfPrompted,
shouldAcceptAlert,
useActualUrl,
});
}

const currentUrl = shouldLoginIfPrompted
? await this.loginIfPrompted(appUrl)
: await browser.getCurrentUrl();
/**
* Navigates browser using the pathname from the appConfig and subUrl as the extended path
* @param appName As defined in the apps config, e.g. 'home'
* @param subUrl The route after the appUrl, e.g. 'tutorial_directory/sampleData'
* @param args additional arguments
*/
public async navigateToRealUrl(
appName: string,
subUrl?: string,
{
basePath = '',
ensureCurrentUrl = true,
shouldLoginIfPrompted = true,
shouldAcceptAlert = true,
useActualUrl = true,
} = {}
) {
const appConfig = {
pathname: `${basePath}${config.get(['apps', appName]).pathname}${subUrl}`,
};

if (ensureCurrentUrl && !currentUrl.includes(appUrl)) {
throw new Error(`expected ${currentUrl}.includes(${appUrl})`);
}
await this.navigate({
appConfig,
ensureCurrentUrl,
shouldLoginIfPrompted,
shouldAcceptAlert,
useActualUrl,
});
}

Expand Down
45 changes: 39 additions & 6 deletions x-pack/plugins/endpoint/public/applications/endpoint/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,57 @@ import * as React from 'react';
import ReactDOM from 'react-dom';
import { CoreStart, AppMountParameters } from 'kibana/public';
import { I18nProvider, FormattedMessage } from '@kbn/i18n/react';
import { History, createBrowserHistory } from 'history';
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need these if you use <BrowserRouter /> component from react-router-dom instead of <Router />

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call, changed out

import { Route, Router, Switch } from 'react-router-dom';

/**
* This module will be loaded asynchronously to reduce the bundle size of your plugin's main bundle.
*/
export function renderApp(coreStart: CoreStart, { element }: AppMountParameters) {
export function renderApp(coreStart: CoreStart, { appBasePath, element }: AppMountParameters) {
coreStart.http.get('/api/endpoint/hello-world');

ReactDOM.render(<AppRoot />, element);
const history = createBrowserHistory({ basename: appBasePath });
ReactDOM.render(<AppRoot history={history} />, element);

return () => {
ReactDOM.unmountComponentAtNode(element);
};
}

const AppRoot = React.memo(() => (
interface RouterProps {
history: History;
}

const AppRoot: React.FunctionComponent<RouterProps> = React.memo(({ history }) => (
<I18nProvider>
<h1 data-test-subj="welcomeTitle">
<FormattedMessage id="xpack.endpoint.welcomeTitle" defaultMessage="Hello World" />
</h1>
<Router history={history}>
Copy link
Contributor

@paul-tavares paul-tavares Dec 27, 2019

Choose a reason for hiding this comment

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

Consider using React Router's HashRouter instead --- until we can figure out how to use the Functional test framework with "real" URLs and then switch over to react-router's BrowserRotuer component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could also probably write a helper function to just navigate us to a real url. Other plugins in Kibana all seem to use hashes and I just wanna understand why.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the "old" platform client side framework was hash based so that might be the reason why all (most) current functional test use it. 🤷‍♂

Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we figured out support for "real" URLs, consider using react-router's <BrowserRouter /> component instead, which avoid having to create/manage a history instance ourselves

<Switch>
<Route
exact
path="/"
render={() => (
<h1 data-test-subj="welcomeTitle">
<FormattedMessage id="xpack.endpoint.welcomeTitle" defaultMessage="Hello World" />
</h1>
)}
/>
<Route
path="/management"
render={() => (
<h1 data-test-subj="endpointManagement">
<FormattedMessage
id="xpack.endpoint.endpointManagement"
defaultMessage="Manage Endpoints"
/>
</h1>
)}
/>
<Route
render={() => (
<FormattedMessage id="xpack.endpoint.notFound" defaultMessage="Page Not Found" />
)}
/>
</Switch>
</Router>
</I18nProvider>
));
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ export default function({ getPageObjects, getService }: FtrProviderContext) {
});
await testSubjects.existOrFail('welcomeTitle');
});

it(`endpoint management shows 'Manage Endpoints'`, async () => {
await pageObjects.common.navigateToRealUrl('endpoint', '/management', {
basePath: '/s/custom_space',
ensureCurrentUrl: false,
shouldLoginIfPrompted: false,
});
await testSubjects.existOrFail('endpointManagement');
});
});

describe('space with endpoint disabled', () => {
Expand Down