Merge pull request #1196 from buster-so/nate/hot-fix-charts

Nate/hot fix charts with date object and replace 0

apps/web/src/components/ui/charts/BusterChartJS/BusterChartJSComponent.tsx

@@ -92,7 +92,7 @@ export const BusterChartJSComponent = React.memo(
         lineGroupType,
         barGroupType,
       });
-      const previousData = usePreviousRef(data);
+      const previousDataLabels = usePreviousRef(data.labels);
 
       const { chartPlugins, chartOptions } = useChartSpecificOptions({
         selectedChartType,
@@ -182,7 +182,7 @@ export const BusterChartJSComponent = React.memo(
 
       const updateMode = useMemoizedFn((): UpdateMode => {
         if (!ref) return 'default';
-        const areLabelsChanged = previousData?.labels !== data.labels;
+        const areLabelsChanged = previousDataLabels !== data.labels;
         if (areLabelsChanged) return 'default'; //this will disable animation - this was 'none', I am not sure why...
         return 'default';
       });

apps/web/src/components/ui/charts/BusterChartJS/hooks/useSeriesOptions/createTickDate.ts

@@ -35,6 +35,7 @@ export const createTickDates = (
     if (isAQuarter !== -1) {
       return createQuarterTickDates(ticks, xColumnLabelFormats, isAQuarter);
     }
+
     const dateTicks = ticks.flatMap((item) => {
       return item.map<Date>((item) => {
         return createDayjsDate(item as string).toDate(); //do not join because it will turn into a string

apps/web/src/components/ui/charts/BusterChartJS/hooks/useSeriesOptions/useSeriesOptions.ts

@@ -61,7 +61,7 @@ export const useSeriesOptions = ({
       sizeKey,
       columnSettings,
     });
-  }, [datasetOptions, columnSettings, columnLabelFormats, xAxisKeys, sizeKey]);
+  }, [datasetOptions, columnSettings, columnLabelFormats, xAxisKeys, sizeKey, selectedChartType]);
 
   const sizeOptions: SeriesBuilderProps['sizeOptions'] = useMemo(() => {
     if (!sizeKey || sizeKey.length === 0) {
@@ -81,7 +81,7 @@ export const useSeriesOptions = ({
       minValue: Number(assosciatedColumn?.min_value),
       maxValue: Number(assosciatedColumn?.max_value),
     };
-  }, [sizeKey]);
+  }, [sizeKey, columnMetadata]);
 
   const datasets: ChartProps<ChartJSChartType>['data']['datasets'] = useMemo(() => {
     return dataBuilderRecord[selectedChartType]({
@@ -113,6 +113,7 @@ export const useSeriesOptions = ({
     barShowTotalAtTop,
     yAxisKeys,
     y2AxisKeys,
+    selectedChartType,
   ]);
 
   return {

apps/web/src/controllers/MetricController/MetricViewChartController/MetricEditController/MetricStylingApp/StylingAppVisualize/SelectAxis/helper.test.ts

@@ -149,23 +149,29 @@ describe('getChartTypeDropZones', () => {
         barLayout: 'vertical', // barLayout doesn't affect line charts
       });
 
-      expect(result).toHaveLength(4);
+      expect(result).toHaveLength(5);
       expect(result[0]).toEqual({
         id: SelectAxisContainerId.XAxis,
         title: 'X-Axis',
         items: ['date_column'],
       });
+
       expect(result[1]).toEqual({
         id: SelectAxisContainerId.YAxis,
         title: 'Y-Axis',
         items: ['sales', 'profit'],
       });
       expect(result[2]).toEqual({
+        id: SelectAxisContainerId.ColorBy,
+        title: 'Color By',
+        items: [],
+      });
+      expect(result[3]).toEqual({
         id: SelectAxisContainerId.CategoryAxis,
         title: 'Category',
         items: [],
       });
-      expect(result[3]).toEqual({
+      expect(result[4]).toEqual({
         id: SelectAxisContainerId.Tooltip,
         title: 'Tooltip',
         items: ['additional_info'],

apps/web/src/lib/columnFormatter.test.ts

@@ -369,4 +369,150 @@ describe('formatLabel', () => {
       );
     });
   });
+
+  describe('textProp to Number conversion logic', () => {
+    it('should convert string numbers to Number when all conditions are met', () => {
+      // All conditions met: textProp not null/undefined, columnType='number',
+      // useKeyFormatter=false, replaceMissingDataWith=0, textProp is not object
+      expect(
+        formatLabel('123.45', {
+          columnType: 'number',
+          style: 'number',
+          replaceMissingDataWith: 0,
+        })
+      ).toBe('123.45');
+    });
+
+    it('should convert boolean true to Number when all conditions are met', () => {
+      expect(
+        formatLabel(true, {
+          columnType: 'number',
+          style: 'number',
+          replaceMissingDataWith: 0,
+        })
+      ).toBe('1');
+    });
+
+    it('should convert boolean false to Number when all conditions are met', () => {
+      expect(
+        formatLabel(false, {
+          columnType: 'number',
+          style: 'number',
+          replaceMissingDataWith: 0,
+        })
+      ).toBe('0');
+    });
+
+    it('should NOT convert when textProp is null', () => {
+      expect(
+        formatLabel(null, {
+          columnType: 'number',
+          style: 'string',
+          replaceMissingDataWith: 0,
+        })
+      ).toBe('0');
+    });
+
+    it('should NOT convert when textProp is undefined', () => {
+      expect(
+        formatLabel(undefined, {
+          columnType: 'number',
+          style: 'string',
+          replaceMissingDataWith: 0,
+        })
+      ).toBe('0');
+    });
+
+    it('should NOT convert when columnType is not "number"', () => {
+      expect(
+        formatLabel('123', {
+          columnType: 'text',
+          style: 'string',
+          replaceMissingDataWith: 0,
+        })
+      ).toBe('123');
+    });
+
+    it('should NOT convert when useKeyFormatter is true', () => {
+      expect(
+        formatLabel(
+          '123',
+          {
+            columnType: 'number',
+            style: 'string',
+            replaceMissingDataWith: 0,
+          },
+          true
+        )
+      ).toBe('123');
+    });
+
+    it('should NOT convert when replaceMissingDataWith is not 0', () => {
+      expect(
+        formatLabel('123', {
+          columnType: 'number',
+          style: 'string',
+          replaceMissingDataWith: null,
+        })
+      ).toBe('123');
+
+      expect(
+        formatLabel('123', {
+          columnType: 'number',
+          style: 'string',
+          replaceMissingDataWith: 'N/A',
+        })
+      ).toBe('123');
+
+      expect(
+        formatLabel('123', {
+          columnType: 'number',
+          style: 'string',
+          replaceMissingDataWith: 1,
+        })
+      ).toBe('123');
+    });
+
+    it('should NOT convert when textProp is an object (Date)', () => {
+      const testDate = new Date('2024-03-14T12:00:00Z');
+      const result = formatLabel(testDate, {
+        columnType: 'number',
+        style: 'string',
+        replaceMissingDataWith: 0,
+      });
+      // Date objects bypass Number conversion and get converted to string
+      expect(result).toMatch(/Thu Mar 14 2024/);
+    });
+
+    it('should NOT convert when textProp is an object (general object)', () => {
+      const testObject = { value: 123 };
+      const result = formatLabel(testObject as any, {
+        columnType: 'number',
+        style: 'string',
+        replaceMissingDataWith: 0,
+      });
+      // Objects bypass Number conversion and get converted to string representation
+      expect(result).toBe('[object Object]');
+    });
+
+    it('should handle edge case with string "0" when all conditions are met', () => {
+      expect(
+        formatLabel('0', {
+          columnType: 'number',
+          style: 'number',
+          replaceMissingDataWith: 0,
+        })
+      ).toBe('0');
+    });
+
+    it('should handle edge case with empty string when all conditions are met', () => {
+      expect(
+        formatLabel('', {
+          columnType: 'number',
+          style: 'number',
+          replaceMissingDataWith: 0,
+        })
+      ).toBe('0');
+    });
+  });
 });

apps/web/src/lib/columnFormatter.ts

@@ -43,9 +43,11 @@ export const formatLabel = (
     textIsNotNullOrUndefined &&
     columnType === 'number' &&
     !useKeyFormatter &&
-    replaceMissingDataWith === 0
+    replaceMissingDataWith === 0 &&
+    typeof textProp !== 'object' //object is a date
       ? Number(textProp)
       : textProp;
+
   let formattedText = text;
 
   if (text === null || text === undefined) {