diff --git a/models/fixtures/branch.yml b/models/fixtures/branch.yml index 03e21d04b4..717230149b 100644 --- a/models/fixtures/branch.yml +++ b/models/fixtures/branch.yml @@ -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 diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 1ce2047b13..72c3690b0c 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -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 diff --git a/services/pull/comment.go b/services/pull/comment.go index f12edaf032..ca156d3d92 100644 --- a/services/pull/comment.go +++ b/services/pull/comment.go @@ -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 { diff --git a/services/pull/pull.go b/services/pull/pull.go index c7b3a0d523..d374d90a62 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -244,6 +244,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 { diff --git a/tests/integration/git_helper_for_declarative_test.go b/tests/integration/git_helper_for_declarative_test.go index 91c30f7278..45c755582e 100644 --- a/tests/integration/git_helper_for_declarative_test.go +++ b/tests/integration/git_helper_for_declarative_test.go @@ -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), })) } } diff --git a/tests/integration/git_push_test.go b/tests/integration/git_push_test.go index b02387c3b9..a51bf5730f 100644 --- a/tests/integration/git_push_test.go +++ b/tests/integration/git_push_test.go @@ -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++ { diff --git a/tests/integration/pull_comment_test.go b/tests/integration/pull_comment_test.go new file mode 100644 index 0000000000..abab65247b --- /dev/null +++ b/tests/integration/pull_comment_test.go @@ -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) }) + }) +} diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index 18f7e80e82..2eb5e94cf9 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -260,14 +260,16 @@ 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) - }) + 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) }) }