From 2eefdc1da367b055a50d99cfc5d4582fda3f5280 Mon Sep 17 00:00:00 2001 From: Arik Fraimovich Date: Tue, 26 Jun 2018 21:50:16 +0300 Subject: [PATCH] Updated queries screen (& unified API). --- client/app/components/app-header/index.js | 7 +- client/app/pages/queries-list/index.js | 104 ++++-------------- .../app/pages/queries-list/queries-list.html | 7 +- client/app/services/query.js | 7 +- redash/handlers/queries.py | 8 +- tests/handlers/test_queries.py | 28 +++++ 6 files changed, 61 insertions(+), 100 deletions(-) diff --git a/client/app/components/app-header/index.js b/client/app/components/app-header/index.js index 5ed5f0d6f..26bb18660 100644 --- a/client/app/components/app-header/index.js +++ b/client/app/components/app-header/index.js @@ -6,10 +6,7 @@ import './app-header.css'; const logger = debug('redash:appHeader'); -function controller( - $rootScope, $location, $uibModal, Auth, currentUser, - clientConfig, Dashboard, Query, -) { +function controller($rootScope, $location, $uibModal, Auth, currentUser, clientConfig, Dashboard, Query) { this.logoUrl = logoUrl; this.basePath = clientConfig.basePath; this.currentUser = currentUser; @@ -39,7 +36,7 @@ function controller( }; this.searchQueries = () => { - $location.path('/queries/search').search({ q: this.term }); + $location.path('/queries').search({ q: this.term }); }; this.logout = () => { diff --git a/client/app/pages/queries-list/index.js b/client/app/pages/queries-list/index.js index 5246de54e..58ec8cdfe 100644 --- a/client/app/pages/queries-list/index.js +++ b/client/app/pages/queries-list/index.js @@ -1,7 +1,7 @@ import moment from 'moment'; import _ from 'underscore'; -import { Paginator, LivePaginator } from '@/lib/pagination'; +import { LivePaginator } from '@/lib/pagination'; import template from './queries-list.html'; class QueriesListCtrl { @@ -26,33 +26,28 @@ class QueriesListCtrl { this.showEmptyState = false; this.loaded = false; - const self = this; - this.selectedTags = new Set(); this.onTagsUpdate = (tags) => { this.selectedTags = tags; this.update(); }; - function queriesFetcher(requestedPage, itemsPerPage, paginator) { + const queriesFetcher = (requestedPage, itemsPerPage, paginator) => { $location.search('page', requestedPage); - const request = Object.assign({}, self.defaultOptions, { + const request = Object.assign({}, this.defaultOptions, { page: requestedPage, page_size: itemsPerPage, - tags: [...self.selectedTags], // convert Set to Array + tags: [...this.selectedTags], // convert Set to Array }); - if (_.isString(self.term) && self.term !== '') { - request.q = self.term; - request.include_drafts = true; - $location.path('queries/search').search('q', self.term); - } else if (self.currentPage === 'search') { - $location.search('q', self.term); + if (_.isString(this.term) && this.term !== '') { + request.q = this.term; + $location.search('q', this.term); } - return self.resource(request).$promise.then((data) => { - self.loaded = true; + return this.resource(request).$promise.then((data) => { + this.loaded = true; const rows = data.results.map((query) => { query.created_at = moment(query.created_at); query.retrieved_at = moment(query.retrieved_at); @@ -61,9 +56,9 @@ class QueriesListCtrl { paginator.updateRows(rows, data.count); - self.showEmptyState = data.count === 0; + this.showEmptyState = data.count === 0; }); - } + }; this.navigateTo = ($event, url) => { if ($event.altKey || $event.ctrlKey || $event.metaKey || $event.shiftKey) { @@ -74,23 +69,12 @@ class QueriesListCtrl { $location.url(url); }; - if (['favorites', 'search'].indexOf(this.currentPage) >= 0) { - this.paginator = new Paginator([], { page }); + this.paginator = new LivePaginator(queriesFetcher, { page }); - this.update = () => { - this.paginator.setPage(1); - queriesFetcher(this.paginator.page, this.paginator.itemsPerPage, this.paginator); - }; - - this.update(); - } else { - this.paginator = new LivePaginator(queriesFetcher, { page }); - - this.update = () => { - // `queriesFetcher` will be called by paginator - this.paginator.setPage(1); - }; - } + this.update = () => { + // `queriesFetcher` will be called by paginator + this.paginator.setPage(1); + }; } } @@ -139,64 +123,16 @@ export default function init(ngModule) { title: 'Favorite Queries', resolve: { currentPage: () => 'favorites', - resource: (Query, $q) => { + resource: (Query) => { 'ngInject'; - return (request) => { - const result = { - results: [], - }; - result.$promise = $q((resolve, reject) => { - // convert plain array to paginator - Query.favorites(request) - .$promise.then((data) => { - result.count = data.length; - result.results = data; - result.page = 1; - result.page_size = data.length; - - resolve(result); - }) - .catch(reject); - }); - return result; - }; - }, - }, - }, - route, - ), - '/queries/search': _.extend( - { - title: 'Queries Search', - resolve: { - currentPage: () => 'search', - resource: (Query, $q) => { - 'ngInject'; - - return (request) => { - const result = { - results: [], - }; - result.$promise = $q((resolve, reject) => { - // convert plain array to paginator - Query.search(request) - .$promise.then((data) => { - result.count = data.length; - result.results = data; - result.page = 1; - result.page_size = data.length; - - resolve(result); - }) - .catch(reject); - }); - return result; - }; + return Query.favorites.bind(Query); }, }, }, route, ), + // TODO: setup redirect? + // '/queries/search': _.extend( }; } diff --git a/client/app/pages/queries-list/queries-list.html b/client/app/pages/queries-list/queries-list.html index 5aefd1ddd..b72f58bf8 100644 --- a/client/app/pages/queries-list/queries-list.html +++ b/client/app/pages/queries-list/queries-list.html @@ -19,7 +19,7 @@ -
+
@@ -109,12 +109,11 @@ /> My Queries
-
+
-
- \ No newline at end of file + diff --git a/client/app/services/query.js b/client/app/services/query.js index 0c848f607..aca34a5a3 100644 --- a/client/app/services/query.js +++ b/client/app/services/query.js @@ -168,11 +168,6 @@ function QueryResource($resource, $http, $q, $location, currentUser, QueryResult 'api/queries/:id', { id: '@id' }, { - search: { - method: 'get', - isArray: true, - url: 'api/queries/search', - }, recent: { method: 'get', isArray: true, @@ -199,7 +194,7 @@ function QueryResource($resource, $http, $q, $location, currentUser, QueryResult }, favorites: { method: 'get', - isArray: true, + isArray: false, url: 'api/queries/favorites', }, favorite: { diff --git a/redash/handlers/queries.py b/redash/handlers/queries.py index e5b37b752..aa32a44d8 100644 --- a/redash/handlers/queries.py +++ b/redash/handlers/queries.py @@ -139,7 +139,13 @@ class QueryListResource(BaseResource): Responds with an array of :ref:`query ` objects. """ - results = models.Query.all_queries(self.current_user.group_ids, self.current_user.id) + search_term = request.args.get('q') + + if search_term: + results = models.Query.search(search_term, self.current_user.group_ids, include_drafts=True, limit=None) + else: + results = models.Query.all_queries(self.current_user.group_ids, self.current_user.id) + results = filter_by_tags(results, models.Query.tags) page = request.args.get('page', 1, type=int) diff --git a/tests/handlers/test_queries.py b/tests/handlers/test_queries.py index deb95d9bd..336b8de9d 100644 --- a/tests/handlers/test_queries.py +++ b/tests/handlers/test_queries.py @@ -105,6 +105,34 @@ class TestQueryResourcePost(BaseTestCase): self.assertEqual(rv.json['name'], 'Testing') self.assertEqual(rv.json['last_modified_by']['id'], user.id) +class TestQueryListResourceGet(BaseTestCase): + def test_returns_queries(self): + q1 = self.factory.create_query() + q2 = self.factory.create_query() + q3 = self.factory.create_query() + + rv = self.make_request('get', '/api/queries') + + assert len(rv.json['results']) == 3 + assert set(map(lambda d: d['id'], rv.json['results'])) == set([q1.id, q2.id, q3.id]) + + def test_filters_with_tags(self): + q1 = self.factory.create_query(tags=[u'test']) + q2 = self.factory.create_query() + q3 = self.factory.create_query() + + rv = self.make_request('get', '/api/queries?tags=test') + assert len(rv.json['results']) == 1 + assert set(map(lambda d: d['id'], rv.json['results'])) == set([q1.id]) + + def test_search_term(self): + q1 = self.factory.create_query(name="Sales") + q2 = self.factory.create_query(name="Q1 sales") + q3 = self.factory.create_query(name="Ops") + + rv = self.make_request('get', '/api/queries?q=sales') + assert len(rv.json['results']) == 2 + assert set(map(lambda d: d['id'], rv.json['results'])) == set([q1.id, q2.id]) class TestQueryListResourcePost(BaseTestCase): def test_create_query(self):