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: dashboard should not add extra_filters onto chart annotation #10115

Merged

Conversation

graceguo-supercat
Copy link

@graceguo-supercat graceguo-supercat commented Jun 20, 2020

SUMMARY

We used to be able to show filtered vs all data in the same chart by using annotation. See below is an example, region vs global.

But I recently found there was an issue introduced by #8057, where dashboard filter (extra_filters param) is applied to chart's annotation layer. This will cause chart and annotation layer both have same query, so we can't compare filtered data vs original annotation layer.

This PR is try to fix this issue, not passing dashboard filter parameters to chart's annotation layer.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

before:
Screen Shot 2020-06-20 at 1 02 36 AM

after:
Screen Shot 2020-06-20 at 1 06 17 AM

TEST PLAN

Manual test

ADDITIONAL INFORMATION

@ktmud @etr2460 @mistercrunch

Copy link
Member

@ktmud ktmud left a comment

Choose a reason for hiding this comment

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

LGTM!

@graceguo-supercat graceguo-supercat merged commit 6910053 into apache:master Jun 21, 2020
auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.37.0 labels Mar 12, 2024
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 size/XS 🚢 0.37.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants