[sqllab] Fixed js error when results are not available

[vera-liu]

Issue:

• [sql-lab] js bug when clicking visualize the data from this query #1646
• https://github.com/airbnb/superset/issues/1463

Problem:

• query results could be null in VisualizeModal, triggering js error
• defaults were not shown in VisualizeModal for past queries

Solution:

• add modal message when query results are not available
• we flush results of latestQuery everytime we start a new query https://github.com/airbnb/superset/blob/94dde075b3eab41797725e1e02c7f87b6b45471a/superset/assets/javascripts/SqlLab/reducers.js#L138. I guess the original incentive was flushing query results in case we have too many data in the store. I changed this to only flushing results.data and results.query, leaving results.columns in store, to be used by VisualizeModal
• move setState from componentWillMount to componentWillReceiveProps, so that column attributes could be passed in

Before:

After:

@ascott @bkyryliuk @mistercrunch

[bkyryliuk]

s/No results available for this query, please rerun

[ascott]

it would be nice to have actionable feedback for the user here, but if there are no results for a query, re-running it won't likely return new results. i think we could save figuring out what actionable feedback we could show here for another PR.

[ascott]

ah, i see what's happening now, i missed reading about how we flush the query results except for the latest query.

[bkyryliuk]

would be great to have the unit test for it, optional

[ascott]

thanks for taking this on @vera-liu. i'm not sure showing this modal is the best solution. like bogdan suggested, we could add text to tell the user to re-run the query but i don't think this is a very good user experience.

comments on your other ideas:

1. remove visualize button for all queries in query history except for latest query (users should run past queries in new editors to visualize them)

we could remove the visualize button from the query history list items altogether. if it's only present for the latest query, that seems confusing/looks like a bug if it's not displayed for the other queries. for the latest query, users will be able to use the visualize button in the results tab.

2. rerun query when visualize modal is triggered (this might not work with large queries and async ones

this could be a good solution but would definitely be more time consuming to get it right. i think in this case we would want to show an icon/tooltip that says re-run query to visualize. once clicked we would switch back to the results tab, update the query in the editor, and automatically start running the query. we would want to do this so the user can see that the query is running. however we also have an icon/tooltip that says overwrite text in editor with this query which is essentially the same thing, minus auto-running the query.

i think the best solution here, balancing impact and effort, would be to remove the visualize icon from the query history list, and only have access to it from the results tab.

[vera-liu]

Done:

• After discussing, I decided to leave columns in results when flushing data out (only results.columns are used by VisualizeModal, and they are pretty lightweight, should be ok to keep them in store while flushing results.data)
• I also move setState from componentWillMount to componentWillReceiveProps in VisualizeModal, as I found that componentWillMount is not called when switching from one query's modal to another, causing issue https://github.com/airbnb/superset/issues/1463

[bkyryliuk]

@vera-liu - LGTM, please test on staging