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

[dashboard] fix filter_scopes when copy dashboard with duplicate_slices #9188

Merged

Conversation

graceguo-supercat
Copy link

CATEGORY

Choose one

  • Bug Fix
  • Enhancement (new features, refinement)
  • Refactor
  • Add tests
  • Build / Development Environment
  • Documentation

SUMMARY

When user copy dashboard with also copy (duplicate) charts, dashboard duplicates each slices in the old dash and use new slice_id in the new dash. We should update filter_scopes with new slice ids.

TEST PLAN

CI and manual test

ADDITIONAL INFORMATION

REVIEWERS

@serenajiang @john-bodley

@codecov-io
Copy link

Codecov Report

Merging #9188 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9188      +/-   ##
==========================================
- Coverage   59.16%   59.13%   -0.04%     
==========================================
  Files         372      372              
  Lines       11935    11938       +3     
  Branches     2927     2925       -2     
==========================================
- Hits         7061     7059       -2     
- Misses       4694     4699       +5     
  Partials      180      180
Impacted Files Coverage Δ
...ontend/src/components/ListView/TableCollection.tsx 88.88% <0%> (-3.71%) ⬇️
...frontend/src/views/dashboardList/DashboardList.tsx 69.72% <0%> (-3.43%) ⬇️
...uperset-frontend/src/views/chartList/ChartList.tsx 74.48% <0%> (-0.26%) ⬇️
...et-frontend/src/components/ConfirmStatusChange.tsx 100% <0%> (ø) ⬆️
...rset-frontend/src/components/ListView/ListView.tsx 92.4% <0%> (+0.19%) ⬆️

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 c1750af...eb04f8a. Read the comment docs.

@@ -1195,6 +1195,20 @@ def copy_dash(self, dashboard_id):
old_id = "{}".format(value.get("meta").get("chartId"))
new_id = int(old_to_new_sliceids[old_id])
value["meta"]["chartId"] = new_id

if "filter_scopes" in data:
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks pretty similar to the chunk of code here. Can we pull this out into a function?

Copy link
Author

Choose a reason for hiding this comment

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

added a util function to clean up.

@graceguo-supercat graceguo-supercat merged commit 421aeb4 into apache:master Feb 24, 2020
@graceguo-supercat graceguo-supercat deleted the gg-FixCopyDashFilterScopes branch June 11, 2020 23:20
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.36.0 labels Feb 28, 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/S 🚢 0.36.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants