Skip to content

Commit b9efbe9

Browse files
lunnylunny
authored
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" />
1 parent e4cb48a commit b9efbe9

File tree

3 files changed

+57
-81
lines changed

3 files changed

+57
-81
lines changed

‎services/agit/agit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
250250
if err != nil {
251251
return nil, fmt.Errorf("failed to load pull issue. Error: %w", err)
252252
}
253-
comment, err := pull_service.CreatePushPullComment(ctx, pusher, pr, oldCommitID, opts.NewCommitIDs[i])
253+
comment, err := pull_service.CreatePushPullComment(ctx, pusher, pr, oldCommitID, opts.NewCommitIDs[i], forcePush.Value())
254254
if err == nil && comment != nil {
255255
notify_service.PullRequestPushCommits(ctx, pusher, pr, comment)
256256
}

‎services/pull/comment.go

Lines changed: 22 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -14,81 +14,69 @@ import (
1414
)
1515

1616
// getCommitIDsFromRepo get commit IDs from repo in between oldCommitID and newCommitID
17-
// isForcePush will be true if oldCommit isn't on the branch
1817
// Commit on baseBranch will skip
19-
func getCommitIDsFromRepo(ctx context.Context, repo *repo_model.Repository, oldCommitID, newCommitID, baseBranch string) (commitIDs []string, isForcePush bool, err error) {
18+
func getCommitIDsFromRepo(ctx context.Context, repo *repo_model.Repository, oldCommitID, newCommitID, baseBranch string) (commitIDs []string, err error) {
2019
gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo)
2120
if err != nil {
22-
return nil, false, err
21+
return nil, err
2322
}
2423
defer closer.Close()
2524

2625
oldCommit, err := gitRepo.GetCommit(oldCommitID)
2726
if err != nil {
28-
return nil, false, err
27+
return nil, err
2928
}
3029

3130
newCommit, err := gitRepo.GetCommit(newCommitID)
3231
if err != nil {
33-
return nil, false, err
34-
}
35-
36-
isForcePush, err = newCommit.IsForcePush(oldCommitID)
37-
if err != nil {
38-
return nil, false, err
39-
}
40-
41-
if isForcePush {
42-
commitIDs = make([]string, 2)
43-
commitIDs[0] = oldCommitID
44-
commitIDs[1] = newCommitID
45-
46-
return commitIDs, isForcePush, err
32+
return nil, err
4733
}
4834

4935
// Find commits between new and old commit excluding base branch commits
5036
commits, err := gitRepo.CommitsBetweenNotBase(newCommit, oldCommit, baseBranch)
5137
if err != nil {
52-
return nil, false, err
38+
return nil, err
5339
}
5440

5541
commitIDs = make([]string, 0, len(commits))
5642
for i := len(commits) - 1; i >= 0; i-- {
5743
commitIDs = append(commitIDs, commits[i].ID.String())
5844
}
5945

60-
return commitIDs, isForcePush, err
46+
return commitIDs, err
6147
}
6248

6349
// CreatePushPullComment create push code to pull base comment
64-
func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *issues_model.PullRequest, oldCommitID, newCommitID string) (comment *issues_model.Comment, err error) {
50+
func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *issues_model.PullRequest, oldCommitID, newCommitID string, isForcePush bool) (comment *issues_model.Comment, err error) {
6551
if pr.HasMerged || oldCommitID == "" || newCommitID == "" {
6652
return nil, nil
6753
}
6854

69-
ops := &issues_model.CreateCommentOptions{
70-
Type: issues_model.CommentTypePullRequestPush,
71-
Doer: pusher,
72-
Repo: pr.BaseRepo,
55+
opts := &issues_model.CreateCommentOptions{
56+
Type: issues_model.CommentTypePullRequestPush,
57+
Doer: pusher,
58+
Repo: pr.BaseRepo,
59+
IsForcePush: isForcePush,
60+
Issue: pr.Issue,
7361
}
7462

7563
var data issues_model.PushActionContent
76-
77-
data.CommitIDs, data.IsForcePush, err = getCommitIDsFromRepo(ctx, pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch)
78-
if err != nil {
79-
return nil, err
64+
if opts.IsForcePush {
65+
data.CommitIDs = []string{oldCommitID, newCommitID}
66+
} else {
67+
data.CommitIDs, err = getCommitIDsFromRepo(ctx, pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch)
68+
if err != nil {
69+
return nil, err
70+
}
8071
}
8172

82-
ops.Issue = pr.Issue
83-
8473
dataJSON, err := json.Marshal(data)
8574
if err != nil {
8675
return nil, err
8776
}
8877

89-
ops.Content = string(dataJSON)
90-
91-
comment, err = issues_model.CreateComment(ctx, ops)
78+
opts.Content = string(dataJSON)
79+
comment, err = issues_model.CreateComment(ctx, opts)
9280

9381
return comment, err
9482
}

‎services/pull/pull.go

Lines changed: 34 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828
"code.gitea.io/gitea/modules/gitrepo"
2929
"code.gitea.io/gitea/modules/globallock"
3030
"code.gitea.io/gitea/modules/graceful"
31-
"code.gitea.io/gitea/modules/json"
3231
"code.gitea.io/gitea/modules/log"
3332
repo_module "code.gitea.io/gitea/modules/repository"
3433
"code.gitea.io/gitea/modules/setting"
@@ -141,36 +140,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
141140
return err
142141
}
143142

144-
compareInfo, err := GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo,
145-
git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName(), false, false)
146-
if err != nil {
147-
return err
148-
}
149-
if len(compareInfo.Commits) == 0 {
150-
return nil
151-
}
152-
153-
data := issues_model.PushActionContent{IsForcePush: false}
154-
data.CommitIDs = make([]string, 0, len(compareInfo.Commits))
155-
for i := len(compareInfo.Commits) - 1; i >= 0; i-- {
156-
data.CommitIDs = append(data.CommitIDs, compareInfo.Commits[i].ID.String())
157-
}
158-
159-
dataJSON, err := json.Marshal(data)
160-
if err != nil {
161-
return err
162-
}
163-
164-
ops := &issues_model.CreateCommentOptions{
165-
Type: issues_model.CommentTypePullRequestPush,
166-
Doer: issue.Poster,
167-
Repo: repo,
168-
Issue: pr.Issue,
169-
IsForcePush: false,
170-
Content: string(dataJSON),
171-
}
172-
173-
if _, err = issues_model.CreateComment(ctx, ops); err != nil {
143+
if _, err := CreatePushPullComment(ctx, issue.Poster, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName(), false); err != nil {
174144
return err
175145
}
176146

@@ -331,24 +301,42 @@ func ChangeTargetBranch(ctx context.Context, pr *issues_model.PullRequest, doer
331301
pr.CommitsAhead = divergence.Ahead
332302
pr.CommitsBehind = divergence.Behind
333303

334-
if err := pr.UpdateColsIfNotMerged(ctx, "merge_base", "status", "conflicted_files", "changed_protected_files", "base_branch", "commits_ahead", "commits_behind"); err != nil {
304+
// add first push codes comment
305+
baseGitRepo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo)
306+
if err != nil {
335307
return err
336308
}
309+
defer baseGitRepo.Close()
337310

338-
// Create comment
339-
options := &issues_model.CreateCommentOptions{
340-
Type: issues_model.CommentTypeChangeTargetBranch,
341-
Doer: doer,
342-
Repo: pr.Issue.Repo,
343-
Issue: pr.Issue,
344-
OldRef: oldBranch,
345-
NewRef: targetBranch,
346-
}
347-
if _, err = issues_model.CreateComment(ctx, options); err != nil {
348-
return fmt.Errorf("CreateChangeTargetBranchComment: %w", err)
349-
}
311+
return db.WithTx(ctx, func(ctx context.Context) error {
312+
if err := pr.UpdateColsIfNotMerged(ctx, "merge_base", "status", "conflicted_files", "changed_protected_files", "base_branch", "commits_ahead", "commits_behind"); err != nil {
313+
return err
314+
}
350315

351-
return nil
316+
// Create comment
317+
options := &issues_model.CreateCommentOptions{
318+
Type: issues_model.CommentTypeChangeTargetBranch,
319+
Doer: doer,
320+
Repo: pr.Issue.Repo,
321+
Issue: pr.Issue,
322+
OldRef: oldBranch,
323+
NewRef: targetBranch,
324+
}
325+
if _, err = issues_model.CreateComment(ctx, options); err != nil {
326+
return fmt.Errorf("CreateChangeTargetBranchComment: %w", err)
327+
}
328+
329+
// Delete all old push comments and insert new push comments
330+
if _, err := db.GetEngine(ctx).Where("issue_id = ?", pr.IssueID).
331+
And("type = ?", issues_model.CommentTypePullRequestPush).
332+
NoAutoCondition().
333+
Delete(new(issues_model.Comment)); err != nil {
334+
return err
335+
}
336+
337+
_, err = CreatePushPullComment(ctx, doer, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName(), false)
338+
return err
339+
})
352340
}
353341

354342
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) {
409397
}
410398

411399
StartPullRequestCheckImmediately(ctx, pr)
412-
comment, err := CreatePushPullComment(ctx, opts.Doer, pr, opts.OldCommitID, opts.NewCommitID)
400+
comment, err := CreatePushPullComment(ctx, opts.Doer, pr, opts.OldCommitID, opts.NewCommitID, opts.IsForcePush)
413401
if err == nil && comment != nil {
414402
notify_service.PullRequestPushCommits(ctx, opts.Doer, pr, comment)
415403
}

0 commit comments

Comments
 (0)