GS providers improvements (#75174) (#75272)

* exclude apps with non visible navlinks from results * change SO provider to prefix search * fix service tests
elastic · Aug 18, 2020 · ea08e93 · ea08e93
1 parent 01b8afe
commit ea08e93
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');
+ });
  });
 }