mirror of
https://github.com/go-gitea/gitea.git
synced 2026-02-22 06:45:46 +01:00
Merge 2ec5564bdf46a6f9669790367548b03f60241b99 into bbea5e6c2d75a4a710d7838b7bec7e851e046d3c
This commit is contained in:
Lunny Xiao
GitHub
commit
5c83835875
@ -6,11 +6,13 @@ package pull
|
||||
import (
|
||||
"context"
|
||||
|
||||
"code.gitea.io/gitea/models/db"
|
||||
issues_model "code.gitea.io/gitea/models/issues"
|
||||
repo_model "code.gitea.io/gitea/models/repo"
|
||||
user_model "code.gitea.io/gitea/models/user"
|
||||
"code.gitea.io/gitea/modules/gitrepo"
|
||||
"code.gitea.io/gitea/modules/json"
|
||||
"code.gitea.io/gitea/modules/log"
|
||||
)
|
||||
|
||||
// getCommitIDsFromRepo get commit IDs from repo in between oldCommitID and newCommitID
|
||||
@ -53,35 +55,65 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *iss
|
||||
}
|
||||
|
||||
opts := &issues_model.CreateCommentOptions{
|
||||
Type: issues_model.CommentTypePullRequestPush,
|
||||
Doer: pusher,
|
||||
Repo: pr.BaseRepo,
|
||||
IsForcePush: isForcePush,
|
||||
Issue: pr.Issue,
|
||||
Type: issues_model.CommentTypePullRequestPush,
|
||||
Doer: pusher,
|
||||
Repo: pr.BaseRepo,
|
||||
Issue: pr.Issue,
|
||||
}
|
||||
|
||||
var data issues_model.PushActionContent
|
||||
if opts.IsForcePush {
|
||||
data.CommitIDs = []string{oldCommitID, newCommitID}
|
||||
data.IsForcePush = true
|
||||
} else {
|
||||
data.CommitIDs, err = getCommitIDsFromRepo(ctx, pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch)
|
||||
if err != nil {
|
||||
data.CommitIDs, err = getCommitIDsFromRepo(ctx, pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch)
|
||||
if err != nil {
|
||||
// For force-push events, a missing/unreachable old commit should not prevent
|
||||
// deleting stale push comments or creating the force-push timeline entry.
|
||||
if !isForcePush {
|
||||
return nil, err
|
||||
}
|
||||
// It maybe an empty pull request. Only non-empty pull request need to create push comment
|
||||
if len(data.CommitIDs) == 0 {
|
||||
return nil, nil //nolint:nilnil // return nil because no comment needs to be created
|
||||
log.Error("getCommitIDsFromRepo: %v", err)
|
||||
}
|
||||
// It maybe an empty pull request. Only non-empty pull request need to create push comment
|
||||
// for force push, we always need to delete the old push comment so don't return here.
|
||||
if len(data.CommitIDs) == 0 && !isForcePush {
|
||||
return nil, nil //nolint:nilnil // return nil because no comment needs to be created
|
||||
}
|
||||
|
||||
return db.WithTx2(ctx, func(ctx context.Context) (*issues_model.Comment, error) {
|
||||
if isForcePush {
|
||||
if _, err := db.GetEngine(ctx).Where("issue_id = ?", pr.IssueID).
|
||||
And("type = ?", issues_model.CommentTypePullRequestPush).
|
||||
NoAutoCondition().
|
||||
Delete(new(issues_model.Comment)); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
dataJSON, err := json.Marshal(data)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if len(data.CommitIDs) > 0 {
|
||||
dataJSON, err := json.Marshal(data)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
opts.Content = string(dataJSON)
|
||||
comment, err = issues_model.CreateComment(ctx, opts)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
opts.Content = string(dataJSON)
|
||||
comment, err = issues_model.CreateComment(ctx, opts)
|
||||
if isForcePush { // if it's a force push, we need to add a force push comment
|
||||
data.CommitIDs = []string{oldCommitID, newCommitID}
|
||||
data.IsForcePush = true
|
||||
dataJSON, err := json.Marshal(data)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
opts.Content = string(dataJSON)
|
||||
opts.IsForcePush = true // FIXME: it seems the field is unnecessary any more because PushActionContent includes IsForcePush field
|
||||
comment, err = issues_model.CreateComment(ctx, opts)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
return comment, err
|
||||
return comment, err
|
||||
})
|
||||
}
|
||||
|
||||
173
services/pull/comment_test.go
Normal file
173
services/pull/comment_test.go
Normal file
@ -0,0 +1,173 @@
|
||||
// Copyright 2025 The Gitea Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
package pull
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
issues_model "code.gitea.io/gitea/models/issues"
|
||||
"code.gitea.io/gitea/models/unittest"
|
||||
user_model "code.gitea.io/gitea/models/user"
|
||||
"code.gitea.io/gitea/modules/gitrepo"
|
||||
"code.gitea.io/gitea/modules/json"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestCreatePushPullCommentForcePushDeletesOldComments(t *testing.T) {
|
||||
t.Run("base-branch-only", func(t *testing.T) {
|
||||
assert.NoError(t, unittest.PrepareTestDatabase())
|
||||
|
||||
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
|
||||
assert.NoError(t, pr.LoadIssue(t.Context()))
|
||||
assert.NoError(t, pr.LoadBaseRepo(t.Context()))
|
||||
|
||||
pusher := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
|
||||
|
||||
_, err := issues_model.CreateComment(t.Context(), &issues_model.CreateCommentOptions{
|
||||
Type: issues_model.CommentTypePullRequestPush,
|
||||
Doer: pusher,
|
||||
Repo: pr.BaseRepo,
|
||||
Issue: pr.Issue,
|
||||
Content: "{}",
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
_, err = issues_model.CreateComment(t.Context(), &issues_model.CreateCommentOptions{
|
||||
Type: issues_model.CommentTypePullRequestPush,
|
||||
Doer: pusher,
|
||||
Repo: pr.BaseRepo,
|
||||
Issue: pr.Issue,
|
||||
Content: "{}",
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
|
||||
comments, err := issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{
|
||||
IssueID: pr.IssueID,
|
||||
Type: issues_model.CommentTypePullRequestPush,
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
assert.Len(t, comments, 2)
|
||||
|
||||
gitRepo, err := gitrepo.OpenRepository(t.Context(), pr.BaseRepo)
|
||||
assert.NoError(t, err)
|
||||
defer gitRepo.Close()
|
||||
|
||||
headCommit, err := gitRepo.GetBranchCommit(pr.BaseBranch)
|
||||
assert.NoError(t, err)
|
||||
oldCommit := headCommit
|
||||
if headCommit.ParentCount() > 0 {
|
||||
parentCommit, err := headCommit.Parent(0)
|
||||
assert.NoError(t, err)
|
||||
oldCommit = parentCommit
|
||||
}
|
||||
|
||||
comment, err := CreatePushPullComment(t.Context(), pusher, pr, oldCommit.ID.String(), headCommit.ID.String(), true)
|
||||
assert.NoError(t, err)
|
||||
assert.NotNil(t, comment)
|
||||
var createdData issues_model.PushActionContent
|
||||
assert.NoError(t, json.Unmarshal([]byte(comment.Content), &createdData))
|
||||
assert.True(t, createdData.IsForcePush)
|
||||
|
||||
// When both commits are on the base branch, CommitsBetweenNotBase should
|
||||
// typically return no commits, so only the force-push comment is expected.
|
||||
commits, err := gitRepo.CommitsBetweenNotBase(headCommit, oldCommit, pr.BaseBranch)
|
||||
assert.NoError(t, err)
|
||||
assert.Empty(t, commits)
|
||||
|
||||
comments, err = issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{
|
||||
IssueID: pr.IssueID,
|
||||
Type: issues_model.CommentTypePullRequestPush,
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
assert.Len(t, comments, 1)
|
||||
|
||||
forcePushCount := 0
|
||||
for _, comment := range comments {
|
||||
var pushData issues_model.PushActionContent
|
||||
assert.NoError(t, json.Unmarshal([]byte(comment.Content), &pushData))
|
||||
if pushData.IsForcePush {
|
||||
forcePushCount++
|
||||
}
|
||||
}
|
||||
assert.Equal(t, 1, forcePushCount)
|
||||
})
|
||||
|
||||
t.Run("head-vs-base-branch", func(t *testing.T) {
|
||||
assert.NoError(t, unittest.PrepareTestDatabase())
|
||||
|
||||
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
|
||||
assert.NoError(t, pr.LoadIssue(t.Context()))
|
||||
assert.NoError(t, pr.LoadBaseRepo(t.Context()))
|
||||
|
||||
pusher := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
|
||||
|
||||
_, err := issues_model.CreateComment(t.Context(), &issues_model.CreateCommentOptions{
|
||||
Type: issues_model.CommentTypePullRequestPush,
|
||||
Doer: pusher,
|
||||
Repo: pr.BaseRepo,
|
||||
Issue: pr.Issue,
|
||||
Content: "{}",
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
_, err = issues_model.CreateComment(t.Context(), &issues_model.CreateCommentOptions{
|
||||
Type: issues_model.CommentTypePullRequestPush,
|
||||
Doer: pusher,
|
||||
Repo: pr.BaseRepo,
|
||||
Issue: pr.Issue,
|
||||
Content: "{}",
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
|
||||
comments, err := issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{
|
||||
IssueID: pr.IssueID,
|
||||
Type: issues_model.CommentTypePullRequestPush,
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
assert.Len(t, comments, 2)
|
||||
|
||||
gitRepo, err := gitrepo.OpenRepository(t.Context(), pr.BaseRepo)
|
||||
assert.NoError(t, err)
|
||||
defer gitRepo.Close()
|
||||
|
||||
// In this subtest, use the head branch for the new commit and the base branch
|
||||
// for the old commit so that CommitsBetweenNotBase returns non-empty results.
|
||||
headCommit, err := gitRepo.GetBranchCommit(pr.HeadBranch)
|
||||
assert.NoError(t, err)
|
||||
|
||||
baseCommit, err := gitRepo.GetBranchCommit(pr.BaseBranch)
|
||||
assert.NoError(t, err)
|
||||
oldCommit := baseCommit
|
||||
|
||||
comment, err := CreatePushPullComment(t.Context(), pusher, pr, oldCommit.ID.String(), headCommit.ID.String(), true)
|
||||
assert.NoError(t, err)
|
||||
assert.NotNil(t, comment)
|
||||
var createdData issues_model.PushActionContent
|
||||
assert.NoError(t, json.Unmarshal([]byte(comment.Content), &createdData))
|
||||
assert.True(t, createdData.IsForcePush)
|
||||
|
||||
commits, err := gitRepo.CommitsBetweenNotBase(headCommit, oldCommit, pr.BaseBranch)
|
||||
assert.NoError(t, err)
|
||||
// For this scenario we expect at least one commit between head and base
|
||||
// that is not on the base branch, so data.CommitIDs should be non-empty.
|
||||
assert.NotEmpty(t, commits)
|
||||
|
||||
comments, err = issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{
|
||||
IssueID: pr.IssueID,
|
||||
Type: issues_model.CommentTypePullRequestPush,
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
// Two comments should exist now: one regular push comment and one force-push comment.
|
||||
assert.Len(t, comments, 2)
|
||||
|
||||
forcePushCount := 0
|
||||
for _, comment := range comments {
|
||||
var pushData issues_model.PushActionContent
|
||||
assert.NoError(t, json.Unmarshal([]byte(comment.Content), &pushData))
|
||||
if pushData.IsForcePush {
|
||||
forcePushCount++
|
||||
}
|
||||
}
|
||||
assert.Equal(t, 1, forcePushCount)
|
||||
})
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user