mirror
/
gitea
mirror of https://github.com/go-gitea/gitea
2
0
Fork 0
Code Issues Releases Wiki Activity

Allow non-admin users to delete review requests (#29057)

Browse Source 
Fix #14459

The following users can add/remove review requests of a PR
- the poster of the PR
- the owner or collaborators of the repository
- members with read permission on the pull requests unit
pull/29320/head^2
Zettat123 9 months ago committed by GitHub
parent 267dbb4e93
commit c42083a339
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
33 changed files with 656 additions and 103 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 24
      models/fixtures/access.yml
  2. 6
      models/fixtures/collaboration.yml
  3. 24
      models/fixtures/email_address.yml
  4. 34
      models/fixtures/issue.yml
  5. 18
      models/fixtures/org_user.yml
  6. 18
      models/fixtures/pull_request.yml
  7. 42
      models/fixtures/repo_unit.yml
  8. 62
      models/fixtures/repository.yml
  9. 22
      models/fixtures/team.yml
  10. 12
      models/fixtures/team_repo.yml
  11. 36
      models/fixtures/team_unit.yml
  12. 18
      models/fixtures/team_user.yml
  13. 148
      models/fixtures/user.yml
  14. 2
      models/issues/issue_test.go
  15. 6
      models/repo/repo_list_test.go
  16. 6
      models/user/user_test.go
  17. 14
      modules/indexer/issues/indexer_test.go
  18. 21
      routers/web/repo/issue.go
  19. 121
      services/issue/assignee.go
  20. 1
      tests/gitea-repositories-meta/org41/repo61.git/HEAD
  21. 6
      tests/gitea-repositories-meta/org41/repo61.git/config
  22. 1
      tests/gitea-repositories-meta/org41/repo61.git/description
  23. 6
      tests/gitea-repositories-meta/org41/repo61.git/info/exclude
  24. 1
      tests/gitea-repositories-meta/user40/repo60.git/HEAD
  25. 6
      tests/gitea-repositories-meta/user40/repo60.git/config
  26. 1
      tests/gitea-repositories-meta/user40/repo60.git/description
  27. 6
      tests/gitea-repositories-meta/user40/repo60.git/info/exclude
  28. 8
      tests/integration/api_issue_test.go
  29. 4
      tests/integration/api_nodeinfo_test.go
  30. 4
      tests/integration/api_org_test.go
  31. 43
      tests/integration/api_pull_review_test.go
  32. 6
      tests/integration/api_repo_test.go
  33. 8
      tests/integration/issue_test.go

24
models/fixtures/access.yml
Unescape View File

@ -135,3 +135,27 @@
user_id: 31
repo_id: 28
mode: 4
-
id: 24
user_id: 38
repo_id: 60
mode: 2
-
id: 25
user_id: 38
repo_id: 61
mode: 1
-
id: 26
user_id: 39
repo_id: 61
mode: 1
-
id: 27
user_id: 40
repo_id: 61
mode: 4

6
models/fixtures/collaboration.yml
Unescape View File

@ -45,3 +45,9 @@
repo_id: 22
user_id: 18
mode: 2 # write
-
id: 9
repo_id: 60
user_id: 38
mode: 2 # write

24
models/fixtures/email_address.yml
Unescape View File

@ -293,3 +293,27 @@
lower_email: user37@example.com
is_activated: true
is_primary: true
-
id: 38
uid: 38
email: user38@example.com
lower_email: user38@example.com
is_activated: true
is_primary: true
-
id: 39
uid: 39
email: user39@example.com
lower_email: user39@example.com
is_activated: true
is_primary: true
-
id: 40
uid: 40
email: user40@example.com
lower_email: user40@example.com
is_activated: true
is_primary: true

34
models/fixtures/issue.yml
Unescape View File

@ -338,3 +338,37 @@
created_unix: 978307210
updated_unix: 978307210
is_locked: false
-
id: 21
repo_id: 60
index: 1
poster_id: 39
original_author_id: 0
name: repo60 pull1
content: content for the 1st issue
milestone_id: 0
priority: 0
is_closed: false
is_pull: true
num_comments: 0
created_unix: 1707270422
updated_unix: 1707270422
is_locked: false
-
id: 22
repo_id: 61
index: 1
poster_id: 40
original_author_id: 0
name: repo61 pull1
content: content for the 1st issue
milestone_id: 0
priority: 0
is_closed: false
is_pull: true
num_comments: 0
created_unix: 1707270422
updated_unix: 1707270422
is_locked: false

18
models/fixtures/org_user.yml
Unescape View File

@ -99,3 +99,21 @@
uid: 5
org_id: 36
is_public: true
-
id: 18
uid: 38
org_id: 41
is_public: true
-
id: 19
uid: 39
org_id: 41
is_public: true
-
id: 20
uid: 40
org_id: 41
is_public: true

18
models/fixtures/pull_request.yml
Unescape View File

@ -99,3 +99,21 @@
index: 1
head_repo_id: 23
base_repo_id: 23
-
id: 9
type: 0 # gitea pull request
status: 2 # mergable
issue_id: 21
index: 1
head_repo_id: 60
base_repo_id: 60
-
id: 10
type: 0 # gitea pull request
status: 2 # mergable
issue_id: 22
index: 1
head_repo_id: 61
base_repo_id: 61

42
models/fixtures/repo_unit.yml
Unescape View File

@ -676,3 +676,45 @@
type: 1
config: "{}"
created_unix: 946684810
-
id: 102
repo_id: 60
type: 1
config: "{}"
created_unix: 946684810
-
id: 103
repo_id: 60
type: 2
config: "{\"EnableTimetracker\":true,\"AllowOnlyContributorsToTrackTime\":true}"
created_unix: 946684810
-
id: 104
repo_id: 60
type: 3
config: "{\"IgnoreWhitespaceConflicts\":false,\"AllowMerge\":true,\"AllowRebase\":true,\"AllowRebaseMerge\":true,\"AllowSquash\":true}"
created_unix: 946684810
-
id: 105
repo_id: 61
type: 1
config: "{}"
created_unix: 946684810
-
id: 106
repo_id: 61
type: 2
config: "{\"EnableTimetracker\":true,\"AllowOnlyContributorsToTrackTime\":true}"
created_unix: 946684810
-
id: 107
repo_id: 61
type: 3
config: "{\"IgnoreWhitespaceConflicts\":false,\"AllowMerge\":true,\"AllowRebase\":true,\"AllowRebaseMerge\":true,\"AllowSquash\":true}"
created_unix: 946684810

62
models/fixtures/repository.yml
Unescape View File

@ -1706,3 +1706,65 @@
is_private: true
status: 0
num_issues: 0
-
id: 60
owner_id: 40
owner_name: user40
lower_name: repo60
name: repo60
default_branch: main
num_watches: 0
num_stars: 0
num_forks: 0
num_issues: 0
num_closed_issues: 0
num_pulls: 1
num_closed_pulls: 0
num_milestones: 0
num_closed_milestones: 0
num_projects: 0
num_closed_projects: 0
is_private: false
is_empty: false
is_archived: false
is_mirror: false
status: 0
is_fork: false
fork_id: 0
is_template: false
template_id: 0
size: 0
is_fsck_enabled: true
close_issues_via_commit_in_any_branch: false
-
id: 61
owner_id: 41
owner_name: org41
lower_name: repo61
name: repo61
default_branch: main
num_watches: 0
num_stars: 0
num_forks: 0
num_issues: 0
num_closed_issues: 0
num_pulls: 1
num_closed_pulls: 0
num_milestones: 0
num_closed_milestones: 0
num_projects: 0
num_closed_projects: 0
is_private: false
is_empty: false
is_archived: false
is_mirror: false
status: 0
is_fork: false
fork_id: 0
is_template: false
template_id: 0
size: 0
is_fsck_enabled: true
close_issues_via_commit_in_any_branch: false

22
models/fixtures/team.yml
Unescape View File

@ -217,3 +217,25 @@
num_members: 1
includes_all_repositories: false
can_create_org_repo: true
-
id: 21
org_id: 41
lower_name: owners
name: Owners
authorize: 4 # owner
num_repos: 1
num_members: 1
includes_all_repositories: true
can_create_org_repo: true
-
id: 22
org_id: 41
lower_name: team1
name: Team1
authorize: 1 # read
num_repos: 1
num_members: 2
includes_all_repositories: false
can_create_org_repo: false

12
models/fixtures/team_repo.yml
Unescape View File

@ -63,3 +63,15 @@
org_id: 17
team_id: 9
repo_id: 24
-
id: 12
org_id: 41
team_id: 21
repo_id: 61
-
id: 13
org_id: 41
team_id: 22
repo_id: 61

36
models/fixtures/team_unit.yml
Unescape View File

@ -286,3 +286,39 @@
team_id: 2
type: 8
access_mode: 2
-
id: 49
team_id: 21
type: 1
access_mode: 4
-
id: 50
team_id: 21
type: 2
access_mode: 4
-
id: 51
team_id: 21
type: 3
access_mode: 4
-
id: 52
team_id: 22
type: 1
access_mode: 1
-
id: 53
team_id: 22
type: 2
access_mode: 1
-
id: 54
team_id: 22
type: 3
access_mode: 1

18
models/fixtures/team_user.yml
Unescape View File

@ -129,3 +129,21 @@
org_id: 17
team_id: 9
uid: 15
-
id: 23
org_id: 41
team_id: 21
uid: 40
-
id: 24
org_id: 41
team_id: 22
uid: 38
-
id: 25
org_id: 41
team_id: 22
uid: 39

148
models/fixtures/user.yml
Unescape View File

@ -1369,3 +1369,151 @@
repo_admin_change_team_access: false
theme: ""
keep_activity_private: false
-
id: 38
lower_name: user38
name: user38
full_name: User38
email: user38@example.com
keep_email_private: false
email_notifications_preference: enabled
passwd: ZogKvWdyEx:password
passwd_hash_algo: dummy
must_change_password: false
login_source: 0
login_name: user38
type: 0
salt: ZogKvWdyEx
max_repo_creation: -1
is_active: true
is_admin: false
is_restricted: false
allow_git_hook: false
allow_import_local: false
allow_create_organization: true
prohibit_login: false
avatar: avatar38
avatar_email: user38@example.com
use_custom_avatar: false
num_followers: 0
num_following: 0
num_stars: 0
num_repos: 0
num_teams: 0
num_members: 0
visibility: 0
repo_admin_change_team_access: false
theme: ""
keep_activity_private: false
-
id: 39
lower_name: user39
name: user39
full_name: User39
email: user39@example.com
keep_email_private: false
email_notifications_preference: enabled
passwd: ZogKvWdyEx:password
passwd_hash_algo: dummy
must_change_password: false
login_source: 0
login_name: user39
type: 0
salt: ZogKvWdyEx
max_repo_creation: -1
is_active: true
is_admin: false
is_restricted: false
allow_git_hook: false
allow_import_local: false
allow_create_organization: true
prohibit_login: false
avatar: avatar39
avatar_email: user39@example.com
use_custom_avatar: false
num_followers: 0
num_following: 0
num_stars: 0
num_repos: 0
num_teams: 0
num_members: 0
visibility: 0
repo_admin_change_team_access: false
theme: ""
keep_activity_private: false
-
id: 40
lower_name: user40
name: user40
full_name: User40
email: user40@example.com
keep_email_private: false
email_notifications_preference: onmention
passwd: ZogKvWdyEx:password
passwd_hash_algo: dummy
must_change_password: false
login_source: 0
login_name: user40
type: 0
salt: ZogKvWdyEx
max_repo_creation: -1
is_active: true
is_admin: false
is_restricted: false
allow_git_hook: false
allow_import_local: false
allow_create_organization: true
prohibit_login: false
avatar: avatar40
avatar_email: user40@example.com
use_custom_avatar: false
num_followers: 0
num_following: 0
num_stars: 0
num_repos: 1
num_teams: 0
num_members: 0
visibility: 0
repo_admin_change_team_access: false
theme: ""
keep_activity_private: false
-
id: 41
lower_name: org41
name: org41
full_name: Org41
email: org41@example.com
keep_email_private: false
email_notifications_preference: onmention
passwd: ZogKvWdyEx:password
passwd_hash_algo: dummy
must_change_password: false
login_source: 0
login_name: org41
type: 1
salt: ZogKvWdyEx
max_repo_creation: -1
is_active: false
is_admin: false
is_restricted: false
allow_git_hook: false
allow_import_local: false
allow_create_organization: true
prohibit_login: false
avatar: avatar41
avatar_email: org41@example.com
use_custom_avatar: false
num_followers: 0
num_following: 0
num_stars: 0
num_repos: 1
num_teams: 2
num_members: 3
visibility: 0
repo_admin_change_team_access: false
theme: ""
keep_activity_private: false

2
models/issues/issue_test.go
Unescape View File

@ -379,7 +379,7 @@ func TestCountIssues(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
count, err := issues_model.CountIssues(db.DefaultContext, &issues_model.IssuesOptions{})
assert.NoError(t, err)
assert.EqualValues(t, 20, count)
assert.EqualValues(t, 22, count)
}
func TestIssueLoadAttributes(t *testing.T) {

6
models/repo/repo_list_test.go
Unescape View File

@ -138,12 +138,12 @@ func getTestCases() []struct {
{
name: "AllPublic/PublicRepositoriesOfUserIncludingCollaborative",
opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, AllPublic: true, Template: util.OptionalBoolFalse},
count: 31,
count: 33,
},
{
name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborative",
opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true, AllLimited: true, Template: util.OptionalBoolFalse},
count: 36,
count: 38,
},
{
name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborativeByName",
@ -158,7 +158,7 @@ func getTestCases() []struct {
{
name: "AllPublic/PublicRepositoriesOfOrganization",
opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 17, AllPublic: true, Collaborate: util.OptionalBoolFalse, Template: util.OptionalBoolFalse},
count: 31,
count: 33,
},
{
name: "AllTemplates",

6
models/user/user_test.go
Unescape View File

@ -89,7 +89,7 @@ func TestSearchUsers(t *testing.T) {
[]int64{19, 25})
testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 4, PageSize: 2}},
[]int64{26})
[]int64{26, 41})
testOrgSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 5, PageSize: 2}},
[]int64{})
@ -101,13 +101,13 @@ func TestSearchUsers(t *testing.T) {
}
testUserSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}},
[]int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37})
[]int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37, 38, 39, 40})
testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolFalse},
[]int64{9})
testUserSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue},
[]int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37})
[]int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37, 38, 39, 40})
testUserSuccess(&user_model.SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue},
[]int64{1, 10, 11, 12, 13, 14, 15, 16, 18})

14
modules/indexer/issues/indexer_test.go
Unescape View File

@ -218,7 +218,7 @@ func searchIssueIsPull(t *testing.T) {
SearchOptions{
IsPull: util.OptionalBoolTrue,
},
[]int64{12, 11, 20, 19, 9, 8, 3, 2},
[]int64{22, 21, 12, 11, 20, 19, 9, 8, 3, 2},
},
}
for _, test := range tests {
@ -239,7 +239,7 @@ func searchIssueIsClosed(t *testing.T) {
SearchOptions{
IsClosed: util.OptionalBoolFalse,
},
[]int64{17, 16, 15, 14, 13, 12, 11, 20, 6, 19, 18, 10, 7, 9, 8, 3, 2, 1},
[]int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 19, 18, 10, 7, 9, 8, 3, 2, 1},
},
{
SearchOptions{
@ -305,7 +305,7 @@ func searchIssueByLabelID(t *testing.T) {
SearchOptions{
ExcludedLabelIDs: []int64{1},
},
[]int64{17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3},
[]int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3},
},
}
for _, test := range tests {
@ -329,7 +329,7 @@ func searchIssueByTime(t *testing.T) {
SearchOptions{
UpdatedAfterUnix: int64Pointer(0),
},
[]int64{17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 1},
[]int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 1},
},
}
for _, test := range tests {
@ -350,7 +350,7 @@ func searchIssueWithOrder(t *testing.T) {
SearchOptions{
SortBy: internal.SortByCreatedAsc,
},
[]int64{1, 2, 3, 8, 9, 4, 7, 10, 18, 19, 5, 6, 20, 11, 12, 13, 14, 15, 16, 17},
[]int64{1, 2, 3, 8, 9, 4, 7, 10, 18, 19, 5, 6, 20, 11, 12, 13, 14, 15, 16, 17, 21, 22},
},
}
for _, test := range tests {
@ -410,8 +410,8 @@ func searchIssueWithPaginator(t *testing.T) {
PageSize: 5,
},
},
[]int64{17, 16, 15, 14, 13},
20,
[]int64{22, 21, 17, 16, 15},
22,
},
}
for _, test := range tests {

21
routers/web/repo/issue.go
Unescape View File

@ -711,16 +711,12 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is
tmp.ItemID = -review.ReviewerTeamID
}
if ctx.Repo.IsAdmin() {
// Admin can dismiss or re-request any review requests
if canChooseReviewer {
// Users who can choose reviewers can also remove review requests
tmp.CanChange = true
} else if ctx.Doer != nil && ctx.Doer.ID == review.ReviewerID && review.Type == issues_model.ReviewTypeRequest {
// A user can refuse review requests
tmp.CanChange = true
} else if (canChooseReviewer || (ctx.Doer != nil && ctx.Doer.ID == issue.PosterID)) && review.Type != issues_model.ReviewTypeRequest &&
ctx.Doer.ID != review.ReviewerID {
// The poster of the PR, a manager, or official reviewers can re-request review from other reviewers
tmp.CanChange = true
}
pullReviews = append(pullReviews, tmp)
@ -1525,18 +1521,9 @@ func ViewIssue(ctx *context.Context) {
}
if issue.IsPull {
canChooseReviewer := ctx.Repo.CanWrite(unit.TypePullRequests)
canChooseReviewer := false
if ctx.Doer != nil && ctx.IsSigned {
if !canChooseReviewer {
canChooseReviewer = ctx.Doer.ID == issue.PosterID
}
if !canChooseReviewer {
canChooseReviewer, err = issues_model.IsOfficialReviewer(ctx, issue, ctx.Doer)
if err != nil {
ctx.ServerError("IsOfficialReviewer", err)
return
}
}
canChooseReviewer = issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, repo, issue)
}
RetrieveRepoReviewers(ctx, repo, issue, canChooseReviewer)

121
services/issue/assignee.go
Unescape View File

@ -10,6 +10,7 @@ import (
"code.gitea.io/gitea/models/organization"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/log"
@ -113,10 +114,10 @@ func IsValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User,
return err
}
var pemResult bool
canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue)
if isAdd {
pemResult = permReviewer.CanAccessAny(perm.AccessModeRead, unit.TypePullRequests)
if !pemResult {
if !permReviewer.CanAccessAny(perm.AccessModeRead, unit.TypePullRequests) {
return issues_model.ErrNotValidReviewRequest{
Reason: "Reviewer can't read",
UserID: doer.ID,
@ -124,51 +125,42 @@ func IsValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User,
}
}
if doer.ID == issue.PosterID && issue.OriginalAuthorID == 0 && lastreview != nil && lastreview.Type != issues_model.ReviewTypeRequest {
return nil
}
pemResult = doer.ID == issue.PosterID
if !pemResult {
pemResult = permDoer.CanAccessAny(perm.AccessModeWrite, unit.TypePullRequests)
}
if !pemResult {
pemResult, err = issues_model.IsOfficialReviewer(ctx, issue, doer)
if err != nil {
return err
}
if !pemResult {
if reviewer.ID == issue.PosterID && issue.OriginalAuthorID == 0 {
return issues_model.ErrNotValidReviewRequest{
Reason: "Doer can't choose reviewer",
Reason: "poster of pr can't be reviewer",
UserID: doer.ID,
RepoID: issue.Repo.ID,
}
}
if canDoerChangeReviewRequests {
return nil
}
if doer.ID == issue.PosterID && issue.OriginalAuthorID == 0 && lastreview != nil && lastreview.Type != issues_model.ReviewTypeRequest {
return nil
}
if reviewer.ID == issue.PosterID && issue.OriginalAuthorID == 0 {
return issues_model.ErrNotValidReviewRequest{
Reason: "poster of pr can't be reviewer",
Reason: "Doer can't choose reviewer",
UserID: doer.ID,
RepoID: issue.Repo.ID,
}
}
} else {
if canDoerChangeReviewRequests {
return nil
}
if lastreview != nil && lastreview.Type == issues_model.ReviewTypeRequest && lastreview.ReviewerID == doer.ID {
return nil
}
pemResult = permDoer.IsAdmin()
if !pemResult {
return issues_model.ErrNotValidReviewRequest{
Reason: "Doer is not admin",
Reason: "Doer can't remove reviewer",
UserID: doer.ID,
RepoID: issue.Repo.ID,
}
}
}
return nil
}
// IsValidTeamReviewRequest Check permission for ReviewRequest Team
@ -181,11 +173,7 @@ func IsValidTeamReviewRequest(ctx context.Context, reviewer *organization.Team,
}
}
permission, err := access_model.GetUserRepoPermission(ctx, issue.Repo, doer)
if err != nil {
log.Error("Unable to GetUserRepoPermission for %-v in %-v#%d", doer, issue.Repo, issue.Index)
return err
}
canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue)
if isAdd {
if issue.Repo.IsPrivate {
@ -200,30 +188,26 @@ func IsValidTeamReviewRequest(ctx context.Context, reviewer *organization.Team,
}
}
doerCanWrite := permission.CanAccessAny(perm.AccessModeWrite, unit.TypePullRequests)
if !doerCanWrite && doer.ID != issue.PosterID {
official, err := issues_model.IsOfficialReviewer(ctx, issue, doer)
if err != nil {
log.Error("Unable to Check if IsOfficialReviewer for %-v in %-v#%d", doer, issue.Repo, issue.Index)
return err
if canDoerChangeReviewRequests {
return nil
}
if !official {
return issues_model.ErrNotValidReviewRequest{
Reason: "Doer can't choose reviewer",
UserID: doer.ID,
RepoID: issue.Repo.ID,
}
}
if canDoerChangeReviewRequests {
return nil
}
} else if !permission.IsAdmin() {
return issues_model.ErrNotValidReviewRequest{
Reason: "Only admin users can remove team requests. Doer is not admin",
Reason: "Doer can't remove reviewer",
UserID: doer.ID,
RepoID: issue.Repo.ID,
}
}
return nil
}
// TeamReviewRequest add or remove a review request from a team for this PR, and make comment for it.
@ -264,3 +248,50 @@ func TeamReviewRequest(ctx context.Context, issue *issues_model.Issue, doer *use
return comment, err
}
// CanDoerChangeReviewRequests returns if the doer can add/remove review requests of a PR
func CanDoerChangeReviewRequests(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue) bool {
// The poster of the PR can change the reviewers
if doer.ID == issue.PosterID {
return true
}
// The owner of the repo can change the reviewers
if doer.ID == repo.OwnerID {
return true
}
// Collaborators of the repo can change the reviewers
isCollaborator, err := repo_model.IsCollaborator(ctx, repo.ID, doer.ID)
if err != nil {
log.Error("IsCollaborator: %v", err)
return false
}
if isCollaborator {
return true
}
// If the repo's owner is an organization, members of teams with read permission on pull requests can change reviewers
if repo.Owner.IsOrganization() {
teams, err := organization.GetTeamsWithAccessToRepo(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead)
if err != nil {
log.Error("GetTeamsWithAccessToRepo: %v", err)
return false
}
for _, team := range teams {
if !team.UnitEnabled(ctx, unit.TypePullRequests) {
continue
}
isMember, err := organization.IsTeamMember(ctx, repo.OwnerID, team.ID, doer.ID)
if err != nil {
log.Error("IsTeamMember: %v", err)
continue
}
if isMember {
return true
}
}
}
return false
}

1
tests/gitea-repositories-meta/org41/repo61.git/HEAD
Unescape View File

@ -0,0 +1 @@
ref: refs/heads/master

6
tests/gitea-repositories-meta/org41/repo61.git/config
Unescape View File

@ -0,0 +1,6 @@
[core]
repositoryformatversion = 0
filemode = false
bare = true
symlinks = false
ignorecase = true

1
tests/gitea-repositories-meta/org41/repo61.git/description
Unescape View File

@ -0,0 +1 @@
Unnamed repository; edit this file 'description' to name the repository.

6
tests/gitea-repositories-meta/org41/repo61.git/info/exclude
Unescape View File

@ -0,0 +1,6 @@
# git ls-files --others --exclude-from=.git/info/exclude
# Lines that start with '#' are comments.
# For a project mostly in C, the following would be a good set of
# exclude patterns (uncomment them if you want to use them):
# *.[oa]
# *~

1
tests/gitea-repositories-meta/user40/repo60.git/HEAD
Unescape View File

@ -0,0 +1 @@
ref: refs/heads/master

6
tests/gitea-repositories-meta/user40/repo60.git/config
Unescape View File

@ -0,0 +1,6 @@
[core]
repositoryformatversion = 0
filemode = false
bare = true
symlinks = false
ignorecase = true

1
tests/gitea-repositories-meta/user40/repo60.git/description
Unescape View File

@ -0,0 +1 @@
Unnamed repository; edit this file 'description' to name the repository.

6
tests/gitea-repositories-meta/user40/repo60.git/info/exclude
Unescape View File

@ -0,0 +1,6 @@
# git ls-files --others --exclude-from=.git/info/exclude
# Lines that start with '#' are comments.
# For a project mostly in C, the following would be a good set of
# exclude patterns (uncomment them if you want to use them):
# *.[oa]
# *~

8
tests/integration/api_issue_test.go
Unescape View File

@ -217,7 +217,7 @@ func TestAPISearchIssues(t *testing.T) {
defer tests.PrepareTestEnv(t)()
// as this API was used in the frontend, it uses UI page size
expectedIssueCount := 18 // from the fixtures
expectedIssueCount := 20 // from the fixtures
if expectedIssueCount > setting.UI.IssuePagingNum {
expectedIssueCount = setting.UI.IssuePagingNum
}
@ -257,7 +257,7 @@ func TestAPISearchIssues(t *testing.T) {
req = NewRequest(t, "GET", link.String()).AddTokenAuth(token)
resp = MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues)
assert.EqualValues(t, "20", resp.Header().Get("X-Total-Count"))
assert.EqualValues(t, "22", resp.Header().Get("X-Total-Count"))
assert.Len(t, apiIssues, 20)
query.Add("limit", "10")
@ -265,7 +265,7 @@ func TestAPISearchIssues(t *testing.T) {
req = NewRequest(t, "GET", link.String()).AddTokenAuth(token)
resp = MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues)
assert.EqualValues(t, "20", resp.Header().Get("X-Total-Count"))
assert.EqualValues(t, "22", resp.Header().Get("X-Total-Count"))
assert.Len(t, apiIssues, 10)
query = url.Values{"assigned": {"true"}, "state": {"all"}}
@ -315,7 +315,7 @@ func TestAPISearchIssuesWithLabels(t *testing.T) {
defer tests.PrepareTestEnv(t)()
// as this API was used in the frontend, it uses UI page size
expectedIssueCount := 18 // from the fixtures
expectedIssueCount := 20 // from the fixtures
if expectedIssueCount > setting.UI.IssuePagingNum {
expectedIssueCount = setting.UI.IssuePagingNum
}

4
tests/integration/api_nodeinfo_test.go
Unescape View File

@ -32,8 +32,8 @@ func TestNodeinfo(t *testing.T) {
DecodeJSON(t, resp, &nodeinfo)
assert.True(t, nodeinfo.OpenRegistrations)
assert.Equal(t, "gitea", nodeinfo.Software.Name)
assert.Equal(t, 26, nodeinfo.Usage.Users.Total)
assert.Equal(t, 20, nodeinfo.Usage.LocalPosts)
assert.Equal(t, 29, nodeinfo.Usage.Users.Total)
assert.Equal(t, 22, nodeinfo.Usage.LocalPosts)
assert.Equal(t, 3, nodeinfo.Usage.LocalComments)
})
}

4
tests/integration/api_org_test.go
Unescape View File

@ -177,7 +177,7 @@ func TestAPIGetAll(t *testing.T) {
var apiOrgList []*api.Organization
DecodeJSON(t, resp, &apiOrgList)
assert.Len(t, apiOrgList, 11)
assert.Len(t, apiOrgList, 12)
assert.Equal(t, "Limited Org 36", apiOrgList[1].FullName)
assert.Equal(t, "limited", apiOrgList[1].Visibility)
@ -186,7 +186,7 @@ func TestAPIGetAll(t *testing.T) {
resp = MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiOrgList)
assert.Len(t, apiOrgList, 7)
assert.Len(t, apiOrgList, 8)
assert.Equal(t, "org 17", apiOrgList[0].FullName)
assert.Equal(t, "public", apiOrgList[0].Visibility)
}

43
tests/integration/api_pull_review_test.go
Unescape View File

@ -279,6 +279,49 @@ func TestAPIPullReviewRequest(t *testing.T) {
}).AddTokenAuth(token)
MakeRequest(t, req, http.StatusNoContent)
// a collaborator can add/remove a review request
pullIssue21 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 21})
assert.NoError(t, pullIssue21.LoadAttributes(db.DefaultContext))
pull21Repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: pullIssue21.RepoID}) // repo60
user38Session := loginUser(t, "user38")
user38Token := getTokenForLoggedInUser(t, user38Session, auth_model.AccessTokenScopeWriteRepository)
req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers", pull21Repo.OwnerName, pull21Repo.Name, pullIssue21.Index), &api.PullReviewRequestOptions{
Reviewers: []string{"user4@example.com"},
}).AddTokenAuth(user38Token)
MakeRequest(t, req, http.StatusCreated)
req = NewRequestWithJSON(t, http.MethodDelete, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers", pull21Repo.OwnerName, pull21Repo.Name, pullIssue21.Index), &api.PullReviewRequestOptions{
Reviewers: []string{"user4@example.com"},
}).AddTokenAuth(user38Token)
MakeRequest(t, req, http.StatusNoContent)
// the poster of the PR can add/remove a review request
user39Session := loginUser(t, "user39")
user39Token := getTokenForLoggedInUser(t, user39Session, auth_model.AccessTokenScopeWriteRepository)
req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers", pull21Repo.OwnerName, pull21Repo.Name, pullIssue21.Index), &api.PullReviewRequestOptions{
Reviewers: []string{"user8"},
}).AddTokenAuth(user39Token)
MakeRequest(t, req, http.StatusCreated)
req = NewRequestWithJSON(t, http.MethodDelete, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers", pull21Repo.OwnerName, pull21Repo.Name, pullIssue21.Index), &api.PullReviewRequestOptions{
Reviewers: []string{"user8"},
}).AddTokenAuth(user39Token)
MakeRequest(t, req, http.StatusNoContent)
// user with read permission on pull requests unit can add/remove a review request
pullIssue22 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 22})
assert.NoError(t, pullIssue22.LoadAttributes(db.DefaultContext))
pull22Repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: pullIssue22.RepoID}) // repo61
req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers", pull22Repo.OwnerName, pull22Repo.Name, pullIssue22.Index), &api.PullReviewRequestOptions{
Reviewers: []string{"user38"},
}).AddTokenAuth(user39Token) // user39 is from a team with read permission on pull requests unit
MakeRequest(t, req, http.StatusCreated)
req = NewRequestWithJSON(t, http.MethodDelete, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers", pull22Repo.OwnerName, pull22Repo.Name, pullIssue22.Index), &api.PullReviewRequestOptions{
Reviewers: []string{"user38"},
}).AddTokenAuth(user39Token) // user39 is from a team with read permission on pull requests unit
MakeRequest(t, req, http.StatusNoContent)
// Test team review request
pullIssue12 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 12})
assert.NoError(t, pullIssue12.LoadAttributes(db.DefaultContext))

6
tests/integration/api_repo_test.go
Unescape View File

@ -93,9 +93,9 @@ func TestAPISearchRepo(t *testing.T) {
}{
{
name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50&private=false", expectedResults: expectedResults{
nil: {count: 33},
user: {count: 33},
user2: {count: 33},
nil: {count: 35},
user: {count: 35},
user2: {count: 35},
},
},
{

8
tests/integration/issue_test.go
Unescape View File

@ -407,7 +407,7 @@ func TestSearchIssues(t *testing.T) {
session := loginUser(t, "user2")
expectedIssueCount := 18 // from the fixtures
expectedIssueCount := 20 // from the fixtures
if expectedIssueCount > setting.UI.IssuePagingNum {
expectedIssueCount = setting.UI.IssuePagingNum
}
@ -444,7 +444,7 @@ func TestSearchIssues(t *testing.T) {
req = NewRequest(t, "GET", link.String())
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues)
assert.EqualValues(t, "20", resp.Header().Get("X-Total-Count"))
assert.EqualValues(t, "22", resp.Header().Get("X-Total-Count"))
assert.Len(t, apiIssues, 20)
query.Add("limit", "5")
@ -452,7 +452,7 @@ func TestSearchIssues(t *testing.T) {
req = NewRequest(t, "GET", link.String())
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiIssues)
assert.EqualValues(t, "20", resp.Header().Get("X-Total-Count"))
assert.EqualValues(t, "22", resp.Header().Get("X-Total-Count"))
assert.Len(t, apiIssues, 5)
query = url.Values{"assigned": {"true"}, "state": {"all"}}
@ -501,7 +501,7 @@ func TestSearchIssues(t *testing.T) {
func TestSearchIssuesWithLabels(t *testing.T) {
defer tests.PrepareTestEnv(t)()
expectedIssueCount := 18 // from the fixtures
expectedIssueCount := 20 // from the fixtures
if expectedIssueCount > setting.UI.IssuePagingNum {
expectedIssueCount = setting.UI.IssuePagingNum
}

Write Preview
Loading…
Cancel
Save