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

feat(legacy-plugin-chart-sankey): allow sort by metric #831

Merged
merged 1 commit into from
Nov 10, 2020

Conversation

ktmud
Copy link
Contributor

@ktmud ktmud commented Nov 9, 2020

🏆 Enhancements

Adding a "Sort by metric" checkbox control for Sankey chart. If selected, the query results will be sorted by metric in descending order. This is useful when there are many rows of data but it's not easy to regroup source and targets via custom SQL queries (e.g. when working with a datasource that does not support arbitrary SQL queries).

Also added a caveat warning for row limit:

@ktmud ktmud requested a review from a team as a code owner November 9, 2020 21:59
@vercel
Copy link

vercel bot commented Nov 9, 2020

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/51o09fp8b
✅ Preview: https://superset-ui-git-sankey-sort-by-metric.superset.vercel.app

@codecov
Copy link

codecov bot commented Nov 9, 2020

Codecov Report

Merging #831 (5c4fa7e) into master (8c992ae) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #831      +/-   ##
==========================================
- Coverage   25.82%   25.82%   -0.01%     
==========================================
  Files         362      363       +1     
  Lines        7976     7977       +1     
  Branches     1066     1066              
==========================================
  Hits         2060     2060              
- Misses       5796     5797       +1     
  Partials      120      120              
Impacted Files Coverage Δ
...ins/legacy-plugin-chart-sankey/src/controlPanel.ts 0.00% <0.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 8c992ae...5c4fa7e. Read the comment docs.

@villebro
Copy link
Contributor

A by-thought: if there are many orphaned paths (=both source and target only used in a single path), I wonder if these should be excluded from the chart (not sure what the behaviour is currently).

@ktmud ktmud merged commit 1f8a700 into apache-superset:master Nov 10, 2020
@ktmud
Copy link
Contributor Author

ktmud commented Nov 10, 2020

A by-thought: if there are many orphaned paths (=both source and target only used in a single path), I wonder if these should be excluded from the chart (not sure what the behavior is currently).

I think they would just be considered part of the flow at the first level. Even though the source might actually be at the second or later level if there's no row limit... That's why a warning about row limit is important.

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

Successfully merging this pull request may close these issues.

2 participants