diff options
21 files changed, 694 insertions, 171 deletions
diff --git a/integrations/api_issue_test.go b/integrations/api_issue_test.go index d742049335..9311d50c5c 100644 --- a/integrations/api_issue_test.go +++ b/integrations/api_issue_test.go @@ -153,7 +153,7 @@ func TestAPISearchIssues(t *testing.T) { var apiIssues []*api.Issue DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 9) + assert.Len(t, apiIssues, 10) query := url.Values{} query.Add("token", token) @@ -161,7 +161,7 @@ func TestAPISearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 9) + assert.Len(t, apiIssues, 10) query.Add("state", "closed") link.RawQuery = query.Encode() @@ -182,7 +182,7 @@ func TestAPISearchIssues(t *testing.T) { req = NewRequest(t, "GET", link.String()) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 1) + assert.Len(t, apiIssues, 2) } func TestAPISearchIssuesWithLabels(t *testing.T) { @@ -197,7 +197,7 @@ func TestAPISearchIssuesWithLabels(t *testing.T) { var apiIssues []*api.Issue DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 9) + assert.Len(t, apiIssues, 10) query := url.Values{} query.Add("token", token) @@ -205,7 +205,7 @@ func TestAPISearchIssuesWithLabels(t *testing.T) { req = NewRequest(t, "GET", link.String()) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiIssues) - assert.Len(t, apiIssues, 9) + assert.Len(t, apiIssues, 10) query.Add("labels", "label1") link.RawQuery = query.Encode() diff --git a/integrations/api_pull_review_test.go b/integrations/api_pull_review_test.go index 28eed87255..261a3a8bfa 100644 --- a/integrations/api_pull_review_test.go +++ b/integrations/api_pull_review_test.go @@ -122,4 +122,110 @@ func TestAPIPullReview(t *testing.T) { assert.EqualValues(t, 0, review.CodeCommentsCount) 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) + + // test get review requests + // to make it simple, use same api with get review + pullIssue12 := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 12}).(*models.Issue) + assert.NoError(t, pullIssue12.LoadAttributes()) + repo3 := models.AssertExistsAndLoadBean(t, &models.Repository{ID: pullIssue12.RepoID}).(*models.Repository) + + req = NewRequestf(t, http.MethodGet, "/api/v1/repos/%s/%s/pulls/%d/reviews?token=%s", repo3.OwnerName, repo3.Name, pullIssue12.Index, token) + resp = session.MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &reviews) + assert.EqualValues(t, 11, reviews[0].ID) + assert.EqualValues(t, "REQUEST_REVIEW", reviews[0].State) + assert.EqualValues(t, 0, reviews[0].CodeCommentsCount) + assert.EqualValues(t, false, reviews[0].Stale) + assert.EqualValues(t, true, reviews[0].Official) + assert.EqualValues(t, "test_team", reviews[0].ReviewerTeam.Name) + + assert.EqualValues(t, 12, reviews[1].ID) + assert.EqualValues(t, "REQUEST_REVIEW", reviews[1].State) + assert.EqualValues(t, 0, reviews[0].CodeCommentsCount) + assert.EqualValues(t, false, reviews[1].Stale) + assert.EqualValues(t, true, reviews[1].Official) + assert.EqualValues(t, 1, reviews[1].Reviewer.ID) +} + +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{"user4@example.com", "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) + + // Test team review request + pullIssue12 := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 12}).(*models.Issue) + assert.NoError(t, pullIssue12.LoadAttributes()) + repo3 := models.AssertExistsAndLoadBean(t, &models.Repository{ID: pullIssue12.RepoID}).(*models.Repository) + + // Test add Team Review Request + req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers?token=%s", repo3.OwnerName, repo3.Name, pullIssue12.Index, token), &api.PullReviewRequestOptions{ + TeamReviewers: []string{"team1", "owners"}, + }) + session.MakeRequest(t, req, http.StatusCreated) + + // Test add Team Review Request to not allowned + req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers?token=%s", repo3.OwnerName, repo3.Name, pullIssue12.Index, token), &api.PullReviewRequestOptions{ + TeamReviewers: []string{"test_team"}, + }) + session.MakeRequest(t, req, http.StatusUnprocessableEntity) + + // Test add Team Review Request to not exist + req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers?token=%s", repo3.OwnerName, repo3.Name, pullIssue12.Index, token), &api.PullReviewRequestOptions{ + TeamReviewers: []string{"not_exist_team"}, + }) + session.MakeRequest(t, req, http.StatusNotFound) + + // Test Remove team Review Request + req = NewRequestWithJSON(t, http.MethodDelete, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers?token=%s", repo3.OwnerName, repo3.Name, pullIssue12.Index, token), &api.PullReviewRequestOptions{ + TeamReviewers: []string{"team1"}, + }) + session.MakeRequest(t, req, http.StatusNoContent) + + // empty request test + req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers?token=%s", repo3.OwnerName, repo3.Name, pullIssue12.Index, token), &api.PullReviewRequestOptions{}) + session.MakeRequest(t, req, http.StatusCreated) + + req = NewRequestWithJSON(t, http.MethodDelete, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/requested_reviewers?token=%s", repo3.OwnerName, repo3.Name, pullIssue12.Index, token), &api.PullReviewRequestOptions{}) + session.MakeRequest(t, req, http.StatusNoContent) } diff --git a/integrations/gitea-repositories-meta/user3/repo3.git/objects/d2/2b4d4daa5be07329fcef6ed458f00cf3392da0 b/integrations/gitea-repositories-meta/user3/repo3.git/objects/d2/2b4d4daa5be07329fcef6ed458f00cf3392da0 Binary files differnew file mode 100644 index 0000000000..e319f8ce34 --- /dev/null +++ b/integrations/gitea-repositories-meta/user3/repo3.git/objects/d2/2b4d4daa5be07329fcef6ed458f00cf3392da0 diff --git a/integrations/gitea-repositories-meta/user3/repo3.git/objects/ec/f0db3c1ec806522de4b491fb9a3c7457398c61 b/integrations/gitea-repositories-meta/user3/repo3.git/objects/ec/f0db3c1ec806522de4b491fb9a3c7457398c61 Binary files differnew file mode 100644 index 0000000000..ed431f70d3 --- /dev/null +++ b/integrations/gitea-repositories-meta/user3/repo3.git/objects/ec/f0db3c1ec806522de4b491fb9a3c7457398c61 diff --git a/integrations/gitea-repositories-meta/user3/repo3.git/objects/ee/16d127df463aa491e08958120f2108b02468df b/integrations/gitea-repositories-meta/user3/repo3.git/objects/ee/16d127df463aa491e08958120f2108b02468df Binary files differnew file mode 100644 index 0000000000..e177f69e37 --- /dev/null +++ b/integrations/gitea-repositories-meta/user3/repo3.git/objects/ee/16d127df463aa491e08958120f2108b02468df diff --git a/integrations/gitea-repositories-meta/user3/repo3.git/refs/heads/test_branch b/integrations/gitea-repositories-meta/user3/repo3.git/refs/heads/test_branch new file mode 100644 index 0000000000..dfe0c6a128 --- /dev/null +++ b/integrations/gitea-repositories-meta/user3/repo3.git/refs/heads/test_branch @@ -0,0 +1 @@ +d22b4d4daa5be07329fcef6ed458f00cf3392da0 diff --git a/models/error.go b/models/error.go index be94d78891..b2273f74c9 100644 --- a/models/error.go +++ b/models/error.go @@ -2003,7 +2003,7 @@ type ErrNotValidReviewRequest struct { // IsErrNotValidReviewRequest checks if an error is a ErrNotValidReviewRequest. func IsErrNotValidReviewRequest(err error) bool { - _, ok := err.(ErrReviewNotExist) + _, ok := err.(ErrNotValidReviewRequest) return ok } diff --git a/models/fixtures/issue.yml b/models/fixtures/issue.yml index 39a96dc550..3e836bf5d1 100644 --- a/models/fixtures/issue.yml +++ b/models/fixtures/issue.yml @@ -135,3 +135,15 @@ is_pull: true created_unix: 1579194806 updated_unix: 1579194806 + +- + id: 12 + repo_id: 3 + index: 2 + poster_id: 2 + name: pull6 + content: content for the a pull request + is_closed: false + is_pull: true + created_unix: 1602935696 + updated_unix: 1602935696 diff --git a/models/fixtures/pull_request.yml b/models/fixtures/pull_request.yml index b555da83ef..d45baa711c 100644 --- a/models/fixtures/pull_request.yml +++ b/models/fixtures/pull_request.yml @@ -63,3 +63,16 @@ base_branch: branch1 merge_base: 1234567890abcdef has_merged: false + +- + id: 6 + type: 0 # gitea pull request + status: 2 # mergable + issue_id: 12 + index: 2 + head_repo_id: 3 + base_repo_id: 3 + head_branch: test_branch + base_branch: master + merge_base: 2a47ca4b614a9f5a + has_merged: false diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index a44e480270..c7f55a8f70 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -41,7 +41,7 @@ is_private: true num_issues: 1 num_closed_issues: 0 - num_pulls: 0 + num_pulls: 1 num_closed_pulls: 0 num_watches: 0 num_projects: 1 diff --git a/models/fixtures/review.yml b/models/fixtures/review.yml index 3db0b47353..c7c16fb109 100644 --- a/models/fixtures/review.yml +++ b/models/fixtures/review.yml @@ -86,3 +86,22 @@ official: true updated_unix: 946684815 created_unix: 946684815 + +- + id: 11 + type: 4 + reviewer_id: 0 + reviewer_team_id: 7 + issue_id: 12 + official: true + updated_unix: 1602936509 + created_unix: 1602936509 + +- + id: 12 + type: 4 + reviewer_id: 1 + issue_id: 12 + official: true + updated_unix: 1603196749 + created_unix: 1603196749
\ No newline at end of file diff --git a/models/review.go b/models/review.go index 326b06b5ed..aeb5f21ea9 100644 --- a/models/review.go +++ b/models/review.go @@ -627,13 +627,14 @@ func AddReviewRequest(issue *Issue, reviewer, doer *User) (*Comment, error) { } } - if _, err = createReview(sess, CreateReviewOptions{ + review, err = createReview(sess, CreateReviewOptions{ Type: ReviewTypeRequest, Issue: issue, Reviewer: reviewer, Official: official, Stale: false, - }); err != nil { + }) + if err != nil { return nil, err } @@ -644,6 +645,7 @@ func AddReviewRequest(issue *Issue, reviewer, doer *User) (*Comment, error) { Issue: issue, RemovedAssignee: false, // Use RemovedAssignee as !isRequest AssigneeID: reviewer.ID, // Use AssigneeID as reviewer ID + ReviewID: review.ID, }) if err != nil { return nil, err @@ -732,7 +734,7 @@ func AddTeamReviewRequest(issue *Issue, reviewer *Team, doer *User) (*Comment, e } } - if _, err = createReview(sess, CreateReviewOptions{ + if review, err = createReview(sess, CreateReviewOptions{ Type: ReviewTypeRequest, Issue: issue, ReviewerTeam: reviewer, @@ -755,6 +757,7 @@ func AddTeamReviewRequest(issue *Issue, reviewer *Team, doer *User) (*Comment, e Issue: issue, RemovedAssignee: false, // Use RemovedAssignee as !isRequest AssigneeTeamID: reviewer.ID, // Use AssigneeTeamID as reviewer team ID + ReviewID: review.ID, }) if err != nil { return nil, fmt.Errorf("createComment(): %v", err) @@ -894,6 +897,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, diff --git a/modules/convert/convert.go b/modules/convert/convert.go index e81df0c0c3..5d056c3795 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -284,6 +284,10 @@ func ToOrganization(org *models.User) *api.Organization { // ToTeam convert models.Team to api.Team func ToTeam(team *models.Team) *api.Team { + if team == nil { + return nil + } + return &api.Team{ ID: team.ID, Name: team.Name, diff --git a/modules/convert/pull_review.go b/modules/convert/pull_review.go index 032d3617fc..0ef1fec39c 100644 --- a/modules/convert/pull_review.go +++ b/modules/convert/pull_review.go @@ -28,6 +28,7 @@ func ToPullReview(r *models.Review, doer *models.User) (*api.PullReview, error) result := &api.PullReview{ ID: r.ID, Reviewer: ToUser(r.Reviewer, doer != nil, auth), + ReviewerTeam: ToTeam(r.ReviewerTeam), State: api.ReviewStateUnknown, Body: r.Content, CommitID: r.CommitID, diff --git a/modules/structs/pull_review.go b/modules/structs/pull_review.go index bf9eafc243..07fa968d28 100644 --- a/modules/structs/pull_review.go +++ b/modules/structs/pull_review.go @@ -30,6 +30,7 @@ const ( type PullReview struct { ID int64 `json:"id"` Reviewer *User `json:"user"` + ReviewerTeam *Team `json:"team"` State ReviewStateType `json:"state"` Body string `json:"body"` CommitID string `json:"commit_id"` @@ -90,3 +91,9 @@ 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"` + TeamReviewers []string `json:"team_reviewers"` +} 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 diff --git a/services/issue/assignee.go b/services/issue/assignee.go index f48e55e53c..f24a242f6b 100644 --- a/services/issue/assignee.go +++ b/services/issue/assignee.go @@ -6,6 +6,7 @@ package issue import ( "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/notification" ) @@ -53,8 +54,7 @@ func ToggleAssignee(issue *models.Issue, doer *models.User, assigneeID int64) (r } // ReviewRequest add or remove a review request from a user for this PR, and make comment for it. -func ReviewRequest(issue *models.Issue, doer *models.User, reviewer *models.User, isAdd bool) (err error) { - var comment *models.Comment +func ReviewRequest(issue *models.Issue, doer *models.User, reviewer *models.User, isAdd bool) (comment *models.Comment, err error) { if isAdd { comment, err = models.AddReviewRequest(issue, reviewer, doer) } else { @@ -62,19 +62,171 @@ func ReviewRequest(issue *models.Issue, doer *models.User, reviewer *models.User } if err != nil { - return + return nil, err } if comment != nil { notification.NotifyPullReviewRequest(doer, issue, reviewer, isAdd, comment) } + return +} + +// IsValidReviewRequest Check permission for ReviewRequest +func IsValidReviewRequest(reviewer, doer *models.User, isAdd bool, issue *models.Issue, permDoer *models.Permission) 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 + } + + if permDoer == nil { + permDoer = new(models.Permission) + *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 +} + +// IsValidTeamReviewRequest Check permission for ReviewRequest Team +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 } // TeamReviewRequest add or remove a review request from a team for this PR, and make comment for it. -func TeamReviewRequest(issue *models.Issue, doer *models.User, reviewer *models.Team, isAdd bool) (err error) { - var comment *models.Comment +func TeamReviewRequest(issue *models.Issue, doer *models.User, reviewer *models.Team, isAdd bool) (comment *models.Comment, err error) { if isAdd { comment, err = models.AddTeamReviewRequest(issue, reviewer, doer) } else { @@ -106,5 +258,5 @@ func TeamReviewRequest(issue *models.Issue, doer *models.User, reviewer *models. notification.NotifyPullReviewRequest(doer, issue, member, isAdd, comment) } - return nil + return } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index e7655f02a8..90a76643dd 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -7164,6 +7164,114 @@ } } }, + "/repos/{owner}/{repo}/pulls/{index}/requested_reviewers": { + "post": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "create review requests for a pull request", + "operationId": "repoCreatePullReviewRequests", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "index of the pull request", + "name": "index", + "in": "path", + "required": true + }, + { + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/PullReviewRequestOptions" + } + } + ], + "responses": { + "201": { + "$ref": "#/responses/PullReviewList" + }, + "404": { + "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/validationError" + } + } + }, + "delete": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "cancel review requests for a pull request", + "operationId": "repoDeletePullReviewRequests", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "index of the pull request", + "name": "index", + "in": "path", + "required": true + }, + { + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/PullReviewRequestOptions" + } + } + ], + "responses": { + "204": { + "$ref": "#/responses/empty" + }, + "404": { + "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/validationError" + } + } + } + }, "/repos/{owner}/{repo}/pulls/{index}/reviews": { "get": { "produces": [ @@ -14540,6 +14648,9 @@ "format": "date-time", "x-go-name": "Submitted" }, + "team": { + "$ref": "#/definitions/Team" + }, "user": { "$ref": "#/definitions/User" } @@ -14614,6 +14725,27 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "PullReviewRequestOptions": { + "description": "PullReviewRequestOptions are options to add or remove pull review requests", + "type": "object", + "properties": { + "reviewers": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "Reviewers" + }, + "team_reviewers": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "TeamReviewers" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "Reaction": { "description": "Reaction contain one reaction", "type": "object", @@ -16162,7 +16294,7 @@ "parameterBodies": { "description": "parameterBodies", "schema": { - "$ref": "#/definitions/MigrateRepoOptions" + "$ref": "#/definitions/PullReviewRequestOptions" } }, "redirect": { |