Skip to content
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.

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

[listview] Please display message when chart cannot be open because of missing dataset #12464

Closed
adam-stasiak opened this issue Jan 12, 2021 · 16 comments · Fixed by #12705
Closed
Labels
listview Namespace | Anything related to lists, such as Dashboards, Charts, Datasets, etc.

Comments

@adam-stasiak
Copy link
Contributor

Case is that when we delete dataset used by chart and then try to visit this chart - > we are sticked in listing view. Click on chart should redirect us to explore but does not.

Expected results

I should see chart open with information saying about missingdataset.

Actual results

Listing is refreshed

Screenshots

Nagranie.z.ekranu.2021-01-12.o.18.40.06.mov

How to reproduce the bug

  1. Go to charts listing
  2. Note name of dataset to remove
  3. Go to dataset and remove this dataset
  4. Go to chart listing and tap on chart name with empty dataset

Environment

docker
90915db

Checklist

Make sure to follow these steps before submitting your issue - thank you!

  • [ x] I have checked the superset logs for python stacktraces and included it here as text if there are any.
  • [ x] I have reproduced the issue with at least the latest released version of superset.
  • [ x] I have checked the issue tracker for the same issue and I haven't found one similar.

Additional context

@adam-stasiak adam-stasiak added the #bug Bug report label Jan 12, 2021
@junlincc junlincc removed #bug Bug report Polidea labels Jan 12, 2021
@junlincc junlincc changed the title [explore] chart cannot be open when dataset was removed [listview] chart cannot be open when dataset was removed Jan 12, 2021
@junlincc junlincc added P1 Priority item - Major listview Namespace | Anything related to lists, such as Dashboards, Charts, Datasets, etc. labels Jan 12, 2021
@junlincc
Copy link
Member

junlincc commented Jan 12, 2021

@nytai just want to bring this to your attention.

we will take this on. @kkucharc Thanks so much!

@adam-stasiak many thanks for finding this issue 🙏

@nytai
Copy link
Member

nytai commented Jan 12, 2021

hmm, what should happen in this case? The chart would no longer be useable if the underlying dataset is missing.

@kkucharc
Copy link
Contributor

@nytai Well when I refresh chart when dataset was already deleted, I had NotFound screen:
Screenshot 2021-01-12 at 19 33 41
Maybe we should update state and not show chart on the lists anymore. Or we can have some error page.

@junlincc
Copy link
Member

junlincc commented Jan 12, 2021

hmm, what should happen in this case? The chart would no longer be useable if the underlying dataset is missing.

if the chart is no longer useable, should we still display it on the list view, or at least notify user the chart is not available because the underlying data has been deleted? i would mistaken it as a bug in list view.

@junlincc
Copy link
Member

junlincc commented Jan 12, 2021

Maybe we should update state and not show chart on the lists anymore. Or we can have some error page.

exactly

@junlincc junlincc removed the P1 Priority item - Major label Jan 12, 2021
@nytai
Copy link
Member

nytai commented Jan 12, 2021

Well in that case should deleting a dataset cause all associated charts to be deleted? I know it's showing up as a UI bug but this is more of an api/data issue

@adam-stasiak adam-stasiak changed the title [listview] chart cannot be open when dataset was removed [listview] Please display message when chart cannot be open because of missing dataset Jan 12, 2021
@adam-stasiak
Copy link
Contributor Author

Toast message for this would be ok @kkucharc

@junlincc
Copy link
Member

We are putting a quick bandaid on this issue by adding a lightweight msg for now. I don't think we should hide or delete charts for the users at all.

cc @Steejay

@nytai
Copy link
Member

nytai commented Jan 12, 2021

I'm pretty sure this has always been an issue in some form. Deleting a dataset that's connected to a lot of charts affects those charts in weird ways. We've talked about not even allowing a user to delete a dataset until they've deleted all associated charts and dashboards.

Popping a toast would be nice, but I'm pretty sure those are just regular html links for the chart. The exception is likely on the backend as @kkucharc pointed out.

@mistercrunch or @zuzana-vej any thoughts on what should be correct behavior for this issue?

@junlincc
Copy link
Member

i understand there's a bigger conversation behind, that's why we are not suggesting any significant changes. simply adding a notification will do more benefit than harm at this point.

@kkucharc
Copy link
Contributor

I added notification message.

If I may have suggestion for the future discussions:

  1. It would be good to introduce some error pages -> If someone would like share a chart's link and the chart is deleted in meantime, it would be great to show the user some user-friendly page.
  2. Maybe it's not bad idea to give possibility to user to delete a datasource but also give some prompt "are you sure?" maybe even with the list of charts it will affect.

@ktmud
Copy link
Member

ktmud commented Jan 12, 2021

Longer term solution should be allowing Explore page to open without a datasource.

@nytai
Copy link
Member

nytai commented Jan 12, 2021

@kkucharc 2 is what happens now (at least from the list view).

Screen Shot 2021-01-12 at 12 00 46 PM

Longer term solution should be allowing Explore page to open without a datasource.

Yes, ideally the user could go in an fix the chart

@dpgaspar
Copy link
Member

dpgaspar commented Jan 12, 2021

I vote for restricting it at the API level. In that case it would be nice for the delete prompt to change

cc: @junlincc , @nytai , @adam-stasiak

@nytai
Copy link
Member

nytai commented Jan 13, 2021

@dpgaspar you mean just not allowing users to delete datasets that are connected to charts/dashboards? I think we've talked about this before. We should change the prompt, maybe provide links to the objects that need to be deleted first.

@Steejay
Copy link

Steejay commented Jan 14, 2021

I think preserving the chart would be ideal. Even though deleting the datasource will break the associated charts, i dont think we want charts to suddenly disappear for the user.

Flow: Open chart > error message displayed in Explore that the associated dataset has been deleted. From here a user can decide to delete the chart themselves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
listview Namespace | Anything related to lists, such as Dashboards, Charts, Datasets, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants