Disable merging a WIP Pull request (#4529)

* prevent pull request to be merged when PR is a WIP

* add tests

* add helper to prepend WIP: in PR title

* move default wip prefixes into settings

* use configurable WIP prefixes in javascript and default to first one in templates

* add documentation

* add unit test on pull model

Signed-off-by: Julien Tant <julien@craftyx.fr>

2 files changed, 63 insertions, 1 deletions

diff --git a/models/pull.go b/models/pull.go
index 7faf1f1172..79f6d7005d 100644
--- a/models/pull.go
+++ b/models/pull.go
@@ -214,7 +214,7 @@ func (pr *PullRequest) APIFormat() *api.PullRequest {
 	}
 
 	if pr.Status != PullRequestStatusChecking {
-		mergeable := pr.Status != PullRequestStatusConflict
+		mergeable := pr.Status != PullRequestStatusConflict && !pr.IsWorkInProgress()
 		apiPullRequest.Mergeable = mergeable
 	}
 	if pr.HasMerged {
@@ -1247,6 +1247,37 @@ func (pr *PullRequest) checkAndUpdateStatus() {
 	}
 }
 
+// IsWorkInProgress determine if the Pull Request is a Work In Progress by its title
+func (pr *PullRequest) IsWorkInProgress() bool {
+	if err := pr.LoadIssue(); err != nil {
+		log.Error(4, "LoadIssue: %v", err)
+		return false
+	}
+
+	for _, prefix := range setting.Repository.PullRequest.WorkInProgressPrefixes {
+		if strings.HasPrefix(strings.ToUpper(pr.Issue.Title), prefix) {
+			return true
+		}
+	}
+	return false
+}
+
+// GetWorkInProgressPrefix returns the prefix used to mark the pull request as a work in progress.
+// It returns an empty string when none were found
+func (pr *PullRequest) GetWorkInProgressPrefix() string {
+	if err := pr.LoadIssue(); err != nil {
+		log.Error(4, "LoadIssue: %v", err)
+		return ""
+	}
+
+	for _, prefix := range setting.Repository.PullRequest.WorkInProgressPrefixes {
+		if strings.HasPrefix(strings.ToUpper(pr.Issue.Title), prefix) {
+			return pr.Issue.Title[0:len(prefix)]
+		}
+	}
+	return ""
+}
+
 // TestPullRequests checks and tests untested patches of pull requests.
 // TODO: test more pull requests at same time.
 func TestPullRequests() {
diff --git a/models/pull_test.go b/models/pull_test.go
index e725193bb1..1dad664077 100644
--- a/models/pull_test.go
+++ b/models/pull_test.go
@@ -237,3 +237,34 @@ func TestChangeUsernameInPullRequests(t *testing.T) {
 	}
 	CheckConsistencyFor(t, &PullRequest{})
 }
+
+func TestPullRequest_IsWorkInProgress(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	pr := AssertExistsAndLoadBean(t, &PullRequest{ID: 2}).(*PullRequest)
+	pr.LoadIssue()
+
+	assert.False(t, pr.IsWorkInProgress())
+
+	pr.Issue.Title = "WIP: " + pr.Issue.Title
+	assert.True(t, pr.IsWorkInProgress())
+
+	pr.Issue.Title = "[wip]: " + pr.Issue.Title
+	assert.True(t, pr.IsWorkInProgress())
+}
+
+func TestPullRequest_GetWorkInProgressPrefixWorkInProgress(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	pr := AssertExistsAndLoadBean(t, &PullRequest{ID: 2}).(*PullRequest)
+	pr.LoadIssue()
+
+	assert.Empty(t, pr.GetWorkInProgressPrefix())
+
+	original := pr.Issue.Title
+	pr.Issue.Title = "WIP: " + original
+	assert.Equal(t, "WIP:", pr.GetWorkInProgressPrefix())
+
+	pr.Issue.Title = "[wip] " + original
+	assert.Equal(t, "[wip]", pr.GetWorkInProgressPrefix())
+}