diff options
| -rw-r--r-- | models/action.go | 74 | ||||
| -rw-r--r-- | models/action_test.go | 29 |
2 files changed, 63 insertions, 40 deletions
diff --git a/models/action.go b/models/action.go index cde0fb080e..d917c37746 100644 --- a/models/action.go +++ b/models/action.go @@ -476,8 +476,34 @@ func getIssueFromRef(repo *Repository, ref string) (*Issue, error) { return issue, nil } +func changeIssueStatus(repo *Repository, doer *User, ref string, refMarked map[int64]bool, status bool) error { + issue, err := getIssueFromRef(repo, ref) + if err != nil { + return err + } + + if issue == nil || refMarked[issue.ID] { + return nil + } + refMarked[issue.ID] = true + + if issue.RepoID != repo.ID || issue.IsClosed == status { + return nil + } + + issue.Repo = repo + if err = issue.ChangeStatus(doer, status); err != nil { + // Don't return an error when dependencies are open as this would let the push fail + if IsErrDependenciesLeft(err) { + return nil + } + return err + } + return nil +} + // UpdateIssuesCommit checks if issues are manipulated by commit message. -func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit) error { +func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, branchName string) error { // Commits are appended in the reverse order. for i := len(commits) - 1; i >= 0; i-- { c := commits[i] @@ -500,51 +526,21 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit) err } } + // Change issue status only if the commit has been pushed to the default branch. + if repo.DefaultBranch != branchName { + continue + } + refMarked = make(map[int64]bool) - // FIXME: can merge this one and next one to a common function. for _, ref := range issueCloseKeywordsPat.FindAllString(c.Message, -1) { - issue, err := getIssueFromRef(repo, ref) - if err != nil { - return err - } - - if issue == nil || refMarked[issue.ID] { - continue - } - refMarked[issue.ID] = true - - if issue.RepoID != repo.ID || issue.IsClosed { - continue - } - - issue.Repo = repo - if err = issue.ChangeStatus(doer, true); err != nil { - // Don't return an error when dependencies are open as this would let the push fail - if IsErrDependenciesLeft(err) { - return nil - } + if err := changeIssueStatus(repo, doer, ref, refMarked, true); err != nil { return err } } // It is conflict to have close and reopen at same time, so refsMarked doesn't need to reinit here. for _, ref := range issueReopenKeywordsPat.FindAllString(c.Message, -1) { - issue, err := getIssueFromRef(repo, ref) - if err != nil { - return err - } - - if issue == nil || refMarked[issue.ID] { - continue - } - refMarked[issue.ID] = true - - if issue.RepoID != repo.ID || !issue.IsClosed { - continue - } - - issue.Repo = repo - if err = issue.ChangeStatus(doer, false); err != nil { + if err := changeIssueStatus(repo, doer, ref, refMarked, false); err != nil { return err } } @@ -609,7 +605,7 @@ func CommitRepoAction(opts CommitRepoActionOptions) error { opts.Commits.CompareURL = repo.ComposeCompareURL(opts.OldCommitID, opts.NewCommitID) } - if err = UpdateIssuesCommit(pusher, repo, opts.Commits.Commits); err != nil { + if err = UpdateIssuesCommit(pusher, repo, opts.Commits.Commits, refName); err != nil { log.Error(4, "updateIssuesCommit: %v", err) } } diff --git a/models/action_test.go b/models/action_test.go index d0e0a5d8fa..0310b0ad5d 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -227,10 +227,37 @@ func TestUpdateIssuesCommit(t *testing.T) { AssertNotExistsBean(t, commentBean) AssertNotExistsBean(t, &Issue{RepoID: repo.ID, Index: 2}, "is_closed=1") - assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits)) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, repo.DefaultBranch)) AssertExistsAndLoadBean(t, commentBean) AssertExistsAndLoadBean(t, issueBean, "is_closed=1") CheckConsistencyFor(t, &Action{}) + + // Test that push to a non-default branch closes no issue. + pushCommits = []*PushCommit{ + { + Sha1: "abcdef1", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user4@example.com", + AuthorName: "User Four", + Message: "close #1", + }, + } + repo = AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository) + commentBean = &Comment{ + Type: CommentTypeCommitRef, + CommitSHA: "abcdef1", + PosterID: user.ID, + IssueID: 6, + } + issueBean = &Issue{RepoID: repo.ID, Index: 1} + + AssertNotExistsBean(t, commentBean) + AssertNotExistsBean(t, &Issue{RepoID: repo.ID, Index: 1}, "is_closed=1") + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, "non-existing-branch")) + AssertExistsAndLoadBean(t, commentBean) + AssertNotExistsBean(t, issueBean, "is_closed=1") + CheckConsistencyFor(t, &Action{}) } func testCorrectRepoAction(t *testing.T, opts CommitRepoActionOptions, actionBean *Action) { |