MM-18357: Adds pagination to team search. (#12910)

* MM-18357: Adds pagination to team search.

* MM-18357: Adds new client method for paginated requests.

* MM-18357: Adds feedback about non-supported pagination-permissions combo.

* MM-18357: Removes unnecessary conversion.

* MM-18357: Removes paginate parameter and uses nil on page and perpage instead.
This commit is contained in:
Martin Kraft
2019-11-28 08:11:02 -05:00
committed by GitHub
parent 955f8c4e8e
commit 14bcd1f0a1
11 changed files with 187 additions and 7 deletions

View File

@@ -789,13 +789,22 @@ func searchTeams(c *Context, w http.ResponseWriter, r *http.Request) {
}
var teams []*model.Team
var totalCount int64
var err *model.AppError
if c.App.SessionHasPermissionTo(c.App.Session, model.PERMISSION_LIST_PRIVATE_TEAMS) && c.App.SessionHasPermissionTo(c.App.Session, model.PERMISSION_LIST_PUBLIC_TEAMS) {
teams, err = c.App.SearchAllTeams(props.Term)
teams, totalCount, err = c.App.SearchAllTeams(props)
} else if c.App.SessionHasPermissionTo(c.App.Session, model.PERMISSION_LIST_PRIVATE_TEAMS) {
if props.Page != nil || props.PerPage != nil {
c.Err = model.NewAppError("searchTeams", "api.team.search_teams.pagination_not_implemented.private_team_search", nil, "", http.StatusNotImplemented)
return
}
teams, err = c.App.SearchPrivateTeams(props.Term)
} else if c.App.SessionHasPermissionTo(c.App.Session, model.PERMISSION_LIST_PUBLIC_TEAMS) {
if props.Page != nil || props.PerPage != nil {
c.Err = model.NewAppError("searchTeams", "api.team.search_teams.pagination_not_implemented.public_team_search", nil, "", http.StatusNotImplemented)
return
}
teams, err = c.App.SearchPublicTeams(props.Term)
} else {
teams = []*model.Team{}
@@ -808,7 +817,15 @@ func searchTeams(c *Context, w http.ResponseWriter, r *http.Request) {
c.App.SanitizeTeams(c.App.Session, teams)
w.Write([]byte(model.TeamListToJson(teams)))
var payload []byte
if props.Page != nil && props.PerPage != nil {
twc := &model.TeamsWithCount{Teams: teams, TotalCount: totalCount}
payload = model.TeamsWithCountToJson(twc)
} else {
payload = []byte(model.TeamListToJson(teams))
}
w.Write(payload)
}
func teamExists(c *Context, w http.ResponseWriter, r *http.Request) {

View File

@@ -922,6 +922,79 @@ func TestSearchAllTeams(t *testing.T) {
CheckUnauthorizedStatus(t, resp)
}
func TestSearchAllTeamsPaged(t *testing.T) {
th := Setup().InitBasic()
defer th.TearDown()
commonRandom := model.NewId()
teams := [3]*model.Team{}
for i := 0; i < 3; i++ {
uid := model.NewId()
newTeam, err := th.App.CreateTeam(&model.Team{
DisplayName: fmt.Sprintf("%s %d %s", commonRandom, i, uid),
Name: fmt.Sprintf("%s-%d-%s", commonRandom, i, uid),
Type: model.TEAM_OPEN,
Email: th.GenerateTestEmail(),
})
require.Nil(t, err)
teams[i] = newTeam
}
testCases := []struct {
Name string
Search *model.TeamSearch
ExpectedTeams []string
ExpectedTotalCount int64
}{
{
Name: "Get all teams on one page",
Search: &model.TeamSearch{Term: commonRandom, Page: model.NewInt(0), PerPage: model.NewInt(100)},
ExpectedTeams: []string{teams[0].Id, teams[1].Id, teams[2].Id},
ExpectedTotalCount: 3,
},
{
Name: "Get 2 teams on the first page",
Search: &model.TeamSearch{Term: commonRandom, Page: model.NewInt(0), PerPage: model.NewInt(2)},
ExpectedTeams: []string{teams[0].Id, teams[1].Id},
ExpectedTotalCount: 3,
},
{
Name: "Get 1 team on the second page",
Search: &model.TeamSearch{Term: commonRandom, Page: model.NewInt(1), PerPage: model.NewInt(2)},
ExpectedTeams: []string{teams[2].Id},
ExpectedTotalCount: 3,
},
{
Name: "SearchTeamsPaged paginates results by default",
Search: &model.TeamSearch{Term: commonRandom},
ExpectedTeams: []string{teams[0].Id, teams[1].Id, teams[2].Id},
ExpectedTotalCount: 3,
},
{
Name: "No results",
Search: &model.TeamSearch{Term: model.NewId()},
ExpectedTeams: []string{},
ExpectedTotalCount: 0,
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
teams, count, resp := th.SystemAdminClient.SearchTeamsPaged(tc.Search)
require.Nil(t, resp.Error)
require.Equal(t, tc.ExpectedTotalCount, count)
require.Equal(t, len(tc.ExpectedTeams), len(teams))
for i, team := range teams {
require.Equal(t, tc.ExpectedTeams[i], team.Id)
}
})
}
_, _, resp := th.Client.SearchTeamsPaged(&model.TeamSearch{Term: commonRandom, PerPage: model.NewInt(100)})
require.Equal(t, "api.team.search_teams.pagination_not_implemented.public_team_search", resp.Error.Id)
require.Equal(t, http.StatusNotImplemented, resp.StatusCode)
}
func TestSearchAllTeamsSanitization(t *testing.T) {
th := Setup().InitBasic()
defer th.TearDown()

View File

@@ -149,7 +149,8 @@ func (api *PluginAPI) GetTeam(teamId string) (*model.Team, *model.AppError) {
}
func (api *PluginAPI) SearchTeams(term string) ([]*model.Team, *model.AppError) {
return api.app.SearchAllTeams(term)
teams, _, err := api.app.SearchAllTeams(&model.TeamSearch{Term: term})
return teams, err
}
func (api *PluginAPI) GetTeamByName(name string) (*model.Team, *model.AppError) {

View File

@@ -710,8 +710,13 @@ func (a *App) GetAllPublicTeamsPageWithCount(offset int, limit int) (*model.Team
return &model.TeamsWithCount{Teams: teams, TotalCount: totalCount}, nil
}
func (a *App) SearchAllTeams(term string) ([]*model.Team, *model.AppError) {
return a.Srv.Store.Team().SearchAll(term)
// SearchAllTeams returns a team list and the total count of the results
func (a *App) SearchAllTeams(searchOpts *model.TeamSearch) ([]*model.Team, int64, *model.AppError) {
if searchOpts.IsPaginated() {
return a.Srv.Store.Team().SearchAllPaged(searchOpts.Term, *searchOpts.Page, *searchOpts.PerPage)
}
results, err := a.Srv.Store.Team().SearchAll(searchOpts.Term)
return results, int64(len(results)), err
}
func (a *App) SearchPublicTeams(term string) ([]*model.Team, *model.AppError) {

View File

@@ -310,7 +310,7 @@ func searchTeamCmdF(command *cobra.Command, args []string) error {
var teams []*model.Team
for _, searchTerm := range args {
foundTeams, err := a.SearchAllTeams(searchTerm)
foundTeams, _, err := a.SearchAllTeams(&model.TeamSearch{Term: searchTerm})
if err != nil {
return err
}

View File

@@ -2034,6 +2034,14 @@
"id": "api.team.remove_user_from_team.removed",
"translation": "%v removed from the team."
},
{
"id": "api.team.search_teams.pagination_not_implemented.private_team_search",
"translation": "Pagination not implemented for private-only team search."
},
{
"id": "api.team.search_teams.pagination_not_implemented.public_team_search",
"translation": "Pagination not implemented for public-only team search."
},
{
"id": "api.team.set_team_icon.array.app_error",
"translation": "Empty array under 'image' in request"

View File

@@ -1661,6 +1661,23 @@ func (c *Client4) SearchTeams(search *TeamSearch) ([]*Team, *Response) {
return TeamListFromJson(r.Body), BuildResponse(r)
}
// SearchTeamsPaged returns a page of teams and the total count matching the provided search term.
func (c *Client4) SearchTeamsPaged(search *TeamSearch) ([]*Team, int64, *Response) {
if search.Page == nil {
search.Page = NewInt(0)
}
if search.PerPage == nil {
search.PerPage = NewInt(100)
}
r, err := c.DoApiPost(c.GetTeamsRoute()+"/search", search.ToJson())
if err != nil {
return nil, 0, BuildErrorResponse(r, err)
}
defer closeBody(r)
twc := TeamsWithCountFromJson(r.Body)
return twc.Teams, twc.TotalCount, BuildResponse(r)
}
// TeamExists returns true or false if the team exist or not.
func (c *Client4) TeamExists(name, etag string) (bool, *Response) {
r, err := c.DoApiGet(c.GetTeamByNameRoute(name)+"/exists", etag)

View File

@@ -9,7 +9,13 @@ import (
)
type TeamSearch struct {
Term string `json:"term"`
Term string `json:"term"`
Page *int `json:"page,omitempty"`
PerPage *int `json:"per_page,omitempty"`
}
func (t *TeamSearch) IsPaginated() bool {
return t.Page != nil && t.PerPage != nil
}
// ToJson convert a TeamSearch to json string

View File

@@ -305,6 +305,26 @@ func (s SqlTeamStore) SearchAll(term string) ([]*model.Team, *model.AppError) {
return teams, nil
}
// SearchAllPaged returns a teams list and the total count of teams that matched the search.
func (s SqlTeamStore) SearchAllPaged(term string, page int, perPage int) ([]*model.Team, int64, *model.AppError) {
var teams []*model.Team
var totalCount int64
offset := page * perPage
term = sanitizeSearchTerm(term, "\\")
if _, err := s.GetReplica().Select(&teams, "SELECT * FROM Teams WHERE Name LIKE :Term OR DisplayName LIKE :Term ORDER BY DisplayName, Name LIMIT :Limit OFFSET :Offset", map[string]interface{}{"Term": term + "%", "Limit": perPage, "Offset": offset}); err != nil {
return nil, 0, model.NewAppError("SqlTeamStore.SearchAllPage", "store.sql_team.search_all_team.app_error", nil, "term="+term+", "+err.Error(), http.StatusInternalServerError)
}
totalCount, err := s.GetReplica().SelectInt("SELECT COUNT(*) FROM Teams WHERE Name LIKE :Term OR DisplayName LIKE :Term", map[string]interface{}{"Term": term + "%"})
if err != nil {
return nil, 0, model.NewAppError("SqlTeamStore.SearchAllPage", "store.sql_team.search_all_team.app_error", nil, "term="+term+", "+err.Error(), http.StatusInternalServerError)
}
return teams, totalCount, nil
}
func (s SqlTeamStore) SearchOpen(term string) ([]*model.Team, *model.AppError) {
var teams []*model.Team

View File

@@ -68,6 +68,7 @@ type TeamStore interface {
Get(id string) (*model.Team, *model.AppError)
GetByName(name string) (*model.Team, *model.AppError)
SearchAll(term string) ([]*model.Team, *model.AppError)
SearchAllPaged(term string, page int, perPage int) ([]*model.Team, int64, *model.AppError)
SearchOpen(term string) ([]*model.Team, *model.AppError)
SearchPrivate(term string) ([]*model.Team, *model.AppError)
GetAll() ([]*model.Team, *model.AppError)

View File

@@ -908,6 +908,38 @@ func (_m *TeamStore) SearchAll(term string) ([]*model.Team, *model.AppError) {
return r0, r1
}
// SearchAllPaged provides a mock function with given fields: term, page, perPage
func (_m *TeamStore) SearchAllPaged(term string, page int, perPage int) ([]*model.Team, int64, *model.AppError) {
ret := _m.Called(term, page, perPage)
var r0 []*model.Team
if rf, ok := ret.Get(0).(func(string, int, int) []*model.Team); ok {
r0 = rf(term, page, perPage)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).([]*model.Team)
}
}
var r1 int64
if rf, ok := ret.Get(1).(func(string, int, int) int64); ok {
r1 = rf(term, page, perPage)
} else {
r1 = ret.Get(1).(int64)
}
var r2 *model.AppError
if rf, ok := ret.Get(2).(func(string, int, int) *model.AppError); ok {
r2 = rf(term, page, perPage)
} else {
if ret.Get(2) != nil {
r2 = ret.Get(2).(*model.AppError)
}
}
return r0, r1, r2
}
// SearchOpen provides a mock function with given fields: term
func (_m *TeamStore) SearchOpen(term string) ([]*model.Team, *model.AppError) {
ret := _m.Called(term)