Fix the display and jump logic for recent assets (opensearch-project#…

…8136) * Hide global objects in recent assets when workspace enabled, and jump to the global for workspace objects hwen workspace disabled Signed-off-by: Kapian1234 <[email protected]> * Changeset file for PR opensearch-project#8136 created/updated * Update recently-accessed-service Signed-off-by: Kapian1234 <[email protected]> * fix the boolean value Signed-off-by: Kapian1234 <[email protected]> * add unit test and fix some bugs Signed-off-by: Kapian1234 <[email protected]> * rename test cases Signed-off-by: Kapian1234 <[email protected]> * Pass workspaceEnabled to createRecentNavLink Signed-off-by: Kapian1234 <[email protected]> * Update unit tests Signed-off-by: Kapian1234 <[email protected]> * Remove workspaceEnabled props in recent_items Signed-off-by: Kapian1234 <[email protected]> * Fix links at recent accessed items Signed-off-by: Kapian1234 <[email protected]> * Fix unit tests Signed-off-by: Kapian1234 <[email protected]> * Update snapshots Signed-off-by: Kapian1234 <[email protected]> * Modify unit tests Signed-off-by: Kapian1234 <[email protected]> --------- Signed-off-by: Kapian1234 <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Yulong Ruan <[email protected]> Co-authored-by: SuZhou-Joe <[email protected]>
d-buckner · Jan 17, 2025 · c211e09 · c211e09
1 parent 96dd558
commit c211e09
Show file tree

Hide file tree

Showing 12 changed files with 95 additions and 17 deletions.
diff --git a/changelogs/fragments/8136.yml b/changelogs/fragments/8136.yml
@@ -0,0 +1,2 @@
+fix:
+- Fix the display and jump logic for recent assets ([#8136](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8136))
diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx
@@ -260,7 +260,7 @@ export class ChromeService {
 
     const navControls = this.navControls.start();
     const navLinks = this.navLinks.start({ application, http });
-    const recentlyAccessed = await this.recentlyAccessed.start({ http, workspaces });
+    const recentlyAccessed = await this.recentlyAccessed.start({ http, workspaces, application });
     const docTitle = this.docTitle.start({ document: window.document });
     const navGroup = await this.navGroup.start({
       navLinks,

diff --git a/src/core/public/chrome/recently_accessed/recently_accessed_service.test.ts b/src/core/public/chrome/recently_accessed/recently_accessed_service.test.ts
@@ -65,6 +65,7 @@ class LocalStorageMock implements Storage {
 
 describe('RecentlyAccessed#start()', () => {
   let originalLocalStorage: Storage;
+  let workspaceEnabled = false;
   beforeAll(() => {
     originalLocalStorage = window.localStorage;
 
@@ -78,7 +79,19 @@ describe('RecentlyAccessed#start()', () => {
   const getStart = async () => {
     const http = httpServiceMock.createStartContract();
     const workspaces = workspacesServiceMock.createStartContract();
-    const recentlyAccessed = await new RecentlyAccessedService().start({ http, workspaces });
+    const application = {
+      ...jest.requireActual('../../application'),
+      capabilities: {
+        workspaces: {
+          enabled: workspaceEnabled,
+        },
+      },
+    };
+    const recentlyAccessed = await new RecentlyAccessedService().start({
+      http,
+      workspaces,
+      application,
+    });
     return { http, recentlyAccessed, workspaces };
   };
 
@@ -160,4 +173,23 @@ Array [
       { link: '/app/item1', label: 'Item 1', id: 'item1', workspaceId: 'foo' },
     ]);
   });
+
+  it('should not get objects without related workspace when workspace enabled', async () => {
+    workspaceEnabled = true;
+
+    const { recentlyAccessed } = await getStart();
+    recentlyAccessed.add('/app/item1', 'Item 1', 'item1');
+    expect(recentlyAccessed.get()).toEqual([]);
+  });
+
+  it('should get objects with related workspace when workspace enabled', async () => {
+    workspaceEnabled = true;
+
+    const { recentlyAccessed, workspaces } = await getStart();
+    workspaces.currentWorkspaceId$.next('foo');
+    recentlyAccessed.add('/app/item1', 'Item 1', 'item1');
+    expect(recentlyAccessed.get()).toEqual([
+      { link: '/app/item1', label: 'Item 1', id: 'item1', workspaceId: 'foo' },
+    ]);
+  });
 });
diff --git a/src/core/public/chrome/recently_accessed/recently_accessed_service.ts b/src/core/public/chrome/recently_accessed/recently_accessed_service.ts
@@ -29,11 +29,12 @@
  */
 
 import { Observable } from 'rxjs';
-
+import { map } from 'rxjs/operators';
 import { PersistedLog } from './persisted_log';
 import { createLogKey } from './create_log_key';
 import { HttpSetup } from '../../http';
 import { WorkspacesStart } from '../../workspace';
+import { InternalApplicationStart } from '../../application';
 
 /** @public */
 export interface ChromeRecentlyAccessedHistoryItem {
@@ -50,16 +51,18 @@ export interface ChromeRecentlyAccessedHistoryItem {
 interface StartDeps {
   http: HttpSetup;
   workspaces: WorkspacesStart;
+  application: InternalApplicationStart;
 }
 
 /** @internal */
 export class RecentlyAccessedService {
-  async start({ http, workspaces }: StartDeps): Promise<ChromeRecentlyAccessed> {
+  async start({ http, workspaces, application }: StartDeps): Promise<ChromeRecentlyAccessed> {
     const logKey = await createLogKey('recentlyAccessed', http.basePath.getBasePath());
     const history = new PersistedLog<ChromeRecentlyAccessedHistoryItem>(logKey, {
       maxLength: 20,
       isEqual: (oldItem, newItem) => oldItem.id === newItem.id,
     });
+    const workspaceEnabled = application.capabilities.workspaces.enabled;
 
     return {
       /** Adds a new item to the history. */
@@ -81,10 +84,16 @@ export class RecentlyAccessedService {
       },
 
       /** Gets the current array of history items. */
-      get: () => history.get(),
+      get: () => history.get().filter((item) => (workspaceEnabled ? !!item.workspaceId : true)),
 
       /** Gets an observable of the current array of history items. */
-      get$: () => history.get$(),
+      get$: () => {
+        return history.get$().pipe(
+          map((items) => {
+            return items.filter((item) => (workspaceEnabled ? !!item.workspaceId : true));
+          })
+        );
+      },
     };
   }
 }

diff --git a/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav.test.tsx.snap
diff --git a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap
diff --git a/src/core/public/chrome/ui/header/collapsible_nav.test.tsx b/src/core/public/chrome/ui/header/collapsible_nav.test.tsx
@@ -95,6 +95,7 @@ function mockProps(branding = {}) {
     customNavLink$: new BehaviorSubject(undefined),
     branding,
     logos: getLogos(branding, mockBasePath.serverBasePath),
+    workspaceEnabled: true,
   };
 }
 

diff --git a/src/core/public/chrome/ui/header/collapsible_nav.tsx b/src/core/public/chrome/ui/header/collapsible_nav.tsx
@@ -90,6 +90,7 @@ interface Props {
   navigateToUrl: InternalApplicationStart['navigateToUrl'];
   customNavLink$: Rx.Observable<ChromeNavLink | undefined>;
   logos: Logos;
+  workspaceEnabled: boolean | undefined;
 }
 
 export function CollapsibleNav({
@@ -105,6 +106,7 @@ export function CollapsibleNav({
   navigateToApp,
   navigateToUrl,
   logos,
+  workspaceEnabled,
   ...observables
 }: Props) {
   const navLinks = useObservable(observables.navLinks$, []).filter((link) => !link.hidden);
@@ -193,7 +195,8 @@ export function CollapsibleNav({
                 link,
                 navLinks,
                 basePath,
-                navigateToUrl
+                navigateToUrl,
+                !!workspaceEnabled
               );
 
               return {

diff --git a/src/core/public/chrome/ui/header/header.tsx b/src/core/public/chrome/ui/header/header.tsx
@@ -689,6 +689,7 @@ export function Header({
             }}
             customNavLink$={observables.customNavLink$}
             logos={logos}
+            workspaceEnabled={application.capabilities.workspaces.enabled}
           />
         )}
       </header>

diff --git a/src/core/public/chrome/ui/header/nav_link.test.tsx b/src/core/public/chrome/ui/header/nav_link.test.tsx
@@ -100,9 +100,27 @@ describe('createRecentNavLink', () => {
       },
       mockNavLinks,
       mockBasePath,
-      mockedNavigateToUrl
+      mockedNavigateToUrl,
+      true
     );
 
     expect(recentLink.href).toEqual('http://localhost/test/w/foo/app/foo');
   });
+
+  it('create a recent link when workspace disabled', () => {
+    const recentLink = createRecentNavLink(
+      {
+        id: 'foo',
+        label: 'foo',
+        link: '/app/foo',
+        workspaceId: 'foo',
+      },
+      mockNavLinks,
+      mockBasePath,
+      mockedNavigateToUrl,
+      false
+    );
+
+    expect(recentLink.href).toEqual('http://localhost/test/app/foo');
+  });
 });
diff --git a/src/core/public/chrome/ui/header/nav_link.tsx b/src/core/public/chrome/ui/header/nav_link.tsx
@@ -125,13 +125,17 @@ export function createRecentNavLink(
   recentLink: ChromeRecentlyAccessedHistoryItem,
   navLinks: ChromeNavLink[],
   basePath: HttpStart['basePath'],
-  navigateToUrl: InternalApplicationStart['navigateToUrl']
+  navigateToUrl: InternalApplicationStart['navigateToUrl'],
+  workspaceEnabled: boolean = false
 ): RecentNavLink {
   const { link, label, workspaceId } = recentLink;
   const href = relativeToAbsolute(
-    basePath.prepend(formatUrlWithWorkspaceId(link, workspaceId || '', basePath), {
-      withoutClientBasePath: true,
-    })
+    basePath.prepend(
+      workspaceEnabled ? formatUrlWithWorkspaceId(link, workspaceId || '', basePath) : link,
+      {
+        withoutClientBasePath: true,
+      }
+    )
   );
   const navLink = navLinks.find((nl) => href.startsWith(nl.baseUrl));
   let titleAndAriaLabel = label;

diff --git a/src/plugins/saved_objects_management/public/management_section/recent_work.tsx b/src/plugins/saved_objects_management/public/management_section/recent_work.tsx
@@ -241,7 +241,7 @@ export const RecentWork = (props: { core: CoreStart; workspaceEnabled?: boolean
                 ...recentAccessItem.meta,
                 workspaceName: findWorkspace?.name,
                 updatedAt: moment(obj?.updated_at).valueOf(),
-                link: href,
+                link: workspaceEnabled ? href : link,
                 label: label || obj.meta.title,
               };
             })
@@ -254,7 +254,7 @@ export const RecentWork = (props: { core: CoreStart; workspaceEnabled?: boolean
     } finally {
       setIsLoading(false);
     }
-  }, [core.http, currentWorkspace, recentAccessed, workspaceList]);
+  }, [core.http, currentWorkspace, recentAccessed, workspaceList, workspaceEnabled]);
 
   useEffect(() => {
     // reset to allOption
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		fix:
		- Fix the display and jump logic for recent assets ([#8136](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8136))