diff --git a/models/action.go b/models/action.go index 9050be22a6403..17cb4dc2acf61 100644 --- a/models/action.go +++ b/models/action.go @@ -52,14 +52,43 @@ const ( ActionMirrorSyncDelete // 20 ) +// 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 ( + 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 + KeywordMask KeywordMaskType +} + 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 + // populate with details to find keywords for reference, reopen, close + issueKeywordsToFind = []*IssueKeywordsToFind{ + { + Pattern: regexp.MustCompile(issueRefRegexpStr), + KeywordMask: KeywordReference, + }, + { + Pattern: regexp.MustCompile(assembleKeywordsPattern(issueReopenKeywords)), + KeywordMask: KeywordReopen, + }, + { + Pattern: regexp.MustCompile(assembleKeywordsPattern(issueCloseKeywords)), + KeywordMask: KeywordClose, + }, + } ) const issueRefRegexpStr = `(?:\S+/\S=)?#\d+` @@ -68,12 +97,6 @@ func assembleKeywordsPattern(words []string) string { return fmt.Sprintf(`(?i)(?:%s) %s`, strings.Join(words, "|"), issueRefRegexpStr) } -func init() { - issueCloseKeywordsPat = regexp.MustCompile(assembleKeywordsPattern(issueCloseKeywords)) - issueReopenKeywordsPat = regexp.MustCompile(assembleKeywordsPattern(issueReopenKeywords)) - issueReferenceKeywordsPat = regexp.MustCompile(issueRefRegexpStr) -} - // Action represents user operation type and other information to // repository. It implemented interface base.Actioner so that can be // used in template render. @@ -438,74 +461,129 @@ func getIssueFromRef(repo *Repository, ref string) (*Issue, error) { return issue, 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) { +// findIssueReferencesInString iterates over the keywords to find in a message and accumulates the findings into refs +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) if err != nil { - return err + return nil, err } - if issue == nil || refMarked[issue.ID] { - continue + if issue != nil { + refs[issue.ID] |= kwToFind.KeywordMask } - refMarked[issue.ID] = true + } + } + return refs, nil +} - message := fmt.Sprintf(`%s`, repo.Link(), c.Sha1, c.Message) - if err = CreateRefComment(doer, repo, issue, message, c.Sha1); err != nil { - return err +// 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, issue *Issue) error { + // take no action if both KeywordClose and KeywordOpen are set + switch mask { + case KeywordClose: + if err := issue.ChangeStatus(doer, issue.Repo, true); err != nil { + // Don't return the error when dependencies are still open as this would cause a push, merge, etc. to fail + if IsErrDependenciesLeft(err) { + return nil } + return err } + case KeywordReopen: + if err := issue.ChangeStatus(doer, issue.Repo, false); err != nil { + return err + } + } + return nil +} - 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) +// UpdateIssuesComment checks if issues are manipulated by a regular comment, code comment, or review comment (also issue/pull request title and description) +func UpdateIssuesComment(doer *User, repo *Repository, commentIssue *Issue, comment *Comment, canOpenClose bool) error { + var refString string + if comment != nil { + if comment.Type != CommentTypeComment && comment.Type != CommentTypeCode && comment.Type != CommentTypeReview { + return 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 & KeywordReference) == KeywordReference { + if comment != nil && comment.Type == CommentTypeComment { + err = CreateCommentRefComment(doer, issue, commentIssue.ID, comment.ID) + } else if comment != nil && comment.Type == CommentTypeCode { + err = CreateCodeRefComment(doer, issue, commentIssue.ID, comment.ID) + } else if comment != nil && comment.Type == CommentTypeReview { + err = CreateReviewRefComment(doer, issue, commentIssue.ID, comment.ID) + } else if commentIssue.IsPull { + err = CreatePullRefComment(doer, issue, commentIssue.ID) + } else { + err = CreateIssueRefComment(doer, issue, commentIssue.ID) + } if err != nil { return err } + } - if issue == nil || refMarked[issue.ID] { - continue + // only change issue status if this is a pull request in the issue's repo + if canOpenClose && comment == nil && commentIssue.IsPull && repo.ID == issue.RepoID { + if err = changeIssueStatus(mask&(KeywordReopen|KeywordClose), doer, issue); err != nil { + return err } - refMarked[issue.ID] = true + } + } + return nil +} - if issue.RepoID != repo.ID || issue.IsClosed { - continue - } +// 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. + for i := len(commits) - 1; i >= 0; i-- { + c := commits[i] - if err = issue.ChangeStatus(doer, repo, true); 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 - } + refs, err := findIssueReferencesInString(c.Message, repo) + if 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) + for id, mask := range refs { + issue, err := GetIssueByID(id) if err != nil { return err } - - if issue == nil || refMarked[issue.ID] { + if issue == nil { continue } - refMarked[issue.ID] = true - if issue.RepoID != repo.ID || !issue.IsClosed { - continue + if (mask & KeywordReference) == KeywordReference { + if err = CreateCommitRefComment(doer, issue, repo.ID, c.Sha1); err != nil { + return err + } } - if err = issue.ChangeStatus(doer, repo, false); err != nil { - return err + // only change issue status if the commit is merged to the issue's repo + if commitsAreMerged && repo.ID == issue.RepoID { + if err = changeIssueStatus(mask&(KeywordReopen|KeywordClose), doer, issue); err != nil { + return err + } } } } @@ -569,8 +647,8 @@ func CommitRepoAction(opts CommitRepoActionOptions) error { opts.Commits.CompareURL = repo.ComposeCompareURL(opts.OldCommitID, opts.NewCommitID) } - if err = UpdateIssuesCommit(pusher, repo, opts.Commits.Commits); err != nil { - log.Error(4, "updateIssuesCommit: %v", err) + if err = UpdateIssuesCommit(pusher, repo, opts.Commits.Commits, true); err != nil { + log.Error(4, "UpdateIssuesCommit: %v", err) } } @@ -724,21 +802,91 @@ 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{ +// 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 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 { + log.Error(4, "UpdateIssuesComment: %v", err) + } + + if err := notifyWatchers(x, &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, - }) + }); 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) +// 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) + } + + if err := UpdateIssuesComment(doer, repo, pull, nil, false); err != nil { + log.Error(4, "UpdateIssuesComment: %v", err) + } + + 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 +} + +// 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) + } + + // 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 } func mirrorSyncAction(e Engine, opType ActionType, repo *Repository, refName string, data []byte) error { diff --git a/models/action_test.go b/models/action_test.go index d0e0a5d8fab1f..b962cdc2830ae 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" @@ -184,53 +185,415 @@ func Test_getIssueFromRef(t *testing.T) { } } -func TestUpdateIssuesCommit(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: 2}).(*Issue) + refIssue1 := AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}).(*Issue) + refIssue2 := AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 3}).(*Issue) + assert.EqualValues(t, true, commentIssue.IsPull) + assert.EqualValues(t, false, refIssue1.IsClosed) + assert.EqualValues(t, false, refIssue2.IsClosed) + + // TODO: comments are loaded and then this doesnt work... + commentBean := []*Comment{ + { + Type: CommentTypePullRef, + CommitSHA: base.EncodeSha1(fmt.Sprintf("%d", commentIssue.ID)), + PosterID: user.ID, + IssueID: commentIssue.ID, + }, + { + Type: CommentTypePullRef, + CommitSHA: base.EncodeSha1(fmt.Sprintf("%d", commentIssue.ID)), + PosterID: user.ID, + IssueID: refIssue1.ID, + }, + { + Type: CommentTypePullRef, + CommitSHA: base.EncodeSha1(fmt.Sprintf("%d", commentIssue.ID)), + PosterID: user.ID, + IssueID: refIssue2.ID, + }, + } + + // test issue/pull request closing multiple issues + commentIssue.Title = "close #1" + commentIssue.Content = "close #3" + AssertNotExistsBean(t, commentBean[0]) + AssertNotExistsBean(t, commentBean[1]) + AssertNotExistsBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, 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: 1}, isClosed) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 3}, isClosed) + CheckConsistencyFor(t, &Action{}) + + // test issue/pull request re-opening multiple issues + commentIssue.Title = "reopen #1" + commentIssue.Content = "reopen #3" + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, 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: 1}, 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 #3" + commentIssue.Content = "close #3 and reference #2" + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, commentBean[2]) + 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: 3}, isOpen) + CheckConsistencyFor(t, &Action{}) + } +} + +func TestUpdateIssuesCommentComments(t *testing.T) { + isOpen := "is_closed!=1" + assert.NoError(t, PrepareTestDatabase()) - pushCommits := []*PushCommit{ + 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: 2}).(*Issue) + refIssue1 := AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}).(*Issue) + refIssue2 := AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 3}).(*Issue) + assert.EqualValues(t, true, commentIssue.IsPull) + assert.EqualValues(t, false, refIssue1.IsClosed) + assert.EqualValues(t, false, refIssue2.IsClosed) + + comment := Comment{ + ID: 123456789, + Type: CommentTypeComment, + PosterID: user.ID, + Poster: user, + IssueID: commentIssue.ID, + Content: "", + } + + commentBean := []*Comment{ { - Sha1: "abcdef1", - CommitterEmail: "user2@example.com", - CommitterName: "User Two", - AuthorEmail: "user4@example.com", - AuthorName: "User Four", - Message: "start working on #FST-1, #1", + Type: CommentTypeCommentRef, + CommitSHA: base.EncodeSha1(fmt.Sprintf("%d", comment.ID)), + PosterID: user.ID, + IssueID: commentIssue.ID, }, { - Sha1: "abcdef2", - CommitterEmail: "user2@example.com", - CommitterName: "User Two", - AuthorEmail: "user2@example.com", - AuthorName: "User Two", - Message: "a plain message", + Type: CommentTypeCommentRef, + CommitSHA: base.EncodeSha1(fmt.Sprintf("%d", comment.ID)), + PosterID: user.ID, + IssueID: refIssue1.ID, }, { - Sha1: "abcdef2", + Type: CommentTypeCommentRef, + CommitSHA: base.EncodeSha1(fmt.Sprintf("%d", comment.ID)), + PosterID: user.ID, + IssueID: refIssue2.ID, + }, + } + + // test comment referencing issue including self-referencing + comment.Content = "this is a comment that mentions #1 and #2 too" + AssertNotExistsBean(t, commentBean[0]) + AssertNotExistsBean(t, commentBean[1]) + AssertNotExistsBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, 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: 1}, 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 #1 and #3 too" + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertNotExistsBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, 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: 1}, isOpen) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 3}, isOpen) + CheckConsistencyFor(t, &Action{}) +} + +type Message struct { + Message string + Sha1 string + IssueID int64 +} + +func preparePushCommits(userID int64, repoID int64, messages []*Message) ([]*PushCommit, []*Comment) { + var pushCommits []*PushCommit + var commentBean []*Comment + + for _, message := range messages { + pushCommits = append(pushCommits, &PushCommit{ + Sha1: message.Sha1, CommitterEmail: "user2@example.com", CommitterName: "User Two", AuthorEmail: "user2@example.com", AuthorName: "User Two", - Message: "close #2", - }, + Message: message.Message, + }) + commentBean = append(commentBean, &Comment{ + Type: CommentTypeCommitRef, + CommitSHA: base.EncodeSha1(fmt.Sprintf("%d %s", repoID, message.Sha1)), + PosterID: userID, + IssueID: message.IssueID, + }) } - user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) - repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) - repo.Owner = user + return pushCommits, commentBean +} - commentBean := &Comment{ - Type: CommentTypeCommitRef, - CommitSHA: "abcdef1", - PosterID: user.ID, - IssueID: 1, - } - issueBean := &Issue{RepoID: repo.ID, Index: 2} +func TestUpdateIssuesCommit(t *testing.T) { + 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 + } - AssertNotExistsBean(t, commentBean) - AssertNotExistsBean(t, &Issue{RepoID: repo.ID, Index: 2}, "is_closed=1") - assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits)) - AssertExistsAndLoadBean(t, commentBean) - AssertExistsAndLoadBean(t, issueBean, "is_closed=1") - CheckConsistencyFor(t, &Action{}) + assert.NoError(t, PrepareTestDatabase()) + user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) + repo.Owner = user + + commitIssue := AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 2}).(*Issue) + refIssue := AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}).(*Issue) + assert.EqualValues(t, true, commitIssue.IsPull) + assert.EqualValues(t, false, refIssue.IsClosed) + + // test re-open of already open issue + pushCommits, commentBean := preparePushCommits(user.ID, repo.ID, []*Message{ + { + Message: "reopen #1", + Sha1: "abcdef1", + IssueID: refIssue.ID, + }, + }) + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isOpen) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isOpen) + CheckConsistencyFor(t, &Action{}) + + // test simultaneous open and close on an already open issue + pushCommits, commentBean = preparePushCommits(user.ID, repo.ID, []*Message{ + { + Message: "reopen #1 and then close #1", + Sha1: "abcdef2", + IssueID: refIssue.ID, + }, + }) + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isOpen) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isOpen) + CheckConsistencyFor(t, &Action{}) + + // test close of an open issue + pushCommits, commentBean = preparePushCommits(user.ID, repo.ID, []*Message{ + { + Message: "closes #1", + Sha1: "abcdef3", + IssueID: refIssue.ID, + }, + }) + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isOpen) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isClosed) + CheckConsistencyFor(t, &Action{}) + + // test close of an already closed issue + pushCommits, commentBean = preparePushCommits(user.ID, repo.ID, []*Message{ + { + Message: "closes #1", + Sha1: "abcdef4", + IssueID: refIssue.ID, + }, + }) + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isClosed) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isClosed) + CheckConsistencyFor(t, &Action{}) + + // test simultaneous open and close on a closed issue + pushCommits, commentBean = preparePushCommits(user.ID, repo.ID, []*Message{ + { + Message: "close #1 and reopen #1", + Sha1: "abcdef5", + IssueID: refIssue.ID, + }, + }) + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isClosed) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isClosed) + CheckConsistencyFor(t, &Action{}) + + // test referencing an closed issue + pushCommits, commentBean = preparePushCommits(user.ID, repo.ID, []*Message{ + { + Message: "for details on how to open, see #1", + Sha1: "abcdef6", + IssueID: refIssue.ID, + }, + }) + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isClosed) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isClosed) + CheckConsistencyFor(t, &Action{}) + + // test re-open a closed issue + pushCommits, commentBean = preparePushCommits(user.ID, repo.ID, []*Message{ + { + Message: "reopens #1", + Sha1: "abcdef7", + IssueID: refIssue.ID, + }, + }) + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isClosed) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isOpen) + CheckConsistencyFor(t, &Action{}) + + // test referencing an open issue + pushCommits, commentBean = preparePushCommits(user.ID, repo.ID, []*Message{ + { + Message: "for details on how to close, see #1", + Sha1: "abcdef8", + IssueID: refIssue.ID, + }, + }) + AssertNotExistsBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isOpen) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isOpen) + CheckConsistencyFor(t, &Action{}) + + // test close-then-open commit order + pushCommits, commentBean = preparePushCommits(user.ID, repo.ID, []*Message{ + { + Message: "reopened #1", + Sha1: "abcdef10", + IssueID: refIssue.ID, + }, + { + Message: "fixes #1", + Sha1: "abcdef9", + IssueID: refIssue.ID, + }, + }) + AssertNotExistsBean(t, commentBean[0]) + AssertNotExistsBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isOpen) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isOpen) + CheckConsistencyFor(t, &Action{}) + + // test open-then-close commit order + pushCommits, commentBean = preparePushCommits(user.ID, repo.ID, []*Message{ + { + Message: "resolved #1", + Sha1: "abcdef12", + IssueID: refIssue.ID, + }, + { + Message: "reopened #1", + Sha1: "abcdef11", + IssueID: refIssue.ID, + }, + }) + AssertNotExistsBean(t, commentBean[0]) + AssertNotExistsBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isOpen) + assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, commitsAreMerged)) + AssertExistsAndLoadBean(t, commentBean[0]) + AssertExistsAndLoadBean(t, commentBean[1]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, isClosed) + CheckConsistencyFor(t, &Action{}) + + // test more complex commit pattern + pushCommits, commentBean = preparePushCommits(user.ID, repo.ID, []*Message{ + { + Message: "start working on #FST-1, #2", + Sha1: "abcdef15", + IssueID: commitIssue.ID, + }, + { + Message: "reopen #1", + Sha1: "abcdef14", + IssueID: refIssue.ID, + }, + { + Message: "close #1", + Sha1: "abcdef13", + IssueID: refIssue.ID, + }, + }) + AssertNotExistsBean(t, commentBean[0]) + AssertNotExistsBean(t, commentBean[1]) + AssertNotExistsBean(t, commentBean[2]) + AssertExistsAndLoadBean(t, &Issue{RepoID: repo.ID, Index: 1}, 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: 1}, isOpen) + CheckConsistencyFor(t, &Action{}) + } } func testCorrectRepoAction(t *testing.T, opts CommitRepoActionOptions, actionBean *Action) { @@ -379,6 +742,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 +753,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/issue.go b/models/issue.go index 88e96e5706092..cdde362a614b9 100644 --- a/models/issue.go +++ b/models/issue.go @@ -829,6 +829,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 } @@ -894,6 +898,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 } @@ -1068,6 +1076,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 96b162ca196b2..f52241c5dbc8d 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -11,17 +11,18 @@ import ( "fmt" "strings" - "code.gitea.io/git" - "code.gitea.io/gitea/modules/markup/markdown" - "code.gitea.io/gitea/modules/setting" "github.com/Unknwon/com" "github.com/go-xorm/builder" "github.com/go-xorm/xorm" api "code.gitea.io/sdk/gitea" + "code.gitea.io/git" + "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/markup/markdown" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" ) @@ -36,50 +37,54 @@ const ( // Enumerate all the comment types const ( // Plain comment, can be associated with a commit (CommitID > 0) and a line (LineNum > 0) - CommentTypeComment CommentType = iota - CommentTypeReopen - CommentTypeClose + CommentTypeComment CommentType = iota // 0 + CommentTypeReopen // 1 + CommentTypeClose // 2 // References. - CommentTypeIssueRef + CommentTypeIssueRef // 3 // Reference from a commit (not part of a pull request) - CommentTypeCommitRef + CommentTypeCommitRef // 4 // Reference from a comment - CommentTypeCommentRef + CommentTypeCommentRef // 5 // Reference from a pull request - CommentTypePullRef + CommentTypePullRef // 6 // Labels changed - CommentTypeLabel + CommentTypeLabel // 7 // Milestone changed - CommentTypeMilestone + CommentTypeMilestone // 8 // Assignees changed - CommentTypeAssignees + CommentTypeAssignees // 9 // Change Title - CommentTypeChangeTitle + CommentTypeChangeTitle // 10 // Delete Branch - CommentTypeDeleteBranch + CommentTypeDeleteBranch // 11 // Start a stopwatch for time tracking - CommentTypeStartTracking + CommentTypeStartTracking // 12 // Stop a stopwatch for time tracking - CommentTypeStopTracking + CommentTypeStopTracking // 13 // Add time manual for time tracking - CommentTypeAddTimeManual + CommentTypeAddTimeManual // 14 // Cancel a stopwatch for time tracking - CommentTypeCancelTracking + CommentTypeCancelTracking // 15 // Added a due date - CommentTypeAddedDeadline + CommentTypeAddedDeadline // 16 // Modified the due date - CommentTypeModifiedDeadline + CommentTypeModifiedDeadline // 17 // Removed a due date - CommentTypeRemovedDeadline + CommentTypeRemovedDeadline // 18 // Dependency added - CommentTypeAddDependency + CommentTypeAddDependency // 19 //Dependency removed - CommentTypeRemoveDependency + CommentTypeRemoveDependency // 20 // Comment a line of code - CommentTypeCode + CommentTypeCode // 21 // Reviews a pull request by giving general feedback - CommentTypeReview + CommentTypeReview // 22 + // Reference from a code review + CommentTypeReviewRef // 23 + // Reference from a code comment + CommentTypeCodeRef // 24 ) // CommentTag defines comment tag type @@ -127,7 +132,15 @@ type Comment struct { CreatedUnix util.TimeStamp `xorm:"INDEX created"` UpdatedUnix util.TimeStamp `xorm:"INDEX updated"` - // Reference issue in commit message + // Reference issue in commit, comments, issues, or pull requests + RefExists bool `xorm:"-"` + RefIssue *Issue `xorm:"-"` + RefComment *Comment `xorm:"-"` + RefCommitMessage string `xorm:"-"` + RefCommitShortSHA string `xorm:"-"` + RefURL string `xorm:"-"` + // CommentType*Ref types use this as a unique reference identifier to prevent duplicate references + // other types use it for the commit SHA CommitSHA string `xorm:"VARCHAR(40)"` Attachments []*Attachment `xorm:"-"` @@ -263,6 +276,83 @@ func (c *Comment) EventTag() string { return "event-" + com.ToStr(c.ID) } +// LoadReference if comment.Type is CommentType{Issue,Commit,Comment,Pull,Review,Code}Ref, then load RefIssue, RefComment, etc. +func (c *Comment) LoadReference() error { + if c.Type == CommentTypeIssueRef || c.Type == CommentTypePullRef { + var issueID int64 + 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 + } + + c.RefIssue = refIssue + c.RefURL = refIssue.HTMLURL() + c.RefExists = true + } + } else if c.Type == CommentTypeCommitRef { + contentParts := strings.SplitN(c.Content, " ", 2) + if len(contentParts) == 2 { + var repoID int64 + 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.RefCommitMessage = refCommit.CommitMessage + c.RefCommitShortSHA = base.TruncateString(refCommit.ID.String(), 10) + c.RefURL = fmt.Sprintf("%s/commit/%s", refRepo.Link(), refCommit.ID.String()) + c.RefExists = true + } + } + } else if c.Type == CommentTypeCommentRef || c.Type == CommentTypeReviewRef || c.Type == CommentTypeCodeRef { + var commentID int64 + 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 + } + + c.RefIssue = refIssue + c.RefComment = refComment + c.RefURL = refComment.HTMLURL() + c.RefExists = true + } + } + return nil +} + // LoadLabel if comment.Type is CommentTypeLabel, then load Label func (c *Comment) LoadLabel() error { var label Label @@ -778,6 +868,13 @@ func CreateComment(opts *CreateCommentOptions) (comment *Comment, err error) { if opts.Type == CommentTypeComment { UpdateIssueIndexer(opts.Issue.ID) } + + if opts.Type == CommentTypeComment || opts.Type == CommentTypeCode || opts.Type == CommentTypeReview { + if err = CreateOrUpdateCommentAction(comment.Poster, opts.Repo, opts.Issue, comment); err != nil { + return nil, err + } + } + return comment, nil } @@ -862,17 +959,17 @@ func CreateCodeComment(doer *User, repo *Repository, issue *Issue, content, tree }) } -// 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, issue *Issue, content string, 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) @@ -881,16 +978,55 @@ func CreateRefComment(doer *User, repo *Repository, issue *Issue, content, commi } _, err = CreateComment(&CreateCommentOptions{ - Type: CommentTypeCommitRef, + Type: commentType, Doer: doer, - Repo: repo, + Repo: issue.Repo, Issue: issue, - CommitSHA: commitSHA, + CommitSHA: refSHA, Content: content, }) return err } +// CreateCommitRefComment creates a commit reference comment to issue. +func CreateCommitRefComment(doer *User, issue *Issue, commitRepoID int64, commitSHA string) error { + content := fmt.Sprintf("%d %s", commitRepoID, commitSHA) + return createRefComment(doer, issue, content, base.EncodeSha1(content), CommentTypeCommitRef) +} + +// CreateCommentRefComment creates a comment reference comment to issue. +func CreateCommentRefComment(doer *User, issue *Issue, commentIssueID int64, commentID int64) error { + return createCommentRefComment(doer, issue, commentIssueID, commentID, CommentTypeCommentRef) +} + +// CreateIssueRefComment creates an issue reference comment to issue. +func CreateIssueRefComment(doer *User, issue *Issue, commentIssueID int64) error { + content := fmt.Sprintf(`%d`, commentIssueID) + return createRefComment(doer, issue, content, base.EncodeSha1(content), CommentTypeIssueRef) +} + +// CreatePullRefComment creates a pull request reference comment to issue. +func CreatePullRefComment(doer *User, issue *Issue, commentIssueID int64) error { + content := fmt.Sprintf(`%d`, commentIssueID) + return createRefComment(doer, issue, content, base.EncodeSha1(content), CommentTypePullRef) +} + +// CreateReviewRefComment creates a review reference comment to issue. +func CreateReviewRefComment(doer *User, issue *Issue, commentIssueID int64, commentID int64) error { + return createCommentRefComment(doer, issue, commentIssueID, commentID, CommentTypeReviewRef) +} + +// CreateCodeRefComment creates a code comment reference comment to issue. +func CreateCodeRefComment(doer *User, issue *Issue, commentIssueID int64, commentID int64) error { + return createCommentRefComment(doer, issue, commentIssueID, commentID, CommentTypeCodeRef) +} + +// createCommentRefComment is the common implementation for Create{Comment,Review,Code}RefComment functions +func createCommentRefComment(doer *User, issue *Issue, commentIssueID int64, commentID int64, commentType CommentType) error { + content := fmt.Sprintf(`%d`, commentID) + return createRefComment(doer, issue, content, base.EncodeSha1(content), commentType) +} + // GetCommentByID returns the comment by given ID. func GetCommentByID(id int64) (*Comment, error) { c := new(Comment) @@ -991,6 +1127,17 @@ func UpdateComment(doer *User, c *Comment, oldContent string) error { } mode, _ := AccessLevel(doer, c.Issue.Repo) + issue, err := GetIssueByID(c.IssueID) + if err != nil { + return err + } + + if c.Type == CommentTypeComment || c.Type == CommentTypeCode || c.Type == CommentTypeReview { + if err = CreateOrUpdateCommentAction(c.Poster, issue.Repo, issue, c); err != nil { + return err + } + } + if err := PrepareWebhooks(c.Issue.Repo, HookEventIssueComment, &api.IssueCommentPayload{ Action: api.HookIssueCommentEdited, Issue: c.Issue.APIFormat(), diff --git a/models/issue_comment_test.go b/models/issue_comment_test.go index 91dd5c17322dd..d98cdd18e2d86 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,20 +42,28 @@ 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) + } } func TestFetchCodeComments(t *testing.T) { diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 6c28989c2e80a..b62cb9cdbb2a1 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -202,6 +202,8 @@ var migrations = []Migration{ NewMigration("add must_change_password column for users table", addMustChangePassword), // v74 -> v75 NewMigration("add approval whitelists to protected branches", addApprovalWhitelistsToProtectedBranches), + // v75 -> v76 + NewMigration("reformat old-style reference comments, add new-style reference comments", reformatAndAddReferenceComments), } // Migrate database to current version diff --git a/models/migrations/v75.go b/models/migrations/v75.go new file mode 100644 index 0000000000000..eae5cb6c4da1e --- /dev/null +++ b/models/migrations/v75.go @@ -0,0 +1,280 @@ +// Copyright 2018 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "container/list" + "fmt" + "strings" + + "code.gitea.io/git" + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/util" + "github.com/go-xorm/xorm" +) + +func reformatAndAddReferenceComments(x *xorm.Engine) error { + const batchSize = 100 + + sess := x.NewSession() + defer sess.Close() + + if err := sess.Begin(); err != nil { + return err + } + + var maxCommentID int64 + + // create an repo lookup to map the repo's full name to the repository object + var repos []*models.Repository + repoLookup := make(map[string]*models.Repository) + if err := x.Find(&repos); err != nil { + return err + } + for _, repo := range repos { + repoLookup[repo.FullName()] = repo + } + + // create an issue updated lookup to save the current UpdateUnix values since UpdatedUnix gets updated as we migrate + var issues []*models.Issue + issueUpdatedLookup := make(map[int64]util.TimeStamp) + if err := x.Find(&issues); err != nil { + return err + } + for _, issue := range issues { + issueUpdatedLookup[issue.ID] = issue.UpdatedUnix + } + + // convert or remove old-style commit reference comments + for refCommentStart := 0; ; refCommentStart += batchSize { + refComments := make([]*models.Comment, 0, batchSize) + if err := x.Where("comment.type = ?", models.CommentTypeCommitRef).Limit(batchSize, refCommentStart).Find(&refComments); err != nil { + return err + } + if len(refComments) == 0 { + break + } + + for _, refComment := range refComments { + if err := refComment.LoadReference(); err != nil { + // if load failed, parse the content into a repo link and a commit SHA... + if strings.HasPrefix(refComment.Content, ``) + contentParts := strings.SplitN(content, `">`, 2) + if len(contentParts) == 2 { + linkParts := strings.SplitN(contentParts[0], `/commit/`, 2) + if len(linkParts) == 2 && len(linkParts[1]) == 40 { + repoLinkParts := strings.Split(linkParts[0], "/") + commitSha1 := linkParts[1] + // ...then check if the repo/commit exist... + if len(repoLinkParts) >= 2 { + repoFullName := strings.Join(repoLinkParts[len(repoLinkParts)-2:], "/") + refRepo := repoLookup[repoFullName] + if refRepo != nil { + gitRepo, err := git.OpenRepository(refRepo.RepoPath()) + if err == nil { + refCommit, err := gitRepo.GetCommit(commitSha1) + if err == nil { + // ...and update the ref comment to the new-style + refComment.Content = fmt.Sprintf("%d %s", refRepo.ID, refCommit.ID.String()) + refComment.CommitSHA = base.EncodeSha1(refComment.Content) + + if _, err := x.ID(refComment.ID).AllCols().Update(refComment); err == nil { + // continue if successful in order to skip over the delete below + continue + } + } + } + } + } + } + } + } + + // delete the comment if unable to convert it + if _, err := x.Delete(refComment); err != nil { + return err + } + } + } + } + + // for every issue and every comment, try to process the title+contents as ref only, no open/close action + for issueStart := 0; ; issueStart += batchSize { + issues = make([]*models.Issue, 0, batchSize) + if err := x.Limit(batchSize, issueStart).Find(&issues); err != nil { + return err + } + if len(issues) == 0 { + break + } + + if err := models.IssueList(issues).LoadAttributes(); err != nil { + continue + } + + for _, issue := range issues { + if _, err := x.Table("comment").Select("coalesce(max(comment.id), 0)").Get(&maxCommentID); err != nil { + return err + } + + if err := models.UpdateIssuesComment(issue.Poster, issue.Repo, issue, nil, false); err != nil { + continue + } + + // try to handle the commit messages if this is a pull request + for once := true; once && issue.IsPull; once = false { + var ( + err error + pr *models.PullRequest + baseBranch *models.Branch + headBranch *models.Branch + baseCommit *git.Commit + headCommit *git.Commit + baseGitRepo *git.Repository + ) + + if pr, err = issue.GetPullRequest(); err != nil { + continue + } + + if err = pr.GetBaseRepo(); err != nil { + continue + } + + if err = pr.GetHeadRepo(); err != nil { + continue + } + + if baseBranch, err = pr.BaseRepo.GetBranch(pr.BaseBranch); err != nil { + continue + } + if baseCommit, err = baseBranch.GetCommit(); err != nil { + continue + } + if headBranch, err = pr.HeadRepo.GetBranch(pr.HeadBranch); err != nil { + continue + } + if headCommit, err = headBranch.GetCommit(); err != nil { + continue + } + if baseGitRepo, err = git.OpenRepository(pr.BaseRepo.RepoPath()); err != nil { + continue + } + + mergeBase, err := baseGitRepo.GetMergeBase(baseCommit.ID.String(), headCommit.ID.String()) + if err != nil { + continue + } + + l, err := baseGitRepo.CommitsBetweenIDs(headCommit.ID.String(), mergeBase) + if err != nil { + continue + } + + commits := models.ListToPushCommits(l) + if err := models.UpdateIssuesCommit(issue.Poster, pr.BaseRepo, commits.Commits, false); err != nil { + continue + } + } + + if _, err := x.Exec("UPDATE `comment` SET `created_unix` = ?, `updated_unix` = ? WHERE `id` > ?", issueUpdatedLookup[issue.ID], issueUpdatedLookup[issue.ID], maxCommentID); err != nil { + return err + } + + // try to handle all comments on this issue + for commentStart := 0; ; commentStart += batchSize { + comments := make([]*models.Comment, 0, batchSize) + if err := x.Limit(batchSize, commentStart).Find(&comments); err != nil { + return err + } + if len(comments) == 0 { + break + } + + for _, comment := range comments { + if comment.Type != models.CommentTypeComment && comment.Type != models.CommentTypeCode && comment.Type != models.CommentTypeReview { + continue + } + + if _, err := x.Table("comment").Select("coalesce(max(comment.id), 0)").Get(&maxCommentID); err != nil { + return err + } + + if err := comment.LoadIssue(); err != nil { + continue + } + + if err := models.UpdateIssuesComment(comment.Poster, issue.Repo, issue, comment, false); err != nil { + continue + } + + if _, err := x.Exec("UPDATE `comment` SET `created_unix` = ?, `updated_unix` = ? WHERE `id` > ?", comment.UpdatedUnix, comment.UpdatedUnix, maxCommentID); err != nil { + return err + } + } + } + } + } + + // for every repo, try to process the commits an all branches for issue references + for _, repo := range repoLookup { + if _, err := x.Table("comment").Select("coalesce(max(comment.id), 0)").Get(&maxCommentID); err != nil { + return err + } + + var ( + err error + gitRepo *git.Repository + branches []*models.Branch + defaultBranch *models.Branch + defaultCommit *git.Commit + ) + + if gitRepo, err = git.OpenRepository(repo.RepoPath()); err != nil { + continue + } + + if defaultBranch, err = repo.GetBranch(repo.DefaultBranch); err != nil { + continue + } + if defaultCommit, err = defaultBranch.GetCommit(); err != nil { + continue + } + + if branches, err = repo.GetBranches(); err != nil { + continue + } + + for _, branch := range branches { + var branchCommit *git.Commit + if branchCommit, err = branch.GetCommit(); err != nil { + continue + } + + var l *list.List + if branch.Name == repo.DefaultBranch { + l, err = branchCommit.CommitsBefore() + } else { + l, err = gitRepo.CommitsBetweenIDs(branchCommit.ID.String(), defaultCommit.ID.String()) + } + if err != nil { + continue + } + + commits := models.ListToPushCommits(l) + if err := models.UpdateIssuesCommit(repo.MustOwner(), repo, commits.Commits, false); err != nil { + continue + } + } + + if _, err := x.Exec("UPDATE `comment` SET `created_unix` = ?, `updated_unix` = ? WHERE `id` > ?", repo.UpdatedUnix, repo.UpdatedUnix, maxCommentID); err != nil { + return err + } + } + + return sess.Commit() +} diff --git a/models/pull.go b/models/pull.go index 0041f83dc820c..15088c8469a5a 100644 --- a/models/pull.go +++ b/models/pull.go @@ -452,10 +452,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)) @@ -495,13 +491,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(), @@ -587,6 +588,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 @@ -776,25 +782,54 @@ func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []str return fmt.Errorf("Commit: %v", err) } - 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) + if err := pr.PushToBaseRepo(); err != nil { + return fmt.Errorf("PushToBaseRepo: %v", err) } + UpdateIssueIndexer(pull.ID) + pr.Issue = pull pull.PullRequest = pr mode, _ := AccessLevel(pull.Poster, 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 fmt.Errorf("GetBranch: %v", err) + } + if baseCommit, err = baseBranch.GetCommit(); err != nil { + return fmt.Errorf("GetCommit: %v", err) + } + if headBranch, err = pr.HeadRepo.GetBranch(pr.HeadBranch); err != nil { + return fmt.Errorf("GetBranch: %v", err) + } + if headCommit, err = headBranch.GetCommit(); err != nil { + return fmt.Errorf("GetCommit: %v", err) + } + if baseGitRepo, err = git.OpenRepository(pr.BaseRepo.RepoPath()); err != nil { + return fmt.Errorf("OpenRepository: %v", err) + } + + mergeBase, err := baseGitRepo.GetMergeBase(baseCommit.ID.String(), headCommit.ID.String()) + if err != nil { + return fmt.Errorf("GetMergeBase: %v", err) + } + + l, err := baseGitRepo.CommitsBetweenIDs(headCommit.ID.String(), mergeBase) + if err != nil { + return fmt.Errorf("CommitsBetweenIDs: %v", err) + } + + 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, diff --git a/models/update.go b/models/update.go index 0f71cd1e709a6..1773d3287eabc 100644 --- a/models/update.go +++ b/models/update.go @@ -68,7 +68,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 } @@ -184,7 +184,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 { @@ -278,5 +278,75 @@ 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 + } + + mergeBase, err := headGitRepo.GetMergeBase(baseCommit.ID.String(), headCommit.ID.String()) + if err != nil { + log.Error(4, "GetMergeBase: %v", err) + continue + } + + l, err := headGitRepo.CommitsBetweenIDs(headCommit.ID.String(), mergeBase) + if err != nil { + log.Error(4, "CommitsBetweenIDs: %v", err) + continue + } + + commits := ListToPushCommits(l) + if err = CommitPullRequestAction(pusher, pr.BaseRepo, commits); err != nil { + log.Error(4, "CommitPullRequestAction [%d]: %v", pr.ID, err) + continue + } + } + } + return repo, nil } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 1a748e8b209ec..b2219754f86f3 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -736,7 +736,12 @@ 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.review_ref_at = `referenced this issue from a review of a pull request %[2]s` +issues.code_ref_at = `referenced this issue from a comment on a pull request diff %[2]s` issues.poster = Poster issues.collaborator = Collaborator issues.owner = Owner diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 9fbadcd076682..5ac1fce5946f2 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -294,9 +294,6 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption } ctx.Error(500, "NewPullRequest", err) return - } else if err := pr.PushToBaseRepo(); err != nil { - ctx.Error(500, "PushToBaseRepo", err) - return } notification.NotifyNewPullRequest(pr) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index dd1c5cfa56c9f..716414e23d488 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -725,6 +725,15 @@ 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 || + comment.Type == models.CommentTypeReviewRef || + comment.Type == models.CommentTypeCodeRef { + 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/routers/repo/pull.go b/routers/repo/pull.go index 4adfb96e748be..dd23ed2aa0b37 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -894,9 +894,6 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm) } ctx.ServerError("NewPullRequest", err) return - } else if err := pullRequest.PushToBaseRepo(); err != nil { - ctx.ServerError("PushToBaseRepo", err) - return } notification.NotifyNewPullRequest(pullRequest) diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 953aa4c2f02db..4138da68cd306 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -81,7 +81,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}}
@@ -91,7 +104,59 @@
- {{.Content | Str2html}} + {{.RefCommitMessage | Escape}} ({{.RefCommitShortSHA | Escape}}) +
+
+ {{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 and (eq .Type 23) .RefExists}} +
+ + + + + {{.Poster.Name}} {{$.i18n.Tr "repo.issues.review_ref_at" .EventTag $createdStr | Safe}} + +
+ + {{.RefIssue.Title | Escape}} (#{{.RefIssue.Index}}) +
+
+ {{else if and (eq .Type 24) .RefExists}} +
+ + + + + {{.Poster.Name}} {{$.i18n.Tr "repo.issues.code_ref_at" .EventTag $createdStr | Safe}} + +
+ + {{.RefIssue.Title | Escape}} (#{{.RefIssue.Index}})
{{else if eq .Type 7}} @@ -243,7 +308,7 @@
- #{{.DependentIssue.Index}} {{.DependentIssue.Title}} + {{.DependentIssue.Title | Escape}} (#{{.DependentIssue.Index}})
{{else if eq .Type 20}} @@ -257,7 +322,7 @@
- #{{.DependentIssue.Index}} {{.DependentIssue.Title}} + {{.DependentIssue.Title | Escape}} (#{{.DependentIssue.Index}})
{{else if eq .Type 22}}