Skip to content

Commit d37f246

Browse files
splitt3rCalK16
splitt3r
authored and
CalK16
committed
Add reviewers selection to new pull request form
1 parent 9914c9a commit d37f246

File tree

12 files changed

+768
-22
lines changed

12 files changed

+768
-22
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,7 @@ func CreatePullRequest(ctx *context.APIContext) {
554554
}
555555
}
556556

557-
if err := pull_service.NewPullRequest(ctx, repo, prIssue, labelIDs, []string{}, pr, assigneeIDs); err != nil {
557+
if err := pull_service.NewPullRequest(ctx, repo, prIssue, labelIDs, []string{}, pr, assigneeIDs, []int64{}); err != nil {
558558
if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) {
559559
ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err)
560560
} else if errors.Is(err, user_model.ErrBlockedUser) {

‎routers/web/repo/compare.go‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"code.gitea.io/gitea/models/db"
2020
git_model "code.gitea.io/gitea/models/git"
2121
issues_model "code.gitea.io/gitea/models/issues"
22+
"code.gitea.io/gitea/models/organization"
2223
access_model "code.gitea.io/gitea/models/perm/access"
2324
repo_model "code.gitea.io/gitea/models/repo"
2425
"code.gitea.io/gitea/models/unit"
@@ -39,6 +40,7 @@ import (
3940
"code.gitea.io/gitea/services/context"
4041
"code.gitea.io/gitea/services/context/upload"
4142
"code.gitea.io/gitea/services/gitdiff"
43+
repo_service "code.gitea.io/gitea/services/repository"
4244
)
4345

4446
const (
@@ -789,6 +791,47 @@ func CompareDiff(ctx *context.Context) {
789791
if ctx.Written() {
790792
return
791793
}
794+
795+
// Get reviewer info for pr
796+
var (
797+
reviewers []*user_model.User
798+
teamReviewers []*organization.Team
799+
reviewersResult []*repoReviewerSelection
800+
)
801+
reviewers, err = repo_model.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, ctx.Doer.ID)
802+
if err != nil {
803+
ctx.ServerError("GetReviewers", err)
804+
return
805+
}
806+
807+
teamReviewers, err = repo_service.GetReviewerTeams(ctx, ctx.Repo.Repository)
808+
if err != nil {
809+
ctx.ServerError("GetReviewerTeams", err)
810+
return
811+
}
812+
813+
for _, user := range reviewers {
814+
reviewersResult = append(reviewersResult, &repoReviewerSelection{
815+
IsTeam: false,
816+
CanChange: true,
817+
User: user,
818+
ItemID: user.ID,
819+
})
820+
}
821+
822+
// negative reviewIDs represent team requests
823+
for _, team := range teamReviewers {
824+
reviewersResult = append(reviewersResult, &repoReviewerSelection{
825+
IsTeam: true,
826+
CanChange: true,
827+
Team: team,
828+
ItemID: -team.ID,
829+
})
830+
}
831+
ctx.Data["Reviewers"] = reviewersResult
832+
if ctx.Written() {
833+
return
834+
}
792835
}
793836
}
794837
beforeCommitID := ctx.Data["BeforeCommitID"].(string)

‎routers/web/repo/issue.go‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,15 +1118,15 @@ func DeleteIssue(ctx *context.Context) {
11181118
}
11191119

11201120
// ValidateRepoMetas check and returns repository's meta information
1121-
func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull bool) ([]int64, []int64, int64, int64) {
1121+
func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull bool) ([]int64, []int64, []int64, int64, int64) {
11221122
var (
11231123
repo = ctx.Repo.Repository
11241124
err error
11251125
)
11261126

11271127
labels := RetrieveRepoMetas(ctx, ctx.Repo.Repository, isPull)
11281128
if ctx.Written() {
1129-
return nil, nil, 0, 0
1129+
return nil, nil, nil, 0, 0
11301130
}
11311131

11321132
var labelIDs []int64
@@ -1135,7 +1135,7 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull
11351135
if len(form.LabelIDs) > 0 {
11361136
labelIDs, err = base.StringsToInt64s(strings.Split(form.LabelIDs, ","))
11371137
if err != nil {
1138-
return nil, nil, 0, 0
1138+
return nil, nil, nil, 0, 0
11391139
}
11401140
labelIDMark := make(container.Set[int64])
11411141
labelIDMark.AddMultiple(labelIDs...)
@@ -1158,11 +1158,11 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull
11581158
milestone, err := issues_model.GetMilestoneByRepoID(ctx, ctx.Repo.Repository.ID, milestoneID)
11591159
if err != nil {
11601160
ctx.ServerError("GetMilestoneByID", err)
1161-
return nil, nil, 0, 0
1161+
return nil, nil, nil, 0, 0
11621162
}
11631163
if milestone.RepoID != repo.ID {
11641164
ctx.ServerError("GetMilestoneByID", err)
1165-
return nil, nil, 0, 0
1165+
return nil, nil, nil, 0, 0
11661166
}
11671167
ctx.Data["Milestone"] = milestone
11681168
ctx.Data["milestone_id"] = milestoneID
@@ -1172,11 +1172,11 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull
11721172
p, err := project_model.GetProjectByID(ctx, form.ProjectID)
11731173
if err != nil {
11741174
ctx.ServerError("GetProjectByID", err)
1175-
return nil, nil, 0, 0
1175+
return nil, nil, nil, 0, 0
11761176
}
11771177
if p.RepoID != ctx.Repo.Repository.ID && p.OwnerID != ctx.Repo.Repository.OwnerID {
11781178
ctx.NotFound("", nil)
1179-
return nil, nil, 0, 0
1179+
return nil, nil, nil, 0, 0
11801180
}
11811181

11821182
ctx.Data["Project"] = p
@@ -1188,26 +1188,26 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull
11881188
if len(form.AssigneeIDs) > 0 {
11891189
assigneeIDs, err = base.StringsToInt64s(strings.Split(form.AssigneeIDs, ","))
11901190
if err != nil {
1191-
return nil, nil, 0, 0
1191+
return nil, nil, nil, 0, 0
11921192
}
11931193

11941194
// Check if the passed assignees actually exists and is assignable
11951195
for _, aID := range assigneeIDs {
11961196
assignee, err := user_model.GetUserByID(ctx, aID)
11971197
if err != nil {
11981198
ctx.ServerError("GetUserByID", err)
1199-
return nil, nil, 0, 0
1199+
return nil, nil, nil, 0, 0
12001200
}
12011201

12021202
valid, err := access_model.CanBeAssigned(ctx, assignee, repo, isPull)
12031203
if err != nil {
12041204
ctx.ServerError("CanBeAssigned", err)
1205-
return nil, nil, 0, 0
1205+
return nil, nil, nil, 0, 0
12061206
}
12071207

12081208
if !valid {
12091209
ctx.ServerError("canBeAssigned", repo_model.ErrUserDoesNotHaveAccessToRepo{UserID: aID, RepoName: repo.Name})
1210-
return nil, nil, 0, 0
1210+
return nil, nil, nil, 0, 0
12111211
}
12121212
}
12131213
}
@@ -1217,7 +1217,34 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull
12171217
assigneeIDs = append(assigneeIDs, form.AssigneeID)
12181218
}
12191219

1220-
return labelIDs, assigneeIDs, milestoneID, form.ProjectID
1220+
// Check reviewers
1221+
var reviewerIDs []int64
1222+
if len(form.ReviewerIDs) > 0 {
1223+
reviewerIDs, err = base.StringsToInt64s(strings.Split(form.ReviewerIDs, ","))
1224+
if err != nil {
1225+
return nil, nil, nil, 0, 0
1226+
}
1227+
1228+
// Check if the passed reviewers actually exist
1229+
for _, rID := range reviewerIDs {
1230+
if rID < 0 {
1231+
_, err := organization.GetTeamByID(ctx, -rID)
1232+
if err != nil {
1233+
ctx.ServerError("GetTeamByID", err)
1234+
return nil, nil, nil, 0, 0
1235+
}
1236+
continue
1237+
}
1238+
1239+
_, err := user_model.GetUserByID(ctx, rID)
1240+
if err != nil {
1241+
ctx.ServerError("GetUserByID", err)
1242+
return nil, nil, nil, 0, 0
1243+
}
1244+
}
1245+
}
1246+
1247+
return labelIDs, assigneeIDs, reviewerIDs, milestoneID, form.ProjectID
12211248
}
12221249

12231250
// NewIssuePost response for creating new issue
@@ -1235,7 +1262,7 @@ func NewIssuePost(ctx *context.Context) {
12351262
attachments []string
12361263
)
12371264

1238-
labelIDs, assigneeIDs, milestoneID, projectID := ValidateRepoMetas(ctx, *form, false)
1265+
labelIDs, assigneeIDs, _, milestoneID, projectID := ValidateRepoMetas(ctx, *form, false)
12391266
if ctx.Written() {
12401267
return
12411268
}

‎routers/web/repo/pull.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1268,7 +1268,7 @@ func CompareAndPullRequestPost(ctx *context.Context) {
12681268
return
12691269
}
12701270

1271-
labelIDs, assigneeIDs, milestoneID, projectID := ValidateRepoMetas(ctx, *form, true)
1271+
labelIDs, assigneeIDs, reviewerIDs, milestoneID, _ := ValidateRepoMetas(ctx, *form, true)
12721272
if ctx.Written() {
12731273
return
12741274
}
@@ -1318,7 +1318,7 @@ func CompareAndPullRequestPost(ctx *context.Context) {
13181318
// FIXME: check error in the case two people send pull request at almost same time, give nice error prompt
13191319
// instead of 500.
13201320

1321-
if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs); err != nil {
1321+
if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs, reviewerIDs); err != nil {
13221322
switch {
13231323
case repo_model.IsErrUserDoesNotHaveAccessToRepo(err):
13241324
ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error())

‎services/agit/agit.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
138138
Flow: issues_model.PullRequestFlowAGit,
139139
}
140140

141-
if err := pull_service.NewPullRequest(ctx, repo, prIssue, []int64{}, []string{}, pr, []int64{}); err != nil {
141+
if err := pull_service.NewPullRequest(ctx, repo, prIssue, []int64{}, []string{}, pr, []int64{}, []int64{}); err != nil {
142142
return nil, err
143143
}
144144

‎services/forms/repo_form.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,7 @@ type CreateIssueForm struct {
447447
Title string `binding:"Required;MaxSize(255)"`
448448
LabelIDs string `form:"label_ids"`
449449
AssigneeIDs string `form:"assignee_ids"`
450+
ReviewerIDs string `form:"reviewer_ids"`
450451
Ref string `form:"ref"`
451452
MilestoneID int64
452453
ProjectID int64

‎services/pull/pull.go‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"code.gitea.io/gitea/models/db"
1818
git_model "code.gitea.io/gitea/models/git"
1919
issues_model "code.gitea.io/gitea/models/issues"
20+
"code.gitea.io/gitea/models/organization"
2021
access_model "code.gitea.io/gitea/models/perm/access"
2122
repo_model "code.gitea.io/gitea/models/repo"
2223
"code.gitea.io/gitea/models/unit"
@@ -42,7 +43,7 @@ func getPullWorkingLockKey(prID int64) string {
4243
}
4344

4445
// NewPullRequest creates new pull request with labels for repository.
45-
func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string, pr *issues_model.PullRequest, assigneeIDs []int64) error {
46+
func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *issues_model.Issue, labelIDs []int64, uuids []string, pr *issues_model.PullRequest, assigneeIDs []int64, reviewerIDs []int64) error {
4647
if err := issue.LoadPoster(ctx); err != nil {
4748
return err
4849
}
@@ -116,6 +117,28 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss
116117
assigneeCommentMap[assigneeID] = comment
117118
}
118119

120+
for _, reviewerID := range reviewerIDs {
121+
if reviewerID < 0 {
122+
team, err := organization.GetTeamByID(ctx, -reviewerID)
123+
if err != nil {
124+
return err
125+
}
126+
_, err = issue_service.TeamReviewRequest(ctx, issue, issue.Poster, team, true)
127+
if err != nil {
128+
return err
129+
}
130+
}
131+
132+
reviewer, err := user_model.GetUserByID(ctx, reviewerID)
133+
if err != nil {
134+
return err
135+
}
136+
_, err = issue_service.ReviewRequest(ctx, issue, issue.Poster, reviewer, true)
137+
if err != nil {
138+
return err
139+
}
140+
}
141+
119142
pr.Issue = issue
120143
issue.PullRequest = pr
121144

‎templates/repo/issue/new_form.tmpl‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,58 @@
4848

4949
<div class="issue-content-right ui segment">
5050
{{template "repo/issue/branch_selector_field" .}}
51+
{{if .PageIsComparePull}}
52+
<input id="reviewer_ids" name="reviewer_ids" type="hidden" value="{{.reviewer_ids}}">
53+
<div class="ui {{if not .Reviewers}}disabled{{end}} floating jump select-reviewers dropdown">
54+
<span class="text flex-text-block">
55+
<strong>{{.locale.Tr "repo.issues.review.reviewers"}}</strong>
56+
{{svg "octicon-gear" 16 "gt-ml-2"}}
57+
</span>
58+
<div class="filter menu" data-id="#reviewer_ids">
59+
<div class="ui icon search input">
60+
<i class="icon gt-df gt-ac gt-jc">{{svg "octicon-search" 16}}</i>
61+
<input type="text" placeholder="{{.locale.Tr "repo.issues.filter_reviewers"}}">
62+
</div>
63+
<div class="no-select item">{{.locale.Tr "repo.issues.new.clear_reviewers"}}</div>
64+
{{range .Reviewers}}
65+
{{if .User}}
66+
<a class="item muted" data-id="{{.ItemID}}" data-id-selector="#reviewer_{{.ItemID}}">
67+
<span class="octicon-check invisible">{{svg "octicon-check"}}</span>
68+
<span class="text">
69+
{{ctx.AvatarUtils.Avatar .User 28 "gt-mr-3"}}{{template "repo/search_name" .User}}
70+
</span>
71+
</a>
72+
{{else if .Team}}
73+
<a class="item muted" data-id="{{.ItemID}}" data-id-selector="#reviewer_{{.ItemID}}">
74+
<span class="octicon-check invisible">{{svg "octicon-check" 16}}</span>
75+
<span class="text">
76+
{{svg "octicon-people" 16 "gt-ml-4 gt-mr-2"}}{{$.Repository.OwnerName}}/{{.Team.Name}}
77+
</span>
78+
</a>
79+
{{end}}
80+
{{end}}
81+
</div>
82+
</div>
83+
<div class="ui reviewers list">
84+
<span class="no-select item">
85+
{{.locale.Tr "repo.issues.new.no_reviewers"}}
86+
</span>
87+
<div class="selected">
88+
{{range .Reviewers}}
89+
{{if .User}}
90+
<a class="item gt-p-2 muted gt-hidden" id="reviewer_{{.ItemID}}" href="#">
91+
{{ctx.AvatarUtils.Avatar .User 28 "gt-mr-3 gt-vm"}}{{.User.GetDisplayName}}
92+
</a>
93+
{{else if .Team}}
94+
<a class="item gt-p-2 muted gt-hidden" id="reviewer_{{.ItemID}}" href="#">
95+
{{svg "octicon-people" 20 "gt-mr-3"}}{{$.Repository.OwnerName}}/{{.Team.Name}}
96+
</a>
97+
{{end}}
98+
{{end}}
99+
</div>
100+
</div>
101+
<div class="divider"></div>
102+
{{end}}
51103

52104
<input id="label_ids" name="label_ids" type="hidden" value="{{.label_ids}}">
53105
{{template "repo/issue/labels/labels_selector_field" .}}

‎tests/integration/actions_trigger_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ func TestPullRequestTargetEvent(t *testing.T) {
145145
BaseRepo: baseRepo,
146146
Type: issues_model.PullRequestGitea,
147147
}
148-
err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil)
148+
err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil, nil)
149149
assert.NoError(t, err)
150150

151151
// load and compare ActionRun
@@ -199,7 +199,7 @@ func TestPullRequestTargetEvent(t *testing.T) {
199199
BaseRepo: baseRepo,
200200
Type: issues_model.PullRequestGitea,
201201
}
202-
err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil)
202+
err = pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil, nil)
203203
assert.NoError(t, err)
204204

205205
// the new pull request cannot trigger actions, so there is still only 1 record

‎tests/integration/pull_merge_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ func TestConflictChecking(t *testing.T) {
520520
BaseRepo: baseRepo,
521521
Type: issues_model.PullRequestGitea,
522522
}
523-
err = pull.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil)
523+
err = pull.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil, nil)
524524
assert.NoError(t, err)
525525

526526
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: "PR with conflict!"})

0 commit comments

Comments
 (0)