Skip to content

Commit

Permalink
feat: keep filter order (#5688)
Browse files Browse the repository at this point in the history
  • Loading branch information
kwasniew authored Dec 19, 2023
1 parent b933a03 commit 8306073
Show file tree
Hide file tree
Showing 7 changed files with 175 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,15 @@ const FeatureToggleListTableComponent: VFC = () => {
'features-list-table',
stateConfig,
);

const filterState = {
project: tableState.project,
tag: tableState.tag,
state: tableState.state,
segment: tableState.segment,
createdAt: tableState.createdAt,
};
const {
offset,
limit,
query,
favoritesFirst,
sortBy,
sortOrder,
...filterState
} = tableState;

const {
features = [],
Expand Down Expand Up @@ -130,10 +131,10 @@ const FeatureToggleListTableComponent: VFC = () => {
columnHelper.accessor('favorite', {
header: () => (
<FavoriteIconHeader
isActive={tableState.favoritesFirst}
isActive={favoritesFirst}
onClick={() =>
setTableState({
favoritesFirst: !tableState.favoritesFirst,
favoritesFirst: !favoritesFirst,
})
}
/>
Expand Down Expand Up @@ -211,7 +212,7 @@ const FeatureToggleListTableComponent: VFC = () => {
cell: ({ getValue }) => <FeatureStaleCell value={getValue()} />,
}),
],
[tableState.favoritesFirst],
[favoritesFirst],
);

const data = useMemo(
Expand Down Expand Up @@ -270,9 +271,7 @@ const FeatureToggleListTableComponent: VFC = () => {
<Search
placeholder='Search'
expandable
initialValue={
tableState.query || ''
}
initialValue={query || ''}
onChange={setSearchValue}
id='globalFeatureToggles'
/>
Expand All @@ -298,7 +297,7 @@ const FeatureToggleListTableComponent: VFC = () => {
condition={isSmallScreen}
show={
<Search
initialValue={tableState.query || ''}
initialValue={query || ''}
onChange={setSearchValue}
id='globalFeatureToggles'
/>
Expand All @@ -311,19 +310,19 @@ const FeatureToggleListTableComponent: VFC = () => {
onChange={setTableState}
state={filterState}
/>
<SearchHighlightProvider value={tableState.query || ''}>
<SearchHighlightProvider value={query || ''}>
<PaginatedTable tableInstance={table} totalItems={total} />
</SearchHighlightProvider>
<ConditionallyRender
condition={rows.length === 0}
show={
<Box sx={(theme) => ({ padding: theme.spacing(0, 2, 2) })}>
<ConditionallyRender
condition={(tableState.query || '')?.length > 0}
condition={(query || '')?.length > 0}
show={
<TablePlaceholder>
No feature toggles found matching &ldquo;
{tableState.query}
{query}
&rdquo;
</TablePlaceholder>
}
Expand Down
50 changes: 49 additions & 1 deletion frontend/src/component/filter/Filters/Filters.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { screen } from '@testing-library/react';
import { render } from 'utils/testRenderer';
import { FILTER_ITEM } from 'utils/testIds';
import { Filters, IFilterItem } from './Filters';
import { FilterItemParamHolder, Filters, IFilterItem } from './Filters';

test('shoulder render all available filters', async () => {
const availableFilters: IFilterItem[] = [
Expand Down Expand Up @@ -126,3 +126,51 @@ test('should remove selected item from the add filter list', async () => {
addFilterButton.click();
expect(screen.getByRole('menu').textContent).toBe('Tags');
});

test('should render filters in the order defined by the initial state', async () => {
const initialState: FilterItemParamHolder = {
filterB: { operator: '', values: [] },
filterA: { operator: '', values: [] },
filterC: { operator: '', values: [] },
};

const availableFilters: IFilterItem[] = [
{
label: 'FilterA',
icon: '',
options: [],
filterKey: 'filterA',
singularOperators: ['IRRELEVANT'],
pluralOperators: ['IRRELEVANT'],
},
{
label: 'FilterB',
icon: '',
options: [],
filterKey: 'filterB',
singularOperators: ['IRRELEVANT'],
pluralOperators: ['IRRELEVANT'],
},
{
label: 'FilterC',
icon: '',
options: [],
filterKey: 'filterC',
singularOperators: ['IRRELEVANT'],
pluralOperators: ['IRRELEVANT'],
},
];

render(
<Filters
availableFilters={availableFilters}
onChange={() => {}}
state={initialState}
/>,
);

const filterItems = screen.getAllByTestId(FILTER_ITEM);
const filterTexts = filterItems.map((item) => item.textContent);

expect(filterTexts).toEqual(['FilterB', 'FilterA', 'FilterC']);
});
11 changes: 9 additions & 2 deletions frontend/src/component/filter/Filters/Filters.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,16 @@ export const Filters: VFC<IFilterProps> = ({
};

useEffect(() => {
const newSelectedFilters = availableFilters
const newSelectedFilters = Object.keys(state)
.map((filterKey) =>
availableFilters.find(
(filter) => filterKey === filter.filterKey,
),
)
.filter((filter): filter is IFilterItem => Boolean(filter))
.filter((field) => Boolean(state[field.filterKey]))
.map((field) => field.label);
.map((filter) => filter.label);

const allSelectedFilters = mergeArraysKeepingOrder(
selectedFilters,
newSelectedFilters,
Expand Down
28 changes: 28 additions & 0 deletions frontend/src/hooks/usePersistentTableState.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ function TestComponent({ keyName, queryParamsDefinition }: TestComponentProps) {
<span data-testid='state-value'>
{tableState.query}
</span>
<span data-testid='state-keys'>
{Object.keys(tableState).join(',')}
</span>
<button
type='button'
onClick={() => setTableState({ query: 'after' })}
Expand Down Expand Up @@ -229,4 +232,29 @@ describe('usePersistentTableState', () => {
);
});
});

it('maintains key order', async () => {
createLocalStorage('testKey', {});

render(
<TestComponent
keyName='testKey'
queryParamsDefinition={{
query: StringParam,
another: StringParam,
ignore: StringParam,
}}
/>,
{ route: '/my-url?another=another&query=initialUrl' },
);

expect(screen.getByTestId('state-keys').textContent).toBe(
'another,query,ignore',
);

await waitFor(() => {
const { value } = createLocalStorage('testKey', {});
expect(Object.keys(value)).toStrictEqual(['another', 'query']);
});
});
});
14 changes: 10 additions & 4 deletions frontend/src/hooks/usePersistentTableState.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { useEffect, useCallback } from 'react';
import { useEffect, useCallback, useMemo } from 'react';
import { useSearchParams } from 'react-router-dom';
import { createLocalStorage } from 'utils/createLocalStorage';
import { encodeQueryParams, useQueryParams } from 'use-query-params';
import { QueryParamConfigMap } from 'serialize-query-params/src/types';
import { reorderObject } from '../utils/reorderObject';

const usePersistentSearchParams = <T extends QueryParamConfigMap>(
key: string,
Expand Down Expand Up @@ -43,6 +44,11 @@ export const usePersistentTableState = <T extends QueryParamConfigMap>(
queryParamsDefinition,
);

const [searchParams] = useSearchParams();
const orderedTableState = useMemo(() => {
return reorderObject(tableState, [...searchParams.keys()]);
}, [searchParams, tableState, reorderObject]);

type SetTableStateInternalParam = Parameters<
typeof setTableStateInternal
>[0];
Expand Down Expand Up @@ -76,9 +82,9 @@ export const usePersistentTableState = <T extends QueryParamConfigMap>(
);

useEffect(() => {
const { offset, ...rest } = tableState;
const { offset, ...rest } = orderedTableState;
updateStoredParams(rest);
}, [JSON.stringify(tableState)]);
}, [JSON.stringify(orderedTableState)]);

return [tableState, setTableState] as const;
return [orderedTableState, setTableState] as const;
};
40 changes: 40 additions & 0 deletions frontend/src/utils/reorderObject.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { reorderObject } from './reorderObject';

describe('reorderObject', () => {
it('correctly reorders the object based on provided keys', () => {
const myObj = { a: 1, b: 2, c: 3, d: 4 };
const order = ['b', 'a'];
const result = reorderObject(myObj, order);
const expected = { b: 2, a: 1, c: 3, d: 4 };
expect(result).toEqual(expected);
});

it('ignores non-existent keys in the order array', () => {
const myObj = { a: 1, b: 2, c: 3 };
const order = ['c', 'z', 'a']; // 'z' does not exist in myObj
const result = reorderObject(myObj, order);
const expected = { c: 3, a: 1, b: 2 };
expect(result).toEqual(expected);
});

it('returns the original object when order array is empty', () => {
const myObj = { a: 1, b: 2, c: 3 };
const order: string[] = [];
const result = reorderObject(myObj, order);
expect(result).toEqual(myObj);
});

it('returns the object with the same order when order array contains all object keys', () => {
const myObj = { a: 1, b: 2, c: 3 };
const order = ['a', 'b', 'c'];
const result = reorderObject(myObj, order);
expect(result).toEqual(myObj);
});

it('does not modify the original object', () => {
const myObj = { a: 1, b: 2, c: 3 };
const order = ['b', 'a'];
const result = reorderObject(myObj, order);
expect(myObj).toEqual({ a: 1, b: 2, c: 3 }); // myObj should remain unchanged
});
});
22 changes: 22 additions & 0 deletions frontend/src/utils/reorderObject.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
export const reorderObject = <T extends object>(obj: T, order: string[]): T => {
// Create a set for quick lookup of the ordered keys
const orderSet = new Set(order);

const orderedObj: Partial<T> = {};

// Add explicitly ordered keys to the ordered object
order.forEach((key) => {
if (key in obj) {
orderedObj[key as keyof T] = obj[key as keyof T];
}
});

// Add remaining keys that were not explicitly ordered
Object.keys(obj).forEach((key) => {
if (!orderSet.has(key)) {
orderedObj[key as keyof T] = obj[key as keyof T];
}
});

return orderedObj as T;
};

0 comments on commit 8306073

Please sign in to comment.