From dc607b8e8a8563e04cb09fa9b13f4397a486460a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Tue, 2 Jun 2015 10:24:20 +0200 Subject: [PATCH] Dashboard search now supports filtering by multiple dashboard tags, Closes #2095 --- CHANGELOG.md | 1 + pkg/api/search.go | 4 +-- pkg/search/handlers.go | 41 +++++++++++++++++++++---- pkg/search/handlers_test.go | 12 ++++++++ pkg/search/json_index.go | 7 ----- pkg/search/json_index_test.go | 6 ++-- pkg/search/models.go | 3 +- pkg/services/sqlstore/dashboard.go | 7 +---- pkg/services/sqlstore/dashboard_test.go | 12 -------- public/app/controllers/search.js | 15 ++++++--- public/app/partials/search.html | 13 +++++--- 11 files changed, 74 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9096744be61..13b18a51fcb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - [Issue #2088](https://github.com/grafana/grafana/issues/2088). Roles: New user role `Read Only Editor` that replaces the old `Viewer` role behavior **Backend** +- [Issue #2095](https://github.com/grafana/grafana/issues/2095). Search: Search now supports filtering by multiple dashboard tags - [Issue #1905](https://github.com/grafana/grafana/issues/1905). Github OAuth: You can now configure a Github team membership requirement, thx @dewski - [Issue #2052](https://github.com/grafana/grafana/issues/2052). Github OAuth: You can now configure a Github organization requirement, thx @indrekj - [Issue #1891](https://github.com/grafana/grafana/issues/1891). Security: New config option to disable the use of gravatar for profile images diff --git a/pkg/api/search.go b/pkg/api/search.go index 10329f445bd..4c2ba9195b6 100644 --- a/pkg/api/search.go +++ b/pkg/api/search.go @@ -8,7 +8,7 @@ import ( func Search(c *middleware.Context) { query := c.Query("query") - tag := c.Query("tag") + tags := c.QueryStrings("tag") starred := c.Query("starred") limit := c.QueryInt("limit") @@ -18,7 +18,7 @@ func Search(c *middleware.Context) { searchQuery := search.Query{ Title: query, - Tag: tag, + Tags: tags, UserId: c.UserId, Limit: limit, IsStarred: starred == "true", diff --git a/pkg/search/handlers.go b/pkg/search/handlers.go index d0f9ae56833..3b4fcb1c59f 100644 --- a/pkg/search/handlers.go +++ b/pkg/search/handlers.go @@ -33,7 +33,6 @@ func searchHandler(query *Query) error { dashQuery := FindPersistedDashboardsQuery{ Title: query.Title, - Tag: query.Tag, UserId: query.UserId, Limit: query.Limit, IsStarred: query.IsStarred, @@ -55,6 +54,22 @@ func searchHandler(query *Query) error { hits = append(hits, jsonHits...) } + // filter out results with tag filter + if len(query.Tags) > 0 { + filtered := HitList{} + for _, hit := range hits { + if hasRequiredTags(query.Tags, hit.Tags) { + filtered = append(filtered, hit) + } + } + hits = filtered + } + + // sort tags + for _, hit := range hits { + sort.Strings(hit.Tags) + } + // add isStarred info if err := setIsStarredFlagOnSearchResults(query.UserId, hits); err != nil { return err @@ -63,15 +78,29 @@ func searchHandler(query *Query) error { // sort main result array sort.Sort(hits) - // sort tags - for _, hit := range hits { - sort.Strings(hit.Tags) - } - query.Result = hits return nil } +func stringInSlice(a string, list []string) bool { + for _, b := range list { + if b == a { + return true + } + } + return false +} + +func hasRequiredTags(queryTags, hitTags []string) bool { + for _, queryTag := range queryTags { + if !stringInSlice(queryTag, hitTags) { + return false + } + } + + return true +} + func setIsStarredFlagOnSearchResults(userId int64, hits []*Hit) error { query := m.GetUserStarsQuery{UserId: userId} if err := bus.Dispatch(&query); err != nil { diff --git a/pkg/search/handlers_test.go b/pkg/search/handlers_test.go index ebfade4dc06..193ba73f94a 100644 --- a/pkg/search/handlers_test.go +++ b/pkg/search/handlers_test.go @@ -45,5 +45,17 @@ func TestSearch(t *testing.T) { }) }) + Convey("That filters by tag", func() { + query.Tags = []string{"BB", "AA"} + err := searchHandler(&query) + So(err, ShouldBeNil) + + Convey("should return correct results", func() { + So(len(query.Result), ShouldEqual, 2) + So(query.Result[0].Title, ShouldEqual, "BBAA") + So(query.Result[1].Title, ShouldEqual, "CCAA") + }) + + }) }) } diff --git a/pkg/search/json_index.go b/pkg/search/json_index.go index edf791562c9..a0fc02343e2 100644 --- a/pkg/search/json_index.go +++ b/pkg/search/json_index.go @@ -56,13 +56,6 @@ func (index *JsonDashIndex) Search(query *Query) ([]*Hit, error) { break } - // filter out results with tag filter - if query.Tag != "" { - if !strings.Contains(item.TagsCsv, query.Tag) { - continue - } - } - // add results with matchig title filter if strings.Contains(item.TitleLower, query.Title) { results = append(results, &Hit{ diff --git a/pkg/search/json_index_test.go b/pkg/search/json_index_test.go index 52741cc6806..afd584fffbd 100644 --- a/pkg/search/json_index_test.go +++ b/pkg/search/json_index_test.go @@ -17,14 +17,14 @@ func TestJsonDashIndex(t *testing.T) { }) Convey("Should be able to search index", func() { - res, err := index.Search(&Query{Title: "", Tag: "", Limit: 20}) + res, err := index.Search(&Query{Title: "", Limit: 20}) So(err, ShouldBeNil) So(len(res), ShouldEqual, 3) }) Convey("Should be able to search index by title", func() { - res, err := index.Search(&Query{Title: "home", Tag: "", Limit: 20}) + res, err := index.Search(&Query{Title: "home", Limit: 20}) So(err, ShouldBeNil) So(len(res), ShouldEqual, 1) @@ -32,7 +32,7 @@ func TestJsonDashIndex(t *testing.T) { }) Convey("Should not return when starred is filtered", func() { - res, err := index.Search(&Query{Title: "", Tag: "", IsStarred: true}) + res, err := index.Search(&Query{Title: "", IsStarred: true}) So(err, ShouldBeNil) So(len(res), ShouldEqual, 0) diff --git a/pkg/search/models.go b/pkg/search/models.go index 157d9a292e3..e65428f14b0 100644 --- a/pkg/search/models.go +++ b/pkg/search/models.go @@ -26,7 +26,7 @@ func (s HitList) Less(i, j int) bool { return s[i].Title < s[j].Title } type Query struct { Title string - Tag string + Tags []string OrgId int64 UserId int64 Limit int @@ -37,7 +37,6 @@ type Query struct { type FindPersistedDashboardsQuery struct { Title string - Tag string OrgId int64 UserId int64 Limit int diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 027f2cd2fac..8756c25e13e 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -150,13 +150,8 @@ func SearchDashboards(query *search.FindPersistedDashboardsQuery) error { params = append(params, "%"+query.Title+"%") } - if len(query.Tag) > 0 { - sql.WriteString(" AND dashboard_tag.term=?") - params = append(params, query.Tag) - } - if query.Limit == 0 || query.Limit > 10000 { - query.Limit = 300 + query.Limit = 1000 } sql.WriteString(fmt.Sprintf(" ORDER BY dashboard.title ASC LIMIT %d", query.Limit)) diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index 70675f9c42b..0ddd0db0df6 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -99,18 +99,6 @@ func TestDashboardDataAccess(t *testing.T) { So(len(hit.Tags), ShouldEqual, 2) }) - Convey("Should be able to search for dashboards using tags", func() { - query1 := search.FindPersistedDashboardsQuery{Tag: "webapp", OrgId: 1} - query2 := search.FindPersistedDashboardsQuery{Tag: "tagdoesnotexist", OrgId: 1} - - err := SearchDashboards(&query1) - err = SearchDashboards(&query2) - So(err, ShouldBeNil) - - So(len(query1.Result), ShouldEqual, 1) - So(len(query2.Result), ShouldEqual, 0) - }) - Convey("Should not be able to save dashboard with same name", func() { cmd := m.SaveDashboardCommand{ OrgId: 1, diff --git a/public/app/controllers/search.js b/public/app/controllers/search.js index b980297df16..a762af0f887 100644 --- a/public/app/controllers/search.js +++ b/public/app/controllers/search.js @@ -14,7 +14,7 @@ function (angular, _, config) { $scope.giveSearchFocus = 0; $scope.selectedIndex = -1; $scope.results = []; - $scope.query = { query: '', tag: '', starred: false }; + $scope.query = { query: '', tag: [], starred: false }; $scope.currentSearchId = 0; if ($scope.dashboardViewState.fullscreen) { @@ -82,12 +82,11 @@ function (angular, _, config) { $scope.queryHasNoFilters = function() { var query = $scope.query; - return query.query === '' && query.starred === false && query.tag === ''; + return query.query === '' && query.starred === false && query.tag.length === 0; }; $scope.filterByTag = function(tag, evt) { - $scope.query.tag = tag; - $scope.query.tagcloud = false; + $scope.query.tag.push(tag); $scope.search(); $scope.giveSearchFocus = $scope.giveSearchFocus + 1; if (evt) { @@ -96,6 +95,14 @@ function (angular, _, config) { } }; + $scope.removeTag = function(tag, evt) { + $scope.query.tag = _.without($scope.query.tag, tag); + $scope.search(); + $scope.giveSearchFocus = $scope.giveSearchFocus + 1; + evt.stopPropagation(); + evt.preventDefault(); + }; + $scope.getTags = function() { return backendSrv.get('/api/dashboards/tags').then(function(results) { $scope.tagsMode = true; diff --git a/public/app/partials/search.html b/public/app/partials/search.html index 9c644e96d25..9eaa8d253d2 100644 --- a/public/app/partials/search.html +++ b/public/app/partials/search.html @@ -15,11 +15,14 @@ tags - - | - - {{query.tag}} - + + | + + + + {{tagName}} + +