oppi.li / perf-test
loading up the forgejo repo on tangled to test page performance
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

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

Backport #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*

(cherry picked from commit f2d8ccc5bb2df25557cc0d4d23f2cdd029358274)

Conflicts:
models/repo/user_repo_test.go
because there is one less fixture user compared to Gitea

authored by

6543 and committed by
Earl Warren 51b8d964 60e58255

+25 -10
+6 -2
models/repo/user_repo.go
··· 95 95 // and just waste 1 unit is cheaper than re-allocate memory once. 96 96 users := make([]*user_model.User, 0, len(uniqueUserIDs)+1) 97 97 if len(userIDs) > 0 { 98 - if err = e.In("id", uniqueUserIDs.Values()).OrderBy(user_model.GetOrderByName()).Find(&users); err != nil { 98 + if err = e.In("id", uniqueUserIDs.Values()). 99 + Where(builder.Eq{"`user`.is_active": true}). 100 + OrderBy(user_model.GetOrderByName()). 101 + Find(&users); err != nil { 99 102 return nil, err 100 103 } 101 104 } ··· 117 120 return nil, err 118 121 } 119 122 120 - cond := builder.And(builder.Neq{"`user`.id": posterID}) 123 + cond := builder.And(builder.Neq{"`user`.id": posterID}). 124 + And(builder.Eq{"`user`.is_active": true}) 121 125 122 126 if repo.IsPrivate || repo.Owner.Visibility == api.VisibleTypePrivate { 123 127 // This a private repository:
+16 -7
models/repo/user_repo_test.go
··· 26 26 repo21 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 21}) 27 27 users, err = repo_model.GetRepoAssignees(db.DefaultContext, repo21) 28 28 assert.NoError(t, err) 29 - assert.Len(t, users, 3) 30 - assert.Equal(t, users[0].ID, int64(15)) 31 - assert.Equal(t, users[1].ID, int64(18)) 32 - assert.Equal(t, users[2].ID, int64(16)) 29 + if assert.Len(t, users, 3) { 30 + assert.ElementsMatch(t, []int64{15, 16, 18}, []int64{users[0].ID, users[1].ID, users[2].ID}) 31 + } 32 + 33 + // do not return deactivated users 34 + assert.NoError(t, user_model.UpdateUserCols(db.DefaultContext, &user_model.User{ID: 15, IsActive: false}, "is_active")) 35 + users, err = repo_model.GetRepoAssignees(db.DefaultContext, repo21) 36 + assert.NoError(t, err) 37 + if assert.Len(t, users, 2) { 38 + assert.NotContains(t, []int64{users[0].ID, users[1].ID}, 15) 39 + } 33 40 } 34 41 35 42 func TestRepoGetReviewers(t *testing.T) { ··· 41 48 ctx := db.DefaultContext 42 49 reviewers, err := repo_model.GetReviewers(ctx, repo1, 2, 2) 43 50 assert.NoError(t, err) 44 - assert.Len(t, reviewers, 4) 51 + if assert.Len(t, reviewers, 3) { 52 + assert.ElementsMatch(t, []int64{1, 4, 11}, []int64{reviewers[0].ID, reviewers[1].ID, reviewers[2].ID}) 53 + } 45 54 46 55 // should include doer if doer is not PR poster. 47 56 reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 2) 48 57 assert.NoError(t, err) 49 - assert.Len(t, reviewers, 4) 58 + assert.Len(t, reviewers, 3) 50 59 51 60 // should not include PR poster, if PR poster would be otherwise eligible 52 61 reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 4) 53 62 assert.NoError(t, err) 54 - assert.Len(t, reviewers, 3) 63 + assert.Len(t, reviewers, 2) 55 64 56 65 // test private user repo 57 66 repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
+3 -1
tests/integration/api_repo_test.go
··· 684 684 resp := MakeRequest(t, req, http.StatusOK) 685 685 var reviewers []*api.User 686 686 DecodeJSON(t, resp, &reviewers) 687 - assert.Len(t, reviewers, 4) 687 + if assert.Len(t, reviewers, 3) { 688 + assert.ElementsMatch(t, []int64{1, 4, 11}, []int64{reviewers[0].ID, reviewers[1].ID, reviewers[2].ID}) 689 + } 688 690 } 689 691 690 692 func TestAPIRepoGetAssignees(t *testing.T) {