fix(TimeTable): Match calculations between filtered and non filtered states (#35619)

superset-frontend/src/visualizations/TimeTable/utils/valueCalculations/valueCalculations.test.ts

@@ -49,6 +49,66 @@ describe('valueCalculations', () => {
       expect(result.errorMsg).toBeUndefined();
     });
 
+    test('should skip null values when finding lagged data points', () => {
+      const entriesWithNulls: Entry[] = [
+        { time: '2023-01-05', sales: 500, price: 50 },
+        { time: '2023-01-04', sales: null, price: null },
+        { time: '2023-01-03', sales: null, price: null },
+        { time: '2023-01-02', sales: 200, price: 20 },
+        { time: '2023-01-01', sales: 100, price: 10 },
+      ];
+
+      const column: ColumnConfig = {
+        key: 'test',
+        colType: 'time',
+        comparisonType: 'diff',
+        timeLag: 1,
+      };
+
+      const result = calculateTimeValue(500, 'sales', entriesWithNulls, column);
+
+      expect(result.value).toBe(300);
+    });
+
+    test('should maintain consistency between filtered and unfiltered behavior', () => {
+      const unfilteredEntries: Entry[] = [
+        { time: '2023-01-05', productA: 500, productB: null },
+        { time: '2023-01-04', productA: null, productB: null },
+        { time: '2023-01-03', productA: null, productB: 300 },
+        { time: '2023-01-02', productA: 200, productB: null },
+        { time: '2023-01-01', productA: 100, productB: 200 },
+      ];
+
+      const filteredEntries: Entry[] = [
+        { time: '2023-01-05', productA: 500 },
+        { time: '2023-01-02', productA: 200 },
+        { time: '2023-01-01', productA: 100 },
+      ];
+
+      const column: ColumnConfig = {
+        key: 'test',
+        colType: 'time',
+        comparisonType: 'diff',
+        timeLag: 1,
+      };
+
+      const unfilteredResult = calculateTimeValue(
+        500,
+        'productA',
+        unfilteredEntries,
+        column,
+      );
+      const filteredResult = calculateTimeValue(
+        500,
+        'productA',
+        filteredEntries,
+        column,
+      );
+
+      expect(unfilteredResult.value).toBe(filteredResult.value);
+      expect(unfilteredResult.value).toBe(300);
+    });
+
     test('should calculate perc comparison correctly', () => {
       const column: ColumnConfig = {
         key: 'test',
@@ -125,6 +185,106 @@ describe('valueCalculations', () => {
 
       expect(result.value).toBe(200); // lagged value without comparison
     });
+
+    test('should handle multiple null values with different time lags', () => {
+      const entriesWithGaps: Entry[] = [
+        { time: '2023-01-07', sales: 700 },
+        { time: '2023-01-06', sales: null },
+        { time: '2023-01-05', sales: null },
+        { time: '2023-01-04', sales: null },
+        { time: '2023-01-03', sales: 300 },
+        { time: '2023-01-02', sales: 200 },
+        { time: '2023-01-01', sales: 100 },
+      ];
+
+      const column1: ColumnConfig = {
+        key: 'test',
+        colType: 'time',
+        comparisonType: 'diff',
+        timeLag: 1,
+      };
+
+      const column2: ColumnConfig = {
+        key: 'test',
+        colType: 'time',
+        comparisonType: 'diff',
+        timeLag: 2,
+      };
+
+      const result1 = calculateTimeValue(
+        700,
+        'sales',
+        entriesWithGaps,
+        column1,
+      );
+      const result2 = calculateTimeValue(
+        700,
+        'sales',
+        entriesWithGaps,
+        column2,
+      );
+
+      expect(result1.value).toBe(400);
+      expect(result2.value).toBe(500);
+    });
+
+    test('should work with perc and perc_change when nulls are present', () => {
+      const entriesWithNulls: Entry[] = [
+        { time: '2023-01-04', sales: 400 },
+        { time: '2023-01-03', sales: null },
+        { time: '2023-01-02', sales: null },
+        { time: '2023-01-01', sales: 200 },
+      ];
+
+      const percColumn: ColumnConfig = {
+        key: 'test',
+        colType: 'time',
+        comparisonType: 'perc',
+        timeLag: 1,
+      };
+
+      const percChangeColumn: ColumnConfig = {
+        key: 'test',
+        colType: 'time',
+        comparisonType: 'perc_change',
+        timeLag: 1,
+      };
+
+      const percResult = calculateTimeValue(
+        400,
+        'sales',
+        entriesWithNulls,
+        percColumn,
+      );
+      const percChangeResult = calculateTimeValue(
+        400,
+        'sales',
+        entriesWithNulls,
+        percChangeColumn,
+      );
+
+      expect(percResult.value).toBe(2);
+      expect(percChangeResult.value).toBe(1);
+    });
+
+    test('should return null when not enough non-null values for time lag', () => {
+      const sparseEntries: Entry[] = [
+        { time: '2023-01-03', sales: 300 },
+        { time: '2023-01-02', sales: null },
+        { time: '2023-01-01', sales: null },
+      ];
+
+      const column: ColumnConfig = {
+        key: 'test',
+        colType: 'time',
+        comparisonType: 'diff',
+        timeLag: 2,
+      };
+
+      const result = calculateTimeValue(300, 'sales', sparseEntries, column);
+
+      expect(result.value).toBeNull();
+    });
   });
 
   // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks

superset-frontend/src/visualizations/TimeTable/utils/valueCalculations/valueCalculations.ts

@@ -42,25 +42,47 @@ export function calculateTimeValue(
       errorMsg: `The time lag set at ${timeLag} is too large for the length of data at ${reversedEntries.length}. No data available.`,
     };
 
-  let laggedValue: number | null;
+  let laggedValue: number | null = null;
 
-  if (timeLag < 0)
-    laggedValue = reversedEntries[totalLag + timeLag][valueField];
-  else laggedValue = reversedEntries[timeLag][valueField];
+  if (timeLag < 0) {
+    const index = totalLag + timeLag;
+    if (index >= 0 && index < totalLag) {
+      laggedValue = reversedEntries[index][valueField];
+    }
+  } else if (timeLag === 0) {
+    laggedValue = recent;
+  } else {
+    // Find the Nth actual data point, skipping null values
+    let dataPointsFound = 0;
+    reversedEntries.slice(1, totalLag).some(entry => {
+      const searchValue = entry[valueField];
+      if (typeof searchValue === 'number') {
+        dataPointsFound += 1;
+        if (dataPointsFound === timeLag) {
+          laggedValue = searchValue;
+          return true;
+        }
+      }
+      return false;
+    });
+  }
 
-  if (typeof laggedValue !== 'number' || typeof recent !== 'number')
+  // For comparison operations, both values must be numbers
+  if (typeof laggedValue === 'number' && typeof recent === 'number') {
+    if (column.comparisonType === 'diff')
+      return { value: recent - laggedValue };
+    if (column.comparisonType === 'perc')
+      return { value: recent / laggedValue };
+    if (column.comparisonType === 'perc_change')
+      return { value: recent / laggedValue - 1 };
+  }
+
+  // If recent is null or undefined, return null (can't do meaningful calculations)
+  if (typeof recent !== 'number') {
     return { value: null };
+  }
 
-  let calculatedValue: number;
-
-  if (column.comparisonType === 'diff') calculatedValue = recent - laggedValue;
-  else if (column.comparisonType === 'perc')
-    calculatedValue = recent / laggedValue;
-  else if (column.comparisonType === 'perc_change')
-    calculatedValue = recent / laggedValue - 1;
-  else calculatedValue = laggedValue;
-
-  return { value: calculatedValue };
+  return { value: laggedValue };
 }
 
 /**