Skip to content

Commit

Permalink
Add push commits history comment on PR time-line (#11167)
Browse files Browse the repository at this point in the history
* 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 <info@mrsdizzie.com>

* Apply suggestions from code review

* fix ui view

Co-authored-by: silverwind <me@silverwind.io>

* 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 <info@mrsdizzie.com>
Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: silverwind <me@silverwind.io>
  • Loading branch information
5 people authored May 20, 2020
1 parent 9e0e2a9 commit 0903b1a
Show file tree
Hide file tree
Showing 18 changed files with 478 additions and 6 deletions.
148 changes: 148 additions & 0 deletions models/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
package models

import (
"container/list"
"encoding/json"
"fmt"
"strings"

Expand Down Expand Up @@ -90,6 +92,8 @@ const (
CommentTypeReviewRequest
// merge pull request
CommentTypeMergePull
// push to PR head branch
CommentTypePullPush
)

// CommentTag defines comment tag type
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -738,6 +796,7 @@ type CreateCommentOptions struct {
RefCommentID int64
RefAction references.XRefAction
RefIsPull bool
IsForcePush bool
}

// CreateComment creates comment of issue or commit.
Expand Down Expand Up @@ -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
}
11 changes: 11 additions & 0 deletions models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
8 changes: 4 additions & 4 deletions modules/git/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
18 changes: 18 additions & 0 deletions modules/git/repo_commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
1 change: 1 addition & 0 deletions modules/notification/base/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions modules/notification/base/null.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
}
Expand Down
28 changes: 28 additions & 0 deletions modules/notification/mail/mail.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
7 changes: 7 additions & 0 deletions modules/notification/notification.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions modules/notification/ui/ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
3 changes: 3 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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 <a href="%[3]s">%[2]s</a> to <a href="%[5]s">%[4]s</a>. %[6]s`
issues.due_date_form = "yyyy-mm-dd"
issues.due_date_form_add = "Add due date"
issues.due_date_form_edit = "Edit"
Expand Down
6 changes: 6 additions & 0 deletions routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions services/mailer/mail.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
Loading

0 comments on commit 0903b1a

Please sign in to comment.