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

Add ApplicationService app status management #50223

Merged
merged 40 commits into from
Jan 12, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
e87d550
add unimplemented registerAppStatusUpdater & remove observers for app…
pgayvallet Nov 11, 2019
041cef1
adapt NavLinksService to use new application observables
pgayvallet Nov 11, 2019
76ee873
merge availableApps$ and availableLegacyApps$
pgayvallet Nov 11, 2019
f278903
updating core docs
pgayvallet Nov 11, 2019
5dba8fc
adapt the navLink updating methods
pgayvallet Nov 12, 2019
865eec5
filters the inaccessible apps from availableApps$
pgayvallet Nov 12, 2019
09cd6d8
restrict access to navigateToApp depending on app status
pgayvallet Nov 12, 2019
0bde308
Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…
pgayvallet Nov 19, 2019
ccd6344
fixes due to merge
pgayvallet Nov 19, 2019
dd8c438
add statusUpdater$ to AppBase
pgayvallet Nov 19, 2019
038ae4e
export new types
pgayvallet Nov 19, 2019
4aa0e2c
disable navlink depending on app status
pgayvallet Nov 19, 2019
162ec01
update generated doc
pgayvallet Nov 19, 2019
1c76e68
update snapshots for disabled prop
pgayvallet Nov 19, 2019
c5385f0
Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…
pgayvallet Nov 19, 2019
b031186
Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…
pgayvallet Nov 19, 2019
8ace804
Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…
pgayvallet Nov 22, 2019
0bfe560
Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…
pgayvallet Nov 27, 2019
7dca7fa
Address josh review
pgayvallet Nov 27, 2019
a6e9d8f
Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…
pgayvallet Dec 2, 2019
a5852a1
Address review comments
pgayvallet Dec 3, 2019
8ccfc62
Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…
pgayvallet Dec 3, 2019
eee0d44
Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…
pgayvallet Dec 3, 2019
6a0d0c1
fix merge conflicts
pgayvallet Dec 3, 2019
132938b
Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…
pgayvallet Jan 6, 2020
514a362
adapt changes due to merge
pgayvallet Jan 7, 2020
196d183
update generated doc
pgayvallet Jan 7, 2020
ad9ebb8
add comment and fix navlink url for custom url apps
pgayvallet Jan 7, 2020
57be7fd
add AppNavLinkStatus type to split app/navlink states
pgayvallet Jan 7, 2020
1c8d040
Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…
pgayvallet Jan 7, 2020
b1b6159
fix typo
pgayvallet Jan 7, 2020
87a7c40
review comments and improvements
pgayvallet Jan 9, 2020
626493f
Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…
pgayvallet Jan 9, 2020
f2b1e78
add functional tests
pgayvallet Jan 10, 2020
ab73ba3
Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…
pgayvallet Jan 10, 2020
12f469d
update generated docs and migration guide
pgayvallet Jan 10, 2020
f488946
fix wrong type cast on AppsMenuProvider.readLinks
pgayvallet Jan 10, 2020
815617b
Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…
pgayvallet Jan 10, 2020
cd6a2d7
Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…
pgayvallet Jan 12, 2020
b66df7e
properly type return of navigateToApp
pgayvallet Jan 12, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/core/public/chrome/ui/header/header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ class HeaderUI extends Component<Props, State> {
.filter(navLink => !navLink.hidden)
.map(navLink => ({
key: navLink.id,
label: navLink.title,
label: navLink.tooltip ?? navLink.title,

// Use href and onClick to support "open in new tab" and SPA navigation in the same link
href: navLink.href,
Expand Down
11 changes: 10 additions & 1 deletion test/functional/services/apps_menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export function AppsMenuProvider({ getService }: FtrProviderContext) {

return new (class AppsMenu {
/**
* Get the text and href from each of the links in the apps menu
* Get the attributes from each of the links in the apps menu
*/
public async readLinks() {
const appMenu = await testSubjects.find('navDrawer');
Expand All @@ -37,12 +37,21 @@ export function AppsMenuProvider({ getService }: FtrProviderContext) {
return {
text: $(link).text(),
href: $(link).attr('href'),
disabled: $(link).attr('disabled') != null,
};
});

return links;
}

/**
* Get the attributes from the link with the given name.
* @param name
*/
public async getLink(name: string) {
return (await this.readLinks()).find(nl => nl.text === name);
}

/**
* Determine if an app link with the given name exists
* @param name
Expand Down
8 changes: 8 additions & 0 deletions test/plugin_functional/plugins/core_app_status/kibana.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id": "core_app_status",
"version": "0.0.1",
"kibanaVersion": "kibana",
"configPath": ["core_app_status"],
"server": false,
"ui": true
}
17 changes: 17 additions & 0 deletions test/plugin_functional/plugins/core_app_status/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"name": "core_app_status",
"version": "1.0.0",
"main": "target/test/plugin_functional/plugins/core_app_status",
"kibana": {
"version": "kibana",
"templateVersion": "1.0.0"
},
"license": "Apache-2.0",
"scripts": {
"kbn": "node ../../../../scripts/kbn.js",
"build": "rm -rf './target' && tsc"
},
"devDependencies": {
"typescript": "3.5.3"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import React from 'react';
import { render, unmountComponentAtNode } from 'react-dom';
import {
EuiPage,
EuiPageBody,
EuiPageContent,
EuiPageContentBody,
EuiPageContentHeader,
EuiPageContentHeaderSection,
EuiPageHeader,
EuiPageHeaderSection,
EuiTitle,
} from '@elastic/eui';

import { AppMountContext, AppMountParameters } from 'kibana/public';

const AppStatusApp = () => (
<EuiPage>
<EuiPageBody data-test-subj="appStatusApp">
<EuiPageHeader>
<EuiPageHeaderSection>
<EuiTitle size="l">
<h1>Welcome to App Status Test App!</h1>
</EuiTitle>
</EuiPageHeaderSection>
</EuiPageHeader>
<EuiPageContent>
<EuiPageContentHeader>
<EuiPageContentHeaderSection>
<EuiTitle>
<h2>App Status Test App home page section title</h2>
</EuiTitle>
</EuiPageContentHeaderSection>
</EuiPageContentHeader>
<EuiPageContentBody>App Status Test App content</EuiPageContentBody>
</EuiPageContent>
</EuiPageBody>
</EuiPage>
);

export const renderApp = (context: AppMountContext, { element }: AppMountParameters) => {
render(<AppStatusApp />, element);

return () => unmountComponentAtNode(element);
};
24 changes: 24 additions & 0 deletions test/plugin_functional/plugins/core_app_status/public/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { PluginInitializer } from 'kibana/public';
import { CoreAppStatusPlugin, CoreAppStatusPluginSetup, CoreAppStatusPluginStart } from './plugin';

export const plugin: PluginInitializer<CoreAppStatusPluginSetup, CoreAppStatusPluginStart> = () =>
new CoreAppStatusPlugin();
56 changes: 56 additions & 0 deletions test/plugin_functional/plugins/core_app_status/public/plugin.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { Plugin, CoreSetup, AppUpdater, AppUpdatableFields, CoreStart } from 'kibana/public';
import { BehaviorSubject } from 'rxjs';

export class CoreAppStatusPlugin
implements Plugin<CoreAppStatusPluginSetup, CoreAppStatusPluginStart> {
private appUpdater = new BehaviorSubject<AppUpdater>(() => ({}));

public setup(core: CoreSetup, deps: {}) {
core.application.register({
id: 'app_status',
title: 'App Status',
euiIconType: 'snowflake',
updater$: this.appUpdater,
async mount(context, params) {
const { renderApp } = await import('./application');
return renderApp(context, params);
},
});

return {};
}

public start(core: CoreStart) {
return {
setAppStatus: (status: Partial<AppUpdatableFields>) => {
this.appUpdater.next(() => status);
},
navigateToApp: async (appId: string) => {
return core.application.navigateToApp(appId);
},
};
}
public stop() {}
}

export type CoreAppStatusPluginSetup = ReturnType<CoreAppStatusPlugin['setup']>;
export type CoreAppStatusPluginStart = ReturnType<CoreAppStatusPlugin['start']>;
14 changes: 14 additions & 0 deletions test/plugin_functional/plugins/core_app_status/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"extends": "../../../../tsconfig.json",
"compilerOptions": {
"outDir": "./target",
"skipLibCheck": true
},
"include": [
"index.ts",
"public/**/*.ts",
"public/**/*.tsx",
"../../../../typings/**/*",
],
"exclude": []
}
112 changes: 112 additions & 0 deletions test/plugin_functional/test_suites/core_plugins/application_status.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import expect from '@kbn/expect';
import {
AppNavLinkStatus,
AppStatus,
AppUpdatableFields,
} from '../../../../src/core/public/application/types';
import { PluginFunctionalProviderContext } from '../../services';
import { CoreAppStatusPluginStart } from '../../plugins/core_app_status/public/plugin';
import '../../plugins/core_provider_plugin/types';

// eslint-disable-next-line import/no-default-export
export default function({ getService, getPageObjects }: PluginFunctionalProviderContext) {
const PageObjects = getPageObjects(['common']);
const browser = getService('browser');
const appsMenu = getService('appsMenu');

const setAppStatus = async (s: Partial<AppUpdatableFields>) => {
await browser.executeAsync(async (status: Partial<AppUpdatableFields>, cb: Function) => {
const plugin = window.__coreProvider.start.plugins
.core_app_status as CoreAppStatusPluginStart;
plugin.setAppStatus(status);
cb();
}, s);
};

const navigateToApp = async (i: string) => {
return await browser.executeAsync(async (appId, cb: Function) => {
// navigating in legacy mode performs a page refresh
// and webdriver seems to re-execute the script after the reload
// as it considers it didn't end on the previous session.
// however when testing navigation to NP app, __coreProvider is not accessible
// so we need to check for existence.
if (!window.__coreProvider) {
cb();
}
const plugin = window.__coreProvider.start.plugins
.core_app_status as CoreAppStatusPluginStart;
try {
await plugin.navigateToApp(appId);
cb();
} catch (e) {
cb(e.message);
Copy link
Contributor

Choose a reason for hiding this comment

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

optional: we can use an object to be more explicit cb({error: e.message})

Copy link
Contributor Author

@pgayvallet pgayvallet Jan 12, 2020

Choose a reason for hiding this comment

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

Done. As you already mentioned, our wrong signature on browser.executeAsync forces to cast when returning a typed thing from the callback. But even the webdriver signature is quite wrong, as it does not explicitly specify the last cb parameter's type...

// webdriver/index.d.ts
executeAsyncScript<T>(script: string|Function, ...var_args: any[]): Promise<T>;

}
}, i);
};

describe('application status management', () => {
beforeEach(async () => {
await PageObjects.common.navigateToApp('settings');
});

it('can change the navLink status at runtime', async () => {
await setAppStatus({
navLinkStatus: AppNavLinkStatus.disabled,
});
let link = await appsMenu.getLink('App Status');
expect(link).not.to.eql(undefined);
expect(link!.disabled).to.eql(true);

await setAppStatus({
navLinkStatus: AppNavLinkStatus.hidden,
});
link = await appsMenu.getLink('App Status');
expect(link).to.eql(undefined);

await setAppStatus({
navLinkStatus: AppNavLinkStatus.visible,
tooltip: 'Some tooltip',
});
link = await appsMenu.getLink('Some tooltip'); // the tooltip replaces the name in the selector we use.
expect(link).not.to.eql(undefined);
expect(link!.disabled).to.eql(false);
});

it('shows an error when navigating to an inaccessible app', async () => {
await setAppStatus({
status: AppStatus.inaccessible,
});

const error = await navigateToApp('app_status');
expect(error).to.contain('Trying to navigate to an inaccessible application: app_status');
});

it('allows to navigate to an accessible app', async () => {
await setAppStatus({
status: AppStatus.accessible,
});

const error = await navigateToApp('app_status');
expect(error).to.eql(null);
});
});
}
1 change: 1 addition & 0 deletions test/plugin_functional/test_suites/core_plugins/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,6 @@ export default function({ loadTestFile }: PluginFunctionalProviderContext) {
loadTestFile(require.resolve('./ui_plugins'));
loadTestFile(require.resolve('./ui_settings'));
loadTestFile(require.resolve('./top_nav'));
loadTestFile(require.resolve('./application_status'));
});
}