Get repo assignees and reviewers should ignore deactivated users (#30770)

If an user is deactivated, it should not be in the list of users who are
suggested to be assigned or review-requested.

old assignees or reviewers are not affected.

---
*Sponsored by Kithara Software GmbH*
This commit is contained in:
6543 2024-04-30 10:43:08 +02:00 committed by GitHub
parent 059b2718a5
commit f2d8ccc5bb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 26 additions and 8 deletions

View File

@ -130,7 +130,10 @@ func GetRepoAssignees(ctx context.Context, repo *Repository) (_ []*user_model.Us
// and just waste 1 unit is cheaper than re-allocate memory once. // and just waste 1 unit is cheaper than re-allocate memory once.
users := make([]*user_model.User, 0, len(uniqueUserIDs)+1) users := make([]*user_model.User, 0, len(uniqueUserIDs)+1)
if len(userIDs) > 0 { if len(userIDs) > 0 {
if err = e.In("id", uniqueUserIDs.Values()).OrderBy(user_model.GetOrderByName()).Find(&users); err != nil { if err = e.In("id", uniqueUserIDs.Values()).
Where(builder.Eq{"`user`.is_active": true}).
OrderBy(user_model.GetOrderByName()).
Find(&users); err != nil {
return nil, err return nil, err
} }
} }
@ -152,7 +155,8 @@ func GetReviewers(ctx context.Context, repo *Repository, doerID, posterID int64)
return nil, err return nil, err
} }
cond := builder.And(builder.Neq{"`user`.id": posterID}) cond := builder.And(builder.Neq{"`user`.id": posterID}).
And(builder.Eq{"`user`.is_active": true})
if repo.IsPrivate || repo.Owner.Visibility == api.VisibleTypePrivate { if repo.IsPrivate || repo.Owner.Visibility == api.VisibleTypePrivate {
// This a private repository: // This a private repository:

View File

@ -9,6 +9,7 @@ import (
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo" repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
@ -25,10 +26,19 @@ func TestRepoAssignees(t *testing.T) {
repo21 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 21}) repo21 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 21})
users, err = repo_model.GetRepoAssignees(db.DefaultContext, repo21) users, err = repo_model.GetRepoAssignees(db.DefaultContext, repo21)
assert.NoError(t, err) assert.NoError(t, err)
assert.Len(t, users, 4) if assert.Len(t, users, 4) {
assert.ElementsMatch(t, []int64{10, 15, 16, 18}, []int64{users[0].ID, users[1].ID, users[2].ID, users[3].ID}) assert.ElementsMatch(t, []int64{10, 15, 16, 18}, []int64{users[0].ID, users[1].ID, users[2].ID, users[3].ID})
} }
// do not return deactivated users
assert.NoError(t, user_model.UpdateUserCols(db.DefaultContext, &user_model.User{ID: 15, IsActive: false}, "is_active"))
users, err = repo_model.GetRepoAssignees(db.DefaultContext, repo21)
assert.NoError(t, err)
if assert.Len(t, users, 3) {
assert.NotContains(t, []int64{users[0].ID, users[1].ID, users[2].ID}, 15)
}
}
func TestRepoGetReviewers(t *testing.T) { func TestRepoGetReviewers(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase()) assert.NoError(t, unittest.PrepareTestDatabase())
@ -38,17 +48,19 @@ func TestRepoGetReviewers(t *testing.T) {
ctx := db.DefaultContext ctx := db.DefaultContext
reviewers, err := repo_model.GetReviewers(ctx, repo1, 2, 2) reviewers, err := repo_model.GetReviewers(ctx, repo1, 2, 2)
assert.NoError(t, err) assert.NoError(t, err)
assert.Len(t, reviewers, 4) if assert.Len(t, reviewers, 3) {
assert.ElementsMatch(t, []int64{1, 4, 11}, []int64{reviewers[0].ID, reviewers[1].ID, reviewers[2].ID})
}
// should include doer if doer is not PR poster. // should include doer if doer is not PR poster.
reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 2) reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 2)
assert.NoError(t, err) assert.NoError(t, err)
assert.Len(t, reviewers, 4) assert.Len(t, reviewers, 3)
// should not include PR poster, if PR poster would be otherwise eligible // should not include PR poster, if PR poster would be otherwise eligible
reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 4) reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 4)
assert.NoError(t, err) assert.NoError(t, err)
assert.Len(t, reviewers, 3) assert.Len(t, reviewers, 2)
// test private user repo // test private user repo
repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})

View File

@ -684,7 +684,9 @@ func TestAPIRepoGetReviewers(t *testing.T) {
resp := MakeRequest(t, req, http.StatusOK) resp := MakeRequest(t, req, http.StatusOK)
var reviewers []*api.User var reviewers []*api.User
DecodeJSON(t, resp, &reviewers) DecodeJSON(t, resp, &reviewers)
assert.Len(t, reviewers, 4) if assert.Len(t, reviewers, 3) {
assert.ElementsMatch(t, []int64{1, 4, 11}, []int64{reviewers[0].ID, reviewers[1].ID, reviewers[2].ID})
}
} }
func TestAPIRepoGetAssignees(t *testing.T) { func TestAPIRepoGetAssignees(t *testing.T) {