Skip to content

Commit e11c7da

Browse files
authored
fix(profiles): Need to wrap user query in parenthesis (#71102)
Users can use boolean operators in the query which can have an impact on the condition. For example they added `transaction:foo OR transaction:bar` the query actually becomes `(has:profile.id AND transaction:foo) OR transaction:bar`. which is incorrect.
1 parent b1bc0bd commit e11c7da

File tree

4 files changed

+116
-5
lines changed

4 files changed

+116
-5
lines changed

static/app/utils/profiling/hooks/useProfileEvents.spec.tsx

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,61 @@ describe('useProfileEvents', function () {
3737
MockApiClient.addMockResponse({
3838
url: `/organizations/${organization.slug}/events/`,
3939
body,
40-
match: [MockApiClient.matchQuery({dataset: 'profiles'})],
40+
match: [MockApiClient.matchQuery({dataset: 'profiles', query: 'transaction:foo'})],
4141
});
4242

4343
const {result} = renderHook(useProfileEvents, {
4444
wrapper: TestContext,
4545
initialProps: {
4646
fields,
47+
query: 'transaction:foo',
48+
sort: {key: 'count()', order: 'desc' as const},
49+
referrer: '',
50+
},
51+
});
52+
53+
await waitFor(() => result.current.isSuccess);
54+
expect(result.current.data).toEqual(body);
55+
});
56+
57+
it('handles querying the api using discover', async function () {
58+
const {organization: organizationUsingTransactions} = initializeOrg({
59+
organization: {features: ['profiling-using-transactions']},
60+
});
61+
62+
function TestContextUsingTransactions({children}: {children?: ReactNode}) {
63+
return (
64+
<QueryClientProvider client={makeTestQueryClient()}>
65+
<OrganizationContext.Provider value={organizationUsingTransactions}>
66+
{children}
67+
</OrganizationContext.Provider>
68+
</QueryClientProvider>
69+
);
70+
}
71+
72+
const fields = ['count()'];
73+
74+
const body: EventsResults<(typeof fields)[number]> = {
75+
data: [],
76+
meta: {fields: {}, units: {}},
77+
};
78+
79+
MockApiClient.addMockResponse({
80+
url: `/organizations/${organization.slug}/events/`,
81+
body,
82+
match: [
83+
MockApiClient.matchQuery({
84+
dataset: 'discover',
85+
query: 'has:profile.id (transaction:foo)',
86+
}),
87+
],
88+
});
89+
90+
const {result} = renderHook(useProfileEvents, {
91+
wrapper: TestContextUsingTransactions,
92+
initialProps: {
93+
fields,
94+
query: 'transaction:foo',
4795
sort: {key: 'count()', order: 'desc' as const},
4896
referrer: '',
4997
},

static/app/utils/profiling/hooks/useProfileEvents.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export function useProfileEvents<F extends string>({
4040
let dataset: 'profiles' | 'discover' = 'profiles';
4141
if (organization.features.includes('profiling-using-transactions')) {
4242
dataset = 'discover';
43-
query = `has:profile.id ${query ?? ''}`;
43+
query = `has:profile.id ${query ? `(${query})` : ''}`;
4444
}
4545

4646
const path = `/organizations/${organization.slug}/events/`;

static/app/utils/profiling/hooks/useProfileEventsStats.spec.tsx

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,15 @@ describe('useProfileEvents', function () {
7171
units: {count: null},
7272
},
7373
},
74-
match: [MockApiClient.matchQuery({dataset: 'profiles'})],
74+
match: [MockApiClient.matchQuery({dataset: 'profiles', query: 'transaction:foo'})],
7575
});
7676

7777
const {result} = renderHook(useProfileEventsStats, {
7878
wrapper: TestContext,
7979
initialProps: {
8080
dataset: 'profiles' as const,
8181
yAxes,
82+
query: 'transaction:foo',
8283
referrer: '',
8384
},
8485
});
@@ -126,14 +127,15 @@ describe('useProfileEvents', function () {
126127
},
127128
},
128129
},
129-
match: [MockApiClient.matchQuery({dataset: 'profiles'})],
130+
match: [MockApiClient.matchQuery({dataset: 'profiles', query: 'transaction:foo'})],
130131
});
131132

132133
const {result} = renderHook(useProfileEventsStats, {
133134
wrapper: TestContext,
134135
initialProps: {
135136
dataset: 'profiles' as const,
136137
yAxes,
138+
query: 'transaction:foo',
137139
referrer: '',
138140
},
139141
});
@@ -152,4 +154,65 @@ describe('useProfileEvents', function () {
152154
timestamps: [0, 5],
153155
});
154156
});
157+
158+
it('handles 1 axis using discover', async function () {
159+
const {organization: organizationUsingTransactions} = initializeOrg({
160+
organization: {features: ['profiling-using-transactions']},
161+
});
162+
163+
function TestContextUsingTransactions({children}: {children?: ReactNode}) {
164+
return (
165+
<QueryClientProvider client={makeTestQueryClient()}>
166+
<OrganizationContext.Provider value={organizationUsingTransactions}>
167+
{children}
168+
</OrganizationContext.Provider>
169+
</QueryClientProvider>
170+
);
171+
}
172+
173+
const yAxes = ['count()'];
174+
175+
MockApiClient.addMockResponse({
176+
url: `/organizations/${organization.slug}/events-stats/`,
177+
body: {
178+
data: [
179+
[0, [{count: 1}]],
180+
[5, [{count: 2}]],
181+
],
182+
start: 0,
183+
end: 10,
184+
meta: {
185+
fields: {count: 'integer'},
186+
units: {count: null},
187+
},
188+
},
189+
match: [
190+
MockApiClient.matchQuery({
191+
dataset: 'discover',
192+
query: 'has:profile.id (transaction:foo)',
193+
}),
194+
],
195+
});
196+
197+
const {result} = renderHook(useProfileEventsStats, {
198+
wrapper: TestContextUsingTransactions,
199+
initialProps: {
200+
dataset: 'profiles' as const,
201+
yAxes,
202+
query: 'transaction:foo',
203+
referrer: '',
204+
},
205+
});
206+
207+
await waitFor(() => result.current.isSuccess);
208+
expect(result.current.data).toEqual({
209+
data: [{axis: 'count()', values: [1, 2]}],
210+
meta: {
211+
dataset: 'discover',
212+
start: 0,
213+
end: 10,
214+
},
215+
timestamps: [0, 5],
216+
});
217+
});
155218
});

static/app/utils/profiling/hooks/useProfileEventsStats.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export function useProfileEventsStats<F extends string>({
3939
}
4040

4141
if (dataset === 'discover') {
42-
query = `has:profile.id ${query ?? ''}`;
42+
query = `has:profile.id ${query ? `(${query})` : ''}`;
4343
}
4444

4545
const path = `/organizations/${organization.slug}/events-stats/`;

0 commit comments

Comments
 (0)