1
1
mirror of https://github.com/go-gitea/gitea synced 2025-09-10 10:48:28 +00:00

Fix push commits comments when changing the pull request target branch (#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"
/>
This commit is contained in:
Lunny Xiao
2025-09-09 12:40:54 -07:00
committed by GitHub
parent e4cb48a7e0
commit b9efbe9fe6
3 changed files with 57 additions and 81 deletions

View File

@@ -250,7 +250,7 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to load pull issue. Error: %w", err) 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 { if err == nil && comment != nil {
notify_service.PullRequestPushCommits(ctx, pusher, pr, comment) notify_service.PullRequestPushCommits(ctx, pusher, pr, comment)
} }

View File

@@ -14,42 +14,28 @@ import (
) )
// getCommitIDsFromRepo get commit IDs from repo in between oldCommitID and newCommitID // 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 // 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) gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo)
if err != nil { if err != nil {
return nil, false, err return nil, err
} }
defer closer.Close() defer closer.Close()
oldCommit, err := gitRepo.GetCommit(oldCommitID) oldCommit, err := gitRepo.GetCommit(oldCommitID)
if err != nil { if err != nil {
return nil, false, err return nil, err
} }
newCommit, err := gitRepo.GetCommit(newCommitID) newCommit, err := gitRepo.GetCommit(newCommitID)
if err != nil { if err != nil {
return nil, false, err return nil, 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
} }
// Find commits between new and old commit excluding base branch commits // Find commits between new and old commit excluding base branch commits
commits, err := gitRepo.CommitsBetweenNotBase(newCommit, oldCommit, baseBranch) commits, err := gitRepo.CommitsBetweenNotBase(newCommit, oldCommit, baseBranch)
if err != nil { if err != nil {
return nil, false, err return nil, err
} }
commitIDs = make([]string, 0, len(commits)) commitIDs = make([]string, 0, len(commits))
@@ -57,38 +43,40 @@ func getCommitIDsFromRepo(ctx context.Context, repo *repo_model.Repository, oldC
commitIDs = append(commitIDs, commits[i].ID.String()) commitIDs = append(commitIDs, commits[i].ID.String())
} }
return commitIDs, isForcePush, err return commitIDs, err
} }
// CreatePushPullComment create push code to pull base comment // 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 == "" { if pr.HasMerged || oldCommitID == "" || newCommitID == "" {
return nil, nil return nil, nil
} }
ops := &issues_model.CreateCommentOptions{ opts := &issues_model.CreateCommentOptions{
Type: issues_model.CommentTypePullRequestPush, Type: issues_model.CommentTypePullRequestPush,
Doer: pusher, Doer: pusher,
Repo: pr.BaseRepo, Repo: pr.BaseRepo,
IsForcePush: isForcePush,
Issue: pr.Issue,
} }
var data issues_model.PushActionContent var data issues_model.PushActionContent
if opts.IsForcePush {
data.CommitIDs, data.IsForcePush, err = getCommitIDsFromRepo(ctx, pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch) data.CommitIDs = []string{oldCommitID, newCommitID}
if err != nil { } else {
return nil, err 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) dataJSON, err := json.Marshal(data)
if err != nil { if err != nil {
return nil, err return nil, err
} }
ops.Content = string(dataJSON) opts.Content = string(dataJSON)
comment, err = issues_model.CreateComment(ctx, opts)
comment, err = issues_model.CreateComment(ctx, ops)
return comment, err return comment, err
} }

View File

@@ -28,7 +28,6 @@ import (
"code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/globallock" "code.gitea.io/gitea/modules/globallock"
"code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/graceful"
"code.gitea.io/gitea/modules/json"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
repo_module "code.gitea.io/gitea/modules/repository" repo_module "code.gitea.io/gitea/modules/repository"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
@@ -141,36 +140,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
return err return err
} }
compareInfo, err := GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, if _, err := CreatePushPullComment(ctx, issue.Poster, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName(), false); err != nil {
git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName(), false, false)
if err != nil {
return err
}
if len(compareInfo.Commits) == 0 {
return nil
}
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 return err
} }
@@ -331,24 +301,42 @@ func ChangeTargetBranch(ctx context.Context, pr *issues_model.PullRequest, doer
pr.CommitsAhead = divergence.Ahead pr.CommitsAhead = divergence.Ahead
pr.CommitsBehind = divergence.Behind 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 return err
} }
defer baseGitRepo.Close()
// Create comment return db.WithTx(ctx, func(ctx context.Context) error {
options := &issues_model.CreateCommentOptions{ if err := pr.UpdateColsIfNotMerged(ctx, "merge_base", "status", "conflicted_files", "changed_protected_files", "base_branch", "commits_ahead", "commits_behind"); err != nil {
Type: issues_model.CommentTypeChangeTargetBranch, return err
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 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.GetGitHeadRefName(), false)
return err
})
} }
func checkForInvalidation(ctx context.Context, requests issues_model.PullRequestList, repoID int64, doer *user_model.User, branch string) error { func checkForInvalidation(ctx context.Context, requests issues_model.PullRequestList, repoID int64, doer *user_model.User, branch string) error {
@@ -409,7 +397,7 @@ func AddTestPullRequestTask(opts TestPullRequestOptions) {
} }
StartPullRequestCheckImmediately(ctx, pr) 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 { if err == nil && comment != nil {
notify_service.PullRequestPushCommits(ctx, opts.Doer, pr, comment) notify_service.PullRequestPushCommits(ctx, opts.Doer, pr, comment)
} }