Browse Source

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.
Tony Xiao 9 months ago
parent
commit
e11c7da945

+ 49 - 1
static/app/utils/profiling/hooks/useProfileEvents.spec.tsx

@@ -37,13 +37,61 @@ describe('useProfileEvents', function () {
     MockApiClient.addMockResponse({
       url: `/organizations/${organization.slug}/events/`,
       body,
-      match: [MockApiClient.matchQuery({dataset: 'profiles'})],
+      match: [MockApiClient.matchQuery({dataset: 'profiles', query: 'transaction:foo'})],
     });
 
     const {result} = renderHook(useProfileEvents, {
       wrapper: TestContext,
       initialProps: {
         fields,
+        query: 'transaction:foo',
+        sort: {key: 'count()', order: 'desc' as const},
+        referrer: '',
+      },
+    });
+
+    await waitFor(() => result.current.isSuccess);
+    expect(result.current.data).toEqual(body);
+  });
+
+  it('handles querying the api using discover', async function () {
+    const {organization: organizationUsingTransactions} = initializeOrg({
+      organization: {features: ['profiling-using-transactions']},
+    });
+
+    function TestContextUsingTransactions({children}: {children?: ReactNode}) {
+      return (
+        <QueryClientProvider client={makeTestQueryClient()}>
+          <OrganizationContext.Provider value={organizationUsingTransactions}>
+            {children}
+          </OrganizationContext.Provider>
+        </QueryClientProvider>
+      );
+    }
+
+    const fields = ['count()'];
+
+    const body: EventsResults<(typeof fields)[number]> = {
+      data: [],
+      meta: {fields: {}, units: {}},
+    };
+
+    MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/events/`,
+      body,
+      match: [
+        MockApiClient.matchQuery({
+          dataset: 'discover',
+          query: 'has:profile.id (transaction:foo)',
+        }),
+      ],
+    });
+
+    const {result} = renderHook(useProfileEvents, {
+      wrapper: TestContextUsingTransactions,
+      initialProps: {
+        fields,
+        query: 'transaction:foo',
         sort: {key: 'count()', order: 'desc' as const},
         referrer: '',
       },

+ 1 - 1
static/app/utils/profiling/hooks/useProfileEvents.tsx

@@ -40,7 +40,7 @@ export function useProfileEvents<F extends string>({
   let dataset: 'profiles' | 'discover' = 'profiles';
   if (organization.features.includes('profiling-using-transactions')) {
     dataset = 'discover';
-    query = `has:profile.id ${query ?? ''}`;
+    query = `has:profile.id ${query ? `(${query})` : ''}`;
   }
 
   const path = `/organizations/${organization.slug}/events/`;

+ 65 - 2
static/app/utils/profiling/hooks/useProfileEventsStats.spec.tsx

@@ -71,7 +71,7 @@ describe('useProfileEvents', function () {
           units: {count: null},
         },
       },
-      match: [MockApiClient.matchQuery({dataset: 'profiles'})],
+      match: [MockApiClient.matchQuery({dataset: 'profiles', query: 'transaction:foo'})],
     });
 
     const {result} = renderHook(useProfileEventsStats, {
@@ -79,6 +79,7 @@ describe('useProfileEvents', function () {
       initialProps: {
         dataset: 'profiles' as const,
         yAxes,
+        query: 'transaction:foo',
         referrer: '',
       },
     });
@@ -126,7 +127,7 @@ describe('useProfileEvents', function () {
           },
         },
       },
-      match: [MockApiClient.matchQuery({dataset: 'profiles'})],
+      match: [MockApiClient.matchQuery({dataset: 'profiles', query: 'transaction:foo'})],
     });
 
     const {result} = renderHook(useProfileEventsStats, {
@@ -134,6 +135,7 @@ describe('useProfileEvents', function () {
       initialProps: {
         dataset: 'profiles' as const,
         yAxes,
+        query: 'transaction:foo',
         referrer: '',
       },
     });
@@ -152,4 +154,65 @@ describe('useProfileEvents', function () {
       timestamps: [0, 5],
     });
   });
+
+  it('handles 1 axis using discover', async function () {
+    const {organization: organizationUsingTransactions} = initializeOrg({
+      organization: {features: ['profiling-using-transactions']},
+    });
+
+    function TestContextUsingTransactions({children}: {children?: ReactNode}) {
+      return (
+        <QueryClientProvider client={makeTestQueryClient()}>
+          <OrganizationContext.Provider value={organizationUsingTransactions}>
+            {children}
+          </OrganizationContext.Provider>
+        </QueryClientProvider>
+      );
+    }
+
+    const yAxes = ['count()'];
+
+    MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/events-stats/`,
+      body: {
+        data: [
+          [0, [{count: 1}]],
+          [5, [{count: 2}]],
+        ],
+        start: 0,
+        end: 10,
+        meta: {
+          fields: {count: 'integer'},
+          units: {count: null},
+        },
+      },
+      match: [
+        MockApiClient.matchQuery({
+          dataset: 'discover',
+          query: 'has:profile.id (transaction:foo)',
+        }),
+      ],
+    });
+
+    const {result} = renderHook(useProfileEventsStats, {
+      wrapper: TestContextUsingTransactions,
+      initialProps: {
+        dataset: 'profiles' as const,
+        yAxes,
+        query: 'transaction:foo',
+        referrer: '',
+      },
+    });
+
+    await waitFor(() => result.current.isSuccess);
+    expect(result.current.data).toEqual({
+      data: [{axis: 'count()', values: [1, 2]}],
+      meta: {
+        dataset: 'discover',
+        start: 0,
+        end: 10,
+      },
+      timestamps: [0, 5],
+    });
+  });
 });

+ 1 - 1
static/app/utils/profiling/hooks/useProfileEventsStats.tsx

@@ -39,7 +39,7 @@ export function useProfileEventsStats<F extends string>({
   }
 
   if (dataset === 'discover') {
-    query = `has:profile.id ${query ?? ''}`;
+    query = `has:profile.id ${query ? `(${query})` : ''}`;
   }
 
   const path = `/organizations/${organization.slug}/events-stats/`;