Fix Storage Chart
🛠️ Fix Data Fetching, Conditional Logic, and Runtime Error in History Charts
Overview
This PR addresses critical issues in the AbstractHistoryChart and SingleChartComponent 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
- Problem: The
queryHistoricTimeseriesDatamethod was improperly returning empty data regardless of the API response, causing charts to display 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.
- Problem: The
-
Faulty Conditional Statement
- Problem: The condition
was always evaluating toif ("_sum/EssActivePowerL1" && "_sum/EssActivePowerL2" && "_sum/EssActivePowerL3" in result.data && this.showPhases == true) {truedue to operator precedence, leading to logical errors. - Solution: Updated the condition to explicitly check each channel using the
inoperator:if ("_sum/EssActivePowerL1" in result.data && "_sum/EssActivePowerL2" in result.data && "_sum/EssActivePowerL3" in result.data && this.showPhases === true) {
- Problem: The condition
-
Runtime Error in SingleChartComponent
- Problem: Encountered a runtime error:
This was caused by attempting to callsinglechart.component.ts:184 TypeError: Cannot read properties of undefined (reading 'forEach') at singlechart.component.ts:89:57 at _ZoneDelegate.invoke (zone.js:369:28) ...forEachon an undefined property, likely due to missing data validation. - Solution: Added checks to ensure that the data arrays are defined before invoking
forEach. Enhanced error handling to initialize empty charts gracefully when data is missing.
- Problem: Encountered a runtime error:
Changes Made
-
Refactored
queryHistoricTimeseriesDataMethod:- 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
ifstatement to individually check each required channel's existence inresult.data. - Used strict equality (
===) forthis.showPhasesto ensure type-safe comparison.
- Rewrote the
-
Resolved Runtime Error:
- Added null and undefined checks before calling
forEachon data arrays. - Ensured that
result.datacontains the necessary properties before processing. - Updated error handling to log meaningful messages and prevent application crashes.
- Added null and undefined checks before calling
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 and runtime exceptions prevents the application from crashing and aids in debugging.
- Improved Code Quality: Addressing TypeScript warnings and runtime errors leads to cleaner, more maintainable code.
Testing
- Manual Testing:
- Verified that charts now display correct data when API responses are received.
- Confirmed that conditional statements behave as expected without triggering TypeScript warnings.
- Reproduced and fixed the runtime error by ensuring data arrays are defined before processing.
- Tested error scenarios to ensure charts initialize gracefully without breaking the application.
Please review the changes and approve the merge to enhance the reliability and accuracy of our history chart components.
Codecov Report
:white_check_mark: All 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
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
@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
Has this already been fixed in a PR ?
Has this already been fixed in a PR ?
One of our colleagues is currently working on refactoring the AbstractHistoryChart. Unfortunately, I don't know how long it will take to complete. The other two changed files can be added.
This PR has been automatically marked as stale due to inactivity. It will be closed in 7 days if no further activity occurs.
This PR has been closed due to inactivity
It was automatically closed because there has been no recent activity. If the PR is still relevant, please feel free to reopen and update it and add any new information.