Skip to content

Move review request functions to a standalone file #37358

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
silverwind merged 3 commits into from
Apr 23, 2026
+335 −321
Merged

Move review request functions to a standalone file #37358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a94513a
move review request functions to a standalone file
lunny Apr 22, 2026
08691f7
Merge branch 'main' into lunny/refactor_assignee
lunny Apr 22, 2026
fa92ef8
Merge branch 'main' into lunny/refactor_assignee
lunny Apr 23, 2026
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
285 changes: 50 additions & 235 deletions services/issue/assignee.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,10 @@ import (
"context"

issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/organization"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/container"
notify_service "code.gitea.io/gitea/services/notify"
)

Expand Down Expand Up @@ -62,267 +59,85 @@ func ToggleAssigneeWithNotify(ctx context.Context, issue *issues_model.Issue, do
return removed, comment, err
}

// 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 *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, false)
} else {
comment, err = issues_model.RemoveReviewRequest(ctx, issue, reviewer, doer)
}

if err != nil {
return nil, err
}

if comment != nil {
notify_service.PullRequestReviewRequest(ctx, doer, issue, reviewer, isAdd, comment)
}

return comment, err
}

// 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 {
if reviewer.IsOrganization() {
return issues_model.ErrNotValidReviewRequest{
Reason: "Organization can't be added as reviewer",
UserID: doer.ID,
RepoID: issue.Repo.ID,
}
}
if doer.IsOrganization() {
return issues_model.ErrNotValidReviewRequest{
Reason: "Organization can't be doer to add reviewer",
UserID: doer.ID,
RepoID: issue.Repo.ID,
}
}
// UpdateAssignees is a helper function to add or delete one or multiple issue assignee(s)
// Deleting is done the GitHub way (quote from their api documentation):
// https://developer.github.com/v3/issues/#edit-an-issue
// "assignees" (array): Logins for Users to assign to this issue.
// Pass one or more user logins to replace the set of assignees on this Issue.
// Send an empty array ([]) to clear all assignees from the Issue.
func UpdateAssignees(ctx context.Context, issue *issues_model.Issue, oneAssignee string, multipleAssignees []string, doer *user_model.User) (err error) {
uniqueAssignees := container.SetOf(multipleAssignees...)

permReviewer, err := access_model.GetIndividualUserRepoPermission(ctx, issue.Repo, reviewer)
if err != nil {
return err
// Keep the old assignee thingy for compatibility reasons
if oneAssignee != "" {
uniqueAssignees.Add(oneAssignee)
}

if permDoer == nil {
permDoer = new(access_model.Permission)
*permDoer, err = access_model.GetDoerRepoPermission(ctx, issue.Repo, doer)
// Loop through all assignees to add them
allNewAssignees := make([]*user_model.User, 0, len(uniqueAssignees))
for _, assigneeName := range uniqueAssignees.Values() {
assignee, err := user_model.GetUserByName(ctx, assigneeName)
if err != nil {
return err
}
}

lastReview, err := issues_model.GetReviewByIssueIDAndUserID(ctx, issue.ID, reviewer.ID)
if err != nil && !issues_model.IsErrReviewNotExist(err) {
return err
}

canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue.PosterID)

if isAdd {
if !permReviewer.CanAccessAny(perm.AccessModeRead, unit.TypePullRequests) {
return issues_model.ErrNotValidReviewRequest{
Reason: "Reviewer can't read",
UserID: doer.ID,
RepoID: issue.Repo.ID,
}
}

if reviewer.ID == issue.PosterID && issue.OriginalAuthorID == 0 {
return issues_model.ErrNotValidReviewRequest{
Reason: "poster of pr can't be reviewer",
UserID: doer.ID,
RepoID: issue.Repo.ID,
}
if user_model.IsUserBlockedBy(ctx, doer, assignee.ID) {
return user_model.ErrBlockedUser
}

if canDoerChangeReviewRequests {
return nil
}

if doer.ID == issue.PosterID && issue.OriginalAuthorID == 0 && lastReview != nil && lastReview.Type != issues_model.ReviewTypeRequest {
return nil
}

return issues_model.ErrNotValidReviewRequest{
Reason: "Doer can't choose reviewer",
UserID: doer.ID,
RepoID: issue.Repo.ID,
}
allNewAssignees = append(allNewAssignees, assignee)
}

if canDoerChangeReviewRequests {
return nil
}

if lastReview != nil && lastReview.Type == issues_model.ReviewTypeRequest && lastReview.ReviewerID == doer.ID {
return nil
}

return issues_model.ErrNotValidReviewRequest{
Reason: "Doer can't remove reviewer",
UserID: doer.ID,
RepoID: issue.Repo.ID,
}
}

// 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 {
if doer.IsOrganization() {
return issues_model.ErrNotValidReviewRequest{
Reason: "Organization can't be doer to add reviewer",
UserID: doer.ID,
RepoID: issue.Repo.ID,
}
// Delete all old assignees not passed
if err = DeleteNotPassedAssignee(ctx, issue, doer, allNewAssignees); err != nil {
return err
}

canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue.PosterID)

if isAdd {
if issue.Repo.IsPrivate {
hasTeam := organization.HasTeamRepo(ctx, reviewer.OrgID, reviewer.ID, issue.RepoID)

if !hasTeam {
return issues_model.ErrNotValidReviewRequest{
Reason: "Reviewing team can't read repo",
UserID: doer.ID,
RepoID: issue.Repo.ID,
}
}
}

if canDoerChangeReviewRequests {
return nil
}

return issues_model.ErrNotValidReviewRequest{
Reason: "Doer can't choose reviewer",
UserID: doer.ID,
RepoID: issue.Repo.ID,
// Add all new assignees
// Update the assignee. The function will check if the user exists, is already
// assigned (which he shouldn't as we deleted all assignees before) and
// has access to the repo.
for _, assignee := range allNewAssignees {
// Extra method to prevent double adding (which would result in removing)
_, err = AddAssigneeIfNotAssigned(ctx, issue, doer, assignee.ID, true)
if err != nil {
return err
}
}

if canDoerChangeReviewRequests {
return nil
}

return issues_model.ErrNotValidReviewRequest{
Reason: "Doer can't remove reviewer",
UserID: doer.ID,
RepoID: issue.Repo.ID,
}
return err
}

// 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)
// AddAssigneeIfNotAssigned adds an assignee only if he isn't already assigned to the issue.
// Also checks for access of assigned user
func AddAssigneeIfNotAssigned(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, assigneeID int64, notify bool) (comment *issues_model.Comment, err error) {
assignee, err := user_model.GetUserByID(ctx, assigneeID)
if err != nil {
return nil, err
}
if isAdd {
comment, err = issues_model.AddTeamReviewRequest(ctx, issue, reviewer, doer, false)
} else {
comment, err = issues_model.RemoveTeamReviewRequest(ctx, issue, reviewer, doer)
}

// Check if the user is already assigned
isAssigned, err := issues_model.IsUserAssignedToIssue(ctx, issue, assignee)
if err != nil {
return nil, err
}

if comment == nil || !isAdd {
return nil, nil //nolint:nilnil // return nil because no comment was created or it is a removal
}

return comment, teamReviewRequestNotify(ctx, issue, doer, reviewer, isAdd, comment)
}

func ReviewRequestNotify(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, reviewNotifiers []*ReviewRequestNotifier) {
for _, reviewNotifier := range reviewNotifiers {
if reviewNotifier.Reviewer != nil {
notify_service.PullRequestReviewRequest(ctx, issue.Poster, issue, reviewNotifier.Reviewer, reviewNotifier.IsAdd, reviewNotifier.Comment)
} else if reviewNotifier.ReviewTeam != nil {
if err := teamReviewRequestNotify(ctx, issue, issue.Poster, reviewNotifier.ReviewTeam, reviewNotifier.IsAdd, reviewNotifier.Comment); err != nil {
log.Error("teamReviewRequestNotify: %v", err)
}
}
}
}

// teamReviewRequestNotify notify all user in this team
func teamReviewRequestNotify(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, reviewer *organization.Team, isAdd bool, comment *issues_model.Comment) error {
// notify all user in this team
if err := comment.LoadIssue(ctx); err != nil {
return err
if isAssigned {
// nothing to do
return nil, nil //nolint:nilnil // return nil because the user is already assigned
}

members, err := organization.GetTeamMembers(ctx, &organization.SearchMembersOptions{
TeamID: reviewer.ID,
})
valid, err := access_model.CanBeAssigned(ctx, assignee, issue.Repo, issue.IsPull)
if err != nil {
return err
}

for _, member := range members {
if member.ID == comment.Issue.PosterID {
continue
}
comment.AssigneeID = member.ID
notify_service.PullRequestReviewRequest(ctx, doer, issue, member, isAdd, comment)
}

return err
}

// 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, posterID int64) bool {
if repo.IsArchived {
return false
}
// The poster of the PR can change the reviewers
if doer.ID == posterID {
return true
}

// The owner of the repo can change the reviewers
if doer.ID == repo.OwnerID {
return true
}

// Collaborators of the repo can change the reviewers
isCollaborator, err := repo_model.IsCollaborator(ctx, repo.ID, doer.ID)
if err != nil {
log.Error("IsCollaborator: %v", err)
return false
return nil, err
}
if isCollaborator {
return true
if !valid {
return nil, repo_model.ErrUserDoesNotHaveAccessToRepo{UserID: assigneeID, RepoName: issue.Repo.Name}
}

// If the repo's owner is an organization, members of teams with read permission on pull requests can change reviewers
if repo.Owner.IsOrganization() {
teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests)
if err != nil {
log.Error("GetTeamsWithAccessToRepo: %v", err)
return false
}
for _, team := range teams {
if !team.UnitEnabled(ctx, unit.TypePullRequests) {
continue
}
isMember, err := organization.IsTeamMember(ctx, repo.OwnerID, team.ID, doer.ID)
if err != nil {
log.Error("IsTeamMember: %v", err)
continue
}
if isMember {
return true
}
}
if notify {
_, comment, err = ToggleAssigneeWithNotify(ctx, issue, doer, assigneeID)
return comment, err
}

return false
_, comment, err = issues_model.ToggleIssueAssignee(ctx, issue, doer, assigneeID)
return comment, err
}
Loading