-
Notifications
You must be signed in to change notification settings - Fork 417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Storage Chart #2828
base: develop
Are you sure you want to change the base?
Fix Storage Chart #2828
Conversation
# 🛠️ Fix Data Fetching and Conditional Logic in History Charts ## **Overview** This PR addresses critical issues in the `AbstractHistoryChart` component related to data fetching and conditional logic, ensuring that historical timeseries data is correctly retrieved and processed for accurate chart rendering. ## **Issues Addressed** 1. **Incorrect Data Fetching Logic** - **Problem:** The `queryHistoricTimeseriesData` method was improperly returning empty data regardless of the API response, leading to charts displaying undefined or empty datasets. - **Solution:** Refactored the method to properly handle asynchronous API responses, ensuring that actual data is returned and errors are handled gracefully. 2. **Faulty Conditional Statement** - **Problem:** The condition ```typescript if ("_sum/EssActivePowerL1" && "_sum/EssActivePowerL2" && "_sum/EssActivePowerL3" in result.data && this.showPhases == true) { ``` was always evaluating to `true` due to operator precedence, causing logical errors. - **Solution:** Updated the condition to explicitly check each channel using the `in` operator: ```typescript if ("_sum/EssActivePowerL1" in result.data && "_sum/EssActivePowerL2" in result.data && "_sum/EssActivePowerL3" in result.data && this.showPhases === true) { ``` ## **Changes Made** - **Refactored `queryHistoricTimeseriesData` Method:** - Removed the immediate return of empty data. - Ensured the method resolves with actual API responses. - Implemented proper error handling by rejecting promises on failures. - Added a stale response check to prevent race conditions. - **Fixed Conditional Logic:** - Rewrote the `if` statement to individually check each required channel's existence in `result.data`. - Used strict equality (`===`) for `this.showPhases` to ensure type-safe comparison. ## **Why These Changes?** - **Accurate Data Representation:** Ensures charts display real and complete historical data, improving reliability. - **Elimination of Logical Errors:** Correct conditional checks prevent unintended code execution, maintaining data integrity. - **Enhanced Error Handling:** Properly managing API errors prevents the application from masking issues and aids in debugging. - **Improved Code Quality:** Addressing TypeScript warnings leads to cleaner, more maintainable code. ## **Testing** - Verified that charts now display correct data when API responses are received. - Confirmed that conditional statements behave as expected without triggering TypeScript warnings. - Ensured that error scenarios gracefully initialize empty charts without breaking the application. --- **Please review the changes and approve the merge to enhance the reliability and accuracy of our history chart components.**
Codecov ReportAll modified and coverable lines are covered by tests ✅ Additional details and impacted files@@ Coverage Diff @@
## develop #2828 +/- ##
==========================================
Coverage 56.01% 56.01%
Complexity 8197 8197
==========================================
Files 2103 2103
Lines 89017 89017
Branches 6560 6560
==========================================
Hits 49854 49854
+ Misses 37445 37442 -3
- Partials 1718 1721 +3 |
@lukasrgr could you review? |
@lukasrgr any news ? |
@Sn0w3y i am currently working on this issue as well. Your changes in singlechart and totalchart component look fine to me, but there are some changes in abstracthistorychart i solved differently. Anyways thanks for this contribution. |
@Sn0w3y sorry for my late response, im working on refactoring the storage chart anyway, than we dont have to touch this implementation of the abstracthistorychart |
🛠️ Fix Data Fetching, Conditional Logic, and Runtime Error in History Charts
Overview
This PR addresses critical issues in the
AbstractHistoryChart
andSingleChartComponent
related to data fetching, conditional logic, and a runtime error. These fixes ensure that historical timeseries data is accurately retrieved and processed, and that the application handles data conditions correctly to prevent crashes.Issues Addressed
Incorrect Data Fetching Logic
queryHistoricTimeseriesData
method was improperly returning empty data regardless of the API response, causing charts to display undefined or empty datasets.Faulty Conditional Statement
true
due to operator precedence, leading to logical errors.in
operator:Runtime Error in SingleChartComponent
forEach
on an undefined property, likely due to missing data validation.forEach
. Enhanced error handling to initialize empty charts gracefully when data is missing.Changes Made
Refactored
queryHistoricTimeseriesData
Method:Fixed Conditional Logic:
if
statement to individually check each required channel's existence inresult.data
.===
) forthis.showPhases
to ensure type-safe comparison.Resolved Runtime Error:
forEach
on data arrays.result.data
contains the necessary properties before processing.Why These Changes?
Testing
Please review the changes and approve the merge to enhance the reliability and accuracy of our history chart components.