Skip to content

Commit 6fb9583

Browse files
wxiaoguangwxiaoguang
committed
fix duplicate code
1 parent 8de6e40 commit 6fb9583

File tree

8 files changed

+110
-158
lines changed

8 files changed

+110
-158
lines changed

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

Lines changed: 6 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
activities_model "code.gitea.io/gitea/models/activities"
1717
git_model "code.gitea.io/gitea/models/git"
1818
issues_model "code.gitea.io/gitea/models/issues"
19-
"code.gitea.io/gitea/models/organization"
2019
access_model "code.gitea.io/gitea/models/perm/access"
2120
pull_model "code.gitea.io/gitea/models/pull"
2221
repo_model "code.gitea.io/gitea/models/repo"
@@ -554,53 +553,20 @@ func CreatePullRequest(ctx *context.APIContext) {
554553
return
555554
}
556555
}
557-
// handle reviewers
558-
var reviewerIDs []int64
559-
560-
for _, r := range form.Reviewers {
561-
var reviewer *user_model.User
562-
if strings.Contains(r, "@") {
563-
reviewer, err = user_model.GetUserByEmail(ctx, r)
564-
} else {
565-
reviewer, err = user_model.GetUserByName(ctx, r)
566-
}
567-
568-
if err != nil {
569-
if user_model.IsErrUserNotExist(err) {
570-
ctx.NotFound("UserNotExist", fmt.Sprintf("User with id '%s' not exist", r))
571-
return
572-
}
573-
ctx.Error(http.StatusInternalServerError, "GetUser", err)
574-
return
575-
}
576-
reviewerIDs = append(reviewerIDs, reviewer.ID)
577-
}
578-
579-
// handle teams as reviewers
580-
if ctx.Repo.Repository.Owner.IsOrganization() && len(form.TeamReviewers) > 0 {
581-
for _, t := range form.TeamReviewers {
582-
var teamReviewer *organization.Team
583-
teamReviewer, err = organization.GetTeam(ctx, ctx.Repo.Owner.ID, t)
584-
if err != nil {
585-
if organization.IsErrTeamNotExist(err) {
586-
ctx.NotFound("TeamNotExist", fmt.Sprintf("Team '%s' not exist", t))
587-
return
588-
}
589-
ctx.Error(http.StatusInternalServerError, "ReviewRequest", err)
590-
return
591-
}
592-
reviewerIDs = append(reviewerIDs, -teamReviewer.ID)
593-
}
594-
}
595556

596557
prOpts := &pull_service.NewPullRequestOptions{
597558
Repo: repo,
598559
Issue: prIssue,
599560
LabelIDs: labelIDs,
600561
PullRequest: pr,
601562
AssigneeIDs: assigneeIDs,
602-
ReviewerIDs: reviewerIDs,
603563
}
564+
565+
prOpts.Reviewers, prOpts.TeamReviewers = parseReviewersByNames(ctx, form.Reviewers, form.TeamReviewers)
566+
if ctx.Written() {
567+
return
568+
}
569+
604570
if err := pull_service.NewPullRequest(ctx, prOpts); err != nil {
605571
if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) {
606572
ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err)

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

Lines changed: 54 additions & 54 deletions
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/issue.go‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,8 +1160,11 @@ func DeleteIssue(ctx *context.Context) {
11601160

11611161
// ValidateRepoMetas check and returns repository's meta information
11621162
func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull bool) (ret struct {
1163-
LabelIDs, AssigneeIDs, ReviewerIDs []int64
1164-
MilestoneID, ProjectID int64
1163+
LabelIDs, AssigneeIDs []int64
1164+
MilestoneID, ProjectID int64
1165+
1166+
Reviewers []*user_model.User
1167+
TeamReviewers []*organization.Team
11651168
},
11661169
) {
11671170
var (
@@ -1263,34 +1266,37 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull
12631266
}
12641267

12651268
// Check reviewers
1266-
var reviewerIDs []int64
1269+
var reviewers []*user_model.User
1270+
var teamReviewers []*organization.Team
12671271
if isPull && len(form.ReviewerIDs) > 0 {
1268-
reviewerIDs, err = base.StringsToInt64s(strings.Split(form.ReviewerIDs, ","))
1272+
reviewerIDs, err := base.StringsToInt64s(strings.Split(form.ReviewerIDs, ","))
12691273
if err != nil {
12701274
return ret
12711275
}
1272-
12731276
// Check if the passed reviewers (user/team) actually exist
12741277
for _, rID := range reviewerIDs {
12751278
// negative reviewIDs represent team requests
12761279
if rID < 0 {
1277-
_, err := organization.GetTeamByID(ctx, -rID)
1280+
teamReviewer, err := organization.GetTeamByID(ctx, -rID)
12781281
if err != nil {
12791282
ctx.ServerError("GetTeamByID", err)
12801283
return ret
12811284
}
1285+
teamReviewers = append(teamReviewers, teamReviewer)
12821286
continue
12831287
}
12841288

1285-
_, err := user_model.GetUserByID(ctx, rID)
1289+
reviewer, err := user_model.GetUserByID(ctx, rID)
12861290
if err != nil {
12871291
ctx.ServerError("GetUserByID", err)
12881292
return ret
12891293
}
1294+
reviewers = append(reviewers, reviewer)
12901295
}
12911296
}
12921297

1293-
ret.LabelIDs, ret.AssigneeIDs, ret.ReviewerIDs, ret.MilestoneID, ret.ProjectID = labelIDs, assigneeIDs, reviewerIDs, milestoneID, form.ProjectID
1298+
ret.LabelIDs, ret.AssigneeIDs, ret.MilestoneID, ret.ProjectID = labelIDs, assigneeIDs, milestoneID, form.ProjectID
1299+
ret.Reviewers, ret.TeamReviewers = reviewers, teamReviewers
12941300
return ret
12951301
}
12961302

@@ -2556,7 +2562,7 @@ func UpdatePullReviewRequest(ctx *context.Context) {
25562562
return
25572563
}
25582564

2559-
err = issue_service.IsValidTeamReviewRequest(ctx, team, ctx.Doer, action == "attach", issue)
2565+
_, err = issue_service.TeamReviewRequest(ctx, issue, ctx.Doer, team, action == "attach")
25602566
if err != nil {
25612567
if issues_model.IsErrNotValidReviewRequest(err) {
25622568
log.Warn(
@@ -2567,12 +2573,6 @@ func UpdatePullReviewRequest(ctx *context.Context) {
25672573
ctx.Status(http.StatusForbidden)
25682574
return
25692575
}
2570-
ctx.ServerError("IsValidTeamReviewRequest", err)
2571-
return
2572-
}
2573-
2574-
_, err = issue_service.TeamReviewRequest(ctx, issue, ctx.Doer, team, action == "attach")
2575-
if err != nil {
25762576
ctx.ServerError("TeamReviewRequest", err)
25772577
return
25782578
}
@@ -2594,7 +2594,7 @@ func UpdatePullReviewRequest(ctx *context.Context) {
25942594
return
25952595
}
25962596

2597-
err = issue_service.IsValidReviewRequest(ctx, reviewer, ctx.Doer, action == "attach", issue, nil)
2597+
_, err = issue_service.ReviewRequest(ctx, issue, ctx.Doer, &ctx.Repo.Permission, reviewer, action == "attach")
25982598
if err != nil {
25992599
if issues_model.IsErrNotValidReviewRequest(err) {
26002600
log.Warn(
@@ -2605,12 +2605,6 @@ func UpdatePullReviewRequest(ctx *context.Context) {
26052605
ctx.Status(http.StatusForbidden)
26062606
return
26072607
}
2608-
ctx.ServerError("isValidReviewRequest", err)
2609-
return
2610-
}
2611-
2612-
_, err = issue_service.ReviewRequest(ctx, issue, ctx.Doer, reviewer, action == "attach")
2613-
if err != nil {
26142608
if issues_model.IsErrReviewRequestOnClosedPR(err) {
26152609
ctx.Status(http.StatusForbidden)
26162610
return

‎routers/web/repo/pull.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1274,7 +1274,7 @@ func CompareAndPullRequestPost(ctx *context.Context) {
12741274
return
12751275
}
12761276

1277-
labelIDs, assigneeIDs, reviewerIDs, milestoneID, projectID := validateRet.LabelIDs, validateRet.AssigneeIDs, validateRet.ReviewerIDs, validateRet.MilestoneID, validateRet.ProjectID
1277+
labelIDs, assigneeIDs, milestoneID, projectID := validateRet.LabelIDs, validateRet.AssigneeIDs, validateRet.MilestoneID, validateRet.ProjectID
12781278

12791279
if setting.Attachment.Enabled {
12801280
attachments = form.Files
@@ -1327,7 +1327,8 @@ func CompareAndPullRequestPost(ctx *context.Context) {
13271327
AttachmentUUIDs: attachments,
13281328
PullRequest: pullRequest,
13291329
AssigneeIDs: assigneeIDs,
1330-
ReviewerIDs: reviewerIDs,
1330+
Reviewers: validateRet.Reviewers,
1331+
TeamReviewers: validateRet.TeamReviewers,
13311332
}
13321333
if err := pull_service.NewPullRequest(ctx, prOpts); err != nil {
13331334
switch {

0 commit comments

Comments
 (0)