GS providers improvements (elastic#75174)

* exclude apps with non visible navlinks from results * change SO provider to prefix search * fix service tests
pgayvallet · Aug 18, 2020 · 5d7e871 · 5d7e871
1 parent 855b06c
commit 5d7e871
Show file tree

Hide file tree

Showing 8 changed files with 84 additions and 21 deletions.
diff --git a/src/core/public/application/application_service.test.ts b/src/core/public/application/application_service.test.ts
@@ -117,15 +117,15 @@ describe('#setup()', () => {
         expect.objectContaining({
           id: 'app1',
           legacy: false,
-          navLinkStatus: AppNavLinkStatus.default,
+          navLinkStatus: AppNavLinkStatus.visible,
           status: AppStatus.accessible,
         })
       );
       expect(applications.get('app2')).toEqual(
         expect.objectContaining({
           id: 'app2',
           legacy: false,
-          navLinkStatus: AppNavLinkStatus.default,
+          navLinkStatus: AppNavLinkStatus.visible,
           status: AppStatus.accessible,
         })
       );
@@ -142,7 +142,7 @@ describe('#setup()', () => {
         expect.objectContaining({
           id: 'app1',
           legacy: false,
-          navLinkStatus: AppNavLinkStatus.default,
+          navLinkStatus: AppNavLinkStatus.hidden,
           status: AppStatus.inaccessible,
           defaultPath: 'foo/bar',
           tooltip: 'App inaccessible due to reason',
@@ -152,7 +152,7 @@ describe('#setup()', () => {
         expect.objectContaining({
           id: 'app2',
           legacy: false,
-          navLinkStatus: AppNavLinkStatus.default,
+          navLinkStatus: AppNavLinkStatus.visible,
           status: AppStatus.accessible,
         })
       );
@@ -268,7 +268,7 @@ describe('#setup()', () => {
         expect.objectContaining({
           id: 'app2',
           legacy: false,
-          navLinkStatus: AppNavLinkStatus.default,
+          navLinkStatus: AppNavLinkStatus.visible,
           status: AppStatus.accessible,
           tooltip: 'App accessible',
         })
@@ -523,7 +523,7 @@ describe('#start()', () => {
         appRoute: '/app/app1',
         id: 'app1',
         legacy: false,
-        navLinkStatus: AppNavLinkStatus.default,
+        navLinkStatus: AppNavLinkStatus.visible,
         status: AppStatus.accessible,
       })
     );
@@ -532,7 +532,7 @@ describe('#start()', () => {
         appUrl: '/my-url',
         id: 'app2',
         legacy: true,
-        navLinkStatus: AppNavLinkStatus.default,
+        navLinkStatus: AppNavLinkStatus.visible,
         status: AppStatus.accessible,
       })
     );

diff --git a/src/core/public/application/utils.test.ts b/src/core/public/application/utils.test.ts
@@ -18,15 +18,15 @@
  */
 
 import { of } from 'rxjs';
-import { LegacyApp, App, AppStatus, AppNavLinkStatus } from './types';
+import { App, AppNavLinkStatus, AppStatus, LegacyApp } from './types';
 import { BasePath } from '../http/base_path';
 import {
-  removeSlashes,
   appendAppPath,
+  getAppInfo,
   isLegacyApp,
-  relativeToAbsolute,
   parseAppUrl,
-  getAppInfo,
+  relativeToAbsolute,
+  removeSlashes,
 } from './utils';
 
 describe('removeSlashes', () => {
@@ -494,7 +494,7 @@ describe('getAppInfo', () => {
       id: 'some-id',
       title: 'some-title',
       status: AppStatus.accessible,
-      navLinkStatus: AppNavLinkStatus.default,
+      navLinkStatus: AppNavLinkStatus.visible,
       appRoute: `/app/some-id`,
       legacy: false,
     });
@@ -509,8 +509,35 @@ describe('getAppInfo', () => {
       id: 'some-id',
       title: 'some-title',
       status: AppStatus.accessible,
-      navLinkStatus: AppNavLinkStatus.default,
+      navLinkStatus: AppNavLinkStatus.visible,
       legacy: true,
     });
   });
+
+  it('computes the navLinkStatus depending on the app status', () => {
+    expect(
+      getAppInfo(
+        createApp({
+          navLinkStatus: AppNavLinkStatus.default,
+          status: AppStatus.inaccessible,
+        })
+      )
+    ).toEqual(
+      expect.objectContaining({
+        navLinkStatus: AppNavLinkStatus.hidden,
+      })
+    );
+    expect(
+      getAppInfo(
+        createApp({
+          navLinkStatus: AppNavLinkStatus.default,
+          status: AppStatus.accessible,
+        })
+      )
+    ).toEqual(
+      expect.objectContaining({
+        navLinkStatus: AppNavLinkStatus.visible,
+      })
+    );
+  });
 });
diff --git a/src/core/public/application/utils.ts b/src/core/public/application/utils.ts
@@ -18,7 +18,15 @@
  */
 
 import { IBasePath } from '../http';
-import { App, LegacyApp, PublicAppInfo, PublicLegacyAppInfo, ParsedAppUrl } from './types';
+import {
+  App,
+  AppNavLinkStatus,
+  AppStatus,
+  LegacyApp,
+  ParsedAppUrl,
+  PublicAppInfo,
+  PublicLegacyAppInfo,
+} from './types';
 
 /**
  * Utility to remove trailing, leading or duplicate slashes.
@@ -116,20 +124,26 @@ const removeBasePath = (url: string, basePath: IBasePath, origin: string): strin
 };
 
 export function getAppInfo(app: App<unknown> | LegacyApp): PublicAppInfo | PublicLegacyAppInfo {
+  const navLinkStatus =
+    app.navLinkStatus === AppNavLinkStatus.default
+      ? app.status === AppStatus.inaccessible
+        ? AppNavLinkStatus.hidden
+        : AppNavLinkStatus.visible
+      : app.navLinkStatus!;
   if (isLegacyApp(app)) {
     const { updater$, ...infos } = app;
     return {
       ...infos,
       status: app.status!,
-      navLinkStatus: app.navLinkStatus!,
+      navLinkStatus,
       legacy: true,
     };
   } else {
     const { updater$, mount, ...infos } = app;
     return {
       ...infos,
       status: app.status!,
-      navLinkStatus: app.navLinkStatus!,
+      navLinkStatus,
       appRoute: app.appRoute!,
       legacy: false,
     };

diff --git a/x-pack/plugins/global_search_providers/public/providers/application.test.ts b/x-pack/plugins/global_search_providers/public/providers/application.test.ts
@@ -6,7 +6,7 @@
 
 import { getAppResultsMock } from './application.test.mocks';
 
-import { of, EMPTY } from 'rxjs';
+import { EMPTY, of } from 'rxjs';
 import { TestScheduler } from 'rxjs/testing';
 import { ApplicationStart, AppNavLinkStatus, AppStatus, PublicAppInfo } from 'src/core/public';
 import {
@@ -100,6 +100,20 @@ describe('applicationResultProvider', () => {
     expect(getAppResultsMock).toHaveBeenCalledWith('term', [expectApp('app1')]);
   });
 
+  it('ignores apps with non-visible navlink', async () => {
+    application.applications$ = of(
+      createAppMap([
+        createApp({ id: 'app1', title: 'App 1', navLinkStatus: AppNavLinkStatus.visible }),
+        createApp({ id: 'disabled', title: 'disabled', navLinkStatus: AppNavLinkStatus.disabled }),
+        createApp({ id: 'hidden', title: 'hidden', navLinkStatus: AppNavLinkStatus.hidden }),
+      ])
+    );
+    const provider = createApplicationResultProvider(Promise.resolve(application));
+    await provider.find('term', defaultOption).toPromise();
+
+    expect(getAppResultsMock).toHaveBeenCalledWith('term', [expectApp('app1')]);
+  });
+
   it('ignores chromeless apps', async () => {
     application.applications$ = of(
       createAppMap([

diff --git a/x-pack/plugins/global_search_providers/public/providers/application.ts b/x-pack/plugins/global_search_providers/public/providers/application.ts
@@ -17,7 +17,8 @@ export const createApplicationResultProvider = (
     mergeMap((application) => application.applications$),
     map((apps) =>
       [...apps.values()].filter(
-        (app) => app.status === 0 && (app.legacy === true || app.chromeless !== true)
+        // only include non-chromeless enabled apps with visible navLinks
+        (app) => app.status === 0 && app.navLinkStatus === 1 && app.chromeless !== true
       )
     ),
     shareReplay(1)

diff --git a/x-pack/plugins/global_search_providers/server/providers/saved_objects/provider.test.ts b/x-pack/plugins/global_search_providers/server/providers/saved_objects/provider.test.ts
@@ -112,7 +112,7 @@ describe('savedObjectsResultProvider', () => {
     expect(context.core.savedObjects.client.find).toHaveBeenCalledWith({
       page: 1,
       perPage: defaultOption.maxResults,
-      search: 'term',
+      search: 'term*',
       preference: 'pref',
       searchFields: ['title', 'description'],
       type: ['typeA', 'typeB'],

diff --git a/x-pack/plugins/global_search_providers/server/providers/saved_objects/provider.ts b/x-pack/plugins/global_search_providers/server/providers/saved_objects/provider.ts
@@ -25,7 +25,7 @@ export const createSavedObjectsResultProvider = (): GlobalSearchResultProvider =
       const responsePromise = client.find({
         page: 1,
         perPage: maxResults,
-        search: term,
+        search: term ? `${term}*` : undefined,
         preference,
         searchFields,
         type: searchableTypes.map((type) => type.name),

diff --git a/x-pack/test/plugin_functional/test_suites/global_search/global_search_providers.ts b/x-pack/test/plugin_functional/test_suites/global_search/global_search_providers.ts
@@ -40,7 +40,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
       expect(results.length).to.be(1);
       expect(results[0].type).to.be('index-pattern');
       expect(results[0].title).to.be('logstash-*');
-      expect(results[0].score).to.be.greaterThan(1);
+      expect(results[0].score).to.be.greaterThan(0.9);
     });
 
     it('can search for visualizations', async () => {
@@ -70,5 +70,12 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
       expect(results.map((r) => r.title)).to.contain('dashboard with map');
       expect(results.map((r) => r.title)).to.contain('Amazing Dashboard');
     });
+
+    it('can search by prefix', async () => {
+      const results = await findResultsWithAPI('Amaz');
+      expect(results.length).to.be(1);
+      expect(results[0].type).to.be('dashboard');
+      expect(results[0].title).to.be('Amazing Dashboard');
+    });
   });
 }