[MM-12476] Consistent paging arguments limit/offset vs page/perPage for plugin API (#9838)

* Change GetTeamMembers() and GetPublicChannelsForTeam() arguments to page, perPage for plugin API

* Add test for GetPublicChannelsForTeam()

* Add test for GetTeamMembers()

* Changes as requested

* Change return from GetPublicChannelsForTeam() to []*model.Channel

app/channel_test.go

@@ -4,6 +4,7 @@
 package app
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
@@ -747,3 +748,46 @@ func TestGetChannelMembersTimezones(t *testing.T) {
 	}
 	assert.Equal(t, 2, len(timezones))
 }
+
+func TestGetPublicChannelsForTeam(t *testing.T) {
+	th := Setup()
+	team := th.CreateTeam()
+	defer th.TearDown()
+
+	var expectedChannels []*model.Channel
+
+	townSquare, err := th.App.GetChannelByName("town-square", team.Id, false)
+	require.Nil(t, err)
+	require.NotNil(t, townSquare)
+	expectedChannels = append(expectedChannels, townSquare)
+
+	offTopic, err := th.App.GetChannelByName("off-topic", team.Id, false)
+	require.Nil(t, err)
+	require.NotNil(t, offTopic)
+	expectedChannels = append(expectedChannels, offTopic)
+
+	for i := 0; i < 8; i++ {
+		channel := model.Channel{
+			DisplayName: fmt.Sprintf("Public %v", i),
+			Name:        fmt.Sprintf("public_%v", i),
+			Type:        model.CHANNEL_OPEN,
+			TeamId:      team.Id,
+		}
+		rchannel, err := th.App.CreateChannel(&channel, false)
+		require.Nil(t, err)
+		require.NotNil(t, rchannel)
+		defer th.App.PermanentDeleteChannel(rchannel)
+
+		// Store the user ids for comparison later
+		expectedChannels = append(expectedChannels, rchannel)
+	}
+
+	// Fetch public channels multipile times
+	channelList, err := th.App.GetPublicChannelsForTeam(team.Id, 0, 5)
+	require.Nil(t, err)
+	channelList2, err := th.App.GetPublicChannelsForTeam(team.Id, 5, 5)
+	require.Nil(t, err)
+
+	channels := append(*channelList, *channelList2...)
+	assert.ElementsMatch(t, expectedChannels, channels)
+}

app/plugin_api.go

@@ -132,8 +132,8 @@ func (api *PluginAPI) DeleteTeamMember(teamId, userId, requestorId string) *mode
 	return api.app.RemoveUserFromTeam(teamId, userId, requestorId)
 }
 
-func (api *PluginAPI) GetTeamMembers(teamId string, offset, limit int) ([]*model.TeamMember, *model.AppError) {
+func (api *PluginAPI) GetTeamMembers(teamId string, page, perPage int) ([]*model.TeamMember, *model.AppError) {
-	return api.app.GetTeamMembers(teamId, offset, limit)
+	return api.app.GetTeamMembers(teamId, page*perPage, perPage)
 }
 
 func (api *PluginAPI) GetTeamMember(teamId, userId string) (*model.TeamMember, *model.AppError) {
@@ -246,8 +246,9 @@ func (api *PluginAPI) DeleteChannel(channelId string) *model.AppError {
 	return api.app.DeleteChannel(channel, "")
 }
 
-func (api *PluginAPI) GetPublicChannelsForTeam(teamId string, offset, limit int) (*model.ChannelList, *model.AppError) {
+func (api *PluginAPI) GetPublicChannelsForTeam(teamId string, page, perPage int) ([]*model.Channel, *model.AppError) {
-	return api.app.GetPublicChannelsForTeam(teamId, offset, limit)
+	channels, err := api.app.GetPublicChannelsForTeam(teamId, page*perPage, perPage)
+	return *channels, err
 }
 
 func (api *PluginAPI) GetChannel(channelId string) (*model.Channel, *model.AppError) {

app/team_test.go

@@ -4,10 +4,14 @@
 package app
 
 import (
+	"fmt"
+	"sort"
 	"strings"
 	"testing"
 
 	"github.com/mattermost/mattermost-server/model"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestCreateTeam(t *testing.T) {
@@ -683,3 +687,45 @@ func TestAppUpdateTeamScheme(t *testing.T) {
 		t.Fatal("Wrong Team SchemeId")
 	}
 }
+
+func TestGetTeamMembers(t *testing.T) {
+	th := Setup().InitBasic()
+	defer th.TearDown()
+
+	var userIDs sort.StringSlice
+	userIDs = append(userIDs, th.BasicUser.Id)
+	userIDs = append(userIDs, th.BasicUser2.Id)
+
+	for i := 0; i < 8; i++ {
+		user := model.User{
+			Email:    strings.ToLower(model.NewId()) + "success+test@example.com",
+			Username: fmt.Sprintf("user%v", i),
+			Password: "passwd1",
+		}
+		ruser, err := th.App.CreateUser(&user)
+		require.Nil(t, err)
+		require.NotNil(t, ruser)
+		defer th.App.PermanentDeleteUser(&user)
+
+		_, err = th.App.AddUserToTeam(th.BasicTeam.Id, ruser.Id, "")
+		require.Nil(t, err)
+
+		// Store the user ids for comparison later
+		userIDs = append(userIDs, ruser.Id)
+	}
+	// Sort them because the result of GetTeamMembers() is also sorted
+	sort.Sort(userIDs)
+
+	// Fetch team members multipile times
+	members, err := th.App.GetTeamMembers(th.BasicTeam.Id, 0, 5)
+	require.Nil(t, err)
+	// This should return 5 members
+	members2, err := th.App.GetTeamMembers(th.BasicTeam.Id, 5, 6)
+	require.Nil(t, err)
+	members = append(members, members2...)
+
+	require.Equal(t, len(userIDs), len(members))
+	for i, member := range members {
+		assert.Equal(t, userIDs[i], member.UserId)
+	}
+}

plugin/api.go

@@ -139,7 +139,7 @@ type API interface {
 	DeleteTeamMember(teamId, userId, requestorId string) *model.AppError
 
 	// GetTeamMembers returns the memberships of a specific team.
-	GetTeamMembers(teamId string, offset, limit int) ([]*model.TeamMember, *model.AppError)
+	GetTeamMembers(teamId string, page, perPage int) ([]*model.TeamMember, *model.AppError)
 
 	// GetTeamMember returns a specific membership.
 	GetTeamMember(teamId, userId string) (*model.TeamMember, *model.AppError)
@@ -154,7 +154,7 @@ type API interface {
 	DeleteChannel(channelId string) *model.AppError
 
 	// GetPublicChannelsForTeam gets a list of all channels.
-	GetPublicChannelsForTeam(teamId string, offset, limit int) (*model.ChannelList, *model.AppError)
+	GetPublicChannelsForTeam(teamId string, page, perPage int) ([]*model.Channel, *model.AppError)
 
 	// GetChannel gets a channel.
 	GetChannel(channelId string) (*model.Channel, *model.AppError)

plugin/client_rpc_generated.go

@@ -1456,8 +1456,8 @@ type Z_GetTeamMembersReturns struct {
 	B *model.AppError
 }
 
-func (g *apiRPCClient) GetTeamMembers(teamId string, offset, limit int) ([]*model.TeamMember, *model.AppError) {
+func (g *apiRPCClient) GetTeamMembers(teamId string, page, perPage int) ([]*model.TeamMember, *model.AppError) {
-	_args := &Z_GetTeamMembersArgs{teamId, offset, limit}
+	_args := &Z_GetTeamMembersArgs{teamId, page, perPage}
 	_returns := &Z_GetTeamMembersReturns{}
 	if err := g.client.Call("Plugin.GetTeamMembers", _args, _returns); err != nil {
 		log.Printf("RPC call to GetTeamMembers API failed: %s", err.Error())
@@ -1467,7 +1467,7 @@ func (g *apiRPCClient) GetTeamMembers(teamId string, offset, limit int) ([]*mode
 
 func (s *apiRPCServer) GetTeamMembers(args *Z_GetTeamMembersArgs, returns *Z_GetTeamMembersReturns) error {
 	if hook, ok := s.impl.(interface {
-		GetTeamMembers(teamId string, offset, limit int) ([]*model.TeamMember, *model.AppError)
+		GetTeamMembers(teamId string, page, perPage int) ([]*model.TeamMember, *model.AppError)
 	}); ok {
 		returns.A, returns.B = hook.GetTeamMembers(args.A, args.B, args.C)
 	} else {
@@ -1601,12 +1601,12 @@ type Z_GetPublicChannelsForTeamArgs struct {
 }
 
 type Z_GetPublicChannelsForTeamReturns struct {
-	A *model.ChannelList
+	A []*model.Channel
 	B *model.AppError
 }
 
-func (g *apiRPCClient) GetPublicChannelsForTeam(teamId string, offset, limit int) (*model.ChannelList, *model.AppError) {
+func (g *apiRPCClient) GetPublicChannelsForTeam(teamId string, page, perPage int) ([]*model.Channel, *model.AppError) {
-	_args := &Z_GetPublicChannelsForTeamArgs{teamId, offset, limit}
+	_args := &Z_GetPublicChannelsForTeamArgs{teamId, page, perPage}
 	_returns := &Z_GetPublicChannelsForTeamReturns{}
 	if err := g.client.Call("Plugin.GetPublicChannelsForTeam", _args, _returns); err != nil {
 		log.Printf("RPC call to GetPublicChannelsForTeam API failed: %s", err.Error())
@@ -1616,7 +1616,7 @@ func (g *apiRPCClient) GetPublicChannelsForTeam(teamId string, offset, limit int
 
 func (s *apiRPCServer) GetPublicChannelsForTeam(args *Z_GetPublicChannelsForTeamArgs, returns *Z_GetPublicChannelsForTeamReturns) error {
 	if hook, ok := s.impl.(interface {
-		GetPublicChannelsForTeam(teamId string, offset, limit int) (*model.ChannelList, *model.AppError)
+		GetPublicChannelsForTeam(teamId string, page, perPage int) ([]*model.Channel, *model.AppError)
 	}); ok {
 		returns.A, returns.B = hook.GetPublicChannelsForTeam(args.A, args.B, args.C)
 	} else {

plugin/plugintest/api.go

@@ -1036,22 +1036,22 @@ func (_m *API) GetProfileImage(userId string) ([]byte, *model.AppError) {
 	return r0, r1
 }
 
-// GetPublicChannelsForTeam provides a mock function with given fields: teamId, offset, limit
+// GetPublicChannelsForTeam provides a mock function with given fields: teamId, page, perPage
-func (_m *API) GetPublicChannelsForTeam(teamId string, offset int, limit int) (*model.ChannelList, *model.AppError) {
+func (_m *API) GetPublicChannelsForTeam(teamId string, page int, perPage int) ([]*model.Channel, *model.AppError) {
-	ret := _m.Called(teamId, offset, limit)
+	ret := _m.Called(teamId, page, perPage)
 
-	var r0 *model.ChannelList
+	var r0 []*model.Channel
-	if rf, ok := ret.Get(0).(func(string, int, int) *model.ChannelList); ok {
+	if rf, ok := ret.Get(0).(func(string, int, int) []*model.Channel); ok {
-		r0 = rf(teamId, offset, limit)
+		r0 = rf(teamId, page, perPage)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(*model.ChannelList)
+			r0 = ret.Get(0).([]*model.Channel)
 		}
 	}
 
 	var r1 *model.AppError
 	if rf, ok := ret.Get(1).(func(string, int, int) *model.AppError); ok {
-		r1 = rf(teamId, offset, limit)
+		r1 = rf(teamId, page, perPage)
 	} else {
 		if ret.Get(1) != nil {
 			r1 = ret.Get(1).(*model.AppError)
@@ -1225,13 +1225,13 @@ func (_m *API) GetTeamMember(teamId string, userId string) (*model.TeamMember, *
 	return r0, r1
 }
 
-// GetTeamMembers provides a mock function with given fields: teamId, offset, limit
+// GetTeamMembers provides a mock function with given fields: teamId, page, perPage
-func (_m *API) GetTeamMembers(teamId string, offset int, limit int) ([]*model.TeamMember, *model.AppError) {
+func (_m *API) GetTeamMembers(teamId string, page int, perPage int) ([]*model.TeamMember, *model.AppError) {
-	ret := _m.Called(teamId, offset, limit)
+	ret := _m.Called(teamId, page, perPage)
 
 	var r0 []*model.TeamMember
 	if rf, ok := ret.Get(0).(func(string, int, int) []*model.TeamMember); ok {
-		r0 = rf(teamId, offset, limit)
+		r0 = rf(teamId, page, perPage)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).([]*model.TeamMember)
@@ -1240,7 +1240,7 @@ func (_m *API) GetTeamMembers(teamId string, offset int, limit int) ([]*model.Te
 
 	var r1 *model.AppError
 	if rf, ok := ret.Get(1).(func(string, int, int) *model.AppError); ok {
-		r1 = rf(teamId, offset, limit)
+		r1 = rf(teamId, page, perPage)
 	} else {
 		if ret.Get(1) != nil {
 			r1 = ret.Get(1).(*model.AppError)