diff --git a/client/app/components/QueryBasedParameterInput.jsx b/client/app/components/QueryBasedParameterInput.jsx index 8c82bda54e..b0743d6198 100644 --- a/client/app/components/QueryBasedParameterInput.jsx +++ b/client/app/components/QueryBasedParameterInput.jsx @@ -7,43 +7,6 @@ import { Query } from '@/services/query'; const { Option } = Select; -function optionsFromQueryResult(queryResult) { - const columns = queryResult.data.columns; - const numColumns = columns.length; - let options = []; - // If there are multiple columns, check if there is a column - // named 'name' and column named 'value'. If name column is present - // in results, use name from name column. Similar for value column. - // Default: Use first string column for name and value. - if (numColumns > 0) { - let nameColumn = null; - let valueColumn = null; - columns.forEach((column) => { - const columnName = column.name.toLowerCase(); - if (columnName === 'name') { - nameColumn = column.name; - } - if (columnName === 'value') { - valueColumn = column.name; - } - // Assign first string column as name and value column. - if (nameColumn === null) { - nameColumn = column.name; - } - if (valueColumn === null) { - valueColumn = column.name; - } - }); - if (nameColumn !== null && valueColumn !== null) { - options = queryResult.data.rows.map(row => ({ - name: row[nameColumn], - value: row[valueColumn], - })); - } - } - return options; -} - export class QueryBasedParameterInput extends React.Component { static propTypes = { value: PropTypes.any, // eslint-disable-line react/forbid-prop-types @@ -81,9 +44,8 @@ export class QueryBasedParameterInput extends React.Component { _loadOptions(queryId) { if (queryId && (queryId !== this.state.queryId)) { this.setState({ loading: true }); - Query.resultById({ id: queryId }, (result) => { + Query.dropdownOptions({ id: queryId }, (options) => { if (this.props.queryId === queryId) { - const options = optionsFromQueryResult(result.query_result); this.setState({ options, loading: false }); const found = find(options, option => option.value === this.props.value) !== undefined; diff --git a/client/app/services/query.js b/client/app/services/query.js index 2d152a2896..003ea64e78 100644 --- a/client/app/services/query.js +++ b/client/app/services/query.js @@ -366,6 +366,11 @@ function QueryResource( isArray: false, url: 'api/queries/:id/results.json', }, + dropdownOptions: { + method: 'get', + isArray: true, + url: 'api/queries/:id/dropdown', + }, favorites: { method: 'get', isArray: false, diff --git a/redash/handlers/api.py b/redash/handlers/api.py index 1fef2f5409..46916af663 100644 --- a/redash/handlers/api.py +++ b/redash/handlers/api.py @@ -10,7 +10,7 @@ from redash.handlers.data_sources import DataSourceTypeListResource, DataSourceListResource, DataSourceSchemaResource, DataSourceResource, DataSourcePauseResource, DataSourceTestResource from redash.handlers.events import EventsResource from redash.handlers.queries import QueryArchiveResource, QueryForkResource, QueryRefreshResource, QueryListResource, QueryRecentResource, QuerySearchResource, QueryResource, MyQueriesResource -from redash.handlers.query_results import QueryResultListResource, QueryResultResource, JobResource +from redash.handlers.query_results import QueryResultListResource, QueryResultDropdownResource, QueryResultResource, JobResource from redash.handlers.users import UserResource, UserListResource, UserInviteResource, UserResetPasswordResource, UserDisableResource, UserRegenerateApiKeyResource from redash.handlers.visualizations import VisualizationListResource from redash.handlers.visualizations import VisualizationResource @@ -69,10 +69,10 @@ def json_representation(data, code, headers=None): api.add_org_resource(EventsResource, '/api/events', endpoint='events') -api.add_org_resource(QueryFavoriteListResource, '/api/queries/favorites', endpoint='query_fovorites') -api.add_org_resource(QueryFavoriteResource, '/api/queries//favorite', endpoint='query_fovorite') -api.add_org_resource(DashboardFavoriteListResource, '/api/dashboards/favorites', endpoint='dashboard_fovorites') -api.add_org_resource(DashboardFavoriteResource, '/api/dashboards//favorite', endpoint='dashboard_fovorite') +api.add_org_resource(QueryFavoriteListResource, '/api/queries/favorites', endpoint='query_favorites') +api.add_org_resource(QueryFavoriteResource, '/api/queries//favorite', endpoint='query_favorite') +api.add_org_resource(DashboardFavoriteListResource, '/api/dashboards/favorites', endpoint='dashboard_favorites') +api.add_org_resource(DashboardFavoriteResource, '/api/dashboards//favorite', endpoint='dashboard_favorite') api.add_org_resource(QueryTagsResource, '/api/queries/tags', endpoint='query_tags') api.add_org_resource(DashboardTagsResource, '/api/dashboards/tags', endpoint='dashboard_tags') @@ -90,6 +90,7 @@ def json_representation(data, code, headers=None): api.add_org_resource(CheckPermissionResource, '/api///acl/', endpoint='check_permissions') api.add_org_resource(QueryResultListResource, '/api/query_results', endpoint='query_results') +api.add_org_resource(QueryResultDropdownResource, '/api/queries//dropdown', endpoint='query_result_dropdown') api.add_org_resource(QueryResultResource, '/api/query_results/.', '/api/query_results/', diff --git a/redash/handlers/query_results.py b/redash/handlers/query_results.py index 45be31301f..b51220b9e2 100644 --- a/redash/handlers/query_results.py +++ b/redash/handlers/query_results.py @@ -10,8 +10,8 @@ require_permission, view_only) from redash.tasks import QueryTask from redash.tasks.queries import enqueue_query -from redash.utils import (collect_parameters_from_request, gen_query_hash, json_dumps, json_loads, utcnow) -from redash.utils.parameterized_query import ParameterizedQuery +from redash.utils import (collect_parameters_from_request, gen_query_hash, json_dumps, utcnow) +from redash.utils.parameterized_query import ParameterizedQuery, dropdown_values def error_response(message): @@ -131,6 +131,11 @@ def post(self): ONE_YEAR = 60 * 60 * 24 * 365.25 +class QueryResultDropdownResource(BaseResource): + def get(self, query_id): + return dropdown_values(query_id) + + class QueryResultResource(BaseResource): @staticmethod def add_cors_headers(headers): @@ -188,8 +193,7 @@ def post(self, query_id): max_age = int(params.get('max_age', 0)) query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org) - parameter_schema = map(self._convert_queries_to_enums, - query.options.get("parameters", [])) + parameter_schema = query.options.get("parameters", []) if not has_access(query.data_source.groups, self.current_user, not_view_only): return {'job': {'status': 4, 'error': 'You do not have permission to run queries with this data source.'}}, 403 diff --git a/redash/utils/parameterized_query.py b/redash/utils/parameterized_query.py index c1ec2d38bd..ee2f645cda 100644 --- a/redash/utils/parameterized_query.py +++ b/redash/utils/parameterized_query.py @@ -1,9 +1,38 @@ import pystache +from functools import partial +from flask_login import current_user +from redash.authentication.org_resolving import current_org from numbers import Number -from redash.utils import mustache_render +from redash import models +from redash.utils import mustache_render, json_loads +from redash.permissions import require_access, view_only from funcy import distinct from dateutil.parser import parse + +def _pluck_name_and_value(default_column, row): + row = {k.lower(): v for k, v in row.items()} + name_column = "name" if "name" in row.keys() else default_column + value_column = "value" if "value" in row.keys() else default_column + + return {"name": row[name_column], "value": row[value_column]} + + +def _load_result(query_id): + query = models.Query.get_by_id_and_org(query_id, current_org) + 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) + + return json_loads(query_result.data) + + +def dropdown_values(query_id): + data = _load_result(query_id) + first_column = data["columns"][0]["name"] + pluck = partial(_pluck_name_and_value, first_column) + return map(pluck, data["rows"]) + + def _collect_key_names(nodes): keys = [] for node in nodes._parse_tree: @@ -76,6 +105,7 @@ def _valid(self, name, value): "text": lambda value: isinstance(value, basestring), "number": lambda value: isinstance(value, Number), "enum": lambda value: value in definition["enumOptions"], + "query": lambda value: value in [v["value"] for v in dropdown_values(definition["queryId"])], "date": _is_date, "datetime-local": _is_date, "datetime-with-seconds": _is_date, diff --git a/tests/handlers/test_query_results.py b/tests/handlers/test_query_results.py index 8a65233ef6..2a06ae158c 100644 --- a/tests/handlers/test_query_results.py +++ b/tests/handlers/test_query_results.py @@ -170,6 +170,16 @@ def test_signed_in_user_and_different_query_result(self): self.assertEquals(rv.status_code, 403) +class TestQueryResultDropdownResource(BaseTestCase): + def test_checks_for_access_to_the_query(self): + ds2 = self.factory.create_data_source(group=self.factory.org.admin_group, view_only=False) + query = self.factory.create_query(data_source=ds2) + + rv = self.make_request('get', '/api/queries/{}/dropdown'.format(query.id)) + + self.assertEquals(rv.status_code, 403) + + class TestQueryResultExcelResponse(BaseTestCase): def test_renders_excel_file(self): query = self.factory.create_query() diff --git a/tests/utils/test_parameterized_query.py b/tests/utils/test_parameterized_query.py index c63acc96d4..6828bc91c4 100644 --- a/tests/utils/test_parameterized_query.py +++ b/tests/utils/test_parameterized_query.py @@ -1,7 +1,9 @@ from unittest import TestCase +from mock import patch +from collections import namedtuple import pytest -from redash.utils.parameterized_query import ParameterizedQuery, InvalidParameterError +from redash.utils.parameterized_query import ParameterizedQuery, InvalidParameterError, dropdown_values class TestParameterizedQuery(TestCase): @@ -110,6 +112,31 @@ def test_validates_enum_parameters(self): self.assertEquals("foo baz", query.text) + @patch('redash.utils.parameterized_query.dropdown_values') + def test_raises_on_invalid_query_parameters(self, _): + schema = [{"name": "bar", "type": "query", "queryId": 1}] + query = ParameterizedQuery("foo", schema) + + with pytest.raises(InvalidParameterError): + query.apply({"bar": 7}) + + @patch('redash.utils.parameterized_query.dropdown_values', return_value=[{"value": "baz"}]) + def test_raises_on_unlisted_query_value_parameters(self, _): + schema = [{"name": "bar", "type": "query", "queryId": 1}] + query = ParameterizedQuery("foo", schema) + + with pytest.raises(InvalidParameterError): + query.apply({"bar": "shlomo"}) + + @patch('redash.utils.parameterized_query.dropdown_values', return_value=[{"value": "baz"}]) + def test_validates_query_parameters(self, _): + schema = [{"name": "bar", "type": "query", "queryId": 1}] + query = ParameterizedQuery("foo {{bar}}", schema) + + query.apply({"bar": "baz"}) + + self.assertEquals("foo baz", query.text) + def test_raises_on_invalid_date_range_parameters(self): schema = [{"name": "bar", "type": "date-range"}] query = ParameterizedQuery("foo", schema) @@ -131,3 +158,17 @@ def test_raises_on_unexpected_param_types(self): with pytest.raises(InvalidParameterError): query.apply({"bar": "baz"}) + + @patch('redash.utils.parameterized_query._load_result', return_value={ + "columns": [{"name": "id"}, {"name": "Name"}, {"name": "Value"}], + "rows": [{"id": 5, "Name": "John", "Value": "John Doe"}]}) + def test_dropdown_values_prefers_name_and_value_columns(self, _): + values = dropdown_values(1) + self.assertEquals(values, [{"name": "John", "value": "John Doe"}]) + + @patch('redash.utils.parameterized_query._load_result', return_value={ + "columns": [{"name": "id"}, {"name": "fish"}, {"name": "poultry"}], + "rows": [{"fish": "Clown", "id": 5, "poultry": "Hen"}]}) + def test_dropdown_values_compromises_for_first_column(self, _): + values = dropdown_values(1) + self.assertEquals(values, [{"name": 5, "value": 5}])