Skip to content

feat(native-filters): add optional sort metric to select filter#14346

Merged
villebro merged 10 commits into
apache:masterfrom
preset-io:villebro/native-select-sort-metric
Apr 27, 2021
Merged

feat(native-filters): add optional sort metric to select filter#14346
villebro merged 10 commits into
apache:masterfrom
preset-io:villebro/native-select-sort-metric

Conversation

@villebro
Copy link
Copy Markdown
Member

SUMMARY

Add support for sort metric to native select filter. This is a requirement to establish feature parity with Filter Box.

SCREENSHOTS

An optional field appears on the filter config modal for the select filter:
image
When selected, the column values are sorted by the selected metric (in this case in descending order)
image

TEST PLAN

Local testing + CI

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

const datasetId = formFilter?.dataset?.value;

useEffect(() => {
if (datasetId) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If #14313 is merged first, that won't be necessary - metrics can be accessed in formFilter.dataset?.metrics

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh nice, let's get that one in first then 👍

@zhaoyongjie
Copy link
Copy Markdown
Member

This PR looks well! tested in local environment.

image

name="sortMetric"
options={metrics.map((metric: Metric) => ({
value: metric.metric_name,
label: metric.metric_name,
Copy link
Copy Markdown
Member

@zhaoyongjie zhaoyongjie Apr 26, 2021

Choose a reason for hiding this comment

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

one nit:

Suggested change
label: metric.metric_name,
label: metric.verbose_name ?? metric.metric_name,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks @zhaoyongjie ; thanks for catching this!

@pull-request-size pull-request-size Bot added size/L and removed size/M labels Apr 26, 2021
@junlincc junlincc added the dashboard:native-filters Related to the native filters of the Dashboard label Apr 26, 2021
@pull-request-size pull-request-size Bot added size/M and removed size/L labels Apr 27, 2021
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 27, 2021

Codecov Report

Merging #14346 (e58d934) into master (87a895c) will decrease coverage by 0.15%.
The diff coverage is 41.37%.

❗ Current head e58d934 differs from pull request most recent head 55eb4c7. Consider uploading reports for the commit 55eb4c7 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14346      +/-   ##
==========================================
- Coverage   76.86%   76.71%   -0.16%     
==========================================
  Files         954      954              
  Lines       48207    48230      +23     
  Branches     6008     6021      +13     
==========================================
- Hits        37056    37001      -55     
- Misses      10956    11034      +78     
  Partials      195      195              
Flag Coverage Δ
javascript 71.32% <41.37%> (-0.32%) ⬇️

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

Impacted Files Coverage Δ
...tiveFilters/FilterBar/FilterSets/FiltersHeader.tsx 91.66% <0.00%> (-5.56%) ⬇️
...mponents/nativeFilters/FiltersConfigModal/utils.ts 65.78% <ø> (ø)
...ontend/src/filters/components/Select/buildQuery.ts 25.00% <0.00%> (-8.34%) ⬇️
...et-frontend/src/filters/components/Select/types.ts 100.00% <ø> (ø)
...onfigModal/FiltersConfigForm/FiltersConfigForm.tsx 68.92% <38.46%> (+2.46%) ⬆️
...nd/src/dashboard/components/nativeFilters/utils.ts 79.48% <66.66%> (-1.60%) ⬇️
superset-frontend/src/reduxUtils.ts 69.04% <71.42%> (-0.58%) ⬇️
...trols/DateFilterControl/components/CommonFrame.tsx 41.66% <0.00%> (-50.00%) ⬇️
...nents/nativeFilters/FilterBar/FilterSets/index.tsx 47.47% <0.00%> (-29.30%) ⬇️
...et-frontend/src/dashboard/actions/nativeFilters.ts 57.14% <0.00%> (-23.81%) ⬇️
... and 8 more

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 87a895c...55eb4c7. Read the comment docs.

@villebro villebro merged commit 40fb94d into apache:master Apr 27, 2021
@villebro villebro deleted the villebro/native-select-sort-metric branch April 27, 2021 17:28
amitmiran137 pushed a commit that referenced this pull request Apr 28, 2021
* feat(native-filters): add optional sort metric to select filter

* use verbose name when defined

* fixes

* lint

* disable flaky test

* disable flaky test

* disable flaky test

(cherry picked from commit 40fb94d)
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
…he#14346)

* feat(native-filters): add optional sort metric to select filter

* use verbose name when defined

* fixes

* lint

* disable flaky test

* disable flaky test

* disable flaky test
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 First shipped in 1.2.0 labels Mar 12, 2024
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
…he#14346)

* feat(native-filters): add optional sort metric to select filter

* use verbose name when defined

* fixes

* lint

* disable flaky test

* disable flaky test

* disable flaky test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels dashboard:native-filters Related to the native filters of the Dashboard preset-io release:note size/M test:case v1.2 🚢 1.2.0 First shipped in 1.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants