diff --git a/web/src/context/BusterAppLayout/AppLayoutProvider.test.tsx b/web/src/context/BusterAppLayout/AppLayoutProvider.test.tsx index aa467944c..89324f3e6 100644 --- a/web/src/context/BusterAppLayout/AppLayoutProvider.test.tsx +++ b/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.stringContaining('/dashboard?filter=active') - ); + expect(mockPushState).toHaveBeenCalled(); + const url = new URL(window.location.href); + url.searchParams.set('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); + }); }); diff --git a/web/src/context/BusterAppLayout/AppLayoutProvider.tsx b/web/src/context/BusterAppLayout/AppLayoutProvider.tsx index 3c47abc75..1c5bb540e 100644 --- a/web/src/context/BusterAppLayout/AppLayoutProvider.tsx +++ b/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, 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, preserveExisting: boolean) => { - const isRemovingANonExistentParam = Object.keys(params).every( - (key) => !window.location.href.includes(key) - ); + const isRemovingANonExistentParam = Object.entries(params).every(([key, value]) => { + 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, - createQueryParams + onChangeQueryParams }; }; +const createQueryParams = (params: Record, 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 => { const url = new URL(path, window.location.origin); const params: Record = {};