Implemented head_commit for webhooks (#16282)

* Removed Len field.

* Added head_commit webhook field.

* Added comment for returns.

modules/notification/webhook/webhook.go

@@ -562,7 +562,7 @@ func (m *webhookNotifier) NotifyIssueChangeMilestone(doer *models.User, issue *m
 
 func (m *webhookNotifier) NotifyPushCommits(pusher *models.User, repo *models.Repository, opts *repository.PushUpdateOptions, commits *repository.PushCommits) {
 	apiPusher := convert.ToUser(pusher, nil)
-	apiCommits, err := commits.ToAPIPayloadCommits(repo.RepoPath(), repo.HTMLURL())
+	apiCommits, apiHeadCommit, err := commits.ToAPIPayloadCommits(repo.RepoPath(), repo.HTMLURL())
 	if err != nil {
 		log.Error("commits.ToAPIPayloadCommits failed: %v", err)
 		return
@@ -574,6 +574,7 @@ func (m *webhookNotifier) NotifyPushCommits(pusher *models.User, repo *models.Re
 		After:      opts.NewCommitID,
 		CompareURL: setting.AppURL + commits.CompareURL,
 		Commits:    apiCommits,
+		HeadCommit: apiHeadCommit,
 		Repo:       convert.ToRepo(repo, models.AccessModeOwner),
 		Pusher:     apiPusher,
 		Sender:     apiPusher,
@@ -790,7 +791,7 @@ func (m *webhookNotifier) NotifyDeleteRelease(doer *models.User, rel *models.Rel
 
 func (m *webhookNotifier) NotifySyncPushCommits(pusher *models.User, repo *models.Repository, opts *repository.PushUpdateOptions, commits *repository.PushCommits) {
 	apiPusher := convert.ToUser(pusher, nil)
-	apiCommits, err := commits.ToAPIPayloadCommits(repo.RepoPath(), repo.HTMLURL())
+	apiCommits, apiHeadCommit, err := commits.ToAPIPayloadCommits(repo.RepoPath(), repo.HTMLURL())
 	if err != nil {
 		log.Error("commits.ToAPIPayloadCommits failed: %v", err)
 		return
@@ -802,6 +803,7 @@ func (m *webhookNotifier) NotifySyncPushCommits(pusher *models.User, repo *model
 		After:      opts.NewCommitID,
 		CompareURL: setting.AppURL + commits.CompareURL,
 		Commits:    apiCommits,
+		HeadCommit: apiHeadCommit,
 		Repo:       convert.ToRepo(repo, models.AccessModeOwner),
 		Pusher:     apiPusher,
 		Sender:     apiPusher,

modules/repository/commits.go

@@ -28,8 +28,8 @@ type PushCommit struct {
 
 // PushCommits represents list of commits in a push operation.
 type PushCommits struct {
-	Len        int
 	Commits    []*PushCommit
+	HeadCommit *PushCommit
 	CompareURL string
 
 	avatars    map[string]string
@@ -44,67 +44,88 @@ func NewPushCommits() *PushCommits {
 	}
 }
 
-// ToAPIPayloadCommits converts a PushCommits object to
+// toAPIPayloadCommit converts a single PushCommit to an api.PayloadCommit object.
-// api.PayloadCommit format.
+func (pc *PushCommits) toAPIPayloadCommit(repoPath, repoLink string, commit *PushCommit) (*api.PayloadCommit, error) {
-func (pc *PushCommits) ToAPIPayloadCommits(repoPath, repoLink string) ([]*api.PayloadCommit, error) {
-	commits := make([]*api.PayloadCommit, len(pc.Commits))
-
-	if pc.emailUsers == nil {
-		pc.emailUsers = make(map[string]*models.User)
-	}
 	var err error
-	for i, commit := range pc.Commits {
+	authorUsername := ""
-		authorUsername := ""
+	author, ok := pc.emailUsers[commit.AuthorEmail]
-		author, ok := pc.emailUsers[commit.AuthorEmail]
+	if !ok {
-		if !ok {
+		author, err = models.GetUserByEmail(commit.AuthorEmail)
-			author, err = models.GetUserByEmail(commit.AuthorEmail)
+		if err == nil {
-			if err == nil {
-				authorUsername = author.Name
-				pc.emailUsers[commit.AuthorEmail] = author
-			}
-		} else {
 			authorUsername = author.Name
+			pc.emailUsers[commit.AuthorEmail] = author
 		}
+	} else {
+		authorUsername = author.Name
+	}
 
-		committerUsername := ""
+	committerUsername := ""
-		committer, ok := pc.emailUsers[commit.CommitterEmail]
+	committer, ok := pc.emailUsers[commit.CommitterEmail]
-		if !ok {
+	if !ok {
-			committer, err = models.GetUserByEmail(commit.CommitterEmail)
+		committer, err = models.GetUserByEmail(commit.CommitterEmail)
-			if err == nil {
+		if err == nil {
-				// TODO: check errors other than email not found.
+			// TODO: check errors other than email not found.
-				committerUsername = committer.Name
-				pc.emailUsers[commit.CommitterEmail] = committer
-			}
-		} else {
 			committerUsername = committer.Name
+			pc.emailUsers[commit.CommitterEmail] = committer
 		}
+	} else {
+		committerUsername = committer.Name
+	}
 
-		fileStatus, err := git.GetCommitFileStatus(repoPath, commit.Sha1)
+	fileStatus, err := git.GetCommitFileStatus(repoPath, commit.Sha1)
+	if err != nil {
+		return nil, fmt.Errorf("FileStatus [commit_sha1: %s]: %v", commit.Sha1, err)
+	}
+
+	return &api.PayloadCommit{
+		ID:      commit.Sha1,
+		Message: commit.Message,
+		URL:     fmt.Sprintf("%s/commit/%s", repoLink, commit.Sha1),
+		Author: &api.PayloadUser{
+			Name:     commit.AuthorName,
+			Email:    commit.AuthorEmail,
+			UserName: authorUsername,
+		},
+		Committer: &api.PayloadUser{
+			Name:     commit.CommitterName,
+			Email:    commit.CommitterEmail,
+			UserName: committerUsername,
+		},
+		Added:     fileStatus.Added,
+		Removed:   fileStatus.Removed,
+		Modified:  fileStatus.Modified,
+		Timestamp: commit.Timestamp,
+	}, nil
+}
+
+// ToAPIPayloadCommits converts a PushCommits object to api.PayloadCommit format.
+// It returns all converted commits and, if provided, the head commit or an error otherwise.
+func (pc *PushCommits) ToAPIPayloadCommits(repoPath, repoLink string) ([]*api.PayloadCommit, *api.PayloadCommit, error) {
+	commits := make([]*api.PayloadCommit, len(pc.Commits))
+	var headCommit *api.PayloadCommit
+
+	if pc.emailUsers == nil {
+		pc.emailUsers = make(map[string]*models.User)
+	}
+	for i, commit := range pc.Commits {
+		apiCommit, err := pc.toAPIPayloadCommit(repoPath, repoLink, commit)
 		if err != nil {
-			return nil, fmt.Errorf("FileStatus [commit_sha1: %s]: %v", commit.Sha1, err)
+			return nil, nil, err
 		}
 
-		commits[i] = &api.PayloadCommit{
+		commits[i] = apiCommit
-			ID:      commit.Sha1,
+		if pc.HeadCommit != nil && pc.HeadCommit.Sha1 == commits[i].ID {
-			Message: commit.Message,
+			headCommit = apiCommit
-			URL:     fmt.Sprintf("%s/commit/%s", repoLink, commit.Sha1),
-			Author: &api.PayloadUser{
-				Name:     commit.AuthorName,
-				Email:    commit.AuthorEmail,
-				UserName: authorUsername,
-			},
-			Committer: &api.PayloadUser{
-				Name:     commit.CommitterName,
-				Email:    commit.CommitterEmail,
-				UserName: committerUsername,
-			},
-			Added:     fileStatus.Added,
-			Removed:   fileStatus.Removed,
-			Modified:  fileStatus.Modified,
-			Timestamp: commit.Timestamp,
 		}
 	}
-	return commits, nil
+	if pc.HeadCommit != nil && headCommit == nil {
+		var err error
+		headCommit, err = pc.toAPIPayloadCommit(repoPath, repoLink, pc.HeadCommit)
+		if err != nil {
+			return nil, nil, err
+		}
+	}
+	return commits, headCommit, nil
 }
 
 // AvatarLink tries to match user in database with e-mail
@@ -157,13 +178,9 @@ func CommitToPushCommit(commit *git.Commit) *PushCommit {
 // ListToPushCommits transforms a list.List to PushCommits type.
 func ListToPushCommits(l *list.List) *PushCommits {
 	var commits []*PushCommit
-	var actEmail string
 	for e := l.Front(); e != nil; e = e.Next() {
-		commit := e.Value.(*git.Commit)
+		commit := CommitToPushCommit(e.Value.(*git.Commit))
-		if actEmail == "" {
+		commits = append(commits, commit)
-			actEmail = commit.Committer.Email
-		}
-		commits = append(commits, CommitToPushCommit(commit))
 	}
-	return &PushCommits{l.Len(), commits, "", make(map[string]string), make(map[string]*models.User)}
+	return &PushCommits{commits, nil, "", make(map[string]string), make(map[string]*models.User)}
 }

modules/repository/commits_test.go

@@ -46,12 +46,13 @@ func TestPushCommits_ToAPIPayloadCommits(t *testing.T) {
 			Message:        "good signed commit",
 		},
 	}
-	pushCommits.Len = len(pushCommits.Commits)
+	pushCommits.HeadCommit = &PushCommit{Sha1: "69554a6"}
 
 	repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 16}).(*models.Repository)
-	payloadCommits, err := pushCommits.ToAPIPayloadCommits(repo.RepoPath(), "/user2/repo16")
+	payloadCommits, headCommit, err := pushCommits.ToAPIPayloadCommits(repo.RepoPath(), "/user2/repo16")
 	assert.NoError(t, err)
 	assert.Len(t, payloadCommits, 3)
+	assert.NotNil(t, headCommit)
 
 	assert.Equal(t, "69554a6", payloadCommits[0].ID)
 	assert.Equal(t, "not signed commit", payloadCommits[0].Message)
@@ -85,6 +86,17 @@ func TestPushCommits_ToAPIPayloadCommits(t *testing.T) {
 	assert.EqualValues(t, []string{"readme.md"}, payloadCommits[2].Added)
 	assert.EqualValues(t, []string{}, payloadCommits[2].Removed)
 	assert.EqualValues(t, []string{}, payloadCommits[2].Modified)
+
+	assert.Equal(t, "69554a6", headCommit.ID)
+	assert.Equal(t, "not signed commit", headCommit.Message)
+	assert.Equal(t, "/user2/repo16/commit/69554a6", headCommit.URL)
+	assert.Equal(t, "User2", headCommit.Committer.Name)
+	assert.Equal(t, "user2", headCommit.Committer.UserName)
+	assert.Equal(t, "User2", headCommit.Author.Name)
+	assert.Equal(t, "user2", headCommit.Author.UserName)
+	assert.EqualValues(t, []string{}, headCommit.Added)
+	assert.EqualValues(t, []string{}, headCommit.Removed)
+	assert.EqualValues(t, []string{"readme.md"}, headCommit.Modified)
 }
 
 func TestPushCommits_AvatarLink(t *testing.T) {
@@ -109,7 +121,6 @@ func TestPushCommits_AvatarLink(t *testing.T) {
 			Message:        "message2",
 		},
 	}
-	pushCommits.Len = len(pushCommits.Commits)
 
 	assert.Equal(t,
 		"https://secure.gravatar.com/avatar/ab53a2911ddf9b4817ac01ddcd3d975f?d=identicon&s=112",
@@ -177,7 +188,6 @@ func TestListToPushCommits(t *testing.T) {
 	})
 
 	pushCommits := ListToPushCommits(l)
-	assert.Equal(t, 2, pushCommits.Len)
 	if assert.Len(t, pushCommits.Commits, 2) {
 		assert.Equal(t, "Message1", pushCommits.Commits[0].Message)
 		assert.Equal(t, hexString1, pushCommits.Commits[0].Sha1)

routers/api/v1/repo/hook.go

@@ -140,16 +140,17 @@ func TestHook(ctx *context.APIContext) {
 		return
 	}
 
+	commit := convert.ToPayloadCommit(ctx.Repo.Repository, ctx.Repo.Commit)
+
 	if err := webhook.PrepareWebhook(hook, ctx.Repo.Repository, models.HookEventPush, &api.PushPayload{
-		Ref:    git.BranchPrefix + ctx.Repo.Repository.DefaultBranch,
+		Ref:        git.BranchPrefix + ctx.Repo.Repository.DefaultBranch,
-		Before: ctx.Repo.Commit.ID.String(),
+		Before:     ctx.Repo.Commit.ID.String(),
-		After:  ctx.Repo.Commit.ID.String(),
+		After:      ctx.Repo.Commit.ID.String(),
-		Commits: []*api.PayloadCommit{
+		Commits:    []*api.PayloadCommit{commit},
-			convert.ToPayloadCommit(ctx.Repo.Repository, ctx.Repo.Commit),
+		HeadCommit: commit,
-		},
+		Repo:       convert.ToRepo(ctx.Repo.Repository, models.AccessModeNone),
-		Repo:   convert.ToRepo(ctx.Repo.Repository, models.AccessModeNone),
+		Pusher:     convert.ToUserWithAccessMode(ctx.User, models.AccessModeNone),
-		Pusher: convert.ToUserWithAccessMode(ctx.User, models.AccessModeNone),
+		Sender:     convert.ToUserWithAccessMode(ctx.User, models.AccessModeNone),
-		Sender: convert.ToUserWithAccessMode(ctx.User, models.AccessModeNone),
 	}); err != nil {
 		ctx.Error(http.StatusInternalServerError, "PrepareWebhook: ", err)
 		return

routers/web/repo/webhook.go

@@ -1085,28 +1085,30 @@ func TestWebhook(ctx *context.Context) {
 	}
 
 	apiUser := convert.ToUserWithAccessMode(ctx.User, models.AccessModeNone)
-	p := &api.PushPayload{
+
-		Ref:    git.BranchPrefix + ctx.Repo.Repository.DefaultBranch,
+	apiCommit := &api.PayloadCommit{
-		Before: commit.ID.String(),
+		ID:      commit.ID.String(),
-		After:  commit.ID.String(),
+		Message: commit.Message(),
-		Commits: []*api.PayloadCommit{
+		URL:     ctx.Repo.Repository.HTMLURL() + "/commit/" + commit.ID.String(),
-			{
+		Author: &api.PayloadUser{
-				ID:      commit.ID.String(),
+			Name:  commit.Author.Name,
-				Message: commit.Message(),
+			Email: commit.Author.Email,
-				URL:     ctx.Repo.Repository.HTMLURL() + "/commit/" + commit.ID.String(),
+		},
-				Author: &api.PayloadUser{
+		Committer: &api.PayloadUser{
-					Name:  commit.Author.Name,
+			Name:  commit.Committer.Name,
-					Email: commit.Author.Email,
+			Email: commit.Committer.Email,
-				},
-				Committer: &api.PayloadUser{
-					Name:  commit.Committer.Name,
-					Email: commit.Committer.Email,
-				},
-			},
 		},
-		Repo:   convert.ToRepo(ctx.Repo.Repository, models.AccessModeNone),
+	}
-		Pusher: apiUser,
+
-		Sender: apiUser,
+	p := &api.PushPayload{
+		Ref:        git.BranchPrefix + ctx.Repo.Repository.DefaultBranch,
+		Before:     commit.ID.String(),
+		After:      commit.ID.String(),
+		Commits:    []*api.PayloadCommit{apiCommit},
+		HeadCommit: apiCommit,
+		Repo:       convert.ToRepo(ctx.Repo.Repository, models.AccessModeNone),
+		Pusher:     apiUser,
+		Sender:     apiUser,
 	}
 	if err := webhook.PrepareWebhook(w, ctx.Repo.Repository, models.HookEventPush, p); err != nil {
 		ctx.Flash.Error("PrepareWebhook: " + err.Error())

services/repository/push.go

@@ -95,7 +95,6 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error {
 		if opts.IsNewRef() && opts.IsDelRef() {
 			return fmt.Errorf("Old and new revisions are both %s", git.EmptySHA)
 		}
-		var commits = &repo_module.PushCommits{}
 		if opts.IsTag() { // If is tag reference
 			if pusher == nil || pusher.ID != opts.PusherID {
 				var err error
@@ -192,7 +191,8 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error {
 					}
 				}
 
-				commits = repo_module.ListToPushCommits(l)
+				commits := repo_module.ListToPushCommits(l)
+				commits.HeadCommit = repo_module.CommitToPushCommit(newCommit)
 
 				if err := repofiles.UpdateIssuesCommit(pusher, repo, commits.Commits, refName); err != nil {
 					log.Error("updateIssuesCommit: %v", err)

templates/user/dashboard/feeds.tmpl

@@ -101,7 +101,7 @@
 										</li>
 									{{end}}
 								{{end}}
-								{{if and (gt $push.Len 1) $push.CompareURL}}<li><a href="{{AppSubUrl}}/{{$push.CompareURL}}">{{$.i18n.Tr "action.compare_commits" $push.Len}} »</a></li>{{end}}
+								{{if and (gt (len $push.Commits) 1) $push.CompareURL}}<li><a href="{{AppSubUrl}}/{{$push.CompareURL}}">{{$.i18n.Tr "action.compare_commits" (len $push.Commits)}} »</a></li>{{end}}
 							</ul>
 						</div>
 					{{else if eq .GetOpType 6}}