Add review request api

integrations/api_pull_review_test.go

@@ -118,3 +118,50 @@ func TestAPIPullReview(t *testing.T) {
 	req = NewRequestf(t, http.MethodDelete, "/api/v1/repos/%s/%s/pulls/%d/reviews/%d?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, review.ID, token)
 	resp = session.MakeRequest(t, req, http.StatusNoContent)
 }
+
+func TestAPIPullReviewRequest(t *testing.T) {
+	defer prepareTestEnv(t)()
+	pullIssue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 3}).(*models.Issue)
+	assert.NoError(t, pullIssue.LoadAttributes())
+	repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: pullIssue.RepoID}).(*models.Repository)
+
+	// Test add Review Request
+	session := loginUser(t, "user2")
+	token := getTokenForLoggedInUser(t, session)
+	req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, token), &api.PullReviewRequestOptions{
+		Reviewers: []string{"[email protected]", "user8"},
+	})
+	session.MakeRequest(t, req, http.StatusCreated)
+
+	// poster of pr can't be reviewer
+	req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, token), &api.PullReviewRequestOptions{
+		Reviewers: []string{"user1"},
+	})
+	session.MakeRequest(t, req, http.StatusUnprocessableEntity)
+
+	// test user not exist
+	req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, token), &api.PullReviewRequestOptions{
+		Reviewers: []string{"testOther"},
+	})
+	session.MakeRequest(t, req, http.StatusNotFound)
+
+	// Test Remove Review Request
+	session2 := loginUser(t, "user4")
+	token2 := getTokenForLoggedInUser(t, session2)
+
+	req = NewRequestWithJSON(t, http.MethodDelete, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, token2), &api.PullReviewRequestOptions{
+		Reviewers: []string{"user4"},
+	})
+	session.MakeRequest(t, req, http.StatusNoContent)
+
+	// doer is not admin
+	req = NewRequestWithJSON(t, http.MethodDelete, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, token2), &api.PullReviewRequestOptions{
+		Reviewers: []string{"user8"},
+	})
+	session.MakeRequest(t, req, http.StatusUnprocessableEntity)
+
+	req = NewRequestWithJSON(t, http.MethodDelete, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers?token=%s", repo.OwnerName, repo.Name, pullIssue.Index, token), &api.PullReviewRequestOptions{
+		Reviewers: []string{"user8"},
+	})
+	session.MakeRequest(t, req, http.StatusNoContent)
+}

models/review.go

@@ -520,7 +520,7 @@ func AddReviewRequest(issue *Issue, reviewer *User, doer *User) (comment *Commen
 		}
 	}
 
-	_, err = createReview(sess, CreateReviewOptions{
+	review, err = createReview(sess, CreateReviewOptions{
 		Type:     ReviewTypeRequest,
 		Issue:    issue,
 		Reviewer: reviewer,
@@ -539,6 +539,7 @@ func AddReviewRequest(issue *Issue, reviewer *User, doer *User) (comment *Commen
 		Issue:           issue,
 		RemovedAssignee: false,       // Use RemovedAssignee as !isRequest
 		AssigneeID:      reviewer.ID, // Use AssigneeID as reviewer ID
+		ReviewID:        review.ID,
 	})
 
 	if err != nil {
@@ -684,6 +685,10 @@ func DeleteReview(r *Review) error {
 		return fmt.Errorf("review is not allowed to be 0")
 	}
 
+	if r.Type == ReviewTypeRequest {
+		return fmt.Errorf("review request can not be deleted using this method")
+	}
+
 	opts := FindCommentsOptions{
 		Type:     CommentTypeCode,
 		IssueID:  r.IssueID,

modules/structs/pull_review.go

@@ -90,3 +90,8 @@ type SubmitPullReviewOptions struct {
 	Event ReviewStateType `json:"event"`
 	Body  string          `json:"body"`
 }
+
+// PullReviewRequestOptions are options to add or remove pull review requests
+type PullReviewRequestOptions struct {
+	Reviewers []string `json:"reviewers"`
+}

routers/api/v1/api.go

@@ -807,7 +807,9 @@ func RegisterRoutes(m *macaron.Macaron) {
 									Get(repo.GetPullReviewComments)
 							})
 						})
-
+						m.Combo("/requested_reviewers").
+							Delete(reqToken(), bind(api.PullReviewRequestOptions{}), repo.DeletReviewRequests).
+							Post(reqToken(), bind(api.PullReviewRequestOptions{}), repo.CreatReviewRequests)
 					})
 				}, mustAllowPulls, reqRepoReader(models.UnitTypeCode), context.ReferencesGitRepo(false))
 				m.Group("/statuses", func() {

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,167 @@ func prepareSingleReview(ctx *context.APIContext) (*models.Review, *models.PullR
 
 	return review, pr, false
 }
+
+// CreatReviewRequests create review requests to an pull request
+func CreatReviewRequests(ctx *context.APIContext, opts api.PullReviewRequestOptions) {
+	// swagger:operation POST /repos/{owner}/{repo}/pulls/{index}/requested_reviewers repository repoCreatePullReviewRequests
+	// ---
+	// summary: create review requests to an 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)
+}
+
+// DeletReviewRequests delet review requests to an pull request
+func DeletReviewRequests(ctx *context.APIContext, opts api.PullReviewRequestOptions) {
+	// swagger:operation DELETE /repos/{owner}/{repo}/pulls/{index}/requested_reviewers repository repoDeletPullReviewRequests
+	// ---
+	// summary: delet review requests to an 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.IsLegalReviewRequest(reviewer, ctx.User, isAdd, pr.Issue, permDoer)
+		if err != nil {
+			ctx.Error(http.StatusUnprocessableEntity, "IllegalReviewRequest", 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 {
+			if 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
+	}
+}

routers/api/v1/swagger/options.go

@@ -146,4 +146,7 @@ type swaggerParameterBodies struct {
 
 	// in:body
 	SubmitPullReviewOptions api.SubmitPullReviewOptions
+
+	// in:body
+	PullReviewRequestOptions api.PullReviewRequestOptions
 }

routers/repo/issue.go

@@ -1323,72 +1323,6 @@ 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)
@@ -1413,13 +1347,23 @@ func updatePullReviewRequest(ctx *context.Context) {
 				return
 			}
 
-			err = isLegalReviewRequest(reviewer, ctx.User, event == "add", issue)
+			if err := issue.LoadRepo(); err != nil {
+				ctx.ServerError("issue.LoadRepo", err)
+			}
+
+			permDoer, err := models.GetUserRepoPermission(issue.Repo, ctx.User)
+			if err != nil {
+				ctx.ServerError("GetUserRepoPermission", err)
+				return
+			}
+
+			err = issue_service.IsLegalReviewRequest(reviewer, ctx.User, event == "add", issue, permDoer)
 			if err != nil {
 				ctx.ServerError("isLegalRequestReview", err)
 				return
 			}
 
-			err = issue_service.ReviewRequest(issue, ctx.User, reviewer, event == "add")
+			_, err = issue_service.ReviewRequest(issue, ctx.User, reviewer, event == "add")
 			if err != nil {
 				ctx.ServerError("ReviewRequest", err)
 				return