add criteria to task events - fixes #5

README.md

@@ -30,6 +30,10 @@ The request system is driven by a metadata definition which includes system grou
             - `type` (enum, required) - Currently only `CREATE_TASK` is supported
             - `value` (string) - The identifier of the task
 
+        - `criteria` (object) - Defines critera for when the action should be run
+            - `type` - Currently only `FIELD_VALUE` is supported and allows filtering on a field's value
+            - `value` - Matches a field name and value, e.g. foo=bar
+
 
 Architecture
 ------------

app/models/TaskEvent.scala

@@ -7,7 +7,7 @@ package models
 import play.api.libs.json._
 
 // todo: parameterize value
-case class TaskEvent(`type`: TaskEvent.EventType.EventType, value: String, action: TaskEvent.EventAction)
+case class TaskEvent(`type`: TaskEvent.EventType.EventType, value: String, action: TaskEvent.EventAction, criteria: Option[TaskEvent.Criteria])
 
 object TaskEvent {
 
@@ -38,6 +38,23 @@ object TaskEvent {
     implicit val jsonWrites = Json.writes[EventAction]
   }
 
+  object CriteriaType extends Enumeration {
+    type CriteriaType = Value
+
+    val FieldValue = Value("FIELD_VALUE")
+
+    implicit val jsonReads = Reads[CriteriaType] { jsValue =>
+      values.find(_.toString == jsValue.as[String]).fold[JsResult[CriteriaType]](JsError("Could not find that type"))(JsSuccess(_))
+    }
+  }
+
+  case class Criteria(`type`: CriteriaType.CriteriaType, value: String)
+
+  object Criteria {
+    implicit val jsonReads = Json.reads[Criteria]
+    implicit val jsonWrites = Json.writes[Criteria]
+  }
+
   implicit val jsonReads = Json.reads[TaskEvent]
   implicit val jsonWrites = Json.writes[TaskEvent]
 

app/modules/DAOModule.scala

@@ -31,15 +31,19 @@ class DAOModule extends Module {
 }
 
 trait DAO {
+  type NumTotalTasks = Long
+  type NumCompletedTasks = Long
+  type NumComments = Long
+
   def createRequest(name: String, creatorEmail: String): Future[Request]
-  def allRequests(): Future[Seq[(Request, Long, Long)]]
-  def requestsForUser(email: String): Future[Seq[(Request, Long, Long)]]
+  def allRequests(): Future[Seq[(Request, NumTotalTasks, NumCompletedTasks)]]
+  def requestsForUser(email: String): Future[Seq[(Request, NumTotalTasks, NumCompletedTasks)]]
   def request(requestSlug: String): Future[Request]
   def updateRequest(requestSlug: String, state: State.State): Future[Request]
   def createTask(requestSlug: String, prototype: Task.Prototype, completableByType: CompletableByType, completableByValue: String, maybeCompletedBy: Option[String] = None, maybeData: Option[JsObject] = None, state: State = State.InProgress): Future[Task]
   def updateTask(taskId: Int, state: State, maybeCompletedBy: Option[String], maybeData: Option[JsObject]): Future[Task]
   def taskById(taskId: Int): Future[Task]
-  def requestTasks(requestSlug: String, maybeState: Option[State] = None): Future[Seq[(Task, Long)]]
+  def requestTasks(requestSlug: String, maybeState: Option[State] = None): Future[Seq[(Task, NumComments)]]
   def commentOnTask(taskId: Int, email: String, contents: String): Future[Comment]
   def commentsOnTask(taskId: Int): Future[Seq[Comment]]
 }
@@ -79,7 +83,7 @@ class DAOWithCtx @Inject()(database: DatabaseWithCtx)(implicit ec: ExecutionCont
     }
   }
 
-  override def allRequests(): Future[Seq[(Request, Long, Long)]] = {
+  override def allRequests(): Future[Seq[(Request, NumTotalTasks, NumCompletedTasks)]] = {
     run {
       quote {
         for {
@@ -106,7 +110,7 @@ class DAOWithCtx @Inject()(database: DatabaseWithCtx)(implicit ec: ExecutionCont
     updateFuture.flatMap(_ => request(requestSlug))
   }
 
-  override def requestsForUser(email: String): Future[Seq[(Request, Long, Long)]] = {
+  override def requestsForUser(email: String): Future[Seq[(Request, NumTotalTasks, NumCompletedTasks)]] = {
     run {
       quote {
         for {
@@ -187,7 +191,7 @@ class DAOWithCtx @Inject()(database: DatabaseWithCtx)(implicit ec: ExecutionCont
     }
   }
 
-  override def requestTasks(requestSlug: String, maybeState: Option[State] = None): Future[Seq[(Task, Long)]] = {
+  override def requestTasks(requestSlug: String, maybeState: Option[State] = None): Future[Seq[(Task, NumComments)]] = {
     maybeState.fold {
       run {
         quote {

app/utils/TaskEventHandler.scala

@@ -19,19 +19,40 @@ class TaskEventHandler @Inject()(dao: DAO, metadataService: MetadataService)(imp
       task.prototype.taskEvents.filter { taskEvent =>
         taskEvent.`type` == eventType && taskEvent.value == task.state.toString
       } map { taskEvent =>
-        taskEvent.action.`type` match {
-          case TaskEvent.EventActionType.CreateTask =>
-            taskPrototypesFuture.flatMap { taskPrototypes =>
-              taskPrototypes.get(taskEvent.action.value).fold(Future.failed[Task](new Exception(s"Could not find task named '${taskEvent.action.value}'"))) { taskPrototype =>
-                taskPrototype.completableBy.fold(Future.failed[Task](new Exception("Could not create task because it does not have completable_by info"))) { completableBy =>
-                  completableBy.value.fold(Future.failed[Task](new Exception("Could not create task because it does not have a completable_by value"))) { completableByValue =>
-                    dao.createTask(requestSlug, taskPrototype, completableBy.`type`, completableByValue)
+
+        val skipAction = taskEvent.criteria.exists { criteria =>
+          criteria.`type` match {
+            case TaskEvent.CriteriaType.FieldValue =>
+              criteria.value.split("=") match {
+                case Array(field, value) =>
+                  task.data.fold(true) { data =>
+                    // only string values work
+                    !(data \ field).asOpt[String].contains(value)
+                  }
+                case _ =>
+                  false
+              }
+          }
+        }
+
+        if (skipAction) {
+          Future.successful(Seq.empty[Seq[Task]])
+        }
+        else {
+          taskEvent.action.`type` match {
+            case TaskEvent.EventActionType.CreateTask =>
+              taskPrototypesFuture.flatMap { taskPrototypes =>
+                taskPrototypes.get(taskEvent.action.value).fold(Future.failed[Task](new Exception(s"Could not find task named '${taskEvent.action.value}'"))) { taskPrototype =>
+                  taskPrototype.completableBy.fold(Future.failed[Task](new Exception("Could not create task because it does not have completable_by info"))) { completableBy =>
+                    completableBy.value.fold(Future.failed[Task](new Exception("Could not create task because it does not have a completable_by value"))) { completableByValue =>
+                      dao.createTask(requestSlug, taskPrototype, completableBy.`type`, completableByValue)
+                    }
                   }
                 }
               }
-            }
-          case _ =>
-            Future.failed[Task](new Exception(s"Could not process action type: ${taskEvent.action.`type`}"))
+            case _ =>
+              Future.failed[Task](new Exception(s"Could not process action type: ${taskEvent.action.`type`}"))
+          }
         }
       }
     }

examples/metadata.json

@@ -52,6 +52,18 @@
             "type": "CREATE_TASK",
             "value": "review_request"
           }
+        },
+        {
+          "type": "STATE_CHANGE",
+          "value": "COMPLETED",
+          "criteria": {
+            "type": "FIELD_VALUE",
+            "value": "github_org=Foo"
+          },
+          "action": {
+            "type": "CREATE_TASK",
+            "value": "ip_approval"
+          }
         }
       ]
     },

test/modules/DAOMock.scala

@@ -4,30 +4,122 @@
 
 package modules
 
+import java.time.ZonedDateTime
+import java.util.concurrent.ConcurrentHashMap
+import javax.inject.Singleton
+
 import models.State.State
 import models.Task.CompletableByType.CompletableByType
-import models.{Comment, Request, Task}
+import models.{Comment, Request, State, Task}
 import play.api.Mode
 import play.api.db.evolutions.EvolutionsModule
 import play.api.db.{DBModule, HikariCPModule}
 import play.api.inject.bind
 import play.api.inject.guice.GuiceApplicationBuilder
 import play.api.libs.json.JsObject
 
+import scala.collection.JavaConverters._
 import scala.concurrent.Future
+import scala.concurrent.ExecutionContext.Implicits.global
+import scala.util.Try
 
+@Singleton
 class DAOMock extends DAO {
-  override def createRequest(name: String, creatorEmail: String): Future[Request] = Future.failed(new NotImplementedError())
-  override def allRequests(): Future[Seq[(Request, Long, Long)]] = Future.failed(new NotImplementedError())
-  override def requestsForUser(email: String): Future[Seq[(Request, Long, Long)]] = Future.failed(new NotImplementedError())
-  override def updateTask(taskId: Int, state: State, maybeCompletedBy: Option[String], maybeData: Option[JsObject]): Future[Task] = Future.failed(new NotImplementedError())
-  override def commentOnTask(taskId: Int, email: String, contents: String): Future[Comment] = Future.failed(new NotImplementedError())
-  override def request(requestSlug: String): Future[Request] = Future.failed(new NotImplementedError())
-  override def updateRequest(requestSlug: String, state: State): Future[Request] = Future.failed(new NotImplementedError())
-  override def createTask(requestSlug: String, prototype: Task.Prototype, completableByType: CompletableByType, completableByValue: String, maybeCompletedBy: Option[String], maybeData: Option[JsObject], state: State): Future[Task] = Future.failed(new NotImplementedError())
-  override def taskById(taskId: Int): Future[Task] = Future.failed(new NotImplementedError())
-  override def requestTasks(requestSlug: String, maybeState: Option[State]): Future[Seq[(Task, Long)]] = Future.failed(new NotImplementedError())
-  override def commentsOnTask(taskId: Int): Future[Seq[Comment]] = Future.failed(new NotImplementedError())
+  val requests = ConcurrentHashMap.newKeySet[Request].asScala.toBuffer
+  val tasks = ConcurrentHashMap.newKeySet[Task].asScala.toBuffer
+  val comments = ConcurrentHashMap.newKeySet[Comment].asScala.toBuffer
+
+  override def createRequest(name: String, creatorEmail: String): Future[Request] = {
+    Future.successful {
+      val request = Request(DB.slug(name), name, ZonedDateTime.now(), creatorEmail, State.InProgress, None)
+      requests += request
+      request
+    }
+  }
+
+  override def allRequests(): Future[Seq[(Request, NumTotalTasks, NumCompletedTasks)]] = {
+    Future.sequence {
+      requests.map { request =>
+        requestTasks(request.slug).map { requestTasks =>
+          val numCompletedTasks = requestTasks.map(_._1).count(_.state == State.Completed).toLong
+          (request, requestTasks.size.toLong, numCompletedTasks)
+        }
+      }
+    }
+  }
+
+  override def requestsForUser(email: String): Future[Seq[(Request, NumTotalTasks, NumCompletedTasks)]] = {
+    allRequests().map(_.filter(_._1.creatorEmail == email))
+  }
+
+  override def updateTask(taskId: Int, state: State, maybeCompletedBy: Option[String], maybeData: Option[JsObject]): Future[Task] = {
+    tasks.find(_.id == taskId).fold(Future.failed[Task](new Exception("Task not found"))) { task =>
+      val updatedTask = task.copy(
+        state = state,
+        completedByEmail = maybeCompletedBy,
+        data = maybeData
+      )
+
+      tasks -= task
+      tasks += updatedTask
+
+      Future.successful(updatedTask)
+    }
+  }
+
+  override def commentOnTask(taskId: Int, email: String, contents: String): Future[Comment] = {
+    Future.successful {
+      val id = Try(comments.map(_.id).max).getOrElse(0) + 1
+      val comment = Comment(id, email, ZonedDateTime.now(), contents, taskId)
+      comments += comment
+      comment
+    }
+  }
+
+  override def request(requestSlug: String): Future[Request] = {
+    requests.find(_.slug == requestSlug).fold(Future.failed[Request](new Exception("Request not found")))(Future.successful)
+  }
+
+  override def updateRequest(requestSlug: String, state: State): Future[Request] = {
+    request(requestSlug).map { request =>
+      val updatedRequest = request.copy(state = state)
+      requests -= request
+      requests += request
+      updatedRequest
+    }
+  }
+
+  override def createTask(requestSlug: String, prototype: Task.Prototype, completableByType: CompletableByType, completableByValue: String, maybeCompletedBy: Option[String], maybeData: Option[JsObject], state: State): Future[Task] = {
+    Future.successful {
+      val id = Try(tasks.map(_.id).max).getOrElse(0) + 1
+      val task = Task(id, completableByType, completableByValue, maybeCompletedBy, maybeCompletedBy.map(_ => ZonedDateTime.now()), state, prototype, maybeData, requestSlug)
+      tasks += task
+      task
+    }
+  }
+
+  override def taskById(taskId: Int): Future[Task] = {
+    tasks.find(_.id == taskId).fold(Future.failed[Task](new Exception("Task not found")))(Future.successful)
+  }
+
+  override def requestTasks(requestSlug: String, maybeState: Option[State]): Future[Seq[(Task, NumComments)]] = {
+    val requestTasks = tasks.filter(_.requestSlug == requestSlug)
+
+    val requestTasksWithMaybeState = maybeState.fold(requestTasks) { state =>
+      requestTasks.filter(_.state == state)
+    }
+
+    Future.sequence {
+      requestTasksWithMaybeState.toSeq.map { task =>
+        commentsOnTask(task.id).map { comments =>
+          task -> comments.size.toLong
+        }
+      }
+    }
+  }
+  override def commentsOnTask(taskId: Int): Future[Seq[Comment]] = {
+    Future.successful(comments.filter(_.taskId == taskId))
+  }
 }
 
 object DAOMock {

test/utils/DataFacadeSpec.scala

@@ -32,7 +32,7 @@ class DataFacadeSpec extends PlaySpec with GuiceOneAppPerTest {
 
       implicit val fakeRequest = FakeRequest()
 
-      val event = TaskEvent(TaskEvent.EventType.StateChange, State.InProgress.toString, TaskEvent.EventAction(TaskEvent.EventActionType.CreateTask, "review_request"))
+      val event = TaskEvent(TaskEvent.EventType.StateChange, State.InProgress.toString, TaskEvent.EventAction(TaskEvent.EventActionType.CreateTask, "review_request"), None)
       val request = await(dataFacade.createRequest("foo", "foo@bar.com"))
       val prototype = Task.Prototype("asdf", Task.TaskType.Approval, "asdf", None, None, Seq(event))
       val task = await(dataFacade.createTask(request.slug, prototype, Task.CompletableByType.Email, "foo@foo.com"))

test/utils/TaskEventHandlerSpec.scala

@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) Salesforce.com, inc. 2017
+ */
+
+package utils
+
+import models.{State, Task}
+import modules.DAOMock
+import org.scalatestplus.play.PlaySpec
+import org.scalatestplus.play.guice.GuiceOneAppPerTest
+import play.api.libs.json.Json
+import play.api.test.FakeRequest
+import play.api.test.Helpers._
+
+class TaskEventHandlerSpec extends PlaySpec with GuiceOneAppPerTest {
+
+  def taskEventHandler = app.injector.instanceOf[TaskEventHandler]
+  def dataFacade = app.injector.instanceOf[DataFacade]
+  def metadataService = app.injector.instanceOf[MetadataService]
+  implicit val fakeRequest = FakeRequest()
+
+  implicit override def fakeApplication() = DAOMock.noDatabaseAppBuilder().build()
+
+  "TaskEventHandler" must {
+    "automatically add a new task when the metadata says to do so" in {
+      val taskPrototype = await(metadataService.fetchMetadata).tasks("start")
+      val request = await(dataFacade.createRequest("asdf", "asdf@asdf.com"))
+      val task = await(dataFacade.createTask(request.slug, taskPrototype, Task.CompletableByType.Email, "foo@foo.com", Some("foo@foo.com"), None, State.Completed))
+      val tasks = await(dataFacade.requestTasks("asdf@asdf.com", request.slug)).map(_._1)
+
+      tasks.exists(_.prototype.label == "Review Request") mustBe true
+      tasks.exists(_.prototype.label == "IP Approval") mustBe false
+    }
+    "work with criteria and non-matching data" in {
+      val taskPrototype = await(metadataService.fetchMetadata).tasks("start")
+      val data = Json.obj("github_org" -> "Bar")
+      val request = await(dataFacade.createRequest("asdf", "asdf@asdf.com"))
+      val task = await(dataFacade.createTask(request.slug, taskPrototype, Task.CompletableByType.Email, "foo@foo.com", Some("foo@foo.com"), Some(data), State.Completed))
+      val tasks = await(dataFacade.requestTasks("asdf@asdf.com", request.slug)).map(_._1)
+
+      tasks.exists(_.prototype.label == "Review Request") mustBe true
+      tasks.exists(_.prototype.label == "IP Approval") mustBe false
+    }
+    "work with criteria and matching data" in {
+      val taskPrototype = await(metadataService.fetchMetadata).tasks("start")
+      val data = Json.obj("github_org" -> "Foo")
+      val request = await(dataFacade.createRequest("asdf", "asdf@asdf.com"))
+      val task = await(dataFacade.createTask(request.slug, taskPrototype, Task.CompletableByType.Email, "foo@foo.com", Some("foo@foo.com"), Some(data), State.Completed))
+      val tasks = await(dataFacade.requestTasks("asdf@asdf.com", request.slug)).map(_._1)
+
+      tasks.exists(_.prototype.label == "Review Request") mustBe true
+      tasks.exists(_.prototype.label == "IP Approval") mustBe true
+    }
+  }
+
+}