Skip to content

Commit

Permalink
Fix review request API
Browse files Browse the repository at this point in the history
  • Loading branch information
@lunny
lunny committed Nov 25, 2024
1 parent 44909f6 commit 55c2397
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 17 deletions.
78 changes: 75 additions & 3 deletions routers/api/v1/repo/pull_review.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,79 @@ func CreateReviewRequests(ctx *context.APIContext) {
// "$ref": "#/responses/notFound"

opts := web.GetForm(ctx).(*api.PullReviewRequestOptions)
apiReviewRequest(ctx, *opts, true)

pr, err := issues_model.GetPullRequestByIndex(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64(":index"))
if err != nil {
if issues_model.IsErrPullRequestNotExist(err) {
ctx.NotFound("GetPullRequestByIndex", err)
} else {
ctx.Error(http.StatusInternalServerError, "GetPullRequestByIndex", err)
}
return
}

allowedUsers, err := pull_service.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, pr.Issue.PosterID)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetReviewers", err)
return
}
filteredUsers := make([]*user_model.User, 0, len(opts.Reviewers))
for _, reviewer := range opts.Reviewers {
for _, allowedUser := range allowedUsers {
if allowedUser.Name == reviewer || allowedUser.Email == reviewer {
filteredUsers = append(filteredUsers, allowedUser)
break
}
}
}

filteredTeams := make([]*organization.Team, 0, len(opts.TeamReviewers))
if ctx.Repo.Repository.Owner.IsOrganization() && len(opts.TeamReviewers) > 0 {
allowedTeams, err := pull_service.GetReviewerTeams(ctx, ctx.Repo.Repository)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetReviewers", err)
return
}
for _, teamReviewer := range opts.TeamReviewers {
for _, allowedTeam := range allowedTeams {
if allowedTeam.Name == teamReviewer {
filteredTeams = append(filteredTeams, allowedTeam)
break
}
}
}
}
comments, err := issue_service.ReviewRequests(ctx, pr, ctx.Doer, filteredUsers, filteredTeams)
if err != nil {
if issues_model.IsErrReviewRequestOnClosedPR(err) {
ctx.Error(http.StatusForbidden, "", err)
return
}
if issues_model.IsErrNotValidReviewRequest(err) {
ctx.Error(http.StatusUnprocessableEntity, "", err)
return
}
ctx.Error(http.StatusInternalServerError, "ReviewRequests", err)
return
}

reviews := make([]*issues_model.Review, 0, len(filteredUsers))
for _, comment := range comments {
if comment != nil {
if err = comment.LoadReview(ctx); err != nil {
ctx.ServerError("ReviewRequest", err)
return
}
reviews = append(reviews, comment.Review)
}
}

apiReviews, err := convert.ToPullReviewList(ctx, reviews, ctx.Doer)
if err != nil {
ctx.Error(http.StatusInternalServerError, "convertToPullReviewList", err)
return
}
ctx.JSON(http.StatusCreated, apiReviews)
}

// DeleteReviewRequests delete review requests to an pull request
Expand Down Expand Up @@ -730,7 +802,7 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions
}

for _, reviewer := range reviewers {
comment, err := issue_service.ReviewRequest(ctx, pr.Issue, ctx.Doer, &permDoer, reviewer, isAdd)
comment, err := issue_service.ReviewRequest(ctx, pr, ctx.Doer, &permDoer, reviewer, isAdd)
if err != nil {
if issues_model.IsErrReviewRequestOnClosedPR(err) {
ctx.Error(http.StatusForbidden, "", err)
Expand All @@ -755,7 +827,7 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions

if ctx.Repo.Repository.Owner.IsOrganization() && len(opts.TeamReviewers) > 0 {
for _, teamReviewer := range teamReviewers {
comment, err := issue_service.TeamReviewRequest(ctx, pr.Issue, ctx.Doer, teamReviewer, isAdd)
comment, err := issue_service.TeamReviewRequest(ctx, pr, ctx.Doer, teamReviewer, isAdd)
if err != nil {
if issues_model.IsErrReviewRequestOnClosedPR(err) {
ctx.Error(http.StatusForbidden, "", err)
Expand Down
8 changes: 6 additions & 2 deletions routers/web/repo/pull_review.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,10 @@ func UpdatePullReviewRequest(ctx *context.Context) {
ctx.Status(http.StatusForbidden)
return
}
if err := issue.LoadPullRequest(ctx); err != nil {
ctx.ServerError("issue.LoadPullRequest", err)
return
}
if reviewID < 0 {
// negative reviewIDs represent team requests
if err := issue.Repo.LoadOwner(ctx); err != nil {
Expand Down Expand Up @@ -395,7 +399,7 @@ func UpdatePullReviewRequest(ctx *context.Context) {
return
}

_, err = issue_service.TeamReviewRequest(ctx, issue, ctx.Doer, team, action == "attach")
_, err = issue_service.TeamReviewRequest(ctx, issue.PullRequest, ctx.Doer, team, action == "attach")
if err != nil {
if issues_model.IsErrNotValidReviewRequest(err) {
log.Warn(
Expand Down Expand Up @@ -427,7 +431,7 @@ func UpdatePullReviewRequest(ctx *context.Context) {
return
}

_, err = issue_service.ReviewRequest(ctx, issue, ctx.Doer, &ctx.Repo.Permission, reviewer, action == "attach")
_, err = issue_service.ReviewRequest(ctx, issue.PullRequest, ctx.Doer, &ctx.Repo.Permission, reviewer, action == "attach")
if err != nil {
if issues_model.IsErrNotValidReviewRequest(err) {
log.Warn(
Expand Down
41 changes: 31 additions & 10 deletions services/issue/assignee.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/organization"
org_model "code.gitea.io/gitea/models/organization"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
Expand Down Expand Up @@ -61,29 +62,49 @@ func ToggleAssigneeWithNotify(ctx context.Context, issue *issues_model.Issue, do
}

// ReviewRequest add or remove a review request from a user for this PR, and make comment for it.
func ReviewRequest(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, permDoer *access_model.Permission, reviewer *user_model.User, isAdd bool) (comment *issues_model.Comment, err error) {
err = isValidReviewRequest(ctx, reviewer, doer, isAdd, issue, permDoer)
func ReviewRequest(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, permDoer *access_model.Permission, reviewer *user_model.User, isAdd bool) (comment *issues_model.Comment, err error) {
err = isValidReviewRequest(ctx, reviewer, doer, isAdd, pr.Issue, permDoer)
if err != nil {
return nil, err
}

if isAdd {
comment, err = issues_model.AddReviewRequest(ctx, issue, reviewer, doer)
comment, err = issues_model.AddReviewRequest(ctx, pr.Issue, reviewer, doer)
} else {
comment, err = issues_model.RemoveReviewRequest(ctx, issue, reviewer, doer)
comment, err = issues_model.RemoveReviewRequest(ctx, pr.Issue, reviewer, doer)
}

if err != nil {
return nil, err
}

if comment != nil {
notify_service.PullRequestReviewRequest(ctx, doer, issue, reviewer, isAdd, comment)
notify_service.PullRequestReviewRequest(ctx, doer, pr.Issue, reviewer, isAdd, comment)
}

return comment, err
}

func ReviewRequests(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, reviewers []*user_model.User, reviewTeams []*org_model.Team) (comments []*issues_model.Comment, err error) {
for _, reviewer := range reviewers {
comment, err := ReviewRequest(ctx, pr, doer, nil, reviewer, true)
if err != nil {
return nil, err
}
comments = append(comments, comment)
}

for _, reviewTeam := range reviewTeams {
comment, err := TeamReviewRequest(ctx, pr, doer, reviewTeam, true)
if err != nil {
return nil, err
}
comments = append(comments, comment)
}

return comments, nil
}

// isValidReviewRequest Check permission for ReviewRequest
func isValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User, isAdd bool, issue *issues_model.Issue, permDoer *access_model.Permission) error {
if reviewer.IsOrganization() {
Expand Down Expand Up @@ -216,15 +237,15 @@ func isValidTeamReviewRequest(ctx context.Context, reviewer *organization.Team,
}

// TeamReviewRequest add or remove a review request from a team for this PR, and make comment for it.
func TeamReviewRequest(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, reviewer *organization.Team, isAdd bool) (comment *issues_model.Comment, err error) {
err = isValidTeamReviewRequest(ctx, reviewer, doer, isAdd, issue)
func TeamReviewRequest(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, reviewer *organization.Team, isAdd bool) (comment *issues_model.Comment, err error) {
err = isValidTeamReviewRequest(ctx, reviewer, doer, isAdd, pr.Issue)
if err != nil {
return nil, err
}
if isAdd {
comment, err = issues_model.AddTeamReviewRequest(ctx, issue, reviewer, doer)
comment, err = issues_model.AddTeamReviewRequest(ctx, pr.Issue, reviewer, doer)
} else {
comment, err = issues_model.RemoveTeamReviewRequest(ctx, issue, reviewer, doer)
comment, err = issues_model.RemoveTeamReviewRequest(ctx, pr.Issue, reviewer, doer)
}

if err != nil {
Expand All @@ -235,7 +256,7 @@ func TeamReviewRequest(ctx context.Context, issue *issues_model.Issue, doer *use
return nil, nil
}

return comment, teamReviewRequestNotify(ctx, issue, doer, reviewer, isAdd, comment)
return comment, teamReviewRequestNotify(ctx, pr.Issue, doer, reviewer, isAdd, comment)
}

func ReviewRequestNotify(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, reviewNotifiers []*ReviewRequestNotifier) {
Expand Down
4 changes: 2 additions & 2 deletions services/pull/pull.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,12 +212,12 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
}
permDoer, err := access_model.GetUserRepoPermission(ctx, repo, issue.Poster)
for _, reviewer := range opts.Reviewers {
if _, err = issue_service.ReviewRequest(ctx, pr.Issue, issue.Poster, &permDoer, reviewer, true); err != nil {
if _, err = issue_service.ReviewRequest(ctx, pr, issue.Poster, &permDoer, reviewer, true); err != nil {
return err
}
}
for _, teamReviewer := range opts.TeamReviewers {
if _, err = issue_service.TeamReviewRequest(ctx, pr.Issue, issue.Poster, teamReviewer, true); err != nil {
if _, err = issue_service.TeamReviewRequest(ctx, pr, issue.Poster, teamReviewer, true); err != nil {
return err
}
}
Expand Down

0 comments on commit 55c2397

Please sign in to comment.