Skip to content

Commit 18aeca5

Browse files
CalK16splitt3rsebastian-sauerwxiaoguang
authored
Add reviewers selection to new pull request (#32403)
Users could add reviewers when creating new PRs. --------- Co-authored-by: splitt3r <[email protected]> Co-authored-by: Sebastian Sauer <[email protected]> Co-authored-by: bb-ben <[email protected]> Co-authored-by: wxiaoguang <[email protected]>
1 parent d80f99e commit 18aeca5

26 files changed

+500
-268
lines changed

‎modules/structs/pull.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,9 @@ type CreatePullRequestOption struct {
8686
Milestone int64 `json:"milestone"`
8787
Labels []int64 `json:"labels"`
8888
// swagger:strfmt date-time
89-
Deadline *time.Time `json:"due_date"`
89+
Deadline *time.Time `json:"due_date"`
90+
Reviewers []string `json:"reviewers"`
91+
TeamReviewers []string `json:"team_reviewers"`
9092
}
9193

9294
// EditPullRequestOption options when modify pull request

‎options/locale/locale_en-US.ini

+1-1
Original file line numberDiff line numberDiff line change
@@ -1462,7 +1462,7 @@ issues.new.closed_milestone = Closed Milestones
14621462
issues.new.assignees = Assignees
14631463
issues.new.clear_assignees = Clear assignees
14641464
issues.new.no_assignees = No Assignees
1465-
issues.new.no_reviewers = No reviewers
1465+
issues.new.no_reviewers = No Reviewers
14661466
issues.new.blocked_user = Cannot create issue because you are blocked by the repository owner.
14671467
issues.edit.already_changed = Unable to save changes to the issue. It appears the content has already been changed by another user. Please refresh the page and try editing again to avoid overwriting their changes
14681468
issues.edit.blocked_user = Cannot edit content because you are blocked by the poster or repository owner.

‎routers/api/v1/repo/pull.go

+13-1
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,19 @@ func CreatePullRequest(ctx *context.APIContext) {
554554
}
555555
}
556556

557-
if err := pull_service.NewPullRequest(ctx, repo, prIssue, labelIDs, []string{}, pr, assigneeIDs); err != nil {
557+
prOpts := &pull_service.NewPullRequestOptions{
558+
Repo: repo,
559+
Issue: prIssue,
560+
LabelIDs: labelIDs,
561+
PullRequest: pr,
562+
AssigneeIDs: assigneeIDs,
563+
}
564+
prOpts.Reviewers, prOpts.TeamReviewers = parseReviewersByNames(ctx, form.Reviewers, form.TeamReviewers)
565+
if ctx.Written() {
566+
return
567+
}
568+
569+
if err := pull_service.NewPullRequest(ctx, prOpts); err != nil {
558570
if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) {
559571
ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err)
560572
} else if errors.Is(err, user_model.ErrBlockedUser) {

‎routers/api/v1/repo/pull_review.go

+54-54
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,47 @@ func DeleteReviewRequests(ctx *context.APIContext) {
656656
apiReviewRequest(ctx, *opts, false)
657657
}
658658

659+
func parseReviewersByNames(ctx *context.APIContext, reviewerNames, teamReviewerNames []string) (reviewers []*user_model.User, teamReviewers []*organization.Team) {
660+
var err error
661+
for _, r := range reviewerNames {
662+
var reviewer *user_model.User
663+
if strings.Contains(r, "@") {
664+
reviewer, err = user_model.GetUserByEmail(ctx, r)
665+
} else {
666+
reviewer, err = user_model.GetUserByName(ctx, r)
667+
}
668+
669+
if err != nil {
670+
if user_model.IsErrUserNotExist(err) {
671+
ctx.NotFound("UserNotExist", fmt.Sprintf("User '%s' not exist", r))
672+
return nil, nil
673+
}
674+
ctx.Error(http.StatusInternalServerError, "GetUser", err)
675+
return nil, nil
676+
}
677+
678+
reviewers = append(reviewers, reviewer)
679+
}
680+
681+
if ctx.Repo.Repository.Owner.IsOrganization() && len(teamReviewerNames) > 0 {
682+
for _, t := range teamReviewerNames {
683+
var teamReviewer *organization.Team
684+
teamReviewer, err = organization.GetTeam(ctx, ctx.Repo.Owner.ID, t)
685+
if err != nil {
686+
if organization.IsErrTeamNotExist(err) {
687+
ctx.NotFound("TeamNotExist", fmt.Sprintf("Team '%s' not exist", t))
688+
return nil, nil
689+
}
690+
ctx.Error(http.StatusInternalServerError, "ReviewRequest", err)
691+
return nil, nil
692+
}
693+
694+
teamReviewers = append(teamReviewers, teamReviewer)
695+
}
696+
}
697+
return reviewers, teamReviewers
698+
}
699+
659700
func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions, isAdd bool) {
660701
pr, err := issues_model.GetPullRequestByIndex(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64(":index"))
661702
if err != nil {
@@ -672,42 +713,15 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions
672713
return
673714
}
674715

675-
reviewers := make([]*user_model.User, 0, len(opts.Reviewers))
676-
677716
permDoer, err := access_model.GetUserRepoPermission(ctx, pr.Issue.Repo, ctx.Doer)
678717
if err != nil {
679718
ctx.Error(http.StatusInternalServerError, "GetUserRepoPermission", err)
680719
return
681720
}
682721

683-
for _, r := range opts.Reviewers {
684-
var reviewer *user_model.User
685-
if strings.Contains(r, "@") {
686-
reviewer, err = user_model.GetUserByEmail(ctx, r)
687-
} else {
688-
reviewer, err = user_model.GetUserByName(ctx, r)
689-
}
690-
691-
if err != nil {
692-
if user_model.IsErrUserNotExist(err) {
693-
ctx.NotFound("UserNotExist", fmt.Sprintf("User '%s' not exist", r))
694-
return
695-
}
696-
ctx.Error(http.StatusInternalServerError, "GetUser", err)
697-
return
698-
}
699-
700-
err = issue_service.IsValidReviewRequest(ctx, reviewer, ctx.Doer, isAdd, pr.Issue, &permDoer)
701-
if err != nil {
702-
if issues_model.IsErrNotValidReviewRequest(err) {
703-
ctx.Error(http.StatusUnprocessableEntity, "NotValidReviewRequest", err)
704-
return
705-
}
706-
ctx.Error(http.StatusInternalServerError, "IsValidReviewRequest", err)
707-
return
708-
}
709-
710-
reviewers = append(reviewers, reviewer)
722+
reviewers, teamReviewers := parseReviewersByNames(ctx, opts.Reviewers, opts.TeamReviewers)
723+
if ctx.Written() {
724+
return
711725
}
712726

713727
var reviews []*issues_model.Review
@@ -716,12 +730,16 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions
716730
}
717731

718732
for _, reviewer := range reviewers {
719-
comment, err := issue_service.ReviewRequest(ctx, pr.Issue, ctx.Doer, reviewer, isAdd)
733+
comment, err := issue_service.ReviewRequest(ctx, pr.Issue, ctx.Doer, &permDoer, reviewer, isAdd)
720734
if err != nil {
721735
if issues_model.IsErrReviewRequestOnClosedPR(err) {
722736
ctx.Error(http.StatusForbidden, "", err)
723737
return
724738
}
739+
if issues_model.IsErrNotValidReviewRequest(err) {
740+
ctx.Error(http.StatusUnprocessableEntity, "", err)
741+
return
742+
}
725743
ctx.Error(http.StatusInternalServerError, "ReviewRequest", err)
726744
return
727745
}
@@ -736,35 +754,17 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions
736754
}
737755

738756
if ctx.Repo.Repository.Owner.IsOrganization() && len(opts.TeamReviewers) > 0 {
739-
teamReviewers := make([]*organization.Team, 0, len(opts.TeamReviewers))
740-
for _, t := range opts.TeamReviewers {
741-
var teamReviewer *organization.Team
742-
teamReviewer, err = organization.GetTeam(ctx, ctx.Repo.Owner.ID, t)
757+
for _, teamReviewer := range teamReviewers {
758+
comment, err := issue_service.TeamReviewRequest(ctx, pr.Issue, ctx.Doer, teamReviewer, isAdd)
743759
if err != nil {
744-
if organization.IsErrTeamNotExist(err) {
745-
ctx.NotFound("TeamNotExist", fmt.Sprintf("Team '%s' not exist", t))
760+
if issues_model.IsErrReviewRequestOnClosedPR(err) {
761+
ctx.Error(http.StatusForbidden, "", err)
746762
return
747763
}
748-
ctx.Error(http.StatusInternalServerError, "ReviewRequest", err)
749-
return
750-
}
751-
752-
err = issue_service.IsValidTeamReviewRequest(ctx, teamReviewer, ctx.Doer, isAdd, pr.Issue)
753-
if err != nil {
754764
if issues_model.IsErrNotValidReviewRequest(err) {
755-
ctx.Error(http.StatusUnprocessableEntity, "NotValidReviewRequest", err)
765+
ctx.Error(http.StatusUnprocessableEntity, "", err)
756766
return
757767
}
758-
ctx.Error(http.StatusInternalServerError, "IsValidTeamReviewRequest", err)
759-
return
760-
}
761-
762-
teamReviewers = append(teamReviewers, teamReviewer)
763-
}
764-
765-
for _, teamReviewer := range teamReviewers {
766-
comment, err := issue_service.TeamReviewRequest(ctx, pr.Issue, ctx.Doer, teamReviewer, isAdd)
767-
if err != nil {
768768
ctx.ServerError("TeamReviewRequest", err)
769769
return
770770
}

‎routers/web/repo/compare.go

+4
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,10 @@ func CompareDiff(ctx *context.Context) {
792792
if ctx.Written() {
793793
return
794794
}
795+
RetrieveRepoReviewers(ctx, ctx.Repo.Repository, nil, true)
796+
if ctx.Written() {
797+
return
798+
}
795799
}
796800
}
797801
beforeCommitID := ctx.Data["BeforeCommitID"].(string)

0 commit comments

Comments
 (0)