-
Notifications
You must be signed in to change notification settings - Fork 14k
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: Unable to export multiple Dashboards with the same name #20383
fix: Unable to export multiple Dashboards with the same name #20383
Conversation
Codecov Report
@@ Coverage Diff @@
## master #20383 +/- ##
==========================================
- Coverage 66.52% 66.52% -0.01%
==========================================
Files 1739 1739
Lines 65141 65141
Branches 6900 6900
==========================================
- Hits 43336 43335 -1
- Misses 20052 20053 +1
Partials 1753 1753
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@@ -284,7 +288,7 @@ def test_export_dashboard_command_key_order(self, mock_g1, mock_g2): | |||
@pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") | |||
@patch("superset.dashboards.commands.export.suffix") | |||
def test_append_charts(self, mock_suffix): | |||
"""Test that oprhaned charts are added to the dashbaord position""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eagle eyes! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I'd merge it, but I would love a stamp from @betodealmeida or @eschutho to make sure there aren't some other implications (perhaps on the import side of the equation) that I'm unaware of.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @diegomedina248
SUMMARY
Users can export multiple Dashboards in a single ZIP.
However, selecting two Dashboards with the same name won’t work properly - only one is included on the export file.
This PR follows the same approach taken for the charts, and adds the dashboard id at the end of the file name.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Example:
TESTING INSTRUCTIONS
Export Test
.Ensure two files are created under dashboards folder.
ADDITIONAL INFORMATION