From 6ffd4a23de376b257476578819a591f513b62d33 Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Thu, 28 Sep 2023 17:20:51 +0200 Subject: [PATCH] Team: Support `sort` query param for teams search endpoint (#75622) * Teams: Implement backend sorting * Add docs * Make name ordering case insensitive * lint * Fix no lowercasing on memberCount * Add test to double check the filters or correctly OrderBy --- docs/sources/developers/http_api/team.md | 5 +- pkg/api/team.go | 7 ++ pkg/services/searchusers/sortopts/sortopts.go | 2 +- pkg/services/team/model.go | 2 + pkg/services/team/sortopts/sortopts.go | 99 +++++++++++++++++++ pkg/services/team/sortopts/sortopts_test.go | 74 ++++++++++++++ pkg/services/team/teamimpl/store.go | 12 ++- pkg/services/team/teamimpl/store_test.go | 35 +++++++ 8 files changed, 233 insertions(+), 3 deletions(-) create mode 100644 pkg/services/team/sortopts/sortopts.go create mode 100644 pkg/services/team/sortopts/sortopts_test.go diff --git a/docs/sources/developers/http_api/team.md b/docs/sources/developers/http_api/team.md index 2a5b3ac5744..cf4f52c6933 100644 --- a/docs/sources/developers/http_api/team.md +++ b/docs/sources/developers/http_api/team.md @@ -33,7 +33,7 @@ Access to these API endpoints is restricted as follows: ## Team Search With Paging -`GET /api/teams/search?perpage=50&page=1&query=myteam` +`GET /api/teams/search?perpage=50&page=1&query=myteam&sort=memberCount-desc` or @@ -87,6 +87,8 @@ The `totalCount` field in the response can be used for pagination of the teams l The `query` parameter is optional and it will return results where the query value is contained in the `name` field. Query values with spaces need to be URL encoded e.g. `query=my%20team`. +The `sort` param is an optional comma separated list of options to order the search result. Accepted values for the sort filter are: ` name-asc`, `name-desc`, `email-asc`, `email-desc`, `memberCount-asc`, `memberCount-desc`. By default, if `sort` is not specified, the teams list will be ordered by `name` in ascending order. + ### Using the name parameter The `name` parameter returns a single team if the parameter matches the `name` field. @@ -94,6 +96,7 @@ The `name` parameter returns a single team if the parameter matches the `name` f #### Status Codes: - **200** - Ok +- **400** - Bad Request - **401** - Unauthorized - **403** - Permission denied - **404** - Team not found (if searching by name) diff --git a/pkg/api/team.go b/pkg/api/team.go index fb81673c84f..42f61252815 100644 --- a/pkg/api/team.go +++ b/pkg/api/team.go @@ -11,6 +11,7 @@ import ( contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/team" + "github.com/grafana/grafana/pkg/services/team/sortopts" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/web" ) @@ -146,6 +147,11 @@ func (hs *HTTPServer) SearchTeams(c *contextmodel.ReqContext) response.Response page = 1 } + sortOpts, err := sortopts.ParseSortQueryParam(c.Query("sort")) + if err != nil { + return response.Err(err) + } + query := team.SearchTeamsQuery{ OrgID: c.SignedInUser.GetOrgID(), Query: c.Query("query"), @@ -154,6 +160,7 @@ func (hs *HTTPServer) SearchTeams(c *contextmodel.ReqContext) response.Response Limit: perPage, SignedInUser: c.SignedInUser, HiddenUsers: hs.Cfg.HiddenUsers, + SortOpts: sortOpts, } queryResult, err := hs.teamService.SearchTeams(c.Req.Context(), &query) diff --git a/pkg/services/searchusers/sortopts/sortopts.go b/pkg/services/searchusers/sortopts/sortopts.go index da4c3254d9a..c6a19e167b6 100644 --- a/pkg/services/searchusers/sortopts/sortopts.go +++ b/pkg/services/searchusers/sortopts/sortopts.go @@ -65,7 +65,7 @@ func newTimeSortOption(field string, desc bool, index int) model.SortOption { return model.SortOption{ Name: fmt.Sprintf("%v-%v", field, direction), DisplayName: fmt.Sprintf("%v (%v)", cases.Title(language.Und).String(field), description), - Description: fmt.Sprintf("Sort %v in an alphabetically %vending order", field, direction), + Description: fmt.Sprintf("Sort %v by time in an %vending order", field, direction), Index: index, Filter: []model.SortOptionFilter{Sorter{Field: field, Descending: desc}}, } diff --git a/pkg/services/team/model.go b/pkg/services/team/model.go index 704d4febbb2..3ab72eace9a 100644 --- a/pkg/services/team/model.go +++ b/pkg/services/team/model.go @@ -9,6 +9,7 @@ import ( "github.com/grafana/grafana/pkg/kinds/team" "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/search/model" ) // Typed errors @@ -90,6 +91,7 @@ type SearchTeamsQuery struct { Limit int Page int OrgID int64 `xorm:"org_id"` + SortOpts []model.SortOption SignedInUser identity.Requester HiddenUsers map[string]struct{} } diff --git a/pkg/services/team/sortopts/sortopts.go b/pkg/services/team/sortopts/sortopts.go new file mode 100644 index 00000000000..fda1f2ca560 --- /dev/null +++ b/pkg/services/team/sortopts/sortopts.go @@ -0,0 +1,99 @@ +package sortopts + +import ( + "fmt" + "sort" + "strings" + + "github.com/grafana/grafana/pkg/services/search/model" + "github.com/grafana/grafana/pkg/util/errutil" + "golang.org/x/text/cases" + "golang.org/x/text/language" +) + +var ( + // SortOptionsByQueryParam is a map to translate the "sort" query param values to SortOption(s) + SortOptionsByQueryParam = map[string]model.SortOption{ + "name-asc": newSortOption("name", false, true, 0), // Lower case the name ordering + "name-desc": newSortOption("name", true, true, 0), + "email-asc": newSortOption("email", false, false, 1), // Not to slow down the request let's not lower case the email ordering + "email-desc": newSortOption("email", true, false, 1), + "memberCount-asc": newIntSortOption("member_count", false, 2), + "memberCount-desc": newIntSortOption("member_count", true, 2), + } + + ErrorUnknownSortingOption = errutil.BadRequest("unknown sorting option") +) + +type Sorter struct { + Field string + LowerCase bool + Descending bool + WithTableName bool +} + +func (s Sorter) OrderBy() string { + orderBy := "team." + if !s.WithTableName { + orderBy = "" + } + orderBy += s.Field + if s.LowerCase { + orderBy = fmt.Sprintf("LOWER(%v)", orderBy) + } + if s.Descending { + return orderBy + " DESC" + } + return orderBy + " ASC" +} + +func newSortOption(field string, desc bool, lowerCase bool, index int) model.SortOption { + direction := "asc" + description := ("A-Z") + if desc { + direction = "desc" + description = ("Z-A") + } + return model.SortOption{ + Name: fmt.Sprintf("%v-%v", field, direction), + DisplayName: fmt.Sprintf("%v (%v)", cases.Title(language.Und).String(field), description), + Description: fmt.Sprintf("Sort %v in an alphabetically %vending order", field, direction), + Index: index, + Filter: []model.SortOptionFilter{Sorter{Field: field, LowerCase: lowerCase, Descending: desc, WithTableName: true}}, + } +} + +func newIntSortOption(field string, desc bool, index int) model.SortOption { + direction := "asc" + description := ("Fewest-Most") + if desc { + direction = "desc" + description = ("Most-Fewest") + } + return model.SortOption{ + Name: fmt.Sprintf("%v-%v", field, direction), + DisplayName: fmt.Sprintf("%v (%v)", cases.Title(language.Und).String(field), description), + Description: fmt.Sprintf("Sort %v in a numerically %vending order", field, direction), + Index: index, + Filter: []model.SortOptionFilter{Sorter{Field: field, LowerCase: false, Descending: desc, WithTableName: false}}, + } +} + +// ParseSortQueryParam parses the "sort" query param and returns an ordered list of SortOption(s) +func ParseSortQueryParam(param string) ([]model.SortOption, error) { + opts := []model.SortOption{} + if param != "" { + optsStr := strings.Split(param, ",") + for i := range optsStr { + if opt, ok := SortOptionsByQueryParam[optsStr[i]]; !ok { + return nil, ErrorUnknownSortingOption.Errorf("%v option unknown", optsStr[i]) + } else { + opts = append(opts, opt) + } + } + sort.Slice(opts, func(i, j int) bool { + return opts[i].Index < opts[j].Index || (opts[i].Index == opts[j].Index && opts[i].Name < opts[j].Name) + }) + } + return opts, nil +} diff --git a/pkg/services/team/sortopts/sortopts_test.go b/pkg/services/team/sortopts/sortopts_test.go new file mode 100644 index 00000000000..b5f25720917 --- /dev/null +++ b/pkg/services/team/sortopts/sortopts_test.go @@ -0,0 +1,74 @@ +package sortopts + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSorter_Filters(t *testing.T) { + require.Equal(t, SortOptionsByQueryParam["name-asc"].Filter[0].OrderBy(), "LOWER(team.name) ASC") + require.Equal(t, SortOptionsByQueryParam["name-desc"].Filter[0].OrderBy(), "LOWER(team.name) DESC") + require.Equal(t, SortOptionsByQueryParam["email-asc"].Filter[0].OrderBy(), "team.email ASC") + require.Equal(t, SortOptionsByQueryParam["email-desc"].Filter[0].OrderBy(), "team.email DESC") + require.Equal(t, SortOptionsByQueryParam["memberCount-asc"].Filter[0].OrderBy(), "member_count ASC") + require.Equal(t, SortOptionsByQueryParam["memberCount-desc"].Filter[0].OrderBy(), "member_count DESC") +} + +func TestSorter_OrderBy(t *testing.T) { + type fields struct { + Field string + LowerCase bool + Descending bool + WithTableName bool + } + tests := []struct { + name string + fields fields + want string + }{ + { + name: "team.email case sensitive desc", + fields: fields{ + Field: "email", + LowerCase: false, + Descending: true, + WithTableName: true, + }, + want: "team.email DESC", + }, + { + name: "member_count sensitive desc", + fields: fields{ + Field: "member_count", + LowerCase: false, + Descending: true, + WithTableName: false, + }, + want: "member_count DESC", + }, + { + name: "team.name case insensitive asc", + fields: fields{ + Field: "name", + LowerCase: true, + Descending: false, + WithTableName: true, + }, + want: "LOWER(team.name) ASC", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := Sorter{ + Field: tt.fields.Field, + LowerCase: tt.fields.LowerCase, + Descending: tt.fields.Descending, + WithTableName: tt.fields.WithTableName, + } + + got := s.OrderBy() + require.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/services/team/teamimpl/store.go b/pkg/services/team/teamimpl/store.go index ea3debab58a..f2f830a5565 100644 --- a/pkg/services/team/teamimpl/store.go +++ b/pkg/services/team/teamimpl/store.go @@ -215,7 +215,17 @@ func (ss *xormStore) Search(ctx context.Context, query *team.SearchTeamsQuery) ( sql.WriteString(` and` + acFilter.Where) params = append(params, acFilter.Args...) - sql.WriteString(` order by team.name asc`) + if len(query.SortOpts) > 0 { + orderBy := ` order by ` + for i := range query.SortOpts { + for j := range query.SortOpts[i].Filter { + orderBy += query.SortOpts[i].Filter[j].OrderBy() + "," + } + } + sql.WriteString(orderBy[:len(orderBy)-1]) + } else { + sql.WriteString(` order by team.name asc`) + } if query.Limit != 0 { offset := query.Limit * (query.Page - 1) diff --git a/pkg/services/team/teamimpl/store_test.go b/pkg/services/team/teamimpl/store_test.go index e8787a9c105..67d09377717 100644 --- a/pkg/services/team/teamimpl/store_test.go +++ b/pkg/services/team/teamimpl/store_test.go @@ -20,6 +20,7 @@ import ( "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest" "github.com/grafana/grafana/pkg/services/team" + "github.com/grafana/grafana/pkg/services/team/sortopts" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user/userimpl" ) @@ -233,6 +234,40 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) { require.Equal(t, len(query2Result.Teams), 2) }) + t.Run("Should be able to sort teams by descending member count order", func(t *testing.T) { + sortOpts, err := sortopts.ParseSortQueryParam("memberCount-desc") + require.NoError(t, err) + + // Add a team member + err = teamSvc.AddTeamMember(userIds[0], testOrgID, team2.ID, false, 0) + require.NoError(t, err) + defer func() { + err := teamSvc.RemoveTeamMember(context.Background(), + &team.RemoveTeamMemberCommand{OrgID: testOrgID, UserID: userIds[0], TeamID: team2.ID}) + require.NoError(t, err) + }() + + query := &team.SearchTeamsQuery{OrgID: testOrgID, SortOpts: sortOpts, SignedInUser: testUser} + queryResult, err := teamSvc.SearchTeams(context.Background(), query) + require.NoError(t, err) + require.Equal(t, len(queryResult.Teams), 2) + require.EqualValues(t, queryResult.TotalCount, 2) + require.Greater(t, queryResult.Teams[0].MemberCount, queryResult.Teams[1].MemberCount) + }) + + t.Run("Should be able to sort teams by descending name order", func(t *testing.T) { + sortOpts, err := sortopts.ParseSortQueryParam("name-desc") + require.NoError(t, err) + + query := &team.SearchTeamsQuery{OrgID: testOrgID, SortOpts: sortOpts, SignedInUser: testUser} + queryResult, err := teamSvc.SearchTeams(context.Background(), query) + require.NoError(t, err) + require.Equal(t, len(queryResult.Teams), 2) + require.EqualValues(t, queryResult.TotalCount, 2) + require.Equal(t, queryResult.Teams[0].Name, team2.Name) + require.Equal(t, queryResult.Teams[1].Name, team1.Name) + }) + t.Run("Should be able to return all teams a user is member of", func(t *testing.T) { sqlStore = db.InitTestDB(t) setup()