Skip to content

Commit

Permalink
feat(query-builder): Add basic support for selecting multiple values (#…
Browse files Browse the repository at this point in the history
…71457)

Ref #69791

For text and number filters, add checkboxes and allow multiple value
selection. Other filter types retain the same functionality as before.

Behavior is as follows:

- Clicking the row selects the value and exits the token
- Clicking the checkbox selects the value and keeps focus inside the
token
- Pressing backspace will unselect the previous value
- Selected values are always shown at the top of the flyout
  • Loading branch information
malwilley committed May 28, 2024
1 parent 89e7793 commit 3f9ba9a
Show file tree
Hide file tree
Showing 4 changed files with 339 additions and 63 deletions.
2 changes: 1 addition & 1 deletion static/app/components/searchQueryBuilder/filter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ function FilterValue({token, state, item}: SearchQueryTokenProps) {
<ValueEditing ref={ref} {...mergeProps(focusWithinProps, filterButtonProps)}>
<SearchQueryBuilderValueCombobox
token={token}
onChange={() => {
onCommit={() => {
setIsEditing(false);
if (state.collection.getKeyAfter(item.key)) {
state.selectionManager.setFocusedKey(
Expand Down
76 changes: 57 additions & 19 deletions static/app/components/searchQueryBuilder/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ import type {TagCollection} from 'sentry/types/group';
import {FieldKey, FieldKind} from 'sentry/utils/fields';

const MOCK_SUPPORTED_KEYS: TagCollection = {
[FieldKey.AGE]: {key: FieldKey.AGE, name: 'Age', kind: FieldKind.FIELD},
[FieldKey.AGE]: {
key: FieldKey.AGE,
name: 'Age',
kind: FieldKind.FIELD,
predefined: true,
},
[FieldKey.ASSIGNED]: {
key: FieldKey.ASSIGNED,
name: 'Assigned To',
Expand Down Expand Up @@ -95,55 +100,88 @@ describe('SearchQueryBuilder', function () {
).toBeInTheDocument();
});

it('can modify the value by clicking into it', async function () {
render(
<SearchQueryBuilder {...defaultProps} initialQuery="browser.name:firefox" />
);
it('can modify the value by clicking into it (single-select)', async function () {
// `age` is a duration filter which only accepts single values
render(<SearchQueryBuilder {...defaultProps} initialQuery="age:-1d" />);

// Should display as "firefox" to start
// Should display as "-1d" to start
expect(
within(
screen.getByRole('button', {name: 'Edit value for filter: browser.name'})
).getByText('firefox')
screen.getByRole('button', {name: 'Edit value for filter: age'})
).getByText('-1d')
).toBeInTheDocument();

await userEvent.click(
screen.getByRole('button', {name: 'Edit value for filter: browser.name'})
screen.getByRole('button', {name: 'Edit value for filter: age'})
);
// Should have placeholder text of previous value
expect(screen.getByRole('combobox', {name: 'Edit filter value'})).toHaveAttribute(
'placeholder',
'firefox'
'-1d'
);
await userEvent.click(screen.getByRole('combobox', {name: 'Edit filter value'}));

// Clicking the "Chrome option should update the value"
await userEvent.click(screen.getByRole('option', {name: 'Chrome'}));
expect(screen.getByRole('row', {name: 'browser.name:Chrome'})).toBeInTheDocument();
// Clicking the "+14d" option should update the value
await userEvent.click(screen.getByRole('option', {name: '+14d'}));
expect(screen.getByRole('row', {name: 'age:+14d'})).toBeInTheDocument();
expect(
within(
screen.getByRole('button', {name: 'Edit value for filter: browser.name'})
).getByText('Chrome')
screen.getByRole('button', {name: 'Edit value for filter: age'})
).getByText('+14d')
).toBeInTheDocument();
});

it('escapes values with spaces and reserved characters', async function () {
it('can modify the value by clicking into it (multi-select)', async function () {
// `browser.name` is a string filter which accepts multiple values
render(
<SearchQueryBuilder {...defaultProps} initialQuery="browser.name:firefox" />
);

// Should display as "firefox" to start
expect(
within(
screen.getByRole('button', {name: 'Edit value for filter: browser.name'})
).getByText('firefox')
).toBeInTheDocument();

await userEvent.click(
screen.getByRole('button', {name: 'Edit value for filter: browser.name'})
);
await userEvent.keyboard('some" value{enter}');
// Previous value should be rendered before the input
expect(
within(screen.getByRole('row', {name: 'browser.name:firefox'})).getByText(
'firefox,'
)
).toBeInTheDocument();
await userEvent.click(screen.getByRole('combobox', {name: 'Edit filter value'}));

// Clicking the "Chrome option should add it to the list and commit changes
await userEvent.click(screen.getByRole('option', {name: 'Chrome'}));
expect(
screen.getByRole('row', {name: 'browser.name:[firefox,Chrome]'})
).toBeInTheDocument();
expect(
within(
screen.getByRole('button', {name: 'Edit value for filter: browser.name'})
).getByText('[firefox,Chrome]')
).toBeInTheDocument();
});

it('escapes values with spaces and reserved characters', async function () {
render(<SearchQueryBuilder {...defaultProps} initialQuery="" />);
await userEvent.click(screen.getByRole('combobox', {name: 'Add a search term'}));
await userEvent.type(
screen.getByRole('combobox', {name: 'Add a search term'}),
'assigned:some" value{enter}'
);
// Value should be surrounded by quotes and escaped
expect(
screen.getByRole('row', {name: 'browser.name:"some\\" value"'})
screen.getByRole('row', {name: 'assigned:"some\\" value"'})
).toBeInTheDocument();
// Display text should be display the original value
expect(
within(
screen.getByRole('button', {name: 'Edit value for filter: browser.name'})
screen.getByRole('button', {name: 'Edit value for filter: assigned'})
).getByText('some" value')
).toBeInTheDocument();
});
Expand Down
85 changes: 83 additions & 2 deletions static/app/components/searchQueryBuilder/useQueryBuilderState.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {type Reducer, useCallback, useReducer} from 'react';
import {
type ParseResultToken,
TermOperator,
type Token,
Token,
type TokenResult,
} from 'sentry/components/searchSyntax/parser';
import {stringifyToken} from 'sentry/components/searchSyntax/utils';
Expand Down Expand Up @@ -35,11 +35,24 @@ type UpdateTokenValueAction = {
value: string;
};

type MultiSelectFilterValueAction = {
token: TokenResult<Token.FILTER>;
type: 'TOGGLE_FILTER_VALUE';
value: string;
};

type DeleteLastMultiSelectFilterValueAction = {
token: TokenResult<Token.FILTER>;
type: 'DELETE_LAST_MULTI_SELECT_FILTER_VALUE';
};

export type QueryBuilderActions =
| DeleteTokenAction
| UpdateFreeTextAction
| UpdateFilterOpAction
| UpdateTokenValueAction;
| UpdateTokenValueAction
| MultiSelectFilterValueAction
| DeleteLastMultiSelectFilterValueAction;

function removeQueryToken(query: string, token: TokenResult<Token>): string {
return (
Expand Down Expand Up @@ -101,6 +114,70 @@ function updateFreeText(
};
}

function updateFilterMultipleValues(
state: QueryBuilderState,
token: TokenResult<Token.FILTER>,
values: string[]
) {
const uniqNonEmptyValues = Array.from(
new Set(values.filter(value => value.length > 0))
);
if (uniqNonEmptyValues.length === 0) {
return {...state, query: replaceQueryToken(state.query, token.value, '')};
}

const newValue =
uniqNonEmptyValues.length > 1
? `[${uniqNonEmptyValues.join(',')}]`
: uniqNonEmptyValues[0];

return {...state, query: replaceQueryToken(state.query, token.value, newValue)};
}

function multiSelectTokenValue(
state: QueryBuilderState,
action: MultiSelectFilterValueAction
) {
const tokenValue = action.token.value;

switch (tokenValue.type) {
case Token.VALUE_TEXT_LIST:
case Token.VALUE_NUMBER_LIST:
const values = tokenValue.items.map(item => item.value?.text ?? '');
const containsValue = values.includes(action.value);
const newValues = containsValue
? values.filter(value => value !== action.value)
: [...values, action.value];

return updateFilterMultipleValues(state, action.token, newValues);
default:
if (tokenValue.text === action.value) {
return updateFilterMultipleValues(state, action.token, ['']);
}
return updateFilterMultipleValues(state, action.token, [
tokenValue.text,
action.value,
]);
}
}

function deleteLastMultiSelectTokenValue(
state: QueryBuilderState,
action: DeleteLastMultiSelectFilterValueAction
) {
const tokenValue = action.token.value;

switch (tokenValue.type) {
case Token.VALUE_TEXT_LIST:
case Token.VALUE_NUMBER_LIST:
const newValues = tokenValue.items.slice(0, -1).map(item => item.value?.text ?? '');

return updateFilterMultipleValues(state, action.token, newValues);
default:
return updateFilterMultipleValues(state, action.token, ['']);
}
}

export function useQueryBuilderState({initialQuery}: {initialQuery: string}) {
const initialState: QueryBuilderState = {query: initialQuery};

Expand All @@ -121,6 +198,10 @@ export function useQueryBuilderState({initialQuery}: {initialQuery: string}) {
return {
query: replaceQueryToken(state.query, action.token, action.value),
};
case 'TOGGLE_FILTER_VALUE':
return multiSelectTokenValue(state, action);
case 'DELETE_LAST_MULTI_SELECT_FILTER_VALUE':
return deleteLastMultiSelectTokenValue(state, action);
default:
return state;
}
Expand Down
Loading

0 comments on commit 3f9ba9a

Please sign in to comment.