From ccb0c9c32a621a48b99379f382bdf6c8d1ed66f6 Mon Sep 17 00:00:00 2001
From: Rahul Barwal <rahul.barwal@appsmith.com>
Date: Wed, 23 Oct 2024 14:03:46 +0530
Subject: [PATCH] fix: Refactor handling of empty chart data in ChartWidget
 (#37009)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Description
* This pull request handles empty chart dAata more efficiently.
* lso updates tests to ensure this is not overlooked again.

These changes ensure that the ChartWidget can handle scenarios where the
chart data is null or undefined, and that the tests accurately reflect
this behavior.


Fixes #37008
_or_
Fixes `Issue URL`
> [!WARNING]
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags="@tag.Chart"

### :mag: Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/11473049534>
> Commit: 08602d3a0658b1753d4d377ce9673379ab234d3f
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11473049534&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Chart`
> Spec:
> <hr>Wed, 23 Oct 2024 04:52:48 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Bug Fixes**
- Improved data handling in the ChartWidget to prevent runtime errors
when chart data is undefined.

- **Tests**
- Added a new test case to verify the behavior of the emptyChartData
function when series data is null or undefined, enhancing test coverage.

- **Documentation**
- Updated interface to include an optional property for handling data
point clicks in the ChartWidget.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---
 app/client/src/widgets/ChartWidget/widget/index.test.ts | 9 +++++++++
 app/client/src/widgets/ChartWidget/widget/index.tsx     | 5 ++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/app/client/src/widgets/ChartWidget/widget/index.test.ts b/app/client/src/widgets/ChartWidget/widget/index.test.ts
index 083e92f32d..040cc61a89 100644
--- a/app/client/src/widgets/ChartWidget/widget/index.test.ts
+++ b/app/client/src/widgets/ChartWidget/widget/index.test.ts
@@ -72,6 +72,15 @@ describe("emptyChartData", () => {
       expect(emptyChartData(props)).toEqual(true);
     });
 
+    it("returns true if each series is null or undefined", () => {
+      const props = JSON.parse(JSON.stringify(basicEChartProps));
+
+      props.chartData.seriesID1 = { data: undefined };
+      props.chartData.seriesID2 = { data: null };
+
+      expect(emptyChartData(props)).toEqual(true);
+    });
+
     it("returns true if no series is present", () => {
       const props = JSON.parse(JSON.stringify(basicEChartProps));
 
diff --git a/app/client/src/widgets/ChartWidget/widget/index.tsx b/app/client/src/widgets/ChartWidget/widget/index.tsx
index 9b95ed6651..65498ad611 100644
--- a/app/client/src/widgets/ChartWidget/widget/index.tsx
+++ b/app/client/src/widgets/ChartWidget/widget/index.tsx
@@ -51,7 +51,10 @@ export const emptyChartData = (props: ChartWidgetProps) => {
     const builder = new EChartsDatasetBuilder(props.chartType, props.chartData);
 
     for (const seriesID in builder.filteredChartData) {
-      if (Object.keys(props.chartData[seriesID].data).length > 0) {
+      if (
+        Array.isArray(props.chartData[seriesID].data) &&
+        props.chartData[seriesID].data.length > 0
+      ) {
         return false;
       }
     }