add request review from specific reviewers feature in pull request (#10756)

* add request review feature in pull request

add a way to notify specific reviewers to review like github , by add  or delet a  special type
review . The acton is  is similar to Assign ,  so many code reuse the function and items of
Assignee, but the meaning and result is different.

The Permission style is is similar to github, that only writer can add a review request from Reviewers,
but the poster can recall and remove a review request after a reviwer has revied even if he don't have
Write Premission. only manager , the poster and reviewer of a request review can remove it.

The reviewers can be requested to review contain all readers for private repo , for public, contain
all writers and watchers.

The offical Review Request will block merge if Reject can block it.

an other change: add ui otify for Assignees.

Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>
Co-authored-by: Lauris BH <lauris@nix.lv>

Signed-off-by: a1012112796 <1012112796@qq.com>

* new change

* add placeholder string

* do some changes follow #10238 to add review requests num on lists also
change icon for review requests to eye

Co-authored-by: Lauris BH <lauris@nix.lv>
This commit is contained in:
赵智超 2020-04-07 00:33:34 +08:00 committed by GitHub
parent 88c14326b1
commit ef89e75d0e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
24 changed files with 714 additions and 67 deletions

View file

@ -289,6 +289,8 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB
reviewTyp := models.ReviewTypeApprove
if typ == "reject" {
reviewTyp = models.ReviewTypeReject
} else if typ == "waiting" {
reviewTyp = models.ReviewTypeRequest
}
for _, count := range counts {
if count.Type == reviewTyp {
@ -377,6 +379,16 @@ func RetrieveRepoMilestonesAndAssignees(ctx *context.Context, repo *models.Repos
}
}
// RetrieveRepoReviewers find all reviewers of a repository
func RetrieveRepoReviewers(ctx *context.Context, repo *models.Repository, issuePosterID int64) {
var err error
ctx.Data["Reviewers"], err = repo.GetReviewers(ctx.User.ID, issuePosterID)
if err != nil {
ctx.ServerError("GetReviewers", err)
return
}
}
// RetrieveRepoMetas find all the meta information of a repository
func RetrieveRepoMetas(ctx *context.Context, repo *models.Repository, isPull bool) []*models.Label {
if !ctx.Repo.CanWriteIssuesOrPulls(isPull) {
@ -815,6 +827,28 @@ func ViewIssue(ctx *context.Context) {
}
}
if issue.IsPull {
canChooseReviewer := ctx.Repo.CanWrite(models.UnitTypePullRequests)
if !canChooseReviewer && ctx.User != nil && ctx.IsSigned {
canChooseReviewer, err = models.IsOfficialReviewer(issue, ctx.User)
if err != nil {
ctx.ServerError("IsOfficialReviewer", err)
return
}
}
if canChooseReviewer {
RetrieveRepoReviewers(ctx, repo, issue.PosterID)
ctx.Data["CanChooseReviewer"] = true
} else {
ctx.Data["CanChooseReviewer"] = false
}
if ctx.Written() {
return
}
}
if ctx.IsSigned {
// Update issue-user.
if err = issue.ReadBy(ctx.User.ID); err != nil {
@ -926,7 +960,7 @@ func ViewIssue(ctx *context.Context) {
if comment.MilestoneID > 0 && comment.Milestone == nil {
comment.Milestone = ghostMilestone
}
} else if comment.Type == models.CommentTypeAssignees {
} else if comment.Type == models.CommentTypeAssignees || comment.Type == models.CommentTypeReviewRequest {
if err = comment.LoadAssigneeUser(); err != nil {
ctx.ServerError("LoadAssigneeUser", err)
return
@ -1273,6 +1307,122 @@ func UpdateIssueAssignee(ctx *context.Context) {
})
}
func isLegalReviewRequest(reviewer, doer *models.User, isAdd bool, issue *models.Issue) error {
if reviewer.IsOrganization() {
return fmt.Errorf("Organization can't be added as reviewer [user_id: %d, repo_id: %d]", reviewer.ID, issue.PullRequest.BaseRepo.ID)
}
if doer.IsOrganization() {
return fmt.Errorf("Organization can't be doer to add reviewer [user_id: %d, repo_id: %d]", doer.ID, issue.PullRequest.BaseRepo.ID)
}
permReviewer, err := models.GetUserRepoPermission(issue.Repo, reviewer)
if err != nil {
return err
}
permDoer, err := models.GetUserRepoPermission(issue.Repo, doer)
if err != nil {
return err
}
lastreview, err := models.GetReviewerByIssueIDAndUserID(issue.ID, reviewer.ID)
if err != nil {
return err
}
var pemResult bool
if isAdd {
pemResult = permReviewer.CanAccessAny(models.AccessModeRead, models.UnitTypePullRequests)
if !pemResult {
return fmt.Errorf("Reviewer can't read [user_id: %d, repo_name: %s]", reviewer.ID, issue.Repo.Name)
}
if doer.ID == issue.PosterID && lastreview != nil && lastreview.Type != models.ReviewTypeRequest {
return nil
}
pemResult = permDoer.CanAccessAny(models.AccessModeWrite, models.UnitTypePullRequests)
if !pemResult {
pemResult, err = models.IsOfficialReviewer(issue, doer)
if err != nil {
return err
}
if !pemResult {
return fmt.Errorf("Doer can't choose reviewer [user_id: %d, repo_name: %s, issue_id: %d]", doer.ID, issue.Repo.Name, issue.ID)
}
}
if doer.ID == reviewer.ID {
return fmt.Errorf("doer can't be reviewer [user_id: %d, repo_name: %s]", doer.ID, issue.Repo.Name)
}
if reviewer.ID == issue.PosterID {
return fmt.Errorf("poster of pr can't be reviewer [user_id: %d, repo_name: %s]", reviewer.ID, issue.Repo.Name)
}
} else {
if lastreview.Type == models.ReviewTypeRequest && lastreview.ReviewerID == doer.ID {
return nil
}
pemResult = permDoer.IsAdmin()
if !pemResult {
return fmt.Errorf("Doer is not admin [user_id: %d, repo_name: %s]", doer.ID, issue.Repo.Name)
}
}
return nil
}
// updatePullReviewRequest change pull's request reviewers
func updatePullReviewRequest(ctx *context.Context) {
issues := getActionIssues(ctx)
if ctx.Written() {
return
}
reviewID := ctx.QueryInt64("id")
event := ctx.Query("is_add")
if event != "add" && event != "remove" {
ctx.ServerError("updatePullReviewRequest", fmt.Errorf("is_add should not be \"%s\"", event))
return
}
for _, issue := range issues {
if issue.IsPull {
reviewer, err := models.GetUserByID(reviewID)
if err != nil {
ctx.ServerError("GetUserByID", err)
return
}
err = isLegalReviewRequest(reviewer, ctx.User, event == "add", issue)
if err != nil {
ctx.ServerError("isLegalRequestReview", err)
return
}
err = issue_service.ReviewRequest(issue, ctx.User, reviewer, event == "add")
if err != nil {
ctx.ServerError("ReviewRequest", err)
return
}
} else {
ctx.ServerError("updatePullReviewRequest", fmt.Errorf("%d in %d is not Pull Request", issue.ID, issue.Repo.ID))
}
}
ctx.JSON(200, map[string]interface{}{
"ok": true,
})
}
// UpdatePullReviewRequest add or remove review request
func UpdatePullReviewRequest(ctx *context.Context) {
updatePullReviewRequest(ctx)
}
// UpdateIssueStatus change issue's status
func UpdateIssueStatus(ctx *context.Context) {
issues := getActionIssues(ctx)

View file

@ -738,6 +738,7 @@ func RegisterRoutes(m *macaron.Macaron) {
m.Post("/labels", reqRepoIssuesOrPullsWriter, repo.UpdateIssueLabel)
m.Post("/milestone", reqRepoIssuesOrPullsWriter, repo.UpdateIssueMilestone)
m.Post("/assignee", reqRepoIssuesOrPullsWriter, repo.UpdateIssueAssignee)
m.Post("/request_review", reqRepoIssuesOrPullsReader, repo.UpdatePullReviewRequest)
m.Post("/status", reqRepoIssuesOrPullsWriter, repo.UpdateIssueStatus)
}, context.RepoMustNotBeArchived())
m.Group("/comments/:id", func() {

View file

@ -632,6 +632,8 @@ func Issues(ctx *context.Context) {
reviewTyp := models.ReviewTypeApprove
if typ == "reject" {
reviewTyp = models.ReviewTypeReject
} else if typ == "waiting" {
reviewTyp = models.ReviewTypeRequest
}
for _, count := range counts {
if count.Type == reviewTyp {