From b94280c15c914be162ae91e422fda62e7a174e2e Mon Sep 17 00:00:00 2001 From: Michael Nutt Date: Tue, 30 Dec 2014 13:25:14 -0500 Subject: [PATCH] Change from notification `after_success` and `after_failure` to `change` Also removes the extra db field; instead, just send Prior as part of the Request. This reverts commit e8b993e7dacf9c5fd018ed41f24ae9344e587be7. --- plugin/notify/email/email.go | 31 +++------- server/datastore/commit.go | 10 +++ server/datastore/database/commit.go | 35 +++++------ server/datastore/database/commit_test.go | 78 ------------------------ server/datastore/database/database.go | 1 - server/datastore/migrate/migrate.go | 10 --- server/worker/docker/docker.go | 6 ++ shared/model/commit.go | 1 - shared/model/request.go | 1 + 9 files changed, 43 insertions(+), 130 deletions(-) diff --git a/plugin/notify/email/email.go b/plugin/notify/email/email.go index 6d38ff13..666c6d25 100644 --- a/plugin/notify/email/email.go +++ b/plugin/notify/email/email.go @@ -12,11 +12,10 @@ import ( ) const ( - NotifyAlways = "always" // always send email notification - NotifyNever = "never" // never send email notifications - NotifyAuthor = "author" // only send email notifications to the author - NotifyAfterFailure = "after_failure" // only send a notification if the previous commit failed - NotifyAfterSuccess = "after_success" // only send a notification if the previous commit succeeded + NotifyAlways = "always" // always send email notification + NotifyNever = "never" // never send email notifications + NotifyAuthor = "author" // only send email notifications to the author + NotifyAfterChange = "change" // only if the previous commit has a different status NotifyTrue = "true" // alias for NotifyTrue NotifyFalse = "false" // alias for NotifyFalse @@ -72,14 +71,9 @@ func (e *Email) sendFailure(context *model.Request) error { switch e.Failure { case NotifyFalse, NotifyNever, NotifyOff: return nil - // if the last commit in this branch was a success, notify - case NotifyAfterSuccess: - if context.Commit.PriorStatus != "Success" { - return nil - } - // if the last commit in this branch was a failure, notify - case NotifyAfterFailure: - if context.Commit.PriorStatus != "Failure" { + // if the last commit in this branch was a different status, notify + case NotifyAfterChange: + if context.Prior.Status == context.Commit.Status { return nil } // if configured to email the author, replace @@ -115,14 +109,9 @@ func (e *Email) sendSuccess(context *model.Request) error { switch e.Success { case NotifyFalse, NotifyNever, NotifyOff: return nil - // if the last commit in this branch was a success, notify - case NotifyAfterSuccess: - if context.Commit.PriorStatus == "Failure" { - return nil - } - // if the last commit in this branch was a failure, notify - case NotifyAfterFailure: - if context.Commit.PriorStatus == "Success" { + // if the last commit in this branch was a different status, notify + case NotifyAfterChange: + if context.Prior.Status == context.Commit.Status { return nil } // if configured to email the author, replace diff --git a/server/datastore/commit.go b/server/datastore/commit.go index 9f94d955..802125ef 100644 --- a/server/datastore/commit.go +++ b/server/datastore/commit.go @@ -31,6 +31,10 @@ type Commitstore interface { // from the datastore accessible to the specified user. GetCommitListActivity(user *model.User) ([]*model.CommitRepo, error) + // GetCommitPrior retrieves the latest commit + // from the datastore for the specified repository and branch. + GetCommitPrior(commit *model.Commit) (*model.Commit, error) + // PostCommit saves a commit in the datastore. PostCommit(commit *model.Commit) error @@ -82,6 +86,12 @@ func GetCommitListActivity(c context.Context, user *model.User) ([]*model.Commit return FromContext(c).GetCommitListActivity(user) } +// GetCommitPrior retrieves the latest commit +// from the datastore for the specified repository and branch. +func GetCommitPrior(c context.Context, commit *model.Commit) (*model.Commit, error) { + return FromContext(c).GetCommitPrior(commit) +} + // PostCommit saves a commit in the datastore. func PostCommit(c context.Context, commit *model.Commit) error { return FromContext(c).PostCommit(commit) diff --git a/server/datastore/database/commit.go b/server/datastore/database/commit.go index 72f2ecc5..fdb6369a 100644 --- a/server/datastore/database/commit.go +++ b/server/datastore/database/commit.go @@ -40,15 +40,6 @@ func (db *Commitstore) GetCommitLast(repo *model.Repo, branch string) (*model.Co return commit, err } -// GetCommitPrior retrieves the latest commit -// from the datastore for the specified repository -// and branch. -func (db *Commitstore) GetCommitPrior(oldCommit *model.Commit) (*model.Commit, error) { - var commit = new(model.Commit) - var err = meddler.QueryRow(db, commit, rebind(commitPriorQuery), oldCommit.RepoID, oldCommit.Branch, oldCommit.Created) - return commit, err -} - // GetCommitList retrieves a list of latest commits // from the datastore for the specified repository. func (db *Commitstore) GetCommitList(repo *model.Repo) ([]*model.Commit, error) { @@ -73,24 +64,30 @@ func (db *Commitstore) GetCommitListActivity(user *model.User) ([]*model.CommitR return commits, err } +// GetCommitPrior retrieves the latest commit +// from the datastore for the specified repository and branch. +func (db *Commitstore) GetCommitPrior(oldCommit *model.Commit) (*model.Commit, error) { + var commit = new(model.Commit) + var err = meddler.QueryRow(db, commit, rebind(commitPriorQuery), oldCommit.RepoID, oldCommit.Branch, oldCommit.ID) + return commit, err +} + // PostCommit saves a commit in the datastore. func (db *Commitstore) PostCommit(commit *model.Commit) error { if commit.Created == 0 { commit.Created = time.Now().UTC().Unix() } commit.Updated = time.Now().UTC().Unix() - - priorCommit, err := db.GetCommitPrior(commit) - if err == nil { - commit.PriorStatus = priorCommit.Status - } - return meddler.Save(db, commitTable, commit) } // PutCommit saves a commit in the datastore. func (db *Commitstore) PutCommit(commit *model.Commit) error { - return db.PostCommit(commit) + if commit.Created == 0 { + commit.Created = time.Now().UTC().Unix() + } + commit.Updated = time.Now().UTC().Unix() + return meddler.Save(db, commitTable, commit) } // DelCommit removes the commit from the datastore. @@ -185,11 +182,11 @@ LIMIT 1 const commitPriorQuery = ` SELECT * FROM commits -WHERE repo_id = ? +WHERE repo_id = ? AND commit_branch = ? - AND commit_created < ? + AND commit_id < ? AND commit_status IN ('Success', 'Failure') -ORDER BY commit_created DESC +ORDER BY commit_id DESC LIMIT 1 ` diff --git a/server/datastore/database/commit_test.go b/server/datastore/database/commit_test.go index 974d4013..d629ee25 100644 --- a/server/datastore/database/commit_test.go +++ b/server/datastore/database/commit_test.go @@ -48,84 +48,6 @@ func TestCommitstore(t *testing.T) { g.Assert(commit.ID != 0).IsTrue() }) - g.It("Should Get a Commit's prior status", func() { - commit := model.Commit{ - RepoID: 1, - Branch: "foo", - Status: "Failure", - Created: 1419892236, - Sha: "85f8c029b902ed9400bc600bac301a0aadb144ac", - } - err := cs.PostCommit(&commit) - g.Assert(err == nil).IsTrue() - g.Assert(commit.ID != 0).IsTrue() - g.Assert(commit.PriorStatus == "").IsTrue() - - commit = model.Commit{ - RepoID: 1, - Branch: "foo", - Status: "Success", - Created: 1419893583, - Sha: "25f8c029b902ed9400bc600bac301a0aadb144ac", - } - err = cs.PostCommit(&commit) - g.Assert(err == nil).IsTrue() - g.Assert(commit.ID != 0).IsTrue() - g.Assert(commit.PriorStatus == "Failure").IsTrue() - }) - - g.It("Should not find prior status from commits on other branches", func() { - commit := model.Commit{ - RepoID: 1, - Branch: "foo", - Status: "Failure", - Created: 1419892236, - Sha: "85f8c029b902ed9400bc600bac301a0aadb144ac", - } - err := cs.PostCommit(&commit) - g.Assert(err == nil).IsTrue() - g.Assert(commit.ID != 0).IsTrue() - g.Assert(commit.PriorStatus == "").IsTrue() - - commit = model.Commit{ - RepoID: 1, - Branch: "bar", - Status: "Success", - Created: 1419893583, - Sha: "25f8c029b902ed9400bc600bac301a0aadb144ac", - } - err = cs.PostCommit(&commit) - g.Assert(err == nil).IsTrue() - g.Assert(commit.ID != 0).IsTrue() - g.Assert(commit.PriorStatus == "").IsTrue() - }) - - g.It("Should not find prior status from commits that didn't succeed or fail", func() { - commit := model.Commit{ - RepoID: 1, - Branch: "foo", - Status: "Pending", - Created: 1419892236, - Sha: "85f8c029b902ed9400bc600bac301a0aadb144ac", - } - err := cs.PostCommit(&commit) - g.Assert(err == nil).IsTrue() - g.Assert(commit.ID != 0).IsTrue() - g.Assert(commit.PriorStatus == "").IsTrue() - - commit = model.Commit{ - RepoID: 1, - Branch: "bar", - Status: "Success", - Created: 1419893583, - Sha: "25f8c029b902ed9400bc600bac301a0aadb144ac", - } - err = cs.PostCommit(&commit) - g.Assert(err == nil).IsTrue() - g.Assert(commit.ID != 0).IsTrue() - g.Assert(commit.PriorStatus == "").IsTrue() - }) - g.It("Should Get a Commit by ID", func() { commit := model.Commit{ RepoID: 1, diff --git a/server/datastore/database/database.go b/server/datastore/database/database.go index aecb32e9..6ca59d66 100644 --- a/server/datastore/database/database.go +++ b/server/datastore/database/database.go @@ -38,7 +38,6 @@ func Connect(driver, datasource string) (*sql.DB, error) { var migrations = []migration.Migrator{ migrate.Setup, migrate.Migrate_20142110, - migrate.Migrate_20143012, } return migration.Open(driver, datasource, migrations) } diff --git a/server/datastore/migrate/migrate.go b/server/datastore/migrate/migrate.go index 5985cba1..42a3c57c 100644 --- a/server/datastore/migrate/migrate.go +++ b/server/datastore/migrate/migrate.go @@ -39,12 +39,6 @@ func Migrate_20142110(tx migration.LimitedTx) error { return nil } -// Migrate_20143012 adds the prior commit's status to the current commit -func Migrate_20143012(tx migration.LimitedTx) error { - _, err := tx.Exec(transform(commitPriorStatusColumn)) - return err -} - var userTable = ` CREATE TABLE IF NOT EXISTS users ( user_id INTEGER PRIMARY KEY AUTOINCREMENT @@ -116,10 +110,6 @@ var repoTokenUpdate = ` UPDATE repos SET repo_token = ''; ` -var commitPriorStatusColumn = ` -ALTER TABLE commits ADD COLUMN commit_prior_status VARCHAR(255); -` - var commitTable = ` CREATE TABLE IF NOT EXISTS commits ( commit_id INTEGER PRIMARY KEY AUTOINCREMENT diff --git a/server/worker/docker/docker.go b/server/worker/docker/docker.go index bff55e2a..8bbd622f 100644 --- a/server/worker/docker/docker.go +++ b/server/worker/docker/docker.go @@ -120,6 +120,8 @@ func (d *Docker) Do(c context.Context, r *worker.Work) { Depth: git.GitDepth(script.Git), } + priorCommit, _ := datastore.GetCommitPrior(c, r.Commit) + // send all "started" notifications if script.Notifications == nil { script.Notifications = ¬ify.Notification{} @@ -129,6 +131,7 @@ func (d *Docker) Do(c context.Context, r *worker.Work) { Repo: r.Repo, Commit: r.Commit, Host: r.Host, + Prior: priorCommit, }) // create an instance of the Docker builder @@ -168,11 +171,14 @@ func (d *Docker) Do(c context.Context, r *worker.Work) { // notify all listeners that the build is finished commitc.Publish(r) + priorCommit, _ = datastore.GetCommitPrior(c, r.Commit) + // send all "finished" notifications script.Notifications.Send(&model.Request{ User: r.User, Repo: r.Repo, Commit: r.Commit, Host: r.Host, + Prior: priorCommit, }) } diff --git a/shared/model/commit.go b/shared/model/commit.go index 0bc3aa57..da8c0c65 100644 --- a/shared/model/commit.go +++ b/shared/model/commit.go @@ -8,7 +8,6 @@ type Commit struct { ID int64 `meddler:"commit_id,pk" json:"id"` RepoID int64 `meddler:"repo_id" json:"-"` Status string `meddler:"commit_status" json:"status"` - PriorStatus string `meddler:"commit_prior_status" json:"prior_status"` Started int64 `meddler:"commit_started" json:"started_at"` Finished int64 `meddler:"commit_finished" json:"finished_at"` Duration int64 `meddler:"commit_duration" json:"duration"` diff --git a/shared/model/request.go b/shared/model/request.go index 0bf75251..594cbeb2 100644 --- a/shared/model/request.go +++ b/shared/model/request.go @@ -5,4 +5,5 @@ type Request struct { User *User `json:"-"` Repo *Repo `json:"repo"` Commit *Commit `json:"commit"` + Prior *Commit `json:"prior_commit"` }