Trigger sync webhooks on UI commit (#2302)

* Trigger sync webhooks on UI commit

* Also fix UI upload/delete

Author: ethantkoenig

models/repo_editor.go

@@ -155,27 +155,29 @@ func (repo *Repository) UpdateRepoFile(doer *User, opts UpdateRepoFileOptions) (
 	}
 
 	// Simulate push event.
-	pushCommits := &PushCommits{
-		Len:     1,
-		Commits: []*PushCommit{CommitToPushCommit(commit)},
-	}
 	oldCommitID := opts.LastCommitID
 	if opts.NewBranch != opts.OldBranch {
 		oldCommitID = git.EmptySHA
 	}
-	if err := CommitRepoAction(CommitRepoActionOptions{
-		PusherName:  doer.Name,
-		RepoOwnerID: repo.MustOwner().ID,
-		RepoName:    repo.Name,
-		RefFullName: git.BranchPrefix + opts.NewBranch,
-		OldCommitID: oldCommitID,
-		NewCommitID: commit.ID.String(),
-		Commits:     pushCommits,
-	}); err != nil {
-		log.Error(4, "CommitRepoAction: %v", err)
-		return nil
-	}
 
+	if err = repo.GetOwner(); err != nil {
+		return fmt.Errorf("GetOwner: %v", err)
+	}
+	err = PushUpdate(
+		opts.NewBranch,
+		PushUpdateOptions{
+			PusherID:     doer.ID,
+			PusherName:   doer.Name,
+			RepoUserName: repo.Owner.Name,
+			RepoName:     repo.Name,
+			RefFullName:  git.BranchPrefix + opts.NewBranch,
+			OldCommitID:  oldCommitID,
+			NewCommitID:  commit.ID.String(),
+		},
+	)
+	if err != nil {
+		return fmt.Errorf("PushUpdate: %v", err)
+	}
 	return nil
 }
 
@@ -295,23 +297,29 @@ func (repo *Repository) DeleteRepoFile(doer *User, opts DeleteRepoFileOptions) (
 	}
 
 	// Simulate push event.
-	pushCommits := &PushCommits{
-		Len:     1,
-		Commits: []*PushCommit{CommitToPushCommit(commit)},
-	}
-	if err := CommitRepoAction(CommitRepoActionOptions{
-		PusherName:  doer.Name,
-		RepoOwnerID: repo.MustOwner().ID,
-		RepoName:    repo.Name,
-		RefFullName: git.BranchPrefix + opts.NewBranch,
-		OldCommitID: opts.LastCommitID,
-		NewCommitID: commit.ID.String(),
-		Commits:     pushCommits,
-	}); err != nil {
-		log.Error(4, "CommitRepoAction: %v", err)
-		return nil
+	oldCommitID := opts.LastCommitID
+	if opts.NewBranch != opts.OldBranch {
+		oldCommitID = git.EmptySHA
 	}
 
+	if err = repo.GetOwner(); err != nil {
+		return fmt.Errorf("GetOwner: %v", err)
+	}
+	err = PushUpdate(
+		opts.NewBranch,
+		PushUpdateOptions{
+			PusherID:     doer.ID,
+			PusherName:   doer.Name,
+			RepoUserName: repo.Owner.Name,
+			RepoName:     repo.Name,
+			RefFullName:  git.BranchPrefix + opts.NewBranch,
+			OldCommitID:  oldCommitID,
+			NewCommitID:  commit.ID.String(),
+		},
+	)
+	if err != nil {
+		return fmt.Errorf("PushUpdate: %v", err)
+	}
 	return nil
 }
 
@@ -534,21 +542,28 @@ func (repo *Repository) UploadRepoFiles(doer *User, opts UploadRepoFileOptions)
 	}
 
 	// Simulate push event.
-	pushCommits := &PushCommits{
-		Len:     1,
-		Commits: []*PushCommit{CommitToPushCommit(commit)},
-	}
-	if err := CommitRepoAction(CommitRepoActionOptions{
-		PusherName:  doer.Name,
-		RepoOwnerID: repo.MustOwner().ID,
-		RepoName:    repo.Name,
-		RefFullName: git.BranchPrefix + opts.NewBranch,
-		OldCommitID: opts.LastCommitID,
-		NewCommitID: commit.ID.String(),
-		Commits:     pushCommits,
-	}); err != nil {
-		log.Error(4, "CommitRepoAction: %v", err)
-		return nil
+	oldCommitID := opts.LastCommitID
+	if opts.NewBranch != opts.OldBranch {
+		oldCommitID = git.EmptySHA
+	}
+
+	if err = repo.GetOwner(); err != nil {
+		return fmt.Errorf("GetOwner: %v", err)
+	}
+	err = PushUpdate(
+		opts.NewBranch,
+		PushUpdateOptions{
+			PusherID:     doer.ID,
+			PusherName:   doer.Name,
+			RepoUserName: repo.Owner.Name,
+			RepoName:     repo.Name,
+			RefFullName:  git.BranchPrefix + opts.NewBranch,
+			OldCommitID:  oldCommitID,
+			NewCommitID:  commit.ID.String(),
+		},
+	)
+	if err != nil {
+		return fmt.Errorf("PushUpdate: %v", err)
 	}
 
 	return DeleteUploads(uploads...)

models/update.go

@@ -64,7 +64,24 @@ type PushUpdateOptions struct {
 
 // PushUpdate must be called for any push actions in order to
 // generates necessary push action history feeds.
-func PushUpdate(opts PushUpdateOptions) (repo *Repository, err error) {
+func PushUpdate(branch string, opt PushUpdateOptions) error {
+	repo, err := pushUpdate(opt)
+	if err != nil {
+		return err
+	}
+
+	pusher, err := GetUserByID(opt.PusherID)
+	if err != nil {
+		return err
+	}
+
+	log.Trace("TriggerTask '%s/%s' by %s", repo.Name, branch, pusher.Name)
+
+	go AddTestPullRequestTask(pusher, repo.ID, branch, true)
+	return nil
+}
+
+func pushUpdate(opts PushUpdateOptions) (repo *Repository, err error) {
 	isNewRef := opts.OldCommitID == git.EmptySHA
 	isDelRef := opts.NewCommitID == git.EmptySHA
 	if isNewRef && isDelRef {

routers/private/push_update.go

@@ -32,15 +32,7 @@ func PushUpdate(ctx *macaron.Context) {
 		return
 	}
 
-	repo, err := models.PushUpdate(opt)
-	if err != nil {
-		ctx.JSON(500, map[string]interface{}{
-			"err": err.Error(),
-		})
-		return
-	}
-
-	pusher, err := models.GetUserByID(opt.PusherID)
+	err := models.PushUpdate(branch, opt)
 	if err != nil {
 		if models.IsErrUserNotExist(err) {
 			ctx.Error(404)
@@ -51,10 +43,5 @@ func PushUpdate(ctx *macaron.Context) {
 		}
 		return
 	}
-
-	log.Trace("TriggerTask '%s/%s' by %s", repo.Name, branch, pusher.Name)
-
-	go models.HookQueue.Add(repo.ID)
-	go models.AddTestPullRequestTask(pusher, repo.ID, branch, true)
 	ctx.Status(202)
 }