apache / superset

Apache Superset is a Data Visualization and Data Exploration Platform
https://superset.apache.org/
Apache License 2.0
61.98k stars 13.59k forks source link

feat(datasets): Allow swap dataset after deletion #30364

Closed Antonio-RiveroMartnez closed 2 days ago

Antonio-RiveroMartnez commented 4 days ago

SUMMARY

Currently, if you delete a dataset, the charts that were using it don't keep the controls config when swapping to a dataset that have similar structure. This is because the state gets overwritten with form data that doesn't have the controls configs so the sync method is not able to fetch the correct values for them.

This PR keeps the values and other configs when the API response doesn't include dataset id (deleted) and fallback to read from the datasource when getting the controls values in case there's nothing in the state.

One thing to notice is that before, when you first loaded a chart that have no dataset because it was deleted, the explore data was a fallback and didn't trigger the API requests, now, because we're not using this fallback explore but keeping the form data info from the API response, the UI will show a Not Found error until you swap the dataset and update the chart.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

https://github.com/user-attachments/assets/504820d1-de8b-469c-a471-31867649dacd

Now:

https://github.com/user-attachments/assets/f5495027-5270-42d4-9a9e-25fb9cbbe3a2

TESTING INSTRUCTIONS

Create two datasets with the same schema (columns and metrics). Create a chart using one of these and save it. Delete this dataset. Access the chart -- you'll see an option to swap the dataset. Remap the chart to use the other dataset. Your controls config must be kept

ADDITIONAL INFORMATION

Antonio-RiveroMartnez commented 4 days ago

/testenv up

github-actions[bot] commented 4 days ago

@Antonio-RiveroMartnez Ephemeral environment spinning up at http://35.162.103.7:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

Antonio-RiveroMartnez commented 3 days ago

/testenv up

github-actions[bot] commented 3 days ago

@Antonio-RiveroMartnez Ephemeral environment spinning up at http://54.218.73.54:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

sfirke commented 3 days ago

Wow, that is a nice quality of life improvement for users! Huge potential time saver.

rusackas commented 3 days ago

I think this PR is a fantastic step in the right direction ❤️

If there's interest/bandwidth to take things a step further, it's worth noting that deleting a dataset might break HUNDREDS of charts. While this PR allows you to restore them one at a time, it might be a great idea to (also) allow this swap/substitution in the Dataset deletion modal. You could give the user any/all of these choices:

Of course, taking it even further over the horizon, we probably shouldn't be deleting anything but rather doing "soft deletes" where rows are tagged as deleted, so that they can be restored by a DBA. (Note, this is not the same as "Archiving" which requires UI changes and un-archiving capabilities.

Love the PR, but just saying this is the entry of a longer tunnel ;)

github-actions[bot] commented 2 days ago

Ephemeral environment shutdown and build artifacts deleted.