#2246 fully support of webhooks for pull request

README.md

@@ -3,7 +3,7 @@ Gogs - Go Git Service [![Build Status](https://travis-ci.org/gogits/gogs.svg?bra
 
 ![](https://github.com/gogits/gogs/blob/master/public/img/gogs-large-resize.png?raw=true)
 
-##### Current tip version: 0.9.75 (see [Releases](https://github.com/gogits/gogs/releases) for binary versions)
+##### Current tip version: 0.9.76 (see [Releases](https://github.com/gogits/gogs/releases) for binary versions)
 
 | Web | UI  | Preview  |
 |:-------------:|:-------:|:-------:|

cmd/serve.go

@@ -113,7 +113,7 @@ func handleUpdateTask(uuid string, user, repoUser *models.User, reponame string,
 
 	// Ask for running deliver hook and test pull request tasks.
 	reqURL := setting.LocalURL + repoUser.Name + "/" + reponame + "/tasks/trigger?branch=" +
-		strings.TrimPrefix(task.RefName, "refs/heads/") + "&secret=" + base.EncodeMD5(repoUser.Salt)
+		strings.TrimPrefix(task.RefName, "refs/heads/") + "&secret=" + base.EncodeMD5(repoUser.Salt) + "&pusher=" + com.ToStr(user.ID)
 	log.GitLogger.Trace("Trigger task: %s", reqURL)
 
 	resp, err := httplib.Head(reqURL).SetTLSClientConfig(&tls.Config{

conf/locale/locale_en-US.ini

@@ -666,6 +666,8 @@ settings.event_send_everything = I need <strong>everything</strong>.
 settings.event_choose = Let me choose what I need.
 settings.event_create = Create
 settings.event_create_desc = Branch, or tag created
+settings.event_pull_request = Pull Request
+settings.event_pull_request_desc = Pull request opened, closed, reopened, edited, assigned, unassigned, label updated, label cleared, or synchronized.
 settings.event_push = Push
 settings.event_push_desc = Git push to a repository
 settings.active = Active

gogs.go

@@ -17,7 +17,7 @@ import (
 	"github.com/gogits/gogs/modules/setting"
 )
 
-const APP_VER = "0.9.75.0813"
+const APP_VER = "0.9.76.0814"
 
 func init() {
 	runtime.GOMAXPROCS(runtime.NumCPU())

models/action.go

@@ -169,7 +169,7 @@ func (a *Action) GetIssueTitle() string {
 		log.Error(4, "GetIssueByIndex: %v", err)
 		return "500 when get issue"
 	}
-	return issue.Name
+	return issue.Title
 }
 
 func (a *Action) GetIssueContent() string {
@@ -513,11 +513,11 @@ func CommitRepoAction(
 
 	payloadRepo := repo.ComposePayload()
 
-	pusher_email, pusher_name := "", ""
+	var pusherEmail, pusherName string
 	pusher, err := GetUserByName(userName)
 	if err == nil {
-		pusher_email = pusher.Email
+		pusherEmail = pusher.Email
-		pusher_name = pusher.DisplayName()
+		pusherName = pusher.DisplayName()
 	}
 	payloadSender := &api.PayloadUser{
 		UserName:  pusher.Name,
@@ -527,7 +527,7 @@ func CommitRepoAction(
 
 	switch opType {
 	case ACTION_COMMIT_REPO: // Push
-		p := &api.PushPayload{
+		if err = PrepareWebhooks(repo, HOOK_EVENT_PUSH, &api.PushPayload{
 			Ref:        refFullName,
 			Before:     oldCommitID,
 			After:      newCommitID,
@@ -535,13 +535,12 @@ func CommitRepoAction(
 			Commits:    commit.ToApiPayloadCommits(repo.FullLink()),
 			Repo:       payloadRepo,
 			Pusher: &api.PayloadAuthor{
-				Name:     pusher_name,
+				Name:     pusherName,
-				Email:    pusher_email,
+				Email:    pusherEmail,
 				UserName: userName,
 			},
 			Sender: payloadSender,
-		}
+		}); err != nil {
-		if err = PrepareWebhooks(repo, HOOK_EVENT_PUSH, p); err != nil {
 			return fmt.Errorf("PrepareWebhooks: %v", err)
 		}
 
@@ -603,7 +602,7 @@ func mergePullRequestAction(e Engine, actUser *User, repo *Repository, pull *Iss
 		ActUserName:  actUser.Name,
 		ActEmail:     actUser.Email,
 		OpType:       ACTION_MERGE_PULL_REQUEST,
-		Content:      fmt.Sprintf("%d|%s", pull.Index, pull.Name),
+		Content:      fmt.Sprintf("%d|%s", pull.Index, pull.Title),
 		RepoID:       repo.ID,
 		RepoUserName: repo.Owner.Name,
 		RepoName:     repo.Name,

models/issue.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/Unknwon/com"
 	"github.com/go-xorm/xorm"
+	api "github.com/gogits/go-gogs-client"
 	gouuid "github.com/satori/go.uuid"
 
 	"github.com/gogits/gogs/modules/base"
@@ -34,7 +35,7 @@ type Issue struct {
 	ID              int64       `xorm:"pk autoincr"`
 	RepoID          int64       `xorm:"INDEX UNIQUE(repo_index)"`
 	Index           int64       `xorm:"UNIQUE(repo_index)"` // Index in one repository.
-	Name            string
+	Title           string      `xorm:"name"`
 	Repo            *Repository `xorm:"-"`
 	PosterID        int64
 	Poster          *User    `xorm:"-"`
@@ -73,19 +74,6 @@ func (i *Issue) BeforeUpdate() {
 	i.DeadlineUnix = i.Deadline.Unix()
 }
 
-func (issue *Issue) loadAttributes(e Engine) (err error) {
-	issue.Repo, err = getRepositoryByID(e, issue.RepoID)
-	if err != nil {
-		return fmt.Errorf("getRepositoryByID: %v", err)
-	}
-
-	return nil
-}
-
-func (issue *Issue) LoadAttributes() error {
-	return issue.loadAttributes(x)
-}
-
 func (i *Issue) AfterSet(colName string, _ xorm.Cell) {
 	var err error
 	switch colName {
@@ -110,7 +98,7 @@ func (i *Issue) AfterSet(colName string, _ xorm.Cell) {
 		if err != nil {
 			if IsErrUserNotExist(err) {
 				i.PosterID = -1
-				i.Poster = NewFakeUser()
+				i.Poster = NewGhostUser()
 			} else {
 				log.Error(3, "GetUserByID[%d]: %v", i.ID, err)
 			}
@@ -146,17 +134,80 @@ func (i *Issue) AfterSet(colName string, _ xorm.Cell) {
 	}
 }
 
-// HashTag returns unique hash tag for issue.
+func (issue *Issue) loadAttributes(e Engine) (err error) {
-func (i *Issue) HashTag() string {
+	if issue.Repo == nil {
-	return "issue-" + com.ToStr(i.ID)
+		issue.Repo, err = getRepositoryByID(e, issue.RepoID)
+		if err != nil {
+			return fmt.Errorf("getRepositoryByID [%d]: %v", issue.RepoID, err)
+		}
+	}
+
+	if issue.IsPull && issue.PullRequest == nil {
+		// It is possible pull request is not yet created.
+		issue.PullRequest, err = getPullRequestByIssueID(e, issue.ID)
+		if err != nil && !IsErrPullRequestNotExist(err) {
+			return fmt.Errorf("getPullRequestByIssueID [%d]: %v", issue.ID, err)
+		}
+	}
+
+	return nil
+}
+
+func (issue *Issue) LoadAttributes() error {
+	return issue.loadAttributes(x)
 }
 
 // State returns string representation of issue status.
-func (i *Issue) State() string {
+func (i *Issue) State() api.StateType {
 	if i.IsClosed {
-		return "closed"
+		return api.STATE_CLOSED
 	}
-	return "open"
+	return api.STATE_OPEN
+}
+
+// This method assumes some fields assigned with values:
+// Required - Poster, Labels,
+// Optional - Milestone, Assignee, PullRequest
+func (issue *Issue) APIFormat() *api.Issue {
+	apiLabels := make([]*api.Label, len(issue.Labels))
+	for i := range issue.Labels {
+		apiLabels[i] = issue.Labels[i].APIFormat()
+	}
+
+	apiIssue := &api.Issue{
+		ID:       issue.ID,
+		Index:    issue.Index,
+		State:    issue.State(),
+		Title:    issue.Title,
+		Body:     issue.Content,
+		User:     issue.Poster.APIFormat(),
+		Labels:   apiLabels,
+		Comments: issue.NumComments,
+		Created:  issue.Created,
+		Updated:  issue.Updated,
+	}
+
+	if issue.Milestone != nil {
+		apiIssue.Milestone = issue.Milestone.APIFormat()
+	}
+	if issue.Assignee != nil {
+		apiIssue.Assignee = issue.Assignee.APIFormat()
+	}
+	if issue.IsPull {
+		apiIssue.PullRequest = &api.PullRequestMeta{
+			HasMerged: issue.PullRequest.HasMerged,
+		}
+		if issue.PullRequest.HasMerged {
+			apiIssue.PullRequest.Merged = &issue.PullRequest.Merged
+		}
+	}
+
+	return apiIssue
+}
+
+// HashTag returns unique hash tag for issue.
+func (i *Issue) HashTag() string {
+	return "issue-" + com.ToStr(i.ID)
 }
 
 func (issue *Issue) FullLink() string {
@@ -183,23 +234,37 @@ func (i *Issue) HasLabel(labelID int64) bool {
 	return i.hasLabel(x, labelID)
 }
 
+func (issue *Issue) sendLabelUpdatedWebhook(doer *User) {
+	var err error
+	if issue.IsPull {
+		issue.PullRequest.Issue = issue
+		err = PrepareWebhooks(issue.Repo, HOOK_EVENT_PULL_REQUEST, &api.PullRequestPayload{
+			Action:      api.HOOK_ISSUE_LABEL_UPDATED,
+			Index:       issue.Index,
+			PullRequest: issue.PullRequest.APIFormat(),
+			Repository:  issue.Repo.APIFormat(nil),
+			Sender:      doer.APIFormat(),
+		})
+	}
+	if err != nil {
+		log.Error(4, "PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err)
+	} else {
+		go HookQueue.Add(issue.RepoID)
+	}
+}
+
 func (i *Issue) addLabel(e *xorm.Session, label *Label) error {
 	return newIssueLabel(e, i, label)
 }
 
 // AddLabel adds a new label to the issue.
-func (i *Issue) AddLabel(label *Label) (err error) {
+func (issue *Issue) AddLabel(doer *User, label *Label) error {
-	sess := x.NewSession()
+	if err := NewIssueLabel(issue, label); err != nil {
-	defer sessionRelease(sess)
-	if err = sess.Begin(); err != nil {
 		return err
 	}
 
-	if err = i.addLabel(sess, label); err != nil {
+	issue.sendLabelUpdatedWebhook(doer)
-		return err
+	return nil
-	}
-
-	return sess.Commit()
 }
 
 func (issue *Issue) addLabels(e *xorm.Session, labels []*Label) error {
@@ -207,8 +272,13 @@ func (issue *Issue) addLabels(e *xorm.Session, labels []*Label) error {
 }
 
 // AddLabels adds a list of new labels to the issue.
-func (issue *Issue) AddLabels(labels []*Label) error {
+func (issue *Issue) AddLabels(doer *User, labels []*Label) error {
-	return NewIssueLabels(issue, labels)
+	if err := NewIssueLabels(issue, labels); err != nil {
+		return err
+	}
+
+	issue.sendLabelUpdatedWebhook(doer)
+	return nil
 }
 
 func (issue *Issue) getLabels(e Engine) (err error) {
@@ -228,8 +298,13 @@ func (issue *Issue) removeLabel(e *xorm.Session, label *Label) error {
 }
 
 // RemoveLabel removes a label from issue by given ID.
-func (issue *Issue) RemoveLabel(label *Label) (err error) {
+func (issue *Issue) RemoveLabel(doer *User, label *Label) error {
-	return DeleteIssueLabel(issue, label)
+	if err := DeleteIssueLabel(issue, label); err != nil {
+		return err
+	}
+
+	issue.sendLabelUpdatedWebhook(doer)
+	return nil
 }
 
 func (issue *Issue) clearLabels(e *xorm.Session) (err error) {
@@ -246,7 +321,7 @@ func (issue *Issue) clearLabels(e *xorm.Session) (err error) {
 	return nil
 }
 
-func (issue *Issue) ClearLabels() (err error) {
+func (issue *Issue) ClearLabels(doer *User) (err error) {
 	sess := x.NewSession()
 	defer sessionRelease(sess)
 	if err = sess.Begin(); err != nil {
@@ -257,7 +332,27 @@ func (issue *Issue) ClearLabels() (err error) {
 		return err
 	}
 
-	return sess.Commit()
+	if err = sess.Commit(); err != nil {
+		return fmt.Errorf("Commit: %v", err)
+	}
+
+	if issue.IsPull {
+		issue.PullRequest.Issue = issue
+		err = PrepareWebhooks(issue.Repo, HOOK_EVENT_PULL_REQUEST, &api.PullRequestPayload{
+			Action:      api.HOOK_ISSUE_LABEL_CLEARED,
+			Index:       issue.Index,
+			PullRequest: issue.PullRequest.APIFormat(),
+			Repository:  issue.Repo.APIFormat(nil),
+			Sender:      doer.APIFormat(),
+		})
+	}
+	if err != nil {
+		log.Error(4, "PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err)
+	} else {
+		go HookQueue.Add(issue.RepoID)
+	}
+
+	return nil
 }
 
 // ReplaceLabels removes all current labels and add new labels to the issue.
@@ -294,6 +389,16 @@ func (i *Issue) ReadBy(uid int64) error {
 	return UpdateIssueUserByRead(uid, i.ID)
 }
 
+func updateIssueCols(e Engine, issue *Issue, cols ...string) error {
+	_, err := e.Id(issue.ID).Cols(cols...).Update(issue)
+	return err
+}
+
+// UpdateIssueCols only updates values of specific columns for given issue.
+func UpdateIssueCols(issue *Issue, cols ...string) error {
+	return updateIssueCols(x, issue, cols...)
+}
+
 func (i *Issue) changeStatus(e *xorm.Session, doer *User, repo *Repository, isClosed bool) (err error) {
 	// Nothing should be performed if current status is same as target status
 	if i.IsClosed == isClosed {
@@ -336,32 +441,149 @@ func (i *Issue) changeStatus(e *xorm.Session, doer *User, repo *Repository, isCl
 }
 
 // ChangeStatus changes issue status to open or closed.
-func (i *Issue) ChangeStatus(doer *User, repo *Repository, isClosed bool) (err error) {
+func (issue *Issue) ChangeStatus(doer *User, repo *Repository, isClosed bool) (err error) {
 	sess := x.NewSession()
 	defer sessionRelease(sess)
 	if err = sess.Begin(); err != nil {
 		return err
 	}
 
-	if err = i.changeStatus(sess, doer, repo, isClosed); err != nil {
+	if err = issue.changeStatus(sess, doer, repo, isClosed); err != nil {
 		return err
 	}
 
-	return sess.Commit()
+	if err = sess.Commit(); err != nil {
+		return fmt.Errorf("Commit: %v", err)
+	}
+
+	if issue.IsPull {
+		// Merge pull request calls issue.changeStatus so we need to handle separately.
+		issue.PullRequest.Issue = issue
+		apiPullRequest := &api.PullRequestPayload{
+			Index:       issue.Index,
+			PullRequest: issue.PullRequest.APIFormat(),
+			Repository:  repo.APIFormat(nil),
+			Sender:      doer.APIFormat(),
+		}
+		if isClosed {
+			apiPullRequest.Action = api.HOOK_ISSUE_CLOSED
+		} else {
+			apiPullRequest.Action = api.HOOK_ISSUE_REOPENED
+		}
+		err = PrepareWebhooks(repo, HOOK_EVENT_PULL_REQUEST, apiPullRequest)
+	}
+	if err != nil {
+		log.Error(4, "PrepareWebhooks [is_pull: %v, is_closed: %v]: %v", issue.IsPull, isClosed, err)
+	} else {
+		go HookQueue.Add(repo.ID)
 	}
 
-func (i *Issue) GetPullRequest() (err error) {
-	if i.PullRequest != nil {
 	return nil
 }
 
-	i.PullRequest, err = GetPullRequestByIssueID(i.ID)
+func (issue *Issue) ChangeTitle(doer *User, title string) (err error) {
-	return err
+	oldTitle := issue.Title
+	issue.Title = title
+	if err = UpdateIssueCols(issue, "name"); err != nil {
+		return fmt.Errorf("UpdateIssueCols: %v", err)
+	}
+
+	if issue.IsPull {
+		issue.PullRequest.Issue = issue
+		err = PrepareWebhooks(issue.Repo, HOOK_EVENT_PULL_REQUEST, &api.PullRequestPayload{
+			Action: api.HOOK_ISSUE_EDITED,
+			Index:  issue.Index,
+			Changes: &api.ChangesPayload{
+				Title: &api.ChangesFromPayload{
+					From: oldTitle,
+				},
+			},
+			PullRequest: issue.PullRequest.APIFormat(),
+			Repository:  issue.Repo.APIFormat(nil),
+			Sender:      doer.APIFormat(),
+		})
+	}
+	if err != nil {
+		log.Error(4, "PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err)
+	} else {
+		go HookQueue.Add(issue.RepoID)
+	}
+
+	return nil
+}
+
+func (issue *Issue) ChangeContent(doer *User, content string) (err error) {
+	oldContent := issue.Content
+	issue.Content = content
+	if err = UpdateIssueCols(issue, "content"); err != nil {
+		return fmt.Errorf("UpdateIssueCols: %v", err)
+	}
+
+	if issue.IsPull {
+		issue.PullRequest.Issue = issue
+		err = PrepareWebhooks(issue.Repo, HOOK_EVENT_PULL_REQUEST, &api.PullRequestPayload{
+			Action: api.HOOK_ISSUE_EDITED,
+			Index:  issue.Index,
+			Changes: &api.ChangesPayload{
+				Body: &api.ChangesFromPayload{
+					From: oldContent,
+				},
+			},
+			PullRequest: issue.PullRequest.APIFormat(),
+			Repository:  issue.Repo.APIFormat(nil),
+			Sender:      doer.APIFormat(),
+		})
+	}
+	if err != nil {
+		log.Error(4, "PrepareWebhooks [is_pull: %v]: %v", issue.IsPull, err)
+	} else {
+		go HookQueue.Add(issue.RepoID)
+	}
+
+	return nil
+}
+
+func (issue *Issue) ChangeAssignee(doer *User, assigneeID int64) (err error) {
+	issue.AssigneeID = assigneeID
+	if err = UpdateIssueUserByAssignee(issue); err != nil {
+		return fmt.Errorf("UpdateIssueUserByAssignee: %v", err)
+	}
+
+	issue.Assignee, err = GetUserByID(issue.AssigneeID)
+	if err != nil && !IsErrUserNotExist(err) {
+		log.Error(4, "GetUserByID [assignee_id: %v]: %v", issue.AssigneeID, err)
+		return nil
+	}
+
+	// Error not nil here means user does not exist, which is remove assignee.
+	isRemoveAssignee := err != nil
+	if issue.IsPull {
+		issue.PullRequest.Issue = issue
+		apiPullRequest := &api.PullRequestPayload{
+			Index:       issue.Index,
+			PullRequest: issue.PullRequest.APIFormat(),
+			Repository:  issue.Repo.APIFormat(nil),
+			Sender:      doer.APIFormat(),
+		}
+		if isRemoveAssignee {
+			apiPullRequest.Action = api.HOOK_ISSUE_UNASSIGNED
+		} else {
+			apiPullRequest.Action = api.HOOK_ISSUE_ASSIGNED
+		}
+		err = PrepareWebhooks(issue.Repo, HOOK_EVENT_PULL_REQUEST, apiPullRequest)
+	}
+	if err != nil {
+		log.Error(4, "PrepareWebhooks [is_pull: %v, remove_assignee: %v]: %v", issue.IsPull, isRemoveAssignee, err)
+	} else {
+		go HookQueue.Add(issue.RepoID)
+	}
+
+	return nil
 }
 
 // It's caller's responsibility to create action.
 func newIssue(e *xorm.Session, repo *Repository, issue *Issue, labelIDs []int64, uuids []string, isPull bool) (err error) {
-	issue.Name = strings.TrimSpace(issue.Name)
+	issue.Title = strings.TrimSpace(issue.Title)
 	issue.Index = repo.NextIssueIndex()
 
 	if issue.AssigneeID > 0 {
@@ -462,7 +684,7 @@ func NewIssue(repo *Repository, issue *Issue, labelIDs []int64, uuids []string)
 		ActUserName:  issue.Poster.Name,
 		ActEmail:     issue.Poster.Email,
 		OpType:       ACTION_CREATE_ISSUE,
-		Content:      fmt.Sprintf("%d|%s", issue.Index, issue.Name),
+		Content:      fmt.Sprintf("%d|%s", issue.Index, issue.Title),
 		RepoID:       repo.ID,
 		RepoUserName: repo.Owner.Name,
 		RepoName:     repo.Name,
@@ -518,10 +740,9 @@ func GetIssueByIndex(repoID, index int64) (*Issue, error) {
 	return issue, issue.LoadAttributes()
 }
 
-// GetIssueByID returns an issue by given ID.
+func getIssueByID(e Engine, id int64) (*Issue, error) {
-func GetIssueByID(id int64) (*Issue, error) {
 	issue := new(Issue)
-	has, err := x.Id(id).Get(issue)
+	has, err := e.Id(id).Get(issue)
 	if err != nil {
 		return nil, err
 	} else if !has {
@@ -530,6 +751,11 @@ func GetIssueByID(id int64) (*Issue, error) {
 	return issue, issue.LoadAttributes()
 }
 
+// GetIssueByID returns an issue by given ID.
+func GetIssueByID(id int64) (*Issue, error) {
+	return getIssueByID(x, id)
+}
+
 type IssuesOptions struct {
 	UserID      int64
 	AssigneeID  int64
@@ -970,12 +1196,6 @@ func UpdateIssue(issue *Issue) error {
 	return updateIssue(x, issue)
 }
 
-// updateIssueCols only updates values of specific columns for given issue.
-func updateIssueCols(e Engine, issue *Issue, cols ...string) error {
-	_, err := e.Id(issue.ID).Cols(cols...).Update(issue)
-	return err
-}
-
 func updateIssueUsersByStatus(e Engine, issueID int64, isClosed bool) error {
 	_, err := e.Exec("UPDATE `issue_user` SET is_closed=? WHERE issue_id=?", isClosed, issueID)
 	return err
@@ -1112,11 +1332,29 @@ func (m *Milestone) AfterSet(colName string, _ xorm.Cell) {
 }
 
 // State returns string representation of milestone status.
-func (m *Milestone) State() string {
+func (m *Milestone) State() api.StateType {
 	if m.IsClosed {
-		return "closed"
+		return api.STATE_CLOSED
+	}
+	return api.STATE_OPEN
+}
+
+func (m *Milestone) APIFormat() *api.Milestone {
+	apiMilestone := &api.Milestone{
+		ID:           m.ID,
+		State:        m.State(),
+		Title:        m.Name,
+		Description:  m.Content,
+		OpenIssues:   m.NumOpenIssues,
+		ClosedIssues: m.NumClosedIssues,
+	}
+	if m.IsClosed {
+		apiMilestone.Closed = &m.ClosedDate
+	}
+	if m.Deadline.Year() < 9999 {
+		apiMilestone.Deadline = &m.Deadline
 	}
-	return "open"
+	return apiMilestone
 }
 
 // NewMilestone creates new milestone of repository.

models/issue_comment.go

@@ -86,7 +86,7 @@ func (c *Comment) AfterSet(colName string, _ xorm.Cell) {
 		if err != nil {
 			if IsErrUserNotExist(err) {
 				c.PosterID = -1
-				c.Poster = NewFakeUser()
+				c.Poster = NewGhostUser()
 			} else {
 				log.Error(3, "GetUserByID[%d]: %v", c.ID, err)
 			}

models/issue_label.go

@@ -12,6 +12,8 @@ import (
 
 	"github.com/go-xorm/xorm"
 
+	api "github.com/gogits/go-gogs-client"
+
 	"github.com/gogits/gogs/modules/base"
 )
 
@@ -27,9 +29,17 @@ type Label struct {
 	IsChecked       bool `xorm:"-"`
 }
 
+func (label *Label) APIFormat() *api.Label {
+	return &api.Label{
+		ID:    label.ID,
+		Name:  label.Name,
+		Color: label.Color,
+	}
+}
+
 // CalOpenIssues calculates the open issues of label.
-func (m *Label) CalOpenIssues() {
+func (label *Label) CalOpenIssues() {
-	m.NumOpenIssues = m.NumIssues - m.NumClosedIssues
+	label.NumOpenIssues = label.NumIssues - label.NumClosedIssues
 }
 
 // ForegroundColor calculates the text color for labels based

models/issue_mail.go

@@ -15,7 +15,7 @@ import (
 )
 
 func (issue *Issue) MailSubject() string {
-	return fmt.Sprintf("[%s] %s (#%d)", issue.Repo.Name, issue.Name, issue.Index)
+	return fmt.Sprintf("[%s] %s (#%d)", issue.Repo.Name, issue.Title, issue.Index)
 }
 
 // mailIssueCommentToParticipants can be used for both new issue creation and comment.

models/pull.go

@@ -68,7 +68,7 @@ func (pr *PullRequest) BeforeUpdate() {
 	pr.MergedUnix = pr.Merged.Unix()
 }
 
-// Note: don't try to get Pull because will end up recursive querying.
+// Note: don't try to get Issue because will end up recursive querying.
 func (pr *PullRequest) AfterSet(colName string, _ xorm.Cell) {
 	switch colName {
 	case "merged_unix":
@@ -80,6 +80,62 @@ func (pr *PullRequest) AfterSet(colName string, _ xorm.Cell) {
 	}
 }
 
+// Note: don't try to get Issue because will end up recursive querying.
+func (pr *PullRequest) loadAttributes(e Engine) (err error) {
+	if pr.HasMerged && pr.Merger == nil {
+		pr.Merger, err = getUserByID(e, pr.MergerID)
+		if IsErrUserNotExist(err) {
+			pr.MergerID = -1
+			pr.Merger = NewGhostUser()
+		} else if err != nil {
+			return fmt.Errorf("getUserByID [%d]: %v", pr.MergerID, err)
+		}
+	}
+
+	return nil
+}
+
+func (pr *PullRequest) LoadAttributes() error {
+	return pr.loadAttributes(x)
+}
+
+func (pr *PullRequest) LoadIssue() (err error) {
+	pr.Issue, err = GetIssueByID(pr.IssueID)
+	return err
+}
+
+// This method assumes following fields have been assigned with valid values:
+// Required - Issue
+// Optional - Merger
+func (pr *PullRequest) APIFormat() *api.PullRequest {
+	apiIssue := pr.Issue.APIFormat()
+	apiPullRequest := &api.PullRequest{
+		ID:        pr.ID,
+		Index:     pr.Index,
+		State:     apiIssue.State,
+		Title:     apiIssue.Title,
+		Body:      apiIssue.Body,
+		User:      apiIssue.User,
+		Labels:    apiIssue.Labels,
+		Milestone: apiIssue.Milestone,
+		Assignee:  apiIssue.Assignee,
+		Comments:  apiIssue.Comments,
+		HasMerged: pr.HasMerged,
+	}
+
+	if pr.Status != PULL_REQUEST_STATUS_CHECKING {
+		mergeable := pr.Status != PULL_REQUEST_STATUS_CONFLICT
+		apiPullRequest.Mergeable = &mergeable
+	}
+	if pr.HasMerged {
+		apiPullRequest.Merged = &pr.Merged
+		apiPullRequest.MergedCommitID = &pr.MergedCommitID
+		apiPullRequest.MergedBy = pr.Merger.APIFormat()
+	}
+
+	return apiPullRequest
+}
+
 func (pr *PullRequest) getHeadRepo(e Engine) (err error) {
 	pr.HeadRepo, err = getRepositoryByID(e, pr.HeadRepoID)
 	if err != nil && !IsErrRepoNotExist(err) {
@@ -88,7 +144,7 @@ func (pr *PullRequest) getHeadRepo(e Engine) (err error) {
 	return nil
 }
 
-func (pr *PullRequest) GetHeadRepo() (err error) {
+func (pr *PullRequest) GetHeadRepo() error {
 	return pr.getHeadRepo(x)
 }
 
@@ -104,21 +160,6 @@ func (pr *PullRequest) GetBaseRepo() (err error) {
 	return nil
 }
 
-func (pr *PullRequest) GetMerger() (err error) {
-	if !pr.HasMerged || pr.Merger != nil {
-		return nil
-	}
-
-	pr.Merger, err = GetUserByID(pr.MergerID)
-	if IsErrUserNotExist(err) {
-		pr.MergerID = -1
-		pr.Merger = NewFakeUser()
-	} else if err != nil {
-		return fmt.Errorf("GetUserByID: %v", err)
-	}
-	return nil
-}
-
 // IsChecking returns true if this pull request is still checking conflict.
 func (pr *PullRequest) IsChecking() bool {
 	return pr.Status == PULL_REQUEST_STATUS_CHECKING
@@ -130,6 +171,7 @@ func (pr *PullRequest) CanAutoMerge() bool {
 }
 
 // Merge merges pull request to base repository.
+// FIXME: add repoWorkingPull make sure two merges does not happen at same time.
 func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository) (err error) {
 	if err = pr.GetHeadRepo(); err != nil {
 		return fmt.Errorf("GetHeadRepo: %v", err)
@@ -137,6 +179,11 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository) (err error
 		return fmt.Errorf("GetBaseRepo: %v", err)
 	}
 
+	defer func() {
+		go HookQueue.Add(pr.BaseRepo.ID)
+		go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false)
+	}()
+
 	sess := x.NewSession()
 	defer sessionRelease(sess)
 	if err = sess.Begin(); err != nil {
@@ -152,21 +199,6 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository) (err error
 	if err != nil {
 		return fmt.Errorf("OpenRepository: %v", err)
 	}
-	pr.MergedCommitID, err = headGitRepo.GetBranchCommitID(pr.HeadBranch)
-	if err != nil {
-		return fmt.Errorf("GetBranchCommitID: %v", err)
-	}
-
-	if err = mergePullRequestAction(sess, doer, pr.Issue.Repo, pr.Issue); err != nil {
-		return fmt.Errorf("mergePullRequestAction: %v", err)
-	}
-
-	pr.HasMerged = true
-	pr.Merged = time.Now()
-	pr.MergerID = doer.ID
-	if _, err = sess.Id(pr.ID).AllCols().Update(pr); err != nil {
-		return fmt.Errorf("update pull request: %v", err)
-	}
 
 	// Clone base repo.
 	tmpBasePath := path.Join(setting.AppDataPath, "tmp/repos", com.ToStr(time.Now().Nanosecond())+".git")
@@ -222,15 +254,59 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository) (err error
 		return fmt.Errorf("git push: %s", stderr)
 	}
 
+	pr.MergedCommitID, err = headGitRepo.GetBranchCommitID(pr.HeadBranch)
+	if err != nil {
+		return fmt.Errorf("GetBranchCommit: %v", err)
+	}
+
+	pr.HasMerged = true
+	pr.Merged = time.Now()
+	pr.MergerID = doer.ID
+	if _, err = sess.Id(pr.ID).AllCols().Update(pr); err != nil {
+		return fmt.Errorf("update pull request: %v", err)
+	}
+
 	if err = sess.Commit(); err != nil {
 		return fmt.Errorf("Commit: %v", err)
 	}
 
-	// Compose commit repository action
+	if err = MergePullRequestAction(doer, pr.Issue.Repo, pr.Issue); err != nil {
+		log.Error(4, "MergePullRequestAction [%d]: %v", pr.ID, err)
+	}
+
+	// Reload pull request information.
+	if err = pr.LoadAttributes(); err != nil {
+		log.Error(4, "LoadAttributes: %v", err)
+		return nil
+	}
+	if err = PrepareWebhooks(pr.Issue.Repo, HOOK_EVENT_PULL_REQUEST, &api.PullRequestPayload{
+		Action:      api.HOOK_ISSUE_CLOSED,
+		Index:       pr.Index,
+		PullRequest: pr.APIFormat(),
+		Repository:  pr.Issue.Repo.APIFormat(nil),
+		Sender:      doer.APIFormat(),
+	}); err != nil {
+		log.Error(4, "PrepareWebhooks: %v", err)
+		return nil
+	}
+
 	l, err := headGitRepo.CommitsBetweenIDs(pr.MergedCommitID, pr.MergeBase)
 	if err != nil {
-		return fmt.Errorf("CommitsBetween: %v", err)
+		log.Error(4, "CommitsBetweenIDs: %v", err)
+		return nil
+	}
+
+	// TODO: when squash commits, no need to append merge commit.
+	// It is possible that head branch is not fully sync with base branch for merge commits,
+	// so we need to get latest head commit and append merge commit manully
+	// to avoid strange diff commits produced.
+	mergeCommit, err := baseGitRepo.GetBranchCommit(pr.BaseBranch)
+	if err != nil {
+		log.Error(4, "GetBranchCommit: %v", err)
+		return nil
 	}
+	l.PushFront(mergeCommit)
+
 	p := &api.PushPayload{
 		Ref:        "refs/heads/" + pr.BaseBranch,
 		Before:     pr.MergeBase,
@@ -252,8 +328,6 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository) (err error
 	if err = PrepareWebhooks(pr.BaseRepo, HOOK_EVENT_PUSH, p); err != nil {
 		return fmt.Errorf("PrepareWebhooks: %v", err)
 	}
-	go HookQueue.Add(pr.BaseRepo.ID)
-	go AddTestPullRequestTask(pr.BaseRepo.ID, pr.BaseBranch)
 	return nil
 }
 
@@ -331,19 +405,6 @@ func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []str
 		return fmt.Errorf("newIssue: %v", err)
 	}
 
-	// Notify watchers.
-	act := &Action{
-		ActUserID:    pull.Poster.ID,
-		ActUserName:  pull.Poster.Name,
-		ActEmail:     pull.Poster.Email,
-		OpType:       ACTION_CREATE_PULL_REQUEST,
-		Content:      fmt.Sprintf("%d|%s", pull.Index, pull.Name),
-		RepoID:       repo.ID,
-		RepoUserName: repo.Owner.Name,
-		RepoName:     repo.Name,
-		IsPrivate:    repo.IsPrivate,
-	}
-
 	pr.Index = pull.Index
 	if err = repo.SavePatch(pr.Index, patch); err != nil {
 		return fmt.Errorf("SavePatch: %v", err)
@@ -353,6 +414,7 @@ func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []str
 	if err = pr.testPatch(); err != nil {
 		return fmt.Errorf("testPatch: %v", err)
 	}
+	// No conflict appears after test means mergeable.
 	if pr.Status == PULL_REQUEST_STATUS_CHECKING {
 		pr.Status = PULL_REQUEST_STATUS_MERGEABLE
 	}
@@ -366,12 +428,35 @@ func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []str
 		return fmt.Errorf("Commit: %v", err)
 	}
 
-	if err = NotifyWatchers(act); err != nil {
+	if err = NotifyWatchers(&Action{
+		ActUserID:    pull.Poster.ID,
+		ActUserName:  pull.Poster.Name,
+		ActEmail:     pull.Poster.Email,
+		OpType:       ACTION_CREATE_PULL_REQUEST,
+		Content:      fmt.Sprintf("%d|%s", pull.Index, pull.Title),
+		RepoID:       repo.ID,
+		RepoUserName: repo.Owner.Name,
+		RepoName:     repo.Name,
+		IsPrivate:    repo.IsPrivate,
+	}); err != nil {
 		log.Error(4, "NotifyWatchers: %v", err)
 	} else if err = pull.MailParticipants(); err != nil {
 		log.Error(4, "MailParticipants: %v", err)
 	}
 
+	pr.Issue = pull
+	pull.PullRequest = pr
+	if err = PrepareWebhooks(repo, HOOK_EVENT_PULL_REQUEST, &api.PullRequestPayload{
+		Action:      api.HOOK_ISSUE_OPENED,
+		Index:       pull.Index,
+		PullRequest: pr.APIFormat(),
+		Repository:  repo.APIFormat(nil),
+		Sender:      pull.Poster.APIFormat(),
+	}); err != nil {
+		log.Error(4, "PrepareWebhooks: %v", err)
+	}
+	go HookQueue.Add(repo.ID)
+
 	return nil
 }
 
@@ -409,30 +494,38 @@ func GetUnmergedPullRequestsByBaseInfo(repoID int64, branch string) ([]*PullRequ
 		Join("INNER", "issue", "issue.id=pull_request.issue_id").Find(&prs)
 }
 
-// GetPullRequestByID returns a pull request by given ID.
+func getPullRequestByID(e Engine, id int64) (*PullRequest, error) {
-func GetPullRequestByID(id int64) (*PullRequest, error) {
 	pr := new(PullRequest)
-	has, err := x.Id(id).Get(pr)
+	has, err := e.Id(id).Get(pr)
 	if err != nil {
 		return nil, err
 	} else if !has {
 		return nil, ErrPullRequestNotExist{id, 0, 0, 0, "", ""}
 	}
-	return pr, nil
+	return pr, pr.loadAttributes(e)
 }
 
-// GetPullRequestByIssueID returns pull request by given issue ID.
+// GetPullRequestByID returns a pull request by given ID.
-func GetPullRequestByIssueID(issueID int64) (*PullRequest, error) {
+func GetPullRequestByID(id int64) (*PullRequest, error) {
+	return getPullRequestByID(x, id)
+}
+
+func getPullRequestByIssueID(e Engine, issueID int64) (*PullRequest, error) {
 	pr := &PullRequest{
 		IssueID: issueID,
 	}
-	has, err := x.Get(pr)
+	has, err := e.Get(pr)
 	if err != nil {
 		return nil, err
 	} else if !has {
 		return nil, ErrPullRequestNotExist{0, issueID, 0, 0, "", ""}
 	}
-	return pr, nil
+	return pr, pr.loadAttributes(e)
+}
+
+// GetPullRequestByIssueID returns pull request by given issue ID.
+func GetPullRequestByIssueID(issueID int64) (*PullRequest, error) {
+	return getPullRequestByIssueID(x, issueID)
 }
 
 // Update updates all fields of pull request.
@@ -536,6 +629,37 @@ func (pr *PullRequest) AddToTaskQueue() {
 	})
 }
 
+type PullRequestList []*PullRequest
+
+func (prs PullRequestList) loadAttributes(e Engine) error {
+	if len(prs) == 0 {
+		return nil
+	}
+
+	// Load issues.
+	issueIDs := make([]int64, 0, len(prs))
+	for i := range prs {
+		issueIDs = append(issueIDs, prs[i].IssueID)
+	}
+	issues := make([]*Issue, 0, len(issueIDs))
+	if err := e.Where("id > 0").In("id", issueIDs).Find(&issues); err != nil {
+		return fmt.Errorf("find issues: %v", err)
+	}
+
+	set := make(map[int64]*Issue)
+	for i := range issues {
+		set[issues[i].ID] = issues[i]
+	}
+	for i := range prs {
+		prs[i].Issue = set[prs[i].IssueID]
+	}
+	return nil
+}
+
+func (prs PullRequestList) LoadAttributes() error {
+	return prs.loadAttributes(x)
+}
+
 func addHeadRepoTasks(prs []*PullRequest) {
 	for _, pr := range prs {
 		log.Trace("addHeadRepoTasks[%d]: composing new test task", pr.ID)
@@ -553,13 +677,41 @@ func addHeadRepoTasks(prs []*PullRequest) {
 
 // AddTestPullRequestTask adds new test tasks by given head/base repository and head/base branch,
 // and generate new patch for testing as needed.
-func AddTestPullRequestTask(repoID int64, branch string) {
+func AddTestPullRequestTask(doer *User, repoID int64, branch string, isSync bool) {
 	log.Trace("AddTestPullRequestTask [head_repo_id: %d, head_branch: %s]: finding pull requests", repoID, branch)
 	prs, err := GetUnmergedPullRequestsByHeadInfo(repoID, branch)
 	if err != nil {
 		log.Error(4, "Find pull requests [head_repo_id: %d, head_branch: %s]: %v", repoID, branch, err)
 		return
 	}
+
+	if isSync {
+		if err = PullRequestList(prs).LoadAttributes(); err != nil {
+			log.Error(4, "PullRequestList.LoadAttributes: %v", err)
+		}
+
+		if err == nil {
+			for _, pr := range prs {
+				pr.Issue.PullRequest = pr
+				if err = pr.Issue.LoadAttributes(); err != nil {
+					log.Error(4, "LoadAttributes: %v", err)
+					continue
+				}
+				if err = PrepareWebhooks(pr.Issue.Repo, HOOK_EVENT_PULL_REQUEST, &api.PullRequestPayload{
+					Action:      api.HOOK_ISSUE_SYNCHRONIZED,
+					Index:       pr.Issue.Index,
+					PullRequest: pr.Issue.PullRequest.APIFormat(),
+					Repository:  pr.Issue.Repo.APIFormat(nil),
+					Sender:      doer.APIFormat(),
+				}); err != nil {
+					log.Error(4, "PrepareWebhooks [pull_id: %v]: %v", pr.ID, err)
+					continue
+				}
+				go HookQueue.Add(pr.Issue.Repo.ID)
+			}
+		}
+	}
+
 	addHeadRepoTasks(prs)
 
 	log.Trace("AddTestPullRequestTask [base_repo_id: %d, base_branch: %s]: finding pull requests", repoID, branch)

models/repo.go

@@ -216,6 +216,48 @@ func (repo *Repository) AfterSet(colName string, _ xorm.Cell) {
 	}
 }
 
+// MustOwner always returns a valid *User object to avoid
+// conceptually impossible error handling.
+// It creates a fake object that contains error deftail
+// when error occurs.
+func (repo *Repository) MustOwner() *User {
+	return repo.mustOwner(x)
+}
+
+func (repo *Repository) FullName() string {
+	return repo.MustOwner().Name + "/" + repo.Name
+}
+
+func (repo *Repository) FullLink() string {
+	return setting.AppUrl + repo.FullName()
+}
+
+// Arguments that are allowed to be nil: permission
+func (repo *Repository) APIFormat(permission *api.Permission) *api.Repository {
+	cloneLink := repo.CloneLink()
+	return &api.Repository{
+		ID:            repo.ID,
+		Owner:         repo.Owner.APIFormat(),
+		Name:          repo.Name,
+		FullName:      repo.FullName(),
+		Description:   repo.Description,
+		Private:       repo.IsPrivate,
+		Fork:          repo.IsFork,
+		HTMLURL:       repo.FullLink(),
+		SSHURL:        cloneLink.SSH,
+		CloneURL:      cloneLink.HTTPS,
+		Website:       repo.Website,
+		Stars:         repo.NumStars,
+		Forks:         repo.NumForks,
+		Watchers:      repo.NumWatches,
+		OpenIssues:    repo.NumOpenIssues,
+		DefaultBranch: repo.DefaultBranch,
+		Created:       repo.Created,
+		Updated:       repo.Updated,
+		Permissions:   permission,
+	}
+}
+
 func (repo *Repository) getOwner(e Engine) (err error) {
 	if repo.Owner != nil {
 		return nil
@@ -240,14 +282,6 @@ func (repo *Repository) mustOwner(e Engine) *User {
 	return repo.Owner
 }
 
-// MustOwner always returns a valid *User object to avoid
-// conceptually impossible error handling.
-// It creates a fake object that contains error deftail
-// when error occurs.
-func (repo *Repository) MustOwner() *User {
-	return repo.mustOwner(x)
-}
-
 // ComposeMetas composes a map of metas for rendering external issue tracker URL.
 func (repo *Repository) ComposeMetas() map[string]string {
 	if !repo.EnableExternalTracker {
@@ -357,10 +391,6 @@ func (repo *Repository) ComposeCompareURL(oldCommitID, newCommitID string) strin
 	return fmt.Sprintf("%s/%s/compare/%s...%s", repo.MustOwner().Name, repo.Name, oldCommitID, newCommitID)
 }
 
-func (repo *Repository) FullLink() string {
-	return setting.AppUrl + repo.MustOwner().Name + "/" + repo.Name
-}
-
 func (repo *Repository) HasAccess(u *User) bool {
 	has, _ := HasAccess(u, repo, ACCESS_MODE_READ)
 	return has

models/user.go

@@ -25,6 +25,7 @@ import (
 	"github.com/nfnt/resize"
 
 	"github.com/gogits/git-module"
+	api "github.com/gogits/go-gogs-client"
 
 	"github.com/gogits/gogs/modules/avatar"
 	"github.com/gogits/gogs/modules/base"
@@ -130,6 +131,16 @@ func (u *User) AfterSet(colName string, _ xorm.Cell) {
 	}
 }
 
+func (u *User) APIFormat() *api.User {
+	return &api.User{
+		ID:        u.ID,
+		UserName:  u.Name,
+		FullName:  u.FullName,
+		Email:     u.Email,
+		AvatarUrl: u.AvatarLink(),
+	}
+}
+
 // returns true if user login type is LOGIN_PLAIN.
 func (u *User) IsLocal() bool {
 	return u.LoginType <= LOGIN_PLAIN
@@ -468,12 +479,12 @@ func GetUserSalt() string {
 	return base.GetRandomString(10)
 }
 
-// NewFakeUser creates and returns a fake user for someone has deleted his/her account.
+// NewGhostUser creates and returns a fake user for someone has deleted his/her account.
-func NewFakeUser() *User {
+func NewGhostUser() *User {
 	return &User{
 		ID:        -1,
-		Name:      "Someone",
+		Name:      "Ghost",
-		LowerName: "someone",
+		LowerName: "ghost",
 	}
 }
 

models/webhook.go

@@ -60,6 +60,7 @@ func IsValidHookContentType(name string) bool {
 type HookEvents struct {
 	Create      bool `json:"create"`
 	Push        bool `json:"push"`
+	PullRequest bool `json:"pull_request"`
 }
 
 // HookEvent represents events that will delivery hook.
@@ -157,6 +158,12 @@ func (w *Webhook) HasPushEvent() bool {
 		(w.ChooseEvents && w.HookEvents.Push)
 }
 
+// HasPullRequestEvent returns true if hook enabled pull request event.
+func (w *Webhook) HasPullRequestEvent() bool {
+	return w.SendEverything ||
+		(w.ChooseEvents && w.HookEvents.PullRequest)
+}
+
 func (w *Webhook) EventsArray() []string {
 	events := make([]string, 0, 2)
 	if w.HasCreateEvent() {
@@ -311,6 +318,7 @@ type HookEventType string
 const (
 	HOOK_EVENT_CREATE       HookEventType = "create"
 	HOOK_EVENT_PUSH         HookEventType = "push"
+	HOOK_EVENT_PULL_REQUEST HookEventType = "pull_request"
 )
 
 // HookRequest represents hook task request information.
@@ -422,17 +430,13 @@ func UpdateHookTask(t *HookTask) error {
 
 // PrepareWebhooks adds new webhooks to task queue for given payload.
 func PrepareWebhooks(repo *Repository, event HookEventType, p api.Payloader) error {
-	if err := repo.GetOwner(); err != nil {
-		return fmt.Errorf("GetOwner: %v", err)
-	}
-
 	ws, err := GetActiveWebhooksByRepoID(repo.ID)
 	if err != nil {
 		return fmt.Errorf("GetActiveWebhooksByRepoID: %v", err)
 	}
 
 	// check if repo belongs to org and append additional webhooks
-	if repo.Owner.IsOrganization() {
+	if repo.MustOwner().IsOrganization() {
 		// get hooks for org
 		orgws, err := GetActiveWebhooksByOrgID(repo.OwnerID)
 		if err != nil {
@@ -456,6 +460,10 @@ func PrepareWebhooks(repo *Repository, event HookEventType, p api.Payloader) err
 			if !w.HasPushEvent() {
 				continue
 			}
+		case HOOK_EVENT_PULL_REQUEST:
+			if !w.HasPullRequestEvent() {
+				continue
+			}
 		}
 
 		// Use separate objects so modifcations won't be made on payload on non-Gogs type hooks.
@@ -477,7 +485,7 @@ func PrepareWebhooks(repo *Repository, event HookEventType, p api.Payloader) err
 			URL:         w.URL,
 			Payloader:   payloader,
 			ContentType: w.ContentType,
-			EventType:   HOOK_EVENT_PUSH,
+			EventType:   event,
 			IsSSL:       w.IsSSL,
 		}); err != nil {
 			return fmt.Errorf("CreateHookTask: %v", err)

models/webhook_slack.go

@@ -12,6 +12,8 @@ import (
 
 	"github.com/gogits/git-module"
 	api "github.com/gogits/go-gogs-client"
+
+	"github.com/gogits/gogs/modules/setting"
 )
 
 type SlackMeta struct {
@@ -34,6 +36,7 @@ type SlackPayload struct {
 type SlackAttachment struct {
 	Fallback string `json:"fallback"`
 	Color    string `json:"color"`
+	Title    string `json:"title"`
 	Text     string `json:"text"`
 }
 
@@ -49,13 +52,20 @@ func (p *SlackPayload) JSONPayload() ([]byte, error) {
 
 // see: https://api.slack.com/docs/formatting
 func SlackTextFormatter(s string) string {
-	// take only first line of commit
-	first := strings.Split(s, "\n")[0]
 	// replace & < >
-	first = strings.Replace(first, "&", "&amp;", -1)
+	s = strings.Replace(s, "&", "&amp;", -1)
-	first = strings.Replace(first, "<", "&lt;", -1)
+	s = strings.Replace(s, "<", "&lt;", -1)
-	first = strings.Replace(first, ">", "&gt;", -1)
+	s = strings.Replace(s, ">", "&gt;", -1)
-	return first
+	return s
+}
+
+func SlackShortTextFormatter(s string) string {
+	s = strings.Split(s, "\n")[0]
+	// replace & < >
+	s = strings.Replace(s, "&", "&amp;", -1)
+	s = strings.Replace(s, "<", "&lt;", -1)
+	s = strings.Replace(s, ">", "&gt;", -1)
+	return s
 }
 
 func SlackLinkFormatter(url string, text string) string {
@@ -104,24 +114,70 @@ func getSlackPushPayload(p *api.PushPayload, slack *SlackMeta) (*SlackPayload, e
 	var attachmentText string
 	// for each commit, generate attachment text
 	for i, commit := range p.Commits {
-		attachmentText += fmt.Sprintf("%s: %s - %s", SlackLinkFormatter(commit.URL, commit.ID[:7]), SlackTextFormatter(commit.Message), SlackTextFormatter(commit.Author.Name))
+		attachmentText += fmt.Sprintf("%s: %s - %s", SlackLinkFormatter(commit.URL, commit.ID[:7]), SlackShortTextFormatter(commit.Message), SlackTextFormatter(commit.Author.Name))
 		// add linebreak to each commit but the last
 		if i < len(p.Commits)-1 {
 			attachmentText += "\n"
 		}
 	}
 
-	slackAttachments := []SlackAttachment{{
+	return &SlackPayload{
+		Channel:  slack.Channel,
+		Text:     text,
+		Username: slack.Username,
+		IconURL:  slack.IconURL,
+		Attachments: []SlackAttachment{{
 			Color: slack.Color,
 			Text:  attachmentText,
-	}}
+		}},
+	}, nil
+}
+
+func getSlackPullRequestPayload(p *api.PullRequestPayload, slack *SlackMeta) (*SlackPayload, error) {
+	senderLink := SlackLinkFormatter(setting.AppUrl+p.Sender.UserName, p.Sender.UserName)
+	titleLink := SlackLinkFormatter(fmt.Sprintf("%s/%d", setting.AppUrl+p.Repository.FullName+"/pulls", p.Index),
+		fmt.Sprintf("#%d %s", p.Index, p.PullRequest.Title))
+	var text, title, attachmentText string
+	switch p.Action {
+	case api.HOOK_ISSUE_OPENED:
+		text = fmt.Sprintf("[%s] Pull request submitted by %s", p.Repository.FullName, senderLink)
+		title = titleLink
+		attachmentText = SlackTextFormatter(p.PullRequest.Body)
+	case api.HOOK_ISSUE_CLOSED:
+		if p.PullRequest.HasMerged {
+			text = fmt.Sprintf("[%s] Pull request merged: %s by %s", p.Repository.FullName, titleLink, senderLink)
+		} else {
+			text = fmt.Sprintf("[%s] Pull request closed: %s by %s", p.Repository.FullName, titleLink, senderLink)
+		}
+	case api.HOOK_ISSUE_REOPENED:
+		text = fmt.Sprintf("[%s] Pull request re-opened: %s by %s", p.Repository.FullName, titleLink, senderLink)
+	case api.HOOK_ISSUE_EDITED:
+		text = fmt.Sprintf("[%s] Pull request edited: %s by %s", p.Repository.FullName, titleLink, senderLink)
+		attachmentText = SlackTextFormatter(p.PullRequest.Body)
+	case api.HOOK_ISSUE_ASSIGNED:
+		text = fmt.Sprintf("[%s] Pull request assigned to %s: %s by %s", p.Repository.FullName,
+			SlackLinkFormatter(setting.AppUrl+p.PullRequest.Assignee.UserName, p.PullRequest.Assignee.UserName),
+			titleLink, senderLink)
+	case api.HOOK_ISSUE_UNASSIGNED:
+		text = fmt.Sprintf("[%s] Pull request unassigned: %s by %s", p.Repository.FullName, titleLink, senderLink)
+	case api.HOOK_ISSUE_LABEL_UPDATED:
+		text = fmt.Sprintf("[%s] Pull request labels updated: %s by %s", p.Repository.FullName, titleLink, senderLink)
+	case api.HOOK_ISSUE_LABEL_CLEARED:
+		text = fmt.Sprintf("[%s] Pull request labels cleared: %s by %s", p.Repository.FullName, titleLink, senderLink)
+	case api.HOOK_ISSUE_SYNCHRONIZED:
+		text = fmt.Sprintf("[%s] Pull request synchronized: %s by %s", p.Repository.FullName, titleLink, senderLink)
+	}
 
 	return &SlackPayload{
 		Channel:  slack.Channel,
 		Text:     text,
 		Username: slack.Username,
 		IconURL:  slack.IconURL,
-		Attachments: slackAttachments,
+		Attachments: []SlackAttachment{{
+			Color: slack.Color,
+			Title: title,
+			Text:  attachmentText,
+		}},
 	}, nil
 }
 
@@ -138,6 +194,8 @@ func GetSlackPayload(p api.Payloader, event HookEventType, meta string) (*SlackP
 		return getSlackCreatePayload(p.(*api.CreatePayload), slack)
 	case HOOK_EVENT_PUSH:
 		return getSlackPushPayload(p.(*api.PushPayload), slack)
+	case HOOK_EVENT_PULL_REQUEST:
+		return getSlackPullRequestPayload(p.(*api.PullRequestPayload), slack)
 	}
 
 	return s, nil

modules/auth/repo_form.go

@@ -116,6 +116,7 @@ type WebhookForm struct {
 	Events      string
 	Create      bool
 	Push        bool
+	PullRequest bool
 	Active      bool
 }
 

public/css/gogs.css

@@ -2064,6 +2064,14 @@ footer .ui.language .menu {
   margin-left: 5px;
   margin-top: -3px;
 }
+.repository.settings.webhook .events .column {
+  padding-bottom: 0;
+}
+.repository.settings.webhook .events .help {
+  font-size: 13px;
+  margin-left: 26px;
+  padding-top: 0;
+}
 .user-cards .list {
   padding: 0;
 }

public/less/_repository.less

@@ -1090,6 +1090,19 @@
 				}
 			}
 		}
+
+		&.webhook {
+			.events {
+				.column {
+					padding-bottom: 0;
+				}
+				.help {
+					font-size: 13px;
+					margin-left: 26px;
+					padding-top: 0;
+				}
+			}
+		}
 	}
 }
 // End of .repository

routers/api/v1/convert/convert.go

@@ -13,7 +13,6 @@ import (
 	api "github.com/gogits/go-gogs-client"
 
 	"github.com/gogits/gogs/models"
-	"github.com/gogits/gogs/modules/log"
 	"github.com/gogits/gogs/modules/setting"
 )
 
@@ -48,16 +47,16 @@ func ToRepository(owner *models.User, repo *models.Repository, permission api.Pe
 		Description: repo.Description,
 		Private:     repo.IsPrivate,
 		Fork:        repo.IsFork,
-		HtmlUrl:     setting.AppUrl + owner.Name + "/" + repo.Name,
+		HTMLURL:     setting.AppUrl + owner.Name + "/" + repo.Name,
-		CloneUrl:    cl.HTTPS,
+		CloneURL:    cl.HTTPS,
-		SshUrl:      cl.SSH,
+		SSHURL:      cl.SSH,
 		OpenIssues:  repo.NumOpenIssues,
 		Stars:       repo.NumStars,
 		Forks:       repo.NumForks,
 		Watchers:    repo.NumWatches,
 		Created:     repo.Created,
 		Updated:     repo.Updated,
-		Permissions: permission,
+		Permissions: &permission,
 	}
 }
 
@@ -183,7 +182,7 @@ func ToIssue(issue *models.Issue) *api.Issue {
 		ID:        issue.ID,
 		Index:     issue.Index,
 		State:     issue.State(),
-		Title:     issue.Name,
+		Title:     issue.Title,
 		Body:      issue.Content,
 		User:      ToUser(issue.Poster),
 		Labels:    apiLabels,
@@ -194,9 +193,6 @@ func ToIssue(issue *models.Issue) *api.Issue {
 		Updated:   issue.Updated,
 	}
 	if issue.IsPull {
-		if err := issue.GetPullRequest(); err != nil {
-			log.Error(4, "GetPullRequest", err)
-		} else {
 		apiIssue.PullRequest = &api.PullRequestMeta{
 			HasMerged: issue.PullRequest.HasMerged,
 		}
@@ -204,7 +200,6 @@ func ToIssue(issue *models.Issue) *api.Issue {
 			apiIssue.PullRequest.Merged = &issue.PullRequest.Merged
 		}
 	}
-	}
 
 	return apiIssue
 }

routers/api/v1/repo/issue.go

@@ -52,7 +52,7 @@ func GetIssue(ctx *context.APIContext) {
 func CreateIssue(ctx *context.APIContext, form api.CreateIssueOption) {
 	issue := &models.Issue{
 		RepoID:   ctx.Repo.Repository.ID,
-		Name:     form.Title,
+		Title:     form.Title,
 		PosterID: ctx.User.ID,
 		Poster:   ctx.User,
 		Content:  form.Body,
@@ -115,7 +115,7 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
 	}
 
 	if len(form.Title) > 0 {
-		issue.Name = form.Title
+		issue.Title = form.Title
 	}
 	if form.Body != nil {
 		issue.Content = *form.Body

routers/api/v1/repo/issue_label.go

@@ -52,7 +52,7 @@ func AddIssueLabels(ctx *context.APIContext, form api.IssueLabelsOption) {
 		return
 	}
 
-	if err = issue.AddLabels(labels); err != nil {
+	if err = issue.AddLabels(ctx.User, labels); err != nil {
 		ctx.Error(500, "AddLabels", err)
 		return
 	}
@@ -160,7 +160,7 @@ func ClearIssueLabels(ctx *context.APIContext) {
 		return
 	}
 
-	if err := issue.ClearLabels(); err != nil {
+	if err := issue.ClearLabels(ctx.User); err != nil {
 		ctx.Error(500, "ClearLabels", err)
 		return
 	}

routers/repo/http.go

@@ -206,7 +206,7 @@ func HTTP(ctx *context.Context) {
 						RepoName:     reponame,
 					}); err == nil {
 						go models.HookQueue.Add(repo.ID)
-						go models.AddTestPullRequestTask(repo.ID, strings.TrimPrefix(refName, "refs/heads/"))
+						go models.AddTestPullRequestTask(authUser, repo.ID, strings.TrimPrefix(refName, "refs/heads/"), true)
 					}
 
 				}

routers/repo/issue.go

@@ -424,7 +424,7 @@ func NewIssuePost(ctx *context.Context, form auth.CreateIssueForm) {
 
 	issue := &models.Issue{
 		RepoID:      repo.ID,
-		Name:        form.Title,
+		Title:       form.Title,
 		PosterID:    ctx.User.ID,
 		Poster:      ctx.User,
 		MilestoneID: milestoneID,
@@ -500,7 +500,7 @@ func ViewIssue(ctx *context.Context) {
 		}
 		return
 	}
-	ctx.Data["Title"] = issue.Name
+	ctx.Data["Title"] = issue.Title
 
 	// Make sure type and URL matches.
 	if ctx.Params(":type") == "issues" && issue.IsPull {
@@ -517,12 +517,6 @@ func ViewIssue(ctx *context.Context) {
 			return
 		}
 		ctx.Data["PageIsPullList"] = true
-
-		if err = issue.GetPullRequest(); err != nil {
-			ctx.Handle(500, "GetPullRequest", err)
-			return
-		}
-
 		ctx.Data["PageIsPullConversation"] = true
 	} else {
 		MustEnableIssues(ctx)
@@ -668,19 +662,19 @@ func UpdateIssueTitle(ctx *context.Context) {
 		return
 	}
 
-	issue.Name = ctx.QueryTrim("title")
+	title := ctx.QueryTrim("title")
-	if len(issue.Name) == 0 {
+	if len(title) == 0 {
 		ctx.Error(204)
 		return
 	}
 
-	if err := models.UpdateIssue(issue); err != nil {
+	if err := issue.ChangeTitle(ctx.User, title); err != nil {
-		ctx.Handle(500, "UpdateIssue", err)
+		ctx.Handle(500, "ChangeTitle", err)
 		return
 	}
 
 	ctx.JSON(200, map[string]interface{}{
-		"title": issue.Name,
+		"title": issue.Title,
 	})
 }
 
@@ -695,9 +689,9 @@ func UpdateIssueContent(ctx *context.Context) {
 		return
 	}
 
-	issue.Content = ctx.Query("content")
+	content := ctx.Query("content")
-	if err := models.UpdateIssue(issue); err != nil {
+	if err := issue.ChangeContent(ctx.User, content); err != nil {
-		ctx.Handle(500, "UpdateIssue", err)
+		ctx.Handle(500, "ChangeContent", err)
 		return
 	}
 
@@ -713,7 +707,7 @@ func UpdateIssueLabel(ctx *context.Context) {
 	}
 
 	if ctx.Query("action") == "clear" {
-		if err := issue.ClearLabels(); err != nil {
+		if err := issue.ClearLabels(ctx.User); err != nil {
 			ctx.Handle(500, "ClearLabels", err)
 			return
 		}
@@ -730,12 +724,12 @@ func UpdateIssueLabel(ctx *context.Context) {
 		}
 
 		if isAttach && !issue.HasLabel(label.ID) {
-			if err = issue.AddLabel(label); err != nil {
+			if err = issue.AddLabel(ctx.User, label); err != nil {
 				ctx.Handle(500, "AddLabel", err)
 				return
 			}
 		} else if !isAttach && issue.HasLabel(label.ID) {
-			if err = issue.RemoveLabel(label); err != nil {
+			if err = issue.RemoveLabel(ctx.User, label); err != nil {
 				ctx.Handle(500, "RemoveLabel", err)
 				return
 			}
@@ -780,18 +774,16 @@ func UpdateIssueAssignee(ctx *context.Context) {
 		return
 	}
 
-	aid := ctx.QueryInt64("id")
+	assigneeID := ctx.QueryInt64("id")
-	if issue.AssigneeID == aid {
+	if issue.AssigneeID == assigneeID {
 		ctx.JSON(200, map[string]interface{}{
 			"ok": true,
 		})
 		return
 	}
 
-	// Not check for invalid assignee id and give responsibility to owners.
+	if err := issue.ChangeAssignee(ctx.User, assigneeID); err != nil {
-	issue.AssigneeID = aid
+		ctx.Handle(500, "ChangeAssignee", err)
-	if err := models.UpdateIssueUserByAssignee(issue); err != nil {
-		ctx.Handle(500, "UpdateIssueUserByAssignee", err)
 		return
 	}
 
@@ -806,12 +798,6 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) {
 		ctx.HandleError("GetIssueByIndex", models.IsErrIssueNotExist, err, 404)
 		return
 	}
-	if issue.IsPull {
-		if err = issue.GetPullRequest(); err != nil {
-			ctx.Handle(500, "GetPullRequest", err)
-			return
-		}
-	}
 
 	var attachments []string
 	if setting.AttachmentEnabled {

routers/repo/pull.go

@@ -148,7 +148,7 @@ func checkPullInfo(ctx *context.Context) *models.Issue {
 		}
 		return nil
 	}
-	ctx.Data["Title"] = issue.Name
+	ctx.Data["Title"] = issue.Title
 	ctx.Data["Issue"] = issue
 
 	if !issue.IsPull {
@@ -156,10 +156,7 @@ func checkPullInfo(ctx *context.Context) *models.Issue {
 		return nil
 	}
 
-	if err = issue.GetPullRequest(); err != nil {
+	if err = issue.GetHeadRepo(); err != nil {
-		ctx.Handle(500, "GetPullRequest", err)
-		return nil
-	} else if err = issue.GetHeadRepo(); err != nil {
 		ctx.Handle(500, "GetHeadRepo", err)
 		return nil
 	}
@@ -177,17 +174,10 @@ func checkPullInfo(ctx *context.Context) *models.Issue {
 
 func PrepareMergedViewPullInfo(ctx *context.Context, pull *models.Issue) {
 	ctx.Data["HasMerged"] = true
-
-	var err error
-
-	if err = pull.GetMerger(); err != nil {
-		ctx.Handle(500, "GetMerger", err)
-		return
-	}
-
 	ctx.Data["HeadTarget"] = pull.HeadUserName + "/" + pull.HeadBranch
 	ctx.Data["BaseTarget"] = ctx.Repo.Owner.Name + "/" + pull.BaseBranch
 
+	var err error
 	ctx.Data["NumCommits"], err = ctx.Repo.GitRepo.CommitsCountBetween(pull.MergeBase, pull.MergedCommitID)
 	if err != nil {
 		ctx.Handle(500, "Repo.GitRepo.CommitsCountBetween", err)
@@ -252,6 +242,7 @@ func PrepareViewPullInfo(ctx *context.Context, pull *models.Issue) *git.PullRequ
 }
 
 func ViewPullCommits(ctx *context.Context) {
+	ctx.Data["PageIsPullList"] = true
 	ctx.Data["PageIsPullCommits"] = true
 
 	pull := checkPullInfo(ctx)
@@ -302,6 +293,7 @@ func ViewPullCommits(ctx *context.Context) {
 }
 
 func ViewPullFiles(ctx *context.Context) {
+	ctx.Data["PageIsPullList"] = true
 	ctx.Data["PageIsPullFiles"] = true
 
 	pull := checkPullInfo(ctx)
@@ -679,7 +671,7 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm)
 	pullIssue := &models.Issue{
 		RepoID:      repo.ID,
 		Index:       repo.NextIssueIndex(),
-		Name:        form.Title,
+		Title:       form.Title,
 		PosterID:    ctx.User.ID,
 		Poster:      ctx.User,
 		MilestoneID: milestoneID,
@@ -711,11 +703,12 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm)
 }
 
 func TriggerTask(ctx *context.Context) {
+	pusherID := ctx.QueryInt64("pusher")
 	branch := ctx.Query("branch")
 	secret := ctx.Query("secret")
-	if len(branch) == 0 || len(secret) == 0 {
+	if len(branch) == 0 || len(secret) == 0 || pusherID <= 0 {
 		ctx.Error(404)
-		log.Trace("TriggerTask: branch or secret is empty")
+		log.Trace("TriggerTask: branch or secret is empty, or pusher ID is not valid")
 		return
 	}
 	owner, repo := parseOwnerAndRepo(ctx)
@@ -728,9 +721,19 @@ func TriggerTask(ctx *context.Context) {
 		return
 	}
 
-	log.Trace("TriggerTask [%d].(new request): %s", repo.ID, branch)
+	pusher, err := models.GetUserByID(pusherID)
+	if err != nil {
+		if models.IsErrUserNotExist(err) {
+			ctx.Error(404)
+		} else {
+			ctx.Handle(500, "GetUserByID", err)
+		}
+		return
+	}
+
+	log.Trace("TriggerTask '%s/%s' by %s", repo.Name, branch, pusher.Name)
 
 	go models.HookQueue.Add(repo.ID)
-	go models.AddTestPullRequestTask(repo.ID, branch)
+	go models.AddTestPullRequestTask(pusher, repo.ID, branch, true)
 	ctx.Status(202)
 }

routers/repo/release.go

@@ -77,7 +77,7 @@ func Releases(ctx *context.Context) {
 				r.Publisher, err = models.GetUserByID(r.PublisherID)
 				if err != nil {
 					if models.IsErrUserNotExist(err) {
-						r.Publisher = models.NewFakeUser()
+						r.Publisher = models.NewGhostUser()
 					} else {
 						ctx.Handle(500, "GetUserByID", err)
 						return
@@ -126,7 +126,7 @@ func Releases(ctx *context.Context) {
 		r.Publisher, err = models.GetUserByID(r.PublisherID)
 		if err != nil {
 			if models.IsErrUserNotExist(err) {
-				r.Publisher = models.NewFakeUser()
+				r.Publisher = models.NewGhostUser()
 			} else {
 				ctx.Handle(500, "GetUserByID", err)
 				return

routers/repo/webhook.go

@@ -110,6 +110,7 @@ func ParseHookEvent(form auth.WebhookForm) *models.HookEvent {
 		HookEvents: models.HookEvents{
 			Create:      form.Create,
 			Push:        form.Push,
+			PullRequest: form.PullRequest,
 		},
 	}
 }

templates/.VERSION

@@ -1 +1 @@
-0.9.75.0813
+0.9.76.0814

templates/repo/issue/list.tmpl

@@ -102,7 +102,7 @@
 				{{ $timeStr:= TimeSince .Created $.Lang }}
 				<li class="item">
 					<div class="ui {{if .IsRead}}black{{else}}green{{end}} label">#{{.Index}}</div>
-					<a class="title has-emoji" href="{{$.Link}}/{{.Index}}">{{.Name}}</a>
+					<a class="title has-emoji" href="{{$.Link}}/{{.Index}}">{{.Title}}</a>
 
 					{{range .Labels}}
 						<a class="ui label" href="{{$.Link}}?type={{$.ViewType}}&state={{$.State}}&labels={{.ID}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}" style="color: {{.ForegroundColor}}; background-color: {{.Color}}">{{.Name}}</a>

templates/repo/issue/view_title.tmpl

@@ -1,9 +1,9 @@
 <div class="sixteen wide column title">
 	<div class="ui grid">
 		<h1 class="twelve wide column">
-			<span class="index">#{{.Issue.Index}}</span> <span id="issue-title" class="has-emoji">{{.Issue.Name}}</span>
+			<span class="index">#{{.Issue.Index}}</span> <span id="issue-title" class="has-emoji">{{.Issue.Title}}</span>
 			<div id="edit-title-input" class="ui input" style="display: none">
-				<input value="{{.Issue.Name}}">
+				<input value="{{.Issue.Title}}">
 			</div>
 		</h1>
 		{{if .IsIssueOwner}}

templates/repo/settings/hook_settings.tmpl

@@ -42,6 +42,16 @@
 				</div>
 			</div>
 		</div>
+		<!-- Pull Request -->
+		<div class="seven wide column">
+			<div class="field">
+				<div class="ui checkbox">
+					<input class="hidden" name="pull_request" type="checkbox" tabindex="0" {{if .Webhook.PullRequest}}checked{{end}}>
+					<label>{{.i18n.Tr "repo.settings.event_pull_request"}}</label>
+					<span class="help">{{.i18n.Tr "repo.settings.event_pull_request_desc"}}</span>
+				</div>
+			</div>
+		</div>
 	</div>
 </div>