mikebridge opened a new pull request, #45543: URL: https://github.com/apache/superset/pull/45543
### SUMMARY Saving a dashboard that has an archived chart on it fails, and this fixes it. **What users saw.** After a chart on a dashboard was archived, every later save of that dashboard failed with "Dashboard could not be updated." (a 422), even when nothing about the archived chart changed. Opening the dashboard and saving it straight back was enough to trigger it, so archiving any chart on a dashboard blocked further edits to it. **Why.** Saving rebuilds the dashboard's chart list from the layout and compares it with the stored one. The layout still holds the archived chart, as intended (it comes back when the chart is restored), but the stored chart list was loaded with archived charts filtered out. To the save, the archived chart looked newly added, so it tried to insert a `dashboard_slices` row that already exists and failed on the primary key. The existing unit test didn't catch this because its dashboard was built in memory, not loaded through a query, so the filter never applied. @gabotorresruiz found and diagnosed this, with an exact repro, in https://github.com/apache/superset/pull/45482#discussion_r4328666886. Thanks! **What this changes.** `DashboardDAO.set_dash_metadata` reads the stored chart list, archived charts included, directly from `dashboard_slices`, and compares the new list against that. The save then keeps the archived chart's slot and link, so restoring the chart puts it back on the dashboard. If the chart list has unsaved changes that can't be flushed first, the save leaves it alone rather than discard them, and logs a warning. This touches the same function as #45538 (server-side validation of layout chart references), a few lines apart. Whichever merges second needs a small textual merge; there is no behavioural overlap. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A (API behaviour only). ### TESTING INSTRUCTIONS - `pytest tests/integration_tests/dashboards/api_tests.py -k keeps_archived_chart_slot` - `pytest tests/unit_tests/dao/dashboard_test.py -k "soft_deleted_members or install_membership_baseline"` - Manually: put two charts on a dashboard, archive one, then `GET /api/v1/dashboard/<id>` and `PUT` the same layout back. Before this change it returns 422. After, it returns 200, and restoring the archived chart puts it back in its slot. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] 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 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
