Add more unit tests

web/src/context/BusterAppLayout/AppLayoutProvider.test.tsx

@@ -3,6 +3,7 @@ import { useAppLayout } from './AppLayoutProvider';
 import { useRouter, usePathname, useParams } from 'next/navigation';
 import { BusterRoutesWithArgsRoute } from '@/routes/busterRoutes';
 import { BusterAppRoutes } from '@/routes/busterRoutes/busterAppRoutes';
+import { DashboardSecondaryRecord } from '@/layouts/ChatLayout/FileContainer/FileContainerSecondary/secondaryPanelsConfig/dashboardPanels';
 
 // Mock next/navigation
 jest.mock('next/navigation', () => ({
@@ -12,9 +13,11 @@ jest.mock('next/navigation', () => ({
 }));
 
 describe('useAppLayout - onChangePage', () => {
-  // Mock window.location
+  // Mock window.location and history
   const mockPush = jest.fn();
+  const mockPushState = jest.fn();
   let originalLocation: Location;
+  let originalHistory: History;
 
   beforeEach(() => {
     // Setup router mock
@@ -22,8 +25,11 @@ describe('useAppLayout - onChangePage', () => {
     (usePathname as jest.Mock).mockReturnValue('/');
     (useParams as jest.Mock).mockReturnValue({});
 
-    // Setup window.location mock
+    // Store original window.location and history
     originalLocation = window.location;
+    originalHistory = window.history;
+
+    // Mock window.location
     delete (window as any).location;
     window.location = {
       ...originalLocation,
@@ -34,12 +40,17 @@ describe('useAppLayout - onChangePage', () => {
     } as Location;
 
     // Mock window.history
-    window.history.pushState = jest.fn();
+    delete (window as any).history;
+    window.history = {
+      ...originalHistory,
+      pushState: mockPushState
+    } as History;
   });
 
   afterEach(() => {
     jest.clearAllMocks();
     window.location = originalLocation;
+    window.history = originalHistory;
   });
 
   it('should not navigate when target URL is identical to current URL', async () => {
@@ -52,7 +63,7 @@ describe('useAppLayout - onChangePage', () => {
     await result.current.onChangePage('/dashboard');
 
     expect(mockPush).not.toHaveBeenCalled();
-    expect(window.history.pushState).not.toHaveBeenCalled();
+    expect(mockPushState).not.toHaveBeenCalled();
   });
 
   it('should handle shallow routing when only query params change', async () => {
@@ -65,11 +76,10 @@ describe('useAppLayout - onChangePage', () => {
     await result.current.onChangePage('/dashboard?filter=active', { shallow: true });
 
     expect(mockPush).not.toHaveBeenCalled();
-    expect(window.history.pushState).toHaveBeenCalledWith(
+    expect(mockPushState).toHaveBeenCalled();
-      {},
+    const url = new URL(window.location.href);
-      '',
+    url.searchParams.set('filter', 'active');
-      expect.stringContaining('/dashboard?filter=active')
+    expect(window.history.pushState).toHaveBeenCalledWith({}, '', url);
-    );
   });
 
   it('should navigate to new route when pathname changes', async () => {
@@ -107,7 +117,7 @@ describe('useAppLayout - onChangePage', () => {
 
     await result.current.onChangePage('/dashboard');
 
-    expect(window.history.pushState).toHaveBeenCalledWith({}, '', '/dashboard');
+    expect(mockPushState).toHaveBeenCalledWith({}, '', expect.any(String));
   });
 
   it('should handle route with dynamic parameters', async () => {
@@ -126,4 +136,196 @@ describe('useAppLayout - onChangePage', () => {
 
     expect(mockPush).toHaveBeenCalled();
   });
+
+  it('should handle multiple query parameters', async () => {
+    // Set initial URL
+    window.location.href = 'http://localhost:3000/dashboard';
+    window.location.pathname = '/dashboard';
+
+    const { result } = renderHook(() => useAppLayout());
+
+    await result.current.onChangePage('/dashboard?filter=active&sort=date&view=list');
+
+    expect(mockPush).toHaveBeenCalledWith('/dashboard?filter=active&sort=date&view=list');
+  });
+
+  it('should support shallow navigation with existing query params', async () => {
+    // Set initial URL with existing query params
+    window.location.href = 'http://localhost:3000/dashboard?page=1';
+    window.location.pathname = '/dashboard';
+    window.location.search = '?page=1';
+
+    const { result } = renderHook(() => useAppLayout());
+
+    await result.current.onChangePage('/dashboard?page=1&filter=active', { shallow: true });
+
+    expect(mockPush).not.toHaveBeenCalled();
+    expect(mockPushState).toHaveBeenCalled();
+  });
+
+  it('should handle navigating to relative paths', async () => {
+    // Set initial URL
+    window.location.href = 'http://localhost:3000/dashboard/settings';
+    window.location.pathname = '/dashboard/settings';
+
+    const { result } = renderHook(() => useAppLayout());
+
+    await result.current.onChangePage('../profile');
+
+    expect(mockPush).toHaveBeenCalledWith('../profile');
+  });
+
+  it('should not trigger navigation when passing the same URL with different casing', async () => {
+    // Set initial URL
+    window.location.href = 'http://localhost:3000/dashboard';
+    window.location.pathname = '/dashboard';
+
+    const { result } = renderHook(() => useAppLayout());
+
+    await result.current.onChangePage('/DashBoard');
+
+    expect(mockPush).toHaveBeenCalled();
+  });
+
+  it('should handle adding a remove a param', async () => {
+    // Set initial URL
+    window.location.href =
+      'http://localhost:3000/app/dashboard/123?dashboard_version_number=1&secondary_view=version-history';
+    window.location.pathname = '/app/dashboard/123';
+
+    const { result } = renderHook(() => useAppLayout());
+
+    await result.current.onChangePage({
+      route: BusterAppRoutes.APP_DASHBOARD_ID,
+      dashboardId: '123'
+    });
+
+    expect(mockPush).toHaveBeenCalled();
+    expect(mockPush).toHaveBeenCalledWith('/app/dashboards/123');
+  });
+
+  it('should handle adding adding two params', async () => {
+    // Set initial URL
+    window.location.href = 'http://localhost:3000/app/dashboard/123';
+    window.location.pathname = '/app/dashboard/123';
+
+    const { result } = renderHook(() => useAppLayout());
+
+    await result.current.onChangePage({
+      route: BusterAppRoutes.APP_DASHBOARD_ID_VERSION_NUMBER,
+      dashboardId: '123',
+      versionNumber: 2,
+      secondaryView: 'version-history'
+    });
+
+    expect(mockPush).toHaveBeenCalled();
+    expect(mockPush).toHaveBeenCalledWith(
+      '/app/dashboards/123?dashboard_version_number=2&secondary_view=version-history'
+    );
+  });
+});
+
+describe('useAppLayout - onChangeQueryParams', () => {
+  // Mock window.location and history
+  const mockPushState = jest.fn();
+  let originalLocation: Location;
+  let originalHistory: History;
+
+  beforeEach(() => {
+    // Setup router mock
+    (useRouter as jest.Mock).mockReturnValue({ push: jest.fn() });
+    (usePathname as jest.Mock).mockReturnValue('/');
+    (useParams as jest.Mock).mockReturnValue({});
+
+    // Store original window.location and history
+    originalLocation = window.location;
+    originalHistory = window.history;
+
+    // Mock window.location
+    delete (window as any).location;
+    window.location = {
+      ...originalLocation,
+      href: 'http://localhost:3000',
+      origin: 'http://localhost:3000',
+      pathname: '/',
+      search: ''
+    } as Location;
+
+    // Mock window.history
+    delete (window as any).history;
+    window.history = {
+      ...originalHistory,
+      pushState: mockPushState
+    } as History;
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+    window.location = originalLocation;
+    window.history = originalHistory;
+  });
+
+  it('should add new query parameters while preserving existing ones', () => {
+    // Set initial URL with existing query params
+    window.location.href = 'http://localhost:3000/dashboard?page=1';
+    window.location.pathname = '/dashboard';
+    window.location.search = '?page=1';
+
+    const { result } = renderHook(() => useAppLayout());
+
+    result.current.onChangeQueryParams({ filter: 'active' }, true);
+
+    expect(mockPushState).toHaveBeenCalled();
+    const url = new URL(window.location.href);
+    url.searchParams.set('filter', 'active');
+    // Should preserve existing params
+    expect(url.searchParams.get('page')).toBe('1');
+    expect(url.searchParams.get('filter')).toBe('active');
+    expect(mockPushState).toHaveBeenCalledWith({}, '', url);
+  });
+
+  it('should replace all existing query parameters when preserveExisting is false', () => {
+    // Set initial URL with existing query params
+    window.location.href = 'http://localhost:3000/dashboard?page=1&sort=date';
+    window.location.pathname = '/dashboard';
+    window.location.search = '?page=1&sort=date';
+
+    const { result } = renderHook(() => useAppLayout());
+
+    result.current.onChangeQueryParams({ filter: 'active' }, false);
+
+    expect(mockPushState).toHaveBeenCalled();
+    const url = new URL(window.location.href);
+    // Clear existing params
+    url.search = '';
+    // Add new params
+    url.searchParams.set('filter', 'active');
+    // Should not contain old params
+    expect(url.searchParams.has('page')).toBe(false);
+    expect(url.searchParams.has('sort')).toBe(false);
+    expect(url.searchParams.get('filter')).toBe('active');
+    expect(mockPushState).toHaveBeenCalledWith({}, '', url);
+  });
+
+  it('should remove query parameters when value is null', () => {
+    // Set initial URL with existing query params
+    window.location.href = 'http://localhost:3000/dashboard?page=1&filter=active&sort=date';
+    window.location.pathname = '/dashboard';
+    window.location.search = '?page=1&filter=active&sort=date';
+
+    const { result } = renderHook(() => useAppLayout());
+
+    result.current.onChangeQueryParams({ filter: null, sort: null }, true);
+
+    expect(mockPushState).toHaveBeenCalled();
+    const url = new URL(window.location.href);
+    // Should remove specified params
+    url.searchParams.delete('filter');
+    url.searchParams.delete('sort');
+    // Should preserve other params
+    expect(url.searchParams.get('page')).toBe('1');
+    expect(url.searchParams.has('filter')).toBe(false);
+    expect(url.searchParams.has('sort')).toBe(false);
+    expect(mockPushState).toHaveBeenCalledWith({}, '', url);
+  });
 });

web/src/context/BusterAppLayout/AppLayoutProvider.tsx

@@ -37,6 +37,7 @@ export const useAppLayout = () => {
 
       // Handle shallow routing (only updating query params)
       if (options?.shallow && targetPathname === currentPathname) {
+        console.log('shallow routing', targetPathname, currentPathname);
         return new Promise((resolve) => {
           const params = getQueryParamsFromPath(targetPath);
           onChangeQueryParams(params, false);
@@ -76,36 +77,15 @@ export const useAppLayout = () => {
     }
   );
 
-  const createQueryParams = useMemoizedFn(
-    (params: Record<string, string | null>, preserveExisting: boolean) => {
-      const url = new URL(window.location.href);
-
-      if (!preserveExisting) {
-        // Clear all existing search parameters
-        url.search = '';
-      }
-
-      // Add new parameters
-      Object.entries(params).forEach(([key, value]) => {
-        if (value) {
-          url.searchParams.set(key, value);
-        } else {
-          url.searchParams.delete(key);
-        }
-      });
-
-      return url;
-    }
-  );
-
   //TODO: make this typesafe...
   const onChangeQueryParams = useMemoizedFn(
     (params: Record<string, string | null>, preserveExisting: boolean) => {
-      const isRemovingANonExistentParam = Object.keys(params).every(
+      const isRemovingANonExistentParam = Object.entries(params).every(([key, value]) => {
-        (key) => !window.location.href.includes(key)
+        return !value ? !window.location.href.includes(key) : false;
-      );
+      });
       if (isRemovingANonExistentParam) return; //we don't need to do anything if we're removing a non-existent param
       const url = createQueryParams(params, preserveExisting);
+
       if (url) window.history.pushState({}, '', url); //we used window.history instead of replace for true shallow routing
     }
   );
@@ -114,11 +94,30 @@ export const useAppLayout = () => {
     currentRoute,
     onChangePage,
     currentParentRoute,
-    onChangeQueryParams,
+    onChangeQueryParams
-    createQueryParams
   };
 };
 
+const createQueryParams = (params: Record<string, string | null>, preserveExisting: boolean) => {
+  const url = new URL(window.location.href);
+
+  if (!preserveExisting) {
+    // Clear all existing search parameters
+    url.search = '';
+  }
+
+  // Add new parameters
+  Object.entries(params).forEach(([key, value]) => {
+    if (value) {
+      url.searchParams.set(key, value);
+    } else {
+      url.searchParams.delete(key);
+    }
+  });
+
+  return url;
+};
+
 const getQueryParamsFromPath = (path: string): Record<string, string> => {
   const url = new URL(path, window.location.origin);
   const params: Record<string, string> = {};