superset
superset copied to clipboard
fix(pivot_table_v2): Pivot table cannot validly group rows and keep ordering
SUMMARY
Fix and improve value_a_z
, value_z_a
sort order for pivot table
- work on any size of cols and rows in pivot table options, groups them
- keep subtotals with their groups
- keep subtotals on required top/bottom position
- switches to a-z order in case of same numerical values
TESTING INSTRUCTIONS
Create any pivot v2 table with
- at least two rows (or/and to cols) and summable metric
- switch on subtotals
- try switch different orderings (all row/col groups should be consistent)
- try switch subtotal position (all row/col groups should be consistent and subtotals should be at expected place)
ADDITIONAL INFORMATION
Known issue - not tested for non monotonic summable aggregates with natural order (sum, count, avg). It's because it collect raw subtotals sumarrizing internal values. Some aggregate function (min, max for) could have unexpected behavior in sorting.
/testenv up
@zhaoyongjie Container image not yet published for this PR. Please try again when build is complete.
@zhaoyongjie Ephemeral environment creation failed. Please check the Actions logs for details.
@comdiv Thanks for the fixing. It can not work when I populate a column and select value asc
by rows.
As in previous version you should sum whole row, not just 'Abigail' because 'value order' means 'row value order'. Because you cannot express that Abigail column should be sorted while Alicia not. Sum of values used
closing/reopening to see if this'll pass CI. It also still needs a review, of course.
Codecov Report
Attention: Patch coverage is 0%
with 47 lines
in your changes missing coverage. Please review.
Project coverage is 68.74%. Comparing base (
8d4994a
) to head (e9d84df
). Report is 3548 commits behind head on master.
Files | Patch % | Lines |
---|---|---|
...hart-pivot-table/src/react-pivottable/utilities.js | 0.00% | 47 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #20588 +/- ##
==========================================
+ Coverage 66.85% 68.74% +1.88%
==========================================
Files 1753 1918 +165
Lines 65825 84047 +18222
Branches 7006 12771 +5765
==========================================
+ Hits 44010 57781 +13771
- Misses 20030 23468 +3438
- Partials 1785 2798 +1013
Flag | Coverage Δ | |
---|---|---|
javascript | 59.83% <0.00%> (+7.87%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
/testenv up
/testenv up
/testenv up
@comdiv Would you mind rebasing your PR on the latest master? Maybe that will unblock the CI
Also, for the sake of completeness, could you add before/after screenshots or recordings to the description?
There are a bunch of CI tasks not running. Let's kick-start it by closing/reopening and see if that gets us through the full list. Then hopefully the ephemeral will work ¯_(ツ)_/¯