Fix remove org user failure on mssql (#34449) (#34453)

Backport #34449 by @ChristopherHX

* mssql does not support fetching 0 repositories
* remove paging by NumRepos that might be 0
* extend admin api test to purge user 2

Fixes #34448

Co-authored-by: ChristopherHX <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>

Author: 3 people

models/activities/action.go

@@ -530,7 +530,7 @@ func ActivityQueryCondition(ctx context.Context, opts GetFeedsOptions) (builder.
 
 	if opts.RequestedTeam != nil {
 		env := repo_model.AccessibleTeamReposEnv(organization.OrgFromUser(opts.RequestedUser), opts.RequestedTeam)
-		teamRepoIDs, err := env.RepoIDs(ctx, 1, opts.RequestedUser.NumRepos)
+		teamRepoIDs, err := env.RepoIDs(ctx)
 		if err != nil {
 			return nil, fmt.Errorf("GetTeamRepositories: %w", err)
 		}

models/organization/org_test.go

@@ -334,33 +334,14 @@ func TestAccessibleReposEnv_RepoIDs(t *testing.T) {
 	testSuccess := func(userID int64, expectedRepoIDs []int64) {
 		env, err := repo_model.AccessibleReposEnv(db.DefaultContext, org, userID)
 		assert.NoError(t, err)
-		repoIDs, err := env.RepoIDs(db.DefaultContext, 1, 100)
+		repoIDs, err := env.RepoIDs(db.DefaultContext)
 		assert.NoError(t, err)
 		assert.Equal(t, expectedRepoIDs, repoIDs)
 	}
 	testSuccess(2, []int64{3, 5, 32})
 	testSuccess(4, []int64{3, 32})
 }
 
-func TestAccessibleReposEnv_Repos(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-	org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3})
-	testSuccess := func(userID int64, expectedRepoIDs []int64) {
-		env, err := repo_model.AccessibleReposEnv(db.DefaultContext, org, userID)
-		assert.NoError(t, err)
-		repos, err := env.Repos(db.DefaultContext, 1, 100)
-		assert.NoError(t, err)
-		expectedRepos := make(repo_model.RepositoryList, len(expectedRepoIDs))
-		for i, repoID := range expectedRepoIDs {
-			expectedRepos[i] = unittest.AssertExistsAndLoadBean(t,
-				&repo_model.Repository{ID: repoID})
-		}
-		assert.Equal(t, expectedRepos, repos)
-	}
-	testSuccess(2, []int64{3, 5, 32})
-	testSuccess(4, []int64{3, 32})
-}
-
 func TestAccessibleReposEnv_MirrorRepos(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3})

models/repo/org_repo.go

@@ -48,8 +48,7 @@ func GetTeamRepositories(ctx context.Context, opts *SearchTeamRepoOptions) (Repo
 // accessible to a particular user
 type AccessibleReposEnvironment interface {
 	CountRepos(ctx context.Context) (int64, error)
-	RepoIDs(ctx context.Context, page, pageSize int) ([]int64, error)
-	Repos(ctx context.Context, page, pageSize int) (RepositoryList, error)
+	RepoIDs(ctx context.Context) ([]int64, error)
 	MirrorRepos(ctx context.Context) (RepositoryList, error)
 	AddKeyword(keyword string)
 	SetSort(db.SearchOrderBy)
@@ -132,40 +131,18 @@ func (env *accessibleReposEnv) CountRepos(ctx context.Context) (int64, error) {
 	return repoCount, nil
 }
 
-func (env *accessibleReposEnv) RepoIDs(ctx context.Context, page, pageSize int) ([]int64, error) {
-	if page <= 0 {
-		page = 1
-	}
-
-	repoIDs := make([]int64, 0, pageSize)
+func (env *accessibleReposEnv) RepoIDs(ctx context.Context) ([]int64, error) {
+	var repoIDs []int64
 	return repoIDs, db.GetEngine(ctx).
 		Table("repository").
 		Join("INNER", "team_repo", "`team_repo`.repo_id=`repository`.id").
 		Where(env.cond()).
-		GroupBy("`repository`.id,`repository`."+strings.Fields(string(env.orderBy))[0]).
+		GroupBy("`repository`.id,`repository`." + strings.Fields(string(env.orderBy))[0]).
 		OrderBy(string(env.orderBy)).
-		Limit(pageSize, (page-1)*pageSize).
 		Cols("`repository`.id").
 		Find(&repoIDs)
 }
 
-func (env *accessibleReposEnv) Repos(ctx context.Context, page, pageSize int) (RepositoryList, error) {
-	repoIDs, err := env.RepoIDs(ctx, page, pageSize)
-	if err != nil {
-		return nil, fmt.Errorf("GetUserRepositoryIDs: %w", err)
-	}
-
-	repos := make([]*Repository, 0, len(repoIDs))
-	if len(repoIDs) == 0 {
-		return repos, nil
-	}
-
-	return repos, db.GetEngine(ctx).
-		In("`repository`.id", repoIDs).
-		OrderBy(string(env.orderBy)).
-		Find(&repos)
-}
-
 func (env *accessibleReposEnv) MirrorRepoIDs(ctx context.Context) ([]int64, error) {
 	repoIDs := make([]int64, 0, 10)
 	return repoIDs, db.GetEngine(ctx).

services/org/user.go

@@ -64,10 +64,11 @@ func RemoveOrgUser(ctx context.Context, org *organization.Organization, user *us
 	if err != nil {
 		return fmt.Errorf("AccessibleReposEnv: %w", err)
 	}
-	repoIDs, err := env.RepoIDs(ctx, 1, org.NumRepos)
+	repoIDs, err := env.RepoIDs(ctx)
 	if err != nil {
 		return fmt.Errorf("GetUserRepositories [%d]: %w", user.ID, err)
 	}
+
 	for _, repoID := range repoIDs {
 		repo, err := repo_model.GetRepositoryByID(ctx, repoID)
 		if err != nil {

tests/integration/admin_user_test.go

@@ -4,6 +4,7 @@
 package integration
 
 import (
+	"fmt"
 	"net/http"
 	"strconv"
 	"testing"
@@ -72,12 +73,37 @@ func TestAdminDeleteUser(t *testing.T) {
 
 	session := loginUser(t, "user1")
 
-	csrf := GetUserCSRFToken(t, session)
-	req := NewRequestWithValues(t, "POST", "/-/admin/users/8/delete", map[string]string{
-		"_csrf": csrf,
-	})
-	session.MakeRequest(t, req, http.StatusSeeOther)
-
-	assertUserDeleted(t, 8)
-	unittest.CheckConsistencyFor(t, &user_model.User{})
+	usersToDelete := []struct {
+		userID int64
+		purge  bool
+	}{
+		{
+			userID: 2,
+			purge:  true,
+		},
+		{
+			userID: 8,
+		},
+	}
+
+	for _, entry := range usersToDelete {
+		t.Run(fmt.Sprintf("DeleteUser%d", entry.userID), func(t *testing.T) {
+			user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: entry.userID})
+			assert.NotNil(t, user)
+
+			var query string
+			if entry.purge {
+				query = "?purge=true"
+			}
+
+			csrf := GetUserCSRFToken(t, session)
+			req := NewRequestWithValues(t, "POST", fmt.Sprintf("/-/admin/users/%d/delete%s", entry.userID, query), map[string]string{
+				"_csrf": csrf,
+			})
+			session.MakeRequest(t, req, http.StatusSeeOther)
+
+			assertUserDeleted(t, entry.userID)
+			unittest.CheckConsistencyFor(t, &user_model.User{})
+		})
+	}
 }