Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

fix(plugin-chart-table): always sort descending by first metric #935

Merged
merged 2 commits into from
Feb 4, 2021

Conversation

ktmud
Copy link
Contributor

@ktmud ktmud commented Feb 4, 2021

🐛 Bug Fix

Follow up for #930 . When there is not "Sort by" metric specified, the original logic pre the API migration is to always sort by the first metric in descending order, regardless if "Sort descending" is set.

Did some refactoring on table chart's buildQuery:

  • Introduce a more general ensureIsArray utils, as replacement for extractTimeseriesLimitMetric
  • Change queryMode inference logic to be the same as in the control panel configs.

I'm also slipping a small refactor on buildQueryObject where null values are fell back to undefined so that nulls are not sent to the backend. Even thought the backend has now taken care of nulls as well: apache/superset#12905

@ktmud ktmud requested a review from a team as a code owner February 4, 2021 06:32
@vercel
Copy link

vercel bot commented Feb 4, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/superset/superset-ui/8mzs66vqn
✅ Preview: https://superset-ui-git-table-chart-sort-desc.superset.vercel.app

@ktmud ktmud requested a review from villebro February 4, 2021 06:33
Copy link
Contributor

@villebro villebro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many nice improvements!

@codecov
Copy link

codecov bot commented Feb 4, 2021

Codecov Report

Merging #935 (265135f) into master (a8bfc55) will increase coverage by 0.05%.
The diff coverage is 60.86%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #935      +/-   ##
==========================================
+ Coverage   27.37%   27.43%   +0.05%     
==========================================
  Files         412      412              
  Lines        8292     8300       +8     
  Branches     1139     1147       +8     
==========================================
+ Hits         2270     2277       +7     
- Misses       5888     5889       +1     
  Partials      134      134              
Impacted Files Coverage Δ
plugins/plugin-chart-table/src/controlPanel.tsx 38.46% <0.00%> (ø)
plugins/plugin-chart-table/src/buildQuery.ts 69.56% <46.15%> (+1.14%) ⬆️
...ges/superset-ui-core/src/query/buildQueryObject.ts 100.00% <100.00%> (ø)
...ckages/superset-ui-core/src/utils/ensureIsArray.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a8bfc55...265135f. Read the comment docs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants