Fix push commits comments when changing the pull request target branch (#35386) (#35443)

Backport #35386 

When changing the pull request target branch, the pushed commits
comments will not be changed resulted the number are inconsistent
between commits tab number and the pushed commits comments number.

This PR will remove all the previous pushed commits comments and
calculate new comments when changing the target branch.

Before:

<img width="928" height="585" alt="image"

src="https://github.com/user-attachments/assets/35e4d31f-31a1-4d14-83b0-1786721ab0d9"
/>

After:
<img width="816" height="623" alt="image"

src="https://github.com/user-attachments/assets/24b6dafe-9238-4e7e-833d-68472457afab"
/>

Author: lunny

services/agit/agit.go

@@ -250,7 +250,7 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
 		if err != nil {
 			return nil, fmt.Errorf("failed to load pull issue. Error: %w", err)
 		}
-		comment, err := pull_service.CreatePushPullComment(ctx, pusher, pr, oldCommitID, opts.NewCommitIDs[i])
+		comment, err := pull_service.CreatePushPullComment(ctx, pusher, pr, oldCommitID, opts.NewCommitIDs[i], forcePush.Value())
 		if err == nil && comment != nil {
 			notify_service.PullRequestPushCommits(ctx, pusher, pr, comment)
 		}

services/pull/comment.go

@@ -14,81 +14,69 @@ import (
 )
 
 // getCommitIDsFromRepo get commit IDs from repo in between oldCommitID and newCommitID
-// isForcePush will be true if oldCommit isn't on the branch
 // Commit on baseBranch will skip
-func getCommitIDsFromRepo(ctx context.Context, repo *repo_model.Repository, oldCommitID, newCommitID, baseBranch string) (commitIDs []string, isForcePush bool, err error) {
+func getCommitIDsFromRepo(ctx context.Context, repo *repo_model.Repository, oldCommitID, newCommitID, baseBranch string) (commitIDs []string, err error) {
 	gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo)
 	if err != nil {
-		return nil, false, err
+		return nil, err
 	}
 	defer closer.Close()
 
 	oldCommit, err := gitRepo.GetCommit(oldCommitID)
 	if err != nil {
-		return nil, false, err
+		return nil, err
 	}
 
 	newCommit, err := gitRepo.GetCommit(newCommitID)
 	if err != nil {
-		return nil, false, err
-	}
-
-	isForcePush, err = newCommit.IsForcePush(oldCommitID)
-	if err != nil {
-		return nil, false, err
-	}
-
-	if isForcePush {
-		commitIDs = make([]string, 2)
-		commitIDs[0] = oldCommitID
-		commitIDs[1] = newCommitID
-
-		return commitIDs, isForcePush, err
+		return nil, err
 	}
 
 	// Find commits between new and old commit excluding base branch commits
 	commits, err := gitRepo.CommitsBetweenNotBase(newCommit, oldCommit, baseBranch)
 	if err != nil {
-		return nil, false, err
+		return nil, err
 	}
 
 	commitIDs = make([]string, 0, len(commits))
 	for i := len(commits) - 1; i >= 0; i-- {
 		commitIDs = append(commitIDs, commits[i].ID.String())
 	}
 
-	return commitIDs, isForcePush, err
+	return commitIDs, err
 }
 
 // CreatePushPullComment create push code to pull base comment
-func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *issues_model.PullRequest, oldCommitID, newCommitID string) (comment *issues_model.Comment, err error) {
+func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *issues_model.PullRequest, oldCommitID, newCommitID string, isForcePush bool) (comment *issues_model.Comment, err error) {
 	if pr.HasMerged || oldCommitID == "" || newCommitID == "" {
 		return nil, nil
 	}
 
-	ops := &issues_model.CreateCommentOptions{
-		Type: issues_model.CommentTypePullRequestPush,
-		Doer: pusher,
-		Repo: pr.BaseRepo,
+	opts := &issues_model.CreateCommentOptions{
+		Type:        issues_model.CommentTypePullRequestPush,
+		Doer:        pusher,
+		Repo:        pr.BaseRepo,
+		IsForcePush: isForcePush,
+		Issue:       pr.Issue,
 	}
 
 	var data issues_model.PushActionContent
-
-	data.CommitIDs, data.IsForcePush, err = getCommitIDsFromRepo(ctx, pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch)
-	if err != nil {
-		return nil, err
+	if opts.IsForcePush {
+		data.CommitIDs = []string{oldCommitID, newCommitID}
+	} else {
+		data.CommitIDs, err = getCommitIDsFromRepo(ctx, pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch)
+		if err != nil {
+			return nil, err
+		}
 	}
 
-	ops.Issue = pr.Issue
-
 	dataJSON, err := json.Marshal(data)
 	if err != nil {
 		return nil, err
 	}
 
-	ops.Content = string(dataJSON)
-
-	comment, err = issues_model.CreateComment(ctx, ops)
+	opts.Content = string(dataJSON)
+	comment, err = issues_model.CreateComment(ctx, opts)
 
 	return comment, err
 }

services/pull/pull.go

@@ -28,7 +28,6 @@ import (
 	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/globallock"
 	"code.gitea.io/gitea/modules/graceful"
-	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/log"
 	repo_module "code.gitea.io/gitea/modules/repository"
 	"code.gitea.io/gitea/modules/setting"
@@ -142,37 +141,9 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
 			return err
 		}
 
-		compareInfo, err := baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(),
-			git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName(), false, false)
-		if err != nil {
+		if _, err := CreatePushPullComment(ctx, issue.Poster, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName(), false); err != nil {
 			return err
 		}
-		// It maybe an empty pull request. Only non-empty pull request need to create push comment
-		if len(compareInfo.Commits) > 0 {
-			data := issues_model.PushActionContent{IsForcePush: false}
-			data.CommitIDs = make([]string, 0, len(compareInfo.Commits))
-			for i := len(compareInfo.Commits) - 1; i >= 0; i-- {
-				data.CommitIDs = append(data.CommitIDs, compareInfo.Commits[i].ID.String())
-			}
-
-			dataJSON, err := json.Marshal(data)
-			if err != nil {
-				return err
-			}
-
-			ops := &issues_model.CreateCommentOptions{
-				Type:        issues_model.CommentTypePullRequestPush,
-				Doer:        issue.Poster,
-				Repo:        repo,
-				Issue:       pr.Issue,
-				IsForcePush: false,
-				Content:     string(dataJSON),
-			}
-
-			if _, err = issues_model.CreateComment(ctx, ops); err != nil {
-				return err
-			}
-		}
 
 		if !pr.IsWorkInProgress(ctx) {
 			reviewNotifiers, err = issue_service.PullRequestCodeOwnersReview(ctx, pr)
@@ -335,24 +306,42 @@ func ChangeTargetBranch(ctx context.Context, pr *issues_model.PullRequest, doer
 	pr.CommitsAhead = divergence.Ahead
 	pr.CommitsBehind = divergence.Behind
 
-	if err := pr.UpdateColsIfNotMerged(ctx, "merge_base", "status", "conflicted_files", "changed_protected_files", "base_branch", "commits_ahead", "commits_behind"); err != nil {
+	// add first push codes comment
+	baseGitRepo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo)
+	if err != nil {
 		return err
 	}
+	defer baseGitRepo.Close()
 
-	// Create comment
-	options := &issues_model.CreateCommentOptions{
-		Type:   issues_model.CommentTypeChangeTargetBranch,
-		Doer:   doer,
-		Repo:   pr.Issue.Repo,
-		Issue:  pr.Issue,
-		OldRef: oldBranch,
-		NewRef: targetBranch,
-	}
-	if _, err = issues_model.CreateComment(ctx, options); err != nil {
-		return fmt.Errorf("CreateChangeTargetBranchComment: %w", err)
-	}
+	return db.WithTx(ctx, func(ctx context.Context) error {
+		if err := pr.UpdateColsIfNotMerged(ctx, "merge_base", "status", "conflicted_files", "changed_protected_files", "base_branch", "commits_ahead", "commits_behind"); err != nil {
+			return err
+		}
 
-	return nil
+		// Create comment
+		options := &issues_model.CreateCommentOptions{
+			Type:   issues_model.CommentTypeChangeTargetBranch,
+			Doer:   doer,
+			Repo:   pr.Issue.Repo,
+			Issue:  pr.Issue,
+			OldRef: oldBranch,
+			NewRef: targetBranch,
+		}
+		if _, err = issues_model.CreateComment(ctx, options); err != nil {
+			return fmt.Errorf("CreateChangeTargetBranchComment: %w", err)
+		}
+
+		// Delete all old push comments and insert new push comments
+		if _, err := db.GetEngine(ctx).Where("issue_id = ?", pr.IssueID).
+			And("type = ?", issues_model.CommentTypePullRequestPush).
+			NoAutoCondition().
+			Delete(new(issues_model.Comment)); err != nil {
+			return err
+		}
+
+		_, err = CreatePushPullComment(ctx, doer, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName(), false)
+		return err
+	})
 }
 
 func checkForInvalidation(ctx context.Context, requests issues_model.PullRequestList, repoID int64, doer *user_model.User, branch string) error {
@@ -413,7 +402,7 @@ func AddTestPullRequestTask(opts TestPullRequestOptions) {
 			}
 
 			StartPullRequestCheckImmediately(ctx, pr)
-			comment, err := CreatePushPullComment(ctx, opts.Doer, pr, opts.OldCommitID, opts.NewCommitID)
+			comment, err := CreatePushPullComment(ctx, opts.Doer, pr, opts.OldCommitID, opts.NewCommitID, opts.IsForcePush)
 			if err == nil && comment != nil {
 				notify_service.PullRequestPushCommits(ctx, opts.Doer, pr, comment)
 			}