From 76d6184cd0f09f51aa2534092b553e47ee703ff3 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sun, 9 May 2021 03:32:48 +0100 Subject: [PATCH] Display conflict-free merge messages for pull requests (#15773) (#15796) Backport #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 Co-authored-by: Steven <61625851+justusbunsi@users.noreply.github.com> --- models/pull.go | 13 +++++++++++-- models/pull_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/models/pull.go b/models/pull.go index 133f196aa..a1fd7c3e4 100644 --- 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 3cc6abfec..5eaeb60e6 100644 --- 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()) +}