Display conflict-free merge messages for pull requests (#15773)

Repositories using external issue tracker tend to use numeric issues in
commits. To prevent conflicts during issue reference parsing or inside
commit hooks, this change respects these configuration and uses the !
character to refer to pull requests in merge commit messages.

For repositories using squash merges, this was already handled.

Signed-off-by: JustusBunsi <61625851+justusbunsi@users.noreply.github.com>
Co-authored-by: zeripath <art27@cantab.net>

diff --git a/models/pull.go b/models/pull.go
index 133f196aaed4906b7cf28d99fee12e3c6de74c99..a1fd7c3e41865d22c8037697c65a1e1fdeb1835a 100644 (file)
--- a/models/pull.go
+++ b/models/pull.go
@@ -212,12 +212,21 @@ func (pr *PullRequest) GetDefaultMergeMessage() string {
                log.Error("Cannot load issue %d for PR id %d: Error: %v", pr.IssueID, pr.ID, err)
                return ""
        }
+       if err := pr.LoadBaseRepo(); err != nil {
+               log.Error("LoadBaseRepo: %v", err)
+               return ""
+       }
+
+       issueReference := "#"
+       if pr.BaseRepo.UnitEnabled(UnitTypeExternalTracker) {
+               issueReference = "!"
+       }
 
        if pr.BaseRepoID == pr.HeadRepoID {
-               return fmt.Sprintf("Merge pull request '%s' (#%d) from %s into %s", pr.Issue.Title, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch)
+               return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch)
        }
 
-       return fmt.Sprintf("Merge pull request '%s' (#%d) from %s:%s into %s", pr.Issue.Title, pr.Issue.Index, pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseBranch)
+       return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s:%s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseBranch)
 }
 
 // ReviewCount represents a count of Reviews

diff --git a/models/pull_test.go b/models/pull_test.go
index 3cc6abfec7e22eb378771698cc90c9eaa31629a9..5eaeb60e67281a9704c15adaba30b822dfbe7e15 100644 (file)
--- a/models/pull_test.go
+++ b/models/pull_test.go
@@ -234,3 +234,36 @@ func TestPullRequest_GetWorkInProgressPrefixWorkInProgress(t *testing.T) {
        pr.Issue.Title = "[wip] " + original
        assert.Equal(t, "[wip]", pr.GetWorkInProgressPrefix())
 }
+
+func TestPullRequest_GetDefaultMergeMessage_InternalTracker(t *testing.T) {
+       assert.NoError(t, PrepareTestDatabase())
+       pr := AssertExistsAndLoadBean(t, &PullRequest{ID: 2}).(*PullRequest)
+
+       assert.Equal(t, "Merge pull request 'issue3' (#3) from branch2 into master", pr.GetDefaultMergeMessage())
+
+       pr.BaseRepoID = 1
+       pr.HeadRepoID = 2
+       assert.Equal(t, "Merge pull request 'issue3' (#3) from user2/repo1:branch2 into master", pr.GetDefaultMergeMessage())
+}
+
+func TestPullRequest_GetDefaultMergeMessage_ExternalTracker(t *testing.T) {
+       assert.NoError(t, PrepareTestDatabase())
+
+       externalTracker := RepoUnit{
+               Type: UnitTypeExternalTracker,
+               Config: &ExternalTrackerConfig{
+                       ExternalTrackerFormat: "https://someurl.com/{user}/{repo}/{issue}",
+               },
+       }
+       baseRepo := &Repository{Name: "testRepo", ID: 1}
+       baseRepo.Owner = &User{Name: "testOwner"}
+       baseRepo.Units = []*RepoUnit{&externalTracker}
+
+       pr := AssertExistsAndLoadBean(t, &PullRequest{ID: 2, BaseRepo: baseRepo}).(*PullRequest)
+
+       assert.Equal(t, "Merge pull request 'issue3' (!3) from branch2 into master", pr.GetDefaultMergeMessage())
+
+       pr.BaseRepoID = 1
+       pr.HeadRepoID = 2
+       assert.Equal(t, "Merge pull request 'issue3' (!3) from user2/repo1:branch2 into master", pr.GetDefaultMergeMessage())
+}