From 078d08d036a147e2870c168012cd04d8eb1aeff9 Mon Sep 17 00:00:00 2001 From: Emil Tullstedt Date: Wed, 6 May 2020 11:42:52 +0200 Subject: [PATCH] Search: Support multiple order filters (#24230) --- pkg/services/search/service.go | 7 +++-- pkg/services/search/sorting.go | 14 +++++++-- pkg/services/sqlstore/dashboard.go | 7 ++--- pkg/services/sqlstore/dashboard_test.go | 31 ++++++++++++++++++++ pkg/services/sqlstore/searchstore/builder.go | 17 ++++++----- 5 files changed, 57 insertions(+), 19 deletions(-) diff --git a/pkg/services/search/service.go b/pkg/services/search/service.go index b550150bc3c..2db02143710 100644 --- a/pkg/services/search/service.go +++ b/pkg/services/search/service.go @@ -3,7 +3,6 @@ package search import ( "sort" - "github.com/grafana/grafana/pkg/services/sqlstore/searchstore" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/bus" @@ -49,7 +48,7 @@ type FindPersistedDashboardsQuery struct { Page int64 Permission models.PermissionType - SortBy searchstore.FilterOrderBy + Filters []interface{} Result HitList } @@ -86,7 +85,9 @@ func (s *SearchService) searchHandler(query *Query) error { } if sortOpt, exists := s.sortOptions[query.Sort]; exists { - dashboardQuery.SortBy = sortOpt.Filter + for _, filter := range sortOpt.Filter { + dashboardQuery.Filters = append(dashboardQuery.Filters, filter) + } } if err := bus.Dispatch(&dashboardQuery); err != nil { diff --git a/pkg/services/search/sorting.go b/pkg/services/search/sorting.go index 7d8e193b730..e67d98af02d 100644 --- a/pkg/services/search/sorting.go +++ b/pkg/services/search/sorting.go @@ -11,13 +11,17 @@ var ( Name: "alpha-asc", DisplayName: "Alphabetically (A-Z)", Description: "Sort results in an alphabetically ascending order", - Filter: searchstore.TitleSorter{}, + Filter: []SortOptionFilter{ + searchstore.TitleSorter{}, + }, } sortAlphaDesc = SortOption{ Name: "alpha-desc", DisplayName: "Alphabetically (Z-A)", Description: "Sort results in an alphabetically descending order", - Filter: searchstore.TitleSorter{Descending: true}, + Filter: []SortOptionFilter{ + searchstore.TitleSorter{Descending: true}, + }, } ) @@ -25,7 +29,11 @@ type SortOption struct { Name string DisplayName string Description string - Filter searchstore.FilterOrderBy + Filter []SortOptionFilter +} + +type SortOptionFilter interface { + searchstore.FilterOrderBy } // RegisterSortOption allows for hooking in more search options from diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 78b19873a90..70ff51a1a56 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -216,12 +216,7 @@ type DashboardSearchProjection struct { } func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSearchProjection, error) { - if query.SortBy == nil { - query.SortBy = searchstore.TitleSorter{} - } - filters := []interface{}{ - query.SortBy, permissions.DashboardPermissionFilter{ OrgRole: query.SignedInUser.OrgRole, OrgId: query.SignedInUser.OrgId, @@ -231,6 +226,8 @@ func findDashboards(query *search.FindPersistedDashboardsQuery) ([]DashboardSear }, } + filters = append(filters, query.Filters...) + if query.OrgId != 0 { filters = append(filters, searchstore.OrgFilter{OrgId: query.OrgId}) } else if query.SignedInUser.OrgId != 0 { diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index 7b3cfda075c..598856e9ace 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -9,9 +9,13 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/search" + "github.com/grafana/grafana/pkg/services/sqlstore/searchstore" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" + . "github.com/smartystreets/goconvey/convey" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestDashboardDataAccess(t *testing.T) { @@ -401,6 +405,33 @@ func TestDashboardDataAccess(t *testing.T) { }) } +func TestDashboard_SortingOptions(t *testing.T) { + // insertTestDashboard uses GoConvey's assertions. Workaround. + Convey("test with multiple sorting options", t, func() { + InitTestDB(t) + dashB := insertTestDashboard("Beta", 1, 0, false) + dashA := insertTestDashboard("Alfa", 1, 0, false) + + assert.NotZero(t, dashA.Id) + assert.Less(t, dashB.Id, dashA.Id) + + q := &search.FindPersistedDashboardsQuery{ + SignedInUser: &models.SignedInUser{OrgId: 1, UserId: 1, OrgRole: models.ROLE_ADMIN}, + // adding two sorting options (silly no-op example, but it'll complicate the query) + Filters: []interface{}{ + searchstore.TitleSorter{}, + searchstore.TitleSorter{Descending: true}, + }, + } + dashboards, err := findDashboards(q) + require.NoError(t, err) + + require.Len(t, dashboards, 2) + assert.Equal(t, dashA.Id, dashboards[0].Id) + assert.Equal(t, dashB.Id, dashboards[1].Id) + }) +} + func insertTestDashboard(title string, orgId int64, folderId int64, isFolder bool, tags ...interface{}) *models.Dashboard { cmd := models.SaveDashboardCommand{ OrgId: orgId, diff --git a/pkg/services/sqlstore/searchstore/builder.go b/pkg/services/sqlstore/searchstore/builder.go index b3c01f0a18c..df55e6cba23 100644 --- a/pkg/services/sqlstore/searchstore/builder.go +++ b/pkg/services/sqlstore/searchstore/builder.go @@ -113,13 +113,14 @@ func (b *Builder) applyFilters() (ordering string) { b.params = append(b.params, groupParams...) } - if len(orders) > 0 { - orderBy := fmt.Sprintf(" ORDER BY %s", strings.Join(orders, ", ")) - b.sql.WriteString(orderBy) - - order := strings.Join(orderJoins, "") - order += orderBy - return order + if len(orders) < 1 { + orders = append(orders, TitleSorter{}.OrderBy()) } - return " ORDER BY dashboard.id" + + orderBy := fmt.Sprintf(" ORDER BY %s", strings.Join(orders, ", ")) + b.sql.WriteString(orderBy) + + order := strings.Join(orderJoins, "") + order += orderBy + return order }