[7.x] Use app id instead of pluginId to generate navlink from legacy apps (#57542) (#57670)

* Use app id instead of pluginId to generate navlink from legacy apps (#57542)

* properly use app id instead of pluginId to generate navlink

* extract convertToNavLink, add more tests

* use distinct mapping methods

* fix linkToLastSubUrl default value

* nits & doc

* update generated doc

docs/development/core/public/kibana-plugin-public.ihttpfetcherror.name.md

@@ -0,0 +1,11 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [IHttpFetchError](./kibana-plugin-public.ihttpfetcherror.md) &gt; [name](./kibana-plugin-public.ihttpfetcherror.name.md)
+
+## IHttpFetchError.name property
+
+<b>Signature:</b>
+
+```typescript
+readonly name: string;
+```

src/core/server/legacy/plugins/__snapshots__/get_nav_links.test.ts.snap

@@ -0,0 +1,56 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[` 1`] = `
+Array [
+  Object {
+    "category": undefined,
+    "disableSubUrlTracking": undefined,
+    "disabled": false,
+    "euiIconType": undefined,
+    "hidden": false,
+    "icon": undefined,
+    "id": "link-a",
+    "linkToLastSubUrl": true,
+    "order": 0,
+    "subUrlBase": "/some-custom-url",
+    "title": "AppA",
+    "tooltip": "",
+    "url": "/some-custom-url",
+  },
+  Object {
+    "category": undefined,
+    "disableSubUrlTracking": true,
+    "disabled": false,
+    "euiIconType": undefined,
+    "hidden": false,
+    "icon": undefined,
+    "id": "link-b",
+    "linkToLastSubUrl": true,
+    "order": 0,
+    "subUrlBase": "/url-b",
+    "title": "AppB",
+    "tooltip": "",
+    "url": "/url-b",
+  },
+  Object {
+    "category": undefined,
+    "euiIconType": undefined,
+    "icon": undefined,
+    "id": "app-a",
+    "linkToLastSubUrl": true,
+    "order": 0,
+    "title": "AppA",
+    "url": "/app/app-a",
+  },
+  Object {
+    "category": undefined,
+    "euiIconType": undefined,
+    "icon": undefined,
+    "id": "app-b",
+    "linkToLastSubUrl": true,
+    "order": 0,
+    "title": "AppB",
+    "url": "/app/app-b",
+  },
+]
+`;

src/core/server/legacy/plugins/find_legacy_plugin_specs.ts

@@ -30,72 +30,8 @@ import { collectUiExports as collectLegacyUiExports } from '../../../../legacy/u
 
 import { LoggerFactory } from '../../logging';
 import { PackageInfo } from '../../config';
-
-import {
-  LegacyUiExports,
-  LegacyNavLink,
-  LegacyPluginSpec,
-  LegacyPluginPack,
-  LegacyConfig,
-} from '../types';
-
-const REMOVE_FROM_ARRAY: LegacyNavLink[] = [];
-
-function getUiAppsNavLinks({ uiAppSpecs = [] }: LegacyUiExports, pluginSpecs: LegacyPluginSpec[]) {
-  return uiAppSpecs.flatMap(spec => {
-    if (!spec) {
-      return REMOVE_FROM_ARRAY;
-    }
-
-    const id = spec.pluginId || spec.id;
-
-    if (!id) {
-      throw new Error('Every app must specify an id');
-    }
-
-    if (spec.pluginId && !pluginSpecs.some(plugin => plugin.getId() === spec.pluginId)) {
-      throw new Error(`Unknown plugin id "${spec.pluginId}"`);
-    }
-
-    const listed = typeof spec.listed === 'boolean' ? spec.listed : true;
-
-    if (spec.hidden || !listed) {
-      return REMOVE_FROM_ARRAY;
-    }
-
-    return {
-      id,
-      category: spec.category,
-      title: spec.title,
-      order: typeof spec.order === 'number' ? spec.order : 0,
-      icon: spec.icon,
-      euiIconType: spec.euiIconType,
-      url: spec.url || `/app/${id}`,
-      linkToLastSubUrl: spec.linkToLastSubUrl,
-    };
-  });
-}
-
-function getNavLinks(uiExports: LegacyUiExports, pluginSpecs: LegacyPluginSpec[]) {
-  return (uiExports.navLinkSpecs || [])
-    .map<LegacyNavLink>(spec => ({
-      id: spec.id,
-      category: spec.category,
-      title: spec.title,
-      order: typeof spec.order === 'number' ? spec.order : 0,
-      url: spec.url,
-      subUrlBase: spec.subUrlBase || spec.url,
-      disableSubUrlTracking: spec.disableSubUrlTracking,
-      icon: spec.icon,
-      euiIconType: spec.euiIconType,
-      linkToLastSub: 'linkToLastSubUrl' in spec ? spec.linkToLastSubUrl : false,
-      hidden: 'hidden' in spec ? spec.hidden : false,
-      disabled: 'disabled' in spec ? spec.disabled : false,
-      tooltip: spec.tooltip || '',
-    }))
-    .concat(getUiAppsNavLinks(uiExports, pluginSpecs))
-    .sort((a, b) => a.order - b.order);
-}
+import { LegacyPluginSpec, LegacyPluginPack, LegacyConfig } from '../types';
+import { getNavLinks } from './get_nav_links';
 
 export async function findLegacyPluginSpecs(
   settings: unknown,

src/core/server/legacy/plugins/get_nav_links.test.ts

@@ -0,0 +1,283 @@
+/*
+ * 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 { LegacyUiExports, LegacyPluginSpec, LegacyAppSpec, LegacyNavLinkSpec } from '../types';
+import { getNavLinks } from './get_nav_links';
+
+const createLegacyExports = ({
+  uiAppSpecs = [],
+  navLinkSpecs = [],
+}: {
+  uiAppSpecs?: LegacyAppSpec[];
+  navLinkSpecs?: LegacyNavLinkSpec[];
+}): LegacyUiExports => ({
+  uiAppSpecs,
+  navLinkSpecs,
+  injectedVarsReplacers: [],
+  defaultInjectedVarProviders: [],
+  savedObjectMappings: [],
+  savedObjectSchemas: {},
+  savedObjectMigrations: {},
+  savedObjectValidations: {},
+});
+
+const createPluginSpecs = (...ids: string[]): LegacyPluginSpec[] =>
+  ids.map(
+    id =>
+      ({
+        getId: () => id,
+      } as LegacyPluginSpec)
+  );
+
+describe('getNavLinks', () => {
+  describe('generating from uiAppSpecs', () => {
+    it('generates navlinks from legacy app specs', () => {
+      const navlinks = getNavLinks(
+        createLegacyExports({
+          uiAppSpecs: [
+            {
+              id: 'app-a',
+              title: 'AppA',
+              pluginId: 'pluginA',
+            },
+            {
+              id: 'app-b',
+              title: 'AppB',
+              pluginId: 'pluginA',
+            },
+          ],
+        }),
+        createPluginSpecs('pluginA')
+      );
+
+      expect(navlinks.length).toEqual(2);
+      expect(navlinks[0]).toEqual(
+        expect.objectContaining({
+          id: 'app-a',
+          title: 'AppA',
+          url: '/app/app-a',
+        })
+      );
+      expect(navlinks[1]).toEqual(
+        expect.objectContaining({
+          id: 'app-b',
+          title: 'AppB',
+          url: '/app/app-b',
+        })
+      );
+    });
+
+    it('uses the app id to generates the navlink id even if pluginId is specified', () => {
+      const navlinks = getNavLinks(
+        createLegacyExports({
+          uiAppSpecs: [
+            {
+              id: 'app-a',
+              title: 'AppA',
+              pluginId: 'pluginA',
+            },
+            {
+              id: 'app-b',
+              title: 'AppB',
+              pluginId: 'pluginA',
+            },
+          ],
+        }),
+        createPluginSpecs('pluginA')
+      );
+
+      expect(navlinks.length).toEqual(2);
+      expect(navlinks[0].id).toEqual('app-a');
+      expect(navlinks[1].id).toEqual('app-b');
+    });
+
+    it('throws if an app reference a missing plugin', () => {
+      expect(() => {
+        getNavLinks(
+          createLegacyExports({
+            uiAppSpecs: [
+              {
+                id: 'app-a',
+                title: 'AppA',
+                pluginId: 'notExistingPlugin',
+              },
+            ],
+          }),
+          createPluginSpecs('pluginA')
+        );
+      }).toThrowErrorMatchingInlineSnapshot(`"Unknown plugin id \\"notExistingPlugin\\""`);
+    });
+
+    it('uses all known properties of the navlink', () => {
+      const navlinks = getNavLinks(
+        createLegacyExports({
+          uiAppSpecs: [
+            {
+              id: 'app-a',
+              title: 'AppA',
+              category: {
+                label: 'My Category',
+              },
+              order: 42,
+              url: '/some-custom-url',
+              icon: 'fa-snowflake',
+              euiIconType: 'euiIcon',
+              linkToLastSubUrl: true,
+              hidden: false,
+            },
+          ],
+        }),
+        []
+      );
+      expect(navlinks.length).toBe(1);
+      expect(navlinks[0]).toEqual({
+        id: 'app-a',
+        title: 'AppA',
+        category: {
+          label: 'My Category',
+        },
+        order: 42,
+        url: '/some-custom-url',
+        icon: 'fa-snowflake',
+        euiIconType: 'euiIcon',
+        linkToLastSubUrl: true,
+      });
+    });
+  });
+
+  describe('generating from navLinkSpecs', () => {
+    it('generates navlinks from legacy navLink specs', () => {
+      const navlinks = getNavLinks(
+        createLegacyExports({
+          navLinkSpecs: [
+            {
+              id: 'link-a',
+              title: 'AppA',
+              url: '/some-custom-url',
+            },
+            {
+              id: 'link-b',
+              title: 'AppB',
+              url: '/some-other-url',
+              disableSubUrlTracking: true,
+            },
+          ],
+        }),
+        createPluginSpecs('pluginA')
+      );
+
+      expect(navlinks.length).toEqual(2);
+      expect(navlinks[0]).toEqual(
+        expect.objectContaining({
+          id: 'link-a',
+          title: 'AppA',
+          url: '/some-custom-url',
+          hidden: false,
+          disabled: false,
+        })
+      );
+      expect(navlinks[1]).toEqual(
+        expect.objectContaining({
+          id: 'link-b',
+          title: 'AppB',
+          url: '/some-other-url',
+          disableSubUrlTracking: true,
+        })
+      );
+    });
+
+    it('only uses known properties to create the navlink', () => {
+      const navlinks = getNavLinks(
+        createLegacyExports({
+          navLinkSpecs: [
+            {
+              id: 'link-a',
+              title: 'AppA',
+              category: {
+                label: 'My Second Cat',
+              },
+              order: 72,
+              url: '/some-other-custom',
+              subUrlBase: '/some-other-custom/sub',
+              disableSubUrlTracking: true,
+              icon: 'fa-corn',
+              euiIconType: 'euiIconBis',
+              linkToLastSubUrl: false,
+              hidden: false,
+              tooltip: 'My other tooltip',
+            },
+          ],
+        }),
+        []
+      );
+      expect(navlinks.length).toBe(1);
+      expect(navlinks[0]).toEqual({
+        id: 'link-a',
+        title: 'AppA',
+        category: {
+          label: 'My Second Cat',
+        },
+        order: 72,
+        url: '/some-other-custom',
+        subUrlBase: '/some-other-custom/sub',
+        disableSubUrlTracking: true,
+        icon: 'fa-corn',
+        euiIconType: 'euiIconBis',
+        linkToLastSubUrl: false,
+        hidden: false,
+        disabled: false,
+        tooltip: 'My other tooltip',
+      });
+    });
+  });
+
+  describe('generating from both apps and navlinks', () => {
+    const navlinks = getNavLinks(
+      createLegacyExports({
+        uiAppSpecs: [
+          {
+            id: 'app-a',
+            title: 'AppA',
+          },
+          {
+            id: 'app-b',
+            title: 'AppB',
+          },
+        ],
+        navLinkSpecs: [
+          {
+            id: 'link-a',
+            title: 'AppA',
+            url: '/some-custom-url',
+          },
+          {
+            id: 'link-b',
+            title: 'AppB',
+            url: '/url-b',
+            disableSubUrlTracking: true,
+          },
+        ],
+      }),
+      []
+    );
+
+    expect(navlinks.length).toBe(4);
+    expect(navlinks).toMatchSnapshot();
+  });
+});

src/core/server/legacy/plugins/get_nav_links.ts

@@ -0,0 +1,82 @@
+/*
+ * 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 {
+  LegacyUiExports,
+  LegacyNavLink,
+  LegacyPluginSpec,
+  LegacyNavLinkSpec,
+  LegacyAppSpec,
+} from '../types';
+
+function legacyAppToNavLink(spec: LegacyAppSpec): LegacyNavLink {
+  if (!spec.id) {
+    throw new Error('Every app must specify an id');
+  }
+  return {
+    id: spec.id,
+    category: spec.category,
+    title: spec.title ?? spec.id,
+    order: typeof spec.order === 'number' ? spec.order : 0,
+    icon: spec.icon,
+    euiIconType: spec.euiIconType,
+    url: spec.url || `/app/${spec.id}`,
+    linkToLastSubUrl: spec.linkToLastSubUrl ?? true,
+  };
+}
+
+function legacyLinkToNavLink(spec: LegacyNavLinkSpec): LegacyNavLink {
+  return {
+    id: spec.id,
+    category: spec.category,
+    title: spec.title,
+    order: typeof spec.order === 'number' ? spec.order : 0,
+    url: spec.url,
+    subUrlBase: spec.subUrlBase || spec.url,
+    disableSubUrlTracking: spec.disableSubUrlTracking,
+    icon: spec.icon,
+    euiIconType: spec.euiIconType,
+    linkToLastSubUrl: spec.linkToLastSubUrl ?? true,
+    hidden: spec.hidden ?? false,
+    disabled: spec.disabled ?? false,
+    tooltip: spec.tooltip ?? '',
+  };
+}
+
+function isHidden(app: LegacyAppSpec) {
+  return app.listed === false || app.hidden === true;
+}
+
+export function getNavLinks(uiExports: LegacyUiExports, pluginSpecs: LegacyPluginSpec[]) {
+  const navLinkSpecs = uiExports.navLinkSpecs || [];
+  const appSpecs = (uiExports.uiAppSpecs || []).filter(
+    app => app !== undefined && !isHidden(app)
+  ) as LegacyAppSpec[];
+
+  const pluginIds = (pluginSpecs || []).map(spec => spec.getId());
+  appSpecs.forEach(spec => {
+    if (spec.pluginId && !pluginIds.includes(spec.pluginId)) {
+      throw new Error(`Unknown plugin id "${spec.pluginId}"`);
+    }
+  });
+
+  return [...navLinkSpecs.map(legacyLinkToNavLink), ...appSpecs.map(legacyAppToNavLink)].sort(
+    (a, b) => a.order - b.order
+  );
+}

src/core/server/legacy/types.ts

@@ -131,16 +131,20 @@ export type VarsReplacer = (
  * @internal
  * @deprecated
  */
-export type LegacyNavLinkSpec = Record<string, unknown> & ChromeNavLink;
+export type LegacyNavLinkSpec = Partial<LegacyNavLink> & {
+  id: string;
+  title: string;
+  url: string;
+};
 
 /**
  * @internal
  * @deprecated
  */
-export type LegacyAppSpec = Pick<
-  ChromeNavLink,
-  'title' | 'order' | 'icon' | 'euiIconType' | 'url' | 'linkToLastSubUrl' | 'hidden' | 'category'
-> & { pluginId?: string; id?: string; listed?: boolean };
+export type LegacyAppSpec = Partial<LegacyNavLink> & {
+  pluginId?: string;
+  listed?: boolean;
+};
 
 /**
  * @internal

src/core/server/server.api.md

@@ -2125,11 +2125,11 @@ export const validBodyOutput: readonly ["data", "stream"];
 // Warnings were encountered during analysis:
 //
 // src/core/server/http/router/response.ts:316:3 - (ae-forgotten-export) The symbol "KibanaResponse" needs to be exported by the entry point index.d.ts
-// src/core/server/legacy/types.ts:158:3 - (ae-forgotten-export) The symbol "VarsProvider" needs to be exported by the entry point index.d.ts
-// src/core/server/legacy/types.ts:159:3 - (ae-forgotten-export) The symbol "VarsReplacer" needs to be exported by the entry point index.d.ts
-// src/core/server/legacy/types.ts:160:3 - (ae-forgotten-export) The symbol "LegacyNavLinkSpec" needs to be exported by the entry point index.d.ts
-// src/core/server/legacy/types.ts:161:3 - (ae-forgotten-export) The symbol "LegacyAppSpec" needs to be exported by the entry point index.d.ts
-// src/core/server/legacy/types.ts:162:16 - (ae-forgotten-export) The symbol "LegacyPluginSpec" needs to be exported by the entry point index.d.ts
+// src/core/server/legacy/types.ts:162:3 - (ae-forgotten-export) The symbol "VarsProvider" needs to be exported by the entry point index.d.ts
+// src/core/server/legacy/types.ts:163:3 - (ae-forgotten-export) The symbol "VarsReplacer" needs to be exported by the entry point index.d.ts
+// src/core/server/legacy/types.ts:164:3 - (ae-forgotten-export) The symbol "LegacyNavLinkSpec" needs to be exported by the entry point index.d.ts
+// src/core/server/legacy/types.ts:165:3 - (ae-forgotten-export) The symbol "LegacyAppSpec" needs to be exported by the entry point index.d.ts
+// src/core/server/legacy/types.ts:166:16 - (ae-forgotten-export) The symbol "LegacyPluginSpec" needs to be exported by the entry point index.d.ts
 // src/core/server/plugins/plugins_service.ts:43:5 - (ae-forgotten-export) The symbol "InternalPluginInfo" needs to be exported by the entry point index.d.ts
 // src/core/server/plugins/types.ts:226:3 - (ae-forgotten-export) The symbol "KibanaConfigType" needs to be exported by the entry point index.d.ts
 // src/core/server/plugins/types.ts:226:3 - (ae-forgotten-export) The symbol "SharedGlobalConfigKeys" needs to be exported by the entry point index.d.ts