Browse Source

chore(dashboards): Pass per_page query param for RH limits (#34051)

Replacing the previous hack which limited the number of fields
after the results were returned. Instead pass per_page to the
query.
Shruthi 2 years ago
parent
commit
697904831e

+ 3 - 0
static/app/actionCreators/sessions.tsx

@@ -12,6 +12,7 @@ export type DoSessionsRequestOptions = {
   environment?: Readonly<string[]>;
   groupBy?: string[];
   interval?: string;
+  limit?: number;
   orderBy?: string;
   project?: Readonly<number[]>;
   query?: string;
@@ -35,6 +36,7 @@ export const doSessionsRequest = (
     query,
     statsPeriodStart,
     statsPeriodEnd,
+    limit,
     ...dateTime
   }: DoSessionsRequestOptions
 ): Promise<SessionApiResponse> => {
@@ -51,6 +53,7 @@ export const doSessionsRequest = (
       groupBy: groupBy?.filter(g => !!g),
       interval: interval || getInterval({start, end, period: statsPeriod}),
       orderBy,
+      per_page: limit,
       query: query || undefined,
       project,
       start,

+ 2 - 6
static/app/views/dashboardsV2/widgetCard/releaseWidgetQueries.tsx

@@ -125,11 +125,7 @@ class ReleaseWidgetQueries extends React.Component<Props, State> {
         return {
           ...prevState,
           timeseriesResults: prevState.rawResults?.flatMap((rawResult, index) =>
-            transformSessionsResponseToSeries(
-              rawResult,
-              limit,
-              widget.queries[index].name
-            )
+            transformSessionsResponseToSeries(rawResult, widget.queries[index].name)
           ),
         };
       });
@@ -188,6 +184,7 @@ class ReleaseWidgetQueries extends React.Component<Props, State> {
         end,
         environment: environments,
         groupBy: query.columns,
+        limit: this.limit,
         orderBy: query.orderby,
         interval,
         project: projects,
@@ -230,7 +227,6 @@ class ReleaseWidgetQueries extends React.Component<Props, State> {
           const timeseriesResults = [...(prevState.timeseriesResults ?? [])];
           const transformedResult = transformSessionsResponseToSeries(
             data,
-            this.limit,
             widget.queries[requestIndex].name
           );
 

+ 10 - 16
static/app/views/dashboardsV2/widgetCard/transformSessionsResponseToSeries.tsx

@@ -15,23 +15,17 @@ function getSeriesName(
 
 export function transformSessionsResponseToSeries(
   response: SessionApiResponse | null,
-  limit?: number,
   queryAlias?: string
 ): Series[] {
-  if (response === null) {
-    return [];
-  }
-  // Temporarily restrict the number of lines we plot on grouped queries
-  const groups: SessionApiResponse['groups'] = limit
-    ? response.groups.slice(0, limit)
-    : response.groups;
-  return groups.flatMap(group =>
-    Object.keys(group.series).map(field => ({
-      seriesName: getSeriesName(field, group, queryAlias),
-      data: response.intervals.map((interval, index) => ({
-        name: interval,
-        value: group.series[field][index] ?? 0,
-      })),
-    }))
+  return (
+    response?.groups.flatMap(group =>
+      Object.keys(group.series).map(field => ({
+        seriesName: getSeriesName(field, group, queryAlias),
+        data: response.intervals.map((interval, index) => ({
+          name: interval,
+          value: group.series[field][index] ?? 0,
+        })),
+      }))
+    ) ?? []
   );
 }

+ 2 - 0
tests/js/spec/views/dashboardsV2/releaseWidgetQueries.spec.tsx

@@ -298,6 +298,7 @@ describe('Dashboards > ReleaseWidgetQueries', function () {
           field: ['sum(session)'],
           groupBy: [],
           interval: '30m',
+          per_page: 20,
           project: [1],
           statsPeriod: '14d',
         },
@@ -312,6 +313,7 @@ describe('Dashboards > ReleaseWidgetQueries', function () {
           field: ['sum(session)'],
           groupBy: [],
           interval: '30m',
+          per_page: 20,
           project: [1],
           query: 'environment:prod',
           statsPeriod: '14d',

+ 39 - 2
tests/js/spec/views/dashboardsV2/widgetBuilder/widgetBuilder.spec.tsx

@@ -2423,14 +2423,51 @@ describe('WidgetBuilder', function () {
         expect(sessionsDataMock).toHaveBeenLastCalledWith(
           `/organizations/org-slug/sessions/`,
           expect.objectContaining({
-            query: {
+            query: expect.objectContaining({
               environment: [],
               field: [`sum(session)`],
               groupBy: [],
               interval: '5m',
               project: [],
               statsPeriod: '24h',
-            },
+            }),
+          })
+        )
+      );
+    });
+
+    it('makes the calls the session endpoint with the right limit', async function () {
+      renderTestComponent({
+        orgFeatures: releaseHealthFeatureFlags,
+      });
+
+      expect(
+        await screen.findByText('Releases (sessions, crash rates)')
+      ).toBeInTheDocument();
+
+      userEvent.click(screen.getByLabelText(/releases/i));
+
+      userEvent.click(screen.getByText('Table'));
+      userEvent.click(screen.getByText('Line Chart'));
+
+      await selectEvent.select(await screen.findByText('Select group'), 'project');
+
+      expect(screen.getByText('Limit to 5 results')).toBeInTheDocument();
+
+      await waitFor(() =>
+        expect(sessionsDataMock).toHaveBeenLastCalledWith(
+          `/organizations/org-slug/sessions/`,
+          expect.objectContaining({
+            query: expect.objectContaining({
+              environment: [],
+              field: ['sum(session)'],
+              groupBy: ['project'],
+              interval: '5m',
+              orderBy: '-sum_session',
+              per_page: 5,
+              project: [],
+              statsPeriod: '24h',
+            }),
           })
         )
       );

+ 0 - 19
tests/js/spec/views/dashboardsV2/widgetCard/transformSessionsResponseToSeries.spec.tsx

@@ -316,7 +316,6 @@ describe('transformSessionsResponseToSeries', function () {
     expect(
       transformSessionsResponseToSeries(
         TestStubs.SessionUserCountByStatusByRelease(),
-        undefined,
         'Lorem'
       )[0]
     ).toEqual(
@@ -325,22 +324,4 @@ describe('transformSessionsResponseToSeries', function () {
       })
     );
   });
-  it('returns correct number of series if limit is set', () => {
-    expect(
-      transformSessionsResponseToSeries(
-        TestStubs.SessionUserCountByStatusByRelease(),
-        undefined,
-        'Lorem'
-      ).length
-    ).toEqual(16);
-
-    // limit = 3 returns 6 series, 3 for count_unique and 3 for sum
-    expect(
-      transformSessionsResponseToSeries(
-        TestStubs.SessionUserCountByStatusByRelease(),
-        3,
-        'Lorem'
-      ).length
-    ).toEqual(6);
-  });
 });