diff --git a/routers/api/v1/repo/collaborators.go b/routers/api/v1/repo/collaborators.go index 0bbf5a1ea49..6fb08ee286c 100644 --- a/routers/api/v1/repo/collaborators.go +++ b/routers/api/v1/repo/collaborators.go @@ -17,7 +17,6 @@ import ( "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/convert" - issue_service "code.gitea.io/gitea/services/issue" pull_service "code.gitea.io/gitea/services/pull" repo_service "code.gitea.io/gitea/services/repository" ) @@ -322,7 +321,7 @@ func GetReviewers(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - canChooseReviewer := issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, ctx.Repo.Repository, 0) + canChooseReviewer := pull_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, ctx.Repo.Repository, 0) if !canChooseReviewer { ctx.Error(http.StatusForbidden, "GetReviewers", errors.New("doer has no permission to get reviewers")) return diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index 0dbb5827d3e..481a4de499a 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -19,7 +19,6 @@ import ( "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/convert" - issue_service "code.gitea.io/gitea/services/issue" pull_service "code.gitea.io/gitea/services/pull" ) @@ -653,7 +652,7 @@ func CreateReviewRequests(ctx *context.APIContext) { } } } - comments, err := issue_service.ReviewRequests(ctx, pr, ctx.Doer, filteredUsers, filteredTeams) + comments, err := pull_service.ReviewRequests(ctx, pr, ctx.Doer, filteredUsers, filteredTeams) if err != nil { if issues_model.IsErrReviewRequestOnClosedPR(err) { ctx.Error(http.StatusForbidden, "", err) @@ -802,7 +801,7 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions } for _, reviewer := range reviewers { - comment, err := issue_service.ReviewRequest(ctx, pr, ctx.Doer, &permDoer, reviewer, isAdd) + comment, err := pull_service.ReviewRequest(ctx, pr, ctx.Doer, &permDoer, reviewer, isAdd) if err != nil { if issues_model.IsErrReviewRequestOnClosedPR(err) { ctx.Error(http.StatusForbidden, "", err) @@ -827,7 +826,7 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions if ctx.Repo.Repository.Owner.IsOrganization() && len(opts.TeamReviewers) > 0 { for _, teamReviewer := range teamReviewers { - comment, err := issue_service.TeamReviewRequest(ctx, pr, ctx.Doer, teamReviewer, isAdd) + comment, err := pull_service.TeamReviewRequest(ctx, pr, ctx.Doer, teamReviewer, isAdd) if err != nil { if issues_model.IsErrReviewRequestOnClosedPR(err) { ctx.Error(http.StatusForbidden, "", err) diff --git a/routers/web/repo/issue_page_meta.go b/routers/web/repo/issue_page_meta.go index 7eda6e3c736..6ffce394c54 100644 --- a/routers/web/repo/issue_page_meta.go +++ b/routers/web/repo/issue_page_meta.go @@ -18,7 +18,6 @@ import ( "code.gitea.io/gitea/modules/optional" shared_user "code.gitea.io/gitea/routers/web/shared/user" "code.gitea.io/gitea/services/context" - issue_service "code.gitea.io/gitea/services/issue" pull_service "code.gitea.io/gitea/services/pull" ) @@ -186,7 +185,7 @@ func (d *IssuePageMetaData) retrieveReviewersData(ctx *context.Context) { if d.Issue == nil { data.CanChooseReviewer = true } else { - data.CanChooseReviewer = issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, repo, d.Issue.PosterID) + data.CanChooseReviewer = pull_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, repo, d.Issue.PosterID) } } diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index dcef506e190..29b89997b94 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -20,7 +20,6 @@ import ( "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context/upload" "code.gitea.io/gitea/services/forms" - issue_service "code.gitea.io/gitea/services/issue" pull_service "code.gitea.io/gitea/services/pull" user_service "code.gitea.io/gitea/services/user" ) @@ -399,7 +398,7 @@ func UpdatePullReviewRequest(ctx *context.Context) { return } - _, err = issue_service.TeamReviewRequest(ctx, issue.PullRequest, ctx.Doer, team, action == "attach") + _, err = pull_service.TeamReviewRequest(ctx, issue.PullRequest, ctx.Doer, team, action == "attach") if err != nil { if issues_model.IsErrNotValidReviewRequest(err) { log.Warn( @@ -431,7 +430,7 @@ func UpdatePullReviewRequest(ctx *context.Context) { return } - _, err = issue_service.ReviewRequest(ctx, issue.PullRequest, ctx.Doer, &ctx.Repo.Permission, reviewer, action == "attach") + _, err = pull_service.ReviewRequest(ctx, issue.PullRequest, ctx.Doer, &ctx.Repo.Permission, reviewer, action == "attach") if err != nil { if issues_model.IsErrNotValidReviewRequest(err) { log.Warn( diff --git a/services/issue/assignee.go b/services/issue/assignee.go index f408cde92f7..98c3c8c873c 100644 --- a/services/issue/assignee.go +++ b/services/issue/assignee.go @@ -7,14 +7,7 @@ import ( "context" issues_model "code.gitea.io/gitea/models/issues" - "code.gitea.io/gitea/models/organization" - org_model "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" notify_service "code.gitea.io/gitea/services/notify" ) @@ -60,288 +53,3 @@ 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, pr *issues_model.PullRequest, 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, pr.Issue, permDoer) - if err != nil { - return nil, err - } - - if isAdd { - comment, err = issues_model.AddReviewRequest(ctx, pr.Issue, reviewer, doer) - } else { - comment, err = issues_model.RemoveReviewRequest(ctx, pr.Issue, reviewer, doer) - } - - if err != nil { - return nil, err - } - - if comment != nil { - notify_service.PullRequestReviewRequest(ctx, doer, pr.Issue, reviewer, isAdd, comment) - } - - return comment, err -} - -func ReviewRequests(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, reviewers []*user_model.User, reviewTeams []*org_model.Team) (comments []*issues_model.Comment, err error) { - for _, reviewer := range reviewers { - comment, err := ReviewRequest(ctx, pr, doer, nil, reviewer, true) - if err != nil { - return nil, err - } - comments = append(comments, comment) - } - - for _, reviewTeam := range reviewTeams { - comment, err := TeamReviewRequest(ctx, pr, doer, reviewTeam, true) - if err != nil { - return nil, err - } - comments = append(comments, comment) - } - - return comments, nil -} - -// 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, - } - } - - permReviewer, err := access_model.GetUserRepoPermission(ctx, issue.Repo, reviewer) - if err != nil { - return err - } - - if permDoer == nil { - permDoer = new(access_model.Permission) - *permDoer, err = access_model.GetUserRepoPermission(ctx, issue.Repo, doer) - 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 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, - } - } - - 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, - } - } - - 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, - } - } - - if canDoerChangeReviewRequests { - return nil - } - - return issues_model.ErrNotValidReviewRequest{ - Reason: "Doer can't remove reviewer", - UserID: doer.ID, - RepoID: issue.Repo.ID, - } -} - -// TeamReviewRequest add or remove a review request from a team for this PR, and make comment for it. -func TeamReviewRequest(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, reviewer *organization.Team, isAdd bool) (comment *issues_model.Comment, err error) { - err = isValidTeamReviewRequest(ctx, reviewer, doer, isAdd, pr.Issue) - if err != nil { - return nil, err - } - if isAdd { - comment, err = issues_model.AddTeamReviewRequest(ctx, pr.Issue, reviewer, doer) - } else { - comment, err = issues_model.RemoveTeamReviewRequest(ctx, pr.Issue, reviewer, doer) - } - - if err != nil { - return nil, err - } - - if comment == nil || !isAdd { - return nil, nil - } - - return comment, teamReviewRequestNotify(ctx, pr.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 - } - - members, err := organization.GetTeamMembers(ctx, &organization.SearchMembersOptions{ - TeamID: reviewer.ID, - }) - 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 - } - if isCollaborator { - return true - } - - // 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.GetTeamsWithAccessToRepo(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead) - 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 - } - } - } - - return false -} diff --git a/services/issue/issue.go b/services/issue/issue.go index c6a52cc0fe5..e2bddc2e16d 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -17,7 +17,6 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/storage" notify_service "code.gitea.io/gitea/services/notify" ) @@ -90,17 +89,7 @@ func ChangeTitle(ctx context.Context, issue *issues_model.Issue, doer *user_mode return err } - var reviewNotifiers []*ReviewRequestNotifier - if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issues_model.HasWorkInProgressPrefix(title) { - var err error - reviewNotifiers, err = PullRequestCodeOwnersReview(ctx, issue, issue.PullRequest) - if err != nil { - log.Error("PullRequestCodeOwnersReview: %v", err) - } - } - notify_service.IssueChangeTitle(ctx, doer, issue, oldTitle) - ReviewRequestNotify(ctx, issue, issue.Poster, reviewNotifiers) return nil } diff --git a/services/issue/pull.go b/services/issue/pull.go deleted file mode 100644 index 896802108d6..00000000000 --- a/services/issue/pull.go +++ /dev/null @@ -1,147 +0,0 @@ -// Copyright 2024 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package issue - -import ( - "context" - "fmt" - "time" - - issues_model "code.gitea.io/gitea/models/issues" - org_model "code.gitea.io/gitea/models/organization" - user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/git" - "code.gitea.io/gitea/modules/gitrepo" - "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/setting" -) - -func getMergeBase(repo *git.Repository, pr *issues_model.PullRequest, baseBranch, headBranch string) (string, error) { - // Add a temporary remote - tmpRemote := fmt.Sprintf("mergebase-%d-%d", pr.ID, time.Now().UnixNano()) - if err := repo.AddRemote(tmpRemote, repo.Path, false); err != nil { - return "", fmt.Errorf("AddRemote: %w", err) - } - defer func() { - if err := repo.RemoveRemote(tmpRemote); err != nil { - log.Error("getMergeBase: RemoveRemote: %v", err) - } - }() - - mergeBase, _, err := repo.GetMergeBase(tmpRemote, baseBranch, headBranch) - return mergeBase, err -} - -type ReviewRequestNotifier struct { - Comment *issues_model.Comment - IsAdd bool - Reviewer *user_model.User - ReviewTeam *org_model.Team -} - -func PullRequestCodeOwnersReview(ctx context.Context, issue *issues_model.Issue, pr *issues_model.PullRequest) ([]*ReviewRequestNotifier, error) { - files := []string{"CODEOWNERS", "docs/CODEOWNERS", ".gitea/CODEOWNERS"} - - if pr.IsWorkInProgress(ctx) { - return nil, nil - } - - if err := pr.LoadHeadRepo(ctx); err != nil { - return nil, err - } - - if err := pr.LoadBaseRepo(ctx); err != nil { - return nil, err - } - - if pr.BaseRepo.IsFork { - return nil, nil - } - - repo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo) - if err != nil { - return nil, err - } - defer repo.Close() - - commit, err := repo.GetBranchCommit(pr.BaseRepo.DefaultBranch) - if err != nil { - return nil, err - } - - var data string - for _, file := range files { - if blob, err := commit.GetBlobByPath(file); err == nil { - data, err = blob.GetBlobContent(setting.UI.MaxDisplayFileSize) - if err == nil { - break - } - } - } - - rules, _ := issues_model.GetCodeOwnersFromContent(ctx, data) - - // get the mergebase - mergeBase, err := getMergeBase(repo, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName()) - if err != nil { - return nil, err - } - - // https://github.com/go-gitea/gitea/issues/29763, we need to get the files changed - // between the merge base and the head commit but not the base branch and the head commit - changedFiles, err := repo.GetFilesChangedBetween(mergeBase, pr.GetGitRefName()) - if err != nil { - return nil, err - } - - uniqUsers := make(map[int64]*user_model.User) - uniqTeams := make(map[string]*org_model.Team) - for _, rule := range rules { - for _, f := range changedFiles { - if (rule.Rule.MatchString(f) && !rule.Negative) || (!rule.Rule.MatchString(f) && rule.Negative) { - for _, u := range rule.Users { - uniqUsers[u.ID] = u - } - for _, t := range rule.Teams { - uniqTeams[fmt.Sprintf("%d/%d", t.OrgID, t.ID)] = t - } - } - } - } - - notifiers := make([]*ReviewRequestNotifier, 0, len(uniqUsers)+len(uniqTeams)) - - if err := issue.LoadPoster(ctx); err != nil { - return nil, err - } - - for _, u := range uniqUsers { - if u.ID != issue.Poster.ID { - comment, err := issues_model.AddReviewRequest(ctx, issue, u, issue.Poster) - if err != nil { - log.Warn("Failed add assignee user: %s to PR review: %s#%d, error: %s", u.Name, pr.BaseRepo.Name, pr.ID, err) - return nil, err - } - notifiers = append(notifiers, &ReviewRequestNotifier{ - Comment: comment, - IsAdd: true, - Reviewer: u, - }) - } - } - for _, t := range uniqTeams { - comment, err := issues_model.AddTeamReviewRequest(ctx, issue, t, issue.Poster) - if err != nil { - log.Warn("Failed add assignee team: %s to PR review: %s#%d, error: %s", t.Name, pr.BaseRepo.Name, pr.ID, err) - return nil, err - } - notifiers = append(notifiers, &ReviewRequestNotifier{ - Comment: comment, - IsAdd: true, - ReviewTeam: t, - }) - } - - return notifiers, nil -} diff --git a/services/pull/notify.go b/services/pull/notify.go new file mode 100644 index 00000000000..bcece2bef22 --- /dev/null +++ b/services/pull/notify.go @@ -0,0 +1,36 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "context" + + issues_model "code.gitea.io/gitea/models/issues" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/log" + notify_service "code.gitea.io/gitea/services/notify" +) + +type pullNotifier struct { + notify_service.NullNotifier +} + +var _ notify_service.Notifier = &pullNotifier{} + +// NewNotifier create a new indexerNotifier notifier +func NewNotifier() notify_service.Notifier { + return &pullNotifier{} +} + +func (r *pullNotifier) IssueChangeTitle(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, oldTitle string) { + var reviewNotifiers []*ReviewRequestNotifier + if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issues_model.HasWorkInProgressPrefix(issue.Title) { + var err error + reviewNotifiers, err = RequestCodeOwnersReview(ctx, issue, issue.PullRequest) + if err != nil { + log.Error("RequestCodeOwnersReview: %v", err) + } + } + ReviewRequestNotify(ctx, issue, issue.Poster, reviewNotifiers) +} diff --git a/services/pull/pull.go b/services/pull/pull.go index 1180586ad06..751ade986cb 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -115,7 +115,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { } defer baseGitRepo.Close() - var reviewNotifiers []*issue_service.ReviewRequestNotifier + var reviewNotifiers []*ReviewRequestNotifier if err := db.WithTx(ctx, func(ctx context.Context) error { if err := issues_model.NewPullRequest(ctx, repo, issue, labelIDs, uuids, pr); err != nil { return err @@ -175,7 +175,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { } if !pr.IsWorkInProgress(ctx) { - reviewNotifiers, err = issue_service.PullRequestCodeOwnersReview(ctx, issue, pr) + reviewNotifiers, err = RequestCodeOwnersReview(ctx, issue, pr) if err != nil { return err } @@ -190,7 +190,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { } baseGitRepo.Close() // close immediately to avoid notifications will open the repository again - issue_service.ReviewRequestNotify(ctx, issue, issue.Poster, reviewNotifiers) + ReviewRequestNotify(ctx, issue, issue.Poster, reviewNotifiers) mentions, err := issues_model.FindAndUpdateIssueMentions(ctx, issue, issue.Poster, issue.Content) if err != nil { @@ -212,12 +212,12 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { } permDoer, err := access_model.GetUserRepoPermission(ctx, repo, issue.Poster) for _, reviewer := range opts.Reviewers { - if _, err = issue_service.ReviewRequest(ctx, pr, issue.Poster, &permDoer, reviewer, true); err != nil { + if _, err = ReviewRequest(ctx, pr, issue.Poster, &permDoer, reviewer, true); err != nil { return err } } for _, teamReviewer := range opts.TeamReviewers { - if _, err = issue_service.TeamReviewRequest(ctx, pr, issue.Poster, teamReviewer, true); err != nil { + if _, err = TeamReviewRequest(ctx, pr, issue.Poster, teamReviewer, true); err != nil { return err } } diff --git a/services/pull/review_request.go b/services/pull/review_request.go new file mode 100644 index 00000000000..8ca74aad2c2 --- /dev/null +++ b/services/pull/review_request.go @@ -0,0 +1,437 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "context" + "fmt" + "time" + + issues_model "code.gitea.io/gitea/models/issues" + org_model "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/git" + "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" + notify_service "code.gitea.io/gitea/services/notify" +) + +func getMergeBase(repo *git.Repository, pr *issues_model.PullRequest, baseBranch, headBranch string) (string, error) { + // Add a temporary remote + tmpRemote := fmt.Sprintf("mergebase-%d-%d", pr.ID, time.Now().UnixNano()) + if err := repo.AddRemote(tmpRemote, repo.Path, false); err != nil { + return "", fmt.Errorf("AddRemote: %w", err) + } + defer func() { + if err := repo.RemoveRemote(tmpRemote); err != nil { + log.Error("getMergeBase: RemoveRemote: %v", err) + } + }() + + mergeBase, _, err := repo.GetMergeBase(tmpRemote, baseBranch, headBranch) + return mergeBase, err +} + +type ReviewRequestNotifier struct { + Comment *issues_model.Comment + IsAdd bool + Reviewer *user_model.User + ReviewTeam *org_model.Team +} + +func RequestCodeOwnersReview(ctx context.Context, issue *issues_model.Issue, pr *issues_model.PullRequest) ([]*ReviewRequestNotifier, error) { + files := []string{"CODEOWNERS", "docs/CODEOWNERS", ".gitea/CODEOWNERS"} + + if pr.IsWorkInProgress(ctx) { + return nil, nil + } + + if err := pr.LoadHeadRepo(ctx); err != nil { + return nil, err + } + + if err := pr.LoadBaseRepo(ctx); err != nil { + return nil, err + } + + if pr.BaseRepo.IsFork { + return nil, nil + } + + repo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo) + if err != nil { + return nil, err + } + defer repo.Close() + + commit, err := repo.GetBranchCommit(pr.BaseRepo.DefaultBranch) + if err != nil { + return nil, err + } + + var data string + for _, file := range files { + if blob, err := commit.GetBlobByPath(file); err == nil { + data, err = blob.GetBlobContent(setting.UI.MaxDisplayFileSize) + if err == nil { + break + } + } + } + + rules, _ := issues_model.GetCodeOwnersFromContent(ctx, data) + + // get the mergebase + mergeBase, err := getMergeBase(repo, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName()) + if err != nil { + return nil, err + } + + // https://github.com/go-gitea/gitea/issues/29763, we need to get the files changed + // between the merge base and the head commit but not the base branch and the head commit + changedFiles, err := repo.GetFilesChangedBetween(mergeBase, pr.GetGitRefName()) + if err != nil { + return nil, err + } + + uniqUsers := make(map[int64]*user_model.User) + uniqTeams := make(map[string]*org_model.Team) + for _, rule := range rules { + for _, f := range changedFiles { + if (rule.Rule.MatchString(f) && !rule.Negative) || (!rule.Rule.MatchString(f) && rule.Negative) { + for _, u := range rule.Users { + uniqUsers[u.ID] = u + } + for _, t := range rule.Teams { + uniqTeams[fmt.Sprintf("%d/%d", t.OrgID, t.ID)] = t + } + } + } + } + + notifiers := make([]*ReviewRequestNotifier, 0, len(uniqUsers)+len(uniqTeams)) + + if err := issue.LoadPoster(ctx); err != nil { + return nil, err + } + + for _, u := range uniqUsers { + if u.ID != issue.Poster.ID { + comment, err := issues_model.AddReviewRequest(ctx, issue, u, issue.Poster) + if err != nil { + log.Warn("Failed add assignee user: %s to PR review: %s#%d, error: %s", u.Name, pr.BaseRepo.Name, pr.ID, err) + return nil, err + } + notifiers = append(notifiers, &ReviewRequestNotifier{ + Comment: comment, + IsAdd: true, + Reviewer: u, + }) + } + } + for _, t := range uniqTeams { + comment, err := issues_model.AddTeamReviewRequest(ctx, issue, t, issue.Poster) + if err != nil { + log.Warn("Failed add assignee team: %s to PR review: %s#%d, error: %s", t.Name, pr.BaseRepo.Name, pr.ID, err) + return nil, err + } + notifiers = append(notifiers, &ReviewRequestNotifier{ + Comment: comment, + IsAdd: true, + ReviewTeam: t, + }) + } + + return notifiers, nil +} + +// ReviewRequest add or remove a review request from a user for this PR, and make comment for it. +func ReviewRequest(ctx context.Context, pr *issues_model.PullRequest, 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, pr.Issue, permDoer) + if err != nil { + return nil, err + } + + if isAdd { + comment, err = issues_model.AddReviewRequest(ctx, pr.Issue, reviewer, doer) + } else { + comment, err = issues_model.RemoveReviewRequest(ctx, pr.Issue, reviewer, doer) + } + + if err != nil { + return nil, err + } + + if comment != nil { + notify_service.PullRequestReviewRequest(ctx, doer, pr.Issue, reviewer, isAdd, comment) + } + + return comment, err +} + +func ReviewRequests(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, reviewers []*user_model.User, reviewTeams []*org_model.Team) (comments []*issues_model.Comment, err error) { + for _, reviewer := range reviewers { + comment, err := ReviewRequest(ctx, pr, doer, nil, reviewer, true) + if err != nil { + return nil, err + } + comments = append(comments, comment) + } + + for _, reviewTeam := range reviewTeams { + comment, err := TeamReviewRequest(ctx, pr, doer, reviewTeam, true) + if err != nil { + return nil, err + } + comments = append(comments, comment) + } + + return comments, nil +} + +// 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, + } + } + + permReviewer, err := access_model.GetUserRepoPermission(ctx, issue.Repo, reviewer) + if err != nil { + return err + } + + if permDoer == nil { + permDoer = new(access_model.Permission) + *permDoer, err = access_model.GetUserRepoPermission(ctx, issue.Repo, doer) + 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 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, + } + } + + 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 *org_model.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, + } + } + + canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue.PosterID) + + if isAdd { + if issue.Repo.IsPrivate { + hasTeam := org_model.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, + } + } + + if canDoerChangeReviewRequests { + return nil + } + + return issues_model.ErrNotValidReviewRequest{ + Reason: "Doer can't remove reviewer", + UserID: doer.ID, + RepoID: issue.Repo.ID, + } +} + +// TeamReviewRequest add or remove a review request from a team for this PR, and make comment for it. +func TeamReviewRequest(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, reviewer *org_model.Team, isAdd bool) (comment *issues_model.Comment, err error) { + err = isValidTeamReviewRequest(ctx, reviewer, doer, isAdd, pr.Issue) + if err != nil { + return nil, err + } + if isAdd { + comment, err = issues_model.AddTeamReviewRequest(ctx, pr.Issue, reviewer, doer) + } else { + comment, err = issues_model.RemoveTeamReviewRequest(ctx, pr.Issue, reviewer, doer) + } + + if err != nil { + return nil, err + } + + if comment == nil || !isAdd { + return nil, nil + } + + return comment, teamReviewRequestNotify(ctx, pr.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 *org_model.Team, isAdd bool, comment *issues_model.Comment) error { + // notify all user in this team + if err := comment.LoadIssue(ctx); err != nil { + return err + } + + members, err := org_model.GetTeamMembers(ctx, &org_model.SearchMembersOptions{ + TeamID: reviewer.ID, + }) + 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 + } + if isCollaborator { + return true + } + + // 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 := org_model.GetTeamsWithAccessToRepo(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead) + if err != nil { + log.Error("GetTeamsWithAccessToRepo: %v", err) + return false + } + for _, team := range teams { + if !team.UnitEnabled(ctx, unit.TypePullRequests) { + continue + } + isMember, err := org_model.IsTeamMember(ctx, repo.OwnerID, team.ID, doer.ID) + if err != nil { + log.Error("IsTeamMember: %v", err) + continue + } + if isMember { + return true + } + } + } + + return false +} diff --git a/tests/integration/api_pull_review_test.go b/tests/integration/api_pull_review_test.go index ba6b62d0d70..102c4381be5 100644 --- a/tests/integration/api_pull_review_test.go +++ b/tests/integration/api_pull_review_test.go @@ -17,7 +17,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/json" api "code.gitea.io/gitea/modules/structs" - issue_service "code.gitea.io/gitea/services/issue" + pull_service "code.gitea.io/gitea/services/pull" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -425,7 +425,8 @@ func TestAPIPullReviewStayDismissed(t *testing.T) { // user8 dismiss review 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, pullIssue.LoadPullRequest(db.DefaultContext)) + _, err = pull_service.ReviewRequest(db.DefaultContext, pullIssue.PullRequest, user8, &permUser8, user8, false) assert.NoError(t, err) reviewsCountCheck(t,