From 71bc5ab9dc331c84f2f3a6f30095a41efe9548a3 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Tue, 21 Apr 2020 16:34:58 +0800 Subject: [PATCH 01/22] Add push commits history comment on PR time-line * Add notify by email and ui of this comment type also Signed-off-by: a1012112796 <1012112796@qq.com> --- models/issue_comment.go | 198 ++++++++++++++++++ modules/notification/action/action.go | 3 + modules/notification/indexer/indexer.go | 4 + modules/notification/mail/mail.go | 2 + modules/notification/webhook/webhook.go | 4 + options/locale/locale_en-US.ini | 3 + routers/repo/issue.go | 6 + services/pull/pull.go | 38 ++++ templates/repo/commits_list_small.tmpl | 57 +++++ .../repo/issue/view_content/comments.tmpl | 24 ++- web_src/less/_repository.less | 78 +++++++ 11 files changed, 415 insertions(+), 2 deletions(-) create mode 100644 templates/repo/commits_list_small.tmpl diff --git a/models/issue_comment.go b/models/issue_comment.go index f7017435d77d9..11aa9125abf3f 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -7,6 +7,7 @@ package models import ( + "container/list" "fmt" "strings" @@ -90,6 +91,8 @@ const ( CommentTypeReviewRequest // merge pull request CommentTypeMergePull + // push to PR head branch + CommentTypePullPush ) // CommentTag defines comment tag type @@ -167,6 +170,11 @@ type Comment struct { RefRepo *Repository `xorm:"-"` RefIssue *Issue `xorm:"-"` RefComment *Comment `xorm:"-"` + + Commits *list.List `xorm:"-"` + OldCommit string `xorm:"-"` + NewCommit string `xorm:"-"` + IsForcePush bool } // LoadIssue loads issue from database @@ -543,6 +551,48 @@ func (c *Comment) CodeCommentURL() string { return fmt.Sprintf("%s/files#%s", c.Issue.HTMLURL(), c.HashTag()) } +// LoadPushCommits Load push refs commits and add participants +func (c *Comment) LoadPushCommits(participants []*User) error { + var err error = nil + if c.Content == "" { + return err + } + + commitIDs := strings.Split(c.Content, ":") + if int64(len(commitIDs)) != c.Line { + return fmt.Errorf("LoadPushCommits: len of commitIDs is wrong %d - %d", + len(commitIDs), + c.Line) + } + c.Commits, err = getCommitsFromCommitIDs(c.Issue.PullRequest.BaseRepo, commitIDs) + if err != nil { + return err + } + + for e := c.Commits.Front(); e != nil; e = e.Next() { + if c.Poster.Email != e.Value.(*git.Commit).Author.Email { + commiter, err := GetUserByEmail(e.Value.(*git.Commit).Author.Email) + if err != nil || commiter == nil { + if IsErrUserNotExist(err) { + err = nil + } + return err + } + participants = append(participants, commiter) + } + } + + c.Commits = ValidateCommitsWithEmails(c.Commits) + c.Commits = ParseCommitsWithSignature(c.Commits, c.Issue.PullRequest.BaseRepo) + c.Commits = ParseCommitsWithStatus(c.Commits, c.Issue.PullRequest.BaseRepo) + + if c.IsForcePush { + c.OldCommit = commitIDs[0] + c.NewCommit = commitIDs[1] + } + return err +} + func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err error) { var LabelID int64 if opts.Label != nil { @@ -576,6 +626,7 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err RefCommentID: opts.RefCommentID, RefAction: opts.RefAction, RefIsPull: opts.RefIsPull, + IsForcePush: opts.IsForcePush, } if _, err = e.Insert(comment); err != nil { return nil, err @@ -738,6 +789,7 @@ type CreateCommentOptions struct { RefCommentID int64 RefAction references.XRefAction RefIsPull bool + IsForcePush bool } // CreateComment creates comment of issue or commit. @@ -1016,3 +1068,149 @@ func UpdateCommentsMigrationsByType(tp structs.GitServiceType, originalAuthorID }) return err } + +// CreatePushPullCommend create push code to pull base commend +func CreatePushPullCommend(pusher *User, pr *PullRequest, oldCommitID, newCommitID string) (comment *Comment, err error) { + ops := &CreateCommentOptions{ + Type: CommentTypePullPush, + Doer: pusher, + Repo: pr.BaseRepo, + } + + var commitIDs []string + messages := "" + + var isForcePush bool + if oldCommitID != "" && newCommitID != "" { + commitIDs, messages, isForcePush, err = getCommitIDsFromRepo(pr.BaseRepo, oldCommitID, newCommitID) + if err != nil { + return nil, err + } + } else { + return nil, nil + } + + ops.LineNum = int64(len(commitIDs)) + ops.Issue = pr.Issue + ops.IsForcePush = isForcePush + + commitIDlist := "" + + for _, commitID := range commitIDs { + commitIDlist = commitID + ":" + commitIDlist + } + + ops.Content = commitIDlist[0 : len(commitIDlist)-1] + + comment, err = CreateComment(ops) + if err != nil { + return + } + + // Prepare the contents for notify + prmessages := "@" + ops.Doer.Name + if ops.IsForcePush { + prmessages += " force-pushed the " + pr.HeadBranch + " branch from " + oldCommitID + " to " + newCommitID + } else { + if ops.LineNum == 1 { + prmessages += fmt.Sprintf(" pushed 1 commit to %s: \n ", pr.HeadBranch) + } else { + prmessages += fmt.Sprintf(" pushed %d commits to %s: \n ", ops.LineNum, pr.HeadBranch) + } + prmessages += messages + } + + comment.Content = prmessages + + return +} + +// getCommitsFromRepo get commit IDs from repo in betwern oldCommitID and newCommitID +// isForcePush will be true if newCommitID is older than oldCommitID +func getCommitIDsFromRepo(repo *Repository, oldCommitID, newCommitID string) (commitIDs []string, messages string, isForcePush bool, err error) { + if oldCommitID == "" || oldCommitID == git.EmptySHA || newCommitID == "" || newCommitID == git.EmptySHA { + return nil, "", false, nil + } + + repoPath := repo.RepoPath() + gitRepo, err := git.OpenRepository(repoPath) + if err != nil { + return nil, "", false, err + } + defer gitRepo.Close() + + newCommit, err := gitRepo.GetCommit(newCommitID) + if err != nil { + return nil, "", false, err + } + + var commits *list.List + commits, err = newCommit.CommitsBeforeUntil(oldCommitID) + if err != nil { + return nil, "", false, err + } + + commitIDs = make([]string, 0, commits.Len()) + messages = "" + + for e := commits.Front(); e != nil; e = e.Next() { + commitID := e.Value.(*git.Commit).ID.String() + commitMsg := e.Value.(*git.Commit).Message() + commitMsgs := strings.Split(commitMsg, "\n") + messages = "* " + commitID[0:10] + " - " + commitMsgs[0] + " \n " + messages + commitIDs = append(commitIDs, commitID) + } + + // check is force push by check the parent of commitIDs[commits.Len()-1] + isForcePush = true + checkCommit, err := gitRepo.GetCommit(commitIDs[commits.Len()-1]) + if err != nil { + return nil, "", false, err + } + + var parentCommit *git.Commit + parentNum := checkCommit.ParentCount() + if parentNum > 0 { + for i := 0; i < parentNum; i++ { + parentCommit, _ = checkCommit.Parent(i) + if parentCommit.ID.String() == oldCommitID { + isForcePush = false + break + } + } + } + + if isForcePush { + commitIDs = make([]string, 2, 2) + commitIDs[1] = oldCommitID + commitIDs[0] = newCommitID + } + + return +} + +// getCommitsFromCommitIDs get commits from commitIDs +func getCommitsFromCommitIDs(repo *Repository, commitIDs []string) (commits *list.List, err error) { + if commitIDs == nil { + return nil, nil + } + + repoPath := repo.RepoPath() + gitRepo, err := git.OpenRepository(repoPath) + if err != nil { + return nil, err + } + defer gitRepo.Close() + + commits = list.New() + + for _, commitID := range commitIDs { + commit, err := gitRepo.GetCommit(commitID) + if err != nil { + return nil, err + } + commits.PushBack(commit) + } + + return commits, nil +} diff --git a/modules/notification/action/action.go b/modules/notification/action/action.go index 9956940f30b22..5bd1f2e0ea99c 100644 --- a/modules/notification/action/action.go +++ b/modules/notification/action/action.go @@ -89,6 +89,9 @@ func (a *actionNotifier) NotifyIssueChangeStatus(doer *models.User, issue *model // NotifyCreateIssueComment notifies comment on an issue to notifiers func (a *actionNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository, issue *models.Issue, comment *models.Comment) { + if comment.Type == models.CommentTypePullPush { + return + } act := &models.Action{ ActUserID: doer.ID, ActUser: doer, diff --git a/modules/notification/indexer/indexer.go b/modules/notification/indexer/indexer.go index 6caae6fa65a12..7fdf65779cfbe 100644 --- a/modules/notification/indexer/indexer.go +++ b/modules/notification/indexer/indexer.go @@ -31,6 +31,10 @@ func NewNotifier() base.Notifier { func (r *indexerNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository, issue *models.Issue, comment *models.Comment) { + if comment.Type == models.CommentTypePullPush { + return + } + if comment.Type == models.CommentTypeComment { if issue.Comments == nil { if err := issue.LoadDiscussComments(); err != nil { diff --git a/modules/notification/mail/mail.go b/modules/notification/mail/mail.go index b5c2db383153c..172e0421a28b6 100644 --- a/modules/notification/mail/mail.go +++ b/modules/notification/mail/mail.go @@ -37,6 +37,8 @@ func (m *mailNotifier) NotifyCreateIssueComment(doer *models.User, repo *models. act = models.ActionCommentIssue } else if comment.Type == models.CommentTypeCode { act = models.ActionCommentIssue + } else if comment.Type == models.CommentTypePullPush { + act = models.ActionCommentIssue } if err := mailer.MailParticipantsComment(comment, act, issue); err != nil { diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index 625cf119a9ac1..962160a251c2d 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -392,6 +392,10 @@ func (m *webhookNotifier) NotifyUpdateComment(doer *models.User, c *models.Comme func (m *webhookNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository, issue *models.Issue, comment *models.Comment) { + if comment.Type == models.CommentTypePullPush { + return + } + mode, _ := models.AccessLevel(doer, repo) var err error diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index cfad41ceb54c5..38cb143515da7 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1013,6 +1013,9 @@ issues.due_date = Due Date issues.invalid_due_date_format = "Due date format must be 'yyyy-mm-dd'." issues.error_modifying_due_date = "Failed to modify the due date." issues.error_removing_due_date = "Failed to remove the due date." +issues.push_commit_1 = "add %d commit %s" +issues.push_commits_n = "add %d commits %s" +issues.force_push_codes = `force-pushed the %[1]s branch from %[2]s to %[4]s. %[6]s` issues.due_date_form = "yyyy-mm-dd" issues.due_date_form_add = "Add due date" issues.due_date_form_edit = "Edit" diff --git a/routers/repo/issue.go b/routers/repo/issue.go index a7fda4e7692de..ed49a2dde8838 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -995,6 +995,12 @@ func ViewIssue(ctx *context.Context) { ctx.ServerError("LoadResolveDoer", err) return } + } else if comment.Type == models.CommentTypePullPush { + participants = addParticipant(comment.Poster, participants) + if err = comment.LoadPushCommits(participants); err != nil { + ctx.ServerError("LoadPushCommits", err) + return + } } } diff --git a/services/pull/pull.go b/services/pull/pull.go index fb4af06372844..593f729941064 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -57,6 +57,38 @@ func NewPullRequest(repo *models.Repository, pull *models.Issue, labelIDs []int6 notification.NotifyNewPullRequest(pr) + // add first push codes command + baseGitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) + if err != nil { + return nil // just return , It's not important + } + defer baseGitRepo.Close() + + compareInfo, err := baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), + pr.BaseBranch, pr.GetGitRefName()) + + if compareInfo.Commits.Len() > 0 { + comitIDs := "" + for e := compareInfo.Commits.Front(); e != nil; e = e.Next() { + commitID := e.Value.(*git.Commit).ID.String() + comitIDs = commitID + ":" + comitIDs + } + + comitIDs = comitIDs[0 : len(comitIDs)-1] + + ops := &models.CreateCommentOptions{ + Type: models.CommentTypePullPush, + Doer: pull.Poster, + Repo: repo, + LineNum: int64(compareInfo.Commits.Len()), + Issue: pr.Issue, + RemovedAssignee: false, + Content: comitIDs, + } + + _, _ = models.CreateComment(ops) + } + return nil } @@ -237,6 +269,12 @@ func AddTestPullRequestTask(doer *models.User, repoID int64, branch string, isSy } addHeadRepoTasks(prs) + for _, pr := range prs { + comment, err := models.CreatePushPullCommend(doer, pr, oldCommitID, newCommitID) + if err == nil { + notification.NotifyCreateIssueComment(doer, pr.BaseRepo, pr.Issue, comment) + } + } log.Trace("AddTestPullRequestTask [base_repo_id: %d, base_branch: %s]: finding pull requests", repoID, branch) prs, err = models.GetUnmergedPullRequestsByBaseInfo(repoID, branch) diff --git a/templates/repo/commits_list_small.tmpl b/templates/repo/commits_list_small.tmpl new file mode 100644 index 0000000000000..13f9a6d666bfb --- /dev/null +++ b/templates/repo/commits_list_small.tmpl @@ -0,0 +1,57 @@ +{{ $r:= List .Commits}} +{{ $index := 0}} +{{range $r}} + {{ $tag := printf "%s-%d" $.HashTag $index }} + {{ $index = Add $index 1}} +
+ {{svg "octicon-git-commit" 16}} + {{if .User}} + + {{else}} + + {{end}} + + + {{$class := "ui sha label"}} + {{if .Signature}} + {{$class = (printf "%s%s" $class " isSigned")}} + {{if .Verification.Verified}} + {{if eq .Verification.TrustStatus "trusted"}} + {{$class = (printf "%s%s" $class " isVerified")}} + {{else if eq .Verification.TrustStatus "untrusted"}} + {{$class = (printf "%s%s" $class " isVerifiedUntrusted")}} + {{else}} + {{$class = (printf "%s%s" $class " isVerifiedUnmatched")}} + {{end}} + {{else if .Verification.Warning}} + {{$class = (printf "%s%s" $class " isWarning")}} + {{end}} + {{end}} + {{if $.Issue.PullRequest.BaseRepo.Name}} + + {{else}} + + {{end}} + {{ShortSha .ID.String}} + {{if $.Issue.PullRequest.BaseRepo.Name}} + + {{else}} + + {{end}} + + + + {{ $commitLink:= printf "%s/%s/%s/commit/%s" AppSubUrl $.Issue.PullRequest.BaseRepo.OwnerName $.Issue.PullRequest.BaseRepo.Name .ID }} + {{RenderCommitMessageLinkSubject .Message ($.Issue.PullRequest.BaseRepo.Link|Escape) $commitLink $.Issue.PullRequest.BaseRepo.ComposeMetas}} + + {{if IsMultilineCommitMessage .Message}} + + {{end}} + {{if eq (CommitType .) "SignCommitWithStatuses"}} + {{template "repo/commit_status" .Status}} + {{end}} + {{if IsMultilineCommitMessage .Message}} + + {{end}} +
+{{end}} diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 27baaed3f258e..81949a1201cc0 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -7,7 +7,8 @@ 13 = STOP_TRACKING, 14 = ADD_TIME_MANUAL, 16 = ADDED_DEADLINE, 17 = MODIFIED_DEADLINE, 18 = REMOVED_DEADLINE, 19 = ADD_DEPENDENCY, 20 = REMOVE_DEPENDENCY, 21 = CODE, 22 = REVIEW, 23 = ISSUE_LOCKED, 24 = ISSUE_UNLOCKED, 25 = TARGET_BRANCH_CHANGED, - 26 = DELETE_TIME_MANUAL, 27 = REVIEW_REQUEST, 28 = MERGE_PULL_REQUEST --> + 26 = DELETE_TIME_MANUAL, 27 = REVIEW_REQUEST, 28 = MERGE_PULL_REQUEST, + 29 = PULL_PUSH_EVENT --> {{if eq .Type 0}}
{{if .OriginalAuthor }} @@ -547,6 +548,25 @@ {{end}}
- + {{else if eq .Type 29}} +
+ {{svg "octicon-repo-force-push" 16}} + + + + + {{.Poster.GetDisplayName}} + {{ if .IsForcePush }} + {{ $oldCommitLink:= printf "%s/%s/%s/commit/%s" AppSubUrl $.Issue.PullRequest.BaseRepo.OwnerName $.Issue.PullRequest.BaseRepo.Name .OldCommit }} + {{ $newCommitLink:= printf "%s/%s/%s/commit/%s" AppSubUrl $.Issue.PullRequest.BaseRepo.OwnerName $.Issue.PullRequest.BaseRepo.Name .NewCommit }} + {{$.i18n.Tr "repo.issues.force_push_codes" $.Issue.PullRequest.HeadBranch (ShortSha .OldCommit) $oldCommitLink (ShortSha .NewCommit) $newCommitLink $createdStr | Safe}} + {{else}} + {{$.i18n.Tr (TrN $.i18n.Lang .Line "repo.issues.push_commit_1" "repo.issues.push_commits_n") .Line $createdStr | Safe}} + {{end}} + +
+ {{if and (not .IsForcePush) (gt .Line 0) }} + {{template "repo/commits_list_small" .}} + {{end}} {{end}} {{end}} diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less index 884d4193d801f..d2f3f5bc12ec0 100644 --- a/web_src/less/_repository.less +++ b/web_src/less/_repository.less @@ -769,6 +769,11 @@ padding-bottom: 0 !important; } + .badge-nobg { + border: 2px solid white !important; + height: 0px !important; + } + .badge { width: 32px; height: 32px; @@ -810,6 +815,16 @@ &.event > .text { line-height: 30px; } + + &.event.small-line-spacing { + padding: 2px 0 0 15px; + } + + &.event > .commit-status-link { + float: right; + margin-right: 8px; + margin-top: 4px; + } } .comment { @@ -1025,6 +1040,69 @@ } } + & > .shabox { + .sha.label { + margin: 0; + border: 1px solid #bbbbbb; + + &.isSigned.isWarning { + border: 1px solid #db2828; + background: fade(#db2828, 10%); + + .shortsha { + display: inline-block; + padding-top: 1px; + } + + &:hover { + background: fade(#db2828, 30%) !important; + } + } + + &.isSigned.isVerified { + border: 1px solid #21ba45; + background: fade(#21ba45, 10%); + + .shortsha { + display: inline-block; + padding-top: 1px; + } + + &:hover { + background: fade(#21ba45, 30%) !important; + } + } + + &.isSigned.isVerifiedUntrusted { + border: 1px solid #fbbd08; + background: fade(#fbbd08, 10%); + + .shortsha { + display: inline-block; + padding-top: 1px; + } + + &:hover { + background: fade(#fbbd08, 30%) !important; + } + } + + &.isSigned.isVerifiedUnmatched { + border: 1px solid #f2711c; + background: fade(#f2711c, 10%); + + .shortsha { + display: inline-block; + padding-top: 1px; + } + + &:hover { + background: fade(#f2711c, 30%) !important; + } + } + } + } + .detail { font-size: .9rem; margin-top: 5px; From fcedbcff867a8c4b8555e558829b5fb83ccb9f2b Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Tue, 21 Apr 2020 17:44:57 +0800 Subject: [PATCH 02/22] Add migrations for IsForcePush fix lint --- models/migrations/migrations.go | 2 ++ models/migrations/v139.go | 22 ++++++++++++++++++++++ web_src/less/_repository.less | 6 +++--- 3 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 models/migrations/v139.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 6868aad7b190a..60c31962ae51d 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -210,6 +210,8 @@ var migrations = []Migration{ NewMigration("Add Branch Protection Block Outdated Branch", addBlockOnOutdatedBranch), // v138 -> v139 NewMigration("Add ResolveDoerID to Comment table", addResolveDoerIDCommentColumn), + // v139 -> v140 + NewMigration("Add IsForcePush to Comment table", addIsForcePushCommentColumn), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v139.go b/models/migrations/v139.go new file mode 100644 index 0000000000000..5038608349661 --- /dev/null +++ b/models/migrations/v139.go @@ -0,0 +1,22 @@ +// Copyright 2020 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 ( + "fmt" + + "xorm.io/xorm" +) + +func addIsForcePushCommentColumn(x *xorm.Engine) error { + type Comment struct { + IsForcePush bool + } + + if err := x.Sync2(new(Comment)); err != nil { + return fmt.Errorf("Sync2: %v", err) + } + return nil +} diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less index 08c073818025c..eefa72ed65169 100644 --- a/web_src/less/_repository.less +++ b/web_src/less/_repository.less @@ -771,7 +771,7 @@ .badge-nobg { border: 2px solid white !important; - height: 0px !important; + height: 0 !important; } .badge { @@ -1071,7 +1071,7 @@ display: inline-block; padding-top: 1px; } - + &:hover { background: fade(#21ba45, 30%) !important; } @@ -1085,7 +1085,7 @@ display: inline-block; padding-top: 1px; } - + &:hover { background: fade(#fbbd08, 30%) !important; } From cc37c838c6626bbcde1c494fcde25269590b891c Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Wed, 22 Apr 2020 21:27:36 +0800 Subject: [PATCH 03/22] fix code lint fix wrong force-push judgement --- models/issue_comment.go | 97 +++++++++++++++++------------------------ modules/git/commit.go | 2 +- routers/repo/issue.go | 2 +- services/pull/pull.go | 21 +++++---- 4 files changed, 55 insertions(+), 67 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index 11aa9125abf3f..589daad1629ab 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -551,8 +551,8 @@ func (c *Comment) CodeCommentURL() string { return fmt.Sprintf("%s/files#%s", c.Issue.HTMLURL(), c.HashTag()) } -// LoadPushCommits Load push refs commits and add participants -func (c *Comment) LoadPushCommits(participants []*User) error { +// LoadPushCommits Load push commits +func (c *Comment) LoadPushCommits() error { var err error = nil if c.Content == "" { return err @@ -569,19 +569,6 @@ func (c *Comment) LoadPushCommits(participants []*User) error { return err } - for e := c.Commits.Front(); e != nil; e = e.Next() { - if c.Poster.Email != e.Value.(*git.Commit).Author.Email { - commiter, err := GetUserByEmail(e.Value.(*git.Commit).Author.Email) - if err != nil || commiter == nil { - if IsErrUserNotExist(err) { - err = nil - } - return err - } - participants = append(participants, commiter) - } - } - c.Commits = ValidateCommitsWithEmails(c.Commits) c.Commits = ParseCommitsWithSignature(c.Commits, c.Issue.PullRequest.BaseRepo) c.Commits = ParseCommitsWithStatus(c.Commits, c.Issue.PullRequest.BaseRepo) @@ -1071,6 +1058,10 @@ func UpdateCommentsMigrationsByType(tp structs.GitServiceType, originalAuthorID // CreatePushPullCommend create push code to pull base commend func CreatePushPullCommend(pusher *User, pr *PullRequest, oldCommitID, newCommitID string) (comment *Comment, err error) { + if pr.HasMerged || oldCommitID == "" || newCommitID == "" { + return nil, nil + } + ops := &CreateCommentOptions{ Type: CommentTypePullPush, Doer: pusher, @@ -1081,13 +1072,9 @@ func CreatePushPullCommend(pusher *User, pr *PullRequest, oldCommitID, newCommit messages := "" var isForcePush bool - if oldCommitID != "" && newCommitID != "" { - commitIDs, messages, isForcePush, err = getCommitIDsFromRepo(pr.BaseRepo, oldCommitID, newCommitID) - if err != nil { - return nil, err - } - } else { - return nil, nil + commitIDs, messages, isForcePush, err = getCommitIDsFromRepo(pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch) + if err != nil { + return nil, err } ops.LineNum = int64(len(commitIDs)) @@ -1126,12 +1113,9 @@ func CreatePushPullCommend(pusher *User, pr *PullRequest, oldCommitID, newCommit } // getCommitsFromRepo get commit IDs from repo in betwern oldCommitID and newCommitID -// isForcePush will be true if newCommitID is older than oldCommitID -func getCommitIDsFromRepo(repo *Repository, oldCommitID, newCommitID string) (commitIDs []string, messages string, isForcePush bool, err error) { - if oldCommitID == "" || oldCommitID == git.EmptySHA || newCommitID == "" || newCommitID == git.EmptySHA { - return nil, "", false, nil - } - +// isForcePush will be true if oldCommit isn't on the branch +// Commit on baseBranch will skip +func getCommitIDsFromRepo(repo *Repository, oldCommitID, newCommitID, baseBranch string) (commitIDs []string, messages string, isForcePush bool, err error) { repoPath := repo.RepoPath() gitRepo, err := git.OpenRepository(repoPath) if err != nil { @@ -1139,51 +1123,52 @@ func getCommitIDsFromRepo(repo *Repository, oldCommitID, newCommitID string) (co } defer gitRepo.Close() - newCommit, err := gitRepo.GetCommit(newCommitID) + oldCommit, err := gitRepo.GetCommit(oldCommitID) if err != nil { return nil, "", false, err } - var commits *list.List - commits, err = newCommit.CommitsBeforeUntil(oldCommitID) + oldCommitBranch, err := oldCommit.GetBranchName() if err != nil { return nil, "", false, err } - commitIDs = make([]string, 0, commits.Len()) - messages = "" + if oldCommitBranch == "undefined" { + commitIDs = make([]string, 2) + commitIDs[1] = oldCommitID + commitIDs[0] = newCommitID - for e := commits.Front(); e != nil; e = e.Next() { - commitID := e.Value.(*git.Commit).ID.String() - commitMsg := e.Value.(*git.Commit).Message() - commitMsgs := strings.Split(commitMsg, "\n") - messages = "* " + commitID[0:10] + " - " + commitMsgs[0] + " \n " + messages - commitIDs = append(commitIDs, commitID) + return commitIDs, "", true, err } - // check is force push by check the parent of commitIDs[commits.Len()-1] - isForcePush = true - checkCommit, err := gitRepo.GetCommit(commitIDs[commits.Len()-1]) + newCommit, err := gitRepo.GetCommit(newCommitID) if err != nil { return nil, "", false, err } - var parentCommit *git.Commit - parentNum := checkCommit.ParentCount() - if parentNum > 0 { - for i := 0; i < parentNum; i++ { - parentCommit, _ = checkCommit.Parent(i) - if parentCommit.ID.String() == oldCommitID { - isForcePush = false - break - } - } + var commits *list.List + commits, err = newCommit.CommitsBeforeUntil(oldCommitID) + if err != nil { + return nil, "", false, err } - if isForcePush { - commitIDs = make([]string, 2, 2) - commitIDs[1] = oldCommitID - commitIDs[0] = newCommitID + commitIDs = make([]string, 0, commits.Len()) + messages = "" + + for e := commits.Front(); e != nil; e = e.Next() { + commit := e.Value.(*git.Commit) + commitBranch, err := commit.GetBranchName() + if err != nil { + return nil, "", false, err + } + + if commitBranch != baseBranch { + commitID := commit.ID.String() + commitMsg := commit.Message() + commitMsg = strings.Split(commitMsg, "\n")[0] + messages = "* " + commitID[0:10] + " - " + commitMsg + " \n" + messages + commitIDs = append(commitIDs, commitID) + } } return diff --git a/modules/git/commit.go b/modules/git/commit.go index e65782912fea1..21d961b979d04 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -474,7 +474,7 @@ func (c *Commit) GetBranchName() (string, error) { } // name-rev commitID output will be "COMMIT_ID master" or "COMMIT_ID master~12" - return strings.Split(strings.Split(string(data), " ")[1], "~")[0], nil + return strings.Split(strings.Split(strings.Split(string(data), " ")[1], "~")[0], "\n")[0], nil } // CommitFileStatus represents status of files in a commit. diff --git a/routers/repo/issue.go b/routers/repo/issue.go index ed49a2dde8838..19229bc6f8283 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -997,7 +997,7 @@ func ViewIssue(ctx *context.Context) { } } else if comment.Type == models.CommentTypePullPush { participants = addParticipant(comment.Poster, participants) - if err = comment.LoadPushCommits(participants); err != nil { + if err = comment.LoadPushCommits(); err != nil { ctx.ServerError("LoadPushCommits", err) return } diff --git a/services/pull/pull.go b/services/pull/pull.go index 593f729941064..81ca189e2a8fe 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -60,12 +60,15 @@ func NewPullRequest(repo *models.Repository, pull *models.Issue, labelIDs []int6 // add first push codes command baseGitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) if err != nil { - return nil // just return , It's not important + return err } defer baseGitRepo.Close() compareInfo, err := baseGitRepo.GetCompareInfo(pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.GetGitRefName()) + if err != nil { + return err + } if compareInfo.Commits.Len() > 0 { comitIDs := "" @@ -77,13 +80,13 @@ func NewPullRequest(repo *models.Repository, pull *models.Issue, labelIDs []int6 comitIDs = comitIDs[0 : len(comitIDs)-1] ops := &models.CreateCommentOptions{ - Type: models.CommentTypePullPush, - Doer: pull.Poster, - Repo: repo, - LineNum: int64(compareInfo.Commits.Len()), - Issue: pr.Issue, - RemovedAssignee: false, - Content: comitIDs, + Type: models.CommentTypePullPush, + Doer: pull.Poster, + Repo: repo, + LineNum: int64(compareInfo.Commits.Len()), + Issue: pr.Issue, + IsForcePush: false, + Content: comitIDs, } _, _ = models.CreateComment(ops) @@ -271,7 +274,7 @@ func AddTestPullRequestTask(doer *models.User, repoID int64, branch string, isSy addHeadRepoTasks(prs) for _, pr := range prs { comment, err := models.CreatePushPullCommend(doer, pr, oldCommitID, newCommitID) - if err == nil { + if err == nil && comment != nil { notification.NotifyCreateIssueComment(doer, pr.BaseRepo, pr.Issue, comment) } } From 89b67a9bd99e8a22e0d72e3fe32ea12be103fd7d Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Wed, 22 Apr 2020 22:56:37 +0800 Subject: [PATCH 04/22] fix rpo error --- 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 589daad1629ab..ff69596742c29 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -564,14 +564,14 @@ func (c *Comment) LoadPushCommits() error { len(commitIDs), c.Line) } - c.Commits, err = getCommitsFromCommitIDs(c.Issue.PullRequest.BaseRepo, commitIDs) + c.Commits, err = getCommitsFromCommitIDs(c.Issue.Repo, commitIDs) if err != nil { return err } c.Commits = ValidateCommitsWithEmails(c.Commits) - c.Commits = ParseCommitsWithSignature(c.Commits, c.Issue.PullRequest.BaseRepo) - c.Commits = ParseCommitsWithStatus(c.Commits, c.Issue.PullRequest.BaseRepo) + c.Commits = ParseCommitsWithSignature(c.Commits, c.Issue.Repo) + c.Commits = ParseCommitsWithStatus(c.Commits, c.Issue.Repo) if c.IsForcePush { c.OldCommit = commitIDs[0] From a9563d9c5c046f48aa8951132092ed89651e3214 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Wed, 22 Apr 2020 23:08:38 +0800 Subject: [PATCH 05/22] fix lint --- models/issue_comment.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index ff69596742c29..f3c94c2b526ac 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -552,8 +552,7 @@ func (c *Comment) CodeCommentURL() string { } // LoadPushCommits Load push commits -func (c *Comment) LoadPushCommits() error { - var err error = nil +func (c *Comment) LoadPushCommits() (err error) { if c.Content == "" { return err } From 4ea2b3a25f76e4c892b9f254a4741f3497e2f426 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Wed, 22 Apr 2020 23:58:14 +0800 Subject: [PATCH 06/22] fix typo --- models/issue_comment.go | 4 ++-- services/pull/pull.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index f3c94c2b526ac..01e929d874022 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -1055,8 +1055,8 @@ func UpdateCommentsMigrationsByType(tp structs.GitServiceType, originalAuthorID return err } -// CreatePushPullCommend create push code to pull base commend -func CreatePushPullCommend(pusher *User, pr *PullRequest, oldCommitID, newCommitID string) (comment *Comment, err error) { +// CreatePushPullComment create push code to pull base commend +func CreatePushPullComment(pusher *User, pr *PullRequest, oldCommitID, newCommitID string) (comment *Comment, err error) { if pr.HasMerged || oldCommitID == "" || newCommitID == "" { return nil, nil } diff --git a/services/pull/pull.go b/services/pull/pull.go index 81ca189e2a8fe..0a0e9c3d1e1f3 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -273,7 +273,7 @@ func AddTestPullRequestTask(doer *models.User, repoID int64, branch string, isSy addHeadRepoTasks(prs) for _, pr := range prs { - comment, err := models.CreatePushPullCommend(doer, pr, oldCommitID, newCommitID) + comment, err := models.CreatePushPullComment(doer, pr, oldCommitID, newCommitID) if err == nil && comment != nil { notification.NotifyCreateIssueComment(doer, pr.BaseRepo, pr.Issue, comment) } From cce92bd4214a6781487aef0cbd5eafb6aeea15f3 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Fri, 24 Apr 2020 23:07:39 +0800 Subject: [PATCH 07/22] Apply suggestions from code review * Remove commit number check * add own notify fun * fix some typo Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com> --- models/issue_comment.go | 150 +++++++++--------- modules/git/repo_commit.go | 18 +++ modules/notification/action/action.go | 3 - modules/notification/base/notifier.go | 1 + modules/notification/base/null.go | 4 + modules/notification/indexer/indexer.go | 4 - modules/notification/mail/mail.go | 11 +- modules/notification/notification.go | 7 + modules/notification/ui/ui.go | 11 ++ modules/notification/webhook/webhook.go | 4 - services/mailer/mail.go | 2 + services/pull/pull.go | 38 ++--- .../repo/issue/view_content/comments.tmpl | 4 +- 13 files changed, 146 insertions(+), 111 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index 01e929d874022..45c6b1a04bf2f 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -553,32 +553,81 @@ func (c *Comment) CodeCommentURL() string { // LoadPushCommits Load push commits func (c *Comment) LoadPushCommits() (err error) { - if c.Content == "" { - return err + if c.Content == "" || c.Commits != nil || c.Type != CommentTypePullPush { + return nil } commitIDs := strings.Split(c.Content, ":") - if int64(len(commitIDs)) != c.Line { - return fmt.Errorf("LoadPushCommits: len of commitIDs is wrong %d - %d", - len(commitIDs), - c.Line) - } - c.Commits, err = getCommitsFromCommitIDs(c.Issue.Repo, commitIDs) - if err != nil { - return err - } - - c.Commits = ValidateCommitsWithEmails(c.Commits) - c.Commits = ParseCommitsWithSignature(c.Commits, c.Issue.Repo) - c.Commits = ParseCommitsWithStatus(c.Commits, c.Issue.Repo) if c.IsForcePush { c.OldCommit = commitIDs[0] c.NewCommit = commitIDs[1] + } else { + repoPath := c.Issue.Repo.RepoPath() + gitRepo, err := git.OpenRepository(repoPath) + if err != nil { + return err + } + defer gitRepo.Close() + + c.Commits = gitRepo.GetCommitsFromIDs(commitIDs) + c.Commits = ValidateCommitsWithEmails(c.Commits) + c.Commits = ParseCommitsWithSignature(c.Commits, c.Issue.Repo) + c.Commits = ParseCommitsWithStatus(c.Commits, c.Issue.Repo) } + return err } +// LoadPullPushDefaultNotify load default notify message in Content +// with Mardown style for Pull Push commits comment +func (c *Comment) LoadPullPushDefaultNotify() (err error) { + if c.Type != CommentTypePullPush { + return nil + } + if err = c.LoadIssue(); err != nil { + return + } + if err = c.Issue.LoadRepo(); err != nil { + return + } + if err = c.Issue.LoadPullRequest(); err != nil { + return + } + + message := "@" + c.Poster.Name + if c.IsForcePush { + commitIDs := strings.Split(c.Content, ":") + message += " force-pushed the " + c.Issue.PullRequest.HeadBranch + " branch from " + commitIDs[0] + " to " + commitIDs[1] + } else { + if c.Commits == nil { + repoPath := c.Issue.Repo.RepoPath() + gitRepo, err := git.OpenRepository(repoPath) + if err != nil { + return err + } + defer gitRepo.Close() + commitIDs := strings.Split(c.Content, ":") + c.Commits = gitRepo.GetCommitsFromIDs(commitIDs) + } + + if c.Commits.Len() == 1 { + message += fmt.Sprintf(" pushed 1 commit to %s: \n ", c.Issue.Repo.Name) + } else { + message += fmt.Sprintf(" pushed %d commits to %s: \n ", c.Commits.Len(), c.Issue.Repo.Name) + } + + for e := c.Commits.Front(); e != nil; e = e.Next() { + commitMsg := strings.Split(e.Value.(*git.Commit).Message(), "\n")[0] + message += "* " + e.Value.(*git.Commit).ID.String()[0:10] + " - " + commitMsg + " \n" + } + } + + c.Content = message + + return +} + func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err error) { var LabelID int64 if opts.Label != nil { @@ -1068,15 +1117,13 @@ func CreatePushPullComment(pusher *User, pr *PullRequest, oldCommitID, newCommit } var commitIDs []string - messages := "" var isForcePush bool - commitIDs, messages, isForcePush, err = getCommitIDsFromRepo(pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch) + commitIDs, isForcePush, err = getCommitIDsFromRepo(pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch) if err != nil { return nil, err } - ops.LineNum = int64(len(commitIDs)) ops.Issue = pr.Issue ops.IsForcePush = isForcePush @@ -1089,24 +1136,6 @@ func CreatePushPullComment(pusher *User, pr *PullRequest, oldCommitID, newCommit ops.Content = commitIDlist[0 : len(commitIDlist)-1] comment, err = CreateComment(ops) - if err != nil { - return - } - - // Prepare the contents for notify - prmessages := "@" + ops.Doer.Name - if ops.IsForcePush { - prmessages += " force-pushed the " + pr.HeadBranch + " branch from " + oldCommitID + " to " + newCommitID - } else { - if ops.LineNum == 1 { - prmessages += fmt.Sprintf(" pushed 1 commit to %s: \n ", pr.HeadBranch) - } else { - prmessages += fmt.Sprintf(" pushed %d commits to %s: \n ", ops.LineNum, pr.HeadBranch) - } - prmessages += messages - } - - comment.Content = prmessages return } @@ -1114,22 +1143,22 @@ func CreatePushPullComment(pusher *User, pr *PullRequest, oldCommitID, newCommit // getCommitsFromRepo get commit IDs from repo in betwern oldCommitID and newCommitID // isForcePush will be true if oldCommit isn't on the branch // Commit on baseBranch will skip -func getCommitIDsFromRepo(repo *Repository, oldCommitID, newCommitID, baseBranch string) (commitIDs []string, messages string, isForcePush bool, err error) { +func getCommitIDsFromRepo(repo *Repository, oldCommitID, newCommitID, baseBranch string) (commitIDs []string, isForcePush bool, err error) { repoPath := repo.RepoPath() gitRepo, err := git.OpenRepository(repoPath) if err != nil { - return nil, "", false, err + return nil, false, err } defer gitRepo.Close() oldCommit, err := gitRepo.GetCommit(oldCommitID) if err != nil { - return nil, "", false, err + return nil, false, err } oldCommitBranch, err := oldCommit.GetBranchName() if err != nil { - return nil, "", false, err + return nil, false, err } if oldCommitBranch == "undefined" { @@ -1137,64 +1166,33 @@ func getCommitIDsFromRepo(repo *Repository, oldCommitID, newCommitID, baseBranch commitIDs[1] = oldCommitID commitIDs[0] = newCommitID - return commitIDs, "", true, err + return commitIDs, true, err } newCommit, err := gitRepo.GetCommit(newCommitID) if err != nil { - return nil, "", false, err + return nil, false, err } var commits *list.List commits, err = newCommit.CommitsBeforeUntil(oldCommitID) if err != nil { - return nil, "", false, err + return nil, false, err } commitIDs = make([]string, 0, commits.Len()) - messages = "" for e := commits.Front(); e != nil; e = e.Next() { commit := e.Value.(*git.Commit) commitBranch, err := commit.GetBranchName() if err != nil { - return nil, "", false, err + return nil, false, err } if commitBranch != baseBranch { - commitID := commit.ID.String() - commitMsg := commit.Message() - commitMsg = strings.Split(commitMsg, "\n")[0] - messages = "* " + commitID[0:10] + " - " + commitMsg + " \n" + messages - commitIDs = append(commitIDs, commitID) + commitIDs = append(commitIDs, commit.ID.String()) } } return } - -// getCommitsFromCommitIDs get commits from commitIDs -func getCommitsFromCommitIDs(repo *Repository, commitIDs []string) (commits *list.List, err error) { - if commitIDs == nil { - return nil, nil - } - - repoPath := repo.RepoPath() - gitRepo, err := git.OpenRepository(repoPath) - if err != nil { - return nil, err - } - defer gitRepo.Close() - - commits = list.New() - - for _, commitID := range commitIDs { - commit, err := gitRepo.GetCommit(commitID) - if err != nil { - return nil, err - } - commits.PushBack(commit) - } - - return commits, nil -} diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index c5f6d6cdd6404..ab8af1e9cf5fb 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -454,3 +454,21 @@ func (repo *Repository) getBranches(commit *Commit, limit int) ([]string, error) } return branches, nil } + +// GetCommitsFromIDs get commits from commit IDs +func (repo *Repository) GetCommitsFromIDs(commitIDs []string) (commits *list.List) { + if commitIDs == nil || len(commitIDs) <= 0 { + return nil + } + + commits = list.New() + + for _, commitID := range commitIDs { + commit, err := repo.GetCommit(commitID) + if err == nil && commit != nil { + commits.PushBack(commit) + } + } + + return commits +} diff --git a/modules/notification/action/action.go b/modules/notification/action/action.go index 5bd1f2e0ea99c..9956940f30b22 100644 --- a/modules/notification/action/action.go +++ b/modules/notification/action/action.go @@ -89,9 +89,6 @@ func (a *actionNotifier) NotifyIssueChangeStatus(doer *models.User, issue *model // NotifyCreateIssueComment notifies comment on an issue to notifiers func (a *actionNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository, issue *models.Issue, comment *models.Comment) { - if comment.Type == models.CommentTypePullPush { - return - } act := &models.Action{ ActUserID: doer.ID, ActUser: doer, diff --git a/modules/notification/base/notifier.go b/modules/notification/base/notifier.go index 43b68b603c03f..67345daa6ccaf 100644 --- a/modules/notification/base/notifier.go +++ b/modules/notification/base/notifier.go @@ -36,6 +36,7 @@ type Notifier interface { NotifyPullRequestSynchronized(doer *models.User, pr *models.PullRequest) NotifyPullRequestReview(*models.PullRequest, *models.Review, *models.Comment) NotifyPullRequestChangeTargetBranch(doer *models.User, pr *models.PullRequest, oldBranch string) + NotifyPullRequestPushCommits(doer *models.User, pr *models.PullRequest, comment *models.Comment) NotifyCreateIssueComment(*models.User, *models.Repository, *models.Issue, *models.Comment) diff --git a/modules/notification/base/null.go b/modules/notification/base/null.go index 4b5efd80bb545..276e91e159cc3 100644 --- a/modules/notification/base/null.go +++ b/modules/notification/base/null.go @@ -54,6 +54,10 @@ func (*NullNotifier) NotifyPullRequestSynchronized(doer *models.User, pr *models func (*NullNotifier) NotifyPullRequestChangeTargetBranch(doer *models.User, pr *models.PullRequest, oldBranch string) { } +// NotifyPullRequestPushCommits notifies when push commits to pull request's head branch +func (*NullNotifier) NotifyPullRequestPushCommits(doer *models.User, pr *models.PullRequest, comment *models.Comment) { +} + // NotifyUpdateComment places a place holder function func (*NullNotifier) NotifyUpdateComment(doer *models.User, c *models.Comment, oldContent string) { } diff --git a/modules/notification/indexer/indexer.go b/modules/notification/indexer/indexer.go index 7fdf65779cfbe..6caae6fa65a12 100644 --- a/modules/notification/indexer/indexer.go +++ b/modules/notification/indexer/indexer.go @@ -31,10 +31,6 @@ func NewNotifier() base.Notifier { func (r *indexerNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository, issue *models.Issue, comment *models.Comment) { - if comment.Type == models.CommentTypePullPush { - return - } - if comment.Type == models.CommentTypeComment { if issue.Comments == nil { if err := issue.LoadDiscussComments(); err != nil { diff --git a/modules/notification/mail/mail.go b/modules/notification/mail/mail.go index 172e0421a28b6..9001307111a43 100644 --- a/modules/notification/mail/mail.go +++ b/modules/notification/mail/mail.go @@ -38,7 +38,7 @@ func (m *mailNotifier) NotifyCreateIssueComment(doer *models.User, repo *models. } else if comment.Type == models.CommentTypeCode { act = models.ActionCommentIssue } else if comment.Type == models.CommentTypePullPush { - act = models.ActionCommentIssue + act = 0 } if err := mailer.MailParticipantsComment(comment, act, issue); err != nil { @@ -119,3 +119,12 @@ func (m *mailNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *mode log.Error("MailParticipants: %v", err) } } + +func (m *mailNotifier) NotifyPullRequestPushCommits(doer *models.User, pr *models.PullRequest, comment *models.Comment) { + if err := comment.LoadPullPushDefaultNotify(); err != nil { + log.Error("comment.loadPullPushDefaultNotify: %v", err) + comment.Content = " " + } + + m.NotifyCreateIssueComment(doer, comment.Issue.Repo, comment.Issue, comment) +} diff --git a/modules/notification/notification.go b/modules/notification/notification.go index 95117c1f3e558..70b6be8137f57 100644 --- a/modules/notification/notification.go +++ b/modules/notification/notification.go @@ -94,6 +94,13 @@ func NotifyPullRequestChangeTargetBranch(doer *models.User, pr *models.PullReque } } +// NotifyPullRequestPushCommits notifies when push commits to pull request's head branch +func NotifyPullRequestPushCommits(doer *models.User, pr *models.PullRequest, comment *models.Comment) { + for _, notifier := range notifiers { + notifier.NotifyPullRequestPushCommits(doer, pr, comment) + } +} + // NotifyUpdateComment notifies update comment to notifiers func NotifyUpdateComment(doer *models.User, c *models.Comment, oldContent string) { for _, notifier := range notifiers { diff --git a/modules/notification/ui/ui.go b/modules/notification/ui/ui.go index 6ce14a90cc394..c8d4e4a088589 100644 --- a/modules/notification/ui/ui.go +++ b/modules/notification/ui/ui.go @@ -105,6 +105,17 @@ func (ns *notificationService) NotifyPullRequestReview(pr *models.PullRequest, r _ = ns.issueQueue.Push(opts) } +func (ns *notificationService) NotifyPullRequestPushCommits(doer *models.User, pr *models.PullRequest, comment *models.Comment) { + var opts = issueNotificationOpts{ + IssueID: pr.IssueID, + NotificationAuthorID: doer.ID, + } + if comment != nil { + opts.CommentID = comment.ID + } + _ = ns.issueQueue.Push(opts) +} + func (ns *notificationService) NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, assignee *models.User, removed bool, comment *models.Comment) { if !removed { var opts = issueNotificationOpts{ diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index 962160a251c2d..625cf119a9ac1 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -392,10 +392,6 @@ func (m *webhookNotifier) NotifyUpdateComment(doer *models.User, c *models.Comme func (m *webhookNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository, issue *models.Issue, comment *models.Comment) { - if comment.Type == models.CommentTypePullPush { - return - } - mode, _ := models.AccessLevel(doer, repo) var err error diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 3241ae728d880..5ba6716f32295 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -315,6 +315,8 @@ func actionToTemplate(issue *models.Issue, actionType models.ActionType, name = "code" case models.CommentTypeAssignees: name = "assigned" + case models.CommentTypePullPush: + name = "push" default: name = "default" } diff --git a/services/pull/pull.go b/services/pull/pull.go index 0a0e9c3d1e1f3..83ea827659b8a 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -57,7 +57,7 @@ func NewPullRequest(repo *models.Repository, pull *models.Issue, labelIDs []int6 notification.NotifyNewPullRequest(pr) - // add first push codes command + // add first push codes comment baseGitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) if err != nil { return err @@ -69,29 +69,25 @@ func NewPullRequest(repo *models.Repository, pull *models.Issue, labelIDs []int6 if err != nil { return err } + comitIDs := "" + for e := compareInfo.Commits.Front(); e != nil; e = e.Next() { + commitID := e.Value.(*git.Commit).ID.String() + comitIDs = commitID + ":" + comitIDs + } - if compareInfo.Commits.Len() > 0 { - comitIDs := "" - for e := compareInfo.Commits.Front(); e != nil; e = e.Next() { - commitID := e.Value.(*git.Commit).ID.String() - comitIDs = commitID + ":" + comitIDs - } - - comitIDs = comitIDs[0 : len(comitIDs)-1] - - ops := &models.CreateCommentOptions{ - Type: models.CommentTypePullPush, - Doer: pull.Poster, - Repo: repo, - LineNum: int64(compareInfo.Commits.Len()), - Issue: pr.Issue, - IsForcePush: false, - Content: comitIDs, - } + comitIDs = comitIDs[0 : len(comitIDs)-1] - _, _ = models.CreateComment(ops) + ops := &models.CreateCommentOptions{ + Type: models.CommentTypePullPush, + Doer: pull.Poster, + Repo: repo, + Issue: pr.Issue, + IsForcePush: false, + Content: comitIDs, } + _, _ = models.CreateComment(ops) + return nil } @@ -275,7 +271,7 @@ func AddTestPullRequestTask(doer *models.User, repoID int64, branch string, isSy for _, pr := range prs { comment, err := models.CreatePushPullComment(doer, pr, oldCommitID, newCommitID) if err == nil && comment != nil { - notification.NotifyCreateIssueComment(doer, pr.BaseRepo, pr.Issue, comment) + notification.NotifyPullRequestPushCommits(doer, pr, comment) } } diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index afd23bd95b940..33cb2d8f960a6 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -596,11 +596,11 @@ {{ $newCommitLink:= printf "%s/%s/%s/commit/%s" AppSubUrl $.Issue.PullRequest.BaseRepo.OwnerName $.Issue.PullRequest.BaseRepo.Name .NewCommit }} {{$.i18n.Tr "repo.issues.force_push_codes" $.Issue.PullRequest.HeadBranch (ShortSha .OldCommit) $oldCommitLink (ShortSha .NewCommit) $newCommitLink $createdStr | Safe}} {{else}} - {{$.i18n.Tr (TrN $.i18n.Lang .Line "repo.issues.push_commit_1" "repo.issues.push_commits_n") .Line $createdStr | Safe}} + {{$.i18n.Tr (TrN $.i18n.Lang .Commits.Len "repo.issues.push_commit_1" "repo.issues.push_commits_n") .Commits.Len $createdStr | Safe}} {{end}} - {{if and (not .IsForcePush) (gt .Line 0) }} + {{if not .IsForcePush}} {{template "repo/commits_list_small" .}} {{end}} {{end}} From 9a698d449f470b5cdd366e1f23e36d2aad28db1a Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Sat, 25 Apr 2020 10:36:20 +0800 Subject: [PATCH 08/22] fix lint --- modules/git/repo_commit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index ab8af1e9cf5fb..76c2c56833340 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -457,7 +457,7 @@ func (repo *Repository) getBranches(commit *Commit, limit int) ([]string, error) // GetCommitsFromIDs get commits from commit IDs func (repo *Repository) GetCommitsFromIDs(commitIDs []string) (commits *list.List) { - if commitIDs == nil || len(commitIDs) <= 0 { + if commitIDs == nil || len(commitIDs) == 0 { return nil } From bd363e5d44da5760ea426fb86489c716060a2fe8 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Sat, 25 Apr 2020 11:06:14 +0800 Subject: [PATCH 09/22] fix style again, I forgot something before --- modules/git/repo_commit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 76c2c56833340..397c390e84667 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -457,7 +457,7 @@ func (repo *Repository) getBranches(commit *Commit, limit int) ([]string, error) // GetCommitsFromIDs get commits from commit IDs func (repo *Repository) GetCommitsFromIDs(commitIDs []string) (commits *list.List) { - if commitIDs == nil || len(commitIDs) == 0 { + if len(commitIDs) == 0 { return nil } From ebde1d3a1f06de4e442d94640d4ccc2b5393c3a9 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Sun, 26 Apr 2020 10:27:40 +0800 Subject: [PATCH 10/22] Change email notify way --- models/issue_comment.go | 49 ------------------------------- modules/notification/mail/mail.go | 23 +++++++++++++-- templates/mail/issue/default.tmpl | 33 ++++++++++++++++++++- 3 files changed, 52 insertions(+), 53 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index 45c6b1a04bf2f..3e517a1c709ab 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -579,55 +579,6 @@ func (c *Comment) LoadPushCommits() (err error) { return err } -// LoadPullPushDefaultNotify load default notify message in Content -// with Mardown style for Pull Push commits comment -func (c *Comment) LoadPullPushDefaultNotify() (err error) { - if c.Type != CommentTypePullPush { - return nil - } - if err = c.LoadIssue(); err != nil { - return - } - if err = c.Issue.LoadRepo(); err != nil { - return - } - if err = c.Issue.LoadPullRequest(); err != nil { - return - } - - message := "@" + c.Poster.Name - if c.IsForcePush { - commitIDs := strings.Split(c.Content, ":") - message += " force-pushed the " + c.Issue.PullRequest.HeadBranch + " branch from " + commitIDs[0] + " to " + commitIDs[1] - } else { - if c.Commits == nil { - repoPath := c.Issue.Repo.RepoPath() - gitRepo, err := git.OpenRepository(repoPath) - if err != nil { - return err - } - defer gitRepo.Close() - commitIDs := strings.Split(c.Content, ":") - c.Commits = gitRepo.GetCommitsFromIDs(commitIDs) - } - - if c.Commits.Len() == 1 { - message += fmt.Sprintf(" pushed 1 commit to %s: \n ", c.Issue.Repo.Name) - } else { - message += fmt.Sprintf(" pushed %d commits to %s: \n ", c.Commits.Len(), c.Issue.Repo.Name) - } - - for e := c.Commits.Front(); e != nil; e = e.Next() { - commitMsg := strings.Split(e.Value.(*git.Commit).Message(), "\n")[0] - message += "* " + e.Value.(*git.Commit).ID.String()[0:10] + " - " + commitMsg + " \n" - } - } - - c.Content = message - - return -} - func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err error) { var LabelID int64 if opts.Label != nil { diff --git a/modules/notification/mail/mail.go b/modules/notification/mail/mail.go index 9001307111a43..294bc6023a07e 100644 --- a/modules/notification/mail/mail.go +++ b/modules/notification/mail/mail.go @@ -121,10 +121,27 @@ func (m *mailNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *mode } func (m *mailNotifier) NotifyPullRequestPushCommits(doer *models.User, pr *models.PullRequest, comment *models.Comment) { - if err := comment.LoadPullPushDefaultNotify(); err != nil { - log.Error("comment.loadPullPushDefaultNotify: %v", err) - comment.Content = " " + var err error + if err = comment.LoadIssue(); err != nil { + log.Error("comment.LoadIssue: %v", err) + return + } + if err = comment.Issue.LoadRepo(); err != nil { + log.Error("comment.Issue.LoadRepo: %v", err) + return + } + if err = comment.Issue.LoadPullRequest(); err != nil { + log.Error("comment.Issue.LoadPullRequest: %v", err) + return + } + if err = comment.Issue.PullRequest.LoadBaseRepo(); err != nil { + log.Error("comment.Issue.PullRequest.LoadBaseRepo: %v", err) + return + } + if err := comment.LoadPushCommits(); err != nil { + log.Error("comment.LoadPushCommits: %v", err) } + comment.Content = "" m.NotifyCreateIssueComment(doer, comment.Issue.Repo, comment.Issue, comment) } diff --git a/templates/mail/issue/default.tmpl b/templates/mail/issue/default.tmpl index 7cd3975277795..00bb561d6969f 100644 --- a/templates/mail/issue/default.tmpl +++ b/templates/mail/issue/default.tmpl @@ -17,6 +17,25 @@ {{if .IsMention}}

@{{.Doer.Name}} mentioned you:

{{end}} + {{if eq .Comment.Type 29}} +

+ {{.Doer.Name}} + {{if .Comment.IsForcePush}} + {{ $oldCommitLink:= printf "%s%s/%s/commit/%s" AppUrl .Comment.Issue.PullRequest.BaseRepo.OwnerName .Comment.Issue.PullRequest.BaseRepo.Name .Comment.OldCommit}} + {{ $newCommitLink:= printf "%s%s/%s/commit/%s" AppUrl .Comment.Issue.PullRequest.BaseRepo.OwnerName .Comment.Issue.PullRequest.BaseRepo.Name .Comment.NewCommit}} + force-pushed the {{.Comment.Issue.PullRequest.HeadBranch}} from + {{ShortSha .Comment.OldCommit}} + to + {{ShortSha .Comment.NewCommit}}. + {{else}} + {{if eq .Comment.Commits.Len 1}} + {{printf "pushed 1 commit to %s:" .Comment.Issue.PullRequest.HeadBranch}} + {{else}} + {{printf "pushed %d commits to %s:" .Comment.Commits.Len .Comment.Issue.PullRequest.HeadBranch}} + {{end}} + {{end}} +

+ {{end}}

{{if eq .ActionName "close"}} Closed #{{.Issue.Index}}. @@ -46,7 +65,19 @@ 

{{.Patch}}
{{.RenderedContent | Safe}}
- {{end -}} + {{end -}} + {{if eq .Comment.Type 29}} + {{ $r:= List .Comment.Commits}} + + {{end}}

From 712a0cc03b6da4df7f751f46e75c7a50f8ce587b Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Sun, 26 Apr 2020 11:42:57 +0800 Subject: [PATCH 11/22] fix api --- services/pull/pull.go | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/services/pull/pull.go b/services/pull/pull.go index 83ea827659b8a..930dbdc3345a2 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -69,24 +69,27 @@ func NewPullRequest(repo *models.Repository, pull *models.Issue, labelIDs []int6 if err != nil { return err } - comitIDs := "" - for e := compareInfo.Commits.Front(); e != nil; e = e.Next() { - commitID := e.Value.(*git.Commit).ID.String() - comitIDs = commitID + ":" + comitIDs - } - comitIDs = comitIDs[0 : len(comitIDs)-1] + if compareInfo.Commits.Len() > 0 { + comitIDs := "" + for e := compareInfo.Commits.Front(); e != nil; e = e.Next() { + commitID := e.Value.(*git.Commit).ID.String() + comitIDs = commitID + ":" + comitIDs + } - ops := &models.CreateCommentOptions{ - Type: models.CommentTypePullPush, - Doer: pull.Poster, - Repo: repo, - Issue: pr.Issue, - IsForcePush: false, - Content: comitIDs, - } + comitIDs = comitIDs[0 : len(comitIDs)-1] - _, _ = models.CreateComment(ops) + ops := &models.CreateCommentOptions{ + Type: models.CommentTypePullPush, + Doer: pull.Poster, + Repo: repo, + Issue: pr.Issue, + IsForcePush: false, + Content: comitIDs, + } + + _, _ = models.CreateComment(ops) + } return nil } From e28fa6cf97b44e72123940bafb4615cfa198685f Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Mon, 27 Apr 2020 15:55:06 +0800 Subject: [PATCH 12/22] add number check if It's force-push --- models/issue_comment.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/models/issue_comment.go b/models/issue_comment.go index 3e517a1c709ab..cd3602dd10a14 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -560,6 +560,9 @@ func (c *Comment) LoadPushCommits() (err error) { commitIDs := strings.Split(c.Content, ":") if c.IsForcePush { + if len(commitIDs) != 2 { + return nil + } c.OldCommit = commitIDs[0] c.NewCommit = commitIDs[1] } else { From 7d6fb31d8a21d89731812be468c43d5200520f12 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Wed, 29 Apr 2020 16:08:09 +0800 Subject: [PATCH 13/22] Add repo commit link fuction remove unnecessary check skip show push commits comment which not have commits alive --- models/issue_comment.go | 10 +++++++--- models/repo.go | 11 +++++++++++ modules/notification/ui/ui.go | 4 +--- templates/repo/issue/view_content/comments.tmpl | 6 ++---- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index cd3602dd10a14..80c71b4bad59b 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -574,9 +574,13 @@ func (c *Comment) LoadPushCommits() (err error) { defer gitRepo.Close() c.Commits = gitRepo.GetCommitsFromIDs(commitIDs) - c.Commits = ValidateCommitsWithEmails(c.Commits) - c.Commits = ParseCommitsWithSignature(c.Commits, c.Issue.Repo) - c.Commits = ParseCommitsWithStatus(c.Commits, c.Issue.Repo) + if c.Commits.Len() > 0 { + c.Commits = ValidateCommitsWithEmails(c.Commits) + c.Commits = ParseCommitsWithSignature(c.Commits, c.Issue.Repo) + c.Commits = ParseCommitsWithStatus(c.Commits, c.Issue.Repo) + } else { + c.Commits = nil + } } return err diff --git a/models/repo.go b/models/repo.go index 438066e0da319..09a79c7af4f52 100644 --- a/models/repo.go +++ b/models/repo.go @@ -264,6 +264,17 @@ func (repo *Repository) HTMLURL() string { return setting.AppURL + repo.FullName() } +// CommitLink make link to by commit full ID +// note: won't check whether it's an right id +func (repo *Repository) CommitLink(commitID string) (result string) { + if commitID == "" || commitID == "0000000000000000000000000000000000000000" { + result = "" + } else { + result = repo.HTMLURL() + "/commit/" + commitID + } + return +} + // APIURL returns the repository API URL func (repo *Repository) APIURL() string { return setting.AppURL + path.Join("api/v1/repos", repo.FullName()) diff --git a/modules/notification/ui/ui.go b/modules/notification/ui/ui.go index c8d4e4a088589..d0be38b4cbc39 100644 --- a/modules/notification/ui/ui.go +++ b/modules/notification/ui/ui.go @@ -109,9 +109,7 @@ func (ns *notificationService) NotifyPullRequestPushCommits(doer *models.User, p var opts = issueNotificationOpts{ IssueID: pr.IssueID, NotificationAuthorID: doer.ID, - } - if comment != nil { - opts.CommentID = comment.ID + CommentID: comment.ID, } _ = ns.issueQueue.Push(opts) } diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 33cb2d8f960a6..0a7cd4cb836b2 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -583,7 +583,7 @@ {{end}}

- {{else if eq .Type 29}} + {{else if and (eq .Type 29) (or .Commits .IsForcePush)}}
{{svg "octicon-repo-force-push" 16}} @@ -592,9 +592,7 @@ {{.Poster.GetDisplayName}} {{ if .IsForcePush }} - {{ $oldCommitLink:= printf "%s/%s/%s/commit/%s" AppSubUrl $.Issue.PullRequest.BaseRepo.OwnerName $.Issue.PullRequest.BaseRepo.Name .OldCommit }} - {{ $newCommitLink:= printf "%s/%s/%s/commit/%s" AppSubUrl $.Issue.PullRequest.BaseRepo.OwnerName $.Issue.PullRequest.BaseRepo.Name .NewCommit }} - {{$.i18n.Tr "repo.issues.force_push_codes" $.Issue.PullRequest.HeadBranch (ShortSha .OldCommit) $oldCommitLink (ShortSha .NewCommit) $newCommitLink $createdStr | Safe}} + {{$.i18n.Tr "repo.issues.force_push_codes" $.Issue.PullRequest.HeadBranch (ShortSha .OldCommit) ($.Issue.Repo.CommitLink .OldCommit) (ShortSha .NewCommit) ($.Issue.Repo.CommitLink .NewCommit) $createdStr | Safe}} {{else}} {{$.i18n.Tr (TrN $.i18n.Lang .Commits.Len "repo.issues.push_commit_1" "repo.issues.push_commits_n") .Commits.Len $createdStr | Safe}} {{end}} From b2ab4048c8ccc8b75e6f421ebdf56f280fd08b4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B5=B5=E6=99=BA=E8=B6=85?= <1012112796@qq.com> Date: Thu, 30 Apr 2020 23:12:25 +0800 Subject: [PATCH 14/22] Update issue_comment.go --- models/issue_comment.go | 1 + 1 file changed, 1 insertion(+) diff --git a/models/issue_comment.go b/models/issue_comment.go index 80c71b4bad59b..984e096b1ee77 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -580,6 +580,7 @@ func (c *Comment) LoadPushCommits() (err error) { c.Commits = ParseCommitsWithStatus(c.Commits, c.Issue.Repo) } else { c.Commits = nil + c.Content = "" } } From 84217da372eab83bbcca3bc613873a0c4caf1b2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B5=B5=E6=99=BA=E8=B6=85?= <1012112796@qq.com> Date: Fri, 1 May 2020 23:25:40 +0800 Subject: [PATCH 15/22] Apply suggestions from code review Co-authored-by: mrsdizzie --- options/locale/locale_en-US.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 9d1fc26417348..12c6f9d192344 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1018,8 +1018,8 @@ issues.due_date = Due Date issues.invalid_due_date_format = "Due date format must be 'yyyy-mm-dd'." issues.error_modifying_due_date = "Failed to modify the due date." issues.error_removing_due_date = "Failed to remove the due date." -issues.push_commit_1 = "add %d commit %s" -issues.push_commits_n = "add %d commits %s" +issues.push_commit_1 = "added %d commit %s" +issues.push_commits_n = "added %d commits %s" issues.force_push_codes = `force-pushed the %[1]s branch from %[2]s to %[4]s. %[6]s` issues.due_date_form = "yyyy-mm-dd" issues.due_date_form_add = "Add due date" From 8f6f2b7fba8b3945119b83de77f74f1e38caa1ba Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Sun, 3 May 2020 17:30:53 +0800 Subject: [PATCH 16/22] Apply suggestions from code review --- models/issue_comment.go | 7 +++---- templates/repo/issue/view_content/comments.tmpl | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index 984e096b1ee77..95556270f6208 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -174,6 +174,7 @@ type Comment struct { Commits *list.List `xorm:"-"` OldCommit string `xorm:"-"` NewCommit string `xorm:"-"` + CommitsNum int64 `xorm:"-"` IsForcePush bool } @@ -574,13 +575,11 @@ func (c *Comment) LoadPushCommits() (err error) { defer gitRepo.Close() c.Commits = gitRepo.GetCommitsFromIDs(commitIDs) - if c.Commits.Len() > 0 { + c.CommitsNum = int64(c.Commits.Len()) + if c.CommitsNum > 0 { c.Commits = ValidateCommitsWithEmails(c.Commits) c.Commits = ParseCommitsWithSignature(c.Commits, c.Issue.Repo) c.Commits = ParseCommitsWithStatus(c.Commits, c.Issue.Repo) - } else { - c.Commits = nil - c.Content = "" } } diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index fb16fcaae4918..c655355ca3e47 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -595,7 +595,7 @@ {{end}}
- {{else if and (eq .Type 29) (or .Commits .IsForcePush)}} + {{else if and (eq .Type 29) (or (gt .CommitsNum 0) .IsForcePush)}}
{{svg "octicon-repo-force-push" 16}} From b73d975e466ec65618b897dabada16f26afc63ad Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Wed, 6 May 2020 16:06:15 +0800 Subject: [PATCH 17/22] fix ui view Co-authored-by: silverwind --- templates/repo/commits_list_small.tmpl | 2 +- web_src/less/_repository.less | 6 +++--- web_src/less/themes/theme-arc-green.less | 5 +++++ 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/templates/repo/commits_list_small.tmpl b/templates/repo/commits_list_small.tmpl index 13f9a6d666bfb..1450cdafbc733 100644 --- a/templates/repo/commits_list_small.tmpl +++ b/templates/repo/commits_list_small.tmpl @@ -4,7 +4,7 @@ {{ $tag := printf "%s-%d" $.HashTag $index }} {{ $index = Add $index 1}}
- {{svg "octicon-git-commit" 16}} + {{svg "octicon-git-commit" 16}} {{if .User}} {{else}} diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less index bf4b49bf44812..feaa712832bb5 100644 --- a/web_src/less/_repository.less +++ b/web_src/less/_repository.less @@ -769,9 +769,9 @@ padding-bottom: 0 !important; } - .badge-nobg { - border: 2px solid white !important; - height: 0 !important; + .badge.badge-commit { + border-color: transparent; + background: radial-gradient(white 60%, transparent 60%) no-repeat; } .badge { diff --git a/web_src/less/themes/theme-arc-green.less b/web_src/less/themes/theme-arc-green.less index d56b7b8eebd34..94d985f377f01 100644 --- a/web_src/less/themes/theme-arc-green.less +++ b/web_src/less/themes/theme-arc-green.less @@ -708,6 +708,11 @@ a.ui.basic.green.label:hover { color: #9e9e9e; } +.repository.view.issue .comment-list .timeline-item .badge.badge-commit { + border-color: transparent; + background: radial-gradient(#383c4a 60%, transparent 60%) no-repeat; +} + .repository .comment.form .content .form:after { border-right-color: #313c47; } From bcbc8b2e3e7e32f9dbce9abe8e642c6f903b7314 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Wed, 6 May 2020 17:57:41 +0800 Subject: [PATCH 18/22] fix height --- web_src/less/_repository.less | 1 + web_src/less/themes/theme-arc-green.less | 1 + 2 files changed, 2 insertions(+) diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less index feaa712832bb5..ee0f26dceb658 100644 --- a/web_src/less/_repository.less +++ b/web_src/less/_repository.less @@ -772,6 +772,7 @@ .badge.badge-commit { border-color: transparent; background: radial-gradient(white 60%, transparent 60%) no-repeat; + height: 0 !important; } .badge { diff --git a/web_src/less/themes/theme-arc-green.less b/web_src/less/themes/theme-arc-green.less index 94d985f377f01..d20ff8101c35f 100644 --- a/web_src/less/themes/theme-arc-green.less +++ b/web_src/less/themes/theme-arc-green.less @@ -711,6 +711,7 @@ a.ui.basic.green.label:hover { .repository.view.issue .comment-list .timeline-item .badge.badge-commit { border-color: transparent; background: radial-gradient(#383c4a 60%, transparent 60%) no-repeat; + height: 0 !important; } .repository .comment.form .content .form:after { From 22ef854df8aea4d3e9169648909734eda1e7c083 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Wed, 6 May 2020 20:54:58 +0800 Subject: [PATCH 19/22] remove unnecessary style define --- web_src/less/themes/theme-arc-green.less | 1 - 1 file changed, 1 deletion(-) diff --git a/web_src/less/themes/theme-arc-green.less b/web_src/less/themes/theme-arc-green.less index d20ff8101c35f..b70e30753d760 100644 --- a/web_src/less/themes/theme-arc-green.less +++ b/web_src/less/themes/theme-arc-green.less @@ -709,7 +709,6 @@ a.ui.basic.green.label:hover { } .repository.view.issue .comment-list .timeline-item .badge.badge-commit { - border-color: transparent; background: radial-gradient(#383c4a 60%, transparent 60%) no-repeat; height: 0 !important; } From 9001a4f004ebc60d8f0ae1d72cbb1b3b6c268378 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Mon, 11 May 2020 22:33:59 +0800 Subject: [PATCH 20/22] simplify GetBranchName --- modules/git/commit.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/git/commit.go b/modules/git/commit.go index 21d961b979d04..7e0d78f9bec02 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -466,15 +466,15 @@ func (c *Commit) GetSubModule(entryname string) (*SubModule, error) { return nil, nil } -// GetBranchName gets the closes branch name (as returned by 'git name-rev') +// GetBranchName gets the closes branch name (as returned by 'git name-rev --name-only') func (c *Commit) GetBranchName() (string, error) { - data, err := NewCommand("name-rev", c.ID.String()).RunInDirBytes(c.repo.Path) + data, err := NewCommand("name-rev", "--name-only", c.ID.String()).RunInDirBytes(c.repo.Path) if err != nil { return "", err } - // name-rev commitID output will be "COMMIT_ID master" or "COMMIT_ID master~12" - return strings.Split(strings.Split(strings.Split(string(data), " ")[1], "~")[0], "\n")[0], nil + // name-rev commitID output will be "master" or "master~12" + return strings.Split(strings.Split(string(data), "~")[0], "\n")[0], nil } // CommitFileStatus represents status of files in a commit. From 2ca5230b7f5a95a36450447fcec8c1aeb10f91af Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Mon, 18 May 2020 23:47:28 +0800 Subject: [PATCH 21/22] Apply suggestions from code review * save commit ids and isForce push by json * simplify GetBranchName --- models/issue_comment.go | 50 ++++++++++++++++++++------------- models/migrations/migrations.go | 2 -- models/migrations/v139.go | 22 --------------- modules/git/commit.go | 4 +-- services/pull/pull.go | 16 +++++++---- 5 files changed, 42 insertions(+), 52 deletions(-) delete mode 100644 models/migrations/v139.go diff --git a/models/issue_comment.go b/models/issue_comment.go index 95556270f6208..45b80331c74e3 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -8,6 +8,7 @@ package models import ( "container/list" + "encoding/json" "fmt" "strings" @@ -171,11 +172,17 @@ type Comment struct { RefIssue *Issue `xorm:"-"` RefComment *Comment `xorm:"-"` - Commits *list.List `xorm:"-"` - OldCommit string `xorm:"-"` - NewCommit string `xorm:"-"` - CommitsNum int64 `xorm:"-"` - IsForcePush bool + Commits *list.List `xorm:"-"` + OldCommit string `xorm:"-"` + NewCommit string `xorm:"-"` + CommitsNum int64 `xorm:"-"` + IsForcePush bool `xorm:"-"` +} + +// PushActionContent is content of push pull comment +type PushActionContent struct { + IsForcePush bool `json:"is_force_push"` + CommitIDs []string `json:"commit_ids"` } // LoadIssue loads issue from database @@ -558,14 +565,21 @@ func (c *Comment) LoadPushCommits() (err error) { return nil } - commitIDs := strings.Split(c.Content, ":") + var data PushActionContent + + err = json.Unmarshal([]byte(c.Content), &data) + if err != nil { + return + } + + c.IsForcePush = data.IsForcePush if c.IsForcePush { - if len(commitIDs) != 2 { + if len(data.CommitIDs) != 2 { return nil } - c.OldCommit = commitIDs[0] - c.NewCommit = commitIDs[1] + c.OldCommit = data.CommitIDs[0] + c.NewCommit = data.CommitIDs[1] } else { repoPath := c.Issue.Repo.RepoPath() gitRepo, err := git.OpenRepository(repoPath) @@ -574,7 +588,7 @@ func (c *Comment) LoadPushCommits() (err error) { } defer gitRepo.Close() - c.Commits = gitRepo.GetCommitsFromIDs(commitIDs) + c.Commits = gitRepo.GetCommitsFromIDs(data.CommitIDs) c.CommitsNum = int64(c.Commits.Len()) if c.CommitsNum > 0 { c.Commits = ValidateCommitsWithEmails(c.Commits) @@ -1074,24 +1088,20 @@ func CreatePushPullComment(pusher *User, pr *PullRequest, oldCommitID, newCommit Repo: pr.BaseRepo, } - var commitIDs []string + var data PushActionContent - var isForcePush bool - commitIDs, isForcePush, err = getCommitIDsFromRepo(pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch) + data.CommitIDs, data.IsForcePush, err = getCommitIDsFromRepo(pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch) if err != nil { return nil, err } ops.Issue = pr.Issue - ops.IsForcePush = isForcePush - - commitIDlist := "" - - for _, commitID := range commitIDs { - commitIDlist = commitID + ":" + commitIDlist + dataJSON, err := json.Marshal(data) + if err != nil { + return nil, err } - ops.Content = commitIDlist[0 : len(commitIDlist)-1] + ops.Content = string(dataJSON) comment, err = CreateComment(ops) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 60c31962ae51d..6868aad7b190a 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -210,8 +210,6 @@ var migrations = []Migration{ NewMigration("Add Branch Protection Block Outdated Branch", addBlockOnOutdatedBranch), // v138 -> v139 NewMigration("Add ResolveDoerID to Comment table", addResolveDoerIDCommentColumn), - // v139 -> v140 - NewMigration("Add IsForcePush to Comment table", addIsForcePushCommentColumn), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v139.go b/models/migrations/v139.go deleted file mode 100644 index 5038608349661..0000000000000 --- a/models/migrations/v139.go +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright 2020 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 ( - "fmt" - - "xorm.io/xorm" -) - -func addIsForcePushCommentColumn(x *xorm.Engine) error { - type Comment struct { - IsForcePush bool - } - - if err := x.Sync2(new(Comment)); err != nil { - return fmt.Errorf("Sync2: %v", err) - } - return nil -} diff --git a/modules/git/commit.go b/modules/git/commit.go index f02e8e14fe224..8c7732a26b155 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -468,13 +468,13 @@ func (c *Commit) GetSubModule(entryname string) (*SubModule, error) { // GetBranchName gets the closes branch name (as returned by 'git name-rev --name-only') func (c *Commit) GetBranchName() (string, error) { - data, err := NewCommand("name-rev", "--name-only", c.ID.String()).RunInDirBytes(c.repo.Path) + data, err := NewCommand("name-rev", "--name-only", c.ID.String()).RunInDir(c.repo.Path) if err != nil { return "", err } // name-rev commitID output will be "master" or "master~12" - return strings.Split(strings.Split(string(data), "~")[0], "\n")[0], nil + return strings.SplitN(strings.TrimSpace(data), "~", 2)[0], nil } // CommitFileStatus represents status of files in a commit. diff --git a/services/pull/pull.go b/services/pull/pull.go index 930dbdc3345a2..c051641a5b00c 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -8,6 +8,7 @@ import ( "bufio" "bytes" "context" + "encoding/json" "fmt" "os" "path" @@ -71,13 +72,16 @@ func NewPullRequest(repo *models.Repository, pull *models.Issue, labelIDs []int6 } if compareInfo.Commits.Len() > 0 { - comitIDs := "" - for e := compareInfo.Commits.Front(); e != nil; e = e.Next() { - commitID := e.Value.(*git.Commit).ID.String() - comitIDs = commitID + ":" + comitIDs + data := models.PushActionContent{IsForcePush: false} + data.CommitIDs = make([]string, 0, compareInfo.Commits.Len()) + for e := compareInfo.Commits.Back(); e != nil; e = e.Prev() { + data.CommitIDs = append(data.CommitIDs, e.Value.(*git.Commit).ID.String()) } - comitIDs = comitIDs[0 : len(comitIDs)-1] + dataJSON, err := json.Marshal(data) + if err != nil { + return err + } ops := &models.CreateCommentOptions{ Type: models.CommentTypePullPush, @@ -85,7 +89,7 @@ func NewPullRequest(repo *models.Repository, pull *models.Issue, labelIDs []int6 Repo: repo, Issue: pr.Issue, IsForcePush: false, - Content: comitIDs, + Content: string(dataJSON), } _, _ = models.CreateComment(ops) From d8e414bef6657166716ff4780a364a4a66518364 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Tue, 19 May 2020 00:18:57 +0800 Subject: [PATCH 22/22] fix bug --- models/issue_comment.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index 45b80331c74e3..e23fae6715cf8 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -172,11 +172,11 @@ type Comment struct { RefIssue *Issue `xorm:"-"` RefComment *Comment `xorm:"-"` - Commits *list.List `xorm:"-"` - OldCommit string `xorm:"-"` - NewCommit string `xorm:"-"` - CommitsNum int64 `xorm:"-"` - IsForcePush bool `xorm:"-"` + Commits *list.List `xorm:"-"` + OldCommit string `xorm:"-"` + NewCommit string `xorm:"-"` + CommitsNum int64 `xorm:"-"` + IsForcePush bool `xorm:"-"` } // PushActionContent is content of push pull comment @@ -1131,8 +1131,8 @@ func getCommitIDsFromRepo(repo *Repository, oldCommitID, newCommitID, baseBranch if oldCommitBranch == "undefined" { commitIDs = make([]string, 2) - commitIDs[1] = oldCommitID - commitIDs[0] = newCommitID + commitIDs[0] = oldCommitID + commitIDs[1] = newCommitID return commitIDs, true, err } @@ -1150,7 +1150,7 @@ func getCommitIDsFromRepo(repo *Repository, oldCommitID, newCommitID, baseBranch commitIDs = make([]string, 0, commits.Len()) - for e := commits.Front(); e != nil; e = e.Next() { + for e := commits.Back(); e != nil; e = e.Prev() { commit := e.Value.(*git.Commit) commitBranch, err := commit.GetBranchName() if err != nil {