SEPURI-SAI-KRISHNA opened a new pull request, #45301:
URL: https://github.com/apache/superset/pull/45301
### SUMMARY
`update_id_refs` remaps old chart IDs to new ones when a dashboard is
imported. Four metadata fields carry chart IDs. `filter_scopes` and
`default_filters` skip an ID that is not in the map, because #17741 added that
check for stale references. `timed_refresh_immune_slices` and `expanded_slices`
index the map directly, so the import fails with "Import dashboard failed for
an unknown reason" in three cases:
| metadata | result on master |
| --- | --- |
| `timed_refresh_immune_slices: [999]`, chart 999 not in the bundle |
`KeyError: 999` |
| `timed_refresh_immune_slices: ["101"]`, chart 101 **in** the bundle |
`KeyError: '101'` |
| `expanded_slices: {"abc": true}` | `ValueError: invalid literal for int()`
|
The second row is a live reference, not a stale one. These fields can hold
string-typed IDs: `superset/commands/dashboard/export.py` coerces them with
`int()` on the way out, and the MCP `remove_chart_from_dashboard` tool has a
test named "timed_refresh_immune_slices entries may be string-typed IDs". The
import side did no coercion, so a string entry missed the integer-keyed map.
Unresolvable IDs are routine. Nothing prunes these two fields when a chart
is deleted or leaves a dashboard: `DeleteChartCommand` does not touch dashboard
metadata, and the only pruning code in the tree is the MCP tool above. An
incomplete map is routine too: a chart in the layout is skipped at import when
its dataset is not resolvable in the bundle, `/api/v1/assets/import/` lets a
dashboard arrive without every chart yaml it references, and bundles exported
before #40588 were never pruned on the way out.
This adds `_remap_legacy_chart_id`, which coerces like the exporter does and
returns `None` when the ID cannot be resolved, so a live string reference
remaps and an unresolvable one is dropped instead of failing the import.
`filter_scopes` and `default_filters` keep their existing inline checks; they
already behave correctly for these inputs.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable.
### TESTING INSTRUCTIONS
Three tests next to #17741's own test in
`tests/unit_tests/dashboards/commands/importers/v1/utils_test.py`, one per row
of the table above. All three fail on master.
```
pytest tests/unit_tests/dashboards/commands/importers/v1/utils_test.py
```
Manually: export a dashboard that has a refresh-immune or expanded chart,
delete that chart's dataset yaml from the bundle, and import it. On master the
import fails with "Import dashboard failed for an unknown reason". With this
change the dashboard imports and the dangling reference is gone.
### ADDITIONAL INFORMATION
- [x] Has associated issue: #45300
- [ ] 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]
