From 259ad5d61493590732cc9fcc16158fc00ebdcdae Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Wed, 14 Feb 2024 00:35:38 +0100 Subject: [PATCH] agit: Automatically fill in the description If no `-o description=` is provided, fill it in automatically from the first commit, just like title. Also allow filling in either, and specifying them independently. This means that `git push origin HEAD:refs/for/main/my-local-branch` will fill in the PR title, *and* the description, without having to specify additional parameters. The description is the first commit's message without the first two lines (the title and a newline, as customary). Signed-off-by: Gergely Nagy --- services/agit/agit.go | 28 +++++++------ tests/integration/git_test.go | 74 ++++++++++++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 12 deletions(-) diff --git a/services/agit/agit.go b/services/agit/agit.go index bc68372570..98367ab973 100644 --- a/services/agit/agit.go +++ b/services/agit/agit.go @@ -38,6 +38,9 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. _, forcePush = opts.GitPushOptions["force-push"] objectFormat, _ := gitRepo.GetObjectFormat() + title, hasTitle := opts.GitPushOptions["title"] + description, hasDesc := opts.GitPushOptions["description"] + for i := range opts.OldCommitIDs { if opts.NewCommitIDs[i] == objectFormat.EmptyObjectID().String() { results = append(results, private.HookProcReceiveRefResult{ @@ -102,18 +105,21 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. return nil, fmt.Errorf("Failed to get unmerged agit flow pull request in repository: %s/%s Error: %w", ownerName, repoName, err) } - // create a new pull request - if len(title) == 0 { - var has bool - title, has = opts.GitPushOptions["title"] - if !has || len(title) == 0 { - commit, err := gitRepo.GetCommit(opts.NewCommitIDs[i]) - if err != nil { - return nil, fmt.Errorf("Failed to get commit %s in repository: %s/%s Error: %w", opts.NewCommitIDs[i], ownerName, repoName, err) - } - title = strings.Split(commit.CommitMessage, "\n")[0] + // automatically fill out the title and the description from the first commit. + shouldGetCommit := len(title) == 0 || len(description) == 0 + + var commit *git.Commit + if shouldGetCommit { + commit, err = gitRepo.GetCommit(opts.NewCommitIDs[i]) + if err != nil { + return nil, fmt.Errorf("Failed to get commit %s in repository: %s/%s Error: %w", opts.NewCommitIDs[i], ownerName, repoName, err) } - description = opts.GitPushOptions["description"] + } + if !hasTitle || len(title) == 0 { + title = strings.Split(commit.CommitMessage, "\n")[0] + } + if !hasDesc || len(description) == 0 { + _, description, _ = strings.Cut(commit.CommitMessage, "\n\n") } pusher, err := user_model.GetUserByID(ctx, opts.UserID) diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index 0afe9fa580..3de83f2151 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -31,6 +31,7 @@ import ( "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const ( @@ -832,7 +833,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, baseBranch, headB Name: "user2", When: time.Now(), }, - Message: "Testing commit 2", + Message: "Testing commit 2\n\nLonger description.", }) assert.NoError(t, err) commit, err = gitRepo.GetRefCommitID("HEAD") @@ -864,6 +865,77 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, baseBranch, headB assert.False(t, prMsg.HasMerged) assert.Equal(t, commit, prMsg.Head.Sha) }) + t.Run("PushParams", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + t.Run("NoParams", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + _, _, gitErr := git.NewCommand(git.DefaultContext, "push", "origin").AddDynamicArguments("HEAD:refs/for/master/" + headBranch + "-implicit").RunStdString(&git.RunOpts{Dir: dstPath}) + assert.NoError(t, gitErr) + + unittest.AssertCount(t, &issues_model.PullRequest{}, pullNum+3) + pr3 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ + HeadRepoID: repo.ID, + Flow: issues_model.PullRequestFlowAGit, + Index: pr1.Index + 2, + }) + assert.NotEmpty(t, pr3) + err := pr3.LoadIssue(db.DefaultContext) + assert.NoError(t, err) + + _, err2 := doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr3.Index)(t) + require.NoError(t, err2) + + assert.Equal(t, "Testing commit 2", pr3.Issue.Title) + assert.Contains(t, pr3.Issue.Content, "Longer description.") + }) + t.Run("TitleOverride", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + _, _, gitErr := git.NewCommand(git.DefaultContext, "push", "origin", "-o", "title=my-shiny-title").AddDynamicArguments("HEAD:refs/for/master/" + headBranch + "-implicit-2").RunStdString(&git.RunOpts{Dir: dstPath}) + assert.NoError(t, gitErr) + + unittest.AssertCount(t, &issues_model.PullRequest{}, pullNum+4) + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ + HeadRepoID: repo.ID, + Flow: issues_model.PullRequestFlowAGit, + Index: pr1.Index + 3, + }) + assert.NotEmpty(t, pr) + err := pr.LoadIssue(db.DefaultContext) + assert.NoError(t, err) + + _, err = doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr.Index)(t) + require.NoError(t, err) + + assert.Equal(t, "my-shiny-title", pr.Issue.Title) + assert.Contains(t, pr.Issue.Content, "Longer description.") + }) + + t.Run("DescriptionOverride", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + _, _, gitErr := git.NewCommand(git.DefaultContext, "push", "origin", "-o", "description=custom").AddDynamicArguments("HEAD:refs/for/master/" + headBranch + "-implicit-3").RunStdString(&git.RunOpts{Dir: dstPath}) + assert.NoError(t, gitErr) + + unittest.AssertCount(t, &issues_model.PullRequest{}, pullNum+5) + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ + HeadRepoID: repo.ID, + Flow: issues_model.PullRequestFlowAGit, + Index: pr1.Index + 4, + }) + assert.NotEmpty(t, pr) + err := pr.LoadIssue(db.DefaultContext) + assert.NoError(t, err) + + _, err = doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr.Index)(t) + require.NoError(t, err) + + assert.Equal(t, "Testing commit 2", pr.Issue.Title) + assert.Contains(t, pr.Issue.Content, "custom") + }) + }) t.Run("Merge", doAPIMergePullRequest(*ctx, ctx.Username, ctx.Reponame, pr1.Index)) t.Run("CheckoutMasterAgain", doGitCheckoutBranch(dstPath, "master")) }