Add reviewers selection to new pull request (#32403)

Users could add reviewers when creating new PRs.

---------

Co-authored-by: splitt3r <splitt3r@users.noreply.github.com>
Co-authored-by: Sebastian Sauer <sauer.sebastian@gmail.com>
Co-authored-by: bb-ben <70356237+bboerben@users.noreply.github.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

modules/structs/pull.go

@@ -86,7 +86,9 @@ type CreatePullRequestOption struct {
 	Milestone int64    `json:"milestone"`
 	Labels    []int64  `json:"labels"`
 	// swagger:strfmt date-time
-	Deadline *time.Time `json:"due_date"`
+	Deadline      *time.Time `json:"due_date"`
+	Reviewers     []string   `json:"reviewers"`
+	TeamReviewers []string   `json:"team_reviewers"`
 }
 
 // EditPullRequestOption options when modify pull request

options/locale/locale_en-US.ini

@@ -1462,7 +1462,7 @@ issues.new.closed_milestone = Closed Milestones
 issues.new.assignees = Assignees
 issues.new.clear_assignees = Clear assignees
 issues.new.no_assignees = No Assignees
-issues.new.no_reviewers = No reviewers
+issues.new.no_reviewers = No Reviewers
 issues.new.blocked_user = Cannot create issue because you are blocked by the repository owner.
 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
 issues.edit.blocked_user = Cannot edit content because you are blocked by the poster or repository owner.

routers/api/v1/repo/pull.go

@@ -554,7 +554,19 @@ func CreatePullRequest(ctx *context.APIContext) {
 		}
 	}
 
-	if err := pull_service.NewPullRequest(ctx, repo, prIssue, labelIDs, []string{}, pr, assigneeIDs); err != nil {
+	prOpts := &pull_service.NewPullRequestOptions{
+		Repo:        repo,
+		Issue:       prIssue,
+		LabelIDs:    labelIDs,
+		PullRequest: pr,
+		AssigneeIDs: assigneeIDs,
+	}
+	prOpts.Reviewers, prOpts.TeamReviewers = parseReviewersByNames(ctx, form.Reviewers, form.TeamReviewers)
+	if ctx.Written() {
+		return
+	}
+
+	if err := pull_service.NewPullRequest(ctx, prOpts); err != nil {
 		if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) {
 			ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err)
 		} else if errors.Is(err, user_model.ErrBlockedUser) {

routers/api/v1/repo/pull_review.go

@@ -656,6 +656,47 @@ func DeleteReviewRequests(ctx *context.APIContext) {
 	apiReviewRequest(ctx, *opts, false)
 }
 
+func parseReviewersByNames(ctx *context.APIContext, reviewerNames, teamReviewerNames []string) (reviewers []*user_model.User, teamReviewers []*organization.Team) {
+	var err error
+	for _, r := range reviewerNames {
+		var reviewer *user_model.User
+		if strings.Contains(r, "@") {
+			reviewer, err = user_model.GetUserByEmail(ctx, r)
+		} else {
+			reviewer, err = user_model.GetUserByName(ctx, r)
+		}
+
+		if err != nil {
+			if user_model.IsErrUserNotExist(err) {
+				ctx.NotFound("UserNotExist", fmt.Sprintf("User '%s' not exist", r))
+				return nil, nil
+			}
+			ctx.Error(http.StatusInternalServerError, "GetUser", err)
+			return nil, nil
+		}
+
+		reviewers = append(reviewers, reviewer)
+	}
+
+	if ctx.Repo.Repository.Owner.IsOrganization() && len(teamReviewerNames) > 0 {
+		for _, t := range teamReviewerNames {
+			var teamReviewer *organization.Team
+			teamReviewer, err = organization.GetTeam(ctx, ctx.Repo.Owner.ID, t)
+			if err != nil {
+				if organization.IsErrTeamNotExist(err) {
+					ctx.NotFound("TeamNotExist", fmt.Sprintf("Team '%s' not exist", t))
+					return nil, nil
+				}
+				ctx.Error(http.StatusInternalServerError, "ReviewRequest", err)
+				return nil, nil
+			}
+
+			teamReviewers = append(teamReviewers, teamReviewer)
+		}
+	}
+	return reviewers, teamReviewers
+}
+
 func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions, isAdd bool) {
 	pr, err := issues_model.GetPullRequestByIndex(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64(":index"))
 	if err != nil {
@@ -672,42 +713,15 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions
 		return
 	}
 
-	reviewers := make([]*user_model.User, 0, len(opts.Reviewers))
-
 	permDoer, err := access_model.GetUserRepoPermission(ctx, pr.Issue.Repo, ctx.Doer)
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "GetUserRepoPermission", err)
 		return
 	}
 
-	for _, r := range opts.Reviewers {
+	reviewers, teamReviewers := parseReviewersByNames(ctx, opts.Reviewers, opts.TeamReviewers)
-		var reviewer *user_model.User
+	if ctx.Written() {
-		if strings.Contains(r, "@") {
+		return
-			reviewer, err = user_model.GetUserByEmail(ctx, r)
-		} else {
-			reviewer, err = user_model.GetUserByName(ctx, r)
-		}
-
-		if err != nil {
-			if user_model.IsErrUserNotExist(err) {
-				ctx.NotFound("UserNotExist", fmt.Sprintf("User '%s' not exist", r))
-				return
-			}
-			ctx.Error(http.StatusInternalServerError, "GetUser", err)
-			return
-		}
-
-		err = issue_service.IsValidReviewRequest(ctx, reviewer, ctx.Doer, isAdd, pr.Issue, &permDoer)
-		if err != nil {
-			if issues_model.IsErrNotValidReviewRequest(err) {
-				ctx.Error(http.StatusUnprocessableEntity, "NotValidReviewRequest", err)
-				return
-			}
-			ctx.Error(http.StatusInternalServerError, "IsValidReviewRequest", err)
-			return
-		}
-
-		reviewers = append(reviewers, reviewer)
 	}
 
 	var reviews []*issues_model.Review
@@ -716,12 +730,16 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions
 	}
 
 	for _, reviewer := range reviewers {
-		comment, err := issue_service.ReviewRequest(ctx, pr.Issue, ctx.Doer, reviewer, isAdd)
+		comment, err := issue_service.ReviewRequest(ctx, pr.Issue, ctx.Doer, &permDoer, reviewer, isAdd)
 		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, "ReviewRequest", err)
 			return
 		}
@@ -736,35 +754,17 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions
 	}
 
 	if ctx.Repo.Repository.Owner.IsOrganization() && len(opts.TeamReviewers) > 0 {
-		teamReviewers := make([]*organization.Team, 0, len(opts.TeamReviewers))
-		for _, t := range opts.TeamReviewers {
-			var teamReviewer *organization.Team
-			teamReviewer, err = organization.GetTeam(ctx, ctx.Repo.Owner.ID, t)
-			if err != nil {
-				if organization.IsErrTeamNotExist(err) {
-					ctx.NotFound("TeamNotExist", fmt.Sprintf("Team '%s' not exist", t))
-					return
-				}
-				ctx.Error(http.StatusInternalServerError, "ReviewRequest", err)
-				return
-			}
-
-			err = issue_service.IsValidTeamReviewRequest(ctx, teamReviewer, ctx.Doer, isAdd, pr.Issue)
-			if err != nil {
-				if issues_model.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(ctx, pr.Issue, ctx.Doer, teamReviewer, isAdd)
 			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.ServerError("TeamReviewRequest", err)
 				return
 			}

routers/web/repo/compare.go

@@ -792,6 +792,10 @@ func CompareDiff(ctx *context.Context) {
 			if ctx.Written() {
 				return
 			}
+			RetrieveRepoReviewers(ctx, ctx.Repo.Repository, nil, true)
+			if ctx.Written() {
+				return
+			}
 		}
 	}
 	beforeCommitID := ctx.Data["BeforeCommitID"].(string)

routers/web/repo/issue.go

@@ -654,34 +654,66 @@ func retrieveProjects(ctx *context.Context, repo *repo_model.Repository) {
 
 // repoReviewerSelection items to bee shown
 type repoReviewerSelection struct {
-	IsTeam    bool
+	IsTeam         bool
-	Team      *organization.Team
+	Team           *organization.Team
-	User      *user_model.User
+	User           *user_model.User
-	Review    *issues_model.Review
+	Review         *issues_model.Review
-	CanChange bool
+	CanBeDismissed bool
-	Checked   bool
+	CanChange      bool
-	ItemID    int64
+	Requested      bool
+	ItemID         int64
 }
 
-// RetrieveRepoReviewers find all reviewers of a repository
+type issueSidebarReviewersData struct {
+	Repository           *repo_model.Repository
+	RepoOwnerName        string
+	RepoLink             string
+	IssueID              int64
+	CanChooseReviewer    bool
+	OriginalReviews      issues_model.ReviewList
+	TeamReviewers        []*repoReviewerSelection
+	Reviewers            []*repoReviewerSelection
+	CurrentPullReviewers []*repoReviewerSelection
+}
+
+// RetrieveRepoReviewers find all reviewers of a repository. If issue is nil, it means the doer is creating a new PR.
 func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, issue *issues_model.Issue, canChooseReviewer bool) {
-	ctx.Data["CanChooseReviewer"] = canChooseReviewer
+	data := &issueSidebarReviewersData{}
+	data.RepoLink = ctx.Repo.RepoLink
+	data.Repository = repo
+	data.RepoOwnerName = repo.OwnerName
+	data.CanChooseReviewer = canChooseReviewer
 
-	originalAuthorReviews, err := issues_model.GetReviewersFromOriginalAuthorsByIssueID(ctx, issue.ID)
+	var posterID int64
-	if err != nil {
+	var isClosed bool
-		ctx.ServerError("GetReviewersFromOriginalAuthorsByIssueID", err)
+	var reviews issues_model.ReviewList
-		return
-	}
-	ctx.Data["OriginalReviews"] = originalAuthorReviews
 
-	reviews, err := issues_model.GetReviewsByIssueID(ctx, issue.ID)
+	if issue == nil {
-	if err != nil {
+		posterID = ctx.Doer.ID
-		ctx.ServerError("GetReviewersByIssueID", err)
+	} else {
-		return
+		posterID = issue.PosterID
-	}
+		if issue.OriginalAuthorID > 0 {
+			posterID = 0 // for migrated PRs, no poster ID
+		}
 
-	if len(reviews) == 0 && !canChooseReviewer {
+		data.IssueID = issue.ID
-		return
+		isClosed = issue.IsClosed || issue.PullRequest.HasMerged
+
+		originalAuthorReviews, err := issues_model.GetReviewersFromOriginalAuthorsByIssueID(ctx, issue.ID)
+		if err != nil {
+			ctx.ServerError("GetReviewersFromOriginalAuthorsByIssueID", err)
+			return
+		}
+		data.OriginalReviews = originalAuthorReviews
+
+		reviews, err = issues_model.GetReviewsByIssueID(ctx, issue.ID)
+		if err != nil {
+			ctx.ServerError("GetReviewersByIssueID", err)
+			return
+		}
+		if len(reviews) == 0 && !canChooseReviewer {
+			return
+		}
 	}
 
 	var (
@@ -693,11 +725,7 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is
 	)
 
 	if canChooseReviewer {
-		posterID := issue.PosterID
+		var err error
-		if issue.OriginalAuthorID > 0 {
-			posterID = 0
-		}
-
 		reviewers, err = repo_model.GetReviewers(ctx, repo, ctx.Doer.ID, posterID)
 		if err != nil {
 			ctx.ServerError("GetReviewers", err)
@@ -723,9 +751,9 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is
 
 	for _, review := range reviews {
 		tmp := &repoReviewerSelection{
-			Checked: review.Type == issues_model.ReviewTypeRequest,
+			Requested: review.Type == issues_model.ReviewTypeRequest,
-			Review:  review,
+			Review:    review,
-			ItemID:  review.ReviewerID,
+			ItemID:    review.ReviewerID,
 		}
 		if review.ReviewerTeamID > 0 {
 			tmp.IsTeam = true
@@ -756,7 +784,7 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is
 		currentPullReviewers := make([]*repoReviewerSelection, 0, len(pullReviews))
 		for _, item := range pullReviews {
 			if item.Review.ReviewerID > 0 {
-				if err = item.Review.LoadReviewer(ctx); err != nil {
+				if err := item.Review.LoadReviewer(ctx); err != nil {
 					if user_model.IsErrUserNotExist(err) {
 						continue
 					}
@@ -765,7 +793,7 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is
 				}
 				item.User = item.Review.Reviewer
 			} else if item.Review.ReviewerTeamID > 0 {
-				if err = item.Review.LoadReviewerTeam(ctx); err != nil {
+				if err := item.Review.LoadReviewerTeam(ctx); err != nil {
 					if organization.IsErrTeamNotExist(err) {
 						continue
 					}
@@ -776,10 +804,11 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is
 			} else {
 				continue
 			}
-
+			item.CanBeDismissed = ctx.Repo.Permission.IsAdmin() && !isClosed &&
+				(item.Review.Type == issues_model.ReviewTypeApprove || item.Review.Type == issues_model.ReviewTypeReject)
 			currentPullReviewers = append(currentPullReviewers, item)
 		}
-		ctx.Data["PullReviewers"] = currentPullReviewers
+		data.CurrentPullReviewers = currentPullReviewers
 	}
 
 	if canChooseReviewer && reviewersResult != nil {
@@ -807,7 +836,7 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is
 			})
 		}
 
-		ctx.Data["Reviewers"] = reviewersResult
+		data.Reviewers = reviewersResult
 	}
 
 	if canChooseReviewer && teamReviewersResult != nil {
@@ -835,8 +864,10 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is
 			})
 		}
 
-		ctx.Data["TeamReviewers"] = teamReviewersResult
+		data.TeamReviewers = teamReviewersResult
 	}
+
+	ctx.Data["IssueSidebarReviewersData"] = data
 }
 
 // RetrieveRepoMetas find all the meta information of a repository
@@ -1117,7 +1148,14 @@ func DeleteIssue(ctx *context.Context) {
 }
 
 // ValidateRepoMetas check and returns repository's meta information
-func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull bool) ([]int64, []int64, int64, int64) {
+func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull bool) (ret struct {
+	LabelIDs, AssigneeIDs  []int64
+	MilestoneID, ProjectID int64
+
+	Reviewers     []*user_model.User
+	TeamReviewers []*organization.Team
+},
+) {
 	var (
 		repo = ctx.Repo.Repository
 		err  error
@@ -1125,7 +1163,7 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull
 
 	labels := RetrieveRepoMetas(ctx, ctx.Repo.Repository, isPull)
 	if ctx.Written() {
-		return nil, nil, 0, 0
+		return ret
 	}
 
 	var labelIDs []int64
@@ -1134,7 +1172,7 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull
 	if len(form.LabelIDs) > 0 {
 		labelIDs, err = base.StringsToInt64s(strings.Split(form.LabelIDs, ","))
 		if err != nil {
-			return nil, nil, 0, 0
+			return ret
 		}
 		labelIDMark := make(container.Set[int64])
 		labelIDMark.AddMultiple(labelIDs...)
@@ -1157,11 +1195,11 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull
 		milestone, err := issues_model.GetMilestoneByRepoID(ctx, ctx.Repo.Repository.ID, milestoneID)
 		if err != nil {
 			ctx.ServerError("GetMilestoneByID", err)
-			return nil, nil, 0, 0
+			return ret
 		}
 		if milestone.RepoID != repo.ID {
 			ctx.ServerError("GetMilestoneByID", err)
-			return nil, nil, 0, 0
+			return ret
 		}
 		ctx.Data["Milestone"] = milestone
 		ctx.Data["milestone_id"] = milestoneID
@@ -1171,11 +1209,11 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull
 		p, err := project_model.GetProjectByID(ctx, form.ProjectID)
 		if err != nil {
 			ctx.ServerError("GetProjectByID", err)
-			return nil, nil, 0, 0
+			return ret
 		}
 		if p.RepoID != ctx.Repo.Repository.ID && p.OwnerID != ctx.Repo.Repository.OwnerID {
 			ctx.NotFound("", nil)
-			return nil, nil, 0, 0
+			return ret
 		}
 
 		ctx.Data["Project"] = p
@@ -1187,7 +1225,7 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull
 	if len(form.AssigneeIDs) > 0 {
 		assigneeIDs, err = base.StringsToInt64s(strings.Split(form.AssigneeIDs, ","))
 		if err != nil {
-			return nil, nil, 0, 0
+			return ret
 		}
 
 		// Check if the passed assignees actually exists and is assignable
@@ -1195,18 +1233,18 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull
 			assignee, err := user_model.GetUserByID(ctx, aID)
 			if err != nil {
 				ctx.ServerError("GetUserByID", err)
-				return nil, nil, 0, 0
+				return ret
 			}
 
 			valid, err := access_model.CanBeAssigned(ctx, assignee, repo, isPull)
 			if err != nil {
 				ctx.ServerError("CanBeAssigned", err)
-				return nil, nil, 0, 0
+				return ret
 			}
 
 			if !valid {
 				ctx.ServerError("canBeAssigned", repo_model.ErrUserDoesNotHaveAccessToRepo{UserID: aID, RepoName: repo.Name})
-				return nil, nil, 0, 0
+				return ret
 			}
 		}
 	}
@@ -1216,7 +1254,39 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull
 		assigneeIDs = append(assigneeIDs, form.AssigneeID)
 	}
 
-	return labelIDs, assigneeIDs, milestoneID, form.ProjectID
+	// Check reviewers
+	var reviewers []*user_model.User
+	var teamReviewers []*organization.Team
+	if isPull && len(form.ReviewerIDs) > 0 {
+		reviewerIDs, err := base.StringsToInt64s(strings.Split(form.ReviewerIDs, ","))
+		if err != nil {
+			return ret
+		}
+		// Check if the passed reviewers (user/team) actually exist
+		for _, rID := range reviewerIDs {
+			// negative reviewIDs represent team requests
+			if rID < 0 {
+				teamReviewer, err := organization.GetTeamByID(ctx, -rID)
+				if err != nil {
+					ctx.ServerError("GetTeamByID", err)
+					return ret
+				}
+				teamReviewers = append(teamReviewers, teamReviewer)
+				continue
+			}
+
+			reviewer, err := user_model.GetUserByID(ctx, rID)
+			if err != nil {
+				ctx.ServerError("GetUserByID", err)
+				return ret
+			}
+			reviewers = append(reviewers, reviewer)
+		}
+	}
+
+	ret.LabelIDs, ret.AssigneeIDs, ret.MilestoneID, ret.ProjectID = labelIDs, assigneeIDs, milestoneID, form.ProjectID
+	ret.Reviewers, ret.TeamReviewers = reviewers, teamReviewers
+	return ret
 }
 
 // NewIssuePost response for creating new issue
@@ -1234,11 +1304,13 @@ func NewIssuePost(ctx *context.Context) {
 		attachments []string
 	)
 
-	labelIDs, assigneeIDs, milestoneID, projectID := ValidateRepoMetas(ctx, *form, false)
+	validateRet := ValidateRepoMetas(ctx, *form, false)
 	if ctx.Written() {
 		return
 	}
 
+	labelIDs, assigneeIDs, milestoneID, projectID := validateRet.LabelIDs, validateRet.AssigneeIDs, validateRet.MilestoneID, validateRet.ProjectID
+
 	if projectID > 0 {
 		if !ctx.Repo.CanRead(unit.TypeProjects) {
 			// User must also be able to see the project.
@@ -2479,7 +2551,7 @@ func UpdatePullReviewRequest(ctx *context.Context) {
 				return
 			}
 
-			err = issue_service.IsValidTeamReviewRequest(ctx, team, ctx.Doer, action == "attach", issue)
+			_, err = issue_service.TeamReviewRequest(ctx, issue, ctx.Doer, team, action == "attach")
 			if err != nil {
 				if issues_model.IsErrNotValidReviewRequest(err) {
 					log.Warn(
@@ -2490,12 +2562,6 @@ func UpdatePullReviewRequest(ctx *context.Context) {
 					ctx.Status(http.StatusForbidden)
 					return
 				}
-				ctx.ServerError("IsValidTeamReviewRequest", err)
-				return
-			}
-
-			_, err = issue_service.TeamReviewRequest(ctx, issue, ctx.Doer, team, action == "attach")
-			if err != nil {
 				ctx.ServerError("TeamReviewRequest", err)
 				return
 			}
@@ -2517,7 +2583,7 @@ func UpdatePullReviewRequest(ctx *context.Context) {
 			return
 		}
 
-		err = issue_service.IsValidReviewRequest(ctx, reviewer, ctx.Doer, action == "attach", issue, nil)
+		_, err = issue_service.ReviewRequest(ctx, issue, ctx.Doer, &ctx.Repo.Permission, reviewer, action == "attach")
 		if err != nil {
 			if issues_model.IsErrNotValidReviewRequest(err) {
 				log.Warn(
@@ -2528,12 +2594,6 @@ func UpdatePullReviewRequest(ctx *context.Context) {
 				ctx.Status(http.StatusForbidden)
 				return
 			}
-			ctx.ServerError("isValidReviewRequest", err)
-			return
-		}
-
-		_, err = issue_service.ReviewRequest(ctx, issue, ctx.Doer, reviewer, action == "attach")
-		if err != nil {
 			if issues_model.IsErrReviewRequestOnClosedPR(err) {
 				ctx.Status(http.StatusForbidden)
 				return

routers/web/repo/pull.go

@@ -1269,11 +1269,13 @@ func CompareAndPullRequestPost(ctx *context.Context) {
 		return
 	}
 
-	labelIDs, assigneeIDs, milestoneID, projectID := ValidateRepoMetas(ctx, *form, true)
+	validateRet := ValidateRepoMetas(ctx, *form, true)
 	if ctx.Written() {
 		return
 	}
 
+	labelIDs, assigneeIDs, milestoneID, projectID := validateRet.LabelIDs, validateRet.AssigneeIDs, validateRet.MilestoneID, validateRet.ProjectID
+
 	if setting.Attachment.Enabled {
 		attachments = form.Files
 	}
@@ -1318,8 +1320,17 @@ func CompareAndPullRequestPost(ctx *context.Context) {
 	}
 	// FIXME: check error in the case two people send pull request at almost same time, give nice error prompt
 	// instead of 500.
-
+	prOpts := &pull_service.NewPullRequestOptions{
-	if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs); err != nil {
+		Repo:            repo,
+		Issue:           pullIssue,
+		LabelIDs:        labelIDs,
+		AttachmentUUIDs: attachments,
+		PullRequest:     pullRequest,
+		AssigneeIDs:     assigneeIDs,
+		Reviewers:       validateRet.Reviewers,
+		TeamReviewers:   validateRet.TeamReviewers,
+	}
+	if err := pull_service.NewPullRequest(ctx, prOpts); err != nil {
 		switch {
 		case repo_model.IsErrUserDoesNotHaveAccessToRepo(err):
 			ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error())

services/agit/agit.go

@@ -137,8 +137,12 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
 				Type:         issues_model.PullRequestGitea,
 				Flow:         issues_model.PullRequestFlowAGit,
 			}
-
+			prOpts := &pull_service.NewPullRequestOptions{
-			if err := pull_service.NewPullRequest(ctx, repo, prIssue, []int64{}, []string{}, pr, []int64{}); err != nil {
+				Repo:        repo,
+				Issue:       prIssue,
+				PullRequest: pr,
+			}
+			if err := pull_service.NewPullRequest(ctx, prOpts); err != nil {
 				return nil, err
 			}
 

services/forms/repo_form.go

@@ -447,6 +447,7 @@ type CreateIssueForm struct {
 	Title               string `binding:"Required;MaxSize(255)"`
 	LabelIDs            string `form:"label_ids"`
 	AssigneeIDs         string `form:"assignee_ids"`
+	ReviewerIDs         string `form:"reviewer_ids"`
 	Ref                 string `form:"ref"`
 	MilestoneID         int64
 	ProjectID           int64

services/issue/assignee.go

@@ -61,7 +61,12 @@ 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, reviewer *user_model.User, isAdd bool) (comment *issues_model.Comment, err error) {
+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)
+	if err != nil {
+		return nil, err
+	}
+
 	if isAdd {
 		comment, err = issues_model.AddReviewRequest(ctx, issue, reviewer, doer)
 	} else {
@@ -79,8 +84,8 @@ func ReviewRequest(ctx context.Context, issue *issues_model.Issue, doer, reviewe
 	return comment, err
 }
 
-// IsValidReviewRequest Check permission for ReviewRequest
+// 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 {
+func isValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User, isAdd bool, issue *issues_model.Issue, permDoer *access_model.Permission) error {
 	if reviewer.IsOrganization() {
 		return issues_model.ErrNotValidReviewRequest{
 			Reason: "Organization can't be added as reviewer",
@@ -109,7 +114,7 @@ func IsValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User,
 		}
 	}
 
-	lastreview, err := issues_model.GetReviewByIssueIDAndUserID(ctx, issue.ID, reviewer.ID)
+	lastReview, err := issues_model.GetReviewByIssueIDAndUserID(ctx, issue.ID, reviewer.ID)
 	if err != nil && !issues_model.IsErrReviewNotExist(err) {
 		return err
 	}
@@ -137,7 +142,7 @@ func IsValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User,
 			return nil
 		}
 
-		if doer.ID == issue.PosterID && issue.OriginalAuthorID == 0 && lastreview != nil && lastreview.Type != issues_model.ReviewTypeRequest {
+		if doer.ID == issue.PosterID && issue.OriginalAuthorID == 0 && lastReview != nil && lastReview.Type != issues_model.ReviewTypeRequest {
 			return nil
 		}
 
@@ -152,7 +157,7 @@ func IsValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User,
 		return nil
 	}
 
-	if lastreview != nil && lastreview.Type == issues_model.ReviewTypeRequest && lastreview.ReviewerID == doer.ID {
+	if lastReview != nil && lastReview.Type == issues_model.ReviewTypeRequest && lastReview.ReviewerID == doer.ID {
 		return nil
 	}
 
@@ -163,8 +168,8 @@ func IsValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User,
 	}
 }
 
-// IsValidTeamReviewRequest Check permission for ReviewRequest Team
+// isValidTeamReviewRequest Check permission for ReviewRequest Team
-func IsValidTeamReviewRequest(ctx context.Context, reviewer *organization.Team, doer *user_model.User, isAdd bool, issue *issues_model.Issue) error {
+func isValidTeamReviewRequest(ctx context.Context, reviewer *organization.Team, doer *user_model.User, isAdd bool, issue *issues_model.Issue) error {
 	if doer.IsOrganization() {
 		return issues_model.ErrNotValidReviewRequest{
 			Reason: "Organization can't be doer to add reviewer",
@@ -212,6 +217,10 @@ 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)
+	if err != nil {
+		return nil, err
+	}
 	if isAdd {
 		comment, err = issues_model.AddTeamReviewRequest(ctx, issue, reviewer, doer)
 	} else {
@@ -268,6 +277,9 @@ func teamReviewRequestNotify(ctx context.Context, issue *issues_model.Issue, doe
 
 // CanDoerChangeReviewRequests returns if the doer can add/remove review requests of a PR
 func CanDoerChangeReviewRequests(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue) bool {
+	if repo.IsArchived {
+		return false
+	}
 	// The poster of the PR can change the reviewers
 	if doer.ID == issue.PosterID {
 		return true

services/mailer/mailer.go

@@ -382,7 +382,7 @@ func (s *dummySender) Send(from string, to []string, msg io.WriterTo) error {
 	if _, err := msg.WriteTo(&buf); err != nil {
 		return err
 	}
-	log.Info("Mail From: %s To: %v Body: %s", from, to, buf.String())
+	log.Debug("Mail From: %s To: %v Body: %s", from, to, buf.String())
 	return nil
 }
 

services/pull/pull.go

@@ -17,6 +17,7 @@ import (
 	"code.gitea.io/gitea/models/db"
 	git_model "code.gitea.io/gitea/models/git"
 	issues_model "code.gitea.io/gitea/models/issues"
+	"code.gitea.io/gitea/models/organization"
 	access_model "code.gitea.io/gitea/models/perm/access"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unit"
@@ -41,8 +42,20 @@ func getPullWorkingLockKey(prID int64) string {
 	return fmt.Sprintf("pull_working_%d", prID)
 }
 
+type NewPullRequestOptions struct {
+	Repo            *repo_model.Repository
+	Issue           *issues_model.Issue
+	LabelIDs        []int64
+	AttachmentUUIDs []string
+	PullRequest     *issues_model.PullRequest
+	AssigneeIDs     []int64
+	Reviewers       []*user_model.User
+	TeamReviewers   []*organization.Team
+}
+
 // NewPullRequest creates new pull request with labels for repository.
-func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string, pr *issues_model.PullRequest, assigneeIDs []int64) error {
+func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
+	repo, issue, labelIDs, uuids, pr, assigneeIDs := opts.Repo, opts.Issue, opts.LabelIDs, opts.AttachmentUUIDs, opts.PullRequest, opts.AssigneeIDs
 	if err := issue.LoadPoster(ctx); err != nil {
 		return err
 	}
@@ -197,7 +210,17 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss
 		}
 		notify_service.IssueChangeAssignee(ctx, issue.Poster, issue, assignee, false, assigneeCommentMap[assigneeID])
 	}
-
+	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 {
+			return err
+		}
+	}
+	for _, teamReviewer := range opts.TeamReviewers {
+		if _, err = issue_service.TeamReviewRequest(ctx, pr.Issue, issue.Poster, teamReviewer, true); err != nil {
+			return err
+		}
+	}
 	return nil
 }
 

templates/repo/issue/new_form.tmpl

@@ -47,7 +47,11 @@
 	</div>
 
 	<div class="issue-content-right ui segment">
-		{{template "repo/issue/branch_selector_field" .}}
+		{{template "repo/issue/branch_selector_field" $}}
+		{{if .PageIsComparePull}}
+			{{template "repo/issue/sidebar/reviewer_list" dict "IssueSidebarReviewersData" $.IssueSidebarReviewersData}}
+			<div class="divider"></div>
+		{{end}}
 
 		<input id="label_ids" name="label_ids" type="hidden" value="{{.label_ids}}">
 		{{template "repo/issue/labels/labels_selector_field" .}}

templates/repo/issue/sidebar/reviewer_list.tmpl

@@ -1,95 +1,79 @@
-<input id="reviewer_id" name="reviewer_id" type="hidden" value="{{.reviewer_id}}">
+{{$data := .IssueSidebarReviewersData}}
-<div class="ui {{if or (and (not .Reviewers) (not .TeamReviewers)) (not .CanChooseReviewer) .Repository.IsArchived}}disabled{{end}} floating jump select-reviewers-modify dropdown">
+{{$hasCandidates := or $data.Reviewers $data.TeamReviewers}}
-	<a class="text tw-flex tw-items-center muted">
+<div class="issue-sidebar-combo" data-sidebar-combo-for="reviewers"
-		<strong>{{ctx.Locale.Tr "repo.issues.review.reviewers"}}</strong>
+		{{if $data.IssueID}}data-update-url="{{$data.RepoLink}}/issues/request_review?issue_ids={{$data.IssueID}}"{{end}}
-		{{if and .CanChooseReviewer (not .Repository.IsArchived)}}
+>
-			{{svg "octicon-gear" 16 "tw-ml-1"}}
+	<input type="hidden" class="combo-value" name="reviewer_ids">{{/* match CreateIssueForm */}}
-		{{end}}
+	<div class="ui dropdown custom {{if or (not $hasCandidates) (not $data.CanChooseReviewer)}}disabled{{end}}">
-	</a>
+		<a class="muted text">
-	<div class="filter menu" data-action="update" data-issue-id="{{$.Issue.ID}}" data-update-url="{{$.RepoLink}}/issues/request_review">
+			<strong>{{ctx.Locale.Tr "repo.issues.review.reviewers"}}</strong> {{if and $data.CanChooseReviewer}}{{svg "octicon-gear"}}{{end}}
-		{{if .Reviewers}}
+		</a>
-			<div class="ui icon search input">
+		<div class="menu flex-items-menu">
-				<i class="icon">{{svg "octicon-search" 16}}</i>
+			{{if $hasCandidates}}
-				<input type="text" placeholder="{{ctx.Locale.Tr "repo.issues.filter_reviewers"}}">
+				<div class="ui icon search input">
-			</div>
+					<i class="icon">{{svg "octicon-search"}}</i>
-		{{end}}
+					<input type="text" placeholder="{{ctx.Locale.Tr "repo.issues.filter_reviewers"}}">
-		{{if .Reviewers}}
+				</div>
-			{{range .Reviewers}}
+			{{end}}
+			{{range $data.Reviewers}}
 				{{if .User}}
-					<a class="{{if not .CanChange}}ui{{end}} item {{if .Checked}}checked{{end}} {{if not .CanChange}}ban-change{{end}}" href="#" data-id="{{.ItemID}}" data-id-selector="#review_request_{{.ItemID}}" {{if not .CanChange}} data-tooltip-content="{{ctx.Locale.Tr "repo.issues.remove_request_review_block"}}"{{end}}>
+					<a class="item muted {{if .Requested}}checked{{end}}" href="{{.User.HomeLink}}" data-value="{{.ItemID}}" data-can-change="{{.CanChange}}"
-						<span class="octicon-check {{if not .Checked}}tw-invisible{{end}}">{{svg "octicon-check"}}</span>
+						{{if not .CanChange}}data-tooltip-content="{{ctx.Locale.Tr "repo.issues.remove_request_review_block"}}"{{end}}>
-						<span class="text">
+						{{svg "octicon-check"}} {{ctx.AvatarUtils.Avatar .User 20}} {{template "repo/search_name" .User}}
-							{{ctx.AvatarUtils.Avatar .User 28 "tw-mr-2"}}{{template "repo/search_name" .User}}
-						</span>
 					</a>
 				{{end}}
 			{{end}}
-		{{end}}
+			{{if $data.TeamReviewers}}
-		{{if .TeamReviewers}}
+				{{if $data.Reviewers}}<div class="divider"></div>{{end}}
-			{{if .Reviewers}}
+				{{range $data.TeamReviewers}}
-				<div class="divider"></div>
+					{{if .Team}}
-			{{end}}
+						<a class="item muted {{if .Requested}}checked{{end}}" href="#" data-value="{{.ItemID}}" data-can-change="{{.CanChange}}"
-			{{range .TeamReviewers}}
+							{{if not .CanChange}} data-tooltip-content="{{ctx.Locale.Tr "repo.issues.remove_request_review_block"}}"{{end}}>
-				{{if .Team}}
+							{{svg "octicon-check"}} {{svg "octicon-people" 20}} {{$data.RepoOwnerName}}/{{.Team.Name}}
-					<a class="{{if not .CanChange}}ui{{end}} item {{if .Checked}}checked{{end}} {{if not .CanChange}}ban-change{{end}}" href="#" data-id="{{.ItemID}}" data-id-selector="#review_request_team_{{.Team.ID}}" {{if not .CanChange}} data-tooltip-content="{{ctx.Locale.Tr "repo.issues.remove_request_review_block"}}"{{end}}>
+						</a>
-						<span class="octicon-check {{if not .Checked}}tw-invisible{{end}}">{{svg "octicon-check" 16}}</span>
+					{{end}}
-						<span class="text">
-							{{svg "octicon-people" 16 "tw-ml-4 tw-mr-1"}}{{$.Issue.Repo.OwnerName}}/{{.Team.Name}}
-						</span>
-					</a>
 				{{end}}
 			{{end}}
-		{{end}}
+		</div>
 	</div>
-</div>
 
-<div class="ui assignees list">
+	<div class="ui relaxed list flex-items-block tw-my-4">
-	<span class="no-select item {{if or .OriginalReviews .PullReviewers}}tw-hidden{{end}}">{{ctx.Locale.Tr "repo.issues.new.no_reviewers"}}</span>
+		<span class="item empty-list {{if or $data.OriginalReviews $data.CurrentPullReviewers}}tw-hidden{{end}}">
-	<div class="selected">
+			{{ctx.Locale.Tr "repo.issues.new.no_reviewers"}}
-		{{range .PullReviewers}}
+		</span>
-			<div class="item tw-flex tw-items-center tw-py-2">
+		{{range $data.CurrentPullReviewers}}
-				<div class="tw-flex tw-items-center tw-flex-1">
+			<div class="item">
+				<div class="flex-text-inline tw-flex-1">
 					{{if .User}}
-						<a class="muted sidebar-item-link" href="{{.User.HomeLink}}">{{ctx.AvatarUtils.Avatar .User 20 "tw-mr-2"}}{{.User.GetDisplayName}}</a>
+						<a class="muted flex-text-inline" href="{{.User.HomeLink}}">{{ctx.AvatarUtils.Avatar .User 20}} {{.User.GetDisplayName}}</a>
 					{{else if .Team}}
-						<span class="text">{{svg "octicon-people" 20 "tw-mr-2"}}{{$.Issue.Repo.OwnerName}}/{{.Team.Name}}</span>
+						{{svg "octicon-people" 20}} {{$data.RepoOwnerName}}/{{.Team.Name}}
 					{{end}}
 				</div>
-				<div class="tw-flex tw-items-center tw-gap-2">
+				<div class="flex-text-inline">
-					{{if (and $.Permission.IsAdmin (or (eq .Review.Type 1) (eq .Review.Type 3)) (not $.Issue.IsClosed) (not $.Issue.PullRequest.HasMerged))}}
+					{{if .CanBeDismissed}}
-						<a href="#" class="ui muted icon tw-flex tw-items-center show-modal" data-tooltip-content="{{ctx.Locale.Tr "repo.issues.dismiss_review"}}" data-modal="#dismiss-review-modal-{{.Review.ID}}">
+						<a href="#" class="ui muted icon show-modal" data-tooltip-content="{{ctx.Locale.Tr "repo.issues.dismiss_review"}}"
+							data-modal="#issue-sidebar-dismiss-review-modal" data-modal-reviewer-id="{{.Review.ID}}">
 							{{svg "octicon-x" 20}}
 						</a>
-						<div class="ui small modal" id="dismiss-review-modal-{{.Review.ID}}">
-							<div class="header">
-								{{ctx.Locale.Tr "repo.issues.dismiss_review"}}
-							</div>
-							<div class="content">
-								<div class="ui warning message">
-									{{ctx.Locale.Tr "repo.issues.dismiss_review_warning"}}
-								</div>
-								<form class="ui form dismiss-review-form" id="dismiss-review-{{.Review.ID}}" action="{{$.RepoLink}}/issues/dismiss_review" method="post">
-									{{$.CsrfTokenHtml}}
-									<input type="hidden" name="review_id" value="{{.Review.ID}}">
-									<div class="field">
-										<label for="message">{{ctx.Locale.Tr "action.review_dismissed_reason"}}</label>
-										<input id="message" name="message">
-									</div>
-									<div class="text right actions">
-										<button class="ui cancel button">{{ctx.Locale.Tr "settings.cancel"}}</button>
-										<button class="ui red button" type="submit">{{ctx.Locale.Tr "ok"}}</button>
-									</div>
-								</form>
-							</div>
-						</div>
 					{{end}}
 					{{if .Review.Stale}}
-						<span data-tooltip-content="{{ctx.Locale.Tr "repo.issues.is_stale"}}">
+						<span data-tooltip-content="{{ctx.Locale.Tr "repo.issues.is_stale"}}">{{svg "octicon-hourglass" 16}}</span>
-							{{svg "octicon-hourglass" 16}}
-						</span>
 					{{end}}
-					{{if and .CanChange (or .Checked (and (not $.Issue.IsClosed) (not $.Issue.PullRequest.HasMerged)))}}
+					{{if and .CanChange $data.CanChooseReviewer}}
-						<a href="#" class="ui muted icon re-request-review{{if .Checked}} checked{{end}}" data-tooltip-content="{{if .Checked}}{{ctx.Locale.Tr "repo.issues.remove_request_review"}}{{else}}{{ctx.Locale.Tr "repo.issues.re_request_review"}}{{end}}" data-issue-id="{{$.Issue.ID}}" data-id="{{.ItemID}}" data-update-url="{{$.RepoLink}}/issues/request_review">{{svg (Iif .Checked "octicon-trash" "octicon-sync")}}</a>
+						{{if .Requested}}
+							<a href="#" class="ui muted icon link-action"
+								data-tooltip-content="{{ctx.Locale.Tr "repo.issues.remove_request_review"}}"
+								data-url="{{$data.RepoLink}}/issues/request_review?action=detach&issue_ids={{$data.IssueID}}&id={{.ItemID}}">
+								{{svg "octicon-trash"}}
+							</a>
+						{{else}}
+							<a href="#" class="ui muted icon link-action"
+								data-tooltip-content="{{ctx.Locale.Tr "repo.issues.re_request_review"}}"
+								data-url="{{$data.RepoLink}}/issues/request_review?action=attach&issue_ids={{$data.IssueID}}&id={{.ItemID}}">
+								{{svg "octicon-sync"}}
+							</a>
+						{{end}}
 					{{end}}
 					<span {{if .Review.TooltipContent}}data-tooltip-content="{{ctx.Locale.Tr .Review.TooltipContent}}"{{end}}>
 						{{svg (printf "octicon-%s" .Review.Type.Icon) 16 (printf "text %s" (.Review.HTMLTypeColorName))}}
@@ -97,15 +81,16 @@
 				</div>
 			</div>
 		{{end}}
-		{{range .OriginalReviews}}
+		{{range $data.OriginalReviews}}
-			<div class="item tw-flex tw-items-center tw-py-2">
+			<div class="item">
-				<div class="tw-flex tw-items-center tw-flex-1">
+				<div class="flex-text-inline tw-flex-1">
-					<a class="muted" href="{{$.Repository.OriginalURL}}" data-tooltip-content="{{ctx.Locale.Tr "repo.migrated_from_fake" $.Repository.GetOriginalURLHostname}}">
+					{{$originalURLHostname := $data.Repository.GetOriginalURLHostname}}
-						{{svg (MigrationIcon $.Repository.GetOriginalURLHostname) 20 "tw-mr-2"}}
+					{{$originalURL := $data.Repository.OriginalURL}}
-						{{.OriginalAuthor}}
+					<a class="muted flex-text-inline" href="{{$originalURL}}" data-tooltip-content="{{ctx.Locale.Tr "repo.migrated_from_fake" $originalURLHostname}}">
+						{{svg (MigrationIcon $originalURLHostname) 20}} {{.OriginalAuthor}}
 					</a>
 				</div>
-				<div class="tw-flex tw-items-center tw-gap-2">
+				<div class="flex-text-inline">
 					<span {{if .TooltipContent}}data-tooltip-content="{{ctx.Locale.Tr .TooltipContent}}"{{end}}>
 						{{svg (printf "octicon-%s" .Type.Icon) 16 (printf "text %s" (.HTMLTypeColorName))}}
 					</span>
@@ -113,4 +98,29 @@
 			</div>
 		{{end}}
 	</div>
+
+	{{if $data.CurrentPullReviewers}}
+	<div class="ui small modal" id="issue-sidebar-dismiss-review-modal">
+		<div class="header">
+			{{ctx.Locale.Tr "repo.issues.dismiss_review"}}
+		</div>
+		<div class="content">
+			<div class="ui warning message">
+				{{ctx.Locale.Tr "repo.issues.dismiss_review_warning"}}
+			</div>
+			<form class="ui form" action="{{$data.RepoLink}}/issues/dismiss_review" method="post">
+				{{ctx.RootData.CsrfTokenHtml}}
+				<input type="hidden" class="reviewer-id" name="review_id">
+				<div class="field">
+					<label for="issue-sidebar-dismiss-review-message">{{ctx.Locale.Tr "action.review_dismissed_reason"}}</label>
+					<input id="issue-sidebar-dismiss-review-message" name="message">
+				</div>
+				<div class="text right actions">
+					<button class="ui cancel button">{{ctx.Locale.Tr "settings.cancel"}}</button>
+					<button class="ui red button" type="submit">{{ctx.Locale.Tr "ok"}}</button>
+				</div>
+			</form>
+		</div>
+	</div>
+	{{end}}
 </div>

templates/repo/issue/sidebar/wip_switch.tmpl

@@ -1,5 +1,5 @@
 {{if and (or .HasIssuesOrPullsWritePermission .IsIssuePoster) (not .HasMerged) (not .Issue.IsClosed) (not .IsPullWorkInProgress)}}
-	<div class="toggle-wip" data-title="{{.Issue.Title}}" data-wip-prefix="{{index .PullRequestWorkInProgressPrefixes 0}}" data-update-url="{{.Issue.Link}}/title">
+	<div class="toggle-wip tw-mt-2" data-title="{{.Issue.Title}}" data-wip-prefix="{{index .PullRequestWorkInProgressPrefixes 0}}" data-update-url="{{.Issue.Link}}/title">
 		<a class="muted">
 			{{ctx.Locale.Tr "repo.pulls.still_in_progress"}} {{ctx.Locale.Tr "repo.pulls.add_prefix" (index .PullRequestWorkInProgressPrefixes 0)}}
 		</a>

templates/repo/issue/view_content/sidebar.tmpl

@@ -2,7 +2,7 @@
 	{{template "repo/issue/branch_selector_field" $}}
 
 	{{if .Issue.IsPull}}
-		{{template "repo/issue/sidebar/reviewer_list" $}}
+		{{template "repo/issue/sidebar/reviewer_list" dict "IssueSidebarReviewersData" $.IssueSidebarReviewersData}}
 		{{template "repo/issue/sidebar/wip_switch" $}}
 		<div class="divider"></div>
 	{{end}}

templates/swagger/v1_json.tmpl

@@ -20095,6 +20095,20 @@
           "format": "int64",
           "x-go-name": "Milestone"
         },
+        "reviewers": {
+          "type": "array",
+          "items": {
+            "type": "string"
+          },
+          "x-go-name": "Reviewers"
+        },
+        "team_reviewers": {
+          "type": "array",
+          "items": {
+            "type": "string"
+          },
+          "x-go-name": "TeamReviewers"
+        },
         "title": {
           "type": "string",
           "x-go-name": "Title"

tests/integration/actions_trigger_test.go

@@ -145,7 +145,8 @@ func TestPullRequestTargetEvent(t *testing.T) {
 			BaseRepo:   baseRepo,
 			Type:       issues_model.PullRequestGitea,
 		}
-		err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil)
+		prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest}
+		err = pull_service.NewPullRequest(git.DefaultContext, prOpts)
 		assert.NoError(t, err)
 
 		// load and compare ActionRun
@@ -199,7 +200,8 @@ func TestPullRequestTargetEvent(t *testing.T) {
 			BaseRepo:   baseRepo,
 			Type:       issues_model.PullRequestGitea,
 		}
-		err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil)
+		prOpts = &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest}
+		err = pull_service.NewPullRequest(git.DefaultContext, prOpts)
 		assert.NoError(t, err)
 
 		// the new pull request cannot trigger actions, so there is still only 1 record

tests/integration/api_pull_review_test.go

@@ -11,6 +11,7 @@ import (
 	auth_model "code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
+	access_model "code.gitea.io/gitea/models/perm/access"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
@@ -422,7 +423,9 @@ func TestAPIPullReviewStayDismissed(t *testing.T) {
 		pullIssue.ID, user8.ID, 1, 1, 2, false)
 
 	// user8 dismiss review
-	_, err = issue_service.ReviewRequest(db.DefaultContext, pullIssue, user8, user8, false)
+	permUser8, err := access_model.GetUserRepoPermission(db.DefaultContext, pullIssue.Repo, user8)
+	assert.NoError(t, err)
+	_, err = issue_service.ReviewRequest(db.DefaultContext, pullIssue, user8, &permUser8, user8, false)
 	assert.NoError(t, err)
 
 	reviewsCountCheck(t,

tests/integration/pull_merge_test.go

@@ -520,7 +520,8 @@ func TestConflictChecking(t *testing.T) {
 			BaseRepo:   baseRepo,
 			Type:       issues_model.PullRequestGitea,
 		}
-		err = pull.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil)
+		prOpts := &pull.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest}
+		err = pull.NewPullRequest(git.DefaultContext, prOpts)
 		assert.NoError(t, err)
 
 		issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with conflict!"})

tests/integration/pull_update_test.go

@@ -173,7 +173,8 @@ func createOutdatedPR(t *testing.T, actor, forkOrg *user_model.User) *issues_mod
 		BaseRepo:   baseRepo,
 		Type:       issues_model.PullRequestGitea,
 	}
-	err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil)
+	prOpts := &pull_service.NewPullRequestOptions{Repo: baseRepo, Issue: pullIssue, PullRequest: pullRequest}
+	err = pull_service.NewPullRequest(git.DefaultContext, prOpts)
 	assert.NoError(t, err)
 
 	issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "Test Pull -to-update-"})

web_src/css/base.css

@@ -1381,6 +1381,7 @@ table th[data-sortt-desc] .svg {
   align-items: stretch;
 }
 
+.ui.list.flex-items-block > .item,
 .flex-items-block > .item,
 .flex-text-block {
   display: flex;

web_src/css/repo.css

@@ -50,6 +50,15 @@
   width: 300px;
 }
 
+.issue-sidebar-combo .ui.dropdown .item:not(.checked) svg.octicon-check {
+  visibility: hidden;
+}
+/* ideally, we should move these styles to ".ui.dropdown .menu.flex-items-menu > .item ...", could be done later */
+.issue-sidebar-combo .ui.dropdown .menu > .item > img,
+.issue-sidebar-combo .ui.dropdown .menu > .item > svg {
+  margin: 0;
+}
+
 .issue-content-right .dropdown > .menu {
   max-width: 270px;
   min-width: 0;

web_src/js/features/repo-issue-sidebar-combolist.ts

@@ -0,0 +1,89 @@
+import {fomanticQuery} from '../modules/fomantic/base.ts';
+import {POST} from '../modules/fetch.ts';
+import {queryElemChildren, toggleElem} from '../utils/dom.ts';
+
+// if there are draft comments, confirm before reloading, to avoid losing comments
+export function issueSidebarReloadConfirmDraftComment() {
+  const commentTextareas = [
+    document.querySelector<HTMLTextAreaElement>('.edit-content-zone:not(.tw-hidden) textarea'),
+    document.querySelector<HTMLTextAreaElement>('#comment-form textarea'),
+  ];
+  for (const textarea of commentTextareas) {
+    // Most users won't feel too sad if they lose a comment with 10 chars, they can re-type these in seconds.
+    // But if they have typed more (like 50) chars and the comment is lost, they will be very unhappy.
+    if (textarea && textarea.value.trim().length > 10) {
+      textarea.parentElement.scrollIntoView();
+      if (!window.confirm('Page will be reloaded, but there are draft comments. Continuing to reload will discard the comments. Continue?')) {
+        return;
+      }
+      break;
+    }
+  }
+  window.location.reload();
+}
+
+function collectCheckedValues(elDropdown: HTMLElement) {
+  return Array.from(elDropdown.querySelectorAll('.menu > .item.checked'), (el) => el.getAttribute('data-value'));
+}
+
+export function initIssueSidebarComboList(container: HTMLElement) {
+  if (!container) return;
+
+  const updateUrl = container.getAttribute('data-update-url');
+  const elDropdown = container.querySelector<HTMLElement>(':scope > .ui.dropdown');
+  const elList = container.querySelector<HTMLElement>(':scope > .ui.list');
+  const elComboValue = container.querySelector<HTMLInputElement>(':scope > .combo-value');
+  const initialValues = collectCheckedValues(elDropdown);
+
+  elDropdown.addEventListener('click', (e) => {
+    const elItem = (e.target as HTMLElement).closest('.item');
+    if (!elItem) return;
+    e.preventDefault();
+    if (elItem.getAttribute('data-can-change') !== 'true') return;
+    elItem.classList.toggle('checked');
+    elComboValue.value = collectCheckedValues(elDropdown).join(',');
+  });
+
+  const updateToBackend = async (changedValues) => {
+    let changed = false;
+    for (const value of initialValues) {
+      if (!changedValues.includes(value)) {
+        await POST(updateUrl, {data: new URLSearchParams({action: 'detach', id: value})});
+        changed = true;
+      }
+    }
+    for (const value of changedValues) {
+      if (!initialValues.includes(value)) {
+        await POST(updateUrl, {data: new URLSearchParams({action: 'attach', id: value})});
+        changed = true;
+      }
+    }
+    if (changed) issueSidebarReloadConfirmDraftComment();
+  };
+
+  const syncList = (changedValues) => {
+    const elEmptyTip = elList.querySelector('.item.empty-list');
+    queryElemChildren(elList, '.item:not(.empty-list)', (el) => el.remove());
+    for (const value of changedValues) {
+      const el = elDropdown.querySelector<HTMLElement>(`.menu > .item[data-value="${value}"]`);
+      const listItem = el.cloneNode(true) as HTMLElement;
+      listItem.querySelector('svg.octicon-check')?.remove();
+      elList.append(listItem);
+    }
+    const hasItems = Boolean(elList.querySelector('.item:not(.empty-list)'));
+    toggleElem(elEmptyTip, !hasItems);
+  };
+
+  fomanticQuery(elDropdown).dropdown({
+    action: 'nothing', // do not hide the menu if user presses Enter
+    fullTextSearch: 'exact',
+    async onHide() {
+      const changedValues = collectCheckedValues(elDropdown);
+      if (updateUrl) {
+        await updateToBackend(changedValues); // send requests to backend and reload the page
+      } else {
+        syncList(changedValues); // only update the list in the sidebar
+      }
+    },
+  });
+}

web_src/js/features/repo-issue-sidebar.ts

@@ -4,26 +4,7 @@ import {updateIssuesMeta} from './repo-common.ts';
 import {svg} from '../svg.ts';
 import {htmlEscape} from 'escape-goat';
 import {toggleElem} from '../utils/dom.ts';
-
+import {initIssueSidebarComboList, issueSidebarReloadConfirmDraftComment} from './repo-issue-sidebar-combolist.ts';
-// if there are draft comments, confirm before reloading, to avoid losing comments
-function reloadConfirmDraftComment() {
-  const commentTextareas = [
-    document.querySelector('.edit-content-zone:not(.tw-hidden) textarea'),
-    document.querySelector('#comment-form textarea'),
-  ];
-  for (const textarea of commentTextareas) {
-    // Most users won't feel too sad if they lose a comment with 10 chars, they can re-type these in seconds.
-    // But if they have typed more (like 50) chars and the comment is lost, they will be very unhappy.
-    if (textarea && textarea.value.trim().length > 10) {
-      textarea.parentElement.scrollIntoView();
-      if (!window.confirm('Page will be reloaded, but there are draft comments. Continuing to reload will discard the comments. Continue?')) {
-        return;
-      }
-      break;
-    }
-  }
-  window.location.reload();
-}
 
 function initBranchSelector() {
   const elSelectBranch = document.querySelector('.ui.dropdown.select-branch');
@@ -78,7 +59,7 @@ function initListSubmits(selector, outerSelector) {
           );
         }
         if (itemEntries.length) {
-          reloadConfirmDraftComment();
+          issueSidebarReloadConfirmDraftComment();
         }
       }
     },
@@ -142,7 +123,7 @@ function initListSubmits(selector, outerSelector) {
 
     // TODO: Which thing should be done for choosing review requests
     // to make chosen items be shown on time here?
-    if (selector === 'select-reviewers-modify' || selector === 'select-assignees-modify') {
+    if (selector === 'select-assignees-modify') {
       return false;
     }
 
@@ -173,7 +154,7 @@ function initListSubmits(selector, outerSelector) {
           $listMenu.data('issue-id'),
           '',
         );
-        reloadConfirmDraftComment();
+        issueSidebarReloadConfirmDraftComment();
       })();
     }
 
@@ -182,7 +163,7 @@ function initListSubmits(selector, outerSelector) {
       $(this).find('.octicon-check').addClass('tw-invisible');
     });
 
-    if (selector === 'select-reviewers-modify' || selector === 'select-assignees-modify') {
+    if (selector === 'select-assignees-modify') {
       return false;
     }
 
@@ -213,7 +194,7 @@ function selectItem(select_id, input_id) {
           $menu.data('issue-id'),
           $(this).data('id'),
         );
-        reloadConfirmDraftComment();
+        issueSidebarReloadConfirmDraftComment();
       })();
     }
 
@@ -249,7 +230,7 @@ function selectItem(select_id, input_id) {
           $menu.data('issue-id'),
           $(this).data('id'),
         );
-        reloadConfirmDraftComment();
+        issueSidebarReloadConfirmDraftComment();
       })();
     }
 
@@ -276,14 +257,14 @@ export function initRepoIssueSidebar() {
   initBranchSelector();
   initRepoIssueDue();
 
-  // Init labels and assignees
+  // TODO: refactor the legacy initListSubmits&selectItem to initIssueSidebarComboList
   initListSubmits('select-label', 'labels');
   initListSubmits('select-assignees', 'assignees');
   initListSubmits('select-assignees-modify', 'assignees');
-  initListSubmits('select-reviewers-modify', 'assignees');
-
-  // Milestone, Assignee, Project
   selectItem('.select-project', '#project_id');
   selectItem('.select-milestone', '#milestone_id');
   selectItem('.select-assignee', '#assignee_id');
+
+  // init the combo list: a dropdown for selecting reviewers, and a list for showing selected reviewers and related actions
+  initIssueSidebarComboList(document.querySelector('.issue-sidebar-combo[data-sidebar-combo-for="reviewers"]'));
 }

web_src/js/features/repo-issue.ts

@@ -8,7 +8,6 @@ import {parseIssuePageInfo, toAbsoluteUrl} from '../utils.ts';
 import {GET, POST} from '../modules/fetch.ts';
 import {showErrorToast} from '../modules/toast.ts';
 import {initRepoIssueSidebar} from './repo-issue-sidebar.ts';
-import {updateIssuesMeta} from './repo-common.ts';
 
 const {appSubUrl} = window.config;
 
@@ -326,17 +325,6 @@ export function initRepoIssueWipTitle() {
 export function initRepoIssueComments() {
   if (!$('.repository.view.issue .timeline').length) return;
 
-  $('.re-request-review').on('click', async function (e) {
-    e.preventDefault();
-    const url = this.getAttribute('data-update-url');
-    const issueId = this.getAttribute('data-issue-id');
-    const id = this.getAttribute('data-id');
-    const isChecked = this.classList.contains('checked');
-
-    await updateIssuesMeta(url, isChecked ? 'detach' : 'attach', issueId, id);
-    window.location.reload();
-  });
-
   document.addEventListener('click', (e) => {
     const urlTarget = document.querySelector(':target');
     if (!urlTarget) return;