Merge pull request 'agit: Automatically fill in the description' (#2344) from algernon/f/agit/description-autofill into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2344
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Otto <otto@codeberg.org>

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)

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"))
 	}