Skip to content

fix(dashboard): import with overwrite flag replaces charts instead of merging #36551

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.

Sign up for GitHub

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

Jump to bottom
Open
rusackas wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(dashboard): import with overwrite flag replaces charts instead of merging #36551

rusackas wants to merge 2 commits into from

Conversation

@rusackas
Copy link
Member

@rusackas rusackas commented Dec 11, 2025

SUMMARY

Fixes #22127

When importing dashboards via /api/v1/dashboard/import/ with overwrite=True, the dashboard's chart associations were being merged with existing charts instead of being replaced. This made it impossible to use the import API for proper source control workflows where the imported dashboard should exactly match the import file.

Root Cause:
The ImportDashboardsCommand._import() method was only adding new chart relationships without removing existing ones when overwrite=True. In contrast, the ImportAssetsCommand._import() method (from PR #22208) correctly deletes existing relationships before inserting new ones.

The Fix:
This PR mirrors the behavior of the assets import API by:

  1. Deleting all existing dashboard-slice relationships when overwrite=True before inserting the new ones from the import
  2. Only querying existing relationships when overwrite=False (performance optimization)
  3. Changing break to continue when encountering unknown chart UUIDs so all valid charts are processed instead of stopping at the first unknown one (also identified in PR fix(dashboard): Dashboard import commands not correctly replacing charts #25102 review)

Files Changed:

  • superset/commands/dashboard/importers/v1/__init__.py - Core fix implementation
  • tests/unit_tests/dashboards/commands/importers/v1/import_command_test.py - New tests for overwrite behavior

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A - Backend API behavior change

TESTING INSTRUCTIONS

  1. Create a dashboard with 2 charts (chartA and chartB)
  2. Export the dashboard
  3. Extract the ZIP and remove chartB references:
    • Delete charts/chartB.yaml
    • Remove chartB from dashboards/dashboard.yaml position JSON
  4. Re-ZIP and import with overwrite=true: 
    curl -X POST "http://localhost:8088/api/v1/dashboard/import/" \
  -H "Authorization: Bearer $TOKEN" \
  -F "formData=@modified_dashboard.zip;type=application/zip" \
  -F "overwrite=true"
  5. Before fix: Dashboard has both chartA and chartB (merged)
  6. After fix: Dashboard has only chartA (replaced)

ADDITIONAL INFORMATION

Related PRs:

Generated with Claude Code

@rusackas @claude
fix(dashboard): import with overwrite flag replaces charts instead of…
a405e32 
… merging

Fixes #22127

When importing dashboards via /api/v1/dashboard/import/ with overwrite=True,
the dashboard's chart associations were being merged with existing charts
instead of being replaced. This made it impossible to use the import API
for proper source control workflows where the imported dashboard should
exactly match the import file.

The fix mirrors the behavior of the assets import API (PR #22208) by:
1. Deleting all existing dashboard-slice relationships when overwrite=True
   before inserting the new ones from the import
2. Only querying existing relationships when overwrite=False (optimization)
3. Changing break to continue when encountering unknown chart UUIDs so all
   valid charts are processed instead of stopping at the first unknown one

Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@dosubot dosubot bot added api Related to the REST API change:backend Requires changing the backend labels Dec 11, 2025
@github-actions github-actions bot removed the api Related to the REST API label Dec 11, 2025
Copilot AI reviewed Dec 15, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where importing dashboards with overwrite=True was merging chart associations instead of replacing them, preventing proper source control workflows. The fix aligns the dashboard import behavior with the assets import API by deleting existing chart relationships before inserting new ones when overwriting.

Key changes:

  • Deletes all existing dashboard-slice relationships before inserting new ones when overwrite=True
  • Changes break to continue to process all valid charts instead of stopping at first unknown UUID
  • Adds comprehensive unit tests verifying overwrite and merge behaviors

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
superset/commands/dashboard/importers/v1/init.py Core fix: deletes existing chart relationships when overwriting and processes all valid chart UUIDs
tests/unit_tests/dashboards/commands/importers/v1/import_command_test.py New test file with 4 tests covering overwrite replacement, merge behavior, and edge cases

superset/commands/dashboard/importers/v1/__init__.py
Comment on lines +219 to +222
if dashboard_chart_ids:
values = [
{"dashboard_id": dashboard_id, "slice_id": chart_id}
for (dashboard_id, chart_id) in dashboard_chart_ids
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The check if dashboard_chart_ids: guards against inserting an empty list, but the main issue is that when overwrite=True and the imported dashboard has no charts, the old relationships are deleted (line 189-193) but nothing prevents the code from attempting an insert with an empty list. While SQLAlchemy typically handles empty inserts gracefully, this check should be placed before both the delete operation and the insert to maintain consistency and avoid unnecessary database operations when there are no chart relationships to manage.

Copilot uses AI. Check for mistakes.
tests/unit_tests/dashboards/commands/importers/v1/import_command_test.py
Comment on lines +204 to +205
expected_number_of_dashboards = len(dashboards_config_1)
expected_number_of_charts = len(charts_config_1)
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The variable names expected_number_of_dashboards and expected_number_of_charts are misleading. These represent the count of dashboard configurations and chart configurations in the import, not necessarily the expected number after import (especially for charts, which counts chart-dashboard associations in this test, not unique charts). Consider renaming to import_config_dashboard_count and import_config_chart_count, or add assertions that clarify what these values represent in the context of the test.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@betodealmeida betodealmeida Awaiting requested review from betodealmeida

@michael-s-molina michael-s-molina Awaiting requested review from michael-s-molina

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

change:backend Requires changing the backend size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Import dashboard/assets with overwrite flag does not replace dashboards

1 participant

@rusackas