Add review request api (#11355)

* Add review request api

* add : POST /repos/{owner}/{repo}/pulls/{index}/requested_reviewers
* Remove : DELET /repos/{owner}/{repo}/pulls/{index}/requested_reviewers
* fix some request review bug
* block delet request review by models/DeleteReview()

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

* make fmt

* fix bug

* fix test code

* fix typo

* Apply suggestion from code review @jonasfranz

* fix swagger ref

* fix typo

Co-authored-by: Lauris BH <lauris@nix.lv>

* fix comment

* Change response message

* chang response so some simplfy

* Add ErrIllLegalReviewRequest
fix some nits

* make fmt

* Apply suggestions from code review

Co-authored-by: silverwind <me@silverwind.io>

* * Add team support
* fix test
* fix an known bug

* fix nit

* fix test

* Apply suggestions from code review

Co-authored-by: zeripath <art27@cantab.net>

* update get api and add test

Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: zeripath <art27@cantab.net>

4 files changed, 223 insertions, 154 deletions

diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go
index acd97648bf..147cb8e276 100644
--- a/routers/api/v1/api.go
+++ b/routers/api/v1/api.go
@@ -827,7 +827,9 @@ func RegisterRoutes(m *macaron.Macaron) {
 									Get(repo.GetPullReviewComments)
 							})
 						})
-
+						m.Combo("/requested_reviewers").
+							Delete(reqToken(), bind(api.PullReviewRequestOptions{}), repo.DeleteReviewRequests).
+							Post(reqToken(), bind(api.PullReviewRequestOptions{}), repo.CreateReviewRequests)
 					})
 				}, mustAllowPulls, reqRepoReader(models.UnitTypeCode), context.ReferencesGitRepo(false))
 				m.Group("/statuses", func() {
diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go
index 86c084acd7..9e7fd15664 100644
--- a/routers/api/v1/repo/pull_review.go
+++ b/routers/api/v1/repo/pull_review.go
@@ -15,6 +15,7 @@ import (
 	"code.gitea.io/gitea/modules/git"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/routers/api/v1/utils"
+	issue_service "code.gitea.io/gitea/services/issue"
 	pull_service "code.gitea.io/gitea/services/pull"
 )
 
@@ -539,3 +540,214 @@ func prepareSingleReview(ctx *context.APIContext) (*models.Review, *models.PullR
 
 	return review, pr, false
 }
+
+// CreateReviewRequests create review requests to an pull request
+func CreateReviewRequests(ctx *context.APIContext, opts api.PullReviewRequestOptions) {
+	// swagger:operation POST /repos/{owner}/{repo}/pulls/{index}/requested_reviewers repository repoCreatePullReviewRequests
+	// ---
+	// summary: create review requests for a pull request
+	// produces:
+	// - application/json
+	// parameters:
+	// - name: owner
+	//   in: path
+	//   description: owner of the repo
+	//   type: string
+	//   required: true
+	// - name: repo
+	//   in: path
+	//   description: name of the repo
+	//   type: string
+	//   required: true
+	// - name: index
+	//   in: path
+	//   description: index of the pull request
+	//   type: integer
+	//   format: int64
+	//   required: true
+	// - name: body
+	//   in: body
+	//   required: true
+	//   schema:
+	//     "$ref": "#/definitions/PullReviewRequestOptions"
+	// responses:
+	//   "201":
+	//     "$ref": "#/responses/PullReviewList"
+	//   "422":
+	//     "$ref": "#/responses/validationError"
+	//   "404":
+	//     "$ref": "#/responses/notFound"
+	apiReviewRequest(ctx, opts, true)
+}
+
+// DeleteReviewRequests delete review requests to an pull request
+func DeleteReviewRequests(ctx *context.APIContext, opts api.PullReviewRequestOptions) {
+	// swagger:operation DELETE /repos/{owner}/{repo}/pulls/{index}/requested_reviewers repository repoDeletePullReviewRequests
+	// ---
+	// summary: cancel review requests for a pull request
+	// produces:
+	// - application/json
+	// parameters:
+	// - name: owner
+	//   in: path
+	//   description: owner of the repo
+	//   type: string
+	//   required: true
+	// - name: repo
+	//   in: path
+	//   description: name of the repo
+	//   type: string
+	//   required: true
+	// - name: index
+	//   in: path
+	//   description: index of the pull request
+	//   type: integer
+	//   format: int64
+	//   required: true
+	// - name: body
+	//   in: body
+	//   required: true
+	//   schema:
+	//     "$ref": "#/definitions/PullReviewRequestOptions"
+	// responses:
+	//   "204":
+	//     "$ref": "#/responses/empty"
+	//   "422":
+	//     "$ref": "#/responses/validationError"
+	//   "404":
+	//     "$ref": "#/responses/notFound"
+	apiReviewRequest(ctx, opts, false)
+}
+
+func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions, isAdd bool) {
+	pr, err := models.GetPullRequestByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
+	if err != nil {
+		if models.IsErrPullRequestNotExist(err) {
+			ctx.NotFound("GetPullRequestByIndex", err)
+		} else {
+			ctx.Error(http.StatusInternalServerError, "GetPullRequestByIndex", err)
+		}
+		return
+	}
+
+	if err := pr.Issue.LoadRepo(); err != nil {
+		ctx.Error(http.StatusInternalServerError, "pr.Issue.LoadRepo", err)
+		return
+	}
+
+	reviewers := make([]*models.User, 0, len(opts.Reviewers))
+
+	permDoer, err := models.GetUserRepoPermission(pr.Issue.Repo, ctx.User)
+	if err != nil {
+		ctx.Error(http.StatusInternalServerError, "GetUserRepoPermission", err)
+		return
+	}
+
+	for _, r := range opts.Reviewers {
+		var reviewer *models.User
+		if strings.Contains(r, "@") {
+			reviewer, err = models.GetUserByEmail(r)
+		} else {
+			reviewer, err = models.GetUserByName(r)
+		}
+
+		if err != nil {
+			if models.IsErrUserNotExist(err) {
+				ctx.NotFound("UserNotExist", fmt.Sprintf("User '%s' not exist", r))
+				return
+			}
+			ctx.Error(http.StatusInternalServerError, "GetUser", err)
+			return
+		}
+
+		err = issue_service.IsValidReviewRequest(reviewer, ctx.User, isAdd, pr.Issue, &permDoer)
+		if err != nil {
+			if models.IsErrNotValidReviewRequest(err) {
+				ctx.Error(http.StatusUnprocessableEntity, "NotValidReviewRequest", err)
+				return
+			}
+			ctx.Error(http.StatusInternalServerError, "IsValidReviewRequest", err)
+			return
+		}
+
+		reviewers = append(reviewers, reviewer)
+	}
+
+	var reviews []*models.Review
+	if isAdd {
+		reviews = make([]*models.Review, 0, len(reviewers))
+	}
+
+	for _, reviewer := range reviewers {
+		comment, err := issue_service.ReviewRequest(pr.Issue, ctx.User, reviewer, isAdd)
+		if err != nil {
+			ctx.Error(http.StatusInternalServerError, "ReviewRequest", err)
+			return
+		}
+
+		if comment != nil && isAdd {
+			if err = comment.LoadReview(); err != nil {
+				ctx.ServerError("ReviewRequest", err)
+				return
+			}
+			reviews = append(reviews, comment.Review)
+		}
+	}
+
+	if ctx.Repo.Repository.Owner.IsOrganization() && len(opts.TeamReviewers) > 0 {
+
+		teamReviewers := make([]*models.Team, 0, len(opts.TeamReviewers))
+		for _, t := range opts.TeamReviewers {
+			var teamReviewer *models.Team
+			teamReviewer, err = models.GetTeam(ctx.Repo.Owner.ID, t)
+			if err != nil {
+				if models.IsErrTeamNotExist(err) {
+					ctx.NotFound("TeamNotExist", fmt.Sprintf("Team '%s' not exist", t))
+					return
+				}
+				ctx.Error(http.StatusInternalServerError, "ReviewRequest", err)
+				return
+			}
+
+			err = issue_service.IsValidTeamReviewRequest(teamReviewer, ctx.User, isAdd, pr.Issue)
+			if err != nil {
+				if models.IsErrNotValidReviewRequest(err) {
+					ctx.Error(http.StatusUnprocessableEntity, "NotValidReviewRequest", err)
+					return
+				}
+				ctx.Error(http.StatusInternalServerError, "IsValidTeamReviewRequest", err)
+				return
+			}
+
+			teamReviewers = append(teamReviewers, teamReviewer)
+		}
+
+		for _, teamReviewer := range teamReviewers {
+			comment, err := issue_service.TeamReviewRequest(pr.Issue, ctx.User, teamReviewer, isAdd)
+			if err != nil {
+				ctx.ServerError("TeamReviewRequest", err)
+				return
+			}
+
+			if comment != nil && isAdd {
+				if err = comment.LoadReview(); err != nil {
+					ctx.ServerError("ReviewRequest", err)
+					return
+				}
+				reviews = append(reviews, comment.Review)
+			}
+		}
+	}
+
+	if isAdd {
+		apiReviews, err := convert.ToPullReviewList(reviews, ctx.User)
+		if err != nil {
+			ctx.Error(http.StatusInternalServerError, "convertToPullReviewList", err)
+			return
+		}
+		ctx.JSON(http.StatusCreated, apiReviews)
+	} else {
+		ctx.Status(http.StatusNoContent)
+		return
+	}
+}
diff --git a/routers/api/v1/swagger/options.go b/routers/api/v1/swagger/options.go
index ced6589e48..a3bb9cc657 100644
--- a/routers/api/v1/swagger/options.go
+++ b/routers/api/v1/swagger/options.go
@@ -152,4 +152,7 @@ type swaggerParameterBodies struct {
 
 	// in:body
 	MigrateRepoOptions api.MigrateRepoOptions
+
+	// in:body
+	PullReviewRequestOptions api.PullReviewRequestOptions
 }
diff --git a/routers/repo/issue.go b/routers/repo/issue.go
index bc65e73f11..835a952e5e 100644
--- a/routers/repo/issue.go
+++ b/routers/repo/issue.go
@@ -1699,154 +1699,6 @@ func UpdateIssueAssignee(ctx *context.Context) {
 	})
 }
 
-func isValidReviewRequest(reviewer, doer *models.User, isAdd bool, issue *models.Issue) error {
-	if reviewer.IsOrganization() {
-		return models.ErrNotValidReviewRequest{
-			Reason: "Organization can't be added as reviewer",
-			UserID: doer.ID,
-			RepoID: issue.Repo.ID,
-		}
-	}
-	if doer.IsOrganization() {
-		return models.ErrNotValidReviewRequest{
-			Reason: "Organization can't be doer to add reviewer",
-			UserID: doer.ID,
-			RepoID: issue.Repo.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.GetReviewByIssueIDAndUserID(issue.ID, reviewer.ID)
-	if err != nil && !models.IsErrReviewNotExist(err) {
-		return err
-	}
-
-	var pemResult bool
-	if isAdd {
-		pemResult = permReviewer.CanAccessAny(models.AccessModeRead, models.UnitTypePullRequests)
-		if !pemResult {
-			return models.ErrNotValidReviewRequest{
-				Reason: "Reviewer can't read",
-				UserID: doer.ID,
-				RepoID: issue.Repo.ID,
-			}
-		}
-
-		if doer.ID == issue.PosterID && issue.OriginalAuthorID == 0 && 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 models.ErrNotValidReviewRequest{
-					Reason: "Doer can't choose reviewer",
-					UserID: doer.ID,
-					RepoID: issue.Repo.ID,
-				}
-			}
-		}
-
-		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",
-				UserID: doer.ID,
-				RepoID: issue.Repo.ID,
-			}
-		}
-	} else {
-		if lastreview != nil && lastreview.Type == models.ReviewTypeRequest && lastreview.ReviewerID == doer.ID {
-			return nil
-		}
-
-		pemResult = permDoer.IsAdmin()
-		if !pemResult {
-			return models.ErrNotValidReviewRequest{
-				Reason: "Doer is not admin",
-				UserID: doer.ID,
-				RepoID: issue.Repo.ID,
-			}
-		}
-	}
-
-	return nil
-}
-
-func isValidTeamReviewRequest(reviewer *models.Team, doer *models.User, isAdd bool, issue *models.Issue) error {
-	if doer.IsOrganization() {
-		return models.ErrNotValidReviewRequest{
-			Reason: "Organization can't be doer to add reviewer",
-			UserID: doer.ID,
-			RepoID: issue.Repo.ID,
-		}
-	}
-
-	permission, err := models.GetUserRepoPermission(issue.Repo, doer)
-	if err != nil {
-		log.Error("Unable to GetUserRepoPermission for %-v in %-v#%d", doer, issue.Repo, issue.Index)
-		return err
-	}
-
-	if isAdd {
-		if issue.Repo.IsPrivate {
-			hasTeam := models.HasTeamRepo(reviewer.OrgID, reviewer.ID, issue.RepoID)
-
-			if !hasTeam {
-				return models.ErrNotValidReviewRequest{
-					Reason: "Reviewing team can't read repo",
-					UserID: doer.ID,
-					RepoID: issue.Repo.ID,
-				}
-			}
-		}
-
-		doerCanWrite := permission.CanAccessAny(models.AccessModeWrite, models.UnitTypePullRequests)
-		if !doerCanWrite {
-			official, err := models.IsOfficialReviewer(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 !official {
-				return models.ErrNotValidReviewRequest{
-					Reason: "Doer can't choose reviewer",
-					UserID: doer.ID,
-					RepoID: issue.Repo.ID,
-				}
-			}
-		}
-	} else if !permission.IsAdmin() {
-		return models.ErrNotValidReviewRequest{
-			Reason: "Only admin users can remove team requests. Doer is not admin",
-			UserID: doer.ID,
-			RepoID: issue.Repo.ID,
-		}
-	}
-
-	return nil
-}
-
 // UpdatePullReviewRequest add or remove review request
 func UpdatePullReviewRequest(ctx *context.Context) {
 	issues := getActionIssues(ctx)
@@ -1907,7 +1759,7 @@ func UpdatePullReviewRequest(ctx *context.Context) {
 				return
 			}
 
-			err = isValidTeamReviewRequest(team, ctx.User, action == "attach", issue)
+			err = issue_service.IsValidTeamReviewRequest(team, ctx.User, action == "attach", issue)
 			if err != nil {
 				if models.IsErrNotValidReviewRequest(err) {
 					log.Warn(
@@ -1918,11 +1770,11 @@ func UpdatePullReviewRequest(ctx *context.Context) {
 					ctx.Status(403)
 					return
 				}
-				ctx.ServerError("isValidTeamReviewRequest", err)
+				ctx.ServerError("IsValidTeamReviewRequest", err)
 				return
 			}
 
-			err = issue_service.TeamReviewRequest(issue, ctx.User, team, action == "attach")
+			_, err = issue_service.TeamReviewRequest(issue, ctx.User, team, action == "attach")
 			if err != nil {
 				ctx.ServerError("TeamReviewRequest", err)
 				return
@@ -1945,7 +1797,7 @@ func UpdatePullReviewRequest(ctx *context.Context) {
 			return
 		}
 
-		err = isValidReviewRequest(reviewer, ctx.User, action == "attach", issue)
+		err = issue_service.IsValidReviewRequest(reviewer, ctx.User, action == "attach", issue, nil)
 		if err != nil {
 			if models.IsErrNotValidReviewRequest(err) {
 				log.Warn(
@@ -1960,7 +1812,7 @@ func UpdatePullReviewRequest(ctx *context.Context) {
 			return
 		}
 
-		err = issue_service.ReviewRequest(issue, ctx.User, reviewer, action == "attach")
+		_, err = issue_service.ReviewRequest(issue, ctx.User, reviewer, action == "attach")
 		if err != nil {
 			ctx.ServerError("ReviewRequest", err)
 			return