diff --git a/packages/manager/src/components/OrderBy.tsx b/packages/manager/src/components/OrderBy.tsx index f91409c5c02..0d8f2bab051 100644 --- a/packages/manager/src/components/OrderBy.tsx +++ b/packages/manager/src/components/OrderBy.tsx @@ -17,7 +17,6 @@ import { sortByUTFDate, } from 'src/utilities/sort-by'; -import type { AllParams, AnyRoute } from '@tanstack/react-router'; import type { Order } from 'src/hooks/useOrder'; import type { ManagerPreferences } from 'src/types/ManagerPreferences'; @@ -60,7 +59,7 @@ export type CombinedProps = Props; export const getInitialValuesFromUserPreferences = ( preferenceKey: string, preferences: ManagerPreferences, - params: AllParams | Record, // account for both TanStack and React Router params + params: Record, defaultOrderBy?: string, defaultOrder?: Order, prefix?: string diff --git a/packages/manager/src/features/Volumes/VolumesLanding.tsx b/packages/manager/src/features/Volumes/VolumesLanding.tsx index 6a646d0b842..aa2a4305d67 100644 --- a/packages/manager/src/features/Volumes/VolumesLanding.tsx +++ b/packages/manager/src/features/Volumes/VolumesLanding.tsx @@ -69,11 +69,11 @@ export const VolumesLanding = () => { const { handleOrderChange, order, orderBy } = useOrderV2({ initialRoute: { - from: '/volumes', - search: { + defaultOrder: { order: VOLUME_TABLE_DEFAULT_ORDER, orderBy: VOLUME_TABLE_DEFAULT_ORDER_BY, }, + from: '/volumes', }, preferenceKey: VOLUME_TABLE_PREFERENCE_KEY, }); diff --git a/packages/manager/src/hooks/useOrderV2.test.tsx b/packages/manager/src/hooks/useOrderV2.test.tsx index 5277027d93c..37c4f632976 100644 --- a/packages/manager/src/hooks/useOrderV2.test.tsx +++ b/packages/manager/src/hooks/useOrderV2.test.tsx @@ -2,257 +2,154 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import { HttpResponse, http, server } from 'src/mocks/testServer'; import { queryClientFactory } from 'src/queries/base'; -import { usePreferences } from 'src/queries/profile/preferences'; import { wrapWithThemeAndRouter } from 'src/utilities/testHelpers'; import { useOrderV2 } from './useOrderV2'; -import type { OrderSet } from 'src/types/ManagerPreferences'; - -// Default for Sorting -const defaultOrder: OrderSet = { - order: 'asc', - orderBy: 'status', -}; - -// Expected order for preference test -const queryOrder: OrderSet = { - order: 'desc', - orderBy: 'when', -}; - -// Expected order for preference test -const preferenceOrder: OrderSet = { - order: 'asc', - orderBy: 'type', -}; - -// Expected order for calling handleOrderChange -const handleOrderChangeOrder: OrderSet = { - order: 'desc', - orderBy: 'type', -}; +import type { UseOrderV2Props } from './useOrderV2'; const mockNavigate = vi.fn(); +const mockUseSearch = vi.fn(() => ({})); -// Used to mock query params vi.mock('@tanstack/react-router', async () => { const actual = await vi.importActual('@tanstack/react-router'); return { ...actual, useNavigate: vi.fn(() => mockNavigate), - useParams: vi.fn(() => ({ - volumeId: '123', - })), + useSearch: vi.fn(() => mockUseSearch()), }; }); const queryClient = queryClientFactory(); +const defaultProps: UseOrderV2Props = { + initialRoute: { + defaultOrder: { + order: 'asc', + orderBy: 'label', + }, + from: '/', + }, + preferenceKey: 'volumes', +}; -const currentRoute = '/'; - -describe('useOrderV2 hook', () => { +describe('useOrderV2', () => { beforeEach(() => { vi.clearAllMocks(); + queryClient.clear(); }); - it('should use default sort options when there are no query params or preference', async () => { - const { result } = renderHook( - () => - useOrderV2({ - initialRoute: { - from: currentRoute, - search: { - order: defaultOrder.order, - orderBy: defaultOrder.orderBy, - }, - }, - }), - { - wrapper: (ui) => - wrapWithThemeAndRouter(ui.children, { - queryClient, - }), - } - ); - - await waitFor(() => { - expect(result.current.order).toBe(defaultOrder.order); - expect(result.current.orderBy).toBe(defaultOrder.orderBy); + it('should use URL params with prefix', async () => { + mockUseSearch.mockReturnValue({ + 'test-order': 'desc', + 'test-orderBy': 'status', }); - }); - it('query parameters with sort data should take precedence over defaults', async () => { const { result } = renderHook( - () => - useOrderV2({ - initialRoute: { - from: currentRoute, - search: { - order: queryOrder.order, - orderBy: queryOrder.orderBy, - }, - }, - }), + () => useOrderV2({ ...defaultProps, prefix: 'test' }), { - wrapper: (ui) => - wrapWithThemeAndRouter(ui.children, { - queryClient, - }), + wrapper: (ui) => wrapWithThemeAndRouter(ui.children, { queryClient }), } ); await waitFor(() => { - expect(result.current.order).toBe(queryOrder.order); - expect(result.current.orderBy).toBe(queryOrder.orderBy); + expect(result.current.order).toBe('desc'); + expect(result.current.orderBy).toBe('status'); }); }); - it('use preferences are used when there are no query params', async () => { - const queryClient = queryClientFactory(); + it('should use preferences when present and no URL params are provided', async () => { + mockUseSearch.mockReturnValue({}); + server.use( http.get('*/profile/preferences', () => { return HttpResponse.json({ sortKeys: { - 'account-maintenance-order': preferenceOrder, + volumes: { + order: 'desc', + orderBy: 'size', + }, }, }); }) ); - const { result: preferencesResult } = renderHook(() => usePreferences(), { + const { result } = renderHook(() => useOrderV2(defaultProps), { wrapper: (ui) => wrapWithThemeAndRouter(ui.children, { queryClient }), }); - // This is kind of a bug. useOrder currently requires preferences to be cached - // before it works properly. - await waitFor(() => { - expect(preferencesResult.current.data).toBeDefined(); - }); - - const { result } = renderHook( - () => - useOrderV2({ - initialRoute: { - from: currentRoute, - search: undefined, - }, - preferenceKey: 'account-maintenance-order', - }), - { - wrapper: (ui) => wrapWithThemeAndRouter(ui.children, { queryClient }), - } - ); - await waitFor(() => { - expect(result.current.order).toBe(preferenceOrder.order); - expect(result.current.orderBy).toBe(preferenceOrder.orderBy); + expect(result.current.order).toBe('desc'); + expect(result.current.orderBy).toBe('size'); }); }); - it('should change order when handleOrderChange is called with new values', async () => { - const { result } = renderHook( - () => - useOrderV2({ - initialRoute: { - from: currentRoute, - search: undefined, - }, - }), - { - wrapper: (ui) => wrapWithThemeAndRouter(ui.children, { queryClient }), - } - ); + it('should use default values as last priority', async () => { + mockUseSearch.mockReturnValue({}); - act(() => - result.current.handleOrderChange( - handleOrderChangeOrder.orderBy, - handleOrderChangeOrder.order - ) + server.use( + http.get('*/profile/preferences', () => { + return HttpResponse.json({ sortKeys: {} }); + }) ); + const { result } = renderHook(() => useOrderV2(defaultProps), { + wrapper: (ui) => wrapWithThemeAndRouter(ui.children, { queryClient }), + }); + await waitFor(() => { - expect(result.current.order).toBe(handleOrderChangeOrder.order); - expect(result.current.orderBy).toBe(handleOrderChangeOrder.orderBy); + expect(result.current.order).toBe( + defaultProps.initialRoute.defaultOrder.order + ); + expect(result.current.orderBy).toBe( + defaultProps.initialRoute.defaultOrder.orderBy + ); }); }); - it('should update query params when handleOrderChange is called', async () => { - const { result } = renderHook( - () => - useOrderV2({ - initialRoute: { - from: currentRoute, - search: undefined, - }, - preferenceKey: 'account-maintenance-order', - }), - { - wrapper: (ui) => wrapWithThemeAndRouter(ui.children, { queryClient }), - } - ); - - act(() => - result.current.handleOrderChange( - handleOrderChangeOrder.orderBy, - handleOrderChangeOrder.order - ) + it.only('should update URL and preferences when handleOrderChange is called', async () => { + const mutatePreferencesMock = vi.fn(); + server.use( + http.put('*/profile/preferences', async ({ request }) => { + const body = await request.json(); + mutatePreferencesMock(body); + return HttpResponse.json(body); + }) ); - expect(mockNavigate).toHaveBeenCalled(); - - // Get the actual function that was passed for futu - const searchFn = mockNavigate.mock.calls[0][0].search; + mockUseSearch.mockReturnValue({}); - // Test that the search function returns the expected object - expect(searchFn({})).toEqual({ - order: handleOrderChangeOrder.order, - orderBy: handleOrderChangeOrder.orderBy, - volumeId: '123', + const { result } = renderHook(() => useOrderV2(defaultProps), { + wrapper: (ui) => wrapWithThemeAndRouter(ui.children, { queryClient }), }); - await waitFor(() => { - expect(mockNavigate.mock.calls[0][0].to).toBe(currentRoute); - expect(result.current.order).toBe(handleOrderChangeOrder.order); - expect(result.current.orderBy).toBe(handleOrderChangeOrder.orderBy); + act(() => { + result.current.handleOrderChange('size', 'desc'); }); - }); - - it('should update query params when handleOrderChange is called (with prefix)', async () => { - const prefix = 'volume'; - const { result } = renderHook( - () => - useOrderV2({ - initialRoute: { - from: currentRoute, - search: undefined, - }, - prefix, - }), - { - wrapper: (ui) => wrapWithThemeAndRouter(ui.children, { queryClient }), - } - ); - - act(() => - result.current.handleOrderChange( - handleOrderChangeOrder.orderBy, - handleOrderChangeOrder.order - ) - ); - expect(mockNavigate).toHaveBeenCalled(); - - const searchFn = mockNavigate.mock.calls[0][0].search; + mockUseSearch.mockReturnValue({ + order: 'desc', + orderBy: 'size', + }); await waitFor(() => { - expect(searchFn({})).toEqual({ - [`${prefix}-order`]: handleOrderChangeOrder.order, - [`${prefix}-orderBy`]: handleOrderChangeOrder.orderBy, - volumeId: '123', - }); - expect(result.current.order).toBe(handleOrderChangeOrder.order); - expect(result.current.orderBy).toBe(handleOrderChangeOrder.orderBy); + expect(mockNavigate).toHaveBeenCalledWith( + expect.objectContaining({ + search: expect.any(Function), + to: '/', + }) + ); + expect(mutatePreferencesMock).toHaveBeenCalledWith( + expect.objectContaining({ + sortKeys: expect.objectContaining({ + volumes: { + order: 'desc', + orderBy: 'size', + }, + }), + }) + ); + expect(result.current.order).toBe('desc'); + expect(result.current.orderBy).toBe('size'); }); }); }); diff --git a/packages/manager/src/hooks/useOrderV2.ts b/packages/manager/src/hooks/useOrderV2.ts index 0fd90977c47..cdc4425709e 100644 --- a/packages/manager/src/hooks/useOrderV2.ts +++ b/packages/manager/src/hooks/useOrderV2.ts @@ -1,8 +1,5 @@ -import { useNavigate, useParams } from '@tanstack/react-router'; -import { useRef, useState } from 'react'; -import { debounce } from 'throttle-debounce'; +import { useNavigate, useSearch } from '@tanstack/react-router'; -import { getInitialValuesFromUserPreferences } from 'src/components/OrderBy'; import { useMutatePreferences, usePreferences, @@ -10,23 +7,26 @@ import { import type { RoutePaths } from '@tanstack/react-router'; import type { MigrationRouteTree } from 'src/routes'; -import type { OrderSet } from 'src/types/ManagerPreferences'; +import type { OrderSetWithPrefix } from 'src/types/ManagerPreferences'; export type Order = 'asc' | 'desc'; -interface UseOrderV2Props { +export interface UseOrderV2Props { /** * initial order to use when no query params are present * Includes the from and search params */ initialRoute: { + defaultOrder: { + order: Order; + orderBy: string; + }; from: RoutePaths; - search?: OrderSet; }; /** * preference key to save to user preferences */ - preferenceKey?: string; + preferenceKey: string; /** * prefix for the query params in the url */ @@ -34,11 +34,12 @@ interface UseOrderV2Props { } /** - * useOrder is a hook that allows you to handle ordering tables. It takes into account - * the following items when determining initial order + * useOrder is a hook that allows you to handle ordering tables. + * It takes into account the following items when determining initial order: * 1. Query Params (Ex. ?order=asc&orderBy=status) * 2. User Preference - * 3. Initial Order passed as params + * 3. Initial Order + * * When a user changes order using the handleOrderChange function, the query params are * updated and the user preferences are also updated. */ @@ -49,38 +50,44 @@ export const useOrderV2 = ({ }: UseOrderV2Props) => { const { data: preferences } = usePreferences(); const { mutateAsync: updatePreferences } = useMutatePreferences(); - const params = useParams({ from: initialRoute.from }); + const searchParams = useSearch({ from: initialRoute.from }); const navigate = useNavigate(); - const initialOrder = getInitialValuesFromUserPreferences( - preferenceKey || '', - preferences || {}, - params, - initialRoute?.search?.orderBy, - initialRoute?.search?.order, - prefix - ); + const getOrderValues = () => { + // 1. URL params with prefix + if ( + prefix && + `${prefix}-order` in searchParams && + `${prefix}-orderBy` in searchParams + ) { + const prefixedParams = searchParams as OrderSetWithPrefix; + return { + order: prefixedParams[`${prefix}-order`], + orderBy: prefixedParams[`${prefix}-orderBy`], + }; + } - const [orderBy, setOrderBy] = useState(initialOrder.orderBy); - const [order, setOrder] = useState<'asc' | 'desc'>(initialOrder.order); + // 2. Regular URL params + if ('order' in searchParams && 'orderBy' in searchParams) { + return { + order: searchParams.order as Order, + orderBy: searchParams.orderBy as string, + }; + } - const debouncedUpdateUserPreferences = useRef( - debounce(1500, false, (orderBy: string, order: Order) => { - if (preferenceKey) { - updatePreferences({ - sortKeys: { - ...(preferences?.sortKeys ?? {}), - [preferenceKey]: { order, orderBy }, - }, - }); - } - }) - ).current; + // 3. Stored preferences + const prefKey = prefix ? `${prefix}-${preferenceKey}` : preferenceKey; + if (preferenceKey && preferences?.sortKeys?.[prefKey]) { + return preferences.sortKeys[prefKey]; + } - const handleOrderChange = (newOrderBy: string, newOrder: Order) => { - setOrderBy(newOrderBy); - setOrder(newOrder); + // 4. Default values + return initialRoute.defaultOrder; + }; + + const { order, orderBy } = getOrderValues(); + const handleOrderChange = (newOrderBy: string, newOrder: Order) => { const urlData = prefix ? { [`${prefix}-order`]: newOrder, @@ -94,13 +101,19 @@ export const useOrderV2 = ({ navigate({ search: (prev) => ({ ...prev, - ...params, + ...searchParams, ...urlData, }), to: initialRoute.from, }); - debouncedUpdateUserPreferences(newOrderBy, newOrder); + const prefKey = prefix ? `${prefix}-${preferenceKey}` : preferenceKey; + updatePreferences({ + sortKeys: { + ...(preferences?.sortKeys ?? {}), + [prefKey]: { order: newOrder, orderBy: newOrderBy }, + }, + }); }; return { handleOrderChange, order, orderBy }; diff --git a/packages/manager/src/routes/volumes/constants.ts b/packages/manager/src/routes/volumes/constants.ts index fb7e804a189..738d9d16f76 100644 --- a/packages/manager/src/routes/volumes/constants.ts +++ b/packages/manager/src/routes/volumes/constants.ts @@ -1,3 +1,3 @@ -export const VOLUME_TABLE_DEFAULT_ORDER = 'desc'; +export const VOLUME_TABLE_DEFAULT_ORDER = 'asc'; export const VOLUME_TABLE_DEFAULT_ORDER_BY = 'label'; export const VOLUME_TABLE_PREFERENCE_KEY = 'volumes'; diff --git a/packages/manager/src/types/ManagerPreferences.ts b/packages/manager/src/types/ManagerPreferences.ts index 5c89f454313..42f76b0fc23 100644 --- a/packages/manager/src/types/ManagerPreferences.ts +++ b/packages/manager/src/types/ManagerPreferences.ts @@ -7,6 +7,10 @@ export interface OrderSet { orderBy: string; } +export type OrderSetWithPrefix

= { + [K in `${P}-order` | `${P}-orderBy`]: K extends `${P}-order` ? Order : string; +}; + export interface DismissedNotification { created: string; expiry?: string;