@@ -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+
659700func 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 }
0 commit comments