diff --git a/src/core/public/core_system.test.ts b/src/core/public/core_system.test.ts index e5c5319fb85e..b6ff1407eed3 100644 --- a/src/core/public/core_system.test.ts +++ b/src/core/public/core_system.test.ts @@ -17,11 +17,14 @@ * under the License. */ +// tslint:disable no-unused-expression + import { BasePathService } from './base_path'; import { FatalErrorsService } from './fatal_errors'; import { InjectedMetadataService } from './injected_metadata'; import { LegacyPlatformService } from './legacy_platform'; import { LoadingCountService } from './loading_count'; +import { NavLinksService } from './nav_links'; import { NotificationsService } from './notifications'; import { UiSettingsService } from './ui_settings'; @@ -97,6 +100,15 @@ jest.mock('./ui_settings', () => ({ UiSettingsService: MockUiSettingsService, })); +const mockNavLinksContract = {}; +const MockNavLinksService = jest.fn(function(this: any) { + this.start = jest.fn().mockReturnValue(mockNavLinksContract); + this.stop = jest.fn(); +}); +jest.mock('./nav_links', () => ({ + NavLinksService: MockNavLinksService, +})); + import { CoreSystem } from './core_system'; jest.spyOn(CoreSystem.prototype, 'stop'); @@ -112,7 +124,6 @@ beforeEach(() => { describe('constructor', () => { it('creates instances of services', () => { - // tslint:disable no-unused-expression new CoreSystem({ ...defaultCoreSystemParams, }); @@ -124,12 +135,12 @@ describe('constructor', () => { expect(MockLoadingCountService).toHaveBeenCalledTimes(1); expect(MockBasePathService).toHaveBeenCalledTimes(1); expect(MockUiSettingsService).toHaveBeenCalledTimes(1); + expect(MockNavLinksService).toHaveBeenCalledTimes(1); }); it('passes injectedMetadata param to InjectedMetadataService', () => { const injectedMetadata = { injectedMetadata: true } as any; - // tslint:disable no-unused-expression new CoreSystem({ ...defaultCoreSystemParams, injectedMetadata, @@ -145,7 +156,6 @@ describe('constructor', () => { const requireLegacyFiles = { requireLegacyFiles: true } as any; const useLegacyTestHarness = { useLegacyTestHarness: true } as any; - // tslint:disable no-unused-expression new CoreSystem({ ...defaultCoreSystemParams, requireLegacyFiles, @@ -161,7 +171,6 @@ describe('constructor', () => { }); it('passes a dom element to NotificationsService', () => { - // tslint:disable no-unused-expression new CoreSystem({ ...defaultCoreSystemParams, }); @@ -231,6 +240,17 @@ describe('#stop', () => { expect(loadingCountService.stop).toHaveBeenCalled(); }); + it('calls navLinks.stop()', () => { + const coreSystem = new CoreSystem({ + ...defaultCoreSystemParams, + }); + + const [navLinksService] = MockNavLinksService.mock.instances; + expect(navLinksService.stop).not.toHaveBeenCalled(); + coreSystem.stop(); + expect(navLinksService.stop).toHaveBeenCalled(); + }); + it('clears the rootDomElement', () => { const rootDomElement = document.createElement('div'); const coreSystem = new CoreSystem({ @@ -312,6 +332,16 @@ describe('#start()', () => { expect(mockInstance.start).toHaveBeenCalledTimes(1); expect(mockInstance.start).toHaveBeenCalledWith(); }); + + it('calls navLinks#start()', () => { + startCore(); + const [mockInstance] = MockNavLinksService.mock.instances; + expect(mockInstance.start).toHaveBeenCalledTimes(1); + expect(mockInstance.start).toHaveBeenCalledWith({ + injectedMetadata: mockInjectedMetadataStartContract, + basePath: mockBasePathStartContract, + }); + }); }); describe('LegacyPlatform targetDomElement', () => { diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts index 5d39a883e39f..097434570f91 100644 --- a/src/core/public/core_system.ts +++ b/src/core/public/core_system.ts @@ -24,6 +24,7 @@ import { FatalErrorsService } from './fatal_errors'; import { InjectedMetadataParams, InjectedMetadataService } from './injected_metadata'; import { LegacyPlatformParams, LegacyPlatformService } from './legacy_platform'; import { LoadingCountService } from './loading_count'; +import { NavLinksService } from './nav_links'; import { NotificationsService } from './notifications'; import { UiSettingsService } from './ui_settings'; @@ -48,6 +49,7 @@ export class CoreSystem { private readonly loadingCount: LoadingCountService; private readonly uiSettings: UiSettingsService; private readonly basePath: BasePathService; + private readonly navLinks: NavLinksService; private readonly rootDomElement: HTMLElement; private readonly notificationsTargetDomElement: HTMLDivElement; @@ -78,6 +80,7 @@ export class CoreSystem { this.loadingCount = new LoadingCountService(); this.basePath = new BasePathService(); this.uiSettings = new UiSettingsService(); + this.navLinks = new NavLinksService(); this.legacyPlatformTargetDomElement = document.createElement('div'); this.legacyPlatform = new LegacyPlatformService({ @@ -100,6 +103,7 @@ export class CoreSystem { const fatalErrors = this.fatalErrors.start(); const loadingCount = this.loadingCount.start({ fatalErrors }); const basePath = this.basePath.start({ injectedMetadata }); + const navLinks = this.navLinks.start({ injectedMetadata, basePath }); const uiSettings = this.uiSettings.start({ notifications, loadingCount, @@ -112,6 +116,7 @@ export class CoreSystem { notifications, loadingCount, basePath, + navLinks, uiSettings, }); } catch (error) { @@ -123,6 +128,7 @@ export class CoreSystem { this.legacyPlatform.stop(); this.notifications.stop(); this.loadingCount.stop(); + this.navLinks.stop(); this.rootDomElement.textContent = ''; } } diff --git a/src/core/public/injected_metadata/injected_metadata_service.test.ts b/src/core/public/injected_metadata/injected_metadata_service.test.ts index d520ba2b356f..0a7e7e950e8a 100644 --- a/src/core/public/injected_metadata/injected_metadata_service.test.ts +++ b/src/core/public/injected_metadata/injected_metadata_service.test.ts @@ -150,3 +150,30 @@ describe('start.getInjectedVars()', () => { ); }); }); + +describe('start.getNavLinks()', () => { + it('returns the navLinks from the injected vars, readonly', () => { + const start = new InjectedMetadataService({ + injectedMetadata: { + navLinks: [{ id: 1 }, { id: 2 }], + }, + } as any).start(); + + const nav: any = start.getNavLinks(); + expect(nav).toMatchInlineSnapshot(` +Array [ + Object { + "id": 1, + }, + Object { + "id": 2, + }, +] +`); + expect(() => { + nav[0].id = 2; + }).toThrowErrorMatchingInlineSnapshot( + `"Cannot assign to read only property 'id' of object '#'"` + ); + }); +}); diff --git a/src/core/public/injected_metadata/injected_metadata_service.ts b/src/core/public/injected_metadata/injected_metadata_service.ts index ed50067db9a0..e99729790378 100644 --- a/src/core/public/injected_metadata/injected_metadata_service.ts +++ b/src/core/public/injected_metadata/injected_metadata_service.ts @@ -26,6 +26,19 @@ export interface InjectedMetadataParams { version: string; buildNumber: number; basePath: string; + navLinks: Array<{ + id: string; + title: string; + order: number; + url: string; + subUrlBase: string; + linkToLastSubUrl: boolean; + icon?: string; + description?: string; + disabled?: boolean; + tooltip?: string; + hidden?: boolean; + }>; vars: { [key: string]: unknown; }; @@ -33,7 +46,6 @@ export interface InjectedMetadataParams { app: unknown; translations: unknown; bundleId: string; - nav: unknown; version: string; branch: string; buildNum: number; @@ -81,6 +93,15 @@ export class InjectedMetadataService { getInjectedVars: () => { return this.state.vars; }, + + /** + * Returns the navLinks that were included in the injectedMetadata. Use navLinks.get$() to + * access the complete list of navLinks that reflect the correct application state + * @internal + */ + getNavLinks: () => { + return this.state.navLinks; + }, }; } diff --git a/src/core/public/legacy_platform/__snapshots__/legacy_platform_service.test.ts.snap b/src/core/public/legacy_platform/__snapshots__/legacy_platform_service.test.ts.snap index 9e9d34ebfe02..89a2f94b9348 100644 --- a/src/core/public/legacy_platform/__snapshots__/legacy_platform_service.test.ts.snap +++ b/src/core/public/legacy_platform/__snapshots__/legacy_platform_service.test.ts.snap @@ -9,6 +9,7 @@ Array [ "ui/chrome/api/base_path", "ui/chrome/api/ui_settings", "ui/chrome/api/injected_vars", + "ui/chrome/api/nav_links", "ui/chrome", "legacy files", ] @@ -23,6 +24,7 @@ Array [ "ui/chrome/api/base_path", "ui/chrome/api/ui_settings", "ui/chrome/api/injected_vars", + "ui/chrome/api/nav_links", "ui/test_harness", "legacy files", ] diff --git a/src/core/public/legacy_platform/legacy_platform_service.test.ts b/src/core/public/legacy_platform/legacy_platform_service.test.ts index 913cfc79a6ae..b73603dcabe7 100644 --- a/src/core/public/legacy_platform/legacy_platform_service.test.ts +++ b/src/core/public/legacy_platform/legacy_platform_service.test.ts @@ -94,6 +94,14 @@ jest.mock('ui/chrome/api/injected_vars', () => { }; }); +const mockNavLinksInit = jest.fn(); +jest.mock('ui/chrome/api/nav_links', () => { + mockLoadOrder.push('ui/chrome/api/nav_links'); + return { + __newPlatformInit__: mockNavLinksInit, + }; +}); + import { LegacyPlatformService } from './legacy_platform_service'; const fatalErrorsStartContract = {} as any; @@ -118,6 +126,7 @@ const basePathStartContract = { }; const uiSettingsStartContract: any = {}; +const navLinksStartContract: any = {}; const defaultParams = { targetDomElement: document.createElement('div'), @@ -133,6 +142,7 @@ const defaultStartDeps = { loadingCount: loadingCountStartContract, basePath: basePathStartContract, uiSettings: uiSettingsStartContract, + navLinks: navLinksStartContract, }; afterEach(() => { @@ -224,6 +234,17 @@ describe('#start()', () => { expect(mockInjectedVarsInit).toHaveBeenCalledWith(injectedMetadataStartContract); }); + it('passes navLinks service to ui/chrome/api/nav_links', () => { + const legacyPlatform = new LegacyPlatformService({ + ...defaultParams, + }); + + legacyPlatform.start(defaultStartDeps); + + expect(mockNavLinksInit).toHaveBeenCalledTimes(1); + expect(mockNavLinksInit).toHaveBeenCalledWith(navLinksStartContract); + }); + describe('useLegacyTestHarness = false', () => { it('passes the targetDomElement to ui/chrome', () => { const legacyPlatform = new LegacyPlatformService({ diff --git a/src/core/public/legacy_platform/legacy_platform_service.ts b/src/core/public/legacy_platform/legacy_platform_service.ts index 2b7ae5f2bc89..f9da10c8128b 100644 --- a/src/core/public/legacy_platform/legacy_platform_service.ts +++ b/src/core/public/legacy_platform/legacy_platform_service.ts @@ -22,6 +22,7 @@ import { BasePathStartContract } from '../base_path'; import { FatalErrorsStartContract } from '../fatal_errors'; import { InjectedMetadataStartContract } from '../injected_metadata'; import { LoadingCountStartContract } from '../loading_count'; +import { NavLinksStartContract } from '../nav_links'; import { NotificationsStartContract } from '../notifications'; import { UiSettingsClient } from '../ui_settings'; @@ -32,6 +33,7 @@ interface Deps { loadingCount: LoadingCountStartContract; basePath: BasePathStartContract; uiSettings: UiSettingsClient; + navLinks: NavLinksStartContract; } export interface LegacyPlatformParams { @@ -57,6 +59,7 @@ export class LegacyPlatformService { loadingCount, basePath, uiSettings, + navLinks, }: Deps) { // Inject parts of the new platform into parts of the legacy platform // so that legacy APIs/modules can mimic their new platform counterparts @@ -67,6 +70,7 @@ export class LegacyPlatformService { require('ui/chrome/api/base_path').__newPlatformInit__(basePath); require('ui/chrome/api/ui_settings').__newPlatformInit__(uiSettings); require('ui/chrome/api/injected_vars').__newPlatformInit__(injectedMetadata); + require('ui/chrome/api/nav_links').__newPlatformInit__(navLinks); // Load the bootstrap module before loading the legacy platform files so that // the bootstrap module can modify the environment a bit first diff --git a/src/core/public/nav_links/index.ts b/src/core/public/nav_links/index.ts new file mode 100644 index 000000000000..1b988212e7d4 --- /dev/null +++ b/src/core/public/nav_links/index.ts @@ -0,0 +1,20 @@ +/* + * 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. + */ + +export { NavLinksService, NavLinksStartContract, NavLinks, NavLink } from './nav_links_service'; diff --git a/src/core/public/nav_links/nav_links_service.test.ts b/src/core/public/nav_links/nav_links_service.test.ts new file mode 100644 index 000000000000..6c5a53e00f7d --- /dev/null +++ b/src/core/public/nav_links/nav_links_service.test.ts @@ -0,0 +1,544 @@ +/* + * 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. + */ + +const MockPrefixedStorage = jest.fn(() => new Map()); +jest.mock('./prefixed_storage', () => ({ + PrefixedStorage: MockPrefixedStorage, +})); + +import * as Rx from 'rxjs'; +import { map, take, toArray } from 'rxjs/operators'; + +import { InjectedMetadataParams } from '../injected_metadata'; +import { NavLink, NavLinksService } from './nav_links_service'; + +const FOO_NAV_LINK = { + description: 'foo', + disabled: false, + hidden: true, + icon: 'foo.svg', + id: 'foo', + linkToLastSubUrl: true, + order: 599, + subUrlBase: '/abc#/foo', + title: 'Foo', + tooltip: 'Foo', + url: '/abc#/foo', +}; + +const BAR_NAV_LINK = { + description: 'bar', + disabled: false, + hidden: false, + icon: 'bar.svg', + id: 'bar', + linkToLastSubUrl: true, + order: 599, + subUrlBase: '/abc#/bar', + title: 'Bar', + tooltip: 'Bar', + url: '/abc#/bar', +}; + +const createdServices: NavLinksService[] = []; + +function setup({ + navLinks = [], +}: { + navLinks?: InjectedMetadataParams['injectedMetadata']['navLinks']; +} = {}) { + const injectedMetadata = { + getNavLinks: jest.fn().mockReturnValue(navLinks), + }; + + const service = new NavLinksService(); + const start = service.start({ + injectedMetadata: injectedMetadata as any, + basePath: { + get: () => '/abc', + addToPath: (path: string) => `/abc${path}`, + } as any, + }); + + expect(MockPrefixedStorage.mock.results).toHaveLength(1); + const [{ value: lastUrlStore }] = MockPrefixedStorage.mock.results; + + // create a promise that resolves to an array of arrays containing a summary + // for each navLink that was emitted by the navLinksService until it stopped + function summarize(...keys: Array) { + return start + .get$() + .pipe( + map(nls => + nls.map(navLink => + keys + .map(key => { + if (key === 'active' || key === 'disabled' || key === 'hidden') { + return navLink[key] ? key : '-'; + } + + return navLink[key]; + }) + .join(':') + ) + ), + toArray() + ) + .toPromise(); + } + + createdServices.push(service); + return { service, start, injectedMetadata, summarize, lastUrlStore }; +} + +async function changeHash(newHash: string) { + window.location.hash = newHash; + await Rx.fromEvent(window, 'hashchange') + .pipe(take(1)) + .toPromise(); +} + +beforeEach(() => { + window.history.pushState(undefined, '', '/abc#/'); +}); + +afterEach(() => { + while (createdServices.length) { + createdServices.shift()!.stop(); + } + jest.restoreAllMocks(); + jest.clearAllMocks(); +}); + +describe('start.get$()', () => { + it('returns an observable of navLink objects that completes when the service is stopped', async () => { + const { service, start } = setup(); + const observable = start.get$(); + const promise = observable.toPromise(); + expect(observable).toBeInstanceOf(Rx.Observable); + service.stop(); + await expect(promise).resolves.toEqual([]); + }); + + it('includes the navLinks from the injectedMetadata', async () => { + const { service, start } = setup({ + navLinks: [FOO_NAV_LINK], + }); + + const observable = start.get$(); + const promise = observable.toPromise(); + expect(observable).toBeInstanceOf(Rx.Observable); + service.stop(); + await expect(promise).resolves.toMatchInlineSnapshot(` +Array [ + Object { + "active": false, + "description": "foo", + "disabled": false, + "hidden": true, + "icon": "http://localhost/abc/foo.svg", + "id": "foo", + "lastSubUrl": undefined, + "linkToLastSubUrl": true, + "order": 599, + "subUrlBase": "http://localhost/abc#/foo", + "title": "Foo", + "tooltip": "Foo", + "url": "http://localhost/abc#/foo", + }, +] +`); + }); + + it('updates when the URL changes', async () => { + const { service, summarize } = setup({ navLinks: [FOO_NAV_LINK, BAR_NAV_LINK] }); + + const promise = summarize('id', 'active', 'lastSubUrl'); + + await changeHash('#/bar/baz/box'); + await changeHash('#/foo/1/2/3'); + await changeHash('#/abc'); + service.stop(); + + await expect(promise).resolves.toMatchInlineSnapshot(` +Array [ + Array [ + "foo:-:", + "bar:-:", + ], + Array [ + "foo:-:", + "bar:active:http://localhost/abc#/bar/baz/box", + ], + Array [ + "foo:active:http://localhost/abc#/foo/1/2/3", + "bar:-:http://localhost/abc#/bar/baz/box", + ], + Array [ + "foo:-:http://localhost/abc#/foo/1/2/3", + "bar:-:http://localhost/abc#/bar/baz/box", + ], +] +`); + }); + + it('does not update when url changes via history.pushState()', async () => { + const { service, summarize } = setup({ navLinks: [FOO_NAV_LINK, BAR_NAV_LINK] }); + + const promise = summarize('id', 'active', 'lastSubUrl'); + + await changeHash('#/bar/baz/box'); + window.history.pushState(undefined, '', '/abc#/foo/1'); + service.stop(); + + await expect(promise).resolves.toMatchInlineSnapshot(` +Array [ + Array [ + "foo:-:", + "bar:-:", + ], + Array [ + "foo:-:", + "bar:active:http://localhost/abc#/bar/baz/box", + ], +] +`); + }); + + it('does update when url changed via history.pushState() and checkCurrentUrl() is called', async () => { + const { service, start, summarize } = setup({ navLinks: [FOO_NAV_LINK, BAR_NAV_LINK] }); + + const promise = summarize('id', 'active', 'lastSubUrl'); + + await changeHash('#/bar/baz/box'); + window.history.pushState(undefined, '', '/abc#/foo/1'); + start.checkCurrentUrl(); + service.stop(); + + await expect(promise).resolves.toMatchInlineSnapshot(` +Array [ + Array [ + "foo:-:", + "bar:-:", + ], + Array [ + "foo:-:", + "bar:active:http://localhost/abc#/bar/baz/box", + ], + Array [ + "foo:active:http://localhost/abc#/foo/1", + "bar:-:http://localhost/abc#/bar/baz/box", + ], +] +`); + }); +}); + +describe('start.getLastUrl()', () => { + it('gets the most recent lastUrl', async () => { + const { start } = setup({ navLinks: [FOO_NAV_LINK] }); + + start.setLastUrl('foo', 'http://localhost/abc#/foo/1234'); + expect(start.getLastUrl('foo')).toBe('http://localhost/abc#/foo/1234'); + await changeHash('#/foo/5678'); + expect(start.getLastUrl('foo')).toBe('http://localhost/abc#/foo/5678'); + }); + + it('returns undefined for unknown ids', () => { + const { start } = setup({ + navLinks: [], + }); + + expect(start.getLastUrl('foo')).toBe(undefined); + }); +}); + +describe('start.setLastUrl()', () => { + it('triggers update, even if navLink is active', async () => { + const { start, service, summarize } = setup({ navLinks: [FOO_NAV_LINK, BAR_NAV_LINK] }); + await changeHash('#/foo/1234'); + + const promise = summarize('id', 'active', 'lastSubUrl'); + start.setLastUrl('foo', 'http://localhost/abc#/foo/5678'); + start.setLastUrl('bar', 'http://localhost/abc#/bar/5678'); + service.stop(); + + await expect(promise).resolves.toMatchInlineSnapshot(` +Array [ + Array [ + "foo:active:http://localhost/abc#/foo/1234", + "bar:-:", + ], + Array [ + "foo:active:http://localhost/abc#/foo/5678", + "bar:-:", + ], + Array [ + "foo:active:http://localhost/abc#/foo/5678", + "bar:-:http://localhost/abc#/bar/5678", + ], +] +`); + }); + + it('writes lastUrl to sessionStorage', () => { + const { start, lastUrlStore } = setup({ navLinks: [FOO_NAV_LINK] }); + start.setLastUrl('foo', 'http://localhost/abc#/foo/1234'); + expect(lastUrlStore).toMatchInlineSnapshot(` +Map { + "foo" => "http://localhost/abc#/foo/1234", +} +`); + }); +}); + +describe('start.getDefaultUrl()', () => { + it('returns the url property from the injected navLink', () => { + const { start } = setup({ navLinks: [FOO_NAV_LINK] }); + expect(start.getDefaultUrl('foo')).toBe('http://localhost/abc#/foo'); + }); + + it('returns undefined for unknown id', () => { + const { start } = setup(); + expect(start.getDefaultUrl('foo')).toBe(undefined); + }); +}); + +describe('start.showOnly()', () => { + it('triggers update with all but this id active, none if id is unknown', async () => { + const { start, summarize, service } = setup({ navLinks: [FOO_NAV_LINK, BAR_NAV_LINK] }); + const promise = summarize('id', 'hidden'); + start.showOnly('foo'); + start.showOnly('bar'); + start.showOnly('baz'); + service.stop(); + await expect(promise).resolves.toMatchInlineSnapshot(` +Array [ + Array [ + "foo:hidden", + "bar:-", + ], + Array [ + "foo:-", + "bar:hidden", + ], + Array [ + "foo:hidden", + "bar:-", + ], + Array [ + "foo:hidden", + "bar:hidden", + ], +] +`); + }); +}); + +describe('start.filterLastUrl()', () => { + it('calls condition function once for each navLink that has a lastUrl', () => { + const { start } = setup({ navLinks: [FOO_NAV_LINK, BAR_NAV_LINK] }); + start.setLastUrl('bar', 'http://localhost/abc#/bar/1234'); + const stub = jest.fn(); + start.filterLastUrls(stub); + expect(stub).toHaveBeenCalledTimes(1); + expect(stub.mock.calls[0][1]).toBe('bar'); + }); + + it('removes lastUrl if condition returns false', () => { + const { start, lastUrlStore } = setup({ navLinks: [FOO_NAV_LINK, BAR_NAV_LINK] }); + start.setLastUrl('foo', 'http://localhost/abc#/foo/1234'); + start.setLastUrl('bar', 'http://localhost/abc#/bar/1234'); + + expect(lastUrlStore.size).toBe(2); + start.filterLastUrls(lastUrl => lastUrl.includes('/foo/')); + expect(lastUrlStore.size).toBe(1); + }); + + it('only triggers one update if multiple lastUrls are removed', async () => { + const { start, service, summarize } = setup({ navLinks: [FOO_NAV_LINK, BAR_NAV_LINK] }); + start.setLastUrl('foo', 'http://localhost/abc#/foo/1234'); + start.setLastUrl('bar', 'http://localhost/abc#/bar/1234'); + + const promise = summarize('id', 'lastSubUrl'); + start.filterLastUrls(() => false); + service.stop(); + + await expect(promise).resolves.toMatchInlineSnapshot(` +Array [ + Array [ + "foo:http://localhost/abc#/foo/1234", + "bar:http://localhost/abc#/bar/1234", + ], + Array [ + "foo:", + "bar:", + ], +] +`); + }); +}); + +describe('start.setUrlInterceptor()', () => { + it('calls interceptor when added to check current url', () => { + const { start } = setup(); + const stub = jest.fn(); + start.setUrlInterceptor(stub); + expect(stub).toHaveBeenCalledTimes(1); + }); + + it('does not update lastUrl if interceptor returns undefined', async () => { + const { start, service, summarize } = setup({ navLinks: [FOO_NAV_LINK] }); + + const promise = summarize('id', 'active'); + start.setUrlInterceptor(() => undefined); + await changeHash('#/foo/1234'); + service.stop(); + + await expect(promise).resolves.toMatchInlineSnapshot(` +Array [ + Array [ + "foo:-", + ], +] +`); + }); + + it('uses alternate url returned from url interceptor', async () => { + const { start, service, summarize } = setup({ navLinks: [FOO_NAV_LINK, BAR_NAV_LINK] }); + + const promise = summarize('id', 'active', 'lastSubUrl'); + start.setUrlInterceptor(lastUrl => lastUrl.replace('/foo/', '/bar/')); + await changeHash('#/foo/5678'); + service.stop(); + await expect(promise).resolves.toMatchInlineSnapshot(` +Array [ + Array [ + "foo:-:", + "bar:-:", + ], + Array [ + "foo:-:", + "bar:-:", + ], + Array [ + "foo:-:", + "bar:active:http://localhost/abc#/bar/5678", + ], +] +`); + }); +}); + +describe('start.decorate()', () => { + it('triggers updates that include decorations, even if id unknown', async () => { + const { service, start, summarize } = setup({ navLinks: [FOO_NAV_LINK] }); + const promise = summarize('id', 'disabled'); + + start.decorate('foo', { disabled: true }); + start.decorate('bar', { disabled: true }); + service.stop(); + + await expect(promise).resolves.toMatchInlineSnapshot(` +Array [ + Array [ + "foo:-", + ], + Array [ + "foo:disabled", + ], + Array [ + "foo:disabled", + ], +] +`); + }); + + it('merges with previous decorations', async () => { + const { service, start, summarize } = setup({ navLinks: [BAR_NAV_LINK] }); + const promise = summarize('id', 'hidden', 'disabled'); + + start.decorate('bar', { hidden: true }); + start.decorate('bar', { disabled: true }); + start.decorate('bar', { disabled: false }); + start.decorate('bar', { hidden: false }); + service.stop(); + + await expect(promise).resolves.toMatchInlineSnapshot(` +Array [ + Array [ + "bar:-:-", + ], + Array [ + "bar:hidden:-", + ], + Array [ + "bar:hidden:disabled", + ], + Array [ + "bar:hidden:-", + ], + Array [ + "bar:-:-", + ], +] +`); + }); +}); + +describe('#stop()', () => { + it('completes get$() observables', () => { + const { service, start } = setup(); + const complete = jest.fn(); + start.get$().subscribe({ complete }); + expect(complete).not.toHaveBeenCalled(); + service.stop(); + expect(complete).toHaveBeenCalledTimes(1); + }); + + it('stops listening to window `hashchange` and `popstate` events', () => { + const addSpy = jest.spyOn(window, 'addEventListener'); + const removeSpy = jest.spyOn(window, 'removeEventListener'); + + const { service } = setup(); + expect(addSpy.mock.calls).toMatchInlineSnapshot(` +Array [ + Array [ + "hashchange", + [Function], + undefined, + ], +] +`); + expect(removeSpy).not.toHaveBeenCalled(); + + service.stop(); + + expect(addSpy).toHaveBeenCalledTimes(1); + expect(removeSpy.mock.calls).toMatchInlineSnapshot(` +Array [ + Array [ + "hashchange", + [Function], + undefined, + ], +] +`); + }); +}); diff --git a/src/core/public/nav_links/nav_links_service.ts b/src/core/public/nav_links/nav_links_service.ts new file mode 100644 index 000000000000..b55e4f185944 --- /dev/null +++ b/src/core/public/nav_links/nav_links_service.ts @@ -0,0 +1,278 @@ +/* + * 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 * as Url from 'url'; + +import * as Rx from 'rxjs'; +import { + distinctUntilChanged, + filter, + ignoreElements, + map, + startWith, + takeUntil, + tap, +} from 'rxjs/operators'; + +import { BasePathStartContract } from '../base_path'; +import { InjectedMetadataStartContract } from '../injected_metadata'; +import { shareWeakReplay } from '../utils'; +import { PrefixedStorage } from './prefixed_storage'; + +/** + * The properties that can be set using start.decorate() + */ +interface Decoration { + hidden?: boolean; + tooltip?: string; + disabled?: boolean; +} + +type UrlInterceptor = (url: string) => string | void; + +export interface NavLink { + readonly id: string; + readonly active: boolean; + readonly lastSubUrl?: string; + readonly title: string; + readonly order: number; + readonly url: string; + readonly linkToLastSubUrl: boolean; + readonly subUrlBase: string; + readonly icon: string; + readonly hidden: boolean; + readonly disabled: boolean; + readonly tooltip?: string; + readonly description?: string; +} +export type NavLinks = ReadonlyArray; +export type NavLinksStartContract = ReturnType; + +interface Deps { + basePath: BasePathStartContract; + injectedMetadata: InjectedMetadataStartContract; +} + +export class NavLinksService { + private readonly stop$ = new Rx.ReplaySubject(1); + + public start({ injectedMetadata, basePath }: Deps) { + const lastUrlStorage = new PrefixedStorage(window.sessionStorage, `lastUrl:${basePath.get()}`); + const pullLastUrlStorage$ = new Rx.Subject(); + + const showOnly$ = new Rx.BehaviorSubject(undefined); + const decorations$ = new Rx.BehaviorSubject>({}); + const urlInterceptor$ = new Rx.BehaviorSubject(undefined); + + // basic properties for navLinks, currently just read from injected metadata + // but will probably include links from application service soon + const baseNavLinks$ = Rx.of( + injectedMetadata.getNavLinks().map(injected => ({ + id: injected.id, + title: injected.title, + order: injected.order, + url: Url.resolve(window.location.href, injected.url), + subUrlBase: Url.resolve(window.location.href, injected.subUrlBase), + icon: Url.resolve(window.location.href, basePath.addToPath(`/${injected.icon}`)), + linkToLastSubUrl: injected.linkToLastSubUrl, + hidden: !!injected.hidden, + disabled: !!injected.disabled, + tooltip: injected.tooltip, + description: injected.description, + })) + ); + + const checkCurrentUrl$ = new Rx.Subject(); + const currentUrl$ = Rx.merge( + Rx.of([undefined]), + Rx.fromEvent(window, 'hashchange'), + checkCurrentUrl$ + ).pipe( + map(() => window.location.href), + distinctUntilChanged() + ); + + const trackingUrl$ = Rx.combineLatest(currentUrl$, urlInterceptor$).pipe( + map(([currentUrl, interceptor]) => { + if (!interceptor) { + return currentUrl; + } + + return interceptor(currentUrl); + }), + filter((url): url is string => !!url), + shareWeakReplay(1) + ); + + const updateLastUrlStorage$ = Rx.combineLatest(trackingUrl$, baseNavLinks$).pipe( + tap(([trackingUrl, baseNavLinks]) => { + for (const { id, subUrlBase } of baseNavLinks) { + if (trackingUrl.startsWith(subUrlBase)) { + lastUrlStorage.set(id, trackingUrl); + } + } + }) + ); + + const createNavLinks$ = Rx.combineLatest( + trackingUrl$, + baseNavLinks$, + decorations$, + showOnly$, + pullLastUrlStorage$.pipe(startWith(undefined)) + ).pipe( + map(([trackingUrl, baseNavLinks, decorations, showOnly]) => { + return Object.freeze( + baseNavLinks + .map(({ id, subUrlBase, ...baseProps }) => { + const navLink: NavLink = { + id, + subUrlBase, + ...baseProps, + ...decorations[id], + active: trackingUrl.startsWith(subUrlBase), + lastSubUrl: lastUrlStorage.get(id), + ...(showOnly && { hidden: showOnly !== id }), + }; + + return Object.freeze(navLink); + }) + .sort((a, b) => a.order - b.order) + ); + }) + ); + + const navLinks$ = new Rx.BehaviorSubject([]); + + // run createNavLinks$ and updateLastUrlStorage$ in the background and cache results + // into navLinks$ subject for sync getters + Rx.merge(updateLastUrlStorage$.pipe(ignoreElements()), createNavLinks$) + .pipe(takeUntil(this.stop$)) + .subscribe(navLinks$); + + return { + /** + * Get an observable that emits snapshots of the navLinks. + */ + get$: () => navLinks$.asObservable(), + + /** + * Get the current lastUrl for a navLink by id. + */ + getLastUrl: (id: string) => lastUrlStorage.get(id), + + /** + * Set the lastUrl for a navLink by id. + */ + setLastUrl: (id: string, lastUrl: string) => { + lastUrlStorage.set(id, lastUrl); + pullLastUrlStorage$.next(); + }, + + /** + * Get the default url (not the lastUrl) for a navLink by id. + */ + getDefaultUrl: (id: string) => { + const navLink = navLinks$.getValue().find(nl => nl.id === id); + if (navLink) { + return navLink.url; + } + }, + + /** + * Hide all navLinks other than the one with this id. + */ + showOnly: (id: string) => { + showOnly$.next(id); + }, + + /** + * Filter the stored lastUrls, works like Array.filter, the lastUrl is + * deleted unless the condition returns something truthy. + */ + filterLastUrls: (condition: (lastUrl: string, id: string) => boolean) => { + let updated = false; + + for (const { id } of navLinks$.getValue()) { + const lastUrl = lastUrlStorage.get(id); + if (lastUrl === undefined) { + continue; + } + + if (!condition(lastUrl, id)) { + lastUrlStorage.delete(id); + updated = true; + } + } + + if (updated) { + pullLastUrlStorage$.next(); + } + }, + + /** + * Add a function that receives the urls used to determine active and lastUrl states. Returning a + * string uses that as the tracking url, returning undefined tells navLinks to ignore it. + * + * **NOTE:** only one interceptor is allowed at this time because that's all we need right now + * and it avoids piling up interceptors in the browser tests where angular is initialized over + * and over for each test. + */ + setUrlInterceptor: (interceptor: (url: string) => string | void) => { + urlInterceptor$.next(interceptor); + }, + + /** + * Define arbitrary properties that will be merged into the navLink object with the + * specified id. Cannot be used to override active, lastSubUrl, or hidden states caused + * by `navLinks.showOnly()`. + */ + decorate: (id: string, decoration: Decoration) => { + const decorations = decorations$.getValue(); + decorations$.next({ + ...decorations, + [id]: { + ...decorations[id], + ...decoration, + }, + }); + }, + + /** + * Tell core.navLinks that the current url might be out of date. This method must be + * called after using `history.pushState()` or `history.replaceState()` in order for + * the navLinks to see the URL update since there is no way to observe url updates + * caused with those methods. + * + * Angular didn't have this problem because it checked window.location.href at the start + * of every digest cycle, we also only do this in one location and we've tried other + * approaches but they all turn out either really hacky or turn out to have undesirable + * side-effects. This should not be necessary once core implements some sort of routing + * abstraction that prevents us from needing to use `history.pushState()`. + */ + checkCurrentUrl: () => { + checkCurrentUrl$.next(); + }, + }; + } + + public stop() { + this.stop$.next(); + } +} diff --git a/src/core/public/nav_links/prefixed_storage.test.ts b/src/core/public/nav_links/prefixed_storage.test.ts new file mode 100644 index 000000000000..ecabea9f34ba --- /dev/null +++ b/src/core/public/nav_links/prefixed_storage.test.ts @@ -0,0 +1,84 @@ +/* + * 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 { PrefixedStorage } from './prefixed_storage'; + +describe('get()', () => { + it('prefixes id and returns value from browser storage', () => { + const stub = { + getItem: jest.fn().mockReturnValue('foo'), + }; + + const storage = new PrefixedStorage(stub as any, 'bar'); + + expect(storage.get('id')).toBe('foo'); + expect(stub.getItem.mock.calls).toMatchInlineSnapshot(` +Array [ + Array [ + "bar:id", + ], +] +`); + }); + + it('returns undefined if browser storage returns null', () => { + const stub = { + getItem: jest.fn().mockReturnValue(null), + }; + const storage = new PrefixedStorage(stub as any, 'bar'); + expect(storage.get('id')).toBe(undefined); + }); +}); + +describe('set()', () => { + it('prefixes id and sets in browser storage', () => { + const stub = { + setItem: jest.fn(), + }; + + const storage = new PrefixedStorage(stub as any, 'foo'); + storage.set('id', 'value'); + expect(stub.setItem.mock.calls).toMatchInlineSnapshot(` +Array [ + Array [ + "foo:id", + "value", + ], +] +`); + }); +}); + +describe('delete()', () => { + it('prefixes id and deleted from browser storage', () => { + const stub = { + removeItem: jest.fn(), + }; + + const storage = new PrefixedStorage(stub as any, 'foo'); + storage.delete('id'); + expect(stub.removeItem.mock.calls).toMatchInlineSnapshot(` +Array [ + Array [ + "foo:id", + ], +] +`); + }); +}); diff --git a/src/core/public/nav_links/prefixed_storage.ts b/src/core/public/nav_links/prefixed_storage.ts new file mode 100644 index 000000000000..d1ec3fc38090 --- /dev/null +++ b/src/core/public/nav_links/prefixed_storage.ts @@ -0,0 +1,39 @@ +/* + * 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. + */ + +export class PrefixedStorage { + constructor(private readonly browserStorage: Storage, private readonly prefix: string) {} + + public get(id: string) { + const value = this.browserStorage.getItem(this.getKey(id)); + return value === null ? undefined : value; + } + + public set(id: string, value: string) { + this.browserStorage.setItem(this.getKey(id), value); + } + + public delete(id: string) { + this.browserStorage.removeItem(this.getKey(id)); + } + + private getKey(id: string) { + return `${this.prefix}:${id}`; + } +} diff --git a/src/core_plugins/kibana/public/context/index.js b/src/core_plugins/kibana/public/context/index.js index ab332d8fcb2a..8fabcd3ea4ff 100644 --- a/src/core_plugins/kibana/public/context/index.js +++ b/src/core_plugins/kibana/public/context/index.js @@ -66,7 +66,7 @@ function ContextAppRouteController( this.anchorType = $routeParams.type; this.anchorId = $routeParams.id; this.indexPattern = indexPattern; - this.discoverUrl = chrome.getNavLinkById('kibana:discover').lastSubUrl; + this.discoverUrl = chrome.navLinks.getLastUrl('kibana:discover') || chrome.navLinks.getDefaultUrl('kibana:discover'); this.filters = _.cloneDeep(queryFilter.getFilters()); } diff --git a/src/core_plugins/kibana/public/dev_tools/hacks/__tests__/hide_empty_tools.js b/src/core_plugins/kibana/public/dev_tools/hacks/__tests__/hide_empty_tools.js index 6e0e07104fa8..688abed388fd 100644 --- a/src/core_plugins/kibana/public/dev_tools/hacks/__tests__/hide_empty_tools.js +++ b/src/core_plugins/kibana/public/dev_tools/hacks/__tests__/hide_empty_tools.js @@ -17,15 +17,12 @@ * under the License. */ -import expect from 'expect.js'; import sinon from 'sinon'; import chrome from 'ui/chrome'; import { hideEmptyDevTools } from '../hide_empty_tools'; describe('hide dev tools', function () { - let navlinks; - function PrivateWithoutTools() { return []; } @@ -34,26 +31,25 @@ describe('hide dev tools', function () { return ['tool1', 'tool2']; } - function isHidden() { - return !!chrome.getNavLinkById('kibana:dev_tools').hidden; - } - beforeEach(function () { - navlinks = {}; - sinon.stub(chrome, 'getNavLinkById').returns(navlinks); + sinon.stub(chrome.navLinks, 'decorate'); }); it('should hide the app if there are no dev tools', function () { - hideEmptyDevTools(PrivateWithTools); - expect(isHidden()).to.be(false); + hideEmptyDevTools(PrivateWithoutTools); + sinon.assert.calledWith(chrome.navLinks.decorate, 'kibana:dev_tools', { + hidden: true + }); }); it('should not hide the app if there are tools', function () { - hideEmptyDevTools(PrivateWithoutTools); - expect(isHidden()).to.be(true); + hideEmptyDevTools(PrivateWithTools); + sinon.assert.calledWith(chrome.navLinks.decorate, 'kibana:dev_tools', { + hidden: false + }); }); afterEach(function () { - chrome.getNavLinkById.restore(); + chrome.navLinks.decorate.restore(); }); }); diff --git a/src/core_plugins/kibana/public/dev_tools/hacks/hide_empty_tools.js b/src/core_plugins/kibana/public/dev_tools/hacks/hide_empty_tools.js index 3426932ea5e7..769244730d4a 100644 --- a/src/core_plugins/kibana/public/dev_tools/hacks/hide_empty_tools.js +++ b/src/core_plugins/kibana/public/dev_tools/hacks/hide_empty_tools.js @@ -22,11 +22,9 @@ import chrome from 'ui/chrome'; import { DevToolsRegistryProvider } from 'ui/registry/dev_tools'; export function hideEmptyDevTools(Private) { - const hasTools = !!Private(DevToolsRegistryProvider).length; - if (!hasTools) { - const navLink = chrome.getNavLinkById('kibana:dev_tools'); - navLink.hidden = true; - } + chrome.navLinks.decorate('kibana:dev_tools', { + hidden: !Private(DevToolsRegistryProvider).length + }); } uiModules.get('kibana').run(hideEmptyDevTools); diff --git a/src/core_plugins/kibana/public/discover/components/fetch_error/fetch_error.js b/src/core_plugins/kibana/public/discover/components/fetch_error/fetch_error.js index 745693178d96..be704a1ba772 100644 --- a/src/core_plugins/kibana/public/discover/components/fetch_error/fetch_error.js +++ b/src/core_plugins/kibana/public/discover/components/fetch_error/fetch_error.js @@ -38,7 +38,7 @@ const DiscoverFetchError = ({ fetchError }) => { let body; if (fetchError.lang === 'painless') { - const managementUrl = chrome.getNavLinkById('kibana:management').url; + const managementUrl = chrome.navLinks.getDefaultUrl('kibana:management'); const url = `${managementUrl}/kibana/indices`; body = ( diff --git a/src/core_plugins/kibana/public/visualize/editor/editor.js b/src/core_plugins/kibana/public/visualize/editor/editor.js index 07eef274f479..608a14a965b7 100644 --- a/src/core_plugins/kibana/public/visualize/editor/editor.js +++ b/src/core_plugins/kibana/public/visualize/editor/editor.js @@ -366,14 +366,14 @@ function VisEditor( appId: kbnBaseUrl.slice('/app/'.length), appPath: kbnUrl.eval(`${VisualizeConstants.EDIT_PATH}/{{id}}`, { id: savedVis.id }), }); + // Manually insert a new url so the back button will open the saved visualization. $window.history.pushState({}, '', savedVisualizationParsedUrl.getRootRelativePath()); - // Since we aren't reloading the page, only inserting a new browser history item, we need to manually update - // the last url for this app, so directly clicking on the Visualize tab will also bring the user to the saved - // url, not the unsaved one. - chrome.trackSubUrlForApp('kibana:visualize', savedVisualizationParsedUrl); - const lastDashboardAbsoluteUrl = chrome.getNavLinkById('kibana:dashboard').lastSubUrl; + // We must tell core.navLinks to observe the new URL since we're changing it with history.pushState(). + chrome.navLinks.checkCurrentUrl(); + + const lastDashboardAbsoluteUrl = chrome.navLinks.getLastUrl('kibana:dashboard'); const dashboardParsedUrl = absoluteToParsedUrl(lastDashboardAbsoluteUrl, chrome.getBasePath()); dashboardParsedUrl.addQueryParameter(DashboardConstants.NEW_VISUALIZATION_ID_PARAM, savedVis.id); kbnUrl.change(dashboardParsedUrl.appPath); @@ -383,7 +383,8 @@ function VisEditor( kbnUrl.change(`${VisualizeConstants.EDIT_PATH}/{{id}}`, { id: savedVis.id }); } } - }, (err) => { + }) + .catch((err) => { toastNotifications.addDanger({ title: `Error on saving '${savedVis.title}'`, text: err.message, diff --git a/src/core_plugins/tests_bundle/tests_entry_template.js b/src/core_plugins/tests_bundle/tests_entry_template.js index aabe01734cc9..adbda2f4dd83 100644 --- a/src/core_plugins/tests_bundle/tests_entry_template.js +++ b/src/core_plugins/tests_bundle/tests_entry_template.js @@ -47,6 +47,7 @@ new CoreSystem({ injectedMetadata: { version: '1.2.3', buildNumber: 1234, + navLinks: [], legacyMetadata: { version: '1.2.3', buildNum: 1234, diff --git a/src/ui/public/chrome/api/__tests__/nav.js b/src/ui/public/chrome/api/__tests__/nav.js deleted file mode 100644 index 169c9546a4a3..000000000000 --- a/src/ui/public/chrome/api/__tests__/nav.js +++ /dev/null @@ -1,182 +0,0 @@ -/* - * 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 'expect.js'; - -import { initChromeNavApi } from '../nav'; -import StubBrowserStorage from 'test_utils/stub_browser_storage'; -import { KibanaParsedUrl } from '../../../url/kibana_parsed_url'; - -const basePath = '/someBasePath'; - -function init(customInternals = { basePath }) { - const chrome = { - getBasePath: () => customInternals.basePath || '', - }; - const internals = { - nav: [], - ...customInternals, - }; - initChromeNavApi(chrome, internals); - return { chrome, internals }; -} - -describe('chrome nav apis', function () { - describe('#getNavLinkById', () => { - it ('retrieves the correct nav link, given its ID', () => { - const appUrlStore = new StubBrowserStorage(); - const nav = [ - { id: 'kibana:discover', title: 'Discover' } - ]; - const { - chrome - } = init({ appUrlStore, nav }); - - const navLink = chrome.getNavLinkById('kibana:discover'); - expect(navLink).to.eql(nav[0]); - }); - - it ('throws an error if the nav link with the given ID is not found', () => { - const appUrlStore = new StubBrowserStorage(); - const nav = [ - { id: 'kibana:discover', title: 'Discover' } - ]; - const { - chrome - } = init({ appUrlStore, nav }); - - let errorThrown = false; - try { - chrome.getNavLinkById('nonexistent'); - } catch (e) { - errorThrown = true; - } - expect(errorThrown).to.be(true); - }); - }); - - describe('#untrackNavLinksForDeletedSavedObjects', function () { - const appId = 'appId'; - const appUrl = 'https://localhost:9200/app/kibana#test'; - const deletedId = 'IAMDELETED'; - - it('should clear last url when last url contains link to deleted saved object', function () { - const appUrlStore = new StubBrowserStorage(); - const nav = [ - { - id: appId, - title: 'Discover', - linkToLastSubUrl: true, - lastSubUrl: `${appUrl}?id=${deletedId}`, - url: appUrl - } - ]; - const { - chrome - } = init({ appUrlStore, nav }); - - chrome.untrackNavLinksForDeletedSavedObjects([deletedId]); - expect(chrome.getNavLinkById('appId').lastSubUrl).to.be(appUrl); - }); - - it('should not clear last url when last url does not contains link to deleted saved object', function () { - const lastUrl = `${appUrl}?id=anotherSavedObjectId`; - const appUrlStore = new StubBrowserStorage(); - const nav = [ - { - id: appId, - title: 'Discover', - linkToLastSubUrl: true, - lastSubUrl: lastUrl, - url: appUrl - } - ]; - const { - chrome - } = init({ appUrlStore, nav }); - - chrome.untrackNavLinksForDeletedSavedObjects([deletedId]); - expect(chrome.getNavLinkById(appId).lastSubUrl).to.be(lastUrl); - }); - }); - - describe('internals.trackPossibleSubUrl()', function () { - it('injects the globalState of the current url to all links for the same app', function () { - const appUrlStore = new StubBrowserStorage(); - const nav = [ - { - url: 'https://localhost:9200/app/kibana#discover', - subUrlBase: 'https://localhost:9200/app/kibana#discover' - }, - { - url: 'https://localhost:9200/app/kibana#visualize', - subUrlBase: 'https://localhost:9200/app/kibana#visualize' - }, - { - url: 'https://localhost:9200/app/kibana#dashboards', - subUrlBase: 'https://localhost:9200/app/kibana#dashboard' - }, - ].map(l => { - l.lastSubUrl = l.url; - return l; - }); - - const { - internals - } = init({ appUrlStore, nav }); - - internals.trackPossibleSubUrl('https://localhost:9200/app/kibana#dashboard?_g=globalstate'); - expect(internals.nav[0].lastSubUrl).to.be('https://localhost:9200/app/kibana#discover?_g=globalstate'); - expect(internals.nav[0].active).to.be(false); - - expect(internals.nav[1].lastSubUrl).to.be('https://localhost:9200/app/kibana#visualize?_g=globalstate'); - expect(internals.nav[1].active).to.be(false); - - expect(internals.nav[2].lastSubUrl).to.be('https://localhost:9200/app/kibana#dashboard?_g=globalstate'); - expect(internals.nav[2].active).to.be(true); - }); - }); - - describe('internals.trackSubUrlForApp()', function () { - it('injects a manual app url', function () { - const appUrlStore = new StubBrowserStorage(); - const nav = [ - { - id: 'kibana:visualize', - url: 'https://localhost:9200/app/kibana#visualize', - lastSubUrl: 'https://localhost:9200/app/kibana#visualize', - subUrlBase: 'https://localhost:9200/app/kibana#visualize' - } - ]; - - const { chrome, internals } = init({ appUrlStore, nav }); - - const basePath = '/xyz'; - const appId = 'kibana'; - const appPath = 'visualize/1234?_g=globalstate'; - const hostname = 'localhost'; - const port = '9200'; - const protocol = 'https'; - - const kibanaParsedUrl = new KibanaParsedUrl({ basePath, appId, appPath, hostname, port, protocol }); - chrome.trackSubUrlForApp('kibana:visualize', kibanaParsedUrl); - expect(internals.nav[0].lastSubUrl).to.be('https://localhost:9200/xyz/app/kibana#visualize/1234?_g=globalstate'); - }); - }); -}); diff --git a/src/ui/public/chrome/api/__tests__/nav_links.js b/src/ui/public/chrome/api/__tests__/nav_links.js new file mode 100644 index 000000000000..74d563a1df88 --- /dev/null +++ b/src/ui/public/chrome/api/__tests__/nav_links.js @@ -0,0 +1,128 @@ +/* + * 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 ngMock from 'ng_mock'; +import sinon from 'sinon'; +import expect from 'expect.js'; + +import { hashUrl, getUnhashableStatesProvider } from 'ui/state_management/state_hashing'; +import { initChromeNavLinksApi } from '../nav_links'; + +const sandbox = sinon.createSandbox(); + +function setup() { + const chrome = { + getBasePath: () => '/base/path' + }; + + const internal = {}; + let $route; + let getUnhashableStates; + + initChromeNavLinksApi(chrome, internal); + + ngMock.module('kibana', ($routeProvider) => { + $routeProvider.when('/foo/bar', { + redirectTo: '/foo/baz', + }); + $routeProvider.when('/foo/baz', { + template: 'baz', + }); + }); + + sandbox.stub(chrome.navLinks, 'setUrlInterceptor').callsFake(() => { + //noop + }); + + sandbox.stub(chrome.navLinks, 'filterLastUrls').callsFake(() => { + //noop + }); + + ngMock.inject(internal.navLinksAngularInit, ($injector, Private, config) => { + $route = $injector.get('$route'); + getUnhashableStates = Private(getUnhashableStatesProvider); + config.set('state:storeInSessionStorage', true); + }); + + return { chrome, $route, getUnhashableStates }; +} + +describe('navLinks', () => { + beforeEach(() => { + localStorage.clear(); + }); + + afterEach(() => { + sandbox.restore(); + ngMock.inject((config) => { + config.set('state:storeInSessionStorage', false); + }); + }); + + describe('urlInterceptor', () => { + it('is registered with newPlatformNavLinks', () => { + const { chrome } = setup(); + sinon.assert.calledOnce(chrome.navLinks.setUrlInterceptor); + }); + + it('returns url with unhashed states', () => { + const { chrome, getUnhashableStates } = setup(); + + const [interceptor] = chrome.navLinks.setUrlInterceptor.getCall(0).args; + const url = 'http://localhost:5601/base/path/app/kibana#/url?_g=(a:b)'; + + const hashedUrl = hashUrl(getUnhashableStates(), url); + expect(hashedUrl).not.to.be(url); + + const interceptedUrl = interceptor(hashedUrl); + expect(interceptedUrl).to.be(url); + }); + + it('filters out urls for redirect routes', () => { + const { chrome } = setup(); + const [interceptor] = chrome.navLinks.setUrlInterceptor.getCall(0).args; + expect(interceptor('http://localhost:5601/base/path/app/kibana#/foo/bar')).to.be(undefined); + }); + }); + + describe('lastUrl filter', () => { + it('filters during initialization', () => { + const { chrome } = setup(); + sinon.assert.calledOnce(chrome.navLinks.filterLastUrls); + }); + + it('returns true for urls that do not match routes', () => { + const { chrome } = setup(); + const [filter] = chrome.navLinks.filterLastUrls.getCall(0).args; + expect(filter('http://google.com')).to.be(true); + }); + + it('returns true for urls that point to non-redirect routes', () => { + const { chrome } = setup(); + const [filter] = chrome.navLinks.filterLastUrls.getCall(0).args; + expect(filter('http://localhost:5601/base/path/app/kibana#/foo/baz')).to.be(true); + }); + + it('returns false for urls that point to redirect routes', () => { + const { chrome } = setup(); + const [filter] = chrome.navLinks.filterLastUrls.getCall(0).args; + expect(filter('http://localhost:5601/base/path/app/kibana#/foo/bar')).to.be(false); + }); + }); +}); diff --git a/src/ui/public/chrome/api/apps.js b/src/ui/public/chrome/api/apps.js index feb339e163a6..5bd3bf9f08ee 100644 --- a/src/ui/public/chrome/api/apps.js +++ b/src/ui/public/chrome/api/apps.js @@ -22,24 +22,10 @@ import { resolve } from 'url'; // eslint-disable-next-line @elastic/kibana-custom/no-default-export export default function (chrome, internals) { - if (get(internals, 'app.navLink.url')) { internals.app.navLink.url = resolve(window.location.href, internals.app.navLink.url); } - internals.appUrlStore = internals.appUrlStore || window.sessionStorage; - try { - const verifySessionStorage = 'verify sessionStorage'; - internals.appUrlStore.setItem(verifySessionStorage, 1); - internals.appUrlStore.removeItem(verifySessionStorage); - } catch (error) { - throw new Error( - 'Kibana requires access to sessionStorage, and it looks like ' + - 'your browser is restricting it. If you\'re ' + - 'using Safari with private browsing enabled, you can solve this ' + - 'problem by disabling private browsing, or by using another browser.'); - } - /** * ui/chrome apps API * @@ -72,12 +58,4 @@ export default function (chrome, internals) { chrome.getAppUrl = function () { return get(internals, ['app', 'navLink', 'url']); }; - - chrome.getLastUrlFor = function (appId) { - return internals.appUrlStore.getItem(`appLastUrl:${appId}`); - }; - - chrome.setLastUrlFor = function (appId, url) { - internals.appUrlStore.setItem(`appLastUrl:${appId}`, url); - }; } diff --git a/src/ui/public/chrome/api/nav.js b/src/ui/public/chrome/api/nav.js deleted file mode 100644 index 04aab308396c..000000000000 --- a/src/ui/public/chrome/api/nav.js +++ /dev/null @@ -1,141 +0,0 @@ -/* - * 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 { remove } from 'lodash'; -import { relativeToAbsolute } from '../../url/relative_to_absolute'; -import { absoluteToParsedUrl } from '../../url/absolute_to_parsed_url'; - -export function initChromeNavApi(chrome, internals) { - chrome.getNavLinks = function () { - return internals.nav; - }; - - chrome.navLinkExists = (id) => { - return !!internals.nav.find(link => link.id === id); - }; - - chrome.getNavLinkById = (id) => { - const navLink = internals.nav.find(link => link.id === id); - if (!navLink) { - throw new Error(`Nav link for id = ${id} not found`); - } - return navLink; - }; - - chrome.showOnlyById = (id) => { - remove(internals.nav, app => app.id !== id); - }; - - function lastSubUrlKey(link) { - return `lastSubUrl:${link.url}`; - } - - function setLastUrl(link, url) { - if (link.linkToLastSubUrl === false) { - return; - } - - link.lastSubUrl = url; - internals.appUrlStore.setItem(lastSubUrlKey(link), url); - } - - function refreshLastUrl(link) { - link.lastSubUrl = internals.appUrlStore.getItem(lastSubUrlKey(link)) || link.lastSubUrl || link.url; - } - - function injectNewGlobalState(link, fromAppId, newGlobalState) { - const kibanaParsedUrl = absoluteToParsedUrl(link.lastSubUrl, chrome.getBasePath()); - - // don't copy global state if links are for different apps - if (fromAppId !== kibanaParsedUrl.appId) return; - - kibanaParsedUrl.setGlobalState(newGlobalState); - - link.lastSubUrl = kibanaParsedUrl.getAbsoluteUrl(); - } - - /** - * Clear last url for deleted saved objects to avoid loading pages with "Could not locate.." - */ - chrome.untrackNavLinksForDeletedSavedObjects = (deletedIds) => { - function urlContainsDeletedId(url) { - const includedId = deletedIds.find(deletedId => { - return url.includes(deletedId); - }); - if (includedId === undefined) { - return false; - } - return true; - } - - internals.nav.forEach(link => { - if (link.linkToLastSubUrl && urlContainsDeletedId(link.lastSubUrl)) { - setLastUrl(link, link.url); - } - }); - }; - - /** - * Manually sets the last url for the given app. The last url for a given app is updated automatically during - * normal page navigation, so this should only need to be called to insert a last url that was not actually - * navigated to. For instance, when saving an object and redirecting to another page, the last url of the app - * should be the saved instance, but because of the redirect to a different page (e.g. `Save and Add to Dashboard` - * on visualize tab), it won't be tracked automatically and will need to be inserted manually. See - * https://github.com/elastic/kibana/pull/11932 for more background on why this was added. - * @param linkId {String} - an id that represents the navigation link. - * @param kibanaParsedUrl {KibanaParsedUrl} the url to track - */ - chrome.trackSubUrlForApp = (linkId, kibanaParsedUrl) => { - for (const link of internals.nav) { - if (link.id === linkId) { - const absoluteUrl = kibanaParsedUrl.getAbsoluteUrl(); - setLastUrl(link, absoluteUrl); - return; - } - } - }; - - internals.trackPossibleSubUrl = function (url) { - const kibanaParsedUrl = absoluteToParsedUrl(url, chrome.getBasePath()); - - for (const link of internals.nav) { - link.active = url.startsWith(link.subUrlBase); - if (link.active) { - setLastUrl(link, url); - continue; - } - - refreshLastUrl(link); - - const newGlobalState = kibanaParsedUrl.getGlobalState(); - if (newGlobalState) { - injectNewGlobalState(link, kibanaParsedUrl.appId, newGlobalState); - } - } - }; - - internals.nav.forEach(link => { - link.url = relativeToAbsolute(link.url); - link.subUrlBase = relativeToAbsolute(link.subUrlBase); - }); - - // simulate a possible change in url to initialize the - // link.active and link.lastUrl properties - internals.trackPossibleSubUrl(document.location.href); -} diff --git a/src/ui/public/chrome/api/nav_links.js b/src/ui/public/chrome/api/nav_links.js new file mode 100644 index 000000000000..d114085a7737 --- /dev/null +++ b/src/ui/public/chrome/api/nav_links.js @@ -0,0 +1,83 @@ +/* + * 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 { parse as parseUrl } from 'url'; + +import { + getUnhashableStatesProvider, + unhashUrl, +} from '../../state_management/state_hashing'; +import { absoluteToParsedUrl } from '../../url/absolute_to_parsed_url'; + +let newPlatformNavLinks; + +export function __newPlatformInit__(instance) { + if (newPlatformNavLinks) { + throw new Error('ui/chrome/api/injected_vars is already initialized'); + } + + newPlatformNavLinks = instance; +} + +export function initChromeNavLinksApi(chrome, internals) { + chrome.navLinks = newPlatformNavLinks; + + internals.navLinksAngularInit = function ($injector, Private) { + // disable if angular routing is not enabled + if (!$injector.has('$route')) { + return; + } + + const $route = $injector.get('$route'); + const getUnhashableStates = Private(getUnhashableStatesProvider); + const basePath = chrome.getBasePath(); + + const getRouteForUrl = url => { + const parsedUrl = absoluteToParsedUrl(url, basePath); + + if (!parsedUrl.appPath) { + return; + } + + const { pathname: appPathname } = parseUrl(parsedUrl.appPath); + const matchedRoute = Object.values($route.routes).find(route => ( + route && route.regexp && appPathname.match(route.regexp) + )); + + return matchedRoute || $route.routes.null; + }; + + // intercept urls to create unhashed versions of states and ignore redirect only urls which can cause https://github.com/elastic/kibana/pull/13432 + newPlatformNavLinks.setUrlInterceptor(url => { + const route = getRouteForUrl(url); + + if (route && route.redirectTo) { + return; + } + + return unhashUrl(url, getUnhashableStates()); + }); + + // filter the current lastUrls to ensure that none of them point to redirecting angular routes, preventing https://github.com/elastic/kibana/pull/13432 + newPlatformNavLinks.filterLastUrls(lastUrl => { + const route = getRouteForUrl(lastUrl); + return !route || !route.redirectTo; + }); + }; +} diff --git a/src/ui/public/chrome/chrome.js b/src/ui/public/chrome/chrome.js index 57922d39f613..4295c0e942be 100644 --- a/src/ui/public/chrome/chrome.js +++ b/src/ui/public/chrome/chrome.js @@ -34,7 +34,7 @@ import './services'; import { initAngularApi } from './api/angular'; import appsApi from './api/apps'; import controlsApi from './api/controls'; -import { initChromeNavApi } from './api/nav'; +import { initChromeNavLinksApi } from './api/nav_links'; import templateApi from './api/template'; import themeApi from './api/theme'; import translationsApi from './api/translations'; @@ -67,7 +67,7 @@ appsApi(chrome, internals); initChromeXsrfApi(chrome, internals); initChromeBasePathApi(chrome); initChromeInjectedVarsApi(chrome); -initChromeNavApi(chrome, internals); +initChromeNavLinksApi(chrome, internals); initLoadingCountApi(chrome, internals); initAngularApi(chrome, internals); controlsApi(chrome, internals); diff --git a/src/ui/public/chrome/directives/global_nav/app_switcher/__tests__/app_switcher.js b/src/ui/public/chrome/directives/global_nav/app_switcher/__tests__/app_switcher.js index dbb5efe1b06c..36ac287f3ae1 100644 --- a/src/ui/public/chrome/directives/global_nav/app_switcher/__tests__/app_switcher.js +++ b/src/ui/public/chrome/directives/global_nav/app_switcher/__tests__/app_switcher.js @@ -17,32 +17,38 @@ * under the License. */ +import * as Rx from 'rxjs'; import sinon from 'sinon'; import ngMock from 'ng_mock'; import expect from 'expect.js'; +import chrome from 'ui/chrome'; import { DomLocationProvider } from '../../../../../dom_location'; -import { constant, cloneDeep } from 'lodash'; +import { cloneDeep } from 'lodash'; import $ from 'jquery'; -import '../../../..'; import '../app_switcher'; describe('appSwitcher directive', function () { let env; beforeEach(ngMock.module('kibana')); + afterEach(() => { + chrome.navLinks.get$.restore(); + }); function setup(href, links) { - return ngMock.inject(function ($window, $rootScope, $compile, Private) { - const domLocation = Private(DomLocationProvider); + return ngMock.inject(function ($rootScope, $compile, Private) { + sinon.stub(chrome.navLinks, 'get$').callsFake(() => ( + links instanceof Rx.Observable ? links : Rx.of(cloneDeep(links)) + )); - $rootScope.chrome = { - getNavLinks: constant(cloneDeep(links)), - }; + const domLocation = Private(DomLocationProvider); + const $el = $compile($(''))($rootScope); env = { - $scope: $rootScope, - $el: $compile($(''))($rootScope), + $el, + $scope: $el.scope(), + controller: $el.isolateScope().switcher, currentHref: href, location: domLocation }; @@ -66,14 +72,16 @@ describe('appSwitcher directive', function () { active: true, title: 'myLink', url: 'http://localhost:555/app/myApp', - lastSubUrl: 'http://localhost:555/app/myApp#/lastSubUrl' + lastSubUrl: 'http://localhost:555/app/myApp#/lastSubUrl', + linkToLastSubUrl: true, }; const notMyLink = { active: false, title: 'notMyLink', url: 'http://localhost:555/app/notMyApp', - lastSubUrl: 'http://localhost:555/app/notMyApp#/lastSubUrl' + lastSubUrl: 'http://localhost:555/app/notMyApp#/lastSubUrl', + linkToLastSubUrl: true, }; beforeEach(setup('http://localhost:5555/app/myApp/', [myLink, notMyLink])); @@ -96,14 +104,16 @@ describe('appSwitcher directive', function () { active: false, title: 'myLink', url: 'http://localhost:555/app/myApp', - lastSubUrl: 'http://localhost:555/app/myApp#/lastSubUrl' + lastSubUrl: 'http://localhost:555/app/myApp#/lastSubUrl', + linkToLastSubUrl: true, }; const notMyLink = { active: false, title: 'notMyLink', url: 'http://localhost:555/app/notMyApp', - lastSubUrl: 'http://localhost:555/app/notMyApp#/lastSubUrl' + lastSubUrl: 'http://localhost:555/app/notMyApp#/lastSubUrl', + linkToLastSubUrl: true, }; beforeEach(setup('http://localhost:5555/app/myApp/', [myLink, notMyLink])); @@ -121,6 +131,22 @@ describe('appSwitcher directive', function () { }); }); + describe('when links have linkToLastSubUrl: false', function () { + beforeEach(setup('http://localhost:5555/app/myApp/', [{ + active: false, + title: 'myLink', + url: 'http://localhost:555/app/myApp', + lastSubUrl: 'http://localhost:555/app/myApp#/lastSubUrl', + linkToLastSubUrl: false, + }])); + + it('links to the default url', function () { + const [link] = env.$el.findTestSubject('appLink'); + expect(link.href).to.be('http://localhost:555/app/myApp'); + expect(link.href).to.not.be('http://localhost:555/app/myApp#/lastSubUrl'); + }); + }); + describe('clicking a link with matching href but missing hash', function () { const url = 'http://localhost:555/app/myApp?query=1'; beforeEach(setup(url + '#/lastSubUrl', [ @@ -234,4 +260,35 @@ describe('appSwitcher directive', function () { }); }); + describe('navLinks update', () => { + it('applies navLink updates in a sync digest cycle', () => { + const navLinks$ = new Rx.Subject(); + setup('http://localhost:555/app/foo', navLinks$); + const { controller } = env; + + expect(controller.links).to.be(undefined); + navLinks$.next([]); + expect(controller.links).to.eql([]); + }); + + it('persists the $$hashKey on navLink updates', () => { + const navLinks$ = new Rx.Subject(); + setup('http://localhost:555/app/myApp', navLinks$); + const { controller, $scope } = env; + + navLinks$.next([{ id: 'foo' }]); + $scope.$digest(); + const [link] = controller.links; + expect(link.id).to.be('foo'); + expect(link.$$hashKey).to.be.ok(); + + navLinks$.next([{ id: 'foo', lastSubUrl: '#/foo/bar' }]); + $scope.$digest(); + + const [newLink] = controller.links; + expect(newLink.id).to.be(link.id); + expect(newLink.$$hashKey).to.be(link.$$hashKey); + expect(newLink.lastSubUrl).to.be('#/foo/bar'); + }); + }); }); diff --git a/src/ui/public/chrome/directives/global_nav/app_switcher/app_switcher.html b/src/ui/public/chrome/directives/global_nav/app_switcher/app_switcher.html index 2a13b33b23ac..1801c48bd13e 100644 --- a/src/ui/public/chrome/directives/global_nav/app_switcher/app_switcher.html +++ b/src/ui/public/chrome/directives/global_nav/app_switcher/app_switcher.html @@ -5,7 +5,7 @@ is-disabled="link.disabled" tooltip-content="switcher.getTooltip(link)" on-click="switcher.ensureNavigation($event, link)" - url="link.active ? link.url : (link.lastSubUrl || link.url)" + url="link.linkToLastSubUrl && link.lastSubUrl && !link.active ? link.lastSubUrl : link.url" icon="link.icon" label="link.title" > diff --git a/src/ui/public/chrome/directives/global_nav/app_switcher/app_switcher.js b/src/ui/public/chrome/directives/global_nav/app_switcher/app_switcher.js index b349435da17c..201a3f78efde 100644 --- a/src/ui/public/chrome/directives/global_nav/app_switcher/app_switcher.js +++ b/src/ui/public/chrome/directives/global_nav/app_switcher/app_switcher.js @@ -17,8 +17,14 @@ * under the License. */ -import { DomLocationProvider } from '../../../../dom_location'; import { parse } from 'url'; + +import { cloneDeep } from 'lodash'; + +import chrome from 'ui/chrome'; +import { subscribe } from 'ui/utils/subscribe'; + +import { DomLocationProvider } from '../../../../dom_location'; import { uiModules } from '../../../../modules'; import appSwitcherTemplate from './app_switcher.html'; @@ -70,11 +76,25 @@ uiModules template: appSwitcherTemplate, controllerAs: 'switcher', controller($scope, appSwitcherEnsureNavigation, globalNavState) { - if (!$scope.chrome || !$scope.chrome.getNavLinks) { - throw new TypeError('appSwitcher directive requires the "chrome" config-object'); - } + subscribe($scope, chrome.navLinks.get$(), { + next: (newLinks) => { + if (!this.links) { + this.links = cloneDeep(newLinks); + return; + } - this.links = $scope.chrome.getNavLinks(); + const oldLinksById = this.links.reduce((acc, link) => ({ + ...acc, + [link.id]: link + }), {}); + + // merge the newLinks with the oldLinks to persist mutations done by angular + this.links = newLinks.map(newLink => ({ + ...oldLinksById[newLink.id], + ...newLink, + })); + } + }); // links don't cause full-navigation events in certain scenarios // so we force them when needed diff --git a/src/ui/public/chrome/directives/global_nav/global_nav_link/global_nav_link.html b/src/ui/public/chrome/directives/global_nav/global_nav_link/global_nav_link.html index e23fb0ce15e2..8557578a2172 100644 --- a/src/ui/public/chrome/directives/global_nav/global_nav_link/global_nav_link.html +++ b/src/ui/public/chrome/directives/global_nav/global_nav_link/global_nav_link.html @@ -17,7 +17,7 @@ diff --git a/src/ui/public/chrome/directives/global_nav/global_nav_link/global_nav_link.js b/src/ui/public/chrome/directives/global_nav/global_nav_link/global_nav_link.js index 437bec461ba6..a4aa397115ef 100644 --- a/src/ui/public/chrome/directives/global_nav/global_nav_link/global_nav_link.js +++ b/src/ui/public/chrome/directives/global_nav/global_nav_link/global_nav_link.js @@ -17,14 +17,15 @@ * under the License. */ - +import * as Url from 'url'; import globalNavLinkTemplate from './global_nav_link.html'; import './global_nav_link.less'; import { uiModules } from '../../../../modules'; +import chrome from 'ui/chrome'; const module = uiModules.get('kibana'); -module.directive('globalNavLink', chrome => { +module.directive('globalNavLink', () => { return { restrict: 'E', replace: true, @@ -49,6 +50,18 @@ module.directive('globalNavLink', chrome => { return chrome.addBasePath(scope.kbnRoute); } }; + + function resolveIconUrl(icon) { + if (!icon) { + return; + } + + return Url.parse(icon).hostname ? icon : chrome.addBasePath('/' + icon); + } + + scope.$watch('icon', (icon) => { + scope.absoluteIconUrl = resolveIconUrl(icon); + }); } }; }); diff --git a/src/ui/public/chrome/directives/kbn_chrome.js b/src/ui/public/chrome/directives/kbn_chrome.js index 31bdf62646a7..3dad71b1f48f 100644 --- a/src/ui/public/chrome/directives/kbn_chrome.js +++ b/src/ui/public/chrome/directives/kbn_chrome.js @@ -23,16 +23,11 @@ import $ from 'jquery'; import './kbn_chrome.less'; import { uiModules } from '../../modules'; -import { - getUnhashableStatesProvider, - unhashUrl, -} from '../../state_management/state_hashing'; import { notify, GlobalBannerList, banners, } from '../../notify'; -import { SubUrlRouteFilterProvider } from './sub_url_route_filter'; export function kbnChromeProvider(chrome, internals) { @@ -57,37 +52,15 @@ export function kbnChromeProvider(chrome, internals) { }, controllerAs: 'chrome', - controller($scope, $rootScope, $location, $http, Private) { - const getUnhashableStates = Private(getUnhashableStatesProvider); - + controller($scope, $location) { // are we showing the embedded version of the chrome? if (Boolean($location.search().embed)) { internals.permanentlyHideChrome(); } - const subUrlRouteFilter = Private(SubUrlRouteFilterProvider); - - function updateSubUrls() { - const urlWithHashes = window.location.href; - const urlWithStates = unhashUrl(urlWithHashes, getUnhashableStates()); - internals.trackPossibleSubUrl(urlWithStates); - } - - function onRouteChange($event) { - if (subUrlRouteFilter($event)) { - updateSubUrls(); - } - } - - $rootScope.$on('$routeChangeSuccess', onRouteChange); - $rootScope.$on('$routeUpdate', onRouteChange); - updateSubUrls(); // initialize sub urls - // Notifications $scope.notifList = notify._notifs; - // Non-scope based code (e.g., React) - // Banners ReactDOM.render( { - const emit = () => { + const subscription = subscribe($rootScope, uiSettings.getUpdate$(), { + next: ({ key, newValue, oldValue }) => { $rootScope.$broadcast('change:config', newValue, oldValue, key, this); $rootScope.$broadcast(`change:config.${key}`, newValue, oldValue, key, this); - }; - - // this is terrible, but necessary to emulate the same API - // that the `config` service had before where changes were - // emitted to scopes synchronously. All methods that don't - // require knowing if we are currently in a digest cycle are - // async and would deliver events too late for several usecases - // - // If you copy this code elsewhere you better have a good reason :) - $rootScope.$$phase ? emit() : $rootScope.$apply(emit); + } }); $rootScope.$on('$destroy', () => subscription.unsubscribe()); diff --git a/src/ui/public/courier/saved_object/saved_object_loader.js b/src/ui/public/courier/saved_object/saved_object_loader.js index c860fc0eaf33..0dd2269bbacf 100644 --- a/src/ui/public/courier/saved_object/saved_object_loader.js +++ b/src/ui/public/courier/saved_object/saved_object_loader.js @@ -67,7 +67,9 @@ export class SavedObjectLoader { return Promise.all(deletions).then(() => { if (this.chrome) { - this.chrome.untrackNavLinksForDeletedSavedObjects(ids); + this.chrome.navLinks.filterLastUrls(lastUrl => ( + !ids.some(id => lastUrl.includes(id)) + )); } }); } diff --git a/src/ui/public/utils/subscribe.test.ts b/src/ui/public/utils/subscribe.test.ts new file mode 100644 index 000000000000..6c1e838f3f21 --- /dev/null +++ b/src/ui/public/utils/subscribe.test.ts @@ -0,0 +1,143 @@ +/* + * 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. + */ + +const mockFatalError = jest.fn(); +jest.mock('ui/notify/fatal_error', () => ({ + fatalError: mockFatalError, +})); + +import * as Rx from 'rxjs'; +import { subscribe } from './subscribe'; + +let $rootScope: Scope; + +class Scope { + public $$phase?: string; + public $root = $rootScope; + public $apply = jest.fn((fn: () => void) => fn()); +} + +$rootScope = new Scope(); + +afterEach(() => { + jest.clearAllMocks(); +}); + +it('subscribes to the passed observable, returns subscription', () => { + const $scope = new Scope(); + + const unsubSpy = jest.fn(); + const subSpy = jest.fn(() => unsubSpy); + const observable = new Rx.Observable(subSpy); + + const subscription = subscribe($scope as any, observable); + expect(subSpy).toHaveBeenCalledTimes(1); + expect(unsubSpy).not.toHaveBeenCalled(); + + subscription.unsubscribe(); + + expect(subSpy).toHaveBeenCalledTimes(1); + expect(unsubSpy).toHaveBeenCalledTimes(1); +}); + +it('calls observer.next() if already in a digest cycle, wraps in $scope.$apply if not', () => { + const subject = new Rx.Subject(); + const nextSpy = jest.fn(); + const $scope = new Scope(); + + subscribe($scope as any, subject, { next: nextSpy }); + + subject.next(); + expect($scope.$apply).toHaveBeenCalledTimes(1); + expect(nextSpy).toHaveBeenCalledTimes(1); + + jest.clearAllMocks(); + + $rootScope.$$phase = '$digest'; + subject.next(); + expect($scope.$apply).not.toHaveBeenCalled(); + expect(nextSpy).toHaveBeenCalledTimes(1); +}); + +it('reports fatalError if observer.next() throws', () => { + const $scope = new Scope(); + subscribe($scope as any, Rx.of(undefined), { + next() { + throw new Error('foo bar'); + }, + }); + + expect(mockFatalError.mock.calls).toMatchInlineSnapshot(` +Array [ + Array [ + [Error: foo bar], + ], +] +`); +}); + +it('reports fatal error if observer.error is not defined and observable errors', () => { + const $scope = new Scope(); + const error = new Error('foo'); + error.stack = `${error.message}\n---stack trace ---`; + subscribe($scope as any, Rx.throwError(error)); + + expect(mockFatalError.mock.calls).toMatchInlineSnapshot(` +Array [ + Array [ + [Error: Uncaught error in $scope.$subscribe: foo +---stack trace ---], + ], +] +`); +}); + +it('reports fatal error if observer.error throws', () => { + const $scope = new Scope(); + subscribe($scope as any, Rx.throwError(new Error('foo')), { + error: () => { + throw new Error('foo'); + }, + }); + + expect(mockFatalError.mock.calls).toMatchInlineSnapshot(` +Array [ + Array [ + [Error: foo], + ], +] +`); +}); + +it('reports fatal error if observer.complete throws', () => { + const $scope = new Scope(); + subscribe($scope as any, Rx.EMPTY, { + complete: () => { + throw new Error('foo'); + }, + }); + + expect(mockFatalError.mock.calls).toMatchInlineSnapshot(` +Array [ + Array [ + [Error: foo], + ], +] +`); +}); diff --git a/src/ui/public/utils/subscribe.ts b/src/ui/public/utils/subscribe.ts new file mode 100644 index 000000000000..f7918b634b1e --- /dev/null +++ b/src/ui/public/utils/subscribe.ts @@ -0,0 +1,74 @@ +/* + * 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 { IScope } from 'angular'; +import * as Rx from 'rxjs'; +import { fatalError } from 'ui/notify/fatal_error'; + +function callInDigest($scope: IScope, fn: (...args: T) => void, ...args: T) { + try { + // this is terrible, but necessary to synchronously deliver subscription values + // to angular scopes. This is required by some APIs, like the `config` service, + // and beneficial for root level directives where additional digest cycles make + // kibana sluggish to load. + // + // If you copy this code elsewhere you better have a good reason :) + if ($scope.$root.$$phase) { + fn(...args); + } else { + $scope.$apply(() => fn(...args)); + } + } catch (error) { + fatalError(error); + } +} + +/** + * Subscribe to an observable at a $scope, ensuring that the digest cycle + * is run for subscriber hooks and routing errors to fatalError if not handled. + */ +export function subscribe( + $scope: IScope, + observable: Rx.Observable, + observer?: Rx.PartialObserver +) { + return observable.subscribe({ + next(value) { + if (observer && observer.next) { + callInDigest($scope, observer.next, value); + } + }, + error(error) { + callInDigest($scope, () => { + if (observer && observer.error) { + observer.error(error); + } else { + throw new Error( + `Uncaught error in $scope.$subscribe: ${error ? error.stack || error.message : error}` + ); + } + }); + }, + complete() { + if (observer && observer.complete) { + callInDigest($scope, observer.complete); + } + }, + }); +} diff --git a/src/ui/ui_render/ui_render_mixin.js b/src/ui/ui_render/ui_render_mixin.js index f94781cfddf0..73b5aa522236 100644 --- a/src/ui/ui_render/ui_render_mixin.js +++ b/src/ui/ui_render/ui_render_mixin.js @@ -112,7 +112,6 @@ export function uiRenderMixin(kbnServer, server, config) { app, translations, bundleId: `app:${app.getId()}`, - nav: server.getUiNavLinks(), version: kbnServer.version, branch: config.get('pkg.branch'), buildNum: config.get('pkg.buildNum'), @@ -142,6 +141,7 @@ export function uiRenderMixin(kbnServer, server, config) { version: kbnServer.version, buildNumber: config.get('pkg.buildNum'), basePath, + navLinks: server.getUiNavLinks(), vars: await replaceInjectedVars( request, defaults( diff --git a/test/functional/apps/management/_kibana_settings.js b/test/functional/apps/management/_kibana_settings.js index 602a3ca276f7..b50b863bd397 100644 --- a/test/functional/apps/management/_kibana_settings.js +++ b/test/functional/apps/management/_kibana_settings.js @@ -18,12 +18,23 @@ */ import expect from 'expect.js'; +import Url from 'url'; export default function ({ getService, getPageObjects }) { const kibanaServer = getService('kibanaServer'); const remote = getService('remote'); const PageObjects = getPageObjects(['settings', 'common', 'dashboard', 'header']); + function getStatesFromUrl(url) { + const { hash } = Url.parse(url); + if (!hash) { + throw new Error(`Expected url to include hash and app/global state: ${url}`); + } + + const { _g: globalState, _a: appState } = Url.parse(hash.slice(1), true).query; + return { globalState, appState }; + } + describe('kibana settings', function describeIndexTests() { before(async function () { // delete .kibana index and then wait for Kibana to re-create it @@ -59,9 +70,7 @@ export default function ({ getService, getPageObjects }) { await PageObjects.dashboard.clickNewDashboard(); await PageObjects.header.setAbsoluteRange('2015-09-19 06:31:44.000', '2015-09-23 18:31:44.000'); const currentUrl = await remote.getCurrentUrl(); - const urlPieces = currentUrl.match(/(.*)?_g=(.*)&_a=(.*)/); - const globalState = urlPieces[2]; - const appState = urlPieces[3]; + const { globalState, appState } = getStatesFromUrl(currentUrl); // We don't have to be exact, just need to ensure it's greater than when the hashed variation is being used, // which is less than 20 characters. @@ -82,9 +91,7 @@ export default function ({ getService, getPageObjects }) { await PageObjects.dashboard.clickNewDashboard(); await PageObjects.header.setAbsoluteRange('2015-09-19 06:31:44.000', '2015-09-23 18:31:44.000'); const currentUrl = await remote.getCurrentUrl(); - const urlPieces = currentUrl.match(/(.*)?_g=(.*)&_a=(.*)/); - const globalState = urlPieces[2]; - const appState = urlPieces[3]; + const { globalState, appState } = getStatesFromUrl(currentUrl); // We don't have to be exact, just need to ensure it's less than the unhashed version, which will be // greater than 20 characters with the default state plus a time. diff --git a/test/functional/apps/visualize/_area_chart.js b/test/functional/apps/visualize/_area_chart.js index 714878edb0d9..3a7ffa3388ad 100644 --- a/test/functional/apps/visualize/_area_chart.js +++ b/test/functional/apps/visualize/_area_chart.js @@ -149,7 +149,7 @@ export default function ({ getService, getPageObjects }) { it('should hide side editor if embed is set to true in url', async () => { const url = await remote.getCurrentUrl(); - const embedUrl = url.split('/visualize/').pop().replace('?_g=', '?embed=true&_g='); + const embedUrl = url.split('/visualize/').pop().replace('?', '?embed=true&'); await PageObjects.common.navigateToUrl('visualize', embedUrl); await PageObjects.header.waitUntilLoadingHasFinished(); const sideEditorExists = await PageObjects.visualize.getSideEditorExists(); diff --git a/x-pack/plugins/apm/public/hacks/toggle_app_link_in_nav.js b/x-pack/plugins/apm/public/hacks/toggle_app_link_in_nav.js index 90a8f31364f5..1c4d309d65d0 100644 --- a/x-pack/plugins/apm/public/hacks/toggle_app_link_in_nav.js +++ b/x-pack/plugins/apm/public/hacks/toggle_app_link_in_nav.js @@ -6,7 +6,6 @@ import chrome from 'ui/chrome'; -const apmUiEnabled = chrome.getInjected('apmUiEnabled'); -if (apmUiEnabled === false && chrome.navLinkExists('apm')) { - chrome.getNavLinkById('apm').hidden = true; -} +chrome.navLinks.decorate('apm', { + hidden: !chrome.getInjected('apmUiEnabled') +}); diff --git a/x-pack/plugins/dashboard_mode/public/dashboard_viewer.js b/x-pack/plugins/dashboard_mode/public/dashboard_viewer.js index 454afaf884a7..11642ea48d5e 100644 --- a/x-pack/plugins/dashboard_mode/public/dashboard_viewer.js +++ b/x-pack/plugins/dashboard_mode/public/dashboard_viewer.js @@ -47,10 +47,8 @@ uiModules.get('kibana') routes.enable(); routes.otherwise({ redirectTo: defaultUrl() }); -chrome - .setRootController('kibana', function () { - chrome.showOnlyById('kibana:dashboard'); - }); +chrome.navLinks.showOnly('kibana:dashboard'); +chrome.setRootController('kibana', function () {}); uiModules.get('kibana').run(showAppRedirectNotification); diff --git a/x-pack/plugins/graph/public/app.js b/x-pack/plugins/graph/public/app.js index 419040182249..9c1883af0341 100644 --- a/x-pack/plugins/graph/public/app.js +++ b/x-pack/plugins/graph/public/app.js @@ -715,7 +715,7 @@ app.controller('graphuiPlugin', function ($scope, $route, $interval, $http, kbnU .on('zoom', redraw)); - const managementUrl = chrome.getNavLinkById('kibana:management').url; + const managementUrl = chrome.navLinks.getDefaultUrl('kibana:management'); const url = `${managementUrl}/kibana/indices`; if ($scope.indices.length === 0) { diff --git a/x-pack/plugins/graph/public/hacks/toggle_app_link_in_nav.js b/x-pack/plugins/graph/public/hacks/toggle_app_link_in_nav.js index 70162e321f71..4af7cb14aa76 100644 --- a/x-pack/plugins/graph/public/hacks/toggle_app_link_in_nav.js +++ b/x-pack/plugins/graph/public/hacks/toggle_app_link_in_nav.js @@ -11,16 +11,10 @@ import { XPackInfoProvider } from 'plugins/xpack_main/services/xpack_info'; uiModules.get('xpack/graph').run((Private) => { const xpackInfo = Private(XPackInfoProvider); - if (!chrome.navLinkExists('graph')) { - return; - } - const navLink = chrome.getNavLinkById('graph'); - navLink.hidden = true; - const showAppLink = xpackInfo.get('features.graph.showAppLink', false); - navLink.hidden = !showAppLink; - if (showAppLink) { - navLink.disabled = !xpackInfo.get('features.graph.enableAppLink', false); - navLink.tooltip = xpackInfo.get('features.graph.message'); - } + chrome.navLinks.decorate('graph', { + hidden: !xpackInfo.get('features.graph.showAppLink', false), + disabled: !xpackInfo.get('features.graph.enableAppLink', false), + tooltip: xpackInfo.get('features.graph.message'), + }); }); diff --git a/x-pack/plugins/ml/public/hacks/toggle_app_link_in_nav.js b/x-pack/plugins/ml/public/hacks/toggle_app_link_in_nav.js index bed0fe46c323..55fb63f379a5 100644 --- a/x-pack/plugins/ml/public/hacks/toggle_app_link_in_nav.js +++ b/x-pack/plugins/ml/public/hacks/toggle_app_link_in_nav.js @@ -12,14 +12,9 @@ import { uiModules } from 'ui/modules'; uiModules.get('xpack/ml').run((Private) => { const xpackInfo = Private(XPackInfoProvider); - if (!chrome.navLinkExists('ml')) return; - const navLink = chrome.getNavLinkById('ml'); - // hide by default, only show once the xpackInfo is initialized - navLink.hidden = true; - const showAppLink = xpackInfo.get('features.ml.showLinks', false); - navLink.hidden = !showAppLink; - if (showAppLink) { - navLink.disabled = !xpackInfo.get('features.ml.isAvailable', false); - } + chrome.navLinks.decorate('ml', { + hidden: !xpackInfo.get('features.ml.showLinks', false), + disabled: !xpackInfo.get('features.ml.isAvailable', false) + }); }); diff --git a/x-pack/plugins/monitoring/public/hacks/toggle_app_link_in_nav.js b/x-pack/plugins/monitoring/public/hacks/toggle_app_link_in_nav.js index 451793b83dd6..44108247423b 100644 --- a/x-pack/plugins/monitoring/public/hacks/toggle_app_link_in_nav.js +++ b/x-pack/plugins/monitoring/public/hacks/toggle_app_link_in_nav.js @@ -8,9 +8,7 @@ import chrome from 'ui/chrome'; import { uiModules } from 'ui/modules'; uiModules.get('monitoring/hacks').run((monitoringUiEnabled) => { - if (monitoringUiEnabled || !chrome.navLinkExists('monitoring')) { - return; - } - - chrome.getNavLinkById('monitoring').hidden = true; + chrome.navLinks.decorate('monitoring', { + hidden: monitoringUiEnabled + }); }); diff --git a/x-pack/plugins/reporting/public/hacks/job_completion_notifier.js b/x-pack/plugins/reporting/public/hacks/job_completion_notifier.js index 3b15b3557f22..63e1d4536a2e 100644 --- a/x-pack/plugins/reporting/public/hacks/job_completion_notifier.js +++ b/x-pack/plugins/reporting/public/hacks/job_completion_notifier.js @@ -5,6 +5,7 @@ */ import React from 'react'; +import { take } from 'rxjs/operators'; import { toastNotifications } from 'ui/notify'; import chrome from 'ui/chrome'; import { uiModules } from 'ui/modules'; @@ -54,10 +55,12 @@ uiModules.get('kibana') let seeReportLink; - // In-case the license expired/changed between the time they queued the job and the time that - // the job completes, that way we don't give the user a toast to download their report if they can't. - if (chrome.navLinkExists('kibana:management')) { - const managementUrl = chrome.getNavLinkById('kibana:management').url; + // In certain scenarios (like dashboard only mode) the management app is not accessible, which + // we detect by checking that the navLink exists and is not hidden + const navLinks = await chrome.navLinks.get$().pipe(take(1)).toPromise(); + const managementLink = navLinks.find(link => link.id === 'kibana:management'); + if (managementLink && !managementLink.hidden) { + const managementUrl = managementLink.url; const reportingSectionUrl = `${managementUrl}/kibana/reporting`; seeReportLink = (