[Workspace][Feature] Left navigation menu adjustment (opensearch-proj…

…ect#192) * add util function to filter workspace feature by wildcard Signed-off-by: Yulong Ruan <[email protected]> * resolve conflict Signed-off-by: yuye-aws <[email protected]> * update tests and snapshots Signed-off-by: yuye-aws <[email protected]> * small adjustment to left menu Signed-off-by: yuye-aws <[email protected]> * resolve git conflict Signed-off-by: yuye-aws <[email protected]> * rename nav link service function Signed-off-by: yuye-aws <[email protected]> * unit test for workspace plugin.ts Signed-off-by: yuye-aws <[email protected]> * update snapshots Signed-off-by: yuye-aws <[email protected]> * optimize code Signed-off-by: yuye-aws <[email protected]> * optimize code Signed-off-by: yuye-aws <[email protected]> * optimize code Signed-off-by: yuye-aws <[email protected]> * optimize code Signed-off-by: yuye-aws <[email protected]> * optimize code Signed-off-by: yuye-aws <[email protected]> * optimize code Signed-off-by: yuye-aws <[email protected]> --------- Signed-off-by: Yulong Ruan <[email protected]> Signed-off-by: yuye-aws <[email protected]> Co-authored-by: Yulong Ruan <[email protected]>
wanglam · Feb 22, 2024 · 116b5fe · 116b5fe
1 parent d47cdfb
commit 116b5fe
Show file tree

Hide file tree

Showing 22 changed files with 2,012 additions and 1,472 deletions.
diff --git a/src/core/public/chrome/chrome_service.mock.ts b/src/core/public/chrome/chrome_service.mock.ts
@@ -43,7 +43,9 @@ const createStartContractMock = () => {
   const startContract: DeeplyMockedKeys<InternalChromeStart> = {
     getHeaderComponent: jest.fn(),
     navLinks: {
+      setNavLinks: jest.fn(),
       getNavLinks$: jest.fn(),
+      getAllNavLinks$: jest.fn(),
       has: jest.fn(),
       get: jest.fn(),
       getAll: jest.fn(),

diff --git a/src/core/public/chrome/nav_links/nav_link.ts b/src/core/public/chrome/nav_links/nav_link.ts
@@ -93,15 +93,22 @@ export interface ChromeNavLink {
    * Disables a link from being clickable.
    *
    * @internalRemarks
-   * This is only used by the ML and Graph plugins currently. They use this field
+   * This is used by the ML and Graph plugins. They use this field
    * to disable the nav link when the license is expired.
+   * This is also used by recently visited category in left menu
+   * to disable "No recently visited items".
    */
   readonly disabled?: boolean;
 
   /**
    * Hides a link from the navigation.
    */
   readonly hidden?: boolean;
+
+  /**
+   * Links can be navigated through url.
+   */
+  readonly externalLink?: boolean;
 }
 
 /** @public */

diff --git a/src/core/public/chrome/nav_links/nav_links_service.test.ts b/src/core/public/chrome/nav_links/nav_links_service.test.ts
@@ -32,18 +32,12 @@ import { NavLinksService } from './nav_links_service';
 import { take, map, takeLast } from 'rxjs/operators';
 import { App } from '../../application';
 import { BehaviorSubject } from 'rxjs';
+import { ChromeNavLink } from 'opensearch-dashboards/public';
 
 const availableApps = new Map([
   ['app1', { id: 'app1', order: 0, title: 'App 1', icon: 'app1' }],
-  [
-    'app2',
-    {
-      id: 'app2',
-      order: -10,
-      title: 'App 2',
-      euiIconType: 'canvasApp',
-    },
-  ],
+  ['app2', { id: 'app2', order: -10, title: 'App 2', euiIconType: 'canvasApp' }],
+  ['app3', { id: 'app3', order: 10, title: 'App 3', icon: 'app3' }],
   ['chromelessApp', { id: 'chromelessApp', order: 20, title: 'Chromless App', chromeless: true }],
 ]);
 
@@ -66,7 +60,110 @@ describe('NavLinksService', () => {
     start = service.start({ application: mockAppService, http: mockHttp });
   });
 
-  describe('#getNavLinks$()', () => {
+  describe('#getAllNavLinks$()', () => {
+    it('does not include `chromeless` applications', async () => {
+      expect(
+        await start
+          .getAllNavLinks$()
+          .pipe(
+            take(1),
+            map((links) => links.map((l) => l.id))
+          )
+          .toPromise()
+      ).not.toContain('chromelessApp');
+    });
+
+    it('sorts navLinks by `order` property', async () => {
+      expect(
+        await start
+          .getAllNavLinks$()
+          .pipe(
+            take(1),
+            map((links) => links.map((l) => l.id))
+          )
+          .toPromise()
+      ).toEqual(['app2', 'app1', 'app3']);
+    });
+
+    it('emits multiple values', async () => {
+      const navLinkIds$ = start.getAllNavLinks$().pipe(map((links) => links.map((l) => l.id)));
+      const emittedLinks: string[][] = [];
+      navLinkIds$.subscribe((r) => emittedLinks.push(r));
+      start.update('app1', { href: '/foo' });
+
+      service.stop();
+      expect(emittedLinks).toEqual([
+        ['app2', 'app1', 'app3'],
+        ['app2', 'app1', 'app3'],
+      ]);
+    });
+
+    it('completes when service is stopped', async () => {
+      const last$ = start.getAllNavLinks$().pipe(takeLast(1)).toPromise();
+      service.stop();
+      await expect(last$).resolves.toBeInstanceOf(Array);
+    });
+  });
+
+  describe('#getNavLinks$() when non null', () => {
+    // set filtered nav links, nav link with order smaller than 0 will be filtered
+    beforeEach(() => {
+      const filteredNavLinks = new Map<string, ChromeNavLink>();
+      start.getAllNavLinks$().subscribe((links) =>
+        links.forEach((link) => {
+          if (link.order !== undefined && link.order >= 0) {
+            filteredNavLinks.set(link.id, link);
+          }
+        })
+      );
+      start.setNavLinks(filteredNavLinks);
+    });
+
+    it('does not include `app2` applications', async () => {
+      expect(
+        await start
+          .getNavLinks$()
+          .pipe(
+            take(1),
+            map((links) => links.map((l) => l.id))
+          )
+          .toPromise()
+      ).not.toContain('app2');
+    });
+
+    it('sorts navLinks by `order` property', async () => {
+      expect(
+        await start
+          .getNavLinks$()
+          .pipe(
+            take(1),
+            map((links) => links.map((l) => l.id))
+          )
+          .toPromise()
+      ).toEqual(['app1', 'app3']);
+    });
+
+    it('emits multiple values', async () => {
+      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' });
+
+      service.stop();
+      expect(emittedLinks).toEqual([
+        ['app1', 'app3'],
+        ['app1', 'app3'],
+      ]);
+    });
+
+    it('completes when service is stopped', async () => {
+      const last$ = start.getNavLinks$().pipe(takeLast(1)).toPromise();
+      service.stop();
+      await expect(last$).resolves.toBeInstanceOf(Array);
+    });
+  });
+
+  describe('#getNavLinks$() when null', () => {
     it('does not include `chromeless` applications', async () => {
       expect(
         await start
@@ -79,7 +176,19 @@ describe('NavLinksService', () => {
       ).not.toContain('chromelessApp');
     });
 
-    it('sorts navlinks by `order` property', async () => {
+    it('include `app2` applications', async () => {
+      expect(
+        await start
+          .getNavLinks$()
+          .pipe(
+            take(1),
+            map((links) => links.map((l) => l.id))
+          )
+          .toPromise()
+      ).toContain('app2');
+    });
+
+    it('sorts navLinks by `order` property', async () => {
       expect(
         await start
           .getNavLinks$()
@@ -88,7 +197,7 @@ describe('NavLinksService', () => {
             map((links) => links.map((l) => l.id))
           )
           .toPromise()
-      ).toEqual(['app2', 'app1']);
+      ).toEqual(['app2', 'app1', 'app3']);
     });
 
     it('emits multiple values', async () => {
@@ -99,8 +208,8 @@ describe('NavLinksService', () => {
 
       service.stop();
       expect(emittedLinks).toEqual([
-        ['app2', 'app1'],
-        ['app2', 'app1'],
+        ['app2', 'app1', 'app3'],
+        ['app2', 'app1', 'app3'],
       ]);
     });
 
@@ -123,7 +232,7 @@ describe('NavLinksService', () => {
 
   describe('#getAll()', () => {
     it('returns a sorted array of navlinks', () => {
-      expect(start.getAll().map((l) => l.id)).toEqual(['app2', 'app1']);
+      expect(start.getAll().map((l) => l.id)).toEqual(['app2', 'app1', 'app3']);
     });
   });
 
@@ -148,7 +257,7 @@ describe('NavLinksService', () => {
             map((links) => links.map((l) => l.id))
           )
           .toPromise()
-      ).toEqual(['app2', 'app1']);
+      ).toEqual(['app2', 'app1', 'app3']);
     });
 
     it('does nothing on chromeless applications', async () => {
@@ -161,7 +270,7 @@ describe('NavLinksService', () => {
             map((links) => links.map((l) => l.id))
           )
           .toPromise()
-      ).toEqual(['app2', 'app1']);
+      ).toEqual(['app2', 'app1', 'app3']);
     });
 
     it('removes all other links', async () => {

diff --git a/src/core/public/chrome/nav_links/nav_links_service.ts b/src/core/public/chrome/nav_links/nav_links_service.ts
@@ -53,6 +53,16 @@ export interface ChromeNavLinks {
    */
   getNavLinks$(): Observable<Array<Readonly<ChromeNavLink>>>;
 
+  /**
+   * Get an observable for a sorted list of all navlinks.
+   */
+  getAllNavLinks$(): Observable<Array<Readonly<ChromeNavLink>>>;
+
+  /**
+   * Set navlinks.
+   */
+  setNavLinks(navLinks: ReadonlyMap<string, ChromeNavLink>): void;
+
   /**
    * Get the state of a navlink at this point in time.
    * @param id
@@ -132,36 +142,52 @@ export class NavLinksService {
     // manual link modifications to be able to re-apply then after every
     // availableApps$ changes.
     const linkUpdaters$ = new BehaviorSubject<LinksUpdater[]>([]);
-    const navLinks$ = new BehaviorSubject<ReadonlyMap<string, NavLinkWrapper>>(new Map());
+    const displayedNavLinks$ = new BehaviorSubject<ReadonlyMap<string, ChromeNavLink> | undefined>(
+      undefined
+    );
+    const allNavLinks$ = new BehaviorSubject<ReadonlyMap<string, NavLinkWrapper>>(new Map());
 
     combineLatest([appLinks$, linkUpdaters$])
       .pipe(
         map(([appLinks, linkUpdaters]) => {
           return linkUpdaters.reduce((links, updater) => updater(links), appLinks);
         })
       )
-      .subscribe((navlinks) => {
-        navLinks$.next(navlinks);
+      .subscribe((navLinks) => {
+        allNavLinks$.next(navLinks);
       });
 
     const forceAppSwitcherNavigation$ = new BehaviorSubject(false);
 
     return {
       getNavLinks$: () => {
-        return navLinks$.pipe(map(sortNavLinks), takeUntil(this.stop$));
+        return combineLatest([allNavLinks$, displayedNavLinks$]).pipe(
+          map(([allNavLinks, displayedNavLinks]) =>
+            displayedNavLinks === undefined ? sortLinks(allNavLinks) : sortLinks(displayedNavLinks)
+          ),
+          takeUntil(this.stop$)
+        );
+      },
+
+      setNavLinks: (navLinks: ReadonlyMap<string, ChromeNavLink>) => {
+        displayedNavLinks$.next(navLinks);
+      },
+
+      getAllNavLinks$: () => {
+        return allNavLinks$.pipe(map(sortLinks), takeUntil(this.stop$));
       },
 
       get(id: string) {
-        const link = navLinks$.value.get(id);
+        const link = allNavLinks$.value.get(id);
         return link && link.properties;
       },
 
       getAll() {
-        return sortNavLinks(navLinks$.value);
+        return sortLinks(allNavLinks$.value);
       },
 
       has(id: string) {
-        return navLinks$.value.has(id);
+        return allNavLinks$.value.has(id);
       },
 
       showOnly(id: string) {
@@ -209,9 +235,9 @@ export class NavLinksService {
   }
 }
 
-function sortNavLinks(navLinks: ReadonlyMap<string, NavLinkWrapper>) {
+function sortLinks(links: ReadonlyMap<string, NavLinkWrapper | ChromeNavLink>) {
   return sortBy(
-    [...navLinks.values()].map((link) => link.properties),
+    [...links.values()].map((link) => ('properties' in link ? link.properties : link)),
     'order'
   );
 }