From d852aed4f350255d29c1881da074c2de1262e9fe Mon Sep 17 00:00:00 2001
From: Victor Marin <36818606+mdvictor@users.noreply.github.com>
Date: Wed, 20 Sep 2023 09:51:36 +0300
Subject: [PATCH] Fix groupBy error caused by undefined aggregate field
 (#75089)

* fix aggregation field to return empty array instead of undefined in groupBy

* add groupBy test scenario

* remove log
---
 .../src/dataframe/processDataFrame.test.ts    | 50 ++++++++++++++-
 .../transformers/groupBy.test.ts              | 62 +++++++++++++++++++
 .../transformations/transformers/groupBy.ts   |  2 +-
 3 files changed, 112 insertions(+), 2 deletions(-)

diff --git a/packages/grafana-data/src/dataframe/processDataFrame.test.ts b/packages/grafana-data/src/dataframe/processDataFrame.test.ts
index 2e23be6e171..7dad218b969 100644
--- a/packages/grafana-data/src/dataframe/processDataFrame.test.ts
+++ b/packages/grafana-data/src/dataframe/processDataFrame.test.ts
@@ -1,9 +1,10 @@
 import { dateTime } from '../datetime/moment_wrapper';
-import { DataFrameDTO, FieldType, TableData, TimeSeries } from '../types/index';
+import { DataFrameDTO, Field, FieldType, TableData, TimeSeries } from '../types/index';
 
 import { ArrayDataFrame } from './ArrayDataFrame';
 import {
   createDataFrame,
+  guessFieldTypeForField,
   guessFieldTypeFromValue,
   guessFieldTypes,
   isDataFrame,
@@ -403,3 +404,50 @@ describe('reverse DataFrame', () => {
     expect(rev.fields[1].nanos).toBeUndefined();
   });
 });
+
+describe('guessFieldTypeForField', () => {
+  it('should guess types if value exists', () => {
+    const field: Field = {
+      name: 'Field',
+      config: {},
+      type: FieldType.other,
+      values: [1, 2, 3],
+    };
+
+    expect(guessFieldTypeForField(field)).toBe(FieldType.number);
+
+    field.values = [null, null, 3];
+
+    expect(guessFieldTypeForField(field)).toBe(FieldType.number);
+  });
+
+  it('should guess type if name suggests time values', () => {
+    const field: Field = {
+      name: 'Date',
+      config: {},
+      type: FieldType.other,
+      values: [1, 2, 3],
+    };
+
+    expect(guessFieldTypeForField(field)).toBe(FieldType.time);
+
+    field.name = 'time';
+
+    expect(guessFieldTypeForField(field)).toBe(FieldType.time);
+  });
+
+  it('should return undefined if no values present', () => {
+    const field: Field = {
+      name: 'Val',
+      config: {},
+      type: FieldType.other,
+      values: [null, null],
+    };
+
+    expect(guessFieldTypeForField(field)).toBe(undefined);
+
+    field.values = [];
+
+    expect(guessFieldTypeForField(field)).toBe(undefined);
+  });
+});
diff --git a/packages/grafana-data/src/transformations/transformers/groupBy.test.ts b/packages/grafana-data/src/transformations/transformers/groupBy.test.ts
index 8d049f63f52..3485d6615b7 100644
--- a/packages/grafana-data/src/transformations/transformers/groupBy.test.ts
+++ b/packages/grafana-data/src/transformations/transformers/groupBy.test.ts
@@ -305,4 +305,66 @@ describe('GroupBy transformer', () => {
       expect(result[0].fields).toEqual(expected);
     });
   });
+
+  it('should group by and skip fields that do not have values for a group', async () => {
+    const testSeries1 = toDataFrame({
+      name: 'Series1',
+      fields: [
+        { name: 'Time', type: FieldType.time, values: [1688470200000, 1688471100000, 1688470200000, 1688471100000] },
+        { name: 'Value', type: FieldType.number, values: [1, 2, 3, 4] },
+      ],
+    });
+
+    const testSeries2 = toDataFrame({
+      name: 'Series2',
+      fields: [
+        { name: 'Time', type: FieldType.time, values: [] },
+        { name: 'Value', type: FieldType.number, values: [] },
+      ],
+    });
+
+    const cfg: DataTransformerConfig<GroupByTransformerOptions> = {
+      id: DataTransformerID.groupBy,
+      options: {
+        fields: {
+          Series1: {
+            operation: GroupByOperationID.aggregate,
+            aggregations: [ReducerID.sum],
+          },
+          Series2: {
+            operation: GroupByOperationID.aggregate,
+            aggregations: [ReducerID.sum],
+          },
+          Time: {
+            operation: GroupByOperationID.groupBy,
+            aggregations: [],
+          },
+          Value: {
+            operation: GroupByOperationID.aggregate,
+            aggregations: [ReducerID.sum],
+          },
+        },
+      },
+    };
+
+    await expect(transformDataFrame([cfg], [testSeries1, testSeries2])).toEmitValuesWith((received) => {
+      const result = received[0];
+      const expected: Field[] = [
+        {
+          name: 'Time',
+          type: FieldType.time,
+          values: [1688470200000, 1688471100000],
+          config: {},
+        },
+        {
+          name: 'Value (sum)',
+          type: FieldType.number,
+          values: [4, 6],
+          config: {},
+        },
+      ];
+
+      expect(result[0].fields).toEqual(expected);
+    });
+  });
 });
diff --git a/packages/grafana-data/src/transformations/transformers/groupBy.ts b/packages/grafana-data/src/transformations/transformers/groupBy.ts
index 9004d7f704a..61b68a98508 100644
--- a/packages/grafana-data/src/transformations/transformers/groupBy.ts
+++ b/packages/grafana-data/src/transformations/transformers/groupBy.ts
@@ -135,7 +135,7 @@ export const groupByTransformer: DataTransformerInfo<GroupByTransformerOptions>
             for (const aggregation of aggregations) {
               const aggregationField: Field = {
                 name: `${fieldName} (${aggregation})`,
-                values: valuesByAggregation[aggregation],
+                values: valuesByAggregation[aggregation] ?? [],
                 type: FieldType.other,
                 config: {},
               };