Skip to content

Commit 5d7e871

Browse files
pgayvalletpgayvallet
committed
GS providers improvements (#75174)
* exclude apps with non visible navlinks from results * change SO provider to prefix search * fix service tests
1 parent 855b06c commit 5d7e871

8 files changed

Lines changed: 84 additions & 21 deletions

File tree

‎src/core/public/application/application_service.test.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -117,15 +117,15 @@ describe('#setup()', () => {
117117
expect.objectContaining({
118118
id: 'app1',
119119
legacy: false,
120-
navLinkStatus: AppNavLinkStatus.default,
120+
navLinkStatus: AppNavLinkStatus.visible,
121121
status: AppStatus.accessible,
122122
})
123123
);
124124
expect(applications.get('app2')).toEqual(
125125
expect.objectContaining({
126126
id: 'app2',
127127
legacy: false,
128-
navLinkStatus: AppNavLinkStatus.default,
128+
navLinkStatus: AppNavLinkStatus.visible,
129129
status: AppStatus.accessible,
130130
})
131131
);
@@ -142,7 +142,7 @@ describe('#setup()', () => {
142142
expect.objectContaining({
143143
id: 'app1',
144144
legacy: false,
145-
navLinkStatus: AppNavLinkStatus.default,
145+
navLinkStatus: AppNavLinkStatus.hidden,
146146
status: AppStatus.inaccessible,
147147
defaultPath: 'foo/bar',
148148
tooltip: 'App inaccessible due to reason',
@@ -152,7 +152,7 @@ describe('#setup()', () => {
152152
expect.objectContaining({
153153
id: 'app2',
154154
legacy: false,
155-
navLinkStatus: AppNavLinkStatus.default,
155+
navLinkStatus: AppNavLinkStatus.visible,
156156
status: AppStatus.accessible,
157157
})
158158
);
@@ -268,7 +268,7 @@ describe('#setup()', () => {
268268
expect.objectContaining({
269269
id: 'app2',
270270
legacy: false,
271-
navLinkStatus: AppNavLinkStatus.default,
271+
navLinkStatus: AppNavLinkStatus.visible,
272272
status: AppStatus.accessible,
273273
tooltip: 'App accessible',
274274
})
@@ -523,7 +523,7 @@ describe('#start()', () => {
523523
appRoute: '/app/app1',
524524
id: 'app1',
525525
legacy: false,
526-
navLinkStatus: AppNavLinkStatus.default,
526+
navLinkStatus: AppNavLinkStatus.visible,
527527
status: AppStatus.accessible,
528528
})
529529
);
@@ -532,7 +532,7 @@ describe('#start()', () => {
532532
appUrl: '/my-url',
533533
id: 'app2',
534534
legacy: true,
535-
navLinkStatus: AppNavLinkStatus.default,
535+
navLinkStatus: AppNavLinkStatus.visible,
536536
status: AppStatus.accessible,
537537
})
538538
);

‎src/core/public/application/utils.test.ts‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,15 @@
1818
*/
1919

2020
import { of } from 'rxjs';
21-
import { LegacyApp, App, AppStatus, AppNavLinkStatus } from './types';
21+
import { App, AppNavLinkStatus, AppStatus, LegacyApp } from './types';
2222
import { BasePath } from '../http/base_path';
2323
import {
24-
removeSlashes,
2524
appendAppPath,
25+
getAppInfo,
2626
isLegacyApp,
27-
relativeToAbsolute,
2827
parseAppUrl,
29-
getAppInfo,
28+
relativeToAbsolute,
29+
removeSlashes,
3030
} from './utils';
3131

3232
describe('removeSlashes', () => {
@@ -494,7 +494,7 @@ describe('getAppInfo', () => {
494494
id: 'some-id',
495495
title: 'some-title',
496496
status: AppStatus.accessible,
497-
navLinkStatus: AppNavLinkStatus.default,
497+
navLinkStatus: AppNavLinkStatus.visible,
498498
appRoute: `/app/some-id`,
499499
legacy: false,
500500
});
@@ -509,8 +509,35 @@ describe('getAppInfo', () => {
509509
id: 'some-id',
510510
title: 'some-title',
511511
status: AppStatus.accessible,
512-
navLinkStatus: AppNavLinkStatus.default,
512+
navLinkStatus: AppNavLinkStatus.visible,
513513
legacy: true,
514514
});
515515
});
516+
517+
it('computes the navLinkStatus depending on the app status', () => {
518+
expect(
519+
getAppInfo(
520+
createApp({
521+
navLinkStatus: AppNavLinkStatus.default,
522+
status: AppStatus.inaccessible,
523+
})
524+
)
525+
).toEqual(
526+
expect.objectContaining({
527+
navLinkStatus: AppNavLinkStatus.hidden,
528+
})
529+
);
530+
expect(
531+
getAppInfo(
532+
createApp({
533+
navLinkStatus: AppNavLinkStatus.default,
534+
status: AppStatus.accessible,
535+
})
536+
)
537+
).toEqual(
538+
expect.objectContaining({
539+
navLinkStatus: AppNavLinkStatus.visible,
540+
})
541+
);
542+
});
516543
});

‎src/core/public/application/utils.ts‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,15 @@
1818
*/
1919

2020
import { IBasePath } from '../http';
21-
import { App, LegacyApp, PublicAppInfo, PublicLegacyAppInfo, ParsedAppUrl } from './types';
21+
import {
22+
App,
23+
AppNavLinkStatus,
24+
AppStatus,
25+
LegacyApp,
26+
ParsedAppUrl,
27+
PublicAppInfo,
28+
PublicLegacyAppInfo,
29+
} from './types';
2230

2331
/**
2432
* Utility to remove trailing, leading or duplicate slashes.
@@ -116,20 +124,26 @@ const removeBasePath = (url: string, basePath: IBasePath, origin: string): strin
116124
};
117125

118126
export function getAppInfo(app: App<unknown> | LegacyApp): PublicAppInfo | PublicLegacyAppInfo {
127+
const navLinkStatus =
128+
app.navLinkStatus === AppNavLinkStatus.default
129+
? app.status === AppStatus.inaccessible
130+
? AppNavLinkStatus.hidden
131+
: AppNavLinkStatus.visible
132+
: app.navLinkStatus!;
119133
if (isLegacyApp(app)) {
120134
const { updater$, ...infos } = app;
121135
return {
122136
...infos,
123137
status: app.status!,
124-
navLinkStatus: app.navLinkStatus!,
138+
navLinkStatus,
125139
legacy: true,
126140
};
127141
} else {
128142
const { updater$, mount, ...infos } = app;
129143
return {
130144
...infos,
131145
status: app.status!,
132-
navLinkStatus: app.navLinkStatus!,
146+
navLinkStatus,
133147
appRoute: app.appRoute!,
134148
legacy: false,
135149
};

‎x-pack/plugins/global_search_providers/public/providers/application.test.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import { getAppResultsMock } from './application.test.mocks';
88

9-
import { of, EMPTY } from 'rxjs';
9+
import { EMPTY, of } from 'rxjs';
1010
import { TestScheduler } from 'rxjs/testing';
1111
import { ApplicationStart, AppNavLinkStatus, AppStatus, PublicAppInfo } from 'src/core/public';
1212
import {
@@ -100,6 +100,20 @@ describe('applicationResultProvider', () => {
100100
expect(getAppResultsMock).toHaveBeenCalledWith('term', [expectApp('app1')]);
101101
});
102102

103+
it('ignores apps with non-visible navlink', async () => {
104+
application.applications$ = of(
105+
createAppMap([
106+
createApp({ id: 'app1', title: 'App 1', navLinkStatus: AppNavLinkStatus.visible }),
107+
createApp({ id: 'disabled', title: 'disabled', navLinkStatus: AppNavLinkStatus.disabled }),
108+
createApp({ id: 'hidden', title: 'hidden', navLinkStatus: AppNavLinkStatus.hidden }),
109+
])
110+
);
111+
const provider = createApplicationResultProvider(Promise.resolve(application));
112+
await provider.find('term', defaultOption).toPromise();
113+
114+
expect(getAppResultsMock).toHaveBeenCalledWith('term', [expectApp('app1')]);
115+
});
116+
103117
it('ignores chromeless apps', async () => {
104118
application.applications$ = of(
105119
createAppMap([

‎x-pack/plugins/global_search_providers/public/providers/application.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ export const createApplicationResultProvider = (
1717
mergeMap((application) => application.applications$),
1818
map((apps) =>
1919
[...apps.values()].filter(
20-
(app) => app.status === 0 && (app.legacy === true || app.chromeless !== true)
20+
// only include non-chromeless enabled apps with visible navLinks
21+
(app) => app.status === 0 && app.navLinkStatus === 1 && app.chromeless !== true
2122
)
2223
),
2324
shareReplay(1)

‎x-pack/plugins/global_search_providers/server/providers/saved_objects/provider.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ describe('savedObjectsResultProvider', () => {
112112
expect(context.core.savedObjects.client.find).toHaveBeenCalledWith({
113113
page: 1,
114114
perPage: defaultOption.maxResults,
115-
search: 'term',
115+
search: 'term*',
116116
preference: 'pref',
117117
searchFields: ['title', 'description'],
118118
type: ['typeA', 'typeB'],

‎x-pack/plugins/global_search_providers/server/providers/saved_objects/provider.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export const createSavedObjectsResultProvider = (): GlobalSearchResultProvider =
2525
const responsePromise = client.find({
2626
page: 1,
2727
perPage: maxResults,
28-
search: term,
28+
search: term ? `${term}*` : undefined,
2929
preference,
3030
searchFields,
3131
type: searchableTypes.map((type) => type.name),

‎x-pack/test/plugin_functional/test_suites/global_search/global_search_providers.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
4040
expect(results.length).to.be(1);
4141
expect(results[0].type).to.be('index-pattern');
4242
expect(results[0].title).to.be('logstash-*');
43-
expect(results[0].score).to.be.greaterThan(1);
43+
expect(results[0].score).to.be.greaterThan(0.9);
4444
});
4545

4646
it('can search for visualizations', async () => {
@@ -70,5 +70,12 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
7070
expect(results.map((r) => r.title)).to.contain('dashboard with map');
7171
expect(results.map((r) => r.title)).to.contain('Amazing Dashboard');
7272
});
73+
74+
it('can search by prefix', async () => {
75+
const results = await findResultsWithAPI('Amaz');
76+
expect(results.length).to.be(1);
77+
expect(results[0].type).to.be('dashboard');
78+
expect(results[0].title).to.be('Amazing Dashboard');
79+
});
7380
});
7481
}

0 commit comments

Comments
 (0)