fix(assets api): import replaces dashboard

[Stanhoucke]

SUMMARY

For existing dashboards, modifications imported via /api/v1/assets/import/ are only appended if the modified dashboard contains a new chart. Modifications to dashboards that remove a chart are not implemented when imported through the assets API.

This change updates the assets API import method to remove all existing references to chart ids for the imported dashboard(s) before inserting the new chart ids. The change should benefit the management of Superset assets under source control.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

Remove a chart from a dashboard:

• Export a dashboard and unzip assets bundle.
• Remove a chart and all references to it from the exported yaml files
• Import modified assets bundle as a zip by posting to /api/v1/assets/import/ (see https://superset.apache.org/docs/api/#api)
• Navigate to the modified dashboard in the Superset UI and observe the deleted chart is no longer visible

Add a chart to a dashboard:

• In the same dashboard, remove a chart using the UI and save the dashboard
• Import the previously exported assets bundle as a zip by posting to /api/v1/assets/import/
• Navigate to the modified dashboard in the Superset UI and observe the deleted chart is now reinstated

ADDITIONAL INFORMATION

• [x] Has associated issue: https://github.com/apache/superset/issues/22127
• [ ] 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

[nytai]

Can we add a unit test for this behavior? there should already be some for import/export you can expand on

[Stanhoucke]

Sure, I'll add a test 👍

[codecov[bot]]

Codecov Report

Merging #22208 (92df588) into master (a77b2d6) will increase coverage by 1.44%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #22208      +/-   ##
==========================================
+ Coverage   65.49%   66.94%   +1.44%     
==========================================
  Files        1835     1836       +1     
  Lines       69927    70077     +150     
  Branches     7590     7590              
==========================================
+ Hits        45802    46913    +1111     
+ Misses      22159    21198     -961     
  Partials     1966     1966              

Flag	Coverage Δ
hive	52.53% <28.57%> (?)
mysql	77.95% <100.00%> (-0.13%)	:arrow_down:
postgres	78.02% <100.00%> (-0.13%)	:arrow_down:
presto	52.42% <28.57%> (?)
python	81.23% <100.00%> (+2.95%)	:arrow_up:
sqlite	76.48% <100.00%> (-0.13%)	:arrow_down:
unit	51.09% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset/commands/importers/v1/assets.py	85.54% <100.00%> (-0.18%)	:arrow_down:
superset/db_engine_specs/databricks.py	64.96% <0.00%> (-17.97%)	:arrow_down:
superset/charts/data/api.py	88.34% <0.00%> (-1.47%)	:arrow_down:
superset/views/database/views.py	31.27% <0.00%> (-0.09%)	:arrow_down:
superset/models/helpers.py	38.19% <0.00%> (-0.09%)	:arrow_down:
superset/charts/schemas.py	99.35% <0.00%> (ø)
superset/utils/pandas_postprocessing/sort.py	100.00% <0.00%> (ø)
superset/db_engine_specs/risingwave.py	100.00% <0.00%> (ø)
superset/connectors/base/models.py	87.20% <0.00%> (+0.07%)	:arrow_up:
superset/views/database/forms.py	85.89% <0.00%> (+0.18%)	:arrow_up:
... and 66 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[nicolas-gaillard]

hello @eschutho, will this fix be available in 2.1.0? Thank you very much