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: inability to remove chart filter when dashboard time filter is applied #25217

Merged
merged 2 commits into from
Sep 12, 2023

Conversation

lilykuang
Copy link
Member

SUMMARY

When a chart is added to a dashboard with a time range filter applied, adding an additional filter to the chart in Explore results in an issue where the added filter cannot be removed as expected.

Repro Steps:

  1. Create a chart and add it to a dashboard. For example, a pie chart with video_game_sales, where the dimension is "genre" and the metric is "sum(global_sales)."
  2. On the dashboard, apply a native filter for a time range, such as 1998-2023.
  3. Click on the chart title to access it in Explore.
  4. In Explore, add a new filter to the chart. In this case, add the filter "publisher in Nintendo"
  5. Overwrite the chart and return to the dashboard.
  6. Access the pie chart in Explore once more and attempt to remove the state filter. Overwrite the chart.

Expected Results:
The state filter should be removable from the chart after the second overwrite in step 5.

Actual Results:
Unfortunately, the state filter still persists, and it cannot be removed as expected.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:
filter
After:
filter_a

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • 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

@pull-request-size pull-request-size bot added size/M and removed size/S labels Sep 7, 2023
@yousoph
Copy link
Member

yousoph commented Sep 7, 2023

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

@yousoph Ephemeral environment spinning up at http://34.209.85.6:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@@ -76,18 +81,24 @@ export const getSlicePayload = (
// would end up as an extra filter and when overwriting the chart the original
// time range adhoc_filter was lost
if (isEmpty(adhocFilters?.adhoc_filters) && !isEmpty(formDataFromSlice)) {
adhocFilters = extractAddHocFiltersFromFormData(formDataFromSlice);
formDataFromSlice?.adhoc_filters?.forEach(filter => {
Copy link
Member

Choose a reason for hiding this comment

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

@lilykuang the comment states:

Retain adhoc_filters from the slice if no adhoc_filters are present

but now we're only retaining temporal filters where !isExtra. Is this correct or do we also need to retain adhoc filters that are not temporal? If this is correct, can you adjust the comment to reflect the change?

@michael-s-molina michael-s-molina merged commit a9512c1 into apache:master Sep 12, 2023
26 checks passed
@michael-s-molina michael-s-molina added the v3.0 Label added by the release manager to track PRs to be included in the 3.0 branch label Sep 12, 2023
@github-actions
Copy link
Contributor

Ephemeral environment shutdown and build artifacts deleted.

michael-s-molina pushed a commit that referenced this pull request Sep 12, 2023
jinghua-qa pushed a commit to preset-io/superset that referenced this pull request Sep 14, 2023
eschutho pushed a commit to Superset-Community-Partners/superset that referenced this pull request Sep 21, 2023
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
@mistercrunch mistercrunch added 🍒 3.0.0 🍒 3.0.1 🍒 3.0.2 🍒 3.0.3 🍒 3.0.4 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.1.0 labels Mar 8, 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/M v3.0 Label added by the release manager to track PRs to be included in the 3.0 branch 🍒 3.0.0 🍒 3.0.1 🍒 3.0.2 🍒 3.0.3 🍒 3.0.4 🚢 3.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants