Fix wrong review requests when updating the pull request (#34286)

Fix #34224

The previous implementation in #33744 will get the pushed commits
changed files. But it's not always right when push a merged commit. This
PR reverted the logic in #33744 and will always get the PR's changed
files and get code owners.
This commit is contained in:
Lunny Xiao 2025-04-28 15:57:56 -07:00 committed by GitHub
parent 0148d03f21
commit 44d7d2973a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 23 additions and 20 deletions

View File

@ -48,10 +48,6 @@ func IsCodeOwnerFile(f string) bool {
} }
func PullRequestCodeOwnersReview(ctx context.Context, pr *issues_model.PullRequest) ([]*ReviewRequestNotifier, error) { func PullRequestCodeOwnersReview(ctx context.Context, pr *issues_model.PullRequest) ([]*ReviewRequestNotifier, error) {
return PullRequestCodeOwnersReviewSpecialCommits(ctx, pr, "", "") // no commit is provided, then it uses PR's base&head branch
}
func PullRequestCodeOwnersReviewSpecialCommits(ctx context.Context, pr *issues_model.PullRequest, startCommitID, endCommitID string) ([]*ReviewRequestNotifier, error) {
if err := pr.LoadIssue(ctx); err != nil { if err := pr.LoadIssue(ctx); err != nil {
return nil, err return nil, err
} }
@ -100,19 +96,15 @@ func PullRequestCodeOwnersReviewSpecialCommits(ctx context.Context, pr *issues_m
return nil, nil return nil, nil
} }
if startCommitID == "" && endCommitID == "" { // get the mergebase
// get the mergebase mergeBase, err := getMergeBase(repo, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName())
mergeBase, err := getMergeBase(repo, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName()) if err != nil {
if err != nil { return nil, err
return nil, err
}
startCommitID = mergeBase
endCommitID = pr.GetGitRefName()
} }
// https://github.com/go-gitea/gitea/issues/29763, we need to get the files changed // 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 // between the merge base and the head commit but not the base branch and the head commit
changedFiles, err := repo.GetFilesChangedBetween(startCommitID, endCommitID) changedFiles, err := repo.GetFilesChangedBetween(mergeBase, pr.GetGitRefName())
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -138,8 +130,23 @@ func PullRequestCodeOwnersReviewSpecialCommits(ctx context.Context, pr *issues_m
return nil, err return nil, err
} }
// load all reviews from database
latestReivews, _, err := issues_model.GetReviewsByIssueID(ctx, pr.IssueID)
if err != nil {
return nil, err
}
contain := func(list issues_model.ReviewList, u *user_model.User) bool {
for _, review := range list {
if review.ReviewerTeamID == 0 && review.ReviewerID == u.ID {
return true
}
}
return false
}
for _, u := range uniqUsers { for _, u := range uniqUsers {
if u.ID != issue.Poster.ID { if u.ID != issue.Poster.ID && !contain(latestReivews, u) {
comment, err := issues_model.AddReviewRequest(ctx, issue, u, issue.Poster) comment, err := issues_model.AddReviewRequest(ctx, issue, u, issue.Poster)
if err != nil { 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) log.Warn("Failed add assignee user: %s to PR review: %s#%d, error: %s", u.Name, pr.BaseRepo.Name, pr.ID, err)
@ -155,6 +162,7 @@ func PullRequestCodeOwnersReviewSpecialCommits(ctx context.Context, pr *issues_m
}) })
} }
} }
for _, t := range uniqTeams { for _, t := range uniqTeams {
comment, err := issues_model.AddTeamReviewRequest(ctx, issue, t, issue.Poster) comment, err := issues_model.AddTeamReviewRequest(ctx, issue, t, issue.Poster)
if err != nil { if err != nil {

View File

@ -463,12 +463,7 @@ func AddTestPullRequestTask(opts TestPullRequestOptions) {
} }
if !pr.IsWorkInProgress(ctx) { if !pr.IsWorkInProgress(ctx) {
var reviewNotifiers []*issue_service.ReviewRequestNotifier reviewNotifiers, err := issue_service.PullRequestCodeOwnersReview(ctx, pr)
if opts.IsForcePush {
reviewNotifiers, err = issue_service.PullRequestCodeOwnersReview(ctx, pr)
} else {
reviewNotifiers, err = issue_service.PullRequestCodeOwnersReviewSpecialCommits(ctx, pr, opts.OldCommitID, opts.NewCommitID)
}
if err != nil { if err != nil {
log.Error("PullRequestCodeOwnersReview: %v", err) log.Error("PullRequestCodeOwnersReview: %v", err)
} }