From 7907786040100593831bce1d583bb5f6e34c1a16 Mon Sep 17 00:00:00 2001 From: Ethan Koenig Date: Thu, 17 Aug 2017 00:22:08 -0700 Subject: [PATCH] Trigger sync webhooks on UI commit (#2302) * Trigger sync webhooks on UI commit * Also fix UI upload/delete --- models/repo_editor.go | 107 +++++++++++++++++++-------------- models/update.go | 19 +++++- routers/private/push_update.go | 15 +---- 3 files changed, 80 insertions(+), 61 deletions(-) diff --git a/models/repo_editor.go b/models/repo_editor.go index 720e07162..692fe8c51 100644 --- a/models/repo_editor.go +++ b/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...) diff --git a/models/update.go b/models/update.go index cd22189f6..e6cbba64a 100644 --- a/models/update.go +++ b/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 { diff --git a/routers/private/push_update.go b/routers/private/push_update.go index 3008ef0e7..988d27334 100644 --- a/routers/private/push_update.go +++ b/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) }