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 e8b993e7da.

plugin/notify/email/email.go

@@ -12,11 +12,10 @@ import (
 )
 
 const (
-	NotifyAlways       = "always"        // always send email notification
+	NotifyAlways      = "always" // always send email notification
-	NotifyNever        = "never"         // never send email notifications
+	NotifyNever       = "never"  // never send email notifications
-	NotifyAuthor       = "author"        // only send email notifications to the author
+	NotifyAuthor      = "author" // only send email notifications to the author
-	NotifyAfterFailure = "after_failure" // only send a notification if the previous commit failed
+	NotifyAfterChange = "change" // only if the previous commit has a different status
-	NotifyAfterSuccess = "after_success" // only send a notification if the previous commit succeeded
 
 	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
+	// if the last commit in this branch was a different status, notify
-	case NotifyAfterSuccess:
+	case NotifyAfterChange:
-		if context.Commit.PriorStatus != "Success" {
+		if context.Prior.Status == context.Commit.Status {
-			return nil
-		}
-	// if the last commit in this branch was a failure, notify
-	case NotifyAfterFailure:
-		if context.Commit.PriorStatus != "Failure" {
 			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
+	// if the last commit in this branch was a different status, notify
-	case NotifyAfterSuccess:
+	case NotifyAfterChange:
-		if context.Commit.PriorStatus == "Failure" {
+		if context.Prior.Status == context.Commit.Status {
-			return nil
-		}
-	// if the last commit in this branch was a failure, notify
-	case NotifyAfterFailure:
-		if context.Commit.PriorStatus == "Success" {
 			return nil
 		}
 	// if configured to email the author, replace

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)

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
 `
 

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,

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)
 }

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

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 = &notify.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,
 	})
 }

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"`

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"`
 }