MM-12251: Add flag to MoveChannel to remove all deactivated users. (#9515)

8 files changed, 162 insertions, 6 deletions

diff --git a/app/channel.go b/app/channel.go
index 54b589175..93037cf05 100644
--- a/app/channel.go
+++ b/app/channel.go
@@ -1670,7 +1670,13 @@ func (a *App) PermanentDeleteChannel(channel *model.Channel) *model.AppError {
 
 // This function is intended for use from the CLI. It is not robust against people joining the channel while the move
 // is in progress, and therefore should not be used from the API without first fixing this potential race condition.
-func (a *App) MoveChannel(team *model.Team, channel *model.Channel, user *model.User) *model.AppError {
+func (a *App) MoveChannel(team *model.Team, channel *model.Channel, user *model.User, removeDeactivatedMembers bool) *model.AppError {
+	if removeDeactivatedMembers {
+		if result := <-a.Srv.Store.Channel().RemoveAllDeactivatedMembers(channel.Id); result.Err != nil {
+			return result.Err
+		}
+	}
+
 	// Check that all channel members are in the destination team.
 	channelMembers, err := a.GetChannelMembersPage(channel.Id, 0, 10000000)
 	if err != nil {
diff --git a/app/channel_test.go b/app/channel_test.go
index 0501b9406..4b09bbb78 100644
--- a/app/channel_test.go
+++ b/app/channel_test.go
@@ -98,7 +98,7 @@ func TestMoveChannel(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if err := th.App.MoveChannel(targetTeam, channel1, th.BasicUser); err == nil {
+	if err := th.App.MoveChannel(targetTeam, channel1, th.BasicUser, false); err == nil {
 		t.Fatal("Should have failed due to mismatched members.")
 	}
 
@@ -106,7 +106,34 @@ func TestMoveChannel(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if err := th.App.MoveChannel(targetTeam, channel1, th.BasicUser); err != nil {
+	if err := th.App.MoveChannel(targetTeam, channel1, th.BasicUser, false); err != nil {
+		t.Fatal(err)
+	}
+
+	// Test moving a channel with a deactivated user who isn't in the destination team.
+	// It should fail, unless removeDeactivatedMembers is true.
+	deacivatedUser := th.CreateUser()
+	channel2 := th.CreateChannel(sourceTeam)
+
+	if _, err := th.App.AddUserToTeam(sourceTeam.Id, deacivatedUser.Id, ""); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := th.App.AddUserToChannel(th.BasicUser, channel2); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := th.App.AddUserToChannel(deacivatedUser, channel2); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := th.App.UpdateActive(deacivatedUser, false); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := th.App.MoveChannel(targetTeam, channel2, th.BasicUser, false); err == nil {
+		t.Fatal("Should have failed due to mismatched deacivated member.")
+	}
+
+	if err := th.App.MoveChannel(targetTeam, channel2, th.BasicUser, true); err != nil {
 		t.Fatal(err)
 	}
 }
diff --git a/cmd/mattermost/commands/channel.go b/cmd/mattermost/commands/channel.go
index 7fb8fea8c..82fb1e966 100644
--- a/cmd/mattermost/commands/channel.go
+++ b/cmd/mattermost/commands/channel.go
@@ -117,6 +117,7 @@ func init() {
 	ChannelCreateCmd.Flags().Bool("private", false, "Create a private channel.")
 
 	MoveChannelsCmd.Flags().String("username", "", "Required. Username who is moving the channel.")
+	MoveChannelsCmd.Flags().Bool("remove-deactivated-users", false, "Automatically remove any deactivated users from the channel before moving it.")
 
 	DeleteChannelsCmd.Flags().Bool("confirm", false, "Confirm you really want to delete the channels.")
 
@@ -366,6 +367,8 @@ func moveChannelsCmdF(command *cobra.Command, args []string) error {
 	}
 	user := getUserFromUserArg(a, username)
 
+	removeDeactivatedMembers, _ := command.Flags().GetBool("remove-deactivated-users")
+
 	channels := getChannelsFromChannelArgs(a, args[1:])
 	for i, channel := range channels {
 		if channel == nil {
@@ -373,7 +376,7 @@ func moveChannelsCmdF(command *cobra.Command, args []string) error {
 			continue
 		}
 		originTeamID := channel.TeamId
-		if err := moveChannel(a, team, channel, user); err != nil {
+		if err := moveChannel(a, team, channel, user, removeDeactivatedMembers); err != nil {
 			CommandPrintErrorln("Unable to move channel '" + channel.Name + "' error: " + err.Error())
 		} else {
 			CommandPrettyPrintln("Moved channel '" + channel.Name + "' to " + team.Name + "(" + team.Id + ") from " + originTeamID + ".")
@@ -383,10 +386,10 @@ func moveChannelsCmdF(command *cobra.Command, args []string) error {
 	return nil
 }
 
-func moveChannel(a *app.App, team *model.Team, channel *model.Channel, user *model.User) *model.AppError {
+func moveChannel(a *app.App, team *model.Team, channel *model.Channel, user *model.User, removeDeactivatedMembers bool) *model.AppError {
 	oldTeamId := channel.TeamId
 
-	if err := a.MoveChannel(team, channel, user); err != nil {
+	if err := a.MoveChannel(team, channel, user, removeDeactivatedMembers); err != nil {
 		return err
 	}
 
diff --git a/i18n/en.json b/i18n/en.json
index 7e74e4d89..7d8bd4122 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -4835,6 +4835,10 @@
     "translation": "Unable to set the user status. Unknown user status."
   },
   {
+    "id": "store.sql_channel.remove_all_deactivated_members.app_error",
+    "translation": "We could not remove the deactivated users from the channel"
+  },
+  {
     "id": "store.sql.convert_string_array",
     "translation": "FromDb: Unable to convert StringArray to *string"
   },
diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go
index 7a9e4e687..b1886c428 100644
--- a/store/sqlstore/channel_store.go
+++ b/store/sqlstore/channel_store.go
@@ -1478,6 +1478,32 @@ func (s SqlChannelStore) RemoveMember(channelId string, userId string) store.Sto
 	})
 }
 
+func (s SqlChannelStore) RemoveAllDeactivatedMembers(channelId string) store.StoreChannel {
+	return store.Do(func(result *store.StoreResult) {
+		query := `
+			DELETE
+			FROM
+				ChannelMembers
+			WHERE 
+				UserId IN (
+					SELECT
+						Id
+					FROM
+						Users
+					WHERE
+						Users.DeleteAt != 0
+				)
+			AND
+				ChannelMembers.ChannelId = :ChannelId
+		`
+
+		_, err := s.GetMaster().Exec(query, map[string]interface{}{"ChannelId": channelId})
+		if err != nil {
+			result.Err = model.NewAppError("SqlChannelStore.RemoveAllDeactivatedMembers", "store.sql_channel.remove_all_deactivated_members.app_error", nil, "channel_id="+channelId+", "+err.Error(), http.StatusInternalServerError)
+		}
+	})
+}
+
 func (s SqlChannelStore) PermanentDeleteMembersByUser(userId string) store.StoreChannel {
 	return store.Do(func(result *store.StoreResult) {
 		if _, err := s.GetMaster().Exec("DELETE FROM ChannelMembers WHERE UserId = :UserId", map[string]interface{}{"UserId": userId}); err != nil {
diff --git a/store/store.go b/store/store.go
index dd49e0c97..8a2051527 100644
--- a/store/store.go
+++ b/store/store.go
@@ -184,6 +184,7 @@ type ChannelStore interface {
 	IsExperimentalPublicChannelsMaterializationEnabled() bool
 	GetAllChannelsForExportAfter(limit int, afterId string) StoreChannel
 	GetChannelMembersForExport(userId string, teamId string) StoreChannel
+	RemoveAllDeactivatedMembers(channelId string) StoreChannel
 }
 
 type ChannelMemberHistoryStore interface {
diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go
index 7cb86242f..34efd810d 100644
--- a/store/storetest/channel_store.go
+++ b/store/storetest/channel_store.go
@@ -80,6 +80,7 @@ func TestChannelStore(t *testing.T, ss store.Store, s SqlSupplier) {
 			t.Run("MaterializedPublicChannels", func(t *testing.T) { testMaterializedPublicChannels(t, ss, s) })
 			t.Run("GetAllChannelsForExportAfter", func(t *testing.T) { testChannelStoreGetAllChannelsForExportAfter(t, ss) })
 			t.Run("GetChannelMembersForExport", func(t *testing.T) { testChannelStoreGetChannelMembersForExport(t, ss) })
+			t.Run("RemoveAllDeactivatedMembers", func(t *testing.T) { testChannelStoreRemoveAllDeactivatedMembers(t, ss) })
 		})
 	}
 }
@@ -2806,3 +2807,75 @@ func testChannelStoreGetChannelMembersForExport(t *testing.T, ss store.Store) {
 	assert.Equal(t, c1.Id, cmfe1.ChannelId)
 	assert.Equal(t, u1.Id, cmfe1.UserId)
 }
+
+func testChannelStoreRemoveAllDeactivatedMembers(t *testing.T, ss store.Store) {
+	// Set up all the objects needed in the store.
+	t1 := model.Team{}
+	t1.DisplayName = "Name"
+	t1.Name = model.NewId()
+	t1.Email = MakeEmail()
+	t1.Type = model.TEAM_OPEN
+	store.Must(ss.Team().Save(&t1))
+
+	c1 := model.Channel{}
+	c1.TeamId = t1.Id
+	c1.DisplayName = "Channel1"
+	c1.Name = "zz" + model.NewId() + "b"
+	c1.Type = model.CHANNEL_OPEN
+	store.Must(ss.Channel().Save(&c1, -1))
+
+	u1 := model.User{}
+	u1.Email = MakeEmail()
+	u1.Nickname = model.NewId()
+	store.Must(ss.User().Save(&u1))
+
+	u2 := model.User{}
+	u2.Email = MakeEmail()
+	u2.Nickname = model.NewId()
+	store.Must(ss.User().Save(&u2))
+
+	u3 := model.User{}
+	u3.Email = MakeEmail()
+	u3.Nickname = model.NewId()
+	store.Must(ss.User().Save(&u3))
+
+	m1 := model.ChannelMember{}
+	m1.ChannelId = c1.Id
+	m1.UserId = u1.Id
+	m1.NotifyProps = model.GetDefaultChannelNotifyProps()
+	store.Must(ss.Channel().SaveMember(&m1))
+
+	m2 := model.ChannelMember{}
+	m2.ChannelId = c1.Id
+	m2.UserId = u2.Id
+	m2.NotifyProps = model.GetDefaultChannelNotifyProps()
+	store.Must(ss.Channel().SaveMember(&m2))
+
+	m3 := model.ChannelMember{}
+	m3.ChannelId = c1.Id
+	m3.UserId = u3.Id
+	m3.NotifyProps = model.GetDefaultChannelNotifyProps()
+	store.Must(ss.Channel().SaveMember(&m3))
+
+	// Get all the channel members. Check there are 3.
+	r1 := <-ss.Channel().GetMembers(c1.Id, 0, 1000)
+	assert.Nil(t, r1.Err)
+	d1 := r1.Data.(*model.ChannelMembers)
+	assert.Len(t, *d1, 3)
+
+	// Deactivate users 1 & 2.
+	u1.DeleteAt = model.GetMillis()
+	u2.DeleteAt = model.GetMillis()
+	require.Nil(t, (<-ss.User().Update(&u1, true)).Err)
+	require.Nil(t, (<-ss.User().Update(&u2, true)).Err)
+
+	// Remove all deactivated users from the channel.
+	assert.Nil(t, (<-ss.Channel().RemoveAllDeactivatedMembers(c1.Id)).Err)
+
+	// Get all the channel members. Check there is now only 1: m3.
+	r2 := <-ss.Channel().GetMembers(c1.Id, 0, 1000)
+	assert.Nil(t, r1.Err)
+	d2 := r2.Data.(*model.ChannelMembers)
+	assert.Len(t, *d2, 1)
+	assert.Equal(t, (*d2)[0].UserId, u3.Id)
+}
diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go
index 9db85eacf..1c3a2a8f2 100644
--- a/store/storetest/mocks/ChannelStore.go
+++ b/store/storetest/mocks/ChannelStore.go
@@ -779,6 +779,22 @@ func (_m *ChannelStore) PermanentDeleteMembersByUser(userId string) store.StoreC
 	return r0
 }
 
+// RemoveAllDeactivatedMembers provides a mock function with given fields: channelId
+func (_m *ChannelStore) RemoveAllDeactivatedMembers(channelId string) store.StoreChannel {
+	ret := _m.Called(channelId)
+
+	var r0 store.StoreChannel
+	if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok {
+		r0 = rf(channelId)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(store.StoreChannel)
+		}
+	}
+
+	return r0
+}
+
 // RemoveMember provides a mock function with given fields: channelId, userId
 func (_m *ChannelStore) RemoveMember(channelId string, userId string) store.StoreChannel {
 	ret := _m.Called(channelId, userId)