From 00d48b04ed9577a04e9cbbf6ab10aae54246fe85 Mon Sep 17 00:00:00 2001 From: Anna Os Date: Mon, 25 Mar 2024 12:53:22 +0100 Subject: [PATCH] [GH-21216] Function removeUserFromChannel return error (#26428) --- server/cmd/mmctl/commands/channel_users.go | 34 +++++++---- .../mmctl/commands/channel_users_e2e_test.go | 31 ++++++---- .../cmd/mmctl/commands/channel_users_test.go | 60 +++++++++++++++++-- server/public/model/manifest.go | 2 +- 4 files changed, 97 insertions(+), 30 deletions(-) diff --git a/server/cmd/mmctl/commands/channel_users.go b/server/cmd/mmctl/commands/channel_users.go index d87b954a24..acbfa7672f 100644 --- a/server/cmd/mmctl/commands/channel_users.go +++ b/server/cmd/mmctl/commands/channel_users.go @@ -60,22 +60,27 @@ func channelUsersAddCmdF(c client.Client, cmd *cobra.Command, args []string) err return errors.Errorf("unable to find channel %q", args[0]) } + var result *multierror.Error users := getUsersFromUserArgs(c, args[1:]) for i, user := range users { - addUserToChannel(c, channel, user, args[i+1]) + err := addUserToChannel(c, channel, user, args[i+1]) + if err != nil { + printer.PrintError(err.Error()) + result = multierror.Append(result, err) + } } - return nil + return result.ErrorOrNil() } -func addUserToChannel(c client.Client, channel *model.Channel, user *model.User, userArg string) { +func addUserToChannel(c client.Client, channel *model.Channel, user *model.User, userArg string) error { if user == nil { - printer.PrintError("Can't find user '" + userArg + "'") - return + return fmt.Errorf("unable to find user %q", userArg) } if _, _, err := c.AddChannelMember(context.TODO(), channel.Id, user.Id); err != nil { - printer.PrintError("Unable to add '" + userArg + "' to " + channel.Name + ". Error: " + err.Error()) + return fmt.Errorf("unable to add %q to %q. Error: %w", userArg, channel.Name, err) } + return nil } func channelUsersRemoveCmdF(c client.Client, cmd *cobra.Command, args []string) error { @@ -94,27 +99,32 @@ func channelUsersRemoveCmdF(c client.Client, cmd *cobra.Command, args []string) return errors.Errorf("unable to find channel %q", args[0]) } + var result *multierror.Error if allUsers { if err := removeAllUsersFromChannel(c, channel); err != nil { return err } } else { for i, user := range getUsersFromUserArgs(c, args[1:]) { - removeUserFromChannel(c, channel, user, args[i+1]) + err := removeUserFromChannel(c, channel, user, args[i+1]) + if err != nil { + printer.PrintError(err.Error()) + result = multierror.Append(result, err) + } } } - return nil + return result.ErrorOrNil() } -func removeUserFromChannel(c client.Client, channel *model.Channel, user *model.User, userArg string) { +func removeUserFromChannel(c client.Client, channel *model.Channel, user *model.User, userArg string) error { if user == nil { - printer.PrintError("Can't find user '" + userArg + "'") - return + return fmt.Errorf("unable to find user %q", userArg) } if _, err := c.RemoveUserFromChannel(context.TODO(), channel.Id, user.Id); err != nil { - printer.PrintError("Unable to remove '" + userArg + "' from " + channel.Name + ". Error: " + err.Error()) + return fmt.Errorf("unable to remove %q from %q. Error: %w", userArg, channel.Name, err) } + return nil } func removeAllUsersFromChannel(c client.Client, channel *model.Channel) error { diff --git a/server/cmd/mmctl/commands/channel_users_e2e_test.go b/server/cmd/mmctl/commands/channel_users_e2e_test.go index a01ad8b332..51990c3cc5 100644 --- a/server/cmd/mmctl/commands/channel_users_e2e_test.go +++ b/server/cmd/mmctl/commands/channel_users_e2e_test.go @@ -67,10 +67,10 @@ func (s *MmctlE2ETestSuite) TestChannelUsersAddCmdF() { nonexistentUserName := "nonexistent" err := channelUsersAddCmdF(c, &cobra.Command{}, []string{channel.Id, nonexistentUserName}) - s.Require().Nil(err) + s.Require().ErrorContains(err, "unable to find user") + s.Require().ErrorContains(err, nonexistentUserName) s.Require().Len(printer.GetLines(), 0) s.Require().Len(printer.GetErrorLines(), 1) - s.Require().Equal(fmt.Sprintf("Can't find user '%s'", nonexistentUserName), printer.GetErrorLines()[0]) }) s.Run("Add nonexistent user to channel/Client", func() { @@ -85,20 +85,23 @@ func (s *MmctlE2ETestSuite) TestChannelUsersAddCmdF() { nonexistentUserName := "nonexistent" err := channelUsersAddCmdF(s.th.Client, &cobra.Command{}, []string{channel.Id, nonexistentUserName}) - s.Require().Nil(err) + s.Require().ErrorContains(err, "unable to find user") + s.Require().ErrorContains(err, nonexistentUserName) s.Require().Len(printer.GetLines(), 0) s.Require().Len(printer.GetErrorLines(), 1) - s.Require().Equal(fmt.Sprintf("Can't find user '%s'", nonexistentUserName), printer.GetErrorLines()[0]) }) s.Run("Add user to channel without permission/Client", func() { printer.Clean() err := channelUsersAddCmdF(s.th.Client, &cobra.Command{}, []string{channel.Id, user.Id}) - s.Require().Nil(err) + s.Require().ErrorContains(err, "unable to add") + s.Require().ErrorContains(err, user.Id) + s.Require().ErrorContains(err, channelName) + s.Require().ErrorContains(err, "You do not have the appropriate permissions") s.Require().Len(printer.GetLines(), 0) s.Require().Len(printer.GetErrorLines(), 1) - s.Require().Equal(fmt.Sprintf("Unable to add '%s' to %s. Error: You do not have the appropriate permissions.", user.Id, channelName), printer.GetErrorLines()[0]) + s.Require().Equal(fmt.Sprintf("unable to add %q to %q. Error: You do not have the appropriate permissions.", user.Id, channelName), printer.GetErrorLines()[0]) }) s.Run("Add user to channel/Client", func() { @@ -197,10 +200,11 @@ func (s *MmctlE2ETestSuite) TestChannelUsersRemoveCmd() { nonexistentUserName := "nonexistent" err := channelUsersRemoveCmdF(c, &cobra.Command{}, []string{channel.Id, nonexistentUserName}) - s.Require().Nil(err) + s.Require().ErrorContains(err, "unable to find user") + s.Require().ErrorContains(err, nonexistentUserName) s.Require().Len(printer.GetLines(), 0) s.Require().Len(printer.GetErrorLines(), 1) - s.Require().Equal(fmt.Sprintf("Can't find user '%s'", nonexistentUserName), printer.GetErrorLines()[0]) + s.Require().Equal(fmt.Sprintf("unable to find user %q", nonexistentUserName), printer.GetErrorLines()[0]) }) s.Run("Remove nonexistent user from channel/Client", func() { @@ -215,10 +219,11 @@ func (s *MmctlE2ETestSuite) TestChannelUsersRemoveCmd() { nonexistentUserName := "nonexistent" err := channelUsersRemoveCmdF(s.th.Client, &cobra.Command{}, []string{channel.Id, nonexistentUserName}) - s.Require().Nil(err) + s.Require().ErrorContains(err, "unable to find user") + s.Require().ErrorContains(err, nonexistentUserName) s.Require().Len(printer.GetLines(), 0) s.Require().Len(printer.GetErrorLines(), 1) - s.Require().Equal(fmt.Sprintf("Can't find user '%s'", nonexistentUserName), printer.GetErrorLines()[0]) + s.Require().Equal(fmt.Sprintf("unable to find user %q", nonexistentUserName), printer.GetErrorLines()[0]) }) s.Run("Remove user from channel without permission/Client", func() { @@ -233,10 +238,12 @@ func (s *MmctlE2ETestSuite) TestChannelUsersRemoveCmd() { s.Require().Equal(user.Id, (members)[0].UserId) err := channelUsersRemoveCmdF(s.th.Client, &cobra.Command{}, []string{channel.Id, user.Id}) - s.Require().Nil(err) + s.Require().ErrorContains(err, "unable to remove") + s.Require().ErrorContains(err, user.Id) + s.Require().ErrorContains(err, channelName) s.Require().Len(printer.GetLines(), 0) s.Require().Len(printer.GetErrorLines(), 1) - s.Require().Contains(printer.GetErrorLines()[0], fmt.Sprintf("Unable to remove '%s' from %s", user.Id, channelName)) + s.Require().Contains(printer.GetErrorLines()[0], fmt.Sprintf("unable to remove %q from %q", user.Id, channelName)) s.Require().Contains(printer.GetErrorLines()[0], "You do not have the appropriate permissions") }) diff --git a/server/cmd/mmctl/commands/channel_users_test.go b/server/cmd/mmctl/commands/channel_users_test.go index 1853b59f00..2e1e338b64 100644 --- a/server/cmd/mmctl/commands/channel_users_test.go +++ b/server/cmd/mmctl/commands/channel_users_test.go @@ -159,10 +159,10 @@ func (s *MmctlUnitTestSuite) TestChannelUsersAddCmdF() { Return(&model.ChannelMember{}, &model.Response{}, nil). Times(1) err := channelUsersAddCmdF(s.client, cmd, []string{channelArg, nilUserArg, userEmail}) - s.Require().Nil(err) + s.Require().ErrorContains(err, "unable to find user") + s.Require().ErrorContains(err, nilUserArg) s.Len(printer.GetLines(), 0) s.Len(printer.GetErrorLines(), 1) - s.Equal("Can't find user '"+nilUserArg+"'", printer.GetErrorLines()[0]) }) s.Run("Error adding existing user to existing channel", func() { printer.Clean() @@ -191,11 +191,11 @@ func (s *MmctlUnitTestSuite) TestChannelUsersAddCmdF() { Return(nil, &model.Response{}, errors.New("mock error")). Times(1) err := channelUsersAddCmdF(s.client, cmd, []string{channelArg, userEmail}) - s.Require().Nil(err) + s.Require().ErrorContains(err, "unable to add") + s.Require().ErrorContains(err, userEmail) + s.Require().ErrorContains(err, channelName) s.Len(printer.GetLines(), 0) s.Len(printer.GetErrorLines(), 1) - s.Equal("Unable to add '"+userEmail+"' to "+channelName+". Error: mock error", - printer.GetErrorLines()[0]) }) } @@ -439,4 +439,54 @@ func (s *MmctlUnitTestSuite) TestChannelUsersRemoveCmd() { s.Require().Len(printer.GetLines(), 0) s.Require().Len(printer.GetErrorLines(), 1) }) + + s.Run("should remove user from channel throws error", func() { + printer.Clean() + + cmd := &cobra.Command{} + args := []string{argsTeamChannel, userEmail} + + foundTeam := &model.Team{ + Id: teamID, + DisplayName: teamDisplayName, + Name: teamName, + } + + foundChannel := &model.Channel{ + Id: channelID, + Name: channelName, + DisplayName: channelDisplayName, + } + + s.client. + EXPECT(). + GetTeam(context.TODO(), teamName, ""). + Return(foundTeam, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + GetChannelByNameIncludeDeleted(context.TODO(), channelName, foundTeam.Id, ""). + Return(foundChannel, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + GetUserByEmail(context.TODO(), userEmail, ""). + Return(&mockUser, &model.Response{}, nil). + Times(1) + + s.client. + EXPECT(). + RemoveUserFromChannel(context.TODO(), foundChannel.Id, mockUser.Id). + Return(&model.Response{StatusCode: http.StatusNotFound}, errors.New("mock error")). + Times(1) + + err := channelUsersRemoveCmdF(s.client, cmd, args) + s.Require().ErrorContains(err, "unable to remove") + s.Require().ErrorContains(err, userEmail) + s.Require().ErrorContains(err, channelName) + s.Require().Len(printer.GetLines(), 0) + s.Require().Len(printer.GetErrorLines(), 1) + }) } diff --git a/server/public/model/manifest.go b/server/public/model/manifest.go index 415a7e8002..98c4e9abf5 100644 --- a/server/public/model/manifest.go +++ b/server/public/model/manifest.go @@ -62,7 +62,7 @@ type PluginSetting struct { // // "longtext" will result in a multi line string that can be typed in manually. // - // "number" will result in in integer setting that can be typed in manually. + // "number" will result in integer setting that can be typed in manually. // // "username" will result in a text setting that will autocomplete to a username. //