Nemcio/Gitea
1
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2025-10-31 19:06:18 +01:00
Code Issues Packages Projects Releases Wiki Activity

Add rebase push display wrong comments bug (#35560)

Browse Source 
Fix #35518

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Lunny Xiao
2025-10-03 15:16:17 -07:00
committed by GitHub
parent 71360a94cb
commit 17c8aa6587
8 changed files with 177 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
models/fixtures/branch.yml
View File

@@ -213,3 +213,15 @@
is_deleted: false
deleted_by_id: 0
deleted_unix: 0
-
id: 26
repo_id: 10
name: 'feature/1'
commit_id: '65f1bf27bc3bf70f64657658635e66094edbcb4d'
commit_message: 'Initial commit'
commit_time: 1489950479
pusher_id: 2
is_deleted: false
deleted_by_id: 0
deleted_unix: 0

19
routers/web/repo/pull.go
View File

@@ -1583,7 +1583,16 @@ func UpdatePullRequestTarget(ctx *context.Context) {
}
if err := pull_service.ChangeTargetBranch(ctx, pr, ctx.Doer, targetBranch); err != nil {
if issues_model.IsErrPullRequestAlreadyExists(err) {
switch {
case git_model.IsErrBranchNotExist(err):
errorMessage := ctx.Tr("form.target_branch_not_exist")
ctx.Flash.Error(errorMessage)
ctx.JSON(http.StatusBadRequest, map[string]any{
"error": err.Error(),
"user_error": errorMessage,
})
case issues_model.IsErrPullRequestAlreadyExists(err):
err := err.(issues_model.ErrPullRequestAlreadyExists)
RepoRelPath := ctx.Repo.Owner.Name + "/" + ctx.Repo.Repository.Name
@@ -1594,7 +1603,7 @@ func UpdatePullRequestTarget(ctx *context.Context) {
"error": err.Error(),
"user_error": errorMessage,
})
} else if issues_model.IsErrIssueIsClosed(err) {
case issues_model.IsErrIssueIsClosed(err):
errorMessage := ctx.Tr("repo.pulls.is_closed")
ctx.Flash.Error(errorMessage)
@@ -1602,7 +1611,7 @@ func UpdatePullRequestTarget(ctx *context.Context) {
"error": err.Error(),
"user_error": errorMessage,
})
} else if pull_service.IsErrPullRequestHasMerged(err) {
case pull_service.IsErrPullRequestHasMerged(err):
errorMessage := ctx.Tr("repo.pulls.has_merged")
ctx.Flash.Error(errorMessage)
@@ -1610,7 +1619,7 @@ func UpdatePullRequestTarget(ctx *context.Context) {
"error": err.Error(),
"user_error": errorMessage,
})
} else if git_model.IsErrBranchesEqual(err) {
case git_model.IsErrBranchesEqual(err):
errorMessage := ctx.Tr("repo.pulls.nothing_to_compare")
ctx.Flash.Error(errorMessage)
@@ -1618,7 +1627,7 @@ func UpdatePullRequestTarget(ctx *context.Context) {
"error": err.Error(),
"user_error": errorMessage,
})
} else {
default:
ctx.ServerError("UpdatePullRequestTarget", err)
}
return

1
services/pull/comment.go
View File

@@ -63,6 +63,7 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *iss
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 {

11
services/pull/pull.go
View File

@@ -243,6 +243,17 @@ func ChangeTargetBranch(ctx context.Context, pr *issues_model.PullRequest, doer
}
}
exist, err := git_model.IsBranchExist(ctx, pr.BaseRepoID, targetBranch)
if err != nil {
return err
}
if !exist {
return git_model.ErrBranchNotExist{
RepoID: pr.BaseRepoID,
BranchName: targetBranch,
}
}
// Check if branches are equal
branchesEqual, err := IsHeadEqualWithBranch(ctx, pr, targetBranch)
if err != nil {

22
tests/integration/git_helper_for_declarative_test.go
View File

@@ -23,6 +23,7 @@ import (
"code.gitea.io/gitea/tests"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func withKeyFile(t *testing.T, keyname string, callback func(string)) {
@@ -160,20 +161,27 @@ func doGitPushTestRepositoryFail(dstPath string, args ...string) func(*testing.T
}
}
func doGitAddSomeCommits(dstPath, branch string) func(*testing.T) {
return func(t *testing.T) {
doGitCheckoutBranch(dstPath, branch)(t)
type localGitAddCommitOptions struct {
LocalRepoPath string
CheckoutBranch string
TreeFilePath string
TreeFileContent string
}
assert.NoError(t, os.WriteFile(filepath.Join(dstPath, fmt.Sprintf("file-%s.txt", branch)), []byte("file "+branch), 0o644))
assert.NoError(t, git.AddChanges(t.Context(), dstPath, true))
func doGitCheckoutWriteFileCommit(opts localGitAddCommitOptions) func(*testing.T) {
return func(t *testing.T) {
doGitCheckoutBranch(opts.LocalRepoPath, opts.CheckoutBranch)(t)
localFilePath := filepath.Join(opts.LocalRepoPath, opts.TreeFilePath)
require.NoError(t, os.WriteFile(localFilePath, []byte(opts.TreeFileContent), 0o644))
require.NoError(t, git.AddChanges(t.Context(), opts.LocalRepoPath, true))
signature := git.Signature{
Email: "test@test.test",
Name: "test",
}
assert.NoError(t, git.CommitChanges(t.Context(), dstPath, git.CommitChangesOptions{
require.NoError(t, git.CommitChanges(t.Context(), opts.LocalRepoPath, git.CommitChangesOptions{
Committer: &signature,
Author: &signature,
Message: "update " + branch,
Message: fmt.Sprintf("update %s @ %s", opts.TreeFilePath, opts.CheckoutBranch),
}))
}
}

8
tests/integration/git_push_test.go
View File

@@ -58,8 +58,12 @@ func testGitPush(t *testing.T, u *url.URL) {
for i := range 5 {
branchName := fmt.Sprintf("branch-%d", i)
pushed = append(pushed, branchName)
doGitAddSomeCommits(gitPath, branchName)(t)
doGitCheckoutWriteFileCommit(localGitAddCommitOptions{
LocalRepoPath: gitPath,
CheckoutBranch: branchName,
TreeFilePath: fmt.Sprintf("file-%s.txt", branchName),
TreeFileContent: "file " + branchName,
})(t)
}
for i := 5; i < 10; i++ {

109
tests/integration/pull_comment_test.go Normal file
View File

@@ -0,0 +1,109 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package integration
import (
"fmt"
"net/http"
"net/url"
"os"
"testing"
"time"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/unittest"
issues_service "code.gitea.io/gitea/services/issue"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func testWaitForPullRequestStatus(t *testing.T, prIssue *issues_model.Issue, expectedStatus issues_model.PullRequestStatus) (retIssue *issues_model.Issue) {
require.Eventually(t, func() bool {
prIssueCond := *prIssue
retIssue = unittest.AssertExistsAndLoadBean(t, &prIssueCond)
require.NoError(t, retIssue.LoadPullRequest(t.Context()))
return retIssue.PullRequest.Status == expectedStatus
}, 5*time.Second, 20*time.Millisecond)
return retIssue
}
func testPullCommentRebase(t *testing.T, u *url.URL, session *TestSession) {
testPRTitle := "Test PR for rebase comment"
// make a change on forked branch
testEditFile(t, session, "user1", "repo1", "test-branch/rebase", "README.md", "Hello, World (Edited)\n")
testPullCreate(t, session, "user1", "repo1", false, "test-branch/rebase", "test-branch/rebase", testPRTitle)
// create a conflict on base repo branch
testEditFile(t, session, "user2", "repo1", "test-branch/rebase", "README.md", "Hello, World (Edited Conflicted)\n")
// Now the pull request status should be conflicted
testWaitForPullRequestStatus(t, &issues_model.Issue{Title: testPRTitle}, issues_model.PullRequestStatusConflict)
dstPath := t.TempDir()
u.Path = "/user2/repo1.git"
doGitClone(dstPath, u)(t)
doGitCheckoutBranch(dstPath, "test-branch/rebase")(t)
doGitCreateBranch(dstPath, "local-branch/rebase")(t)
content, _ := os.ReadFile(dstPath + "/README.md")
require.Equal(t, "Hello, World (Edited Conflicted)\n", string(content))
doGitCheckoutWriteFileCommit(localGitAddCommitOptions{
LocalRepoPath: dstPath,
CheckoutBranch: "local-branch/rebase",
TreeFilePath: "README.md",
TreeFileContent: "Hello, World (Edited Conflict Resolved)\n",
})(t)
// do force push
u.Path = "/user1/repo1.git"
u.User = url.UserPassword("user1", userPassword)
doGitAddRemote(dstPath, "base-repo", u)(t)
doGitPushTestRepositoryFail(dstPath, "base-repo", "local-branch/rebase:test-branch/rebase")(t)
doGitPushTestRepository(dstPath, "--force", "base-repo", "local-branch/rebase:test-branch/rebase")(t)
// reload the pr
prIssue := testWaitForPullRequestStatus(t, &issues_model.Issue{Title: testPRTitle}, issues_model.PullRequestStatusMergeable)
comments, err := issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{
IssueID: prIssue.ID,
Type: issues_model.CommentTypeUndefined, // get all comments type
})
require.NoError(t, err)
lastComment := comments[len(comments)-1]
assert.NoError(t, issues_service.LoadCommentPushCommits(t.Context(), lastComment))
assert.True(t, lastComment.IsForcePush)
}
func testPullCommentRetarget(t *testing.T, u *url.URL, session *TestSession) {
testPRTitle := "Test PR for retarget comment"
// keep a non-conflict branch
testCreateBranch(t, session, "user2", "repo1", "branch/test-branch/retarget", "test-branch/retarget-no-conflict", http.StatusSeeOther)
// make a change on forked branch
testEditFile(t, session, "user1", "repo1", "test-branch/retarget", "README.md", "Hello, World (Edited)\n")
testPullCreate(t, session, "user1", "repo1", false, "test-branch/retarget", "test-branch/retarget", testPRTitle)
// create a conflict line on user2/repo1 README.md
testEditFile(t, session, "user2", "repo1", "test-branch/retarget", "README.md", "Hello, World (Edited Conflicted)\n")
// Now the pull request status should be conflicted
prIssue := testWaitForPullRequestStatus(t, &issues_model.Issue{Title: testPRTitle}, issues_model.PullRequestStatusConflict)
// do retarget
req := NewRequestWithValues(t, "POST", fmt.Sprintf("/user2/repo1/pull/%d/target_branch", prIssue.PullRequest.Index), map[string]string{
"_csrf": GetUserCSRFToken(t, session),
"target_branch": "test-branch/retarget-no-conflict",
})
session.MakeRequest(t, req, http.StatusOK)
testWaitForPullRequestStatus(t, &issues_model.Issue{Title: testPRTitle}, issues_model.PullRequestStatusMergeable)
}
func TestPullComment(t *testing.T) {
onGiteaRun(t, func(t *testing.T, u *url.URL) {
session := loginUser(t, "user1")
testCreateBranch(t, session, "user2", "repo1", "branch/master", "test-branch/rebase", http.StatusSeeOther)
testCreateBranch(t, session, "user2", "repo1", "branch/master", "test-branch/retarget", http.StatusSeeOther)
testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "")
t.Run("RebaseComment", func(t *testing.T) { testPullCommentRebase(t, u, session) })
t.Run("RetargetComment", func(t *testing.T) { testPullCommentRetarget(t, u, session) })
})
}

12
tests/integration/pull_create_test.go
View File

@@ -260,15 +260,17 @@ func TestCreateAgitPullWithReadPermission(t *testing.T) {
u.Path = "user2/repo1.git"
u.User = url.UserPassword("user4", userPassword)
t.Run("Clone", doGitClone(dstPath, u))
doGitClone(dstPath, u)(t)
doGitCheckoutWriteFileCommit(localGitAddCommitOptions{
LocalRepoPath: dstPath,
CheckoutBranch: "master",
TreeFilePath: "new-file-for-agit.txt",
TreeFileContent: "temp content",
})(t)
t.Run("add commit", doGitAddSomeCommits(dstPath, "master"))
t.Run("do agit pull create", func(t *testing.T) {
err := gitcmd.NewCommand("push", "origin", "HEAD:refs/for/master", "-o").AddDynamicArguments("topic="+"test-topic").Run(t.Context(), &gitcmd.RunOpts{Dir: dstPath})
assert.NoError(t, err)
})
})
}
/*