Browse Source

feat(dam): Ignore falsy fields in metrics widget (#31922)

Matej Minar 3 years ago
parent
commit
d3867c5fc8

+ 1 - 1
static/app/actionCreators/metrics.tsx

@@ -57,7 +57,7 @@ export const doMetricsRequest = (
 
   const urlQuery = Object.fromEntries(
     Object.entries({
-      field,
+      field: field.filter(f => !!f),
       cursor,
       end,
       environment,

+ 8 - 1
static/app/views/dashboardsV2/widgetCard/metricsWidgetQueries.tsx

@@ -62,7 +62,7 @@ class MetricsWidgetQueries extends React.Component<Props, State> {
       'tempId',
       'widgetType',
     ];
-    const ignoredQueryProps = ['name'];
+    const ignoredQueryProps = ['name', 'fields'];
     const widgetQueryNames = widget.queries.map(q => q.name);
     const prevWidgetQueryNames = prevProps.widget.queries.map(q => q.name);
 
@@ -70,13 +70,20 @@ class MetricsWidgetQueries extends React.Component<Props, State> {
       limit !== prevProps.limit ||
       organization.slug !== prevProps.organization.slug ||
       !isSelectionEqual(selection, prevProps.selection) ||
+      // If the widget changed (ignore unimportant fields, + queries as they are handled lower)
       !isEqual(
         omit(widget, ignroredWidgetProps),
         omit(prevProps.widget, ignroredWidgetProps)
       ) ||
+      // If the queries changed (ignore unimportant name, + fields as they are handled lower)
       !isEqual(
         widget.queries.map(q => omit(q, ignoredQueryProps)),
         prevProps.widget.queries.map(q => omit(q, ignoredQueryProps))
+      ) ||
+      // If the fields changed (ignore falsy/empty fields -> they can happen after clicking on Add Overlay)
+      !isEqual(
+        widget.queries.flatMap(q => q.fields.filter(field => !!field)),
+        prevProps.widget.queries.flatMap(q => q.fields.filter(field => !!field))
       )
     ) {
       this.fetchData();

+ 16 - 0
tests/js/spec/actionCreators/metrics.spec.tsx

@@ -79,4 +79,20 @@ describe('Metrics ActionCreator', function () {
       })
     );
   });
+
+  it('ignores falsy fields', function () {
+    doMetricsRequest(api, {
+      ...options,
+      field: [SessionMetric.SENTRY_SESSIONS_SESSION, ''],
+    });
+    expect(mock).toHaveBeenCalledTimes(1);
+    expect(mock).toHaveBeenLastCalledWith(
+      `/organizations/${orgSlug}/metrics/data/`,
+      expect.objectContaining({
+        query: expect.objectContaining({
+          field: ['sentry.sessions.session'],
+        }),
+      })
+    );
+  });
 });

+ 8 - 2
tests/js/spec/views/dashboardsV2/metricsWidgetQueries.spec.tsx

@@ -163,7 +163,7 @@ describe('Dashboards > MetricsWidgetQueries', function () {
     );
   });
 
-  it('does not re-fetch when renaming legend alias', () => {
+  it('does not re-fetch when renaming legend alias / adding falsy fields', () => {
     const mock = MockApiClient.addMockResponse({
       url: '/organizations/org-slug/metrics/data/',
       body: TestStubs.MetricsField({field: SessionMetric.SENTRY_SESSIONS_SESSION}),
@@ -189,7 +189,13 @@ describe('Dashboards > MetricsWidgetQueries', function () {
         api={api}
         widget={{
           ...singleQueryWidget,
-          queries: [{...singleQueryWidget.queries[0], name: 'New Legend Alias'}],
+          queries: [
+            {
+              ...singleQueryWidget.queries[0],
+              name: 'New Legend Alias',
+              fields: [...singleQueryWidget.queries[0].fields, ''],
+            },
+          ],
         }}
         organization={organization}
         selection={selection}