Skip to content
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

feat(TreeData): add optional Aggregators to Tree Data grids #1074

Merged
merged 14 commits into from
Aug 19, 2023

Conversation

ghiscoding
Copy link
Owner

@ghiscoding ghiscoding commented Aug 15, 2023

  • add the possibility to use the same Aggregators that are used in Grouping, however all Aggregators had to be modified to support tree data which has a few differences
  • also note that when Aggregators are used with Grouping then the aggregation are under a separate row with __groupTotals details... however on Tree Data it will have its totals under __treeTotals and also directly on the parent item
  • currently only 5 Aggregators are supported with Tree Data: Avg, Sum, Min, Max and Count
  • also note that AvgAggregator will automatically give you "avg", "count" and "sum", so if the user needs all 3 then it is a lot better to use Avg for better perf. The other reason why calling Avg+Sum will give you worst perf is that each aggregation is separate and will redo the item count and sum work for each aggregators, so it is better to simply use Avg instead of Avg+Sum
  • also added Formatters.treeParseTotals that can be used to parse and auto-detect when to use GroupTotalFormatters or regular Formatter, user must also provide a treeTotalsFormatter or groupTotalsFormatter
    • this formatter will work with both regular Formatters and GroupTotalFormatters, it will auto-detect if it has __treeTotals then it will use the GroupTotalFormatters or else it will try to use regular Formatters.

TODOs

  • fix and add Jest unit tests
  • fix and add Cypress E2E tests
    • make sure to test that adding new files automatically updates the Aggregators totals
  • add Formatter to parse & show Tree Totals (similar to GroupTotalsFormatters)
  • might also need adjustments for Excel/CSV/Text exports
  • does not currently update aggregation when applying filter(s)
    • WILL DO IN SEPARATE PR
    • Ag-Grid has a grid option flag for this suppressAggFilteredOnly or column flag groupAggFiltering
      • true will show full total, while false will only show filtered data total
  • adding new item to hierarchical Tree Data always flicker (only Example 6) because it always go back to top then we scroll back to the new item, it's caused by a reset of the data which we might not need and would require more thorough inspection

below is an example with both Avg and Sum Aggregators displayed

image

- add the possibility to use the same Aggregators that are used in Grouping, however all Aggregators had to be modified to support tree data which has a few differences
- also note that when Aggregators are used with Grouping then the aggregation are under a separate row with `__groupTotals` details... however on Tree Data it will have its totals under `__treeTotals` and also directly on the parent item
- currently only 5 Aggregators are supported with Tree Data: Avg, Sum, Min, Max and Count
- also note that AvgAggregator will automatically give you "avg", "count" and "sum", so if the user needs all 3 then it is a lot better to use Avg for better perf. The other reason why calling Avg+Sum will give you worst perf is that each aggregation is separate and will redo the item count and sum work for each aggregators, so it is better to simply use Avg instead of Avg+Sum
@ghiscoding ghiscoding changed the title feat(TreeData): add optional Aggregators to Tree Data grids WIP - feat(TreeData): add optional Aggregators to Tree Data grids Aug 15, 2023
@codecov
Copy link

codecov bot commented Aug 16, 2023

Codecov Report

Merging #1074 (7ec3d84) into master (07efeb6) will not change coverage.
Report is 8 commits behind head on master.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            master     #1074    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          244       245     +1     
  Lines        16564     16798   +234     
  Branches      5933      6030    +97     
==========================================
+ Hits         16564     16798   +234     
Files Changed Coverage Δ
packages/common/src/aggregators/avgAggregator.ts 100.00% <100.00%> (ø)
packages/common/src/aggregators/cloneAggregator.ts 100.00% <100.00%> (ø)
packages/common/src/aggregators/countAggregator.ts 100.00% <100.00%> (ø)
...kages/common/src/aggregators/distinctAggregator.ts 100.00% <100.00%> (ø)
packages/common/src/aggregators/maxAggregator.ts 100.00% <100.00%> (ø)
packages/common/src/aggregators/minAggregator.ts 100.00% <100.00%> (ø)
packages/common/src/aggregators/sumAggregator.ts 100.00% <100.00%> (ø)
packages/common/src/formatters/formatters.index.ts 100.00% <100.00%> (ø)
packages/common/src/formatters/index.ts 100.00% <100.00%> (ø)
...ackages/common/src/formatters/multipleFormatter.ts 100.00% <100.00%> (ø)
... and 4 more

... and 4 files with indirect coverage changes

- added a new flag to disable cell format auto-detection that is causing issues when having multiple text+numbers in same cell which the auto-detect doesn't work correctly with, the new flag will be used with the following: `excelExportOptions: { autoDetectCellFormat: false }` and that is the same options for both column and/or grid options.
@ghiscoding ghiscoding changed the title WIP - feat(TreeData): add optional Aggregators to Tree Data grids feat(TreeData): add optional Aggregators to Tree Data grids Aug 18, 2023
@ghiscoding ghiscoding merged commit 6af5fd1 into master Aug 19, 2023
@ghiscoding ghiscoding deleted the feat/tree-data-aggregation branch August 19, 2023 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant