Skip to content

Commit

Permalink
feat(query-builder): Add support for duration filters (#74447)
Browse files Browse the repository at this point in the history
Adds support for duration filters, which don't exist in issues search
but can in other contexts. You can test it on the stories page, where
I've added `measurements.lcp` as an option.
  • Loading branch information
malwilley committed Jul 17, 2024
1 parent 7763647 commit 950495d
Show file tree
Hide file tree
Showing 8 changed files with 232 additions and 19 deletions.
147 changes: 145 additions & 2 deletions static/app/components/searchQueryBuilder/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,18 @@ import {
within,
} from 'sentry-test/reactTestingLibrary';

import {SearchQueryBuilder} from 'sentry/components/searchQueryBuilder';
import {
SearchQueryBuilder,
type SearchQueryBuilderProps,
} from 'sentry/components/searchQueryBuilder';
import {
type FieldDefinitionGetter,
type FilterKeySection,
QueryInterfaceType,
} from 'sentry/components/searchQueryBuilder/types';
import {INTERFACE_TYPE_LOCALSTORAGE_KEY} from 'sentry/components/searchQueryBuilder/utils';
import type {TagCollection} from 'sentry/types/group';
import {FieldKey, FieldKind} from 'sentry/utils/fields';
import {FieldKey, FieldKind, FieldValueType} from 'sentry/utils/fields';
import localStorageWrapper from 'sentry/utils/localStorage';

const FILTER_KEYS: TagCollection = {
Expand Down Expand Up @@ -1340,6 +1344,145 @@ describe('SearchQueryBuilder', function () {
});
});

describe('duration', function () {
const durationFilterKeys: TagCollection = {
duration: {
key: 'duration',
name: 'Duration',
},
};

const fieldDefinitionGetter: FieldDefinitionGetter = () => ({
valueType: FieldValueType.DURATION,
kind: FieldKind.FIELD,
});

const durationProps: SearchQueryBuilderProps = {
...defaultProps,
filterKeys: durationFilterKeys,
filterKeySections: [],
fieldDefinitionGetter,
};

it('new duration filters start with greater than operator and default value', async function () {
render(<SearchQueryBuilder {...durationProps} />);
await userEvent.click(getLastInput());
await userEvent.click(screen.getByRole('option', {name: 'duration'}));

// Should start with the > operator and a value of 10ms
expect(
await screen.findByRole('row', {name: 'duration:>10ms'})
).toBeInTheDocument();
});

it('duration filters have the correct operator options', async function () {
render(<SearchQueryBuilder {...durationProps} initialQuery="duration:>100ms" />);
await userEvent.click(
screen.getByRole('button', {name: 'Edit operator for filter: duration'})
);

expect(
await screen.findByRole('option', {name: 'duration is'})
).toBeInTheDocument();
expect(screen.getByRole('option', {name: 'duration is not'})).toBeInTheDocument();
expect(screen.getByRole('option', {name: 'duration >'})).toBeInTheDocument();
expect(screen.getByRole('option', {name: 'duration <'})).toBeInTheDocument();
expect(screen.getByRole('option', {name: 'duration >='})).toBeInTheDocument();
expect(screen.getByRole('option', {name: 'duration <='})).toBeInTheDocument();
});

it('duration filters have the correct value suggestions', async function () {
render(<SearchQueryBuilder {...durationProps} initialQuery="duration:>100ms" />);
await userEvent.click(
screen.getByRole('button', {name: 'Edit value for filter: duration'})
);

// Default suggestions
expect(await screen.findByRole('option', {name: '100ms'})).toBeInTheDocument();
expect(screen.getByRole('option', {name: '100s'})).toBeInTheDocument();
expect(screen.getByRole('option', {name: '100m'})).toBeInTheDocument();
expect(screen.getByRole('option', {name: '100h'})).toBeInTheDocument();

// Entering a number will show unit suggestions for that value
await userEvent.keyboard('7');
expect(await screen.findByRole('option', {name: '7ms'})).toBeInTheDocument();
expect(screen.getByRole('option', {name: '7s'})).toBeInTheDocument();
expect(screen.getByRole('option', {name: '7m'})).toBeInTheDocument();
expect(screen.getByRole('option', {name: '7h'})).toBeInTheDocument();
});

it('duration filters can change operator', async function () {
render(<SearchQueryBuilder {...durationProps} initialQuery="duration:>100ms" />);
await userEvent.click(
screen.getByRole('button', {name: 'Edit operator for filter: duration'})
);

await userEvent.click(await screen.findByRole('option', {name: 'duration <='}));

expect(
await screen.findByRole('row', {name: 'duration:<=100ms'})
).toBeInTheDocument();
});

it('duration filters do not allow invalid values', async function () {
render(<SearchQueryBuilder {...durationProps} initialQuery="duration:>100ms" />);
await userEvent.click(
screen.getByRole('button', {name: 'Edit value for filter: duration'})
);

await userEvent.keyboard('a{Enter}');

// Should have the same value because "a" is not a numeric value
expect(screen.getByRole('row', {name: 'duration:>100ms'})).toBeInTheDocument();

await userEvent.keyboard('{Backspace}7m{Enter}');

// Should accept "7m" as a valid value
expect(
await screen.findByRole('row', {name: 'duration:>7m'})
).toBeInTheDocument();
});

it('duration filters will add a default unit to entered numbers', async function () {
render(<SearchQueryBuilder {...durationProps} initialQuery="duration:>100ms" />);
await userEvent.click(
screen.getByRole('button', {name: 'Edit value for filter: duration'})
);

await userEvent.keyboard('7{Enter}');

// Should accept "7" and add "ms" as the default unit
expect(
await screen.findByRole('row', {name: 'duration:>7ms'})
).toBeInTheDocument();
});

it('keeps previous value when confirming empty value', async function () {
const mockOnChange = jest.fn();
render(
<SearchQueryBuilder
{...durationProps}
onChange={mockOnChange}
initialQuery="duration:>100ms"
/>
);

await userEvent.click(
screen.getByRole('button', {name: 'Edit value for filter: duration'})
);
await userEvent.clear(
await screen.findByRole('combobox', {name: 'Edit filter value'})
);
await userEvent.keyboard('{enter}');

// Should have the same value
expect(
await screen.findByRole('row', {name: 'duration:>100ms'})
).toBeInTheDocument();
expect(mockOnChange).not.toHaveBeenCalled();
});
});

describe('date', function () {
// Transpile the lazy-loaded datepicker up front so tests don't flake
beforeAll(async function () {
Expand Down
8 changes: 7 additions & 1 deletion static/app/components/searchQueryBuilder/index.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type {FilterKeySection} from 'sentry/components/searchQueryBuilder/types'
import SizingWindow from 'sentry/components/stories/sizingWindow';
import storyBook from 'sentry/stories/storyBook';
import type {TagCollection} from 'sentry/types/group';
import {FieldKey, FieldKind} from 'sentry/utils/fields';
import {FieldKey, FieldKind, WebVital} from 'sentry/utils/fields';

const FILTER_KEYS: TagCollection = {
[FieldKey.AGE]: {key: FieldKey.AGE, name: 'Age', kind: FieldKind.FIELD},
Expand Down Expand Up @@ -49,6 +49,11 @@ const FILTER_KEYS: TagCollection = {
name: 'timesSeen',
kind: FieldKind.FIELD,
},
[WebVital.LCP]: {
key: WebVital.LCP,
name: 'lcp',
kind: FieldKind.FIELD,
},
custom_tag_name: {
key: 'custom_tag_name',
name: 'Custom_Tag_Name',
Expand All @@ -65,6 +70,7 @@ const FITLER_KEY_SECTIONS: FilterKeySection[] = [
FieldKey.BROWSER_NAME,
FieldKey.IS,
FieldKey.TIMES_SEEN,
WebVital.LCP,
],
},
{
Expand Down
2 changes: 1 addition & 1 deletion static/app/components/searchQueryBuilder/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import PanelProvider from 'sentry/utils/panelProvider';
import {useDimensions} from 'sentry/utils/useDimensions';
import {useEffectAfterFirstRender} from 'sentry/utils/useEffectAfterFirstRender';

interface SearchQueryBuilderProps {
export interface SearchQueryBuilderProps {
/**
* A complete mapping of all possible filter keys.
* Filter keys not included will not show up when typing and may be shown as invalid.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
value = duration_format

duration_format
= value:numeric
unit:duration_unit? {
return {value, unit}
}

duration_unit = "ms"/"s"/"min"/"m"/"hr"/"h"/"day"/"d"/"wk"/"w"
numeric = [0-9]+ ("." [0-9]*)? { return text(); }
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import grammar from './grammar.pegjs';

type DurationTokenValue = {
value: string;
unit?: string;
};

/**
* This parser is specifically meant for parsing the value of a duartion filter.
* This should mostly mirror the grammar used for search syntax, but is a little
* more lenient. This parser still returns a valid result if the duration does
* not contain a unit which can be used to help create a valid duration or show
* search suggestions.
*/
export function parseFilterValueDuration(query: string): DurationTokenValue | null {
try {
return grammar.parse(query);
} catch (e) {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {getItemsWithKeys} from 'sentry/components/compactSelect/utils';
import {useSearchQueryBuilder} from 'sentry/components/searchQueryBuilder/context';
import {SearchQueryBuilderCombobox} from 'sentry/components/searchQueryBuilder/tokens/combobox';
import {parseFilterValueDate} from 'sentry/components/searchQueryBuilder/tokens/filter/parsers/date/parser';
import {parseFilterValueDuration} from 'sentry/components/searchQueryBuilder/tokens/filter/parsers/duration/parser';
import SpecificDatePicker from 'sentry/components/searchQueryBuilder/tokens/filter/specificDatePicker';
import {
escapeTagValue,
Expand Down Expand Up @@ -81,7 +82,7 @@ function isStringFilterValues(

const NUMERIC_UNITS = ['k', 'm', 'b'] as const;
const RELATIVE_DATE_UNITS = ['m', 'h', 'd', 'w'] as const;
const DURATION_UNITS = ['ms', 's', 'm', 'h', 'd', 'w'] as const;
const DURATION_UNIT_SUGGESTIONS = ['ms', 's', 'm', 'h'] as const;

const DEFAULT_NUMERIC_SUGGESTIONS: SuggestionSection[] = [
{
Expand All @@ -93,7 +94,7 @@ const DEFAULT_NUMERIC_SUGGESTIONS: SuggestionSection[] = [
const DEFAULT_DURATION_SUGGESTIONS: SuggestionSection[] = [
{
sectionText: '',
suggestions: [{value: '100'}, {value: '100k'}, {value: '100m'}, {value: '100b'}],
suggestions: DURATION_UNIT_SUGGESTIONS.map(unit => ({value: `10${unit}`})),
},
];

Expand Down Expand Up @@ -276,23 +277,42 @@ function getNumericSuggestions(inputValue: string): SuggestionSection[] {
return [];
}

function getDurationSuggestions(inputValue: string): SuggestionSection[] {
function getDurationSuggestions(
inputValue: string,
token: TokenResult<Token.FILTER>
): SuggestionSection[] {
if (!inputValue) {
return DEFAULT_DURATION_SUGGESTIONS;
const currentValue =
token.value.type === Token.VALUE_DURATION ? token.value.value : null;

if (!currentValue) {
return DEFAULT_DURATION_SUGGESTIONS;
}

return [
{
sectionText: '',
suggestions: DURATION_UNIT_SUGGESTIONS.map(unit => ({
value: `${currentValue}${unit}`,
})),
},
];
}

if (isNumeric(inputValue)) {
const parsed = parseFilterValueDuration(inputValue);

if (parsed) {
return [
{
sectionText: '',
suggestions: DURATION_UNITS.map(unit => ({
value: `${inputValue}${unit}`,
suggestions: DURATION_UNIT_SUGGESTIONS.map(unit => ({
value: `${parsed.value}${unit}`,
})),
},
];
}

// If the value is not numeric, don't show any suggestions
// If the value doesn't contain any valid number or duration, don't show any suggestions
return [];
}

Expand Down Expand Up @@ -348,24 +368,23 @@ function getPredefinedValues({
filterValue: string;
token: TokenResult<Token.FILTER>;
key?: Tag;
}): SuggestionSection[] {
}): SuggestionSection[] | null {
if (!key) {
return [];
return null;
}

if (!key.values?.length) {
switch (fieldDefinition?.valueType) {
case FieldValueType.NUMBER:
return getNumericSuggestions(filterValue);
case FieldValueType.DURATION:
return getDurationSuggestions(filterValue);
return getDurationSuggestions(filterValue, token);
case FieldValueType.BOOLEAN:
return DEFAULT_BOOLEAN_SUGGESTIONS;
// TODO(malwilley): Better date suggestions
case FieldValueType.DATE:
return getRelativeDateSuggestions(filterValue, token);
default:
return [];
return null;
}
}

Expand Down Expand Up @@ -450,6 +469,17 @@ function cleanFilterValue(
return value;
}
return null;
case FieldValueType.DURATION: {
const parsed = parseFilterValueDuration(value);
if (!parsed) {
return null;
}
// Default to ms if no unit is provided
if (!parsed.unit) {
return `${parsed.value}ms`;
}
return value;
}
case FieldValueType.DATE:
const parsed = parseFilterValueDate(value);

Expand Down Expand Up @@ -517,7 +547,7 @@ function useFilterSuggestions({
}),
[key, filterValue, token, fieldDefinition]
);
const shouldFetchValues = key && !key.predefined && !predefinedValues.length;
const shouldFetchValues = key && !key.predefined && predefinedValues === null;
const canSelectMultipleValues = tokenSupportsMultipleValues(
token,
filterKeys,
Expand Down Expand Up @@ -572,7 +602,7 @@ function useFilterSuggestions({
const suggestionGroups: SuggestionSection[] = useMemo(() => {
return shouldFetchValues
? [{sectionText: '', suggestions: data?.map(value => ({value})) ?? []}]
: predefinedValues;
: predefinedValues ?? [];
}, [data, predefinedValues, shouldFetchValues]);

// Grouped sections for rendering purposes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ function getInitialFilterText(key: string, fieldDefinition: FieldDefinition | nu
switch (fieldDefinition?.valueType) {
case FieldValueType.INTEGER:
case FieldValueType.NUMBER:
case FieldValueType.DURATION:
return `${key}:>${defaultValue}`;
case FieldValueType.STRING:
default:
Expand Down
2 changes: 2 additions & 0 deletions static/app/components/searchQueryBuilder/tokens/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ export function getDefaultFilterValue({
return '100';
case FieldValueType.DATE:
return '-24h';
case FieldValueType.DURATION:
return '10ms';
case FieldValueType.STRING:
default:
return '""';
Expand Down

0 comments on commit 950495d

Please sign in to comment.