Skip to content

Commit

Permalink
Remove chrome.navLinks.update (elastic#99633)
Browse files Browse the repository at this point in the history
  • Loading branch information
joshdover authored May 24, 2021
1 parent 9793a8f commit c9f7ab3
Show file tree
Hide file tree
Showing 19 changed files with 50 additions and 203 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,4 @@ export interface ChromeNavLinks
| [getNavLinks$()](./kibana-plugin-core-public.chromenavlinks.getnavlinks_.md) | Get an observable for a sorted list of navlinks. |
| [has(id)](./kibana-plugin-core-public.chromenavlinks.has.md) | Check whether or not a navlink exists. |
| [showOnly(id)](./kibana-plugin-core-public.chromenavlinks.showonly.md) | Remove all navlinks except the one matching the given id. |
| [update(id, values)](./kibana-plugin-core-public.chromenavlinks.update.md) | Update the navlink for the given id with the updated attributes. Returns the updated navlink or <code>undefined</code> if it does not exist. |

This file was deleted.

This file was deleted.

1 change: 0 additions & 1 deletion docs/development/core/public/kibana-plugin-core-public.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ The plugin integrates with the core system via lifecycle events: `setup`<!-- -->
| [ChromeBreadcrumb](./kibana-plugin-core-public.chromebreadcrumb.md) | |
| [ChromeHelpExtensionLinkBase](./kibana-plugin-core-public.chromehelpextensionlinkbase.md) | |
| [ChromeHelpExtensionMenuLink](./kibana-plugin-core-public.chromehelpextensionmenulink.md) | |
| [ChromeNavLinkUpdateableFields](./kibana-plugin-core-public.chromenavlinkupdateablefields.md) | |
| [FatalErrorsStart](./kibana-plugin-core-public.fatalerrorsstart.md) | FatalErrors stop the Kibana Public Core and displays a fatal error screen with details about the Kibana build and the error. |
| [HttpStart](./kibana-plugin-core-public.httpstart.md) | See [HttpSetup](./kibana-plugin-core-public.httpsetup.md) |
| [IToasts](./kibana-plugin-core-public.itoasts.md) | Methods for adding and removing global toast messages. See [ToastsApi](./kibana-plugin-core-public.toastsapi.md)<!-- -->. |
Expand Down
1 change: 0 additions & 1 deletion src/core/public/chrome/chrome_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const createStartContractMock = () => {
get: jest.fn(),
getAll: jest.fn(),
showOnly: jest.fn(),
update: jest.fn(),
enableForcedAppSwitcherNavigation: jest.fn(),
getForceAppSwitcherNavigation$: jest.fn(),
},
Expand Down
2 changes: 1 addition & 1 deletion src/core/public/chrome/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export type {
ChromeHelpExtensionMenuGitHubLink,
} from './ui/header/header_help_menu';
export type { NavType } from './ui';
export type { ChromeNavLink, ChromeNavLinks, ChromeNavLinkUpdateableFields } from './nav_links';
export type { ChromeNavLink, ChromeNavLinks } from './nav_links';
export type {
ChromeRecentlyAccessed,
ChromeRecentlyAccessedHistoryItem,
Expand Down
2 changes: 1 addition & 1 deletion src/core/public/chrome/nav_links/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@
*/
export { NavLinksService } from './nav_links_service';

export type { ChromeNavLink, ChromeNavLinkUpdateableFields } from './nav_link';
export type { ChromeNavLink } from './nav_link';
export type { ChromeNavLinks } from './nav_links_service';
12 changes: 0 additions & 12 deletions src/core/public/chrome/nav_links/nav_link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* Side Public License, v 1.
*/

import { pick } from '@kbn/std';
import { AppCategory } from '../../';

/**
Expand Down Expand Up @@ -81,11 +80,6 @@ export interface ChromeNavLink {
readonly hidden?: boolean;
}

/** @public */
export type ChromeNavLinkUpdateableFields = Partial<
Pick<ChromeNavLink, 'disabled' | 'hidden' | 'url' | 'href'>
>;

export class NavLinkWrapper {
public readonly id: string;
public readonly properties: Readonly<ChromeNavLink>;
Expand All @@ -98,10 +92,4 @@ export class NavLinkWrapper {
this.id = properties.id;
this.properties = Object.freeze(properties);
}

public update(newProps: ChromeNavLinkUpdateableFields) {
// Enforce limited properties at runtime for JS code
newProps = pick(newProps, ['disabled', 'hidden', 'url', 'href']);
return new NavLinkWrapper({ ...this.properties, ...newProps });
}
}
46 changes: 2 additions & 44 deletions src/core/public/chrome/nav_links/nav_links_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,10 @@ describe('NavLinksService', () => {
const navLinkIds$ = start.getNavLinks$().pipe(map((links) => links.map((l) => l.id)));
const emittedLinks: string[][] = [];
navLinkIds$.subscribe((r) => emittedLinks.push(r));
start.update('app1', { href: '/foo' });
start.showOnly('app1');

service.stop();
expect(emittedLinks).toEqual([
['app2', 'app1'],
['app2', 'app1'],
]);
expect(emittedLinks).toEqual([['app2', 'app1'], ['app1']]);
});

it('completes when service is stopped', async () => {
Expand Down Expand Up @@ -170,45 +167,6 @@ describe('NavLinksService', () => {
});
});

describe('#update()', () => {
it('updates the navlinks and returns the updated link', async () => {
expect(start.update('app2', { hidden: true })).toEqual(
expect.objectContaining({
hidden: true,
id: 'app2',
order: -10,
title: 'App 2',
euiIconType: 'canvasApp',
})
);
const hiddenLinkIds = await start
.getNavLinks$()
.pipe(
take(1),
map((links) => links.filter((l) => l.hidden).map((l) => l.id))
)
.toPromise();
expect(hiddenLinkIds).toEqual(['app2']);
});

it('returns undefined if link does not exist', () => {
expect(start.update('fake', { hidden: true })).toBeUndefined();
});

it('keeps the updated link when availableApps are re-emitted', async () => {
start.update('app2', { hidden: true });
mockAppService.applications$.next(mockAppService.applications$.value);
const hiddenLinkIds = await start
.getNavLinks$()
.pipe(
take(1),
map((links) => links.filter((l) => l.hidden).map((l) => l.id))
)
.toPromise();
expect(hiddenLinkIds).toEqual(['app2']);
});
});

describe('#enableForcedAppSwitcherNavigation()', () => {
it('flips #getForceAppSwitcherNavigation$()', async () => {
await expect(start.getForceAppSwitcherNavigation$().pipe(take(1)).toPromise()).resolves.toBe(
Expand Down
34 changes: 2 additions & 32 deletions src/core/public/chrome/nav_links/nav_links_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { map, takeUntil } from 'rxjs/operators';

import { InternalApplicationStart } from '../../application';
import { HttpStart } from '../../http';
import { ChromeNavLink, ChromeNavLinkUpdateableFields, NavLinkWrapper } from './nav_link';
import { ChromeNavLink, NavLinkWrapper } from './nav_link';
import { toNavLink } from './to_nav_link';

interface StartDeps {
Expand Down Expand Up @@ -58,18 +58,6 @@ export interface ChromeNavLinks {
*/
showOnly(id: string): void;

/**
* Update the navlink for the given id with the updated attributes.
* Returns the updated navlink or `undefined` if it does not exist.
*
* @deprecated Uses the {@link AppBase.updater$} property when registering
* your application with {@link ApplicationSetup.register} instead.
*
* @param id
* @param values
*/
update(id: string, values: ChromeNavLinkUpdateableFields): ChromeNavLink | undefined;

/**
* Enable forced navigation mode, which will trigger a page refresh
* when a nav link is clicked and only the hash is updated.
Expand Down Expand Up @@ -109,6 +97,7 @@ export class NavLinksService {
// now that availableApps$ is an observable, we need to keep record of all
// manual link modifications to be able to re-apply then after every
// availableApps$ changes.
// Only in use by `showOnly` API, can be removed once dashboard_mode is removed in 8.0
const linkUpdaters$ = new BehaviorSubject<LinksUpdater[]>([]);
const navLinks$ = new BehaviorSubject<ReadonlyMap<string, NavLinkWrapper>>(new Map());

Expand Down Expand Up @@ -153,25 +142,6 @@ export class NavLinksService {
linkUpdaters$.next([...linkUpdaters$.value, updater]);
},

update(id: string, values: ChromeNavLinkUpdateableFields) {
if (!this.has(id)) {
return;
}

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

linkUpdaters$.next([...linkUpdaters$.value, updater]);
return this.get(id);
},

enableForcedAppSwitcherNavigation() {
forceAppSwitcherNavigation$.next(true);
},
Expand Down
2 changes: 0 additions & 2 deletions src/core/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import {
ChromeNavControls,
ChromeNavLink,
ChromeNavLinks,
ChromeNavLinkUpdateableFields,
ChromeDocTitle,
ChromeStart,
ChromeRecentlyAccessed,
Expand Down Expand Up @@ -298,7 +297,6 @@ export type {
ChromeNavControls,
ChromeNavLink,
ChromeNavLinks,
ChromeNavLinkUpdateableFields,
ChromeDocTitle,
ChromeRecentlyAccessed,
ChromeRecentlyAccessedHistoryItem,
Expand Down
7 changes: 0 additions & 7 deletions src/core/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -332,15 +332,8 @@ export interface ChromeNavLinks {
getNavLinks$(): Observable<Array<Readonly<ChromeNavLink>>>;
has(id: string): boolean;
showOnly(id: string): void;
// Warning: (ae-unresolved-link) The @link reference could not be resolved: The package "kibana" does not have an export "AppBase"
//
// @deprecated
update(id: string, values: ChromeNavLinkUpdateableFields): ChromeNavLink | undefined;
}

// @public (undocumented)
export type ChromeNavLinkUpdateableFields = Partial<Pick<ChromeNavLink, 'disabled' | 'hidden' | 'url' | 'href'>>;

// @public
export interface ChromeRecentlyAccessed {
// Warning: (ae-unresolved-link) The @link reference could not be resolved: No member was found with name "basePath"
Expand Down
9 changes: 5 additions & 4 deletions x-pack/plugins/apm/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { i18n } from '@kbn/i18n';
import type { ConfigSchema } from '.';
import {
AppMountParameters,
AppNavLinkStatus,
CoreSetup,
CoreStart,
DEFAULT_APP_CATEGORIES,
Expand Down Expand Up @@ -40,7 +41,6 @@ import type {
} from '../../triggers_actions_ui/public';
import { registerApmAlerts } from './components/alerting/register_apm_alerts';
import { featureCatalogueEntry } from './featureCatalogueEntry';
import { toggleAppLinkInNav } from './toggleAppLinkInNav';

export type ApmPluginSetup = ReturnType<ApmPlugin['setup']>;

Expand Down Expand Up @@ -193,6 +193,9 @@ export class ApmPlugin implements Plugin<ApmPluginSetup, ApmPluginStart> {
order: 8500,
euiIconType: 'logoObservability',
category: DEFAULT_APP_CATEGORIES.observability,
navLinkStatus: config.ui.enabled
? AppNavLinkStatus.default
: AppNavLinkStatus.hidden,
meta: {
keywords: [
'RUM',
Expand Down Expand Up @@ -231,7 +234,5 @@ export class ApmPlugin implements Plugin<ApmPluginSetup, ApmPluginStart> {

return {};
}
public start(core: CoreStart, plugins: ApmPluginStartDeps) {
toggleAppLinkInNav(core, this.initializerContext.config.get());
}
public start(core: CoreStart, plugins: ApmPluginStartDeps) {}
}
15 changes: 0 additions & 15 deletions x-pack/plugins/apm/public/toggleAppLinkInNav.ts

This file was deleted.

7 changes: 3 additions & 4 deletions x-pack/plugins/graph/common/check_license.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export function assertNever(x: never): never {
export interface GraphLicenseInformation {
showAppLink: boolean;
enableAppLink: boolean;
message: string;
message?: string;
}

export function checkLicense(license: ILicense | undefined): GraphLicenseInformation {
Expand Down Expand Up @@ -53,20 +53,19 @@ export function checkLicense(license: ILicense | undefined): GraphLicenseInforma
return {
showAppLink: true,
enableAppLink: false,
message: check.message || '',
message: check.message,
};
case 'invalid':
case 'unavailable':
return {
showAppLink: false,
enableAppLink: false,
message: check.message || '',
message: check.message,
};
case 'valid':
return {
showAppLink: true,
enableAppLink: true,
message: '',
};
default:
return assertNever(check.state);
Expand Down
5 changes: 4 additions & 1 deletion x-pack/plugins/graph/public/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,10 @@ export const renderApp = ({ appBasePath, element, kibanaLegacy, ...deps }: Graph
const licenseAllowsToShowThisPage = info.showAppLink && info.enableAppLink;

if (!licenseAllowsToShowThisPage) {
deps.core.notifications.toasts.addDanger(info.message);
if (info.message) {
deps.core.notifications.toasts.addDanger(info.message);
}

// This has to happen in the next tick because otherwise the original navigation
// bringing us to the graph app in the first place
// never completes and the browser enters are redirect loop
Expand Down
Loading

0 comments on commit c9f7ab3

Please sign in to comment.