diff --git a/modules/structs/pull.go b/modules/structs/pull.go index 05a8d596338bb..79c80dacb1f5c 100644 --- a/modules/structs/pull.go +++ b/modules/structs/pull.go @@ -79,7 +79,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 diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index a7a7a4f4c50f9..1ba8f3b9e5d7d 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1366,6 +1366,7 @@ issues.new.assignees = Assignees issues.new.clear_assignees = Clear assignees issues.new.no_assignees = No Assignees issues.new.no_reviewers = No reviewers +issues.new.clear_reviewers = Clear reviewers issues.choose.get_started = Get Started issues.choose.open_external_link = Open issues.choose.blank = Default diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index d6b9dddd9d7ba..a4ac4c68cb66b 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -16,6 +16,7 @@ import ( activities_model "code.gitea.io/gitea/models/activities" 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" pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" @@ -420,8 +421,46 @@ func CreatePullRequest(ctx *context.APIContext) { return } } + // handle reviewers + var reviewerIds []int64 - if err := pull_service.NewPullRequest(ctx, repo, prIssue, labelIDs, []string{}, pr, assigneeIDs); err != nil { + for _, r := range form.Reviewers { + 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 with id '%s' not exist", r)) + return + } + ctx.Error(http.StatusInternalServerError, "GetUser", err) + return + } + reviewerIds = append(reviewerIds, reviewer.ID) + } + + // handle teams as reviewers + if ctx.Repo.Repository.Owner.IsOrganization() && len(form.TeamReviewers) > 0 { + for _, t := range form.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 + } + reviewerIds = append(reviewerIds, -teamReviewer.ID) + } + } + + if err := pull_service.NewPullRequest(ctx, repo, prIssue, labelIDs, []string{}, pr, assigneeIDs, reviewerIds); err != nil { if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err) return diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index b69af3c61cc54..84b820658d7a8 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -19,6 +19,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" @@ -36,6 +37,7 @@ import ( "code.gitea.io/gitea/modules/upload" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/gitdiff" + repo_service "code.gitea.io/gitea/services/repository" ) const ( @@ -800,6 +802,47 @@ func CompareDiff(ctx *context.Context) { if ctx.Written() { return } + + // Get reviewer info for pr + var ( + reviewers []*user_model.User + teamReviewers []*organization.Team + reviewersResult []*repoReviewerSelection + ) + reviewers, err = repo_model.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, ctx.Doer.ID) + if err != nil { + ctx.ServerError("GetReviewers", err) + return + } + + teamReviewers, err = repo_service.GetReviewerTeams(ctx, ctx.Repo.Repository) + if err != nil { + ctx.ServerError("GetReviewerTeams", err) + return + } + + for _, user := range reviewers { + reviewersResult = append(reviewersResult, &repoReviewerSelection{ + IsTeam: false, + CanChange: true, + User: user, + ItemID: user.ID, + }) + } + + // negative reviewIDs represent team requests + for _, team := range teamReviewers { + reviewersResult = append(reviewersResult, &repoReviewerSelection{ + IsTeam: true, + CanChange: true, + Team: team, + ItemID: -team.ID, + }) + } + ctx.Data["Reviewers"] = reviewersResult + if ctx.Written() { + return + } } } beforeCommitID := ctx.Data["BeforeCommitID"].(string) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 94300da868330..f08c3264794a8 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1075,7 +1075,7 @@ 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) ([]int64, []int64, []int64, int64, int64) { var ( repo = ctx.Repo.Repository err error @@ -1083,7 +1083,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 nil, nil, nil, 0, 0 } var labelIDs []int64 @@ -1092,7 +1092,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 nil, nil, nil, 0, 0 } labelIDMark := make(container.Set[int64]) labelIDMark.AddMultiple(labelIDs...) @@ -1115,11 +1115,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 nil, nil, nil, 0, 0 } if milestone.RepoID != repo.ID { ctx.ServerError("GetMilestoneByID", err) - return nil, nil, 0, 0 + return nil, nil, nil, 0, 0 } ctx.Data["Milestone"] = milestone ctx.Data["milestone_id"] = milestoneID @@ -1129,11 +1129,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 nil, nil, nil, 0, 0 } if p.RepoID != ctx.Repo.Repository.ID && p.OwnerID != ctx.Repo.Repository.OwnerID { ctx.NotFound("", nil) - return nil, nil, 0, 0 + return nil, nil, nil, 0, 0 } ctx.Data["Project"] = p @@ -1145,7 +1145,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 nil, nil, nil, 0, 0 } // Check if the passed assignees actually exists and is assignable @@ -1153,18 +1153,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 nil, nil, nil, 0, 0 } valid, err := access_model.CanBeAssigned(ctx, assignee, repo, isPull) if err != nil { ctx.ServerError("CanBeAssigned", err) - return nil, nil, 0, 0 + return nil, nil, nil, 0, 0 } if !valid { ctx.ServerError("canBeAssigned", repo_model.ErrUserDoesNotHaveAccessToRepo{UserID: aID, RepoName: repo.Name}) - return nil, nil, 0, 0 + return nil, nil, nil, 0, 0 } } } @@ -1174,7 +1174,37 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull assigneeIDs = append(assigneeIDs, form.AssigneeID) } - return labelIDs, assigneeIDs, milestoneID, form.ProjectID + // Check reviewers + var reviewerIDs []int64 + if isPull { + if len(form.ReviewerIDs) > 0 { + reviewerIDs, err = base.StringsToInt64s(strings.Split(form.ReviewerIDs, ",")) + if err != nil { + return nil, nil, nil, 0, 0 + } + + // Check if the passed reviewers (user/team) actually exist + for _, rID := range reviewerIDs { + // negative reviewIDs represent team requests + if rID < 0 { + _, err := organization.GetTeamByID(ctx, -rID) + if err != nil { + ctx.ServerError("GetTeamByID", err) + return nil, nil, nil, 0, 0 + } + continue + } + + _, err := user_model.GetUserByID(ctx, rID) + if err != nil { + ctx.ServerError("GetUserByID", err) + return nil, nil, nil, 0, 0 + } + } + } + } + + return labelIDs, assigneeIDs, reviewerIDs, milestoneID, form.ProjectID } // NewIssuePost response for creating new issue @@ -1192,7 +1222,7 @@ func NewIssuePost(ctx *context.Context) { attachments []string ) - labelIDs, assigneeIDs, milestoneID, projectID := ValidateRepoMetas(ctx, *form, false) + labelIDs, assigneeIDs, _, milestoneID, projectID := ValidateRepoMetas(ctx, *form, false) if ctx.Written() { return } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index ec109ed665c4e..8c2082511b017 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1379,7 +1379,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { return } - labelIDs, assigneeIDs, milestoneID, _ := ValidateRepoMetas(ctx, *form, true) + labelIDs, assigneeIDs, reviewerIDs, milestoneID, _ := ValidateRepoMetas(ctx, *form, true) if ctx.Written() { return } @@ -1429,7 +1429,7 @@ 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. - if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs); err != nil { + if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs, reviewerIDs); err != nil { if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error()) return diff --git a/services/agit/agit.go b/services/agit/agit.go index acfedf09d425d..137a4b6b94037 100644 --- a/services/agit/agit.go +++ b/services/agit/agit.go @@ -142,7 +142,7 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. Flow: issues_model.PullRequestFlowAGit, } - if err := pull_service.NewPullRequest(ctx, repo, prIssue, []int64{}, []string{}, pr, []int64{}); err != nil { + if err := pull_service.NewPullRequest(ctx, repo, prIssue, []int64{}, []string{}, pr, []int64{}, []int64{}); err != nil { return nil, err } diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 5df7ec8fd609a..a9f66c378b3b8 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -441,6 +441,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 diff --git a/services/pull/pull.go b/services/pull/pull.go index 2f5143903aa83..7971f232c146f 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -16,6 +16,8 @@ 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" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" @@ -37,7 +39,7 @@ import ( var pullWorkingPool = sync.NewExclusivePool() // 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, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string, pr *issues_model.PullRequest, assigneeIDs, reviewerIDs []int64) error { prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr) if err != nil { if !git_model.IsErrBranchNotExist(err) { @@ -80,6 +82,42 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss assigneeCommentMap[assigneeID] = comment } + for _, reviewerID := range reviewerIDs { + // negative reviewIDs represent team requests + if reviewerID < 0 { + team, err := organization.GetTeamByID(ctx, -reviewerID) + if err != nil { + return err + } + err = issue_service.IsValidTeamReviewRequest(ctx, team, issue.Poster, true, issue) + if err != nil { + return err + } + _, err = issue_service.TeamReviewRequest(ctx, issue, issue.Poster, team, true) + if err != nil { + return err + } + continue + } + + reviewer, err := user_model.GetUserByID(ctx, reviewerID) + if err != nil { + return err + } + permDoer, err := access_model.GetUserRepoPermission(ctx, issue.Repo, issue.Poster) + if err != nil { + return err + } + err = issue_service.IsValidReviewRequest(ctx, reviewer, issue.Poster, true, issue, &permDoer) + if err != nil { + return err + } + _, err = issue_service.ReviewRequest(ctx, issue, issue.Poster, reviewer, true) + if err != nil { + return err + } + } + pr.Issue = issue issue.PullRequest = pr diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index 04ae8456bb3c0..a8aae6f76193e 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -50,6 +50,58 @@
{{template "repo/issue/branch_selector_field" .}} + {{if .PageIsComparePull}} + +
+ + {{ctx.Locale.Tr "repo.issues.review.reviewers"}} + {{svg "octicon-gear" 16 "gt-ml-2"}} + +
+
+ {{svg "octicon-search" 16}} + +
+
{{ctx.Locale.Tr "repo.issues.new.clear_reviewers"}}
+ {{range .Reviewers}} + {{if .User}} + + {{svg "octicon-check"}} + + {{ctx.AvatarUtils.Avatar .User 28 "gt-mr-3"}}{{template "repo/search_name" .User}} + + + {{end}} + {{end}} +
+ {{range .Reviewers}} + {{if .Team}} + + {{svg "octicon-check" 16}} + + {{svg "octicon-people" 16 "gt-ml-4 gt-mr-2"}}{{$.Repository.OwnerName}}/{{.Team.Name}} + + + {{end}} + {{end}} +
+
+
+ + {{ctx.Locale.Tr "repo.issues.new.no_reviewers"}} + +
+ {{range .Reviewers}} + {{if .User}} + {{ctx.AvatarUtils.Avatar .User 20 "gt-mr-3"}}{{.User.GetDisplayName}} + {{else if .Team}} + {{svg "octicon-people" 20 "gt-mr-3"}}{{$.Repository.OwnerName}}/{{.Team.Name}} + {{end}} + {{end}} +
+
+
+ {{end}} {{template "repo/issue/labels/labels_selector_field" .}} @@ -172,7 +224,7 @@
{{range .Assignees}} - + {{ctx.AvatarUtils.Avatar . 28 "gt-mr-3 gt-vm"}}{{.GetDisplayName}} {{end}} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 75a45dc68ac56..2ceac85b21aab 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -18108,6 +18108,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" diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go index 253de70326553..edb29826de09f 100644 --- a/tests/integration/actions_trigger_test.go +++ b/tests/integration/actions_trigger_test.go @@ -133,7 +133,7 @@ func TestPullRequestTargetEvent(t *testing.T) { BaseRepo: baseRepo, Type: issues_model.PullRequestGitea, } - err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil) + err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil, nil) assert.NoError(t, err) // load and compare ActionRun @@ -187,7 +187,7 @@ func TestPullRequestTargetEvent(t *testing.T) { BaseRepo: baseRepo, Type: issues_model.PullRequestGitea, } - err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil) + err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil, nil) assert.NoError(t, err) // the new pull request cannot trigger actions, so there is still only 1 record diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index a4cc3e76fe94a..4b62df9ca7ac5 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -413,7 +413,7 @@ func TestConflictChecking(t *testing.T) { BaseRepo: baseRepo, Type: issues_model.PullRequestGitea, } - err = pull.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil) + err = pull.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil, nil) assert.NoError(t, err) issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with conflict!"}) diff --git a/tests/integration/pull_update_test.go b/tests/integration/pull_update_test.go index e4b2ae65bd61f..6427e049c882b 100644 --- a/tests/integration/pull_update_test.go +++ b/tests/integration/pull_update_test.go @@ -171,7 +171,7 @@ 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) + err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil, nil) assert.NoError(t, err) issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "Test Pull -to-update-"}) diff --git a/web_src/js/features/repo-legacy.js b/web_src/js/features/repo-legacy.js index 08fe21190ac8a..d8d91d373a51a 100644 --- a/web_src/js/features/repo-legacy.js +++ b/web_src/js/features/repo-legacy.js @@ -234,7 +234,8 @@ export function initRepoCommentForm() { initListSubmits('select-label', 'labels'); initListSubmits('select-assignees', 'assignees'); initListSubmits('select-assignees-modify', 'assignees'); - initListSubmits('select-reviewers-modify', 'assignees'); + initListSubmits('select-reviewers', 'reviewers'); + initListSubmits('select-reviewers-modify', 'reviewers'); function selectItem(select_id, input_id) { const $menu = $(`${select_id} .menu`); @@ -263,6 +264,8 @@ export function initRepoCommentForm() { icon = svg('octicon-project', 18, 'gt-mr-3'); } else if (input_id === '#assignee_id') { icon = `avatar`; + } else if (input_id === '#reviewer_id') { + icon = `avatar`; } $list.find('.selected').html(` @@ -299,6 +302,7 @@ export function initRepoCommentForm() { selectItem('.select-project', '#project_id'); selectItem('.select-milestone', '#milestone_id'); selectItem('.select-assignee', '#assignee_id'); + selectItem('.select-reviewer', '#reviewer_id'); } async function onEditContent(event) {