Skip to content

Commit 672e98e

Browse files
committed
Fix test
1 parent ab7cb95 commit 672e98e

File tree

5 files changed

+155
-100
lines changed

5 files changed

+155
-100
lines changed

models/repo/user_repo.go

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -143,62 +143,6 @@ func GetRepoAssignees(ctx context.Context, repo *Repository) (_ []*user_model.Us
143143
return users, nil
144144
}
145145

146-
// GetReviewers get all users can be requested to review:
147-
// - Poster should not be listed
148-
// - For collabrators, all users that have read access or higher to the repository.
149-
// - For repository under orgnization, users under the teams which have read permission or higher of pull request unit
150-
// - Owner will be listed if it's not an organization, not the poster and not in the list of reviewers
151-
func GetReviewers(ctx context.Context, repo *Repository, doerID, posterID int64) ([]*user_model.User, error) {
152-
if err := repo.LoadOwner(ctx); err != nil {
153-
return nil, err
154-
}
155-
156-
e := db.GetEngine(ctx)
157-
uniqueUserIDs := make(container.Set[int64])
158-
159-
if repo.Owner.IsOrganization() {
160-
additionalUserIDs := make([]int64, 0, 10)
161-
if err := e.Table("team_user").
162-
Join("INNER", "team_repo", "`team_repo`.team_id = `team_user`.team_id").
163-
Join("INNER", "team_unit", "`team_unit`.team_id = `team_user`.team_id").
164-
Where("`team_repo`.repo_id = ? AND (`team_unit`.access_mode >= ? AND `team_unit`.`type` = ?)",
165-
repo.ID, perm.AccessModeRead, unit.TypePullRequests).
166-
Distinct("`team_user`.uid").
167-
Select("`team_user`.uid").
168-
Find(&additionalUserIDs); err != nil {
169-
return nil, err
170-
}
171-
uniqueUserIDs.AddMultiple(additionalUserIDs...)
172-
} else {
173-
userIDs := make([]int64, 0, 10)
174-
if err := e.Table("access").
175-
Where("repo_id = ? AND mode >= ?", repo.ID, perm.AccessModeRead).
176-
Select("user_id").
177-
Find(&userIDs); err != nil {
178-
return nil, err
179-
}
180-
uniqueUserIDs.AddMultiple(userIDs...)
181-
}
182-
uniqueUserIDs.Remove(posterID) // posterID should not be in the list of reviewers
183-
184-
// Leave a seat for owner itself to append later, but if owner is an organization
185-
// and just waste 1 unit is cheaper than re-allocate memory once.
186-
users := make([]*user_model.User, 0, len(uniqueUserIDs)+1)
187-
if len(uniqueUserIDs) > 0 {
188-
if err := e.In("id", uniqueUserIDs.Values()).
189-
Where(builder.Eq{"`user`.is_active": true}).
190-
OrderBy(user_model.GetOrderByName()).
191-
Find(&users); err != nil {
192-
return nil, err
193-
}
194-
}
195-
if repo.OwnerID != posterID && !repo.Owner.IsOrganization() && !uniqueUserIDs.Contains(repo.OwnerID) {
196-
users = append(users, repo.Owner)
197-
}
198-
199-
return users, nil
200-
}
201-
202146
// GetIssuePostersWithSearch returns users with limit of 30 whose username started with prefix that have authored an issue/pull request for the given repository
203147
// If isShowFullName is set to true, also include full name prefix search
204148
func GetIssuePostersWithSearch(ctx context.Context, repo *Repository, isPull bool, search string, isShowFullName bool) ([]*user_model.User, error) {

models/repo/user_repo_test.go

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -38,46 +38,3 @@ func TestRepoAssignees(t *testing.T) {
3838
assert.NotContains(t, []int64{users[0].ID, users[1].ID, users[2].ID}, 15)
3939
}
4040
}
41-
42-
func TestRepoGetReviewers(t *testing.T) {
43-
assert.NoError(t, unittest.PrepareTestDatabase())
44-
45-
// test public repo
46-
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
47-
48-
ctx := db.DefaultContext
49-
reviewers, err := repo_model.GetReviewers(ctx, repo1, 2, 2)
50-
assert.NoError(t, err)
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-
}
54-
55-
// should include doer if doer is not PR poster.
56-
reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 2)
57-
assert.NoError(t, err)
58-
assert.Len(t, reviewers, 3)
59-
60-
// should not include PR poster, if PR poster would be otherwise eligible
61-
reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 4)
62-
assert.NoError(t, err)
63-
assert.Len(t, reviewers, 2)
64-
65-
// test private user repo
66-
repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
67-
68-
reviewers, err = repo_model.GetReviewers(ctx, repo2, 2, 4)
69-
assert.NoError(t, err)
70-
assert.Len(t, reviewers, 1)
71-
assert.EqualValues(t, reviewers[0].ID, 2)
72-
73-
// test private org repo
74-
repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3})
75-
76-
reviewers, err = repo_model.GetReviewers(ctx, repo3, 2, 1)
77-
assert.NoError(t, err)
78-
assert.Len(t, reviewers, 2)
79-
80-
reviewers, err = repo_model.GetReviewers(ctx, repo3, 2, 2)
81-
assert.NoError(t, err)
82-
assert.Len(t, reviewers, 1)
83-
}

routers/api/v1/repo/collaborators.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"code.gitea.io/gitea/routers/api/v1/utils"
1919
"code.gitea.io/gitea/services/context"
2020
"code.gitea.io/gitea/services/convert"
21+
pull_service "code.gitea.io/gitea/services/pull"
2122
repo_service "code.gitea.io/gitea/services/repository"
2223
)
2324

@@ -323,7 +324,7 @@ func GetReviewers(ctx *context.APIContext) {
323324
// "404":
324325
// "$ref": "#/responses/notFound"
325326

326-
reviewers, err := repo_model.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, 0)
327+
reviewers, err := pull_service.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, 0)
327328
if err != nil {
328329
ctx.Error(http.StatusInternalServerError, "ListCollaborators", err)
329330
return

services/pull/reviewer.go

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package pull
5+
6+
import (
7+
"context"
8+
"fmt"
9+
10+
"code.gitea.io/gitea/models/db"
11+
"code.gitea.io/gitea/models/perm"
12+
repo_model "code.gitea.io/gitea/models/repo"
13+
"code.gitea.io/gitea/models/unit"
14+
user_model "code.gitea.io/gitea/models/user"
15+
"code.gitea.io/gitea/modules/container"
16+
17+
"xorm.io/builder"
18+
)
19+
20+
// GetReviewers get all users can be requested to review:
21+
// - Poster should not be listed
22+
// - For collaborator, all users that have read access or higher to the repository.
23+
// - For repository under organization, users under the teams which have read permission or higher of pull request unit
24+
// - Owner will be listed if it's not an organization, not the poster and not in the list of reviewers
25+
func GetReviewers(ctx context.Context, repo *repo_model.Repository, doerID, posterID int64) ([]*user_model.User, error) {
26+
if err := repo.LoadOwner(ctx); err != nil {
27+
return nil, err
28+
}
29+
30+
e := db.GetEngine(ctx)
31+
uniqueUserIDs := make(container.Set[int64])
32+
33+
collaboratorIDs := make([]int64, 0, 10)
34+
if err := e.Table("collaboration").Where("repo_id=?", repo.ID).
35+
And("mode >= ?", perm.AccessModeRead).
36+
Select("user_id").
37+
Find(&collaboratorIDs); err != nil {
38+
return nil, err
39+
}
40+
uniqueUserIDs.AddMultiple(collaboratorIDs...)
41+
42+
if repo.Owner.IsOrganization() {
43+
additionalUserIDs := make([]int64, 0, 10)
44+
if err := e.Table("team_user").
45+
Join("INNER", "team_repo", "`team_repo`.team_id = `team_user`.team_id").
46+
Join("INNER", "team_unit", "`team_unit`.team_id = `team_user`.team_id").
47+
Where("`team_repo`.repo_id = ? AND (`team_unit`.access_mode >= ? AND `team_unit`.`type` = ?)",
48+
repo.ID, perm.AccessModeRead, unit.TypePullRequests).
49+
Distinct("`team_user`.uid").
50+
Select("`team_user`.uid").
51+
Find(&additionalUserIDs); err != nil {
52+
return nil, err
53+
}
54+
uniqueUserIDs.AddMultiple(additionalUserIDs...)
55+
56+
if repo.Owner.Visibility.IsLimited() && doerID == 0 {
57+
return nil, fmt.Errorf("permission denied")
58+
}
59+
60+
if (repo.IsPrivate || repo.Owner.Visibility.IsPrivate()) && !uniqueUserIDs.Contains(doerID) {
61+
return nil, fmt.Errorf("permission denied")
62+
}
63+
} else {
64+
userIDs := make([]int64, 0, 10)
65+
if err := e.Table("access").
66+
Where("repo_id = ? AND mode >= ?", repo.ID, perm.AccessModeRead).
67+
Select("user_id").
68+
Find(&userIDs); err != nil {
69+
return nil, err
70+
}
71+
uniqueUserIDs.AddMultiple(userIDs...)
72+
if repo.IsPrivate && !uniqueUserIDs.Contains(doerID) && doerID != repo.OwnerID {
73+
return nil, fmt.Errorf("permission denied")
74+
}
75+
}
76+
77+
uniqueUserIDs.Remove(posterID) // posterID should not be in the list of reviewers
78+
79+
// Leave a seat for owner itself to append later, but if owner is an organization
80+
// and just waste 1 unit is cheaper than re-allocate memory once.
81+
users := make([]*user_model.User, 0, len(uniqueUserIDs)+1)
82+
if len(uniqueUserIDs) > 0 {
83+
if err := e.In("id", uniqueUserIDs.Values()).
84+
Where(builder.Eq{"`user`.is_active": true}).
85+
OrderBy(user_model.GetOrderByName()).
86+
Find(&users); err != nil {
87+
return nil, err
88+
}
89+
}
90+
if repo.OwnerID != posterID && !repo.Owner.IsOrganization() && !uniqueUserIDs.Contains(repo.OwnerID) {
91+
users = append(users, repo.Owner)
92+
}
93+
94+
return users, nil
95+
}

services/pull/reviewer_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package pull_test
5+
6+
import (
7+
"testing"
8+
9+
"code.gitea.io/gitea/models/db"
10+
repo_model "code.gitea.io/gitea/models/repo"
11+
"code.gitea.io/gitea/models/unittest"
12+
pull_service "code.gitea.io/gitea/services/pull"
13+
14+
"github.com/stretchr/testify/assert"
15+
)
16+
17+
func TestRepoGetReviewers(t *testing.T) {
18+
assert.NoError(t, unittest.PrepareTestDatabase())
19+
20+
// test public repo
21+
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
22+
23+
ctx := db.DefaultContext
24+
reviewers, err := pull_service.GetReviewers(ctx, repo1, 2, 0)
25+
assert.NoError(t, err)
26+
if assert.Len(t, reviewers, 1) {
27+
assert.ElementsMatch(t, []int64{2}, []int64{reviewers[0].ID})
28+
}
29+
30+
// should not include doer and remove the poster
31+
reviewers, err = pull_service.GetReviewers(ctx, repo1, 11, 2)
32+
assert.NoError(t, err)
33+
assert.Len(t, reviewers, 0)
34+
35+
// should not include PR poster, if PR poster would be otherwise eligible
36+
reviewers, err = pull_service.GetReviewers(ctx, repo1, 11, 4)
37+
assert.NoError(t, err)
38+
assert.Len(t, reviewers, 1)
39+
40+
// test private user repo
41+
repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
42+
43+
reviewers, err = pull_service.GetReviewers(ctx, repo2, 2, 4)
44+
assert.NoError(t, err)
45+
assert.Len(t, reviewers, 1)
46+
assert.EqualValues(t, reviewers[0].ID, 2)
47+
48+
// test private org repo
49+
repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3})
50+
51+
reviewers, err = pull_service.GetReviewers(ctx, repo3, 2, 1)
52+
assert.NoError(t, err)
53+
assert.Len(t, reviewers, 2)
54+
55+
reviewers, err = pull_service.GetReviewers(ctx, repo3, 2, 2)
56+
assert.NoError(t, err)
57+
assert.Len(t, reviewers, 1)
58+
}

0 commit comments

Comments
 (0)