1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2024-12-24 10:13:10 +00:00

Allow users to self-request a PR review (#19030)

The review request feature was added in https://github.com/go-gitea/gitea/pull/10756,
where the doer got explicitly excluded from available reviewers. I don't see a
functionality or security related reason to forbid this case.

As shown by GitHubs implementation, it may be useful to self-request a review,
to be reminded oneselves about reviewing, while communicating to team mates that a
review is missing.

Co-authored-by: delvh <dev.lh@web.de>
This commit is contained in:
Norwin 2022-03-08 22:48:47 +01:00 committed by GitHub
parent e73c5fd698
commit eceab9e26f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 39 additions and 16 deletions

View file

@ -225,13 +225,21 @@ func getReviewers(ctx context.Context, repo *repo_model.Repository, doerID, post
// This a private repository:
// Anyone who can read the repository is a requestable reviewer
if err := e.
SQL("SELECT * FROM `user` WHERE id in (SELECT user_id FROM `access` WHERE repo_id = ? AND mode >= ? AND user_id NOT IN ( ?, ?)) ORDER BY name",
SQL("SELECT * FROM `user` WHERE id in ("+
"SELECT user_id FROM `access` WHERE repo_id = ? AND mode >= ? AND user_id != ?"+ // private org repos
") ORDER BY name",
repo.ID, perm.AccessModeRead,
doerID, posterID).
posterID).
Find(&users); err != nil {
return nil, err
}
if repo.Owner.Type == user_model.UserTypeIndividual && repo.Owner.ID != posterID {
// as private *user* repos don't generate an entry in the `access` table,
// the owner of a private repo needs to be explicitly added.
users = append(users, repo.Owner)
}
return users, nil
}
@ -244,11 +252,11 @@ func getReviewers(ctx context.Context, repo *repo_model.Repository, doerID, post
"SELECT user_id FROM `watch` WHERE repo_id = ? AND mode IN (?, ?) "+
"UNION "+
"SELECT uid AS user_id FROM `org_user` WHERE org_id = ? "+
") AND id NOT IN (?, ?) ORDER BY name",
") AND id != ? ORDER BY name",
repo.ID, perm.AccessModeRead,
repo.ID, repo_model.WatchModeNormal, repo_model.WatchModeAuto,
repo.OwnerID,
doerID, posterID).
posterID).
Find(&users); err != nil {
return nil, err
}

View file

@ -120,11 +120,34 @@ func TestRepoGetReviewers(t *testing.T) {
assert.NoError(t, err)
assert.Len(t, reviewers, 4)
// test private repo
repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}).(*repo_model.Repository)
reviewers, err = GetReviewers(repo2, 2, 2)
// should include doer if doer is not PR poster.
reviewers, err = GetReviewers(repo1, 11, 2)
assert.NoError(t, err)
assert.Empty(t, reviewers)
assert.Len(t, reviewers, 4)
// should not include PR poster, if PR poster would be otherwise eligible
reviewers, err = GetReviewers(repo1, 11, 4)
assert.NoError(t, err)
assert.Len(t, reviewers, 3)
// test private user repo
repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}).(*repo_model.Repository)
reviewers, err = GetReviewers(repo2, 2, 4)
assert.NoError(t, err)
assert.Len(t, reviewers, 1)
assert.EqualValues(t, reviewers[0].ID, 2)
// test private org repo
repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}).(*repo_model.Repository)
reviewers, err = GetReviewers(repo3, 2, 1)
assert.NoError(t, err)
assert.Len(t, reviewers, 2)
reviewers, err = GetReviewers(repo3, 2, 2)
assert.NoError(t, err)
assert.Len(t, reviewers, 1)
}
func TestRepoGetReviewerTeams(t *testing.T) {

View file

@ -140,14 +140,6 @@ func IsValidReviewRequest(reviewer, doer *user_model.User, isAdd bool, issue *mo
}
}
if doer.ID == reviewer.ID {
return models.ErrNotValidReviewRequest{
Reason: "doer can't be reviewer",
UserID: doer.ID,
RepoID: issue.Repo.ID,
}
}
if reviewer.ID == issue.PosterID && issue.OriginalAuthorID == 0 {
return models.ErrNotValidReviewRequest{
Reason: "poster of pr can't be reviewer",