Skip to content

Commit 2951fc0

Browse files
nsdeschenespriscilawebdev
authored andcommitted
fix/ref(search-bar): Inline replace rather than merge and replace (#102098)
This PR is an attempt to fix an issue @malwilley found when selecting `a or b`, by inline replacing rather than trying to merge and replace values. These changes remove the merging of all raw text values, as that would get into some strange behavior if users were building a complex query with parens or conditional operators.
1 parent cec9b04 commit 2951fc0

File tree

3 files changed

+130
-186
lines changed

3 files changed

+130
-186
lines changed

static/app/components/searchQueryBuilder/hooks/useQueryBuilderState.spec.tsx

Lines changed: 15 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
import type {FocusOverride} from 'sentry/components/searchQueryBuilder/types';
22
import {WildcardOperators} from 'sentry/components/searchSyntax/parser';
33

4-
import {
5-
replaceFreeTextTokens,
6-
type ReplaceTokensWithTextOnPasteAction,
7-
} from './useQueryBuilderState';
4+
import {replaceFreeTextTokens} from './useQueryBuilderState';
85

96
describe('replaceFreeTextTokens', () => {
107
describe('when there are free text tokens', () => {
@@ -15,7 +12,6 @@ describe('replaceFreeTextTokens', () => {
1512
query: string | undefined;
1613
};
1714
input: {
18-
action: ReplaceTokensWithTextOnPasteAction;
1915
currentQuery: string;
2016
getFieldDefinition: () => null;
2117
rawSearchReplacement: string[];
@@ -26,12 +22,6 @@ describe('replaceFreeTextTokens', () => {
2622
{
2723
description: 'when there are no tokens',
2824
input: {
29-
action: {
30-
type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE',
31-
text: '',
32-
tokens: [],
33-
focusOverride: undefined,
34-
},
3525
getFieldDefinition: () => null,
3626
rawSearchReplacement: ['span.description'],
3727
currentQuery: '',
@@ -44,12 +34,6 @@ describe('replaceFreeTextTokens', () => {
4434
{
4535
description: 'when the replace raw search keys is empty',
4636
input: {
47-
action: {
48-
type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE',
49-
text: '',
50-
tokens: [],
51-
focusOverride: undefined,
52-
},
5337
getFieldDefinition: () => null,
5438
rawSearchReplacement: [],
5539
currentQuery: '',
@@ -62,12 +46,6 @@ describe('replaceFreeTextTokens', () => {
6246
{
6347
description: 'when the replace raw search keys is an empty string',
6448
input: {
65-
action: {
66-
type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE',
67-
text: '',
68-
tokens: [],
69-
focusOverride: undefined,
70-
},
7149
getFieldDefinition: () => null,
7250
rawSearchReplacement: [''],
7351
currentQuery: '',
@@ -80,12 +58,6 @@ describe('replaceFreeTextTokens', () => {
8058
{
8159
description: 'when there is no raw search replacement',
8260
input: {
83-
action: {
84-
type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE',
85-
text: '',
86-
tokens: [],
87-
focusOverride: undefined,
88-
},
8961
getFieldDefinition: () => null,
9062
rawSearchReplacement: [],
9163
currentQuery: `browser.name:${WildcardOperators.CONTAINS}"firefox"`,
@@ -98,12 +70,6 @@ describe('replaceFreeTextTokens', () => {
9870
{
9971
description: 'when there are no free text tokens',
10072
input: {
101-
action: {
102-
type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE',
103-
text: '',
104-
tokens: [],
105-
focusOverride: undefined,
106-
},
10773
getFieldDefinition: () => null,
10874
rawSearchReplacement: ['span.description'],
10975
currentQuery: `browser.name:${WildcardOperators.CONTAINS}"firefox"`,
@@ -116,15 +82,9 @@ describe('replaceFreeTextTokens', () => {
11682
{
11783
description: 'when there only valid action tokens',
11884
input: {
119-
action: {
120-
type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE',
121-
text: 'span.op:eq',
122-
tokens: [],
123-
focusOverride: undefined,
124-
},
12585
getFieldDefinition: () => null,
12686
rawSearchReplacement: ['span.description'],
127-
currentQuery: '',
87+
currentQuery: 'span.op:eq',
12888
},
12989
expected: {
13090
query: undefined,
@@ -134,15 +94,9 @@ describe('replaceFreeTextTokens', () => {
13494
{
13595
description: 'when there only space free text tokens in the action',
13696
input: {
137-
action: {
138-
type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE',
139-
text: 'span.op:eq ',
140-
tokens: [],
141-
focusOverride: undefined,
142-
},
14397
getFieldDefinition: () => null,
14498
rawSearchReplacement: ['span.description'],
145-
currentQuery: '',
99+
currentQuery: 'span.op:eq ',
146100
},
147101
expected: {
148102
query: undefined,
@@ -152,15 +106,9 @@ describe('replaceFreeTextTokens', () => {
152106
{
153107
description: 'when there is one free text token',
154108
input: {
155-
action: {
156-
type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE',
157-
text: 'test',
158-
tokens: [],
159-
focusOverride: undefined,
160-
},
161109
getFieldDefinition: () => null,
162110
rawSearchReplacement: ['span.description'],
163-
currentQuery: '',
111+
currentQuery: 'test',
164112
},
165113
expected: {
166114
query: `span.description:${WildcardOperators.CONTAINS}test`,
@@ -170,15 +118,9 @@ describe('replaceFreeTextTokens', () => {
170118
{
171119
description: 'when there is one free text token that has a space',
172120
input: {
173-
action: {
174-
type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE',
175-
text: 'test test',
176-
tokens: [],
177-
focusOverride: undefined,
178-
},
179121
getFieldDefinition: () => null,
180122
rawSearchReplacement: ['span.description'],
181-
currentQuery: '',
123+
currentQuery: 'test test',
182124
},
183125
expected: {
184126
query: `span.description:${WildcardOperators.CONTAINS}"test test"`,
@@ -188,15 +130,9 @@ describe('replaceFreeTextTokens', () => {
188130
{
189131
description: 'when there is already a token present',
190132
input: {
191-
action: {
192-
type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE',
193-
text: 'test',
194-
tokens: [],
195-
focusOverride: undefined,
196-
},
197133
getFieldDefinition: () => null,
198134
rawSearchReplacement: ['span.description'],
199-
currentQuery: 'span.op:eq',
135+
currentQuery: 'span.op:eq test',
200136
},
201137
expected: {
202138
query: `span.op:eq span.description:${WildcardOperators.CONTAINS}test`,
@@ -206,52 +142,34 @@ describe('replaceFreeTextTokens', () => {
206142
{
207143
description: 'when there is already a replace token present',
208144
input: {
209-
action: {
210-
type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE',
211-
text: 'test2',
212-
tokens: [],
213-
focusOverride: undefined,
214-
},
215145
getFieldDefinition: () => null,
216146
rawSearchReplacement: ['span.description'],
217-
currentQuery: `span.description:${WildcardOperators.CONTAINS}test`,
147+
currentQuery: `span.description:${WildcardOperators.CONTAINS}test test2`,
218148
},
219149
expected: {
220-
query: `span.description:${WildcardOperators.CONTAINS}[test,test2]`,
221-
focusOverride: {itemKey: 'freeText:1'},
150+
query: `span.description:${WildcardOperators.CONTAINS}test span.description:${WildcardOperators.CONTAINS}test2`,
151+
focusOverride: {itemKey: 'freeText:2'},
222152
},
223153
},
224154
{
225155
description: 'when there is already a replace token present with a space',
226156
input: {
227-
action: {
228-
type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE',
229-
text: 'other value',
230-
tokens: [],
231-
focusOverride: undefined,
232-
},
233157
getFieldDefinition: () => null,
234158
rawSearchReplacement: ['span.description'],
235-
currentQuery: `span.description:${WildcardOperators.CONTAINS}test`,
159+
currentQuery: `span.description:${WildcardOperators.CONTAINS}test other value`,
236160
},
237161
expected: {
238-
query: `span.description:${WildcardOperators.CONTAINS}[test,"other value"]`,
239-
focusOverride: {itemKey: 'freeText:1'},
162+
query: `span.description:${WildcardOperators.CONTAINS}test span.description:${WildcardOperators.CONTAINS}"other value"`,
163+
focusOverride: {itemKey: 'freeText:2'},
240164
},
241165
},
242166
{
243167
description:
244168
'when there is already a replace token present with a different operator',
245169
input: {
246-
action: {
247-
type: 'REPLACE_TOKENS_WITH_TEXT_ON_PASTE',
248-
text: 'other value',
249-
tokens: [],
250-
focusOverride: undefined,
251-
},
252170
getFieldDefinition: () => null,
253171
rawSearchReplacement: ['span.description'],
254-
currentQuery: `span.description:test`,
172+
currentQuery: `span.description:test other value`,
255173
},
256174
expected: {
257175
query: `span.description:test span.description:${WildcardOperators.CONTAINS}"other value"`,
@@ -262,10 +180,9 @@ describe('replaceFreeTextTokens', () => {
262180

263181
it.each(testCases)('$description', ({input, expected}) => {
264182
const result = replaceFreeTextTokens(
265-
input.action,
183+
input.currentQuery,
266184
input.getFieldDefinition,
267-
input.rawSearchReplacement,
268-
input.currentQuery
185+
input.rawSearchReplacement
269186
);
270187

271188
expect(result?.newQuery).toBe(expected.query);

0 commit comments

Comments
 (0)