Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add push commits history comment on PR time-line #11167

Merged
merged 40 commits into from
May 20, 2020
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
71bc5ab
Add push commits history comment on PR time-line
a1012112796 Apr 21, 2020
2bcb94c
Merge branch 'master' into commits_comment
a1012112796 Apr 21, 2020
fcedbcf
Add migrations for IsForcePush
a1012112796 Apr 21, 2020
cc37c83
fix code lint
a1012112796 Apr 22, 2020
6a713d9
Merge branch 'master' into commits_comment
a1012112796 Apr 22, 2020
89b67a9
fix rpo error
a1012112796 Apr 22, 2020
a9563d9
fix lint
a1012112796 Apr 22, 2020
4ea2b3a
fix typo
a1012112796 Apr 22, 2020
03b0992
Merge branch 'master' into commits_comment
a1012112796 Apr 23, 2020
cce92bd
Apply suggestions from code review
a1012112796 Apr 24, 2020
f0c7aa3
Merge branch 'master' into commits_comment
a1012112796 Apr 25, 2020
9a698d4
fix lint
a1012112796 Apr 25, 2020
bd363e5
fix style again, I forgot something before
a1012112796 Apr 25, 2020
ebde1d3
Change email notify way
a1012112796 Apr 26, 2020
69729ec
Merge branch 'master' into commits_comment
a1012112796 Apr 26, 2020
712a0cc
fix api
a1012112796 Apr 26, 2020
e28fa6c
add number check if It's force-push
a1012112796 Apr 27, 2020
e3481b0
Merge branch 'master' into commits_comment
a1012112796 Apr 27, 2020
7d6fb31
Add repo commit link fuction
a1012112796 Apr 29, 2020
57fcd82
Merge branch 'master' into commits_comment
a1012112796 Apr 29, 2020
b2ab404
Update issue_comment.go
a1012112796 Apr 30, 2020
84217da
Apply suggestions from code review
a1012112796 May 1, 2020
cf05dc4
Merge branch 'master' into commits_comment
a1012112796 May 1, 2020
8f6f2b7
Apply suggestions from code review
a1012112796 May 3, 2020
9f1d123
Merge branch 'master' into commits_comment
a1012112796 May 3, 2020
e3f9aa4
Merge branch 'master' into commits_comment
a1012112796 May 3, 2020
5caae89
Merge branch 'master' into commits_comment
lafriks May 5, 2020
b73d975
fix ui view
a1012112796 May 6, 2020
dcf100e
Merge branch 'master' into commits_comment
a1012112796 May 6, 2020
bcbc8b2
fix height
a1012112796 May 6, 2020
22ef854
remove unnecessary style define
a1012112796 May 6, 2020
7994f4d
Merge branch 'master' into commits_comment
a1012112796 May 6, 2020
9001a4f
simplify GetBranchName
a1012112796 May 11, 2020
24cddbd
Merge branch 'master' into commits_comment
a1012112796 May 11, 2020
92eb002
Merge branch 'master' into commits_comment
a1012112796 May 14, 2020
2ca5230
Apply suggestions from code review
a1012112796 May 18, 2020
c1bfcad
Merge branch 'master' into commits_comment
a1012112796 May 18, 2020
d8e414b
fix bug
a1012112796 May 18, 2020
1b1aed6
Merge branch 'master' into commits_comment
a1012112796 May 18, 2020
c4f9fec
Merge branch 'master' into commits_comment
lafriks May 20, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 138 additions & 0 deletions models/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package models

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

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

// CommentTag defines comment tag type
Expand Down Expand Up @@ -167,6 +170,12 @@ 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
}

// LoadIssue loads issue from database
Expand Down Expand Up @@ -543,6 +552,40 @@ 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
}

commitIDs := strings.Split(c.Content, ":")

if c.IsForcePush {
if len(commitIDs) != 2 {
guillep2k marked this conversation as resolved.
Show resolved Hide resolved
return nil
}
c.OldCommit = commitIDs[0]
c.NewCommit = commitIDs[1]
a1012112796 marked this conversation as resolved.
Show resolved Hide resolved
} 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.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 +619,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 +782,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 +1061,96 @@ 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 commitIDs []string

var isForcePush bool
commitIDs, 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
}

ops.Content = commitIDlist[0 : len(commitIDlist)-1]

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[1] = oldCommitID
commitIDs[0] = 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.Front(); e != nil; e = e.Next() {
commit := e.Value.(*git.Commit)
commitBranch, err := commit.GetBranchName()
guillep2k marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, false, err
}

if commitBranch != baseBranch {
commitIDs = append(commitIDs, commit.ID.String())
}
}

return
}
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions models/migrations/v139.go
Original file line number Diff line number Diff line change
@@ -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
}
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()).RunInDirBytes(c.repo.Path)
a1012112796 marked this conversation as resolved.
Show resolved Hide resolved
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.Split(strings.Split(string(data), "~")[0], "\n")[0], nil
a1012112796 marked this conversation as resolved.
Show resolved Hide resolved
}

// 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) {
a1012112796 marked this conversation as resolved.
Show resolved Hide resolved
}

// 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 @@ -1018,6 +1018,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 @@ -996,6 +996,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