Skip to content

Handle dropdown queries which are detached from data source#3453

Merged
arikfr merged 7 commits into
masterfrom
handle-dropdown-queries-detached-from-data-source
Apr 1, 2019
Merged

Handle dropdown queries which are detached from data source#3453
arikfr merged 7 commits into
masterfrom
handle-dropdown-queries-detached-from-data-source

Conversation

@rauchy

@rauchy rauchy commented Feb 17, 2019

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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