Skip to content

Commit ab2843b

Browse files
authored
Merge branch 'main' into issues/32561
2 parents 61c882c + fe49cb0 commit ab2843b

File tree

12 files changed

+225
-158
lines changed

12 files changed

+225
-158
lines changed

models/organization/team_repo.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"code.gitea.io/gitea/models/db"
1010
"code.gitea.io/gitea/models/perm"
1111
repo_model "code.gitea.io/gitea/models/repo"
12+
"code.gitea.io/gitea/models/unit"
1213

1314
"xorm.io/builder"
1415
)
@@ -83,3 +84,16 @@ func GetTeamsWithAccessToRepo(ctx context.Context, orgID, repoID int64, mode per
8384
OrderBy("name").
8485
Find(&teams)
8586
}
87+
88+
// GetTeamsWithAccessToRepoUnit returns all teams in an organization that have given access level to the repository special unit.
89+
func GetTeamsWithAccessToRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type) ([]*Team, error) {
90+
teams := make([]*Team, 0, 5)
91+
return teams, db.GetEngine(ctx).Where("team_unit.access_mode >= ?", mode).
92+
Join("INNER", "team_repo", "team_repo.team_id = team.id").
93+
Join("INNER", "team_unit", "team_unit.team_id = team.id").
94+
And("team_repo.org_id = ?", orgID).
95+
And("team_repo.repo_id = ?", repoID).
96+
And("team_unit.type = ?", unitType).
97+
OrderBy("name").
98+
Find(&teams)
99+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package organization_test
5+
6+
import (
7+
"testing"
8+
9+
"code.gitea.io/gitea/models/db"
10+
"code.gitea.io/gitea/models/organization"
11+
"code.gitea.io/gitea/models/perm"
12+
"code.gitea.io/gitea/models/repo"
13+
"code.gitea.io/gitea/models/unit"
14+
"code.gitea.io/gitea/models/unittest"
15+
16+
"github.com/stretchr/testify/assert"
17+
)
18+
19+
func TestGetTeamsWithAccessToRepoUnit(t *testing.T) {
20+
assert.NoError(t, unittest.PrepareTestDatabase())
21+
22+
org41 := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 41})
23+
repo61 := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 61})
24+
25+
teams, err := organization.GetTeamsWithAccessToRepoUnit(db.DefaultContext, org41.ID, repo61.ID, perm.AccessModeRead, unit.TypePullRequests)
26+
assert.NoError(t, err)
27+
if assert.Len(t, teams, 2) {
28+
assert.EqualValues(t, 21, teams[0].ID)
29+
assert.EqualValues(t, 22, teams[1].ID)
30+
}
31+
}

models/repo/user_repo.go

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"code.gitea.io/gitea/models/unit"
1212
user_model "code.gitea.io/gitea/models/user"
1313
"code.gitea.io/gitea/modules/container"
14-
api "code.gitea.io/gitea/modules/structs"
1514

1615
"xorm.io/builder"
1716
)
@@ -146,57 +145,6 @@ func GetRepoAssignees(ctx context.Context, repo *Repository) (_ []*user_model.Us
146145
return users, nil
147146
}
148147

149-
// GetReviewers get all users can be requested to review:
150-
// * for private repositories this returns all users that have read access or higher to the repository.
151-
// * for public repositories this returns all users that have read access or higher to the repository,
152-
// all repo watchers and all organization members.
153-
// TODO: may be we should have a busy choice for users to block review request to them.
154-
func GetReviewers(ctx context.Context, repo *Repository, doerID, posterID int64) ([]*user_model.User, error) {
155-
// Get the owner of the repository - this often already pre-cached and if so saves complexity for the following queries
156-
if err := repo.LoadOwner(ctx); err != nil {
157-
return nil, err
158-
}
159-
160-
cond := builder.And(builder.Neq{"`user`.id": posterID}).
161-
And(builder.Eq{"`user`.is_active": true})
162-
163-
if repo.IsPrivate || repo.Owner.Visibility == api.VisibleTypePrivate {
164-
// This a private repository:
165-
// Anyone who can read the repository is a requestable reviewer
166-
167-
cond = cond.And(builder.In("`user`.id",
168-
builder.Select("user_id").From("access").Where(
169-
builder.Eq{"repo_id": repo.ID}.
170-
And(builder.Gte{"mode": perm.AccessModeRead}),
171-
),
172-
))
173-
174-
if repo.Owner.Type == user_model.UserTypeIndividual && repo.Owner.ID != posterID {
175-
// as private *user* repos don't generate an entry in the `access` table,
176-
// the owner of a private repo needs to be explicitly added.
177-
cond = cond.Or(builder.Eq{"`user`.id": repo.Owner.ID})
178-
}
179-
} else {
180-
// This is a "public" repository:
181-
// Any user that has read access, is a watcher or organization member can be requested to review
182-
cond = cond.And(builder.And(builder.In("`user`.id",
183-
builder.Select("user_id").From("access").
184-
Where(builder.Eq{"repo_id": repo.ID}.
185-
And(builder.Gte{"mode": perm.AccessModeRead})),
186-
).Or(builder.In("`user`.id",
187-
builder.Select("user_id").From("watch").
188-
Where(builder.Eq{"repo_id": repo.ID}.
189-
And(builder.In("mode", WatchModeNormal, WatchModeAuto))),
190-
).Or(builder.In("`user`.id",
191-
builder.Select("uid").From("org_user").
192-
Where(builder.Eq{"org_id": repo.OwnerID}),
193-
)))))
194-
}
195-
196-
users := make([]*user_model.User, 0, 8)
197-
return users, db.GetEngine(ctx).Where(cond).OrderBy(user_model.GetOrderByName()).Find(&users)
198-
}
199-
200148
// GetIssuePostersWithSearch returns users with limit of 30 whose username started with prefix that have authored an issue/pull request for the given repository
201149
// If isShowFullName is set to true, also include full name prefix search
202150
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: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import (
1717
"code.gitea.io/gitea/routers/api/v1/utils"
1818
"code.gitea.io/gitea/services/context"
1919
"code.gitea.io/gitea/services/convert"
20+
issue_service "code.gitea.io/gitea/services/issue"
21+
pull_service "code.gitea.io/gitea/services/pull"
2022
repo_service "code.gitea.io/gitea/services/repository"
2123
)
2224

@@ -320,7 +322,13 @@ func GetReviewers(ctx *context.APIContext) {
320322
// "404":
321323
// "$ref": "#/responses/notFound"
322324

323-
reviewers, err := repo_model.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, 0)
325+
canChooseReviewer := issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, ctx.Repo.Repository, 0)
326+
if !canChooseReviewer {
327+
ctx.Error(http.StatusForbidden, "GetReviewers", errors.New("doer has no permission to get reviewers"))
328+
return
329+
}
330+
331+
reviewers, err := pull_service.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, 0)
324332
if err != nil {
325333
ctx.Error(http.StatusInternalServerError, "ListCollaborators", err)
326334
return

routers/web/repo/issue_page_meta.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import (
1919
shared_user "code.gitea.io/gitea/routers/web/shared/user"
2020
"code.gitea.io/gitea/services/context"
2121
issue_service "code.gitea.io/gitea/services/issue"
22-
repo_service "code.gitea.io/gitea/services/repository"
22+
pull_service "code.gitea.io/gitea/services/pull"
2323
)
2424

2525
type issueSidebarMilestoneData struct {
@@ -186,7 +186,7 @@ func (d *IssuePageMetaData) retrieveReviewersData(ctx *context.Context) {
186186
if d.Issue == nil {
187187
data.CanChooseReviewer = true
188188
} else {
189-
data.CanChooseReviewer = issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, repo, d.Issue)
189+
data.CanChooseReviewer = issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, repo, d.Issue.PosterID)
190190
}
191191
}
192192

@@ -231,13 +231,13 @@ func (d *IssuePageMetaData) retrieveReviewersData(ctx *context.Context) {
231231

232232
if data.CanChooseReviewer {
233233
var err error
234-
reviewers, err = repo_model.GetReviewers(ctx, repo, ctx.Doer.ID, posterID)
234+
reviewers, err = pull_service.GetReviewers(ctx, repo, ctx.Doer.ID, posterID)
235235
if err != nil {
236236
ctx.ServerError("GetReviewers", err)
237237
return
238238
}
239239

240-
teamReviewers, err = repo_service.GetReviewerTeams(ctx, repo)
240+
teamReviewers, err = pull_service.GetReviewerTeams(ctx, repo)
241241
if err != nil {
242242
ctx.ServerError("GetReviewerTeams", err)
243243
return

services/issue/assignee.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func isValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User,
119119
return err
120120
}
121121

122-
canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue)
122+
canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue.PosterID)
123123

124124
if isAdd {
125125
if !permReviewer.CanAccessAny(perm.AccessModeRead, unit.TypePullRequests) {
@@ -178,7 +178,7 @@ func isValidTeamReviewRequest(ctx context.Context, reviewer *organization.Team,
178178
}
179179
}
180180

181-
canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue)
181+
canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue.PosterID)
182182

183183
if isAdd {
184184
if issue.Repo.IsPrivate {
@@ -276,12 +276,12 @@ func teamReviewRequestNotify(ctx context.Context, issue *issues_model.Issue, doe
276276
}
277277

278278
// CanDoerChangeReviewRequests returns if the doer can add/remove review requests of a PR
279-
func CanDoerChangeReviewRequests(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue) bool {
279+
func CanDoerChangeReviewRequests(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, posterID int64) bool {
280280
if repo.IsArchived {
281281
return false
282282
}
283283
// The poster of the PR can change the reviewers
284-
if doer.ID == issue.PosterID {
284+
if doer.ID == posterID {
285285
return true
286286
}
287287

services/pull/reviewer.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package pull
5+
6+
import (
7+
"context"
8+
9+
"code.gitea.io/gitea/models/db"
10+
"code.gitea.io/gitea/models/organization"
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+
57+
uniqueUserIDs.Remove(posterID) // posterID should not be in the list of reviewers
58+
59+
// Leave a seat for owner itself to append later, but if owner is an organization
60+
// and just waste 1 unit is cheaper than re-allocate memory once.
61+
users := make([]*user_model.User, 0, len(uniqueUserIDs)+1)
62+
if len(uniqueUserIDs) > 0 {
63+
if err := e.In("id", uniqueUserIDs.Values()).
64+
Where(builder.Eq{"`user`.is_active": true}).
65+
OrderBy(user_model.GetOrderByName()).
66+
Find(&users); err != nil {
67+
return nil, err
68+
}
69+
}
70+
71+
// add owner after all users are loaded because we can avoid load owner twice
72+
if repo.OwnerID != posterID && !repo.Owner.IsOrganization() && !uniqueUserIDs.Contains(repo.OwnerID) {
73+
users = append(users, repo.Owner)
74+
}
75+
76+
return users, nil
77+
}
78+
79+
// GetReviewerTeams get all teams can be requested to review
80+
func GetReviewerTeams(ctx context.Context, repo *repo_model.Repository) ([]*organization.Team, error) {
81+
if err := repo.LoadOwner(ctx); err != nil {
82+
return nil, err
83+
}
84+
if !repo.Owner.IsOrganization() {
85+
return nil, nil
86+
}
87+
88+
return organization.GetTeamsWithAccessToRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests)
89+
}

0 commit comments

Comments
 (0)