From 0903b1ac8c7b64bb571d02cdd69fa671cc1c18c1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E8=B5=B5=E6=99=BA=E8=B6=85?= <1012112796@qq.com>
Date: Wed, 20 May 2020 20:47:24 +0800
Subject: [PATCH] Add push commits history comment on PR time-line (#11167)
* 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>
* Add migrations for IsForcePush
* fix wrong force-push judgement
* 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>
* fix lint
* fix style again, I forgot something before
* Change email notify way
* fix api
* add number check if It's force-push
* Add repo commit link fuction
remove unnecessary check
skip show push commits comment which not have commits alive
* Update issue_comment.go
* Apply suggestions from code review
Co-authored-by: mrsdizzie
* Apply suggestions from code review
* fix ui view
Co-authored-by: silverwind
* fix height
* remove unnecessary style define
* simplify GetBranchName
* Apply suggestions from code review
* save commit ids and isForce push by json
* simplify GetBranchName
* fix bug
Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>
Co-authored-by: mrsdizzie
Co-authored-by: Lauris BH
Co-authored-by: silverwind
---
models/issue_comment.go | 148 ++++++++++++++++++
models/repo.go | 11 ++
modules/git/commit.go | 8 +-
modules/git/repo_commit.go | 18 +++
modules/notification/base/notifier.go | 1 +
modules/notification/base/null.go | 4 +
modules/notification/mail/mail.go | 28 ++++
modules/notification/notification.go | 7 +
modules/notification/ui/ui.go | 9 ++
options/locale/locale_en-US.ini | 3 +
routers/repo/issue.go | 6 +
services/mailer/mail.go | 2 +
services/pull/pull.go | 44 ++++++
templates/mail/issue/default.tmpl | 33 +++-
templates/repo/commits_list_small.tmpl | 57 +++++++
.../repo/issue/view_content/comments.tmpl | 21 ++-
web_src/less/_repository.less | 79 ++++++++++
web_src/less/themes/theme-arc-green.less | 5 +
18 files changed, 478 insertions(+), 6 deletions(-)
create mode 100644 templates/repo/commits_list_small.tmpl
diff --git a/models/issue_comment.go b/models/issue_comment.go
index f7017435d..e23fae671 100644
--- a/models/issue_comment.go
+++ b/models/issue_comment.go
@@ -7,6 +7,8 @@
package models
import (
+ "container/list"
+ "encoding/json"
"fmt"
"strings"
@@ -90,6 +92,8 @@ const (
CommentTypeReviewRequest
// merge pull request
CommentTypeMergePull
+ // push to PR head branch
+ CommentTypePullPush
)
// CommentTag defines comment tag type
@@ -167,6 +171,18 @@ type Comment struct {
RefRepo *Repository `xorm:"-"`
RefIssue *Issue `xorm:"-"`
RefComment *Comment `xorm:"-"`
+
+ 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
@@ -543,6 +559,47 @@ func (c *Comment) CodeCommentURL() string {
return fmt.Sprintf("%s/files#%s", c.Issue.HTMLURL(), c.HashTag())
}
+// LoadPushCommits Load push commits
+func (c *Comment) LoadPushCommits() (err error) {
+ if c.Content == "" || c.Commits != nil || c.Type != CommentTypePullPush {
+ return nil
+ }
+
+ var data PushActionContent
+
+ err = json.Unmarshal([]byte(c.Content), &data)
+ if err != nil {
+ return
+ }
+
+ c.IsForcePush = data.IsForcePush
+
+ if c.IsForcePush {
+ if len(data.CommitIDs) != 2 {
+ return nil
+ }
+ c.OldCommit = data.CommitIDs[0]
+ c.NewCommit = data.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(data.CommitIDs)
+ 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)
+ }
+ }
+
+ return err
+}
+
func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err error) {
var LabelID int64
if opts.Label != nil {
@@ -576,6 +633,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 +796,7 @@ type CreateCommentOptions struct {
RefCommentID int64
RefAction references.XRefAction
RefIsPull bool
+ IsForcePush bool
}
// CreateComment creates comment of issue or commit.
@@ -1016,3 +1075,92 @@ func UpdateCommentsMigrationsByType(tp structs.GitServiceType, originalAuthorID
})
return err
}
+
+// 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
+ }
+
+ ops := &CreateCommentOptions{
+ Type: CommentTypePullPush,
+ Doer: pusher,
+ Repo: pr.BaseRepo,
+ }
+
+ var data PushActionContent
+
+ data.CommitIDs, data.IsForcePush, err = getCommitIDsFromRepo(pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch)
+ if err != nil {
+ return nil, err
+ }
+
+ ops.Issue = pr.Issue
+ dataJSON, err := json.Marshal(data)
+ if err != nil {
+ return nil, err
+ }
+
+ ops.Content = string(dataJSON)
+
+ comment, err = CreateComment(ops)
+
+ return
+}
+
+// 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, isForcePush bool, err error) {
+ repoPath := repo.RepoPath()
+ gitRepo, err := git.OpenRepository(repoPath)
+ if err != nil {
+ return nil, false, err
+ }
+ defer gitRepo.Close()
+
+ oldCommit, err := gitRepo.GetCommit(oldCommitID)
+ if err != nil {
+ return nil, false, err
+ }
+
+ oldCommitBranch, err := oldCommit.GetBranchName()
+ if err != nil {
+ return nil, false, err
+ }
+
+ if oldCommitBranch == "undefined" {
+ commitIDs = make([]string, 2)
+ commitIDs[0] = oldCommitID
+ commitIDs[1] = newCommitID
+
+ return commitIDs, true, err
+ }
+
+ 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())
+
+ for e := commits.Back(); e != nil; e = e.Prev() {
+ commit := e.Value.(*git.Commit)
+ commitBranch, err := commit.GetBranchName()
+ if err != nil {
+ return nil, false, err
+ }
+
+ if commitBranch != baseBranch {
+ commitIDs = append(commitIDs, commit.ID.String())
+ }
+ }
+
+ return
+}
diff --git a/models/repo.go b/models/repo.go
index f79740e74..c45abdf63 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/git/commit.go b/modules/git/commit.go
index 5e492e27e..8c7732a26 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()).RunInDir(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(string(data), " ")[1], "~")[0], nil
+ // name-rev commitID output will be "master" or "master~12"
+ return strings.SplitN(strings.TrimSpace(data), "~", 2)[0], nil
}
// CommitFileStatus represents status of files in a commit.
diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go
index c5f6d6cdd..397c390e8 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 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/base/notifier.go b/modules/notification/base/notifier.go
index 0b3e1173b..428f9a954 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 d2fd51d71..b2ce0742b 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/mail/mail.go b/modules/notification/mail/mail.go
index b980db7e4..795c8af2c 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 = 0
}
if err := mailer.MailParticipantsComment(comment, act, issue); err != nil {
@@ -117,3 +119,29 @@ 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) {
+ 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/modules/notification/notification.go b/modules/notification/notification.go
index d12024663..d17b13b9e 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 7f7de10be..cadc9720d 100644
--- a/modules/notification/ui/ui.go
+++ b/modules/notification/ui/ui.go
@@ -105,6 +105,15 @@ 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,
+ 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/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index d10f4c247..dec5cdd11 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -1029,6 +1029,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 = "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"
issues.due_date_form_edit = "Edit"
diff --git a/routers/repo/issue.go b/routers/repo/issue.go
index afe64c731..181ed59a8 100644
--- a/routers/repo/issue.go
+++ b/routers/repo/issue.go
@@ -999,6 +999,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(); err != nil {
+ ctx.ServerError("LoadPushCommits", err)
+ return
+ }
}
}
diff --git a/services/mailer/mail.go b/services/mailer/mail.go
index dd5af445b..b4217c046 100644
--- a/services/mailer/mail.go
+++ b/services/mailer/mail.go
@@ -319,6 +319,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 fb4af0637..c051641a5 100644
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -8,6 +8,7 @@ import (
"bufio"
"bytes"
"context"
+ "encoding/json"
"fmt"
"os"
"path"
@@ -57,6 +58,43 @@ func NewPullRequest(repo *models.Repository, pull *models.Issue, labelIDs []int6
notification.NotifyNewPullRequest(pr)
+ // add first push codes comment
+ baseGitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
+ if err != nil {
+ 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 {
+ 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())
+ }
+
+ dataJSON, err := json.Marshal(data)
+ if err != nil {
+ return err
+ }
+
+ ops := &models.CreateCommentOptions{
+ Type: models.CommentTypePullPush,
+ Doer: pull.Poster,
+ Repo: repo,
+ Issue: pr.Issue,
+ IsForcePush: false,
+ Content: string(dataJSON),
+ }
+
+ _, _ = models.CreateComment(ops)
+ }
+
return nil
}
@@ -237,6 +275,12 @@ func AddTestPullRequestTask(doer *models.User, repoID int64, branch string, isSy
}
addHeadRepoTasks(prs)
+ for _, pr := range prs {
+ comment, err := models.CreatePushPullComment(doer, pr, oldCommitID, newCommitID)
+ if err == nil && comment != nil {
+ notification.NotifyPullRequestPushCommits(doer, pr, 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/mail/issue/default.tmpl b/templates/mail/issue/default.tmpl
index 7cd397527..00bb561d6 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}}