PLT-6752/PLT-6755 Fixed users removed from a team counting against max team members (#6578)

* PLT-6752 Fixed users removed from a team counting against max team members

* Updated unit tests for max team members

* Fixed being able to have MaxUserPerTeam+1 users in a team

store/sql_team_store.go

@@ -489,12 +489,19 @@ func (s SqlTeamStore) SaveMember(member *model.TeamMember) StoreChannel {
 			return
 		}
 
-		if count, err := s.GetMaster().SelectInt("SELECT COUNT(0) FROM TeamMembers WHERE TeamId = :TeamId", map[string]interface{}{"TeamId": member.TeamId}); err != nil {
+		if count, err := s.GetMaster().SelectInt(
+			`SELECT
+				COUNT(0)
+			FROM
+				TeamMembers
+			WHERE
+				TeamId = :TeamId
+				AND DeleteAt = 0`, map[string]interface{}{"TeamId": member.TeamId}); err != nil {
 			result.Err = model.NewLocAppError("SqlUserStore.Save", "store.sql_user.save.member_count.app_error", nil, "teamId="+member.TeamId+", "+err.Error())
 			storeChannel <- result
 			close(storeChannel)
 			return
-		} else if int(count) > utils.Cfg.TeamSettings.MaxUsersPerTeam {
+		} else if int(count) >= utils.Cfg.TeamSettings.MaxUsersPerTeam {
 			result.Err = model.NewLocAppError("SqlUserStore.Save", "store.sql_user.save.max_accounts.app_error", nil, "teamId="+member.TeamId)
 			storeChannel <- result
 			close(storeChannel)

store/sql_team_store_test.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/mattermost/platform/model"
+	"github.com/mattermost/platform/utils"
 )
 
 func TestTeamStoreSave(t *testing.T) {
@@ -552,6 +553,101 @@ func TestTeamMembers(t *testing.T) {
 	}
 }
 
+func TestSaveTeamMemberMaxMembers(t *testing.T) {
+	Setup()
+
+	MaxUsersPerTeam := utils.Cfg.TeamSettings.MaxUsersPerTeam
+	defer func() {
+		utils.Cfg.TeamSettings.MaxUsersPerTeam = MaxUsersPerTeam
+	}()
+	utils.Cfg.TeamSettings.MaxUsersPerTeam = 5
+
+	team := Must(store.Team().Save(&model.Team{
+		DisplayName: "DisplayName",
+		Name:        "z-z-z" + model.NewId() + "b",
+		Type:        model.TEAM_OPEN,
+	})).(*model.Team)
+	defer func() {
+		<-store.Team().PermanentDelete(team.Id)
+	}()
+
+	userIds := make([]string, utils.Cfg.TeamSettings.MaxUsersPerTeam)
+
+	for i := 0; i < utils.Cfg.TeamSettings.MaxUsersPerTeam; i++ {
+		userIds[i] = Must(store.User().Save(&model.User{
+			Username: model.NewId(),
+			Email:    model.NewId(),
+		})).(*model.User).Id
+
+		defer func(userId string) {
+			<-store.User().PermanentDelete(userId)
+		}(userIds[i])
+
+		Must(store.Team().SaveMember(&model.TeamMember{
+			TeamId: team.Id,
+			UserId: userIds[i],
+		}))
+
+		defer func(userId string) {
+			<-store.Team().RemoveMember(team.Id, userId)
+		}(userIds[i])
+	}
+
+	if result := <-store.Team().GetTotalMemberCount(team.Id); result.Err != nil {
+		t.Fatal(result.Err)
+	} else if count := result.Data.(int64); int(count) != utils.Cfg.TeamSettings.MaxUsersPerTeam {
+		t.Fatalf("should start with 5 team members, had %v instead", count)
+	}
+
+	newUserId := Must(store.User().Save(&model.User{
+		Username: model.NewId(),
+		Email:    model.NewId(),
+	})).(*model.User).Id
+	defer func() {
+		<-store.User().PermanentDelete(newUserId)
+	}()
+
+	if result := <-store.Team().SaveMember(&model.TeamMember{
+		TeamId: team.Id,
+		UserId: newUserId,
+	}); result.Err == nil {
+		t.Fatal("shouldn't be able to save member when at maximum members per team")
+	}
+
+	if result := <-store.Team().GetTotalMemberCount(team.Id); result.Err != nil {
+		t.Fatal(result.Err)
+	} else if count := result.Data.(int64); int(count) != utils.Cfg.TeamSettings.MaxUsersPerTeam {
+		t.Fatalf("should still have 5 team members, had %v instead", count)
+	}
+
+	// Leaving the team from the UI sets DeleteAt instead of using TeamStore.RemoveMember
+	Must(store.Team().UpdateMember(&model.TeamMember{
+		TeamId:   team.Id,
+		UserId:   userIds[0],
+		DeleteAt: 1234,
+	}))
+
+	if result := <-store.Team().GetTotalMemberCount(team.Id); result.Err != nil {
+		t.Fatal(result.Err)
+	} else if count := result.Data.(int64); int(count) != utils.Cfg.TeamSettings.MaxUsersPerTeam-1 {
+		t.Fatalf("should now only have 4 team members, had %v instead", count)
+	}
+
+	if result := <-store.Team().SaveMember(&model.TeamMember{TeamId: team.Id, UserId: newUserId}); result.Err != nil {
+		t.Fatal("should've been able to save new member after deleting one", result.Err)
+	} else {
+		defer func(userId string) {
+			<-store.Team().RemoveMember(team.Id, userId)
+		}(newUserId)
+	}
+
+	if result := <-store.Team().GetTotalMemberCount(team.Id); result.Err != nil {
+		t.Fatal(result.Err)
+	} else if count := result.Data.(int64); int(count) != utils.Cfg.TeamSettings.MaxUsersPerTeam {
+		t.Fatalf("should still have 5 team members again, had %v instead", count)
+	}
+}
+
 func TestGetTeamMember(t *testing.T) {
 	Setup()
 

store/sql_user_store_test.go

@@ -46,7 +46,7 @@ func TestUserStoreSave(t *testing.T) {
 		t.Fatal("should be unique username")
 	}
 
-	for i := 0; i < 50; i++ {
+	for i := 0; i < 49; i++ {
 		u1.Id = ""
 		u1.Email = model.NewId()
 		u1.Username = model.NewId()