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

fix: Aggregations should be available when creating a rollup #1129

Merged
merged 1 commit into from
Mar 3, 2023

Conversation

mofojed
Copy link
Member

@mofojed mofojed commented Mar 3, 2023

  • When there is a rollup config, aggregations go through the rollup config
  • Allow aggregations to be accessed when using a rollup config

- When there is a rollup config, aggregations go through the rollup config
  - https://github.com/deephaven/web-client-ui/blob/a069543812b6c544957ebf664e0918e98a3affbf/packages/iris-grid/src/IrisGrid.tsx#L1288
- Allow aggregations to be accessed when using a rollup config
- A better fix would be to fix the abstraction and have aggregations applied to the rollupconfig within the model, but bigger change. Will do for next release.
@mofojed mofojed requested review from niloc132 and mattrunyon March 3, 2023 18:05
@mofojed mofojed self-assigned this Mar 3, 2023
@codecov
Copy link

codecov bot commented Mar 3, 2023

Codecov Report

Merging #1129 (70fbc92) into main (a069543) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1129   +/-   ##
=======================================
  Coverage   43.38%   43.38%           
=======================================
  Files         435      435           
  Lines       32691    32692    +1     
  Branches     8242     8243    +1     
=======================================
+ Hits        14184    14185    +1     
  Misses      18458    18458           
  Partials       49       49           
Flag Coverage Δ
unit 43.38% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/iris-grid/src/IrisGrid.tsx 27.01% <100.00%> (+0.04%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mattrunyon
Copy link
Collaborator

Is there a related ticket in web-client-ui or core?

@mattrunyon mattrunyon linked an issue Mar 3, 2023 that may be closed by this pull request
@mofojed mofojed merged commit c3d8433 into deephaven:main Mar 3, 2023
@mofojed mofojed deleted the fix-rollup-aggregations branch March 3, 2023 22:02
@github-actions github-actions bot locked and limited conversation to collaborators Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to set aggregations when using a rollup
2 participants