#2246 fully support of webhooks for pull request

8 files changed, 58 insertions, 73 deletions

diff --git a/routers/api/v1/convert/convert.go b/routers/api/v1/convert/convert.go
index cda1d86b45..c3ca9fd5d1 100644
--- a/routers/api/v1/convert/convert.go
+++ b/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,
-		CloneUrl:    cl.HTTPS,
-		SshUrl:      cl.SSH,
+		HTMLURL:     setting.AppUrl + owner.Name + "/" + repo.Name,
+		CloneURL:    cl.HTTPS,
+		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,15 +193,11 @@ 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,
-			}
-			if issue.PullRequest.HasMerged {
-				apiIssue.PullRequest.Merged = &issue.PullRequest.Merged
-			}
+		apiIssue.PullRequest = &api.PullRequestMeta{
+			HasMerged: issue.PullRequest.HasMerged,
+		}
+		if issue.PullRequest.HasMerged {
+			apiIssue.PullRequest.Merged = &issue.PullRequest.Merged
 		}
 	}
 
diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go
index b105b6816c..90dc6e78e3 100644
--- a/routers/api/v1/repo/issue.go
+++ b/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
diff --git a/routers/api/v1/repo/issue_label.go b/routers/api/v1/repo/issue_label.go
index e7cc63cde8..b471d4a08d 100644
--- a/routers/api/v1/repo/issue_label.go
+++ b/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
 	}
diff --git a/routers/repo/http.go b/routers/repo/http.go
index d6abc77452..0615143e38 100644
--- a/routers/repo/http.go
+++ b/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)
 					}
 
 				}
diff --git a/routers/repo/issue.go b/routers/repo/issue.go
index c422b031c6..f3c0f19a87 100644
--- a/routers/repo/issue.go
+++ b/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")
-	if len(issue.Name) == 0 {
+	title := ctx.QueryTrim("title")
+	if len(title) == 0 {
 		ctx.Error(204)
 		return
 	}
 
-	if err := models.UpdateIssue(issue); err != nil {
-		ctx.Handle(500, "UpdateIssue", err)
+	if err := issue.ChangeTitle(ctx.User, title); err != nil {
+		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")
-	if err := models.UpdateIssue(issue); err != nil {
-		ctx.Handle(500, "UpdateIssue", err)
+	content := ctx.Query("content")
+	if err := issue.ChangeContent(ctx.User, content); err != nil {
+		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")
-	if issue.AssigneeID == aid {
+	assigneeID := ctx.QueryInt64("id")
+	if issue.AssigneeID == assigneeID {
 		ctx.JSON(200, map[string]interface{}{
 			"ok": true,
 		})
 		return
 	}
 
-	// Not check for invalid assignee id and give responsibility to owners.
-	issue.AssigneeID = aid
-	if err := models.UpdateIssueUserByAssignee(issue); err != nil {
-		ctx.Handle(500, "UpdateIssueUserByAssignee", err)
+	if err := issue.ChangeAssignee(ctx.User, assigneeID); err != nil {
+		ctx.Handle(500, "ChangeAssignee", 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 {
diff --git a/routers/repo/pull.go b/routers/repo/pull.go
index 77ee4bd348..822d77c719 100644
--- a/routers/repo/pull.go
+++ b/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 {
-		ctx.Handle(500, "GetPullRequest", err)
-		return nil
-	} else if err = issue.GetHeadRepo(); err != nil {
+	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)
 }
diff --git a/routers/repo/release.go b/routers/repo/release.go
index a00bd944cb..d612a1cec5 100644
--- a/routers/repo/release.go
+++ b/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
diff --git a/routers/repo/webhook.go b/routers/repo/webhook.go
index 8b0153e689..9b4c33e4b6 100644
--- a/routers/repo/webhook.go
+++ b/routers/repo/webhook.go
@@ -108,8 +108,9 @@ func ParseHookEvent(form auth.WebhookForm) *models.HookEvent {
 		SendEverything: form.SendEverything(),
 		ChooseEvents:   form.ChooseEvents(),
 		HookEvents: models.HookEvents{
-			Create: form.Create,
-			Push:   form.Push,
+			Create:      form.Create,
+			Push:        form.Push,
+			PullRequest: form.PullRequest,
 		},
 	}
 }