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

Handle dropdown queries which are detached from data source #3453

Merged
merged 7 commits into from
Apr 1, 2019

Conversation

rauchy
Copy link
Contributor

@rauchy rauchy commented Feb 17, 2019

This PR handles an edge case where dropdown queries point to a data source that no longer exists.

@rauchy rauchy requested a review from arikfr February 17, 2019 18:21
@ghost ghost assigned rauchy Feb 17, 2019
@ghost ghost added the in progress label Feb 17, 2019
return json_loads(query_result.data)
if query.data_source:
require_access(query.data_source.groups, current_user, view_only)
query_result = models.QueryResult.get_by_id_and_org(query.latest_query_data_id, current_org)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming the tests fail because of the cyclic import issue. Maybe use query.org here instead of current_org?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually that import was already handled elsewhere. Fixed.

@arikfr arikfr merged commit 1333aae into master Apr 1, 2019
@arikfr arikfr deleted the handle-dropdown-queries-detached-from-data-source branch April 1, 2019 08:19
@arikfr arikfr removed the in progress label Apr 1, 2019
harveyrendell pushed a commit to pushpay/redash that referenced this pull request Nov 14, 2019
…h#3453)

* handle an edge case where dropdown queries are connected to data sources
that no longer exist

* Rethinking it, an empty result set makes no sense and it's better to
throw an error

* remove redundant import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants