MM-58756: paginate webhooks list (#27368)

* MM-58756: paginate webhooks list

* show error if webhook list fails

server/cmd/mmctl/commands/webhook.go

@@ -108,16 +108,19 @@ func listWebhookCmdF(c client.Client, command *cobra.Command, args []string) err
 			continue
 		}
 
-		// Fetch all hooks with a very large limit so we get them all.
 		incomingResult := make(chan StoreResult, 1)
 		go func() {
-			incomingHooks, _, err := c.GetIncomingWebhooksForTeam(context.TODO(), team.Id, 0, 100000000, "")
+			incomingHooks, err := getPages(func(page, numPerPage int, etag string) ([]*model.IncomingWebhook, *model.Response, error) {
+				return c.GetIncomingWebhooksForTeam(context.TODO(), team.Id, page, numPerPage, etag)
+			}, DefaultPageSize)
 			incomingResult <- StoreResult{Data: incomingHooks, Err: err}
 			close(incomingResult)
 		}()
 		outgoingResult := make(chan StoreResult, 1)
 		go func() {
-			outgoingHooks, _, err := c.GetOutgoingWebhooksForTeam(context.TODO(), team.Id, 0, 100000000, "")
+			outgoingHooks, err := getPages(func(page, numPerPage int, etag string) ([]*model.OutgoingWebhook, *model.Response, error) {
+				return c.GetOutgoingWebhooksForTeam(context.TODO(), team.Id, page, numPerPage, etag)
+			}, DefaultPageSize)
 			outgoingResult <- StoreResult{Data: outgoingHooks, Err: err}
 			close(outgoingResult)
 		}()
@@ -128,7 +131,7 @@ func listWebhookCmdF(c client.Client, command *cobra.Command, args []string) err
 				printer.PrintT("Incoming:\t{{.DisplayName}} ({{.Id}}", hook)
 			}
 		} else {
-			printer.PrintError("Unable to list incoming webhooks for '" + team.Id + "'")
+			printer.PrintError("Unable to list incoming webhooks for '" + team.Id + "': " + result.Err.Error())
 		}
 
 		if result := <-outgoingResult; result.Err == nil {
@@ -137,7 +140,7 @@ func listWebhookCmdF(c client.Client, command *cobra.Command, args []string) err
 				printer.PrintT("Outgoing:\t {{.DisplayName}} ({{.Id}})", hook)
 			}
 		} else {
-			printer.PrintError("Unable to list outgoing webhooks for '" + team.Id + "'")
+			printer.PrintError("Unable to list outgoing webhooks for '" + team.Id + "': " + result.Err.Error())
 		}
 	}
 

server/cmd/mmctl/commands/webhook_test.go

@@ -8,9 +8,11 @@ import (
 	"net/http"
 	"strconv"
 
+	gomock "github.com/golang/mock/gomock"
 	"github.com/mattermost/mattermost/server/public/model"
 	"github.com/pkg/errors"
 
+	"github.com/mattermost/mattermost/server/v8/cmd/mmctl/mocks"
 	"github.com/mattermost/mattermost/server/v8/cmd/mmctl/printer"
 
 	"github.com/spf13/cobra"
@@ -23,12 +25,6 @@ func (s *MmctlUnitTestSuite) TestListWebhookCmd() {
 	outgoingWebhookID := "outgoingWebhookID"
 	outgoingWebhookDisplayName := "outgoingWebhookDisplayName"
 
-	s.Run("Listing all webhooks", func() {
-		printer.Clean()
-
-		mockTeam := model.Team{
-			Id: teamID,
-		}
 	mockIncomingWebhook := model.IncomingWebhook{
 		Id:          incomingWebhookID,
 		DisplayName: incomingWebhookDisplayName,
@@ -38,6 +34,33 @@ func (s *MmctlUnitTestSuite) TestListWebhookCmd() {
 		DisplayName: outgoingWebhookDisplayName,
 	}
 
+	var mockIncomingWebhooksPage1 []*model.IncomingWebhook
+	for i := 0; i < 200; i++ {
+		mockIncomingWebhooksPage1 = append(mockIncomingWebhooksPage1, &mockIncomingWebhook)
+	}
+
+	var mockIncomingWebhooksPage2 []*model.IncomingWebhook
+	for i := 0; i < 50; i++ {
+		mockIncomingWebhooksPage2 = append(mockIncomingWebhooksPage2, &mockIncomingWebhook)
+	}
+
+	var mockOutgoingWebhooksPage1 []*model.OutgoingWebhook
+	for i := 0; i < 200; i++ {
+		mockOutgoingWebhooksPage1 = append(mockOutgoingWebhooksPage1, &mockOutgoingWebhook)
+	}
+
+	var mockOutgoingWebhooksPage2 []*model.OutgoingWebhook
+	for i := 0; i < 50; i++ {
+		mockOutgoingWebhooksPage2 = append(mockOutgoingWebhooksPage2, &mockOutgoingWebhook)
+	}
+
+	s.Run("Listing all webhooks", func() {
+		printer.Clean()
+
+		mockTeam := model.Team{
+			Id: teamID,
+		}
+
 		s.client.
 			EXPECT().
 			GetAllTeams(context.TODO(), "", 0, DefaultPageSize).
@@ -52,38 +75,63 @@ func (s *MmctlUnitTestSuite) TestListWebhookCmd() {
 
 		s.client.
 			EXPECT().
-			GetIncomingWebhooksForTeam(context.TODO(), teamID, 0, 100000000, "").
+			GetIncomingWebhooksForTeam(context.TODO(), teamID, 0, 200, "").
-			Return([]*model.IncomingWebhook{&mockIncomingWebhook}, &model.Response{}, nil).
+			Return(mockIncomingWebhooksPage1, &model.Response{}, nil).
 			Times(1)
 
 		s.client.
 			EXPECT().
-			GetOutgoingWebhooksForTeam(context.TODO(), teamID, 0, 100000000, "").
+			GetIncomingWebhooksForTeam(context.TODO(), teamID, 1, 200, "").
-			Return([]*model.OutgoingWebhook{&mockOutgoingWebhook}, &model.Response{}, nil).
+			Return(mockIncomingWebhooksPage2, &model.Response{}, nil).
+			Times(1)
+
+		s.client.
+			EXPECT().
+			GetIncomingWebhooksForTeam(context.TODO(), teamID, 2, 200, "").
+			Return(nil, &model.Response{}, nil).
+			Times(1)
+
+		s.client.
+			EXPECT().
+			GetOutgoingWebhooksForTeam(context.TODO(), teamID, 0, 200, "").
+			Return(mockOutgoingWebhooksPage1, &model.Response{}, nil).
+			Times(1)
+
+		s.client.
+			EXPECT().
+			GetOutgoingWebhooksForTeam(context.TODO(), teamID, 1, 200, "").
+			Return(mockOutgoingWebhooksPage2, &model.Response{}, nil).
+			Times(1)
+
+		s.client.
+			EXPECT().
+			GetOutgoingWebhooksForTeam(context.TODO(), teamID, 2, 200, "").
+			Return(nil, &model.Response{}, nil).
 			Times(1)
 
 		err := listWebhookCmdF(s.client, &cobra.Command{}, []string{})
 		s.Require().Nil(err)
-		s.Len(printer.GetLines(), 2)
+		if s.Len(printer.GetLines(), 500) {
+			for i := 0; i < 250; i++ {
+				s.Require().Equal(&mockIncomingWebhook, printer.GetLines()[i])
+			}
+			for i := 250; i < 500; i++ {
+				s.Require().Equal(&mockOutgoingWebhook, printer.GetLines()[i])
+			}
+		}
+
 		s.Len(printer.GetErrorLines(), 0)
-		s.Require().Equal(&mockIncomingWebhook, printer.GetLines()[0])
-		s.Require().Equal(&mockOutgoingWebhook, printer.GetLines()[1])
 	})
 
 	s.Run("List webhooks by team", func() {
 		printer.Clean()
+		s.mockCtrl = gomock.NewController(s.T())
+		s.client = mocks.NewMockClient(s.mockCtrl)
 
 		mockTeam := model.Team{
 			Id: teamID,
 		}
-		mockIncomingWebhook := model.IncomingWebhook{
+
-			Id:          incomingWebhookID,
-			DisplayName: incomingWebhookDisplayName,
-		}
-		mockOutgoingWebhook := model.OutgoingWebhook{
-			Id:          outgoingWebhookID,
-			DisplayName: outgoingWebhookDisplayName,
-		}
 		s.client.
 			EXPECT().
 			GetTeam(context.TODO(), teamID, "").
@@ -92,26 +140,57 @@ func (s *MmctlUnitTestSuite) TestListWebhookCmd() {
 
 		s.client.
 			EXPECT().
-			GetIncomingWebhooksForTeam(context.TODO(), teamID, 0, 100000000, "").
+			GetIncomingWebhooksForTeam(context.TODO(), teamID, 0, 200, "").
-			Return([]*model.IncomingWebhook{&mockIncomingWebhook}, &model.Response{}, nil).
+			Return(mockIncomingWebhooksPage1, &model.Response{}, nil).
 			Times(1)
 
 		s.client.
 			EXPECT().
-			GetOutgoingWebhooksForTeam(context.TODO(), teamID, 0, 100000000, "").
+			GetIncomingWebhooksForTeam(context.TODO(), teamID, 1, 200, "").
-			Return([]*model.OutgoingWebhook{&mockOutgoingWebhook}, &model.Response{}, nil).
+			Return(mockIncomingWebhooksPage2, &model.Response{}, nil).
+			Times(1)
+
+		s.client.
+			EXPECT().
+			GetIncomingWebhooksForTeam(context.TODO(), teamID, 2, 200, "").
+			Return(nil, &model.Response{}, nil).
+			Times(1)
+
+		s.client.
+			EXPECT().
+			GetOutgoingWebhooksForTeam(context.TODO(), teamID, 0, 200, "").
+			Return(mockOutgoingWebhooksPage1, &model.Response{}, nil).
+			Times(1)
+
+		s.client.
+			EXPECT().
+			GetOutgoingWebhooksForTeam(context.TODO(), teamID, 1, 200, "").
+			Return(mockOutgoingWebhooksPage2, &model.Response{}, nil).
+			Times(1)
+
+		s.client.
+			EXPECT().
+			GetOutgoingWebhooksForTeam(context.TODO(), teamID, 2, 200, "").
+			Return(nil, &model.Response{}, nil).
 			Times(1)
 
 		err := listWebhookCmdF(s.client, &cobra.Command{}, []string{teamID})
 		s.Require().Nil(err)
-		s.Len(printer.GetLines(), 2)
 		s.Len(printer.GetErrorLines(), 0)
-		s.Require().Equal(&mockIncomingWebhook, printer.GetLines()[0])
+		if s.Len(printer.GetLines(), 500) {
-		s.Require().Equal(&mockOutgoingWebhook, printer.GetLines()[1])
+			for i := 0; i < 250; i++ {
+				s.Require().Equal(&mockIncomingWebhook, printer.GetLines()[i])
+			}
+			for i := 250; i < 500; i++ {
+				s.Require().Equal(&mockOutgoingWebhook, printer.GetLines()[i])
+			}
+		}
 	})
 
 	s.Run("Unable to list webhooks", func() {
 		printer.Clean()
+		s.mockCtrl = gomock.NewController(s.T())
+		s.client = mocks.NewMockClient(s.mockCtrl)
 
 		mockTeam := model.Team{
 			Id: teamID,
@@ -132,13 +211,13 @@ func (s *MmctlUnitTestSuite) TestListWebhookCmd() {
 
 		s.client.
 			EXPECT().
-			GetIncomingWebhooksForTeam(context.TODO(), teamID, 0, 100000000, "").
+			GetIncomingWebhooksForTeam(context.TODO(), teamID, 0, 200, "").
 			Return(nil, &model.Response{}, mockError).
 			Times(1)
 
 		s.client.
 			EXPECT().
-			GetOutgoingWebhooksForTeam(context.TODO(), teamID, 0, 100000000, "").
+			GetOutgoingWebhooksForTeam(context.TODO(), teamID, 0, 200, "").
 			Return(nil, &model.Response{}, mockError).
 			Times(1)
 
@@ -146,8 +225,8 @@ func (s *MmctlUnitTestSuite) TestListWebhookCmd() {
 		s.Require().Nil(err)
 		s.Len(printer.GetLines(), 0)
 		s.Len(printer.GetErrorLines(), 2)
-		s.Require().Equal("Unable to list incoming webhooks for '"+teamID+"'", printer.GetErrorLines()[0])
+		s.Require().Equal("Unable to list incoming webhooks for '"+teamID+"': mock error", printer.GetErrorLines()[0])
-		s.Require().Equal("Unable to list outgoing webhooks for '"+teamID+"'", printer.GetErrorLines()[1])
+		s.Require().Equal("Unable to list outgoing webhooks for '"+teamID+"': mock error", printer.GetErrorLines()[1])
 	})
 }