From 3d99ee30c1778182ed098e46080a60d8c7988abb Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Mon, 19 Mar 2018 12:25:42 -0400 Subject: [PATCH 01/24] Update issues when a pull request is merged --- models/action.go | 18 +++++++++++++----- models/action_test.go | 3 ++- models/pull.go | 11 ++++++----- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/models/action.go b/models/action.go index a6fb209a4..32cc91936 100644 --- a/models/action.go +++ b/models/action.go @@ -715,8 +715,12 @@ func TransferRepoAction(doer, oldOwner *User, repo *Repository) error { return transferRepoAction(x, doer, oldOwner, repo) } -func mergePullRequestAction(e Engine, doer *User, repo *Repository, issue *Issue) error { - return notifyWatchers(e, &Action{ +func mergePullRequestAction(e Engine, doer *User, repo *Repository, issue *Issue, commits *PushCommits) (err error) { + if err = UpdateIssuesCommit(doer, repo, commits.Commits); err != nil { + log.Error(4, "updateIssuesCommit: %v", err) + } + + if err = notifyWatchers(e, &Action{ ActUserID: doer.ID, ActUser: doer, OpType: ActionMergePullRequest, @@ -724,12 +728,16 @@ func mergePullRequestAction(e Engine, doer *User, repo *Repository, issue *Issue RepoID: repo.ID, Repo: repo, IsPrivate: repo.IsPrivate, - }) + }); err != nil { + return fmt.Errorf("notifyWatchers: %v", err) + } + + return nil } // MergePullRequestAction adds new action for merging pull request. -func MergePullRequestAction(actUser *User, repo *Repository, pull *Issue) error { - return mergePullRequestAction(x, actUser, repo, pull) +func MergePullRequestAction(actUser *User, repo *Repository, pull *Issue, commits *PushCommits) error { + return mergePullRequestAction(x, actUser, repo, pull, commits) } // GetFeedsOptions options for retrieving feeds diff --git a/models/action_test.go b/models/action_test.go index d0e0a5d8f..5c0c7a3e3 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -379,6 +379,7 @@ func TestMergePullRequestAction(t *testing.T) { repo := AssertExistsAndLoadBean(t, &Repository{ID: 1, OwnerID: user.ID}).(*Repository) repo.Owner = user issue := AssertExistsAndLoadBean(t, &Issue{ID: 3, RepoID: repo.ID}).(*Issue) + commits := &PushCommits{0, make([]*PushCommit, 0), "", nil} actionBean := &Action{ OpType: ActionMergePullRequest, @@ -389,7 +390,7 @@ func TestMergePullRequestAction(t *testing.T) { IsPrivate: repo.IsPrivate, } AssertNotExistsBean(t, actionBean) - assert.NoError(t, MergePullRequestAction(user, repo, issue)) + assert.NoError(t, MergePullRequestAction(user, repo, issue, commits)) AssertExistsAndLoadBean(t, actionBean) CheckConsistencyFor(t, &Action{}) } diff --git a/models/pull.go b/models/pull.go index 90b341907..b76da7bd5 100644 --- a/models/pull.go +++ b/models/pull.go @@ -445,10 +445,6 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle log.Error(4, "setMerged [%d]: %v", pr.ID, err) } - if err = MergePullRequestAction(doer, pr.Issue.Repo, pr.Issue); err != nil { - log.Error(4, "MergePullRequestAction [%d]: %v", pr.ID, err) - } - // Reset cached commit count cache.Remove(pr.Issue.Repo.GetCommitsCountCacheKey(pr.BaseBranch, true)) @@ -488,13 +484,18 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle if mergeStyle == MergeStyleMerge { l.PushFront(mergeCommit) } + commits := ListToPushCommits(l) + + if err = MergePullRequestAction(doer, pr.Issue.Repo, pr.Issue, commits); err != nil { + log.Error(4, "MergePullRequestAction [%d]: %v", pr.ID, err) + } p := &api.PushPayload{ Ref: git.BranchPrefix + pr.BaseBranch, Before: pr.MergeBase, After: mergeCommit.ID.String(), CompareURL: setting.AppURL + pr.BaseRepo.ComposeCompareURL(pr.MergeBase, pr.MergedCommitID), - Commits: ListToPushCommits(l).ToAPIPayloadCommits(pr.BaseRepo.HTMLURL()), + Commits: commits.ToAPIPayloadCommits(pr.BaseRepo.HTMLURL()), Repo: pr.BaseRepo.APIFormat(mode), Pusher: pr.HeadRepo.MustOwner().APIFormat(), Sender: doer.APIFormat(), From 8536f834e5da026db5d47134d34a1f48162a69dd Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Tue, 20 Mar 2018 20:23:42 -0400 Subject: [PATCH 02/24] Factor out detection of reference, close, reopen keywords into a more reusable function --- models/action.go | 127 +++++++++++++++++++++++++++-------------------- 1 file changed, 74 insertions(+), 53 deletions(-) diff --git a/models/action.go b/models/action.go index 32cc91936..5d65f712a 100644 --- a/models/action.go +++ b/models/action.go @@ -49,14 +49,29 @@ const ( ActionDeleteBranch // 17 ) +// KeywordsFoundMaskType represents the bitmask of types of keywords found in a message. +type KeywordsFoundMaskType int + +// Possible bitmask types for keywords that can be found. +const ( + KeywordsFoundReference KeywordsFoundMaskType = 1 << iota // 1 + KeywordsFoundReopen // 2 + KeywordsFoundClose // 4 +) + +// IssueKeywordsToFind represents a pairing of a pattern to use to find keywords in message and the keywords bitmask value. +type IssueKeywordsToFind struct { + Pattern *regexp.Regexp + KeywordsFoundMask KeywordsFoundMaskType +} + var ( // Same as Github. See // https://help.github.com/articles/closing-issues-via-commit-messages issueCloseKeywords = []string{"close", "closes", "closed", "fix", "fixes", "fixed", "resolve", "resolves", "resolved"} issueReopenKeywords = []string{"reopen", "reopens", "reopened"} - issueCloseKeywordsPat, issueReopenKeywordsPat *regexp.Regexp - issueReferenceKeywordsPat *regexp.Regexp + issueKeywordsToFind []*IssueKeywordsToFind ) const issueRefRegexpStr = `(?:\S+/\S=)?#\d+` @@ -66,9 +81,21 @@ func assembleKeywordsPattern(words []string) string { } func init() { - issueCloseKeywordsPat = regexp.MustCompile(assembleKeywordsPattern(issueCloseKeywords)) - issueReopenKeywordsPat = regexp.MustCompile(assembleKeywordsPattern(issueReopenKeywords)) - issueReferenceKeywordsPat = regexp.MustCompile(issueRefRegexpStr) + // populate with details to find keywords for reference, reopen, close + issueKeywordsToFind = []*IssueKeywordsToFind{ + &IssueKeywordsToFind{ + Pattern: regexp.MustCompile(issueRefRegexpStr), + KeywordsFoundMask: KeywordsFoundReference, + }, + &IssueKeywordsToFind{ + Pattern: regexp.MustCompile(assembleKeywordsPattern(issueReopenKeywords)), + KeywordsFoundMask: KeywordsFoundReopen, + }, + &IssueKeywordsToFind{ + Pattern: regexp.MustCompile(assembleKeywordsPattern(issueCloseKeywords)), + KeywordsFoundMask: KeywordsFoundClose, + }, + } } // Action represents user operation type and other information to @@ -435,70 +462,64 @@ func getIssueFromRef(repo *Repository, ref string) (*Issue, error) { return issue, nil } +// findIssueReferencesInString iterates over the keywords to find in a message and accumulates the findings into refs +func findIssueReferencesInString(message string, repo *Repository) (map[int64]KeywordsFoundMaskType, error) { + refs := make(map[int64]KeywordsFoundMaskType) + for _, kwToFind := range issueKeywordsToFind { + for _, ref := range kwToFind.Pattern.FindAllString(message, -1) { + issue, err := getIssueFromRef(repo, ref) + if err != nil { + return nil, err + } + + if issue != nil { + refs[issue.ID] |= kwToFind.KeywordsFoundMask + } + } + } + return refs, nil +} + // UpdateIssuesCommit checks if issues are manipulated by commit message. func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit) error { // Commits are appended in the reverse order. for i := len(commits) - 1; i >= 0; i-- { c := commits[i] - refMarked := make(map[int64]bool) - for _, ref := range issueReferenceKeywordsPat.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 - - message := fmt.Sprintf(`%s`, repo.Link(), c.Sha1, c.Message) - if err = CreateRefComment(doer, repo, issue, message, c.Sha1); err != nil { - return err - } + refs, err := findIssueReferencesInString(c.Message, repo) + if err != nil { + return err } - 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) + for id, mask := range refs { + issue, err := GetIssueByID(id) if err != nil { return err } - - if issue == nil || refMarked[issue.ID] { - continue - } - refMarked[issue.ID] = true - - if issue.RepoID != repo.ID || issue.IsClosed { + if issue == nil { continue } - if err = issue.ChangeStatus(doer, repo, 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 (mask & KeywordsFoundReference) == KeywordsFoundReference { + message := fmt.Sprintf(`%s`, repo.Link(), c.Sha1, c.Message) + if err = CreateRefComment(doer, repo, issue, message, c.Sha1); err != nil { + return err + } } - if issue == nil || refMarked[issue.ID] { - continue - } - refMarked[issue.ID] = true - - if issue.RepoID != repo.ID || !issue.IsClosed { - continue - } - - if err = issue.ChangeStatus(doer, repo, false); err != nil { - return err + // take no action if both KeywordsFoundClose and KeywordsFoundOpen are set + if (mask & (KeywordsFoundReopen|KeywordsFoundClose)) == KeywordsFoundClose { + if issue.RepoID == repo.ID && !issue.IsClosed { + if err = issue.ChangeStatus(doer, repo, true); err != nil { + return err + } + } + } else if (mask & (KeywordsFoundReopen|KeywordsFoundClose)) == KeywordsFoundReopen { + if issue.RepoID == repo.ID && issue.IsClosed { + if err = issue.ChangeStatus(doer, repo, false); err != nil { + return err + } + } } } } From abd1f1f6e1ee9739953f6a18009aa898fec83571 Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Fri, 23 Mar 2018 09:48:23 -0400 Subject: [PATCH 03/24] Only change issue status when commits have been merged --- models/action.go | 38 ++++++++++++++++++++------------------ models/action_test.go | 2 +- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/models/action.go b/models/action.go index 5d65f712a..699c2d644 100644 --- a/models/action.go +++ b/models/action.go @@ -481,7 +481,7 @@ func findIssueReferencesInString(message string, repo *Repository) (map[int64]Ke } // 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, commitsAreMerged bool) error { // Commits are appended in the reverse order. for i := len(commits) - 1; i >= 0; i-- { c := commits[i] @@ -507,17 +507,19 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit) err } } - // take no action if both KeywordsFoundClose and KeywordsFoundOpen are set - if (mask & (KeywordsFoundReopen|KeywordsFoundClose)) == KeywordsFoundClose { - if issue.RepoID == repo.ID && !issue.IsClosed { - if err = issue.ChangeStatus(doer, repo, true); err != nil { - return err + if commitsAreMerged { + // take no action if both KeywordsFoundClose and KeywordsFoundOpen are set + if (mask & (KeywordsFoundReopen|KeywordsFoundClose)) == KeywordsFoundClose { + if issue.RepoID == repo.ID && !issue.IsClosed { + if err = issue.ChangeStatus(doer, repo, true); err != nil { + return err + } } - } - } else if (mask & (KeywordsFoundReopen|KeywordsFoundClose)) == KeywordsFoundReopen { - if issue.RepoID == repo.ID && issue.IsClosed { - if err = issue.ChangeStatus(doer, repo, false); err != nil { - return err + } else if (mask & (KeywordsFoundReopen|KeywordsFoundClose)) == KeywordsFoundReopen { + if issue.RepoID == repo.ID && issue.IsClosed { + if err = issue.ChangeStatus(doer, repo, false); err != nil { + return err + } } } } @@ -581,7 +583,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, true); err != nil { log.Error(4, "updateIssuesCommit: %v", err) } } @@ -736,16 +738,12 @@ func TransferRepoAction(doer, oldOwner *User, repo *Repository) error { return transferRepoAction(x, doer, oldOwner, repo) } -func mergePullRequestAction(e Engine, doer *User, repo *Repository, issue *Issue, commits *PushCommits) (err error) { - if err = UpdateIssuesCommit(doer, repo, commits.Commits); err != nil { - log.Error(4, "updateIssuesCommit: %v", err) - } - +func mergePullRequestAction(e Engine, doer *User, repo *Repository, pull *Issue, commits *PushCommits) (err error) { if err = notifyWatchers(e, &Action{ ActUserID: doer.ID, ActUser: doer, OpType: ActionMergePullRequest, - Content: fmt.Sprintf("%d|%s", issue.Index, issue.Title), + Content: fmt.Sprintf("%d|%s", pull.Index, pull.Title), RepoID: repo.ID, Repo: repo, IsPrivate: repo.IsPrivate, @@ -758,6 +756,10 @@ func mergePullRequestAction(e Engine, doer *User, repo *Repository, issue *Issue // MergePullRequestAction adds new action for merging pull request. func MergePullRequestAction(actUser *User, repo *Repository, pull *Issue, commits *PushCommits) error { + if err := UpdateIssuesCommit(actUser, repo, commits.Commits, true); err != nil { + log.Error(4, "updateIssuesCommit: %v", err) + } + return mergePullRequestAction(x, actUser, repo, pull, commits) } diff --git a/models/action_test.go b/models/action_test.go index 5c0c7a3e3..519cba0b5 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -227,7 +227,7 @@ 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, true)) AssertExistsAndLoadBean(t, commentBean) AssertExistsAndLoadBean(t, issueBean, "is_closed=1") CheckConsistencyFor(t, &Action{}) From 8a9bd7a9b71873007da240b7ad478961de895a24 Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Fri, 23 Mar 2018 10:07:48 -0400 Subject: [PATCH 04/24] Add issue updating from commit messages in new pull request --- models/action.go | 27 ++++++++++++++++++++++++++ models/pull.go | 50 ++++++++++++++++++++++++++++++++++-------------- 2 files changed, 63 insertions(+), 14 deletions(-) diff --git a/models/action.go b/models/action.go index 699c2d644..2fe134349 100644 --- a/models/action.go +++ b/models/action.go @@ -763,6 +763,33 @@ func MergePullRequestAction(actUser *User, repo *Repository, pull *Issue, commit return mergePullRequestAction(x, actUser, repo, pull, commits) } +func newPullRequestAction(e Engine, doer *User, repo *Repository, pull *Issue, commits *PushCommits) (err error) { + if err = NotifyWatchers(&Action{ + ActUserID: doer.ID, + ActUser: doer, + OpType: ActionCreatePullRequest, + Content: fmt.Sprintf("%d|%s", pull.Index, pull.Title), + RepoID: repo.ID, + Repo: repo, + IsPrivate: repo.IsPrivate, + }); err != nil { + log.Error(4, "NotifyWatchers: %v", err) + } else if err = pull.MailParticipants(); err != nil { + log.Error(4, "MailParticipants: %v", err) + } + + return nil +} + +// NewPullRequestAction adds new action for merging pull request. +func NewPullRequestAction(actUser *User, repo *Repository, pull *Issue, commits *PushCommits) error { + if err := UpdateIssuesCommit(actUser, repo, commits.Commits, false); err != nil { + log.Error(4, "updateIssuesCommit: %v", err) + } + + return newPullRequestAction(x, actUser, repo, pull, commits) +} + // GetFeedsOptions options for retrieving feeds type GetFeedsOptions struct { RequestedUser *User diff --git a/models/pull.go b/models/pull.go index b76da7bd5..e890389e1 100644 --- a/models/pull.go +++ b/models/pull.go @@ -772,23 +772,45 @@ func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []str UpdateIssueIndexer(pull.ID) - if err = NotifyWatchers(&Action{ - ActUserID: pull.Poster.ID, - ActUser: pull.Poster, - OpType: ActionCreatePullRequest, - Content: fmt.Sprintf("%d|%s", pull.Index, pull.Title), - RepoID: repo.ID, - Repo: repo, - IsPrivate: repo.IsPrivate, - }); err != nil { - log.Error(4, "NotifyWatchers: %v", err) - } else if err = pull.MailParticipants(); err != nil { - log.Error(4, "MailParticipants: %v", err) - } - pr.Issue = pull pull.PullRequest = pr mode, _ := AccessLevel(pull.Poster.ID, repo) + + var ( + baseBranch *Branch + headBranch *Branch + baseCommit *git.Commit + headCommit *git.Commit + baseGitRepo *git.Repository + ) + if baseBranch, err = pr.BaseRepo.GetBranch(pr.BaseBranch); err != nil { + return nil + } + if baseCommit, err = baseBranch.GetCommit(); err != nil { + return nil + } + if headBranch, err = pr.HeadRepo.GetBranch(pr.HeadBranch); err != nil { + return nil + } + if headCommit, err = headBranch.GetCommit(); err != nil { + return nil + } + if baseGitRepo, err = git.OpenRepository(pr.BaseRepo.RepoPath()); err != nil { + log.Error(4, "OpenRepository", err) + return nil + } + + l, err := baseGitRepo.CommitsBetweenIDs(headCommit.ID.String(), baseCommit.ID.String()) + if err != nil { + log.Error(4, "CommitsBetweenIDs: %v", err) + return nil + } + + commits := ListToPushCommits(l) + if err = NewPullRequestAction(pull.Poster, repo, pull, commits); err != nil { + log.Error(4, "NewPullRequestAction [%d]: %v", pr.ID, err) + } + if err = PrepareWebhooks(repo, HookEventPullRequest, &api.PullRequestPayload{ Action: api.HookIssueOpened, Index: pull.Index, From 58662ff1b6addd3e9011381d98f1de32c43e0c5e Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Tue, 27 Mar 2018 12:53:29 -0400 Subject: [PATCH 05/24] Clean up {Merge,New}PullRequestAction and add UpdatePullRequestAction --- models/action.go | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/models/action.go b/models/action.go index 2fe134349..51fefc96d 100644 --- a/models/action.go +++ b/models/action.go @@ -584,7 +584,7 @@ func CommitRepoAction(opts CommitRepoActionOptions) error { } if err = UpdateIssuesCommit(pusher, repo, opts.Commits.Commits, true); err != nil { - log.Error(4, "updateIssuesCommit: %v", err) + log.Error(4, "UpdateIssuesCommit: %v", err) } } @@ -738,8 +738,13 @@ func TransferRepoAction(doer, oldOwner *User, repo *Repository) error { return transferRepoAction(x, doer, oldOwner, repo) } -func mergePullRequestAction(e Engine, doer *User, repo *Repository, pull *Issue, commits *PushCommits) (err error) { - if err = notifyWatchers(e, &Action{ +// MergePullRequestAction adds new action for merging pull request. +func MergePullRequestAction(doer *User, repo *Repository, pull *Issue, commits *PushCommits) error { + if err := UpdateIssuesCommit(doer, repo, commits.Commits, true); err != nil { + log.Error(4, "UpdateIssuesCommit: %v", err) + } + + if err := notifyWatchers(x, &Action{ ActUserID: doer.ID, ActUser: doer, OpType: ActionMergePullRequest, @@ -754,17 +759,13 @@ func mergePullRequestAction(e Engine, doer *User, repo *Repository, pull *Issue, return nil } -// MergePullRequestAction adds new action for merging pull request. -func MergePullRequestAction(actUser *User, repo *Repository, pull *Issue, commits *PushCommits) error { - if err := UpdateIssuesCommit(actUser, repo, commits.Commits, true); err != nil { - log.Error(4, "updateIssuesCommit: %v", err) +// NewPullRequestAction adds new action for creating a new pull request. +func NewPullRequestAction(doer *User, repo *Repository, pull *Issue, commits *PushCommits) error { + if err := UpdateIssuesCommit(doer, repo, commits.Commits, false); err != nil { + log.Error(4, "UpdateIssuesCommit: %v", err) } - return mergePullRequestAction(x, actUser, repo, pull, commits) -} - -func newPullRequestAction(e Engine, doer *User, repo *Repository, pull *Issue, commits *PushCommits) (err error) { - if err = NotifyWatchers(&Action{ + if err := NotifyWatchers(&Action{ ActUserID: doer.ID, ActUser: doer, OpType: ActionCreatePullRequest, @@ -774,20 +775,20 @@ func newPullRequestAction(e Engine, doer *User, repo *Repository, pull *Issue, c IsPrivate: repo.IsPrivate, }); err != nil { log.Error(4, "NotifyWatchers: %v", err) - } else if err = pull.MailParticipants(); err != nil { + } else if err := pull.MailParticipants(); err != nil { log.Error(4, "MailParticipants: %v", err) } return nil } -// NewPullRequestAction adds new action for merging pull request. -func NewPullRequestAction(actUser *User, repo *Repository, pull *Issue, commits *PushCommits) error { - if err := UpdateIssuesCommit(actUser, repo, commits.Commits, false); err != nil { - log.Error(4, "updateIssuesCommit: %v", err) +// UpdatePullRequestAction adds new action for updating or merging a pull request. +func UpdatePullRequestAction(doer *User, repo *Repository, pull *Issue, commits *PushCommits) error { + if err := UpdateIssuesCommit(doer, repo, commits.Commits, false); err != nil { + log.Error(4, "UpdateIssuesCommit: %v", err) } - return newPullRequestAction(x, actUser, repo, pull, commits) + return nil } // GetFeedsOptions options for retrieving feeds From 13df8f596ebf0ae4585ee6dcc7dc8516577cc164 Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Tue, 27 Mar 2018 12:57:09 -0400 Subject: [PATCH 06/24] Add issue updating from commits being tracked by pull requests --- models/update.go | 68 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 2 deletions(-) diff --git a/models/update.go b/models/update.go index b1bbe0754..0dac18c45 100644 --- a/models/update.go +++ b/models/update.go @@ -67,7 +67,7 @@ type PushUpdateOptions struct { // PushUpdate must be called for any push actions in order to // generates necessary push action history feeds. func PushUpdate(branch string, opt PushUpdateOptions) error { - repo, err := pushUpdate(opt) + repo, err := pushUpdate(branch, opt) if err != nil { return err } @@ -183,7 +183,7 @@ func pushUpdateAddTag(repo *Repository, gitRepo *git.Repository, tagName string) return nil } -func pushUpdate(opts PushUpdateOptions) (repo *Repository, err error) { +func pushUpdate(branch string, opts PushUpdateOptions) (repo *Repository, err error) { isNewRef := opts.OldCommitID == git.EmptySHA isDelRef := opts.NewCommitID == git.EmptySHA if isNewRef && isDelRef { @@ -277,5 +277,69 @@ func pushUpdate(opts PushUpdateOptions) (repo *Repository, err error) { }); err != nil { return nil, fmt.Errorf("CommitRepoAction: %v", err) } + + // create actions that update pull requests tracking the branch that was pushed to + prs, err := GetUnmergedPullRequestsByHeadInfo(repo.ID, branch) + if err != nil { + log.Error(4, "Find pull requests [head_repo_id: %d, head_branch: %s]: %v", repo.ID, branch, err) + } else { + pusher, err := GetUserByID(opts.PusherID) + if err != nil { + return nil, fmt.Errorf("GetUserByID: %v", err) + } + + for _, pr := range prs { + if err = pr.GetHeadRepo(); err != nil { + log.Error(4, "GetHeadRepo: %v", err) + continue + } else if err = pr.GetBaseRepo(); err != nil { + log.Error(4, "GetBaseRepo: %v", err) + continue + } + + var ( + baseBranch *Branch + headBranch *Branch + baseCommit *git.Commit + headCommit *git.Commit + headGitRepo *git.Repository + ) + if baseBranch, err = pr.BaseRepo.GetBranch(pr.BaseBranch); err != nil { + log.Error(4, "BaseRepo.GetBranch: %v", err) + continue + } + if baseCommit, err = baseBranch.GetCommit(); err != nil { + log.Error(4, "baseBranch.GetCommit: %v", err) + continue + } + if headBranch, err = pr.HeadRepo.GetBranch(pr.HeadBranch); err != nil { + log.Error(4, "HeadRepo.GetBranch: %v", err) + continue + } + if headCommit, err = headBranch.GetCommit(); err != nil { + log.Error(4, "headRepo.GetCommit: %v", err) + continue + } + + // NOTICE: this is using pr.HeadRepo rather than pr.BaseRepo to get commits since they are going to be pushed in a go routine + if headGitRepo, err = git.OpenRepository(pr.HeadRepo.RepoPath()); err != nil { + log.Error(4, "OpenRepository", err) + continue + } + + l, err := headGitRepo.CommitsBetweenIDs(headCommit.ID.String(), baseCommit.ID.String()) + if err != nil { + log.Error(4, "CommitsBetweenIDs: %v", err) + continue + } + + commits := ListToPushCommits(l) + if err = UpdatePullRequestAction(pusher, pr.BaseRepo, pr.Issue, commits); err != nil { + log.Error(4, "UpdatePullRequestAction [%d]: %v", pr.ID, err) + continue + } + } + } + return repo, nil } From f4d086b1f5d5011203267ab838587f4bf6949e73 Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Tue, 27 Mar 2018 13:05:14 -0400 Subject: [PATCH 07/24] Fix code formatting --- models/action.go | 10 +++++----- models/pull.go | 8 ++++---- models/update.go | 8 ++++---- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/models/action.go b/models/action.go index 51fefc96d..400ede0e4 100644 --- a/models/action.go +++ b/models/action.go @@ -83,15 +83,15 @@ func assembleKeywordsPattern(words []string) string { func init() { // populate with details to find keywords for reference, reopen, close issueKeywordsToFind = []*IssueKeywordsToFind{ - &IssueKeywordsToFind{ + { Pattern: regexp.MustCompile(issueRefRegexpStr), KeywordsFoundMask: KeywordsFoundReference, }, - &IssueKeywordsToFind{ + { Pattern: regexp.MustCompile(assembleKeywordsPattern(issueReopenKeywords)), KeywordsFoundMask: KeywordsFoundReopen, }, - &IssueKeywordsToFind{ + { Pattern: regexp.MustCompile(assembleKeywordsPattern(issueCloseKeywords)), KeywordsFoundMask: KeywordsFoundClose, }, @@ -509,13 +509,13 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, com if commitsAreMerged { // take no action if both KeywordsFoundClose and KeywordsFoundOpen are set - if (mask & (KeywordsFoundReopen|KeywordsFoundClose)) == KeywordsFoundClose { + if (mask & (KeywordsFoundReopen | KeywordsFoundClose)) == KeywordsFoundClose { if issue.RepoID == repo.ID && !issue.IsClosed { if err = issue.ChangeStatus(doer, repo, true); err != nil { return err } } - } else if (mask & (KeywordsFoundReopen|KeywordsFoundClose)) == KeywordsFoundReopen { + } else if (mask & (KeywordsFoundReopen | KeywordsFoundClose)) == KeywordsFoundReopen { if issue.RepoID == repo.ID && issue.IsClosed { if err = issue.ChangeStatus(doer, repo, false); err != nil { return err diff --git a/models/pull.go b/models/pull.go index e890389e1..d87bf4bc2 100644 --- a/models/pull.go +++ b/models/pull.go @@ -777,10 +777,10 @@ func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []str mode, _ := AccessLevel(pull.Poster.ID, repo) var ( - baseBranch *Branch - headBranch *Branch - baseCommit *git.Commit - headCommit *git.Commit + baseBranch *Branch + headBranch *Branch + baseCommit *git.Commit + headCommit *git.Commit baseGitRepo *git.Repository ) if baseBranch, err = pr.BaseRepo.GetBranch(pr.BaseBranch); err != nil { diff --git a/models/update.go b/models/update.go index 0dac18c45..f20a8d080 100644 --- a/models/update.go +++ b/models/update.go @@ -298,10 +298,10 @@ func pushUpdate(branch string, opts PushUpdateOptions) (repo *Repository, err er } var ( - baseBranch *Branch - headBranch *Branch - baseCommit *git.Commit - headCommit *git.Commit + baseBranch *Branch + headBranch *Branch + baseCommit *git.Commit + headCommit *git.Commit headGitRepo *git.Repository ) if baseBranch, err = pr.BaseRepo.GetBranch(pr.BaseBranch); err != nil { From faf860e707e0cca72161697c122a3747354b1215 Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Mon, 2 Apr 2018 10:02:52 -0400 Subject: [PATCH 08/24] Rename {Update -> Commit}PullRequestAction to be more accurately named --- models/action.go | 4 ++-- models/update.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/models/action.go b/models/action.go index 400ede0e4..4ba7ea09c 100644 --- a/models/action.go +++ b/models/action.go @@ -782,8 +782,8 @@ func NewPullRequestAction(doer *User, repo *Repository, pull *Issue, commits *Pu return nil } -// UpdatePullRequestAction adds new action for updating or merging a pull request. -func UpdatePullRequestAction(doer *User, repo *Repository, pull *Issue, commits *PushCommits) error { +// CommitPullRequestAction adds new action for pushed commits tracked by a pull request. +func CommitPullRequestAction(doer *User, repo *Repository, commits *PushCommits) error { if err := UpdateIssuesCommit(doer, repo, commits.Commits, false); err != nil { log.Error(4, "UpdateIssuesCommit: %v", err) } diff --git a/models/update.go b/models/update.go index f20a8d080..e31aeca2b 100644 --- a/models/update.go +++ b/models/update.go @@ -334,8 +334,8 @@ func pushUpdate(branch string, opts PushUpdateOptions) (repo *Repository, err er } commits := ListToPushCommits(l) - if err = UpdatePullRequestAction(pusher, pr.BaseRepo, pr.Issue, commits); err != nil { - log.Error(4, "UpdatePullRequestAction [%d]: %v", pr.ID, err) + if err = CommitPullRequestAction(pusher, pr.BaseRepo, commits); err != nil { + log.Error(4, "CommitPullRequestAction [%d]: %v", pr.ID, err) continue } } From 8c7a08fccf4f56ba572385f39bfaba0a7163310f Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Mon, 2 Apr 2018 10:10:13 -0400 Subject: [PATCH 09/24] Factor out creation of reference comments --- models/action.go | 2 +- models/issue_comment.go | 41 +++++++++++++++++++++++++++++++---------- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/models/action.go b/models/action.go index 4ba7ea09c..b81e7b3d3 100644 --- a/models/action.go +++ b/models/action.go @@ -502,7 +502,7 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, com if (mask & KeywordsFoundReference) == KeywordsFoundReference { message := fmt.Sprintf(`%s`, repo.Link(), c.Sha1, c.Message) - if err = CreateRefComment(doer, repo, issue, message, c.Sha1); err != nil { + if err = CreateCommitRefComment(doer, repo, issue, message, c.Sha1); err != nil { return err } } diff --git a/models/issue_comment.go b/models/issue_comment.go index 1c7c57dd0..072252dec 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -107,7 +107,8 @@ type Comment struct { CreatedUnix util.TimeStamp `xorm:"INDEX created"` UpdatedUnix util.TimeStamp `xorm:"INDEX updated"` - // Reference issue in commit message + // Reference issue in commit message, comments, issues, or pull requests + // the commit SHA for commit refs otherwise a SHA of a unique reference identifier CommitSHA string `xorm:"VARCHAR(40)"` Attachments []*Attachment `xorm:"-"` @@ -622,17 +623,17 @@ func CreateIssueComment(doer *User, repo *Repository, issue *Issue, content stri return comment, nil } -// CreateRefComment creates a commit reference comment to issue. -func CreateRefComment(doer *User, repo *Repository, issue *Issue, content, commitSHA string) error { - if len(commitSHA) == 0 { - return fmt.Errorf("cannot create reference with empty commit SHA") +// createRefComment creates a commit, comment, issue, or pull request reference comment to issue. +func createRefComment(doer *User, repo *Repository, issue *Issue, content, refSHA string, commentType CommentType) error { + if len(refSHA) == 0 { + return fmt.Errorf("cannot create reference with empty SHA") } - // Check if same reference from same commit has already existed. + // Check if same reference from same issue and comment has already existed. has, err := x.Get(&Comment{ - Type: CommentTypeCommitRef, + Type: commentType, IssueID: issue.ID, - CommitSHA: commitSHA, + CommitSHA: refSHA, }) if err != nil { return fmt.Errorf("check reference comment: %v", err) @@ -641,16 +642,36 @@ func CreateRefComment(doer *User, repo *Repository, issue *Issue, content, commi } _, err = CreateComment(&CreateCommentOptions{ - Type: CommentTypeCommitRef, + Type: commentType, Doer: doer, Repo: repo, Issue: issue, - CommitSHA: commitSHA, + CommitSHA: refSHA, Content: content, }) return err } +// CreateCommitRefComment creates a commit reference comment to issue. +func CreateCommitRefComment(doer *User, repo *Repository, issue *Issue, content, commitSHA string) error { + return createRefComment(doer, repo, issue, content, commitSHA, CommentTypeCommitRef) +} + +// CreateCommentRefComment creates a comment reference comment to issue. +func CreateCommentRefComment(doer *User, repo *Repository, issue *Issue, content, refSHA string) error { + return createRefComment(doer, repo, issue, content, refSHA, CommentTypeCommentRef) +} + +// CreateIssueRefComment creates a comment reference comment to issue. +func CreateIssueRefComment(doer *User, repo *Repository, issue *Issue, content, refSHA string) error { + return createRefComment(doer, repo, issue, content, refSHA, CommentTypeIssueRef) +} + +// CreatePullRefComment creates a comment reference comment to issue. +func CreatePullRefComment(doer *User, repo *Repository, issue *Issue, content, refSHA string) error { + return createRefComment(doer, repo, issue, content, refSHA, CommentTypePullRef) +} + // GetCommentByID returns the comment by given ID. func GetCommentByID(id int64) (*Comment, error) { c := new(Comment) From 59d91723a12291a9a982c0e2e6db01192b07340c Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Mon, 2 Apr 2018 10:19:20 -0400 Subject: [PATCH 10/24] Add the UpdateIssuesComment function The UpdateIssuesComment function handles referencing and manipulation of issues from comments, issues, and pull requests --- models/action.go | 61 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/models/action.go b/models/action.go index b81e7b3d3..5747d3979 100644 --- a/models/action.go +++ b/models/action.go @@ -480,6 +480,67 @@ func findIssueReferencesInString(message string, repo *Repository) (map[int64]Ke return refs, nil } +// UpdateIssuesComment checks if issues are manipulated by a comment +func UpdateIssuesComment(doer *User, repo *Repository, commentIssue *Issue, comment *Comment, canOpenClose bool) error { + var refString string + if comment != nil { + refString = comment.Content + } else { + refString = commentIssue.Title + ": " + commentIssue.Content + } + + refs, err := findIssueReferencesInString(refString, repo) + if err != nil { + return err + } + + for id, mask := range refs { + issue, err := GetIssueByID(id) + if err != nil { + return err + } + if issue == nil || issue.ID == commentIssue.ID { + continue + } + + if (mask & KeywordsFoundReference) == KeywordsFoundReference { + uniqueID := fmt.Sprintf("%d", commentIssue.ID) + if comment != nil { + uniqueID += fmt.Sprintf("@%d", comment.ID) + } + + if comment != nil { + err = CreateCommentRefComment(doer, repo, issue, fmt.Sprintf(`%d`, comment.ID), base.EncodeSha1(uniqueID)) + } else if commentIssue.IsPull { + err = CreatePullRefComment(doer, repo, issue, fmt.Sprintf(`%d`, commentIssue.ID), base.EncodeSha1(uniqueID)) + } else { + err = CreateIssueRefComment(doer, repo, issue, fmt.Sprintf(`%d`, commentIssue.ID), base.EncodeSha1(uniqueID)) + } + if err != nil { + return err + } + } + + if canOpenClose { + // take no action if both KeywordsFoundClose and KeywordsFoundOpen are set + if (mask & (KeywordsFoundReopen | KeywordsFoundClose)) == KeywordsFoundClose { + if issue.RepoID == repo.ID && !issue.IsClosed { + if err = issue.ChangeStatus(doer, repo, true); err != nil { + return err + } + } + } else if (mask & (KeywordsFoundReopen | KeywordsFoundClose)) == KeywordsFoundReopen { + if issue.RepoID == repo.ID && issue.IsClosed { + if err = issue.ChangeStatus(doer, repo, false); err != nil { + return err + } + } + } + } + } + return nil +} + // UpdateIssuesCommit checks if issues are manipulated by commit message. func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, commitsAreMerged bool) error { // Commits are appended in the reverse order. From e20edf65071463123a7d98b740462905603afe22 Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Mon, 2 Apr 2018 10:28:46 -0400 Subject: [PATCH 11/24] Hook up the UpdateIssuesComment function to various actions The UpdateIssuesComment function is now being called when: - a pull request is merged, so the pull request title or description can open/close issues - a new pull request is created, so it can reference an issue from its title or description - a new issue is created, so it can reference an issue from its title or description - a pull request or issue title or description is updated - a comment is created or updated, so it can reference an issue from its title or description --- models/action.go | 32 ++++++++++++++++++++++++++++++++ models/issue.go | 12 ++++++++++++ models/issue_comment.go | 13 +++++++++++++ 3 files changed, 57 insertions(+) diff --git a/models/action.go b/models/action.go index 5747d3979..57f0a01e0 100644 --- a/models/action.go +++ b/models/action.go @@ -805,6 +805,10 @@ func MergePullRequestAction(doer *User, repo *Repository, pull *Issue, commits * log.Error(4, "UpdateIssuesCommit: %v", err) } + if err := UpdateIssuesComment(doer, repo, pull, nil, true); err != nil { + log.Error(4, "UpdateIssuesCommit: %v", err) + } + if err := notifyWatchers(x, &Action{ ActUserID: doer.ID, ActUser: doer, @@ -826,6 +830,10 @@ func NewPullRequestAction(doer *User, repo *Repository, pull *Issue, commits *Pu log.Error(4, "UpdateIssuesCommit: %v", err) } + if err := UpdateIssuesComment(doer, repo, pull, nil, false); err != nil { + log.Error(4, "UpdateIssuesCommit: %v", err) + } + if err := NotifyWatchers(&Action{ ActUserID: doer.ID, ActUser: doer, @@ -849,6 +857,30 @@ func CommitPullRequestAction(doer *User, repo *Repository, commits *PushCommits) log.Error(4, "UpdateIssuesCommit: %v", err) } + // no action added + + return nil +} + +// CreateOrUpdateCommentAction adds new action when creating or updating a comment. +func CreateOrUpdateCommentAction(doer *User, repo *Repository, issue *Issue, comment *Comment) error { + if err := UpdateIssuesComment(doer, repo, issue, comment, false); err != nil { + log.Error(4, "UpdateIssuesComment: %v", err) + } + + // no action added + + return nil +} + +// CreateOrUpdateIssueAction adds new action when creating a new issue or pull request. +func CreateOrUpdateIssueAction(doer *User, repo *Repository, issue *Issue) error { + if err := UpdateIssuesComment(doer, repo, issue, nil, false); err != nil { + log.Error(4, "UpdateIssuesComment: %v", err) + } + + // no action added + return nil } diff --git a/models/issue.go b/models/issue.go index d97266b4e..0200c86ff 100644 --- a/models/issue.go +++ b/models/issue.go @@ -802,6 +802,10 @@ func (issue *Issue) ChangeTitle(doer *User, title string) (err error) { go HookQueue.Add(issue.RepoID) } + if err = CreateOrUpdateIssueAction(doer, issue.Repo, issue); err != nil { + return fmt.Errorf("CreateOrUpdateIssueAction: %v", err) + } + return nil } @@ -867,6 +871,10 @@ func (issue *Issue) ChangeContent(doer *User, content string) (err error) { go HookQueue.Add(issue.RepoID) } + if err = CreateOrUpdateIssueAction(doer, issue.Repo, issue); err != nil { + return fmt.Errorf("CreateOrUpdateIssueAction: %v", err) + } + return nil } @@ -1037,6 +1045,10 @@ func NewIssue(repo *Repository, issue *Issue, labelIDs []int64, assigneeIDs []in UpdateIssueIndexer(issue.ID) + if err = CreateOrUpdateIssueAction(issue.Poster, issue.Repo, issue); err != nil { + return fmt.Errorf("CreateOrUpdateIssueAction: %v", err) + } + if err = NotifyWatchers(&Action{ ActUserID: issue.Poster.ID, ActUser: issue.Poster, diff --git a/models/issue_comment.go b/models/issue_comment.go index 072252dec..06bfa1890 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -590,6 +590,10 @@ func CreateComment(opts *CreateCommentOptions) (comment *Comment, err error) { if opts.Type == CommentTypeComment { UpdateIssueIndexer(opts.Issue.ID) + + if err = CreateOrUpdateCommentAction(comment.Poster, opts.Repo, opts.Issue, comment); err != nil { + return nil, err + } } return comment, nil } @@ -758,6 +762,15 @@ func UpdateComment(doer *User, c *Comment, oldContent string) error { return err } else if c.Type == CommentTypeComment { UpdateIssueIndexer(c.IssueID) + + issue, err := GetIssueByID(c.IssueID) + if err != nil { + return err + } + + if err = CreateOrUpdateCommentAction(c.Poster, issue.Repo, issue, c); err != nil { + return err + } } if err := c.LoadIssue(); err != nil { From f25b15358c08ab857281835cb548e56d4d5b5193 Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Mon, 2 Apr 2018 10:35:59 -0400 Subject: [PATCH 12/24] Add the LoadReference function Add the LoadReference function to prepare data members of reference types of comments for frontend rendering. Also prepares for the transition of commit reference comments into a new message format. --- models/issue_comment.go | 105 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/models/issue_comment.go b/models/issue_comment.go index 06bfa1890..d337fcedf 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -14,6 +14,7 @@ import ( api "code.gitea.io/sdk/gitea" + "code.gitea.io/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/util" @@ -108,6 +109,11 @@ type Comment struct { UpdatedUnix util.TimeStamp `xorm:"INDEX updated"` // Reference issue in commit message, comments, issues, or pull requests + RefExists bool + RefIssue *Issue + RefComment *Comment + RefMessage string + RefURL string // the commit SHA for commit refs otherwise a SHA of a unique reference identifier CommitSHA string `xorm:"VARCHAR(40)"` @@ -226,6 +232,105 @@ func (c *Comment) EventTag() string { return "event-" + com.ToStr(c.ID) } +// LoadReference if comment.Type is CommentType{Issue,Commit,Comment,Pull}Ref, then load RefIssue, RefComment +func (c *Comment) LoadReference() error { + if c.Type == CommentTypeIssueRef || c.Type == CommentTypePullRef { + issueID := int64(0) + n, err := fmt.Sscanf(c.Content, "%d", &issueID) + if err != nil { + return err + } + + if n == 1 { + refIssue, err := GetIssueByID(issueID) + if err != nil { + return err + } + + pullOrIssue := "issues" + if refIssue.IsPull { + pullOrIssue = "pulls" + } + + c.RefIssue = refIssue + c.RefURL = fmt.Sprintf("%s/%s/%d", refIssue.Repo.Link(), pullOrIssue, refIssue.Index) + c.RefExists = true + } + } else if c.Type == CommentTypeCommitRef { + if strings.HasPrefix(c.Content, ``) + contentParts := strings.SplitN(content, `">`, 2) + + if len(contentParts) == 2 { + c.RefURL = contentParts[0] + c.RefMessage = contentParts[1] + c.RefExists = true + } + } else { + // this is a new style commit ref + contentParts := strings.SplitN(c.Content, " ", 2) + if len(contentParts) == 2 { + repoID := int64(0) + n, err := fmt.Sscanf(contentParts[0], "%d", &repoID) + if err != nil { + return err + } + + if n == 1 { + refRepo, err := GetRepositoryByID(repoID) + if err != nil { + return err + } + + gitRepo, err := git.OpenRepository(refRepo.RepoPath()) + if err != nil { + return err + } + + refCommit, err := gitRepo.GetCommit(contentParts[1][:40]) + if err != nil { + return err + } + + c.RefURL = fmt.Sprintf("%s/commit/%s", refRepo.Link(), refCommit.ID.String()) + c.RefMessage = refCommit.CommitMessage + c.RefExists = true + } + } + } + } else if c.Type == CommentTypeCommentRef { + commentID := int64(0) + n, err := fmt.Sscanf(c.Content, "%d", &commentID) + if err != nil { + return err + } + + if n == 1 { + refComment, err := GetCommentByID(commentID) + if err != nil { + return err + } + + refIssue, err := GetIssueByID(refComment.IssueID) + if err != nil { + return err + } + + pullOrIssue := "issues" + if refIssue.IsPull { + pullOrIssue = "pulls" + } + + c.RefIssue = refIssue + c.RefComment = refComment + c.RefURL = fmt.Sprintf("%s/%s/%d#%s", refIssue.Repo.Link(), pullOrIssue, refIssue.Index, refComment.HashTag()) + c.RefExists = true + } + } + return nil +} + // LoadLabel if comment.Type is CommentTypeLabel, then load Label func (c *Comment) LoadLabel() error { var label Label From 96dd2686b1c2e80eca1812af11486c1d269538bb Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Mon, 2 Apr 2018 10:43:38 -0400 Subject: [PATCH 13/24] Hook up frontend templates to render reference comments Commit features: - renders reference comments only if their source still exists (except for old style commit refs) - allows for more complex rendering options, e.g. issue or commit status, quoted comments, etc. - properly handles updates to issue/PR titles, movement of repositories, etc. NOTE: Only the en_US locale is provided by this commit. --- options/locale/locale_en-US.ini | 3 ++ routers/repo/issue.go | 4 ++ .../repo/issue/view_content/comments.tmpl | 43 ++++++++++++++++++- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 2420bd66f..27c2177a2 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -721,7 +721,10 @@ issues.reopen_comment_issue = Comment and Reopen issues.create_comment = Comment issues.closed_at = `closed %[2]s` issues.reopened_at = `reopened %[2]s` +issues.issue_ref_at = `referenced this issue from an issue %[2]s` issues.commit_ref_at = `referenced this issue from a commit %[2]s` +issues.comment_ref_at = `referenced this issue from a comment %[2]s` +issues.pull_ref_at = `referenced this issue from a pull request %[2]s` issues.poster = Poster issues.collaborator = Collaborator issues.owner = Owner diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 18ab1691c..ff9ccea91 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -696,6 +696,10 @@ func ViewIssue(ctx *context.Context) { if !isAdded && !issue.IsPoster(comment.Poster.ID) { participants = append(participants, comment.Poster) } + } else if comment.Type == models.CommentTypeIssueRef || comment.Type == models.CommentTypeCommitRef || comment.Type == models.CommentTypeCommentRef || comment.Type == models.CommentTypePullRef { + if err = comment.LoadReference(); err != nil { + continue + } } else if comment.Type == models.CommentTypeLabel { if err = comment.LoadLabel(); err != nil { ctx.ServerError("LoadLabel", err) diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 4f185e4ae..40e6c43c4 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -82,7 +82,20 @@ {{.Poster.Name}} {{$.i18n.Tr "repo.issues.closed_at" .EventTag $createdStr | Safe}} - {{else if eq .Type 4}} + {{else if and (eq .Type 3) .RefExists}} +
+ + + + + {{.Poster.Name}} {{$.i18n.Tr "repo.issues.issue_ref_at" .EventTag $createdStr | Safe}} + +
+ + {{.RefIssue.Title | Escape}} (#{{.RefIssue.Index}}) +
+
+ {{else if and (eq .Type 4) .RefExists}}
@@ -92,7 +105,33 @@
- {{.Content | Str2html}} + {{.RefMessage | Escape}} ({{ShortSha .CommitSHA}}) +
+
+ {{else if and (eq .Type 5) .RefExists}} +
+ + + + + {{.Poster.Name}} {{$.i18n.Tr "repo.issues.comment_ref_at" .EventTag $createdStr | Safe}} + +
+ + {{.RefIssue.Title | Escape}} (#{{.RefIssue.Index}}) +
+
+ {{else if and (eq .Type 6) .RefExists}} +
+ + + + + {{.Poster.Name}} {{$.i18n.Tr "repo.issues.pull_ref_at" .EventTag $createdStr | Safe}} + +
+ + {{.RefIssue.Title | Escape}} (#{{.RefIssue.Index}})
{{else if eq .Type 7}} From b216bcf335b19662bc7c242981ed3af53408c3fe Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Mon, 2 Apr 2018 10:48:14 -0400 Subject: [PATCH 14/24] Change commit ref comments to use a more parse-able form of content --- models/action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/action.go b/models/action.go index 57f0a01e0..740bcd0d0 100644 --- a/models/action.go +++ b/models/action.go @@ -562,7 +562,7 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, com } if (mask & KeywordsFoundReference) == KeywordsFoundReference { - message := fmt.Sprintf(`%s`, repo.Link(), c.Sha1, c.Message) + message := fmt.Sprintf("%d %s", repo.ID, c.Sha1) if err = CreateCommitRefComment(doer, repo, issue, message, c.Sha1); err != nil { return err } From 0b6e2cd8cc3ae9933d272d0d2749934e2d9803c7 Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Sat, 7 Apr 2018 10:33:51 -0400 Subject: [PATCH 15/24] Applied suggested code changes * better commented KeywordsFound bitmask constants * populate issueKeywordsToFind variable at point of definition rather than in init() * add needed `xorm:"-"` statements to the fields that were added to Comment * also move calculation of uniqueID out of loop --- models/action.go | 32 ++++++++++++++------------------ models/issue_comment.go | 10 +++++----- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/models/action.go b/models/action.go index 740bcd0d0..0b374bb27 100644 --- a/models/action.go +++ b/models/action.go @@ -54,9 +54,9 @@ type KeywordsFoundMaskType int // Possible bitmask types for keywords that can be found. const ( - KeywordsFoundReference KeywordsFoundMaskType = 1 << iota // 1 - KeywordsFoundReopen // 2 - KeywordsFoundClose // 4 + KeywordsFoundReference KeywordsFoundMaskType = 1 << iota // 1 = 1 << 0 + KeywordsFoundReopen // 2 = 1 << 1 + KeywordsFoundClose // 4 = 1 << 2 ) // IssueKeywordsToFind represents a pairing of a pattern to use to find keywords in message and the keywords bitmask value. @@ -71,16 +71,6 @@ var ( issueCloseKeywords = []string{"close", "closes", "closed", "fix", "fixes", "fixed", "resolve", "resolves", "resolved"} issueReopenKeywords = []string{"reopen", "reopens", "reopened"} - issueKeywordsToFind []*IssueKeywordsToFind -) - -const issueRefRegexpStr = `(?:\S+/\S=)?#\d+` - -func assembleKeywordsPattern(words []string) string { - return fmt.Sprintf(`(?i)(?:%s) %s`, strings.Join(words, "|"), issueRefRegexpStr) -} - -func init() { // populate with details to find keywords for reference, reopen, close issueKeywordsToFind = []*IssueKeywordsToFind{ { @@ -96,6 +86,12 @@ func init() { KeywordsFoundMask: KeywordsFoundClose, }, } +) + +const issueRefRegexpStr = `(?:\S+/\S=)?#\d+` + +func assembleKeywordsPattern(words []string) string { + return fmt.Sprintf(`(?i)(?:%s) %s`, strings.Join(words, "|"), issueRefRegexpStr) } // Action represents user operation type and other information to @@ -489,6 +485,11 @@ func UpdateIssuesComment(doer *User, repo *Repository, commentIssue *Issue, comm refString = commentIssue.Title + ": " + commentIssue.Content } + uniqueID := fmt.Sprintf("%d", commentIssue.ID) + if comment != nil { + uniqueID += fmt.Sprintf("@%d", comment.ID) + } + refs, err := findIssueReferencesInString(refString, repo) if err != nil { return err @@ -504,11 +505,6 @@ func UpdateIssuesComment(doer *User, repo *Repository, commentIssue *Issue, comm } if (mask & KeywordsFoundReference) == KeywordsFoundReference { - uniqueID := fmt.Sprintf("%d", commentIssue.ID) - if comment != nil { - uniqueID += fmt.Sprintf("@%d", comment.ID) - } - if comment != nil { err = CreateCommentRefComment(doer, repo, issue, fmt.Sprintf(`%d`, comment.ID), base.EncodeSha1(uniqueID)) } else if commentIssue.IsPull { diff --git a/models/issue_comment.go b/models/issue_comment.go index d337fcedf..f52c47c40 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -109,11 +109,11 @@ type Comment struct { UpdatedUnix util.TimeStamp `xorm:"INDEX updated"` // Reference issue in commit message, comments, issues, or pull requests - RefExists bool - RefIssue *Issue - RefComment *Comment - RefMessage string - RefURL string + RefExists bool `xorm:"-"` + RefIssue *Issue `xorm:"-"` + RefComment *Comment `xorm:"-"` + RefMessage string `xorm:"-"` + RefURL string `xorm:"-"` // the commit SHA for commit refs otherwise a SHA of a unique reference identifier CommitSHA string `xorm:"VARCHAR(40)"` From e26048b685ea7e510e54fe64b940be245e317866 Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Sat, 7 Apr 2018 10:45:49 -0400 Subject: [PATCH 16/24] Improve code coverage of TestUpdateIssuesCommit --- models/action_test.go | 409 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 364 insertions(+), 45 deletions(-) diff --git a/models/action_test.go b/models/action_test.go index 519cba0b5..0f9d4d136 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -7,6 +7,7 @@ import ( "testing" "code.gitea.io/git" + "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/setting" "github.com/stretchr/testify/assert" @@ -185,52 +186,370 @@ func Test_getIssueFromRef(t *testing.T) { } func TestUpdateIssuesCommit(t *testing.T) { - assert.NoError(t, PrepareTestDatabase()) - pushCommits := []*PushCommit{ - { - Sha1: "abcdef1", - CommitterEmail: "user2@example.com", - CommitterName: "User Two", - AuthorEmail: "user4@example.com", - AuthorName: "User Four", - Message: "start working on #FST-1, #1", - }, - { - Sha1: "abcdef2", - CommitterEmail: "user2@example.com", - CommitterName: "User Two", - AuthorEmail: "user2@example.com", - AuthorName: "User Two", - Message: "a plain message", - }, - { - Sha1: "abcdef2", - CommitterEmail: "user2@example.com", - CommitterName: "User Two", - AuthorEmail: "user2@example.com", - AuthorName: "User Two", - Message: "close #2", - }, + for _, commitsAreMerged := range []bool{false, true} { + // if commits were not merged then issue should not change status + isOpen := "is_closed!=1" + isClosed := "is_closed=1" + if !commitsAreMerged { + isClosed = isOpen + } + + assert.NoError(t, PrepareTestDatabase()) + user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) + repo.Owner = user + + // test re-open of already open issue + pushCommits := []*PushCommit{ + { + Sha1: "abcdef1", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "reoopen #2", + }, + } + commentBean := []*Comment{ + { + Type: CommentTypeCommitRef, + CommitSHA: pushCommits[0].Sha1, + PosterID: user.ID, + IssueID: 2, + }, + } + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + CheckConsistencyFor(t, &Action{}) + + // test simultaneous open and close on an already open issue + pushCommits = []*PushCommit{ + { + Sha1: "abcdef2", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "reopen #2 and the close #2", + }, + } + commentBean = []*Comment{ + { + Type: CommentTypeCommitRef, + CommitSHA: pushCommits[0].Sha1, + PosterID: user.ID, + IssueID: 2, + }, + } + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + CheckConsistencyFor(t, &Action{}) + + // test close of an open issue + pushCommits = []*PushCommit{ + { + Sha1: "abcdef3", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "closes #2", + }, + } + commentBean = []*Comment{ + { + Type: CommentTypeCommitRef, + CommitSHA: pushCommits[0].Sha1, + PosterID: user.ID, + IssueID: 2, + }, + } + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isClosed) + CheckConsistencyFor(t, &Action{}) + + // test close of an already closed issue + pushCommits = []*PushCommit{ + { + Sha1: "abcdef4", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "close #2", + }, + } + commentBean = []*Comment{ + { + Type: CommentTypeCommitRef, + CommitSHA: pushCommits[0].Sha1, + PosterID: user.ID, + IssueID: 2, + }, + } + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isClosed) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isClosed) + CheckConsistencyFor(t, &Action{}) + + // test simultaneous open and close on a closed issue + pushCommits = []*PushCommit{ + { + Sha1: "abcdef5", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "close #2 and reopen #2", + }, + } + commentBean = []*Comment{ + { + Type: CommentTypeCommitRef, + CommitSHA: pushCommits[0].Sha1, + PosterID: user.ID, + IssueID: 2, + }, + } + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isClosed) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isClosed) + CheckConsistencyFor(t, &Action{}) + + // test referencing an closed issue + pushCommits = []*PushCommit{ + { + Sha1: "abcdef6", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "for details on how to open, see #2", + }, + } + commentBean = []*Comment{ + { + Type: CommentTypeCommitRef, + CommitSHA: pushCommits[0].Sha1, + PosterID: user.ID, + IssueID: 2, + }, + } + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isClosed) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isClosed) + CheckConsistencyFor(t, &Action{}) + + // test re-open a closed issue + pushCommits = []*PushCommit{ + { + Sha1: "abcdef7", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "reopens #2", + }, + } + commentBean = []*Comment{ + { + Type: CommentTypeCommitRef, + CommitSHA: pushCommits[0].Sha1, + PosterID: user.ID, + IssueID: 2, + }, + } + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isClosed) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + CheckConsistencyFor(t, &Action{}) + + // test referencing an open issue + pushCommits = []*PushCommit{ + { + Sha1: "abcdef8", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "for details on how to close, see #2", + }, + } + commentBean = []*Comment{ + { + Type: CommentTypeCommitRef, + CommitSHA: pushCommits[0].Sha1, + PosterID: user.ID, + IssueID: 2, + }, + } + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + CheckConsistencyFor(t, &Action{}) + + // test close-then-open commit order + pushCommits = []*PushCommit{ + { + Sha1: "abcdef10", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "reopened #2", + }, + { + Sha1: "abcdef9", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "fixes #2", + }, + } + commentBean = []*Comment{ + { + Type: CommentTypeCommitRef, + CommitSHA: pushCommits[0].Sha1, + PosterID: user.ID, + IssueID: 2, + }, + { + Type: CommentTypeCommitRef, + CommitSHA: pushCommits[1].Sha1, + PosterID: user.ID, + IssueID: 2, + }, + } + AssertNotExistsBean(t, commentBean[0]) + AssertNotExistsBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + CheckConsistencyFor(t, &Action{}) + + // test open-then-close commit order + pushCommits = []*PushCommit{ + { + Sha1: "abcdef12", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "resolved #2", + }, + { + Sha1: "abcdef11", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "reopened #2", + }, + } + commentBean = []*Comment{ + { + Type: CommentTypeCommitRef, + CommitSHA: pushCommits[0].Sha1, + PosterID: user.ID, + IssueID: 2, + }, + { + Type: CommentTypeCommitRef, + CommitSHA: pushCommits[1].Sha1, + PosterID: user.ID, + IssueID: 2, + }, + } + AssertNotExistsBean(t, commentBean[0]) + AssertNotExistsBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isClosed) + CheckConsistencyFor(t, &Action{}) + + // test more complex commit pattern + pushCommits = []*PushCommit{ + { + Sha1: "abcdef15", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user4@example.com", + AuthorName: "User Four", + Message: "start working on #FST-1, #1", + }, + { + Sha1: "abcdef14", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "reopen #2", + }, + { + Sha1: "abcdef13", + CommitterEmail: "user2@example.com", + CommitterName: "User Two", + AuthorEmail: "user2@example.com", + AuthorName: "User Two", + Message: "close #2", + }, + } + commentBean = []*Comment{ + { + Type: CommentTypeCommitRef, + CommitSHA: pushCommits[0].Sha1, + PosterID: user.ID, + IssueID: 1, + }, + { + Type: CommentTypeCommitRef, + CommitSHA: pushCommits[1].Sha1, + PosterID: user.ID, + IssueID: 2, + }, + { + Type: CommentTypeCommitRef, + CommitSHA: pushCommits[2].Sha1, + PosterID: user.ID, + IssueID: 2, + }, + } + + AssertNotExistsBean(t, commentBean[0]) + AssertNotExistsBean(t, commentBean[1]) + AssertNotExistsBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isClosed) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + CheckConsistencyFor(t, &Action{}) } - - user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) - repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) - repo.Owner = user - - commentBean := &Comment{ - Type: CommentTypeCommitRef, - CommitSHA: "abcdef1", - PosterID: user.ID, - IssueID: 1, - } - issueBean := &Issue{RepoID: repo.ID, Index: 2} - - AssertNotExistsBean(t, commentBean) - AssertNotExistsBean(t, &Issue{RepoID: repo.ID, Index: 2}, "is_closed=1") - assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, true)) - AssertExistsAndLoadBean(t, commentBean) - AssertExistsAndLoadBean(t, issueBean, "is_closed=1") - CheckConsistencyFor(t, &Action{}) } func testCorrectRepoAction(t *testing.T, opts CommitRepoActionOptions, actionBean *Action) { From c65d62825dec2c7acd9ef88ef55f93ee78f9b429 Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Sat, 7 Apr 2018 11:12:12 -0400 Subject: [PATCH 17/24] Add test for referencing issues in comments --- models/action_test.go | 72 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/models/action_test.go b/models/action_test.go index 0f9d4d136..7ab4141a6 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -185,6 +185,78 @@ func Test_getIssueFromRef(t *testing.T) { } } +func TestUpdateIssuesCommentComments(t *testing.T) { + isOpen := "is_closed!=1" + + assert.NoError(t, PrepareTestDatabase()) + user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) + repo.Owner = user + + commentIssue := AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}).(*Issue) + refIssue1 := AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}).(*Issue) + refIssue2 := AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 3}).(*Issue) + + comment := Comment{ + ID: 123456789, + Type: CommentTypeComment, + PosterID: user.ID, + Poster: user, + IssueID: commentIssue.ID, + Content: "this is a comment that mentions #2 and #1 too", + } + + commentBean := []*Comment{ + { + Type: CommentTypeCommentRef, + CommitSHA: base.EncodeSha1(fmt.Sprintf("%d@%d", commentIssue.ID, comment.ID)), + PosterID: user.ID, + IssueID: commentIssue.ID, + }, + { + Type: CommentTypeCommentRef, + CommitSHA: base.EncodeSha1(fmt.Sprintf("%d@%d", commentIssue.ID, comment.ID)), + PosterID: user.ID, + IssueID: refIssue1.ID, + }, + { + Type: CommentTypeCommentRef, + CommitSHA: base.EncodeSha1(fmt.Sprintf("%d@%d", commentIssue.ID, comment.ID)), + PosterID: user.ID, + IssueID: refIssue2.ID, + }, + } + + // test comment referencing issue including self-referencing + AssertNotExistsBean(t, commentBean[0]) + AssertNotExistsBean(t, commentBean[1]) + AssertNotExistsBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 3}, isOpen) + assert.NoError(t, UpdateIssuesComment(user, repo, commentIssue, &comment, false)) + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertNotExistsBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 3}, isOpen) + CheckConsistencyFor(t, &Action{}) + + // test comment updating issue reference + comment.Content = "this is a comment that mentions #2 and #3 too" + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertNotExistsBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 3}, isOpen) + assert.NoError(t, UpdateIssuesComment(user, repo, commentIssue, &comment, false)) + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 3}, isOpen) + CheckConsistencyFor(t, &Action{}) +} + func TestUpdateIssuesCommit(t *testing.T) { for _, commitsAreMerged := range []bool{false, true} { // if commits were not merged then issue should not change status From 3b0c1240a5551a3d69ff6f2a064c8d16abcf83b5 Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Sat, 7 Apr 2018 11:13:32 -0400 Subject: [PATCH 18/24] Add test for updating issues from issues --- models/action_test.go | 87 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/models/action_test.go b/models/action_test.go index 7ab4141a6..14d116874 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -185,6 +185,93 @@ func Test_getIssueFromRef(t *testing.T) { } } +func TestUpdateIssuesCommentIssues(t *testing.T) { + for _, canOpenClose := range []bool{false, true} { + // if cannot open or close then issue should not change status + isOpen := "is_closed!=1" + isClosed := "is_closed=1" + if !canOpenClose { + isClosed = isOpen + } + + assert.NoError(t, PrepareTestDatabase()) + user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) + repo.Owner = user + + commentIssue := AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}).(*Issue) + refIssue1 := AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}).(*Issue) + refIssue2 := AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 3}).(*Issue) + + commentBean := []*Comment{ + { + Type: CommentTypeIssueRef, + CommitSHA: base.EncodeSha1(fmt.Sprintf("%d", commentIssue.ID)), + PosterID: user.ID, + IssueID: commentIssue.ID, + }, + { + Type: CommentTypeIssueRef, + CommitSHA: base.EncodeSha1(fmt.Sprintf("%d", commentIssue.ID)), + PosterID: user.ID, + IssueID: refIssue1.ID, + }, + { + Type: CommentTypeIssueRef, + CommitSHA: base.EncodeSha1(fmt.Sprintf("%d", commentIssue.ID)), + PosterID: user.ID, + IssueID: refIssue2.ID, + }, + } + + // test issue/pull request closing multiple issues + commentIssue.Title = "close #2" + commentIssue.Content = "close #3" + AssertNotExistsBean(t, commentBean[0]) + AssertNotExistsBean(t, commentBean[1]) + AssertNotExistsBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 3}, isOpen) + assert.NoError(t, UpdateIssuesComment(user, repo, commentIssue, nil, canOpenClose)) + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isClosed) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 3}, isClosed) + CheckConsistencyFor(t, &Action{}) + + // test issue/pull request re-opening multiple issues + commentIssue.Title = "reopen #2" + commentIssue.Content = "reopen #3" + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isClosed) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 3}, isClosed) + assert.NoError(t, UpdateIssuesComment(user, repo, commentIssue, nil, canOpenClose)) + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 3}, isOpen) + CheckConsistencyFor(t, &Action{}) + + // test issue/pull request mixing re-opening and closing issue and self-referencing issue + commentIssue.Title = "reopen #2" + commentIssue.Content = "close #2 and reference #1" + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + assert.NoError(t, UpdateIssuesComment(user, repo, commentIssue, nil, canOpenClose)) + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}, isOpen) + CheckConsistencyFor(t, &Action{}) + } +} + func TestUpdateIssuesCommentComments(t *testing.T) { isOpen := "is_closed!=1" From e3ef3b5c7da5f592ed00e11ebcd145aac3a23dfd Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Sat, 7 Apr 2018 11:17:20 -0400 Subject: [PATCH 19/24] Update TestCreateComment to check referenced issue --- models/issue_comment_test.go | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/models/issue_comment_test.go b/models/issue_comment_test.go index f6a6fbce9..67bd17dc0 100644 --- a/models/issue_comment_test.go +++ b/models/issue_comment_test.go @@ -14,9 +14,27 @@ import ( func TestCreateComment(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) - issue := AssertExistsAndLoadBean(t, &Issue{}).(*Issue) - repo := AssertExistsAndLoadBean(t, &Repository{ID: issue.RepoID}).(*Repository) - doer := AssertExistsAndLoadBean(t, &User{ID: repo.OwnerID}).(*User) + doer := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) + repo.Owner = doer + + issue := AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}).(*Issue) + refIssue := AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}).(*Issue) + + commentBean := []*Comment{ + { + Type: CommentTypeCommentRef, + PosterID: doer.ID, + IssueID: issue.ID, + }, + { + Type: CommentTypeCommentRef, + PosterID: doer.ID, + IssueID: refIssue.ID, + }, + } + AssertNotExistsBean(t, commentBean[0]) + AssertNotExistsBean(t, commentBean[1]) now := time.Now().Unix() comment, err := CreateComment(&CreateCommentOptions{ @@ -24,18 +42,26 @@ func TestCreateComment(t *testing.T) { Doer: doer, Repo: repo, Issue: issue, - Content: "Hello", + Content: "Hello, this comment references issue #2", }) assert.NoError(t, err) then := time.Now().Unix() assert.EqualValues(t, CommentTypeComment, comment.Type) - assert.EqualValues(t, "Hello", comment.Content) + assert.EqualValues(t, "Hello, this comment references issue #2", comment.Content) assert.EqualValues(t, issue.ID, comment.IssueID) assert.EqualValues(t, doer.ID, comment.PosterID) AssertInt64InRange(t, now, then, int64(comment.CreatedUnix)) AssertExistsAndLoadBean(t, comment) // assert actually added to DB + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) updatedIssue := AssertExistsAndLoadBean(t, &Issue{ID: issue.ID}).(*Issue) AssertInt64InRange(t, now, then, int64(updatedIssue.UpdatedUnix)) + + err = commentBean[1].LoadReference() + assert.NoError(t, err) + if assert.NotNil(t, commentBean[1].RefIssue) { + assert.EqualValues(t, issue.ID, commentBean[1].RefIssue.ID) + } } From 7e6475ac1e2377da264621a6da5f4a8096936a08 Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Wed, 23 May 2018 11:37:15 -0400 Subject: [PATCH 20/24] Correct error message --- models/action.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/action.go b/models/action.go index 0b374bb27..b7efcb9be 100644 --- a/models/action.go +++ b/models/action.go @@ -802,7 +802,7 @@ func MergePullRequestAction(doer *User, repo *Repository, pull *Issue, commits * } if err := UpdateIssuesComment(doer, repo, pull, nil, true); err != nil { - log.Error(4, "UpdateIssuesCommit: %v", err) + log.Error(4, "UpdateIssuesComment: %v", err) } if err := notifyWatchers(x, &Action{ @@ -827,7 +827,7 @@ func NewPullRequestAction(doer *User, repo *Repository, pull *Issue, commits *Pu } if err := UpdateIssuesComment(doer, repo, pull, nil, false); err != nil { - log.Error(4, "UpdateIssuesCommit: %v", err) + log.Error(4, "UpdateIssuesComment: %v", err) } if err := NotifyWatchers(&Action{ From 6c1a73a06d2a53c529b37c52a7e65ed2571dca86 Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Fri, 25 May 2018 09:01:03 -0400 Subject: [PATCH 21/24] Trigger MergePullRequestAction when PR is manually merged --- models/action.go | 8 +++++--- models/pull.go | 5 +++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/models/action.go b/models/action.go index b7efcb9be..2b0df6979 100644 --- a/models/action.go +++ b/models/action.go @@ -795,10 +795,12 @@ func TransferRepoAction(doer, oldOwner *User, repo *Repository) error { return transferRepoAction(x, doer, oldOwner, repo) } -// MergePullRequestAction adds new action for merging pull request. +// MergePullRequestAction adds new action for merging pull request (including manually merged pull requests). func MergePullRequestAction(doer *User, repo *Repository, pull *Issue, commits *PushCommits) error { - if err := UpdateIssuesCommit(doer, repo, commits.Commits, true); err != nil { - log.Error(4, "UpdateIssuesCommit: %v", err) + if commits != nil { + if err := UpdateIssuesCommit(doer, repo, commits.Commits, true); err != nil { + log.Error(4, "UpdateIssuesCommit: %v", err) + } } if err := UpdateIssuesComment(doer, repo, pull, nil, true); err != nil { diff --git a/models/pull.go b/models/pull.go index d87bf4bc2..81e1d3cd8 100644 --- a/models/pull.go +++ b/models/pull.go @@ -581,6 +581,11 @@ func (pr *PullRequest) manuallyMerged() bool { return false } log.Info("manuallyMerged[%d]: Marked as manually merged into %s/%s by commit id: %s", pr.ID, pr.BaseRepo.Name, pr.BaseBranch, commit.ID.String()) + + if err = MergePullRequestAction(pr.Merger, pr.Issue.Repo, pr.Issue, nil); err != nil { + log.Error(4, "MergePullRequestAction [%d]: %v", pr.ID, err) + } + return true } return false From 3be891189730b1f115c0a1a40a78a59eaa609ff2 Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Fri, 6 Jul 2018 08:20:25 -0400 Subject: [PATCH 22/24] Change variable names from KeywordsFound* to Keyword* --- models/action.go | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/models/action.go b/models/action.go index 2b0df6979..2e50a8951 100644 --- a/models/action.go +++ b/models/action.go @@ -49,20 +49,20 @@ const ( ActionDeleteBranch // 17 ) -// KeywordsFoundMaskType represents the bitmask of types of keywords found in a message. -type KeywordsFoundMaskType int +// KeywordMaskType represents the bitmask of types of keywords found in a message. +type KeywordMaskType int // Possible bitmask types for keywords that can be found. const ( - KeywordsFoundReference KeywordsFoundMaskType = 1 << iota // 1 = 1 << 0 - KeywordsFoundReopen // 2 = 1 << 1 - KeywordsFoundClose // 4 = 1 << 2 + KeywordReference KeywordMaskType = 1 << iota // 1 = 1 << 0 + KeywordReopen // 2 = 1 << 1 + KeywordClose // 4 = 1 << 2 ) // IssueKeywordsToFind represents a pairing of a pattern to use to find keywords in message and the keywords bitmask value. type IssueKeywordsToFind struct { - Pattern *regexp.Regexp - KeywordsFoundMask KeywordsFoundMaskType + Pattern *regexp.Regexp + KeywordMask KeywordMaskType } var ( @@ -74,16 +74,16 @@ var ( // populate with details to find keywords for reference, reopen, close issueKeywordsToFind = []*IssueKeywordsToFind{ { - Pattern: regexp.MustCompile(issueRefRegexpStr), - KeywordsFoundMask: KeywordsFoundReference, + Pattern: regexp.MustCompile(issueRefRegexpStr), + KeywordMask: KeywordReference, }, { - Pattern: regexp.MustCompile(assembleKeywordsPattern(issueReopenKeywords)), - KeywordsFoundMask: KeywordsFoundReopen, + Pattern: regexp.MustCompile(assembleKeywordsPattern(issueReopenKeywords)), + KeywordMask: KeywordReopen, }, { - Pattern: regexp.MustCompile(assembleKeywordsPattern(issueCloseKeywords)), - KeywordsFoundMask: KeywordsFoundClose, + Pattern: regexp.MustCompile(assembleKeywordsPattern(issueCloseKeywords)), + KeywordMask: KeywordClose, }, } ) @@ -459,8 +459,8 @@ func getIssueFromRef(repo *Repository, ref string) (*Issue, error) { } // findIssueReferencesInString iterates over the keywords to find in a message and accumulates the findings into refs -func findIssueReferencesInString(message string, repo *Repository) (map[int64]KeywordsFoundMaskType, error) { - refs := make(map[int64]KeywordsFoundMaskType) +func findIssueReferencesInString(message string, repo *Repository) (map[int64]KeywordMaskType, error) { + refs := make(map[int64]KeywordMaskType) for _, kwToFind := range issueKeywordsToFind { for _, ref := range kwToFind.Pattern.FindAllString(message, -1) { issue, err := getIssueFromRef(repo, ref) @@ -469,7 +469,7 @@ func findIssueReferencesInString(message string, repo *Repository) (map[int64]Ke } if issue != nil { - refs[issue.ID] |= kwToFind.KeywordsFoundMask + refs[issue.ID] |= kwToFind.KeywordMask } } } @@ -504,7 +504,7 @@ func UpdateIssuesComment(doer *User, repo *Repository, commentIssue *Issue, comm continue } - if (mask & KeywordsFoundReference) == KeywordsFoundReference { + if (mask & KeywordReference) == KeywordReference { if comment != nil { err = CreateCommentRefComment(doer, repo, issue, fmt.Sprintf(`%d`, comment.ID), base.EncodeSha1(uniqueID)) } else if commentIssue.IsPull { @@ -518,14 +518,14 @@ func UpdateIssuesComment(doer *User, repo *Repository, commentIssue *Issue, comm } if canOpenClose { - // take no action if both KeywordsFoundClose and KeywordsFoundOpen are set - if (mask & (KeywordsFoundReopen | KeywordsFoundClose)) == KeywordsFoundClose { + // take no action if both KeywordClose and KeywordOpen are set + if (mask & (KeywordReopen | KeywordClose)) == KeywordClose { if issue.RepoID == repo.ID && !issue.IsClosed { if err = issue.ChangeStatus(doer, repo, true); err != nil { return err } } - } else if (mask & (KeywordsFoundReopen | KeywordsFoundClose)) == KeywordsFoundReopen { + } else if (mask & (KeywordReopen | KeywordClose)) == KeywordReopen { if issue.RepoID == repo.ID && issue.IsClosed { if err = issue.ChangeStatus(doer, repo, false); err != nil { return err @@ -557,7 +557,7 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, com continue } - if (mask & KeywordsFoundReference) == KeywordsFoundReference { + if (mask & KeywordReference) == KeywordReference { message := fmt.Sprintf("%d %s", repo.ID, c.Sha1) if err = CreateCommitRefComment(doer, repo, issue, message, c.Sha1); err != nil { return err @@ -565,14 +565,14 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, com } if commitsAreMerged { - // take no action if both KeywordsFoundClose and KeywordsFoundOpen are set - if (mask & (KeywordsFoundReopen | KeywordsFoundClose)) == KeywordsFoundClose { + // take no action if both KeywordClose and KeywordOpen are set + if (mask & (KeywordReopen | KeywordClose)) == KeywordClose { if issue.RepoID == repo.ID && !issue.IsClosed { if err = issue.ChangeStatus(doer, repo, true); err != nil { return err } } - } else if (mask & (KeywordsFoundReopen | KeywordsFoundClose)) == KeywordsFoundReopen { + } else if (mask & (KeywordReopen | KeywordClose)) == KeywordReopen { if issue.RepoID == repo.ID && issue.IsClosed { if err = issue.ChangeStatus(doer, repo, false); err != nil { return err From da4bba50ef8433397a7c2f239a97f766b5a37d21 Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Fri, 6 Jul 2018 08:31:13 -0400 Subject: [PATCH 23/24] Move common status changing logic into a function --- models/action.go | 50 +++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/models/action.go b/models/action.go index 2e50a8951..4b3ff5ee4 100644 --- a/models/action.go +++ b/models/action.go @@ -476,6 +476,26 @@ func findIssueReferencesInString(message string, repo *Repository) (map[int64]Ke return refs, nil } +// changeIssueStatus encapsulates the logic for changing the status of an issue based on what keywords are marked in the keyword mask +func changeIssueStatus(mask KeywordMaskType, doer *User, repo *Repository, issue *Issue) error { + // take no action if both KeywordClose and KeywordOpen are set + switch mask & (KeywordReopen | KeywordClose) { + case KeywordClose: + if issue.RepoID == repo.ID && !issue.IsClosed { + if err := issue.ChangeStatus(doer, repo, true); err != nil { + return err + } + } + case KeywordReopen: + if issue.RepoID == repo.ID && issue.IsClosed { + if err := issue.ChangeStatus(doer, repo, false); err != nil { + return err + } + } + } + return nil +} + // UpdateIssuesComment checks if issues are manipulated by a comment func UpdateIssuesComment(doer *User, repo *Repository, commentIssue *Issue, comment *Comment, canOpenClose bool) error { var refString string @@ -518,19 +538,8 @@ func UpdateIssuesComment(doer *User, repo *Repository, commentIssue *Issue, comm } if canOpenClose { - // take no action if both KeywordClose and KeywordOpen are set - if (mask & (KeywordReopen | KeywordClose)) == KeywordClose { - if issue.RepoID == repo.ID && !issue.IsClosed { - if err = issue.ChangeStatus(doer, repo, true); err != nil { - return err - } - } - } else if (mask & (KeywordReopen | KeywordClose)) == KeywordReopen { - if issue.RepoID == repo.ID && issue.IsClosed { - if err = issue.ChangeStatus(doer, repo, false); err != nil { - return err - } - } + if err = changeIssueStatus(mask, doer, repo, issue); err != nil { + return err } } } @@ -565,19 +574,8 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, com } if commitsAreMerged { - // take no action if both KeywordClose and KeywordOpen are set - if (mask & (KeywordReopen | KeywordClose)) == KeywordClose { - if issue.RepoID == repo.ID && !issue.IsClosed { - if err = issue.ChangeStatus(doer, repo, true); err != nil { - return err - } - } - } else if (mask & (KeywordReopen | KeywordClose)) == KeywordReopen { - if issue.RepoID == repo.ID && issue.IsClosed { - if err = issue.ChangeStatus(doer, repo, false); err != nil { - return err - } - } + if err = changeIssueStatus(mask, doer, repo, issue); err != nil { + return err } } } From 820a995d13225489618360da56dca4ef5de67fcc Mon Sep 17 00:00:00 2001 From: Keith Rutkowski Date: Fri, 6 Jul 2018 08:48:21 -0400 Subject: [PATCH 24/24] Change "x := int64(0)" statements to "var x int64" --- models/issue_comment.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index f52c47c40..bbcdecd70 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -235,7 +235,7 @@ func (c *Comment) EventTag() string { // LoadReference if comment.Type is CommentType{Issue,Commit,Comment,Pull}Ref, then load RefIssue, RefComment func (c *Comment) LoadReference() error { if c.Type == CommentTypeIssueRef || c.Type == CommentTypePullRef { - issueID := int64(0) + var issueID int64 n, err := fmt.Sscanf(c.Content, "%d", &issueID) if err != nil { return err @@ -271,7 +271,7 @@ func (c *Comment) LoadReference() error { // this is a new style commit ref contentParts := strings.SplitN(c.Content, " ", 2) if len(contentParts) == 2 { - repoID := int64(0) + var repoID int64 n, err := fmt.Sscanf(contentParts[0], "%d", &repoID) if err != nil { return err @@ -300,7 +300,7 @@ func (c *Comment) LoadReference() error { } } } else if c.Type == CommentTypeCommentRef { - commentID := int64(0) + var commentID int64 n, err := fmt.Sscanf(c.Content, "%d", &commentID) if err != nil { return err