Skip to content

Commit 08009ac

Browse files
knudttyknudtty
authored
feat: pinned search filters (#746)
1 parent 0d37943 commit 08009ac

File tree

7 files changed

+390
-23
lines changed

7 files changed

+390
-23
lines changed

‎.changeset/mighty-crabs-fry.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@hyperdx/app": patch
3+
---
4+
5+
feat: add saved filters for searches

‎packages/app/src/DBSearchPage.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1249,6 +1249,7 @@ function DBSearchPage() {
12491249
dateRange: searchedTimeRange,
12501250
with: aliasWith,
12511251
}}
1252+
sourceId={inputSourceObj?.id}
12521253
{...searchFilters}
12531254
/>
12541255
</ErrorBoundary>

‎packages/app/src/__tests__/utils.test.ts

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { TSource } from '@hyperdx/common-utils/dist/types';
2+
import { act, renderHook } from '@testing-library/react';
23

34
import { MetricsDataType, NumberFormat } from '../types';
5+
import * as utils from '../utils';
46
import {
57
formatAttributeClause,
68
formatDate,
@@ -268,3 +270,204 @@ describe('formatNumber', () => {
268270
});
269271
});
270272
});
273+
274+
describe('useLocalStorage', () => {
275+
// Create a mock for localStorage
276+
let localStorageMock: jest.Mocked<Storage>;
277+
278+
beforeEach(() => {
279+
// Clear all mocks between tests
280+
jest.clearAllMocks();
281+
282+
// Create localStorage mock
283+
localStorageMock = {
284+
getItem: jest.fn().mockImplementation((_: string) => null),
285+
setItem: jest.fn(),
286+
removeItem: jest.fn(),
287+
clear: jest.fn(),
288+
key: jest.fn(),
289+
length: 0,
290+
};
291+
292+
// Replace window.localStorage with our mock
293+
Object.defineProperty(window, 'localStorage', {
294+
value: localStorageMock,
295+
writable: true,
296+
});
297+
});
298+
299+
afterAll(() => {
300+
// Restore original implementations
301+
jest.restoreAllMocks();
302+
});
303+
304+
test('should initialize with initial value when localStorage is empty', () => {
305+
// Mock localStorage.getItem to return null (empty)
306+
localStorageMock.getItem.mockReturnValueOnce(null);
307+
308+
const initialValue = { test: 'value' };
309+
const { result } = renderHook(() =>
310+
utils.useLocalStorage('testKey', initialValue),
311+
);
312+
313+
// Check if initialized with initial value
314+
expect(result.current[0]).toEqual(initialValue);
315+
316+
// Verify localStorage was checked
317+
expect(localStorageMock.getItem).toHaveBeenCalledWith('testKey');
318+
});
319+
320+
test('should retrieve existing value from localStorage', () => {
321+
// Mock localStorage to return existing value
322+
const existingValue = { test: 'existing' };
323+
localStorageMock.getItem.mockReturnValueOnce(JSON.stringify(existingValue));
324+
325+
const { result } = renderHook(() =>
326+
utils.useLocalStorage('testKey', { test: 'default' }),
327+
);
328+
329+
// Should use the value from localStorage, not the initial value
330+
expect(result.current[0]).toEqual(existingValue);
331+
expect(localStorageMock.getItem).toHaveBeenCalledWith('testKey');
332+
});
333+
334+
test('should update localStorage when setValue is called', () => {
335+
localStorageMock.getItem.mockReturnValueOnce(null);
336+
337+
const { result } = renderHook(() =>
338+
utils.useLocalStorage('testKey', 'initial'),
339+
);
340+
341+
// Update value
342+
const newValue = 'updated';
343+
act(() => {
344+
result.current[1](newValue);
345+
});
346+
347+
// Check if state updated
348+
expect(result.current[0]).toBe(newValue);
349+
350+
// Check if localStorage was updated
351+
expect(localStorageMock.setItem).toHaveBeenCalledWith(
352+
'testKey',
353+
JSON.stringify(newValue),
354+
);
355+
});
356+
357+
test('should handle functional updates', () => {
358+
localStorageMock.getItem.mockReturnValueOnce(JSON.stringify(0));
359+
360+
const { result } = renderHook(() =>
361+
utils.useLocalStorage<number>('testKey', 0),
362+
);
363+
364+
// Update using function
365+
act(() => {
366+
result.current[1](prev => prev + 1);
367+
});
368+
369+
// Check if state updated correctly
370+
expect(result.current[0]).toBe(1);
371+
372+
// Check if localStorage was updated
373+
expect(localStorageMock.setItem).toHaveBeenCalledWith(
374+
'testKey',
375+
JSON.stringify(1),
376+
);
377+
});
378+
379+
test('should handle storage event from another window', () => {
380+
// Initial setup
381+
localStorageMock.getItem.mockReturnValueOnce(JSON.stringify('initial'));
382+
383+
const { result } = renderHook(() =>
384+
utils.useLocalStorage('testKey', 'initial'),
385+
);
386+
387+
// Update mock to return new value when checked after event
388+
localStorageMock.getItem.mockReturnValue(JSON.stringify('external update'));
389+
390+
// Dispatch storage event
391+
act(() => {
392+
window.dispatchEvent(
393+
new StorageEvent('storage', {
394+
key: 'testKey',
395+
newValue: JSON.stringify('external update'),
396+
}),
397+
);
398+
});
399+
400+
// State should be updated
401+
expect(result.current[0]).toBe('external update');
402+
});
403+
404+
test('should handle customStorage event from same window but different hook instance', () => {
405+
// First hook instance
406+
localStorageMock.getItem.mockReturnValueOnce(JSON.stringify('initial1'));
407+
const { result: result1 } = renderHook(() =>
408+
utils.useLocalStorage('sharedKey', 'initial1'),
409+
);
410+
411+
// Second hook instance
412+
localStorageMock.getItem.mockReturnValueOnce(JSON.stringify('initial1'));
413+
const { result: result2 } = renderHook(() =>
414+
utils.useLocalStorage('sharedKey', 'initial2'),
415+
);
416+
417+
// Clear mock calls count
418+
localStorageMock.getItem.mockClear();
419+
420+
// When the second hook checks localStorage after custom event
421+
localStorageMock.getItem.mockReturnValue(
422+
JSON.stringify('updated by hook 1'),
423+
);
424+
425+
// Update value in the first instance
426+
act(() => {
427+
result1.current[1]('updated by hook 1');
428+
});
429+
430+
// Manually trigger custom event (since it's happening within the same test)
431+
act(() => {
432+
const event = new CustomEvent<utils.CustomStorageChangeDetail>(
433+
'customStorage',
434+
{
435+
detail: {
436+
key: 'sharedKey',
437+
instanceId: 'some-id', // Different from the instance updating
438+
},
439+
},
440+
);
441+
window.dispatchEvent(event);
442+
});
443+
444+
// The second instance should have updated values
445+
expect(result2.current[0]).toBe('updated by hook 1');
446+
});
447+
448+
test('should not update if storage event is for a different key', () => {
449+
// Initial setup
450+
localStorageMock.getItem.mockReturnValueOnce(JSON.stringify('initial'));
451+
const { result } = renderHook(() =>
452+
utils.useLocalStorage('testKey', 'initial'),
453+
);
454+
455+
// Clear the mock calls counter
456+
localStorageMock.getItem.mockClear();
457+
458+
// Simulate storage event for a different key
459+
act(() => {
460+
window.dispatchEvent(
461+
new StorageEvent('storage', {
462+
key: 'differentKey',
463+
newValue: JSON.stringify('different value'),
464+
}),
465+
);
466+
});
467+
468+
// State should remain unchanged
469+
expect(result.current[0]).toBe('initial');
470+
// localStorage should not be accessed since key doesn't match
471+
expect(localStorageMock.getItem).not.toHaveBeenCalled();
472+
});
473+
});

‎packages/app/src/components/DBSearchPageFilters.tsx

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { IconSearch } from '@tabler/icons-react';
2020

2121
import { useAllFields, useGetKeyValues } from '@/hooks/useMetadata';
2222
import useResizable from '@/hooks/useResizable';
23-
import { useSearchPageFilterState } from '@/searchFilters';
23+
import { FilterStateHook, usePinnedFilters } from '@/searchFilters';
2424
import { mergePath } from '@/utils';
2525

2626
import resizeStyles from '../../styles/ResizablePanel.module.scss';
@@ -29,9 +29,11 @@ import classes from '../../styles/SearchPage.module.scss';
2929
type FilterCheckboxProps = {
3030
label: string;
3131
value?: 'included' | 'excluded' | false;
32+
pinned: boolean;
3233
onChange?: (checked: boolean) => void;
3334
onClickOnly?: VoidFunction;
3435
onClickExclude?: VoidFunction;
36+
onClickPin: VoidFunction;
3537
};
3638

3739
export const TextButton = ({
@@ -56,9 +58,11 @@ const emptyFn = () => {};
5658
export const FilterCheckbox = ({
5759
value,
5860
label,
61+
pinned,
5962
onChange,
6063
onClickOnly,
6164
onClickExclude,
65+
onClickPin,
6266
}: FilterCheckboxProps) => {
6367
return (
6468
<div className={classes.filterCheckbox}>
@@ -102,7 +106,16 @@ export const FilterCheckbox = ({
102106
{onClickExclude && (
103107
<TextButton onClick={onClickExclude} label="Exclude" />
104108
)}
109+
<TextButton
110+
onClick={onClickPin}
111+
label={<i className={`bi bi-pin-angle${pinned ? '-fill' : ''}`}></i>}
112+
/>
105113
</div>
114+
{pinned && (
115+
<Text size="xxs" c="gray.6">
116+
<i className="bi bi-pin-angle-fill"></i>
117+
</Text>
118+
)}
106119
</div>
107120
);
108121
};
@@ -119,6 +132,8 @@ export type FilterGroupProps = {
119132
onClearClick: VoidFunction;
120133
onOnlyClick: (value: string) => void;
121134
onExcludeClick: (value: string) => void;
135+
onPinClick: (value: string) => void;
136+
isPinned: (value: string) => boolean;
122137
};
123138

124139
const MAX_FILTER_GROUP_ITEMS = 10;
@@ -132,6 +147,8 @@ export const FilterGroup = ({
132147
onClearClick,
133148
onOnlyClick,
134149
onExcludeClick,
150+
isPinned,
151+
onPinClick,
135152
}: FilterGroupProps) => {
136153
const [search, setSearch] = useState('');
137154
const [isExpanded, setExpanded] = useState(false);
@@ -163,12 +180,18 @@ export const FilterGroup = ({
163180
a: (typeof augmentedOptions)[0],
164181
b: (typeof augmentedOptions)[0],
165182
) => {
183+
const aPinned = isPinned(a.value);
166184
const aIncluded = selectedValues.included.has(a.value);
167185
const aExcluded = selectedValues.excluded.has(a.value);
186+
const bPinned = isPinned(b.value);
168187
const bIncluded = selectedValues.included.has(b.value);
169188
const bExcluded = selectedValues.excluded.has(b.value);
170189

171-
// First sort by included status
190+
// First sort by pinned status
191+
if (aPinned && !bPinned) return -1;
192+
if (!aPinned && bPinned) return 1;
193+
194+
// Then sort by included status
172195
if (aIncluded && !bIncluded) return -1;
173196
if (!aIncluded && bIncluded) return 1;
174197

@@ -186,22 +209,8 @@ export const FilterGroup = ({
186209
}
187210

188211
// Do not rearrange items if all selected values are visible without expanding
189-
const shouldSortBySelected =
190-
isExpanded ||
191-
augmentedOptions.some(
192-
(option, index) =>
193-
(selectedValues.included.has(option.value) ||
194-
selectedValues.excluded.has(option.value)) &&
195-
index >= MAX_FILTER_GROUP_ITEMS,
196-
);
197-
198212
return augmentedOptions
199-
.slice()
200-
.sort((a, b) =>
201-
shouldSortBySelected
202-
? sortBySelectionAndAlpha(a, b)
203-
: a.value.localeCompare(b.value),
204-
)
213+
.sort((a, b) => sortBySelectionAndAlpha(a, b))
205214
.slice(
206215
0,
207216
Math.max(
@@ -255,6 +264,7 @@ export const FilterGroup = ({
255264
<FilterCheckbox
256265
key={option.value}
257266
label={option.label}
267+
pinned={isPinned(option.value)}
258268
value={
259269
selectedValues.included.has(option.value)
260270
? 'included'
@@ -265,6 +275,7 @@ export const FilterGroup = ({
265275
onChange={() => onChange(option.value)}
266276
onClickOnly={() => onOnlyClick(option.value)}
267277
onClickExclude={() => onExcludeClick(option.value)}
278+
onClickPin={() => onPinClick(option.value)}
268279
/>
269280
))}
270281
{optionsLoading ? (
@@ -304,8 +315,6 @@ export const FilterGroup = ({
304315
);
305316
};
306317

307-
type FilterStateHook = ReturnType<typeof useSearchPageFilterState>;
308-
309318
export const DBSearchPageFilters = ({
310319
filters: filterState,
311320
clearAllFilters,
@@ -315,12 +324,17 @@ export const DBSearchPageFilters = ({
315324
chartConfig,
316325
analysisMode,
317326
setAnalysisMode,
327+
sourceId,
318328
}: {
319329
analysisMode: 'results' | 'delta' | 'pattern';
320330
setAnalysisMode: (mode: 'results' | 'delta' | 'pattern') => void;
321331
isLive: boolean;
322332
chartConfig: ChartConfigWithDateRange;
333+
sourceId?: string;
323334
} & FilterStateHook) => {
335+
const { toggleFilterPin, isFilterPinned } = usePinnedFilters(
336+
sourceId ?? null,
337+
);
324338
const { width, startResize } = useResizable(16, 'left');
325339

326340
const { data, isLoading } = useAllFields({
@@ -504,6 +518,8 @@ export const DBSearchPageFilters = ({
504518
onExcludeClick={value => {
505519
setFilterValue(facet.key, value, 'exclude');
506520
}}
521+
onPinClick={value => toggleFilterPin(facet.key, value)}
522+
isPinned={value => isFilterPinned(facet.key, value)}
507523
/>
508524
))}
509525

0 commit comments

Comments
 (0)