Skip to content
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
d37f246
Add reviewers selection to new pull request form
splitt3r Aug 19, 2023
5929b87
Add translatanle clear reviewers label
splitt3r Aug 19, 2023
452828a
Add missing continue statement
splitt3r Aug 19, 2023
906514e
Check pr creator permissions
splitt3r Sep 1, 2023
45b8a96
Remove unintended change
splitt3r Sep 1, 2023
3e0dcea
Check if Pull + run linter
splitt3r Sep 9, 2023
978c465
Fix invisible class name
sebastian-sauer Oct 20, 2023
c92202e
Add request review to API
sebastian-sauer Oct 20, 2023
df3f363
Fix call to locale in tmpl
sebastian-sauer Nov 15, 2023
e4858b2
Improve markup
sebastian-sauer Nov 15, 2023
f604ab1
Fix teamreviewer ids
sebastian-sauer Nov 15, 2023
60d9d96
update template to merge main
CalK16 Oct 31, 2024
f7c0ea4
move logics to proper file
CalK16 Nov 2, 2024
c34dd4a
postpone the review request notification
CalK16 Nov 2, 2024
f7f0b7a
Merge branch 'main' into main
CalK16 Nov 2, 2024
0304324
fix linting issues
CalK16 Nov 2, 2024
800232b
more updates on templates
CalK16 Nov 3, 2024
d8d66b5
simplify a condition statement
CalK16 Nov 3, 2024
25ebf69
fix an internal error
CalK16 Nov 3, 2024
418a13c
Merge branch 'main' into CalK16-pr-reviewer
wxiaoguang Nov 3, 2024
941f7fd
fix conflicts
wxiaoguang Nov 3, 2024
a2a06f1
Merge branch 'main' into CalK16-pr-reviewer
wxiaoguang Nov 8, 2024
8de6e40
make code easier to maintain
wxiaoguang Nov 8, 2024
6fb9583
fix duplicate code
wxiaoguang Nov 8, 2024
e4e2de3
refactor reviewer list
wxiaoguang Nov 8, 2024
637e4fb
fix lint
wxiaoguang Nov 8, 2024
4c9169a
Merge branch 'main' into CalK16-pr-reviewer
wxiaoguang Nov 8, 2024
9ccb69c
fix lint
wxiaoguang Nov 8, 2024
1759ea4
fine tune
wxiaoguang Nov 8, 2024
be102f5
fine tune and add comments
wxiaoguang Nov 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion modules/structs/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1458,10 +1458,11 @@ 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.
issues.new.clear_reviewers = Clear reviewers
issues.choose.get_started = Get Started
issues.choose.open_external_link = Open
issues.choose.blank = Default
Expand Down
41 changes: 40 additions & 1 deletion routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -553,8 +554,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)
} else if errors.Is(err, user_model.ErrBlockedUser) {
Expand Down
106 changes: 89 additions & 17 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -557,8 +557,8 @@ func renderMilestones(ctx *context.Context) {
ctx.Data["ClosedMilestones"] = closedMilestones
}

// RetrieveRepoMilestonesAndAssignees find all the milestones and assignees of a repository
func RetrieveRepoMilestonesAndAssignees(ctx *context.Context, repo *repo_model.Repository) {
// RetrieveRepoMilestonesAndAssignees find all the milestones, assignees, and reviewers of a repository
func RetrieveRepoMilestonesAssigneesAndReviewers(ctx *context.Context, repo *repo_model.Repository) {
var err error
ctx.Data["OpenMilestones"], err = db.Find[issues_model.Milestone](ctx, issues_model.FindMilestoneOptions{
RepoID: repo.ID,
Expand All @@ -585,6 +585,8 @@ func RetrieveRepoMilestonesAndAssignees(ctx *context.Context, repo *repo_model.R
ctx.Data["Assignees"] = shared_user.MakeSelfOnTop(ctx.Doer, assigneeUsers)

handleTeamMentions(ctx)

retrieveReviewers(ctx)
}

func retrieveProjects(ctx *context.Context, repo *repo_model.Repository) {
Expand Down Expand Up @@ -862,7 +864,7 @@ func RetrieveRepoMetas(ctx *context.Context, repo *repo_model.Repository, isPull
labels = append(labels, orgLabels...)
}

RetrieveRepoMilestonesAndAssignees(ctx, repo)
RetrieveRepoMilestonesAssigneesAndReviewers(ctx, repo)
if ctx.Written() {
return nil
}
Expand All @@ -883,6 +885,46 @@ func RetrieveRepoMetas(ctx *context.Context, repo *repo_model.Repository, isPull
return labels
}

func retrieveReviewers(ctx *context.Context) {
// 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
}

// Tries to load and set an issue template. The first return value indicates if a template was loaded.
func setTemplateIfExists(ctx *context.Context, ctxDataKey string, possibleFiles []string) (bool, map[string]error) {
commit, err := ctx.Repo.GitRepo.GetBranchCommit(ctx.Repo.Repository.DefaultBranch)
Expand Down Expand Up @@ -1118,15 +1160,15 @@ 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
)

labels := RetrieveRepoMetas(ctx, ctx.Repo.Repository, isPull)
if ctx.Written() {
return nil, nil, 0, 0
return nil, nil, nil, 0, 0
}

var labelIDs []int64
Expand All @@ -1135,7 +1177,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...)
Expand All @@ -1158,11 +1200,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
Expand All @@ -1172,11 +1214,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
Expand All @@ -1188,26 +1230,26 @@ 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
for _, aID := range assigneeIDs {
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
}
}
}
Expand All @@ -1217,7 +1259,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
Expand All @@ -1235,7 +1307,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
}
Expand Down Expand Up @@ -1543,7 +1615,7 @@ func ViewIssue(ctx *context.Context) {

// Check milestone and assignee.
if ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
RetrieveRepoMilestonesAndAssignees(ctx, repo)
RetrieveRepoMilestonesAssigneesAndReviewers(ctx, repo)
retrieveProjects(ctx, repo)

if ctx.Written() {
Expand Down
4 changes: 2 additions & 2 deletions routers/web/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -1269,7 +1269,7 @@ func CompareAndPullRequestPost(ctx *context.Context) {
return
}

labelIDs, assigneeIDs, milestoneID, projectID := ValidateRepoMetas(ctx, *form, true)
labelIDs, assigneeIDs, reviewerIDs, milestoneID, projectID := ValidateRepoMetas(ctx, *form, true)
if ctx.Written() {
return
}
Expand Down Expand Up @@ -1319,7 +1319,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 {
switch {
case repo_model.IsErrUserDoesNotHaveAccessToRepo(err):
ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error())
Expand Down
2 changes: 1 addition & 1 deletion services/agit/agit.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,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
}

Expand Down
1 change: 1 addition & 0 deletions services/forms/repo_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading