diff --git a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.spec.tsx b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.spec.tsx index 7e7c350402d9a9..b597f412601640 100644 --- a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.spec.tsx +++ b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.spec.tsx @@ -1,10 +1,7 @@ import type {FocusOverride} from 'sentry/components/searchQueryBuilder/types'; import {WildcardOperators} from 'sentry/components/searchSyntax/parser'; -import { - replaceFreeTextTokens, - type ReplaceTokensWithTextOnPasteAction, -} from './useQueryBuilderState'; +import {replaceFreeTextTokens} from './useQueryBuilderState'; describe('replaceFreeTextTokens', () => { describe('when there are free text tokens', () => { @@ -15,7 +12,6 @@ describe('replaceFreeTextTokens', () => { query: string | undefined; }; input: { - action: ReplaceTokensWithTextOnPasteAction; currentQuery: string; getFieldDefinition: () => null; rawSearchReplacement: string[]; @@ -26,12 +22,6 @@ describe('replaceFreeTextTokens', () => { { description: 'when there are no tokens', input: { - action: { - type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE', - text: '', - tokens: [], - focusOverride: undefined, - }, getFieldDefinition: () => null, rawSearchReplacement: ['span.description'], currentQuery: '', @@ -44,12 +34,6 @@ describe('replaceFreeTextTokens', () => { { description: 'when the replace raw search keys is empty', input: { - action: { - type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE', - text: '', - tokens: [], - focusOverride: undefined, - }, getFieldDefinition: () => null, rawSearchReplacement: [], currentQuery: '', @@ -62,12 +46,6 @@ describe('replaceFreeTextTokens', () => { { description: 'when the replace raw search keys is an empty string', input: { - action: { - type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE', - text: '', - tokens: [], - focusOverride: undefined, - }, getFieldDefinition: () => null, rawSearchReplacement: [''], currentQuery: '', @@ -80,12 +58,6 @@ describe('replaceFreeTextTokens', () => { { description: 'when there is no raw search replacement', input: { - action: { - type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE', - text: '', - tokens: [], - focusOverride: undefined, - }, getFieldDefinition: () => null, rawSearchReplacement: [], currentQuery: `browser.name:${WildcardOperators.CONTAINS}"firefox"`, @@ -98,12 +70,6 @@ describe('replaceFreeTextTokens', () => { { description: 'when there are no free text tokens', input: { - action: { - type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE', - text: '', - tokens: [], - focusOverride: undefined, - }, getFieldDefinition: () => null, rawSearchReplacement: ['span.description'], currentQuery: `browser.name:${WildcardOperators.CONTAINS}"firefox"`, @@ -116,15 +82,9 @@ describe('replaceFreeTextTokens', () => { { description: 'when there only valid action tokens', input: { - action: { - type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE', - text: 'span.op:eq', - tokens: [], - focusOverride: undefined, - }, getFieldDefinition: () => null, rawSearchReplacement: ['span.description'], - currentQuery: '', + currentQuery: 'span.op:eq', }, expected: { query: undefined, @@ -134,15 +94,9 @@ describe('replaceFreeTextTokens', () => { { description: 'when there only space free text tokens in the action', input: { - action: { - type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE', - text: 'span.op:eq ', - tokens: [], - focusOverride: undefined, - }, getFieldDefinition: () => null, rawSearchReplacement: ['span.description'], - currentQuery: '', + currentQuery: 'span.op:eq ', }, expected: { query: undefined, @@ -152,15 +106,9 @@ describe('replaceFreeTextTokens', () => { { description: 'when there is one free text token', input: { - action: { - type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE', - text: 'test', - tokens: [], - focusOverride: undefined, - }, getFieldDefinition: () => null, rawSearchReplacement: ['span.description'], - currentQuery: '', + currentQuery: 'test', }, expected: { query: `span.description:${WildcardOperators.CONTAINS}test`, @@ -170,15 +118,9 @@ describe('replaceFreeTextTokens', () => { { description: 'when there is one free text token that has a space', input: { - action: { - type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE', - text: 'test test', - tokens: [], - focusOverride: undefined, - }, getFieldDefinition: () => null, rawSearchReplacement: ['span.description'], - currentQuery: '', + currentQuery: 'test test', }, expected: { query: `span.description:${WildcardOperators.CONTAINS}"test test"`, @@ -188,15 +130,9 @@ describe('replaceFreeTextTokens', () => { { description: 'when there is already a token present', input: { - action: { - type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE', - text: 'test', - tokens: [], - focusOverride: undefined, - }, getFieldDefinition: () => null, rawSearchReplacement: ['span.description'], - currentQuery: 'span.op:eq', + currentQuery: 'span.op:eq test', }, expected: { query: `span.op:eq span.description:${WildcardOperators.CONTAINS}test`, @@ -206,52 +142,34 @@ describe('replaceFreeTextTokens', () => { { description: 'when there is already a replace token present', input: { - action: { - type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE', - text: 'test2', - tokens: [], - focusOverride: undefined, - }, getFieldDefinition: () => null, rawSearchReplacement: ['span.description'], - currentQuery: `span.description:${WildcardOperators.CONTAINS}test`, + currentQuery: `span.description:${WildcardOperators.CONTAINS}test test2`, }, expected: { - query: `span.description:${WildcardOperators.CONTAINS}[test,test2]`, - focusOverride: {itemKey: 'freeText:1'}, + query: `span.description:${WildcardOperators.CONTAINS}test span.description:${WildcardOperators.CONTAINS}test2`, + focusOverride: {itemKey: 'freeText:2'}, }, }, { description: 'when there is already a replace token present with a space', input: { - action: { - type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE', - text: 'other value', - tokens: [], - focusOverride: undefined, - }, getFieldDefinition: () => null, rawSearchReplacement: ['span.description'], - currentQuery: `span.description:${WildcardOperators.CONTAINS}test`, + currentQuery: `span.description:${WildcardOperators.CONTAINS}test other value`, }, expected: { - query: `span.description:${WildcardOperators.CONTAINS}[test,"other value"]`, - focusOverride: {itemKey: 'freeText:1'}, + query: `span.description:${WildcardOperators.CONTAINS}test span.description:${WildcardOperators.CONTAINS}"other value"`, + focusOverride: {itemKey: 'freeText:2'}, }, }, { description: 'when there is already a replace token present with a different operator', input: { - action: { - type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE', - text: 'other value', - tokens: [], - focusOverride: undefined, - }, getFieldDefinition: () => null, rawSearchReplacement: ['span.description'], - currentQuery: `span.description:test`, + currentQuery: `span.description:test other value`, }, expected: { query: `span.description:test span.description:${WildcardOperators.CONTAINS}"other value"`, @@ -262,10 +180,9 @@ describe('replaceFreeTextTokens', () => { it.each(testCases)('$description', ({input, expected}) => { const result = replaceFreeTextTokens( - input.action, + input.currentQuery, input.getFieldDefinition, - input.rawSearchReplacement, - input.currentQuery + input.rawSearchReplacement ); expect(result?.newQuery).toBe(expected.query); diff --git a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx index 60f07e5cd9760c..cead360f31ef37 100644 --- a/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx +++ b/static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.tsx @@ -135,7 +135,7 @@ type UpdateFreeTextActionOnColon = { focusOverride?: FocusOverride; }; -export type ReplaceTokensWithTextOnPasteAction = { +type ReplaceTokensWithTextOnPasteAction = { text: string; tokens: ParseResultToken[]; type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE'; @@ -658,21 +658,6 @@ function updateFilterKey( }; } -/** - * Check to see if the provided token details match the primary search key and operator. - * If so, we want to replace this token, with the merged filter value. - */ -function isTokenToBeReplaced( - token: TokenResult, - primarySearchKey: string -): token is TokenResult { - return ( - token.type === Token.FILTER && - getKeyName(token.key) === primarySearchKey && - token.operator === TermOperator.CONTAINS - ); -} - /** * This function is used to replace free text tokens with the specified * `replaceRawSearchKeys` prop from `SearchQueryBuilder`. This function also handles @@ -686,91 +671,66 @@ function isTokenToBeReplaced( * description:[*test*,"*some text*"]` */ export function replaceFreeTextTokens( - action: - | ReplaceTokensWithTextOnPasteAction - | UpdateFreeTextActionOnCommit - | UpdateFreeTextActionOnBlur - | UpdateFreeTextActionOnExit, + currentQuery: string, getFieldDefinition: FieldDefinitionGetter, - replaceRawSearchKeys: string[], - currentQuery: string + replaceRawSearchKeys: string[] ) { if ( - !action.text || - action.text === '' || + currentQuery.trim().length === 0 || replaceRawSearchKeys.length === 0 || (replaceRawSearchKeys.length !== 0 && replaceRawSearchKeys[0] === '') ) { return undefined; } - const actionTokens = parseQueryBuilderValue(action.text, getFieldDefinition) ?? []; - if (actionTokens.every(token => token.type !== Token.FREE_TEXT)) { - return undefined; - } - - const tokens = parseQueryBuilderValue(currentQuery, getFieldDefinition) ?? []; + const currentQueryTokens = + parseQueryBuilderValue(currentQuery, getFieldDefinition) ?? []; - // TS doesn't know that replaceRawSearchKeys is always defined and non-empty - const primarySearchKey = replaceRawSearchKeys[0] ?? ''; - let tokenToBeReplaced: TokenResult | undefined; - const freeTextToken = actionTokens.find( - token => token.type === Token.FREE_TEXT && /\w/.test(token.value) + const foundFreeTextToken = currentQueryTokens.some( + token => token.type === Token.FREE_TEXT && token.text.trim().length > 0 ); - for (const token of tokens) { - if (isTokenToBeReplaced(token, primarySearchKey)) { - tokenToBeReplaced = token; - break; - } - } - - const valueText = freeTextToken?.text.trim(); - if (!valueText) { + if (!foundFreeTextToken) { return undefined; } - const values = escapeTagValue(valueText); - const filteredTokens = new Set(); - actionTokens.forEach(token => { - const isNotFreeText = token.type !== Token.FREE_TEXT; + const primarySearchKey = replaceRawSearchKeys[0] ?? ''; + const replacedQuery: string[] = []; + for (const token of currentQueryTokens) { + if (token.type === Token.L_PAREN) { + replacedQuery.push('('); + continue; + } - if (isNotFreeText && !isTokenToBeReplaced(token, primarySearchKey)) { - filteredTokens.add(token.text); + if (token.type === Token.R_PAREN) { + replacedQuery.push(')'); + continue; } - }); - tokens.forEach(token => { - const isNotFreeText = token.type !== Token.FREE_TEXT; - if (isNotFreeText && !isTokenToBeReplaced(token, primarySearchKey)) { - filteredTokens.add(token.text); + if (token.type !== Token.FREE_TEXT) { + const stringifiedToken = stringifyToken(token); + if (stringifiedToken.length > 0) { + replacedQuery.push(stringifiedToken); + } + continue; } - }); - - // case when there is a replace key and value present - if (tokenToBeReplaced) { - const previousValue = - tokenToBeReplaced.value.type === Token.VALUE_TEXT_LIST - ? tokenToBeReplaced.value.text.slice(1, -1) - : tokenToBeReplaced.value.text; - - filteredTokens.add( - `${primarySearchKey}:${WildcardOperators.CONTAINS}[${previousValue},${values}]` - ); - } else { - filteredTokens.add(`${primarySearchKey}:${WildcardOperators.CONTAINS}${values}`); - } - const newQuery = Array.from(filteredTokens).join(' '); + if (token.text.trim().length === 0) { + continue; + } - const newParsedQuery = parseQueryBuilderValue(newQuery, getFieldDefinition) ?? []; - const focusedToken = newParsedQuery?.findLast(token => token.type === Token.FREE_TEXT); + const value = escapeTagValue(token.text.trim()); + replacedQuery.push(`${primarySearchKey}:${WildcardOperators.CONTAINS}${value}`); + } + const finalQuery = replacedQuery.join(' ').trim(); + const newParsedQuery = parseQueryBuilderValue(finalQuery, getFieldDefinition) ?? []; + const focusedToken = newParsedQuery?.findLast(token => token.type === Token.FREE_TEXT); const focusOverride = focusedToken ? {itemKey: makeTokenKey(focusedToken, newParsedQuery)} : null; - return {newQuery, focusOverride}; + return {newQuery: finalQuery, focusOverride}; } function updateFreeTextAndReplaceText( @@ -794,10 +754,9 @@ function updateFreeTextAndReplaceText( } const replacedState = replaceFreeTextTokens( - action, + newState.query, getFieldDefinition, - replaceRawSearchKeys ?? [], - newState.query + replaceRawSearchKeys ?? [] ); const query = replacedState?.newQuery ? replacedState.newQuery : newState.query; @@ -873,12 +832,33 @@ export function useQueryBuilderState({ return {...state, committedQuery: state.query}; case 'UPDATE_QUERY': { const shouldCommitQuery = action.shouldCommitQuery ?? true; - return { - ...state, - query: action.query, - committedQuery: shouldCommitQuery ? action.query : state.committedQuery, - focusOverride: action.focusOverride ?? null, - }; + + if ( + !hasWildcardOperators || + !replaceRawSearchKeys || + replaceRawSearchKeys.length === 0 + ) { + return { + ...state, + query: action.query, + committedQuery: shouldCommitQuery ? action.query : state.committedQuery, + focusOverride: action.focusOverride ?? null, + }; + } + + const replacedState = replaceFreeTextTokens( + action.query, + getFieldDefinition, + replaceRawSearchKeys + ); + + const query = replacedState?.newQuery ? replacedState.newQuery : action.query; + const committedQuery = shouldCommitQuery ? query : state.committedQuery; + const focusOverride = replacedState?.focusOverride + ? replacedState.focusOverride + : (action.focusOverride ?? null); + + return {...state, query, committedQuery, focusOverride}; } case 'RESET_FOCUS_OVERRIDE': return { @@ -957,10 +937,9 @@ export function useQueryBuilderState({ } const replacedState = replaceFreeTextTokens( - action, + newState.query, getFieldDefinition, - replaceRawSearchKeys ?? [], - state.query + replaceRawSearchKeys ?? [] ); const query = replacedState?.newQuery ? replacedState.newQuery : newState.query; diff --git a/static/app/components/searchQueryBuilder/index.spec.tsx b/static/app/components/searchQueryBuilder/index.spec.tsx index df787893ac8051..a55ceca237c5e7 100644 --- a/static/app/components/searchQueryBuilder/index.spec.tsx +++ b/static/app/components/searchQueryBuilder/index.spec.tsx @@ -4344,7 +4344,12 @@ describe('SearchQueryBuilder', () => { // Should have tokenized the pasted text expect( screen.getByRole('row', { - name: `span.description:${WildcardOperators.CONTAINS}[test,randomValue]`, + name: `span.description:${WildcardOperators.CONTAINS}test`, + }) + ).toBeInTheDocument(); + expect( + screen.getByRole('row', { + name: `span.description:${WildcardOperators.CONTAINS}randomValue`, }) ).toBeInTheDocument(); // Focus should be at the end of the pasted text @@ -4424,6 +4429,49 @@ describe('SearchQueryBuilder', () => { expect(getLastInput()).toHaveFocus(); }); }); + + describe('selecting from filter key suggestions', () => { + beforeEach(() => { + MockApiClient.addMockResponse({ + url: '/organizations/org-slug/recent-searches/', + body: [{query: 'a or b'}, {query: 'some recent query'}], + }); + }); + + it('should replace the raw search key with the defined key:value', async () => { + render( + , + {organization: {features: ['search-query-builder-wildcard-operators']}} + ); + + await userEvent.click(getLastInput()); + + const aOrBOption = await screen.findByRole('option', {name: 'a or b'}); + expect(aOrBOption).toBeInTheDocument(); + + await userEvent.hover(aOrBOption); + await userEvent.keyboard('{enter}{enter}'); + + expect( + await screen.findByRole('row', { + name: `span.description:${WildcardOperators.CONTAINS}a`, + }) + ).toBeInTheDocument(); + + expect(await screen.findByRole('row', {name: 'OR'})).toBeInTheDocument(); + + expect( + await screen.findByRole('row', { + name: `span.description:${WildcardOperators.CONTAINS}b`, + }) + ).toBeInTheDocument(); + }); + }); }); });