Add ApplicationService app status management #50223

pgayvallet · 2019-11-11T22:10:47Z

Summary

PR adds ways to manage the ApplicationService applications status.

Introduce the AppUpdater type.
Add a registerAppUpdater method on ApplicationSetup that allows control over all applications status.
Add an updater$ on application registration (AppBase) that allows control over a specific registered application.
Change ApplicationService.availableApps to an availableApps$ observable that re-emit any time any application change.
Merge ApplicationService's availableApps and legacyApps for simplification and easier legacy removal when the time comes.

Both approaches relies on observables to update the statuses: every time the Observable emit a new values, the availableApplications$ re-emit the updated list.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, including a check against IE11~~
~~- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~

Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
~~- [ ] This was checked for keyboard-only and screenreader accessibility~~

For maintainers

~~- [ ] This was checked for breaking API changes and was labeled appropriately~~
~~- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately~~

…s and legacyApps

src/core/public/application/application_service.tsx

pgayvallet · 2019-11-12T10:01:38Z

src/core/public/application/application_service.tsx

+        if (this.apps.get(appId) === undefined) {
+          throw new Error(`Trying to navigate to an unknown application: ${appId}`);
+        }
+        const app = availableApps$.value.get(appId);
+        if (app === undefined || app.status !== AppStatus.accessible) {
+          throw new Error(`Trying to navigate to an inaccessible application: ${appId}`);
+        }


What is our strategy regarding error throwing ? Should I create a custom error type every time, even when not enhancing Error with custom props?

That's a good question. navigateToApp is a part of public API and can be called by any plugin. If we throw an error here, it becomes a plugin responsibility to handle the error. I think the core should handle core errors.

So instead of throwing, we should probably redirect to a 404 page or something similar ?

or probably call FatalError API https://github.com/pgayvallet/kibana/blob/09cd6d82af8dd192036d96cabe803c4ce7615a4f/src/core/public/fatal_errors/fatal_errors_service.tsx#L49
Although it doesn't scale well. We can forget to wrap the public method, for example. Probably it worth a separate discussion.

One option in this case is to redirect to the route (basically, remove this check) and rely on the existing "Application Not Found" UI to show. That would be a bit more graceful than breaking the calling code, but may obscure a bug. Though this case also exposes the need for plugins to know which applications are available in order to show/hide links to other apps.

Though this case also exposes the need for plugins to know which applications are available in order to show/hide links to other apps.

Have they got other links besides navLinks?
The platform could provide a list of available apps. Then plugins can use this info to enable/disable links. Should they hide links or make them disabled (for the sake of UI consistency)?
For now Application Not Found is ok. And it's inlined with behavior when a user lands on URL for an unknown or disabled app.

removed the error logic, user will now lands on the 404 page when redirecting to a non-existing app

pgayvallet · 2019-11-12T10:02:42Z

src/core/public/application/capabilities/merge_capabilities.ts

-export const mergeCapabilities = (...sources: Array<Partial<Capabilities>>) =>
-  sources.reduce(
-    (capabilities, source) => {
-      Object.entries(source).forEach(([key, value]) => {


File was unused

pgayvallet · 2019-11-12T10:03:34Z

src/core/public/application/types.ts

-   * An observable for a tooltip shown when hovering over app link.
+   * A tooltip shown when hovering over app link.
   */
-  tooltip$?: Observable<string>;
+  tooltip?: string;


Reverted the tooltip observable, as it was currently unimplemented, and should now be updated using the new AppStatusUpdater mechanism

src/core/public/application/types.ts

pgayvallet · 2019-11-12T10:05:19Z

src/core/public/application/ui/app_router.tsx

-export const AppRouter: React.StatelessComponent<Props> = ({
+export const AppRouter: React.FunctionComponent<Props> = ({


StatelessComponent is deprecated due to react hooks. FunctionComponent is the new terminology.

pgayvallet · 2019-11-12T10:09:22Z

src/core/public/chrome/nav_links/nav_links_service.ts

-        navLinks$.next(
+        const updater: LinksUpdater = navLinks =>
          new Map(
-            [...navLinks$.value.entries()].map(([linkId, link]) => {
+            [...navLinks.entries()].map(([linkId, link]) => {
              return [linkId, link.id === id ? link.update(values) : link] as [
                string,
                NavLinkWrapper
              ];
            })
-          )
-        );
+          );

+        linkUpdaters$.next([...linkUpdaters$.value, updater]);


Before PR, we were reading the application.availableApps only once at startup, which allowed to directly edit the links in NavlinkServices. As availableApps$ is now an Observable, I have to keep track of every 'manually' applied changes in NavlinkServices to reapply them after each availableApps$ emission. Hopefully at some point, we will no longer allow direct edition to Navlinks, and this will disappear, but for now, this seems like a necessary evil for retro-compat.

elasticmachine · 2019-11-12T10:10:32Z

Pinging @elastic/kibana-platform (Team:Platform)

elasticmachine · 2019-11-12T11:16:28Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 09cd6d8

src/core/public/application/application_service.tsx

…tion-filter

elasticmachine · 2019-11-19T12:28:41Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 162ec01

elasticmachine · 2019-11-19T14:07:43Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 1c76e68

pgayvallet · 2020-01-10T10:54:26Z

@elastic/apm-ui @elastic/ml-ui @elastic/kibana-gis

I changed some AppsMenuProvider types for FTR and had to remove all the wrongs and useless castings from readLinks result to Record<string, string> in every security tests. Theses are the only changes impacting your owned files.

…tion-filter

pheyos

ML test file changes LGTM

mshustov · 2020-01-10T12:38:45Z

test/plugin_functional/test_suites/core_plugins/application_status.ts

+        await plugin.navigateToApp(appId);
+        cb();
+      } catch (e) {
+        cb(e.message);


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

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>;

dgieselaar

APM changes LGTM

kindsun

Maps lgtm! Thanks for the clean up!

…tion-filter

kibanamachine · 2020-01-12T17:51:28Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: b66df7e

History

💚 Build #19241 succeeded 815617b
💚 Build #19224 succeeded f488946
💔 Build #19206 failed ab73ba3
💚 Build #19047 succeeded 626493f
💚 Build #18352 succeeded b1b6159

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* add unimplemented registerAppStatusUpdater & remove observers for apps and legacyApps * adapt NavLinksService to use new application observables * merge availableApps$ and availableLegacyApps$ * updating core docs * adapt the navLink updating methods * filters the inaccessible apps from availableApps$ * restrict access to navigateToApp depending on app status * fixes due to merge * add statusUpdater$ to AppBase * export new types * disable navlink depending on app status * update generated doc * update snapshots for disabled prop * Address josh review * Address review comments * fix merge conflicts * adapt changes due to merge * update generated doc * add comment and fix navlink url for custom url apps * add AppNavLinkStatus type to split app/navlink states * fix typo * review comments and improvements * add functional tests * update generated docs and migration guide * fix wrong type cast on AppsMenuProvider.readLinks * properly type return of navigateToApp

* master: (69 commits) [Graph] Fix various a11y issues (elastic#54097) Add ApplicationService app status management (elastic#50223) logs in one time (elastic#54447) Deprecate using `elasticsearch.ssl.certificate` without `elasticsearch.ssl.key` and vice versa (elastic#54392) [Optimizer] Fix a stack overflow with watch_cache when it attempts to delete very large folders. (elastic#54457) Security - Role Mappings UI (elastic#53620) [SIEM] [Detection engine] Permission II (elastic#54292) Allow User to Cleanup Repository from UI (elastic#53047) [Detection engine] Some UX for rule creation (elastic#54471) share specific instances of some ui packages (elastic#54079) [ML] APM modules configs for RUM Javascript and NodeJS (elastic#53792) [APM] Delay rendering invalid license notification (elastic#53924) [Graph] Improve error message on graph requests (elastic#54230) [ILM] Kibana should allow a min_age setting of 0ms in ILM policy phases (elastic#53719) Unit Tests for common/lib (elastic#53736) [Graph] Only show explorable fields (elastic#54101) remove linting rule exception for markdown (elastic#54232) [Monitoring] Fetch shard data more efficiently (elastic#54028) [Maps] Add hiddenLayers option to embeddable map input (elastic#54355) Pass termOrder and hasTermsAgg properties to serializeThresholdWatch function (elastic#54391) ...

* add unimplemented registerAppStatusUpdater & remove observers for apps and legacyApps * adapt NavLinksService to use new application observables * merge availableApps$ and availableLegacyApps$ * updating core docs * adapt the navLink updating methods * filters the inaccessible apps from availableApps$ * restrict access to navigateToApp depending on app status * fixes due to merge * add statusUpdater$ to AppBase * export new types * disable navlink depending on app status * update generated doc * update snapshots for disabled prop * Address josh review * Address review comments * fix merge conflicts * adapt changes due to merge * update generated doc * add comment and fix navlink url for custom url apps * add AppNavLinkStatus type to split app/navlink states * fix typo * review comments and improvements * add functional tests * update generated docs and migration guide * fix wrong type cast on AppsMenuProvider.readLinks * properly type return of navigateToApp

This was already backported, but changes to endpoint app could not be backported, since endpoint app itself hadn't been backported. Now that the endpoint app is backported, reapply the endpoint specific changes from the original commit.

* Add Endpoint plugin and Resolver embeddable (#51994) * Add functional tests for plugins to x-pack (so we can do a functional test of the Resolver embeddable) * Add Endpoint plugin * Add Resolver embeddable * Test that Resolver embeddable can be rendered Conflicts: x-pack/.i18nrc.json x-pack/test/api_integration/apis/index.js * [Endpoint] Register endpoint app (#53527) * register app, create functional test * formatting * update tests * adjust test data for endpoint * add endpoint tests for testing spaces, app enabled, disabled, etc * linting * add read privileges to endpoint * rename variable since its used now * remove deprecated context * remove unused variable * fix type check * correct test suite message Co-Authored-By: Larry Gregory <[email protected]> Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Larry Gregory <[email protected]> * [Endpoint] add react router to endpoint app (#53808) * add react router to endpoint app * linting * linting * linting * correct tests * change history from hash to browser, add new test util * remove default values in helper functions * fix type check, use FunctionComponent as oppsed to FC * use BrowserRouter component * use BrowserRouter component lin * add comments to test framework, change function name to include browserHistory Co-authored-by: Elastic Machine <[email protected]> * EMT-issue-65: add endpoint list api (#53861) add endpoint list api * EMT-65:always return accurate endpoint count (#54423) EMT-65:always return accurate endpoint count, independent of paging properties * Resolver component w/ sample data (#53619) Resolver is a map. It shows processes that ran on a computer. The processes are drawn as nodes and lines connect processes with their parents. Resolver is not yet implemented in Kibana. This PR adds a 'map' type UX. The user can click and drag to pan the map and zoom using trackpad pinching (or ctrl and mousewheel.) There is no code providing actual data. Sample data is included. The sample data is used to draw a map. The fundamental info needed is: process names the parent of a process With this info we can topologically lay out the processes. The sample data isn't yet in a realistic format. We'll be fixing that soon. Related issue: elastic/endpoint-app-team#30 * Resolver test plugin not using mount context. (#54933) Mount context was deprecated. Use core.getStartServices() instead. * Resolver nonlinear zoom (#54936) * [Endpoint] add Redux saga Middleware and app Store (#53906) * Added saga library * Initialize endpoint app redux store * Resolver is overflow: hidden to prevent obscured elements from showing up (#55076) * [Endpoint] Fix saga to start only after store is created and stopped on app unmount (#55245) - added `stop()`/`start()` methods to the Saga Middleware creator factory - adjust tests based on changes - changed application `renderApp` to stop sagas when react app is unmounted * Resolver zoom, pan, and center controls (#55221) * Resolver zoom, pan, and center controls * add tests, fix north panning * fix type issue * update west and east panning to behave like google maps * [Endpoint] FIX: Increase tests `sleep` default duration back to 100ms (#55492) Revert `sleep()` default duration, in the saga tests, back to 100ms in order to prevent intermittent failures during CI runs. Fixes #55464 Fixes #55465 * [Endpoint] EMT-65: make endpoint data types common, restructure (#54772) [Endpoint] EMT-65: make endpoint data types common, use schema changes * Basic Functionality Alert List (#55800) * sets up initial grid and data type * data feeds in from backend but doesnt update * sample data feeding in correctly * Fix combineReducers issue by importing Redux type from 'redux' package * Add usePageId hook that fires action when user navigates to page * Strict typing for middleware * addresses comments and uses better types * move types to common/types.ts * Move types to endpoint/types.ts, address PR comments blah 2 Co-authored-by: Pedro Jaramillo <[email protected]> * [Endpoint] Add Endpoint Details route (#55746) * Add Endpoint Details route * add Endpoint Details tests * sacrifices to the Type gods * update to latest endpoint schema Co-authored-by: Elastic Machine <[email protected]> * [Endpoint] EMT-67: add kql support for endpoint list (#56328) [Endpoint] EMT-67: add kql support for endpoint list * [Endpoint] ERT-82 ERT-83 ERT-84: Alert list API with pagination (#56538) * ERT-82 ERT-83 ERT-84 (partial): Add Alert List API with pagination * Better type safety for alert list API * Add Test to Verify Endpoint App Landing Page (#57129) Conflicts: x-pack/test/functional/page_objects/index.ts * fixes render bug in alert list (#57152) Co-authored-by: Elastic Machine <[email protected]> * Resolver: Animate camera, add sidebar (#55590) This PR adds a sidebar navigation. clicking the icons in the nav will focus the camera on the different nodes. There is an animation effect when the camera moves. Conflicts: yarn.lock * [Endpoint] Task/basic endpoint list (#55623) * Adds host management list to endpoint security plugin Co-authored-by: Elastic Machine <[email protected]> * [Endpoint] Policy List UI route and initial view (#56918) * Initial Policy List view * Add `endpoint/policy` route and displays Policy List * test cases (both unit and functional) Does not yet interact with API (Ingest). * Add ApplicationService app status management (#50223) This was already backported, but changes to endpoint app could not be backported, since endpoint app itself hadn't been backported. Now that the endpoint app is backported, reapply the endpoint specific changes from the original commit. * Implements `getStartServices` on server-side (#55156) This was already backported, but changes to endpoint app could not be backported, since endpoint app itself hadn't been backported. Now that the endpoint app is backported, reapply the endpoint specific changes from the original commit. * [ui/utils/query_string]: Remove unused methods & migrate apps to querystring lib (#56957) This was already backported, but changes to endpoint app could not be backported, since endpoint app itself hadn't been backported. Now that the endpoint app is backported, reapply the endpoint specific changes from the original commit. Co-authored-by: Kevin Logan <[email protected]> Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Larry Gregory <[email protected]> Co-authored-by: nnamdifrankie <[email protected]> Co-authored-by: Davis Plumlee <[email protected]> Co-authored-by: Paul Tavares <[email protected]> Co-authored-by: Pedro Jaramillo <[email protected]> Co-authored-by: Dan Panzarella <[email protected]> Co-authored-by: Madison Caldwell <[email protected]> Co-authored-by: Charlie Pichette <[email protected]> Co-authored-by: Candace Park <[email protected]> Co-authored-by: Pierre Gayvallet <[email protected]> Co-authored-by: Alexey Antonov <[email protected]>

pgayvallet added 4 commits November 11, 2019 12:08

add unimplemented registerAppStatusUpdater & remove observers for app…

e87d550

…s and legacyApps

adapt NavLinksService to use new application observables

041cef1

merge availableApps$ and availableLegacyApps$

76ee873

updating core docs

f278903

This comment has been minimized.

Sign in to view

pgayvallet added 3 commits November 12, 2019 09:38

adapt the navLink updating methods

5dba8fc

filters the inaccessible apps from availableApps$

865eec5

restrict access to navigateToApp depending on app status

09cd6d8

pgayvallet mentioned this pull request Nov 12, 2019

Integrate Licensing & Feature Controls with ApplicationService #45291

Closed

pgayvallet commented Nov 12, 2019

View reviewed changes

pgayvallet requested review from mshustov and joshdover November 12, 2019 10:09

pgayvallet added Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Nov 12, 2019

mshustov reviewed Nov 12, 2019

View reviewed changes

src/core/public/application/application_service.tsx Outdated Show resolved Hide resolved

pgayvallet added 6 commits November 19, 2019 11:52

Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…

0bde308

…tion-filter

fixes due to merge

ccd6344

add statusUpdater$ to AppBase

dd8c438

export new types

038ae4e

disable navlink depending on app status

4aa0e2c

update generated doc

162ec01

pgayvallet added v7.6.0 v8.0.0 labels Nov 19, 2019

update snapshots for disabled prop

1c76e68

pgayvallet added 2 commits January 10, 2020 11:28

update generated docs and migration guide

12f469d

fix wrong type cast on AppsMenuProvider.readLinks

f488946

pgayvallet requested review from a team as code owners January 10, 2020 10:50

walterra requested a review from pheyos January 10, 2020 11:03

Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…

815617b

…tion-filter

pheyos approved these changes Jan 10, 2020

View reviewed changes

mshustov reviewed Jan 10, 2020

View reviewed changes

dgieselaar approved these changes Jan 10, 2020

View reviewed changes

kindsun approved these changes Jan 10, 2020

View reviewed changes

pgayvallet added 2 commits January 12, 2020 17:21

Merge remote-tracking branch 'upstream/master' into kbn-45291-applica…

cd6a2d7

…tion-filter

properly type return of navigateToApp

b66df7e

pgayvallet merged commit aeebedf into elastic:master Jan 12, 2020

pgayvallet mentioned this pull request Jan 12, 2020

[7.x] Add ApplicationService app status management (#50223) #54545

Merged

pgayvallet mentioned this pull request Jan 13, 2020

[Discuss] Functional tests for NP client side #52768

Closed

7 tasks

joshdover mentioned this pull request Jan 16, 2020

[Watcher] Move out of legacy #54752

Merged

5 tasks

joshdover mentioned this pull request Jan 31, 2020

Advanced settings component registry ⇒ kibana platform plugin #55940

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ApplicationService app status management #50223

Add ApplicationService app status management #50223

pgayvallet commented Nov 11, 2019 •

edited

Loading

This comment has been minimized.

pgayvallet Nov 12, 2019

mshustov Nov 12, 2019

pgayvallet Nov 12, 2019

mshustov Nov 12, 2019 •

edited

Loading

joshdover Dec 3, 2019

mshustov Dec 4, 2019

pgayvallet Jan 7, 2020

pgayvallet Nov 12, 2019

pgayvallet Nov 12, 2019

pgayvallet Nov 12, 2019

pgayvallet Nov 12, 2019

elasticmachine commented Nov 12, 2019

elasticmachine commented Nov 12, 2019

elasticmachine commented Nov 19, 2019

elasticmachine commented Nov 19, 2019

pgayvallet commented Jan 10, 2020 •

edited

Loading

pheyos left a comment

mshustov Jan 10, 2020

pgayvallet Jan 12, 2020 •

edited

Loading

dgieselaar left a comment

kindsun left a comment

kibanamachine commented Jan 12, 2020

		export const AppRouter: React.StatelessComponent<Props> = ({
		export const AppRouter: React.FunctionComponent<Props> = ({

Add ApplicationService app status management #50223

Add ApplicationService app status management #50223

Conversation

pgayvallet commented Nov 11, 2019 • edited Loading

Summary

Checklist

For maintainers

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mshustov Nov 12, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Nov 12, 2019

elasticmachine commented Nov 12, 2019

💚 Build Succeeded

elasticmachine commented Nov 19, 2019

💔 Build Failed

elasticmachine commented Nov 19, 2019

💔 Build Failed

pgayvallet commented Jan 10, 2020 • edited Loading

pheyos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgayvallet Jan 12, 2020 • edited Loading

Choose a reason for hiding this comment

dgieselaar left a comment

Choose a reason for hiding this comment

kindsun left a comment

Choose a reason for hiding this comment

kibanamachine commented Jan 12, 2020

💚 Build Succeeded

History

pgayvallet commented Nov 11, 2019 •

edited

Loading

mshustov Nov 12, 2019 •

edited

Loading

pgayvallet commented Jan 10, 2020 •

edited

Loading

pgayvallet Jan 12, 2020 •

edited

Loading