[GAWB-2056] The status of the last run is showing green when there are errors in them

core/src/main/resources/swagger/rawls.yaml

@@ -5051,11 +5051,11 @@ definitions:
       lastSuccessDate:
         type: string
         format: date-time
-        description: The date of the last successful workflow
+        description: The date of the last successful submission
       lastFailureDate:
         type: string
         format: date-time
-        description: The date of the last failed workflow
+        description: The date of the last failed submission
       runningSubmissionsCount:
         type: integer
         description: Count of all the running submissions

core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/slick/DriverComponent.scala

@@ -4,14 +4,13 @@ import java.nio.ByteOrder
 import java.sql.Timestamp
 import java.util.UUID
 
-import akka.util.{ByteString, ByteStringBuilder}
+import akka.util.ByteString
+import org.apache.commons.codec.binary.Base64
 import org.broadinstitute.dsde.rawls.model._
 import org.broadinstitute.dsde.rawls.{RawlsException, RawlsExceptionWithErrorReport}
-import org.joda.time.DateTime
 import slick.driver.JdbcDriver
 import slick.jdbc.{GetResult, PositionedParameters, SQLActionBuilder, SetParameter}
 import spray.http.StatusCodes
-import org.apache.commons.codec.binary.Base64
 
 import scala.concurrent.ExecutionContext
 

core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/slick/WorkspaceComponent.scala

@@ -3,13 +3,16 @@ package org.broadinstitute.dsde.rawls.dataaccess.slick
 import java.sql.Timestamp
 import java.util.{Date, UUID}
 
+import cats.instances.int._
+import cats.instances.option._
+import cats.{Monoid, MonoidK}
 import org.broadinstitute.dsde.rawls.RawlsException
+import org.broadinstitute.dsde.rawls.dataaccess.SlickWorkspaceContext
+import org.broadinstitute.dsde.rawls.model.Attributable.AttributeMap
 import org.broadinstitute.dsde.rawls.model.WorkspaceAccessLevels.WorkspaceAccessLevel
 import org.broadinstitute.dsde.rawls.model._
+import org.broadinstitute.dsde.rawls.util.CollectionUtils
 import org.joda.time.DateTime
-import org.broadinstitute.dsde.rawls.dataaccess.SlickWorkspaceContext
-import org.broadinstitute.dsde.rawls.model.Attributable.AttributeMap
-
 /**
  * Created by dvoet on 2/4/16.
  */
@@ -499,36 +502,70 @@ trait WorkspaceComponent {
     }
 
     /**
-     * gets the submission stats (last workflow failed date, last workflow success date, running submission count)
+     * gets the submission stats (last submission failed date, last submission success date, running submission count)
      * for each workspace
      *
      * @param workspaceIds the workspace ids to query for
      * @return WorkspaceSubmissionStats keyed by workspace id
      */
     def listSubmissionSummaryStats(workspaceIds: Seq[UUID]): ReadAction[Map[UUID, WorkspaceSubmissionStats]] = {
-      // workflow date query: select workspaceId, workflow.status, max(workflow.statusLastChangedDate) ... group by workspaceId, workflow.status
-      val workflowDatesQuery = for {
-        submissions <- submissionQuery if submissions.workspaceId.inSetBind(workspaceIds)
-        workflows <- workflowQuery if submissions.id === workflows.submissionId
-      } yield (submissions.workspaceId, workflows.status, workflows.statusLastChangedDate)
+      // submission date query:
+      //
+      // select workspaceId, case when subFailed > 0 then 'Failed' else 'Succeeded' end as status, max(subEndDate) as subEndDate
+      // from (
+      //   select submission.id, submission.workspaceId, max(case when w.status = 'Failed' then 1 else 0 end) as subFailed, max(w.status_last_changed) as subEndDate
+      //   from submission
+      //   join workflow on workflow.submissionId = submission.id
+      //   where submission.workspaceId in (:workspaceIds)
+      //   and status in ('Succeeded', 'Failed')
+      //   group by 1, 2) v
+      // group by 1, 2
+      //
+      // Explanation:
+      // - inner query gets 2 things per (workspace, submission):
+      // -- whether or not the submission contains any failed workflows
+      // -- the most recent workflow status change date
+      // - outer query gets the most recent workflow status change date per (workspace, submissionStatus), where submissionStatus is:
+      // -- Failed if the submission contains failed workflows
+      // -- Succeeded if the submission does not contain failed workflows
+
+      val innerSubmissionDateQuery = (
+        for {
+          submissions <- submissionQuery if submissions.workspaceId.inSetBind(workspaceIds)
+          workflows <- workflowQuery if submissions.id === workflows.submissionId
+        } yield (submissions.id, submissions.workspaceId, workflows.status, workflows.statusLastChangedDate)
+      ).filter { case (_, _, status, _) =>
+        status === WorkflowStatuses.Failed.toString || status === WorkflowStatuses.Succeeded.toString
+      }.map { case (submissionId, workspaceId, status, statusLastChangedDate) =>
+        (submissionId, workspaceId, Case If (status === WorkflowStatuses.Failed.toString) Then 1 Else 0, statusLastChangedDate)
+      }.groupBy { case (submissionId, workspaceId, _, _) =>
+        (submissionId, workspaceId)
+      }.map { case ((submissionId, workspaceId), recs) =>
+        (submissionId, workspaceId, recs.map(_._3).sum, recs.map(_._4).max)
+      }
 
-      val workflowDatesGroupedQuery = workflowDatesQuery.groupBy { case (wsId, status, _) => (wsId, status) }.
-        map { case ((wsId, wfStatus), records) => (wsId, wfStatus, records.map { case (_, _, lastChanged) => lastChanged }.max) }
+      val outerSubmissionDateQuery = innerSubmissionDateQuery.map { case (_, workspaceId, numFailures, maxDate) =>
+        (workspaceId, Case If (numFailures > 0) Then WorkflowStatuses.Failed.toString Else WorkflowStatuses.Succeeded.toString, maxDate)
+      }.groupBy { case (workspaceId, status, _) =>
+        (workspaceId, status)
+      }.map { case ((workspaceId, status), recs) =>
+        (workspaceId, status, recs.map(_._3).max)
+      }
 
       // running submission query: select workspaceId, count(1) ... where submissions.status === Submitted group by workspaceId
       val runningSubmissionsQuery = (for {
         submissions <- submissionQuery if submissions.workspaceId.inSetBind(workspaceIds) && submissions.status.inSetBind(SubmissionStatuses.activeStatuses.map(_.toString))
       } yield submissions).groupBy(_.workspaceId).map { case (wfId, submissions) => (wfId, submissions.length)}
 
       for {
-        workflowDates <- workflowDatesGroupedQuery.result
+        submissionDates <- outerSubmissionDateQuery.result
         runningSubmissions <- runningSubmissionsQuery.result
       } yield {
-        val workflowDatesByWorkspaceByStatus: Map[UUID, Map[String, Option[Timestamp]]] = groupByWorkspaceIdThenStatus(workflowDates)
+        val submissionDatesByWorkspaceByStatus: Map[UUID, Map[String, Option[Timestamp]]] = groupByWorkspaceIdThenStatus(submissionDates)
         val runningSubmissionCountByWorkspace: Map[UUID, Int] = groupByWorkspaceId(runningSubmissions)
 
         workspaceIds.map { wsId =>
-          val (lastFailedDate, lastSuccessDate) = workflowDatesByWorkspaceByStatus.get(wsId) match {
+          val (lastFailedDate, lastSuccessDate) = submissionDatesByWorkspaceByStatus.get(wsId) match {
             case None => (None, None)
             case Some(datesByStatus) =>
               (datesByStatus.getOrElse(WorkflowStatuses.Failed.toString, None), datesByStatus.getOrElse(WorkflowStatuses.Succeeded.toString, None))
@@ -733,11 +770,26 @@ trait WorkspaceComponent {
   }
 
   private def groupByWorkspaceId(runningSubmissions: Seq[(UUID, Int)]): Map[UUID, Int] = {
-    runningSubmissions.groupBy{ case (wsId, count) => wsId }.mapValues { case Seq((_, count)) => count }
+    CollectionUtils.groupPairs(runningSubmissions)
   }
 
   private def groupByWorkspaceIdThenStatus(workflowDates: Seq[(UUID, String, Option[Timestamp])]): Map[UUID, Map[String, Option[Timestamp]]] = {
-    workflowDates.groupBy { case (wsId, _, _) => wsId }.mapValues(_.groupBy { case (_, status, _) => status }.mapValues { case Seq((_, _, timestamp)) => timestamp })
+    // The function groupTriples, called below, transforms a Seq((T1, T2, T3)) to a Map(T1 -> Map(T2 -> T3)).
+    // It does this by calling foldMap, which in turn requires a monoid for T3. In our case, T3 is an Option[Timestamp],
+    // so we need to provide an implicit monoid for Option[Timestamp].
+    //
+    // There isn't really a sane monoid implementation for Timestamp (what would you do, add them?). Thankfully
+    // it turns out that the UUID/String pairs in workflowDates are always unique, so it doesn't matter what the
+    // monoid does because it'll never be used to combine two Option[Timestamp]s. It just needs to be provided in
+    // order to make the compiler happy.
+    //
+    // To do this, we use the universal monoid for Option, MonoidK[Option]. Note that the inner Option takes no type
+    // parameter: MonoidK doesn't care about the type inside Option, it just calls orElse on the Option for its "combine"
+    // operator. Finally, the call to algebra[Timestamp] turns a MonoidK[Option] into a Monoid[Option[Timestamp]] by
+    // leaving the monoid implementation alone (so it still calls orElse) and poking the Timestamp type into the Option.
+
+    implicit val optionTimestampMonoid: Monoid[Option[Timestamp]] = MonoidK[Option].algebra[Timestamp]
+    CollectionUtils.groupTriples(workflowDates)
   }
 }
 

core/src/main/scala/org/broadinstitute/dsde/rawls/util/CollectionUtils.scala

@@ -1,5 +1,10 @@
 package org.broadinstitute.dsde.rawls.util
 
+import cats.Monoid
+import cats.instances.list._
+import cats.instances.map._
+import cats.syntax.foldable._
+
 object CollectionUtils {
 
   //A saner group by than Scala's.
@@ -10,4 +15,20 @@ object CollectionUtils {
   def groupByTuplesFlatten[A, B]( tupleSeq: Seq[(A, Seq[B])] ): Map[A, Seq[B]] = {
     tupleSeq groupBy { case (a,b) => a } map { case (k, v) => k -> v.flatMap(_._2) }
   }
+
+  /**
+    * Converts a `Seq[(A, B)]` into a `Map[A, B]`, combining the values with a `Monoid[B]` in case of key conflicts.
+    *
+    * For example:
+    * {{{
+    * scala> groupPairs(Seq(("a", 1), ("b", 2), ("a", 3)))
+    * res0: Map[String,Int] = Map(b -> 2, a -> 4)
+    * }}}
+    * */
+  def groupPairs[A, B: Monoid](pairs: Seq[(A, B)]): Map[A, B] =
+    pairs.toList.foldMap { case (a, b) => Map(a -> b) }
+
+  // Same as above but with triples
+  def groupTriples[A, B, C: Monoid](trips: Seq[(A, B, C)]): Map[A, Map[B, C]] =
+    trips.toList.foldMap { case (a, b, c) => Map(a -> Map(b -> c)) }
 }

core/src/test/scala/org/broadinstitute/dsde/rawls/dataaccess/slick/DriverComponentSpec.scala

@@ -69,7 +69,7 @@ class DriverComponentSpec extends TestDriverComponentWithFlatSpecAndMatchers wit
     val queryRecords = runAndWait(query.as[WorkflowRecord])
 
     // first check that we're not just comparing empty seqs
-    assertResult(24) { queryRecords.length }
+    assertResult(26) { queryRecords.length }
 
     assertResult(queryRecords) {
       runAndWait(concatSqlActions(select, where1, reduceSqlActionsWithDelim(statuses), where2).as[WorkflowRecord])

core/src/test/scala/org/broadinstitute/dsde/rawls/dataaccess/slick/TestDriverComponent.scala

@@ -210,6 +210,7 @@ trait TestDriverComponent extends DriverComponent with DataAccess {
     val wsName7 = WorkspaceName("myNamespace", "myWorkspacewithRealmsMethodConfigsAbortedSubmission")
     val wsName8 = WorkspaceName("myNamespace", "myWorkspacewithRealmsMethodConfigsAbortedSuccessfulSubmission")
     val wsName9 = WorkspaceName("myNamespace", "myWorkspaceToTestGrantPermissions")
+    val wsInterleaved = WorkspaceName("myNamespace", "myWorkspaceToTestInterleavedSubmissions")
     val workspaceToTestGrantId = UUID.randomUUID()
 
     val nestedProjectGroup = makeRawlsGroup("nested project group", Set(userOwner))
@@ -297,6 +298,9 @@ trait TestDriverComponent extends DriverComponent with DataAccess {
     // Workspace with realms, with aborted and successful submissions
     val (workspaceTerminatedSubmissions, workspaceTerminatedSubmissionsGroups) = makeWorkspace(billingProject, wsName8.name, Option(realm), UUID.randomUUID().toString, "aBucket", currentTime(), currentTime(), "testUser", wsAttrs, false)
 
+    // Workspace with a successful submission that had another submission run and fail while it was running
+    val (workspaceInterleavedSubmissions, workspaceInterleavedSubmissionsGroups) = makeWorkspace(billingProject, wsInterleaved.name, Option(realm), UUID.randomUUID().toString, "aBucket", currentTime(), currentTime(), "testUser", wsAttrs, false)
+
     // Standard workspace to test grant permissions
     val (workspaceToTestGrant, workspaceToTestGrantGroups) = makeWorkspaceToTestGrant(billingProject, wsName9.name, None, workspaceToTestGrantId.toString, "aBucket", currentTime(), currentTime(), "testUser", wsAttrs, false)
 
@@ -481,6 +485,16 @@ trait TestDriverComponent extends DriverComponent with DataAccess {
         Workflow(Option("workflowSubmitted2"), WorkflowStatuses.Submitted, testDate, sample6.toReference, inputResolutions)
       ), SubmissionStatuses.Submitted, false)
 
+    //two submissions interleaved in time
+    val t1 = new DateTime(2017, 1, 1, 5, 10)
+    val t2 = new DateTime(2017, 1, 1, 5, 15)
+    val t3 = new DateTime(2017, 1, 1, 5, 20)
+    val t4 = new DateTime(2017, 1, 1, 5, 30)
+    val outerSubmission = Submission(UUID.randomUUID().toString(), t1, userOwner, methodConfig.namespace, methodConfig.name, indiv1.toReference,
+      Seq(Workflow(Option("workflowSuccessful1"), WorkflowStatuses.Succeeded, t4, sample1.toReference, inputResolutions)), SubmissionStatuses.Done, false)
+    val innerSubmission = Submission(UUID.randomUUID().toString(), t2, userOwner, methodConfig.namespace, methodConfig.name, indiv1.toReference,
+      Seq(Workflow(Option("workflowFailed1"), WorkflowStatuses.Failed, t3, sample1.toReference, inputResolutions)), SubmissionStatuses.Done, false)
+
     def createWorkspaceGoogleGroups(gcsDAO: GoogleServicesDAO): Unit = {
       val groups = billingProject.groups.values ++
         testProject1.groups.values ++
@@ -495,6 +509,7 @@ trait TestDriverComponent extends DriverComponent with DataAccess {
         workspaceSubmittedSubmissionGroups ++
         workspaceMixedSubmissionsGroups ++
         workspaceTerminatedSubmissionsGroups ++
+        workspaceInterleavedSubmissionsGroups ++
         controlledWorkspaceGroups ++
         Seq(realm.membersGroup, realm.adminsGroup, realm2.membersGroup, realm2.adminsGroup)
 
@@ -514,6 +529,7 @@ trait TestDriverComponent extends DriverComponent with DataAccess {
       workspaceSubmittedSubmission,
       workspaceMixedSubmissions,
       workspaceTerminatedSubmissions,
+      workspaceInterleavedSubmissions,
       workspaceToTestGrant)
     val saveAllWorkspacesAction = DBIO.sequence(allWorkspaces.map(workspaceQuery.save))
 
@@ -549,6 +565,7 @@ trait TestDriverComponent extends DriverComponent with DataAccess {
         DBIO.sequence(workspaceSubmittedSubmissionGroups.map(rawlsGroupQuery.save).toSeq),
         DBIO.sequence(workspaceMixedSubmissionsGroups.map(rawlsGroupQuery.save).toSeq),
         DBIO.sequence(workspaceTerminatedSubmissionsGroups.map(rawlsGroupQuery.save).toSeq),
+        DBIO.sequence(workspaceInterleavedSubmissionsGroups.map(rawlsGroupQuery.save).toSeq),
         DBIO.sequence(workspaceToTestGrantGroups.map(rawlsGroupQuery.save).toSeq),
         managedGroupQuery.createManagedGroup(realm),
         managedGroupQuery.createManagedGroup(realm2),
@@ -647,6 +664,17 @@ trait TestDriverComponent extends DriverComponent with DataAccess {
             submissionQuery.create(context, submissionMixed),
             updateWorkflowExecutionServiceKey("unittestdefault")
           )
+        }),
+        withWorkspaceContext(workspaceInterleavedSubmissions)({ context =>
+          DBIO.seq(
+            entityQuery.save(context, Seq(aliquot1, aliquot2, sample1, sample2, sample3, sample4, sample5, sample6, sample7, sample8, pair1, pair2, ps1, sset1, sset2, sset3, sset4, sset_empty, indiv1, indiv2)),
+
+            methodConfigurationQuery.create(context, methodConfig),
+
+            submissionQuery.create(context, outerSubmission),
+            submissionQuery.create(context, innerSubmission),
+            updateWorkflowExecutionServiceKey("unittestdefault")
+          )
         })
       )
     }

core/src/test/scala/org/broadinstitute/dsde/rawls/dataaccess/slick/WorkspaceComponentSpec.scala

@@ -114,4 +114,50 @@ class WorkspaceComponentSpec extends TestDriverComponentWithFlatSpecAndMatchers
       runAndWait(workspaceQuery.delete(workspace.toWorkspaceName))
     }
   }
+
+  it should "list submission summary stats" in withDefaultTestDatabase {
+    implicit def toWorkspaceId(ws: Workspace): UUID = UUID.fromString(ws.workspaceId)
+
+    // no submissions: 0 successful, 0 failed, 0 running
+    val wsIdNoSubmissions: UUID = testData.workspaceNoSubmissions
+    assertResult(Map(wsIdNoSubmissions -> WorkspaceSubmissionStats(None, None, 0))) {
+      runAndWait(workspaceQuery.listSubmissionSummaryStats(Seq(wsIdNoSubmissions)))
+    }
+
+    // successful submission: 1 successful, 0 failed, 0 running
+    val wsIdSuccessfulSubmission: UUID = testData.workspaceSuccessfulSubmission
+    assertResult(Map(wsIdSuccessfulSubmission -> WorkspaceSubmissionStats(Some(testDate), None, 0))) {
+      runAndWait(workspaceQuery.listSubmissionSummaryStats(Seq(wsIdSuccessfulSubmission)))
+    }
+
+    // failed submission: 0 successful, 1 failed, 0 running
+    val wsIdFailedSubmission: UUID = testData.workspaceFailedSubmission
+    assertResult(Map(wsIdFailedSubmission -> WorkspaceSubmissionStats(None, Some(testDate), 0))) {
+      runAndWait(workspaceQuery.listSubmissionSummaryStats(Seq(wsIdFailedSubmission)))
+    }
+
+    // submitted submissions: 0 successful, 0 failed, 1 running
+    val wsIdSubmittedSubmission: UUID = testData.workspaceSubmittedSubmission
+    assertResult(Map(wsIdSubmittedSubmission -> WorkspaceSubmissionStats(None, None, 1))) {
+      runAndWait(workspaceQuery.listSubmissionSummaryStats(Seq(wsIdSubmittedSubmission)))
+    }
+
+    // mixed submissions: 0 successful, 1 failed, 1 running
+    val wsIdMixedSubmission: UUID = testData.workspaceMixedSubmissions
+    assertResult(Map(wsIdMixedSubmission -> WorkspaceSubmissionStats(None, Some(testDate), 1))) {
+      runAndWait(workspaceQuery.listSubmissionSummaryStats(Seq(wsIdMixedSubmission)))
+    }
+
+    // terminated submissions: 1 successful, 1 failed, 0 running
+    val wsIdTerminatedSubmission: UUID = testData.workspaceTerminatedSubmissions
+    assertResult(Map(wsIdTerminatedSubmission -> WorkspaceSubmissionStats(Some(testDate), Some(testDate), 0))) {
+      runAndWait(workspaceQuery.listSubmissionSummaryStats(Seq(wsIdTerminatedSubmission)))
+    }
+
+    // interleaved submissions: 1 successful, 1 failed, 0 running
+    val wsIdInterleavedSubmissions: UUID = testData.workspaceInterleavedSubmissions
+    assertResult(Map(wsIdInterleavedSubmissions -> WorkspaceSubmissionStats(Option(testData.t4), Option(testData.t3), 0))) {
+      runAndWait(workspaceQuery.listSubmissionSummaryStats(Seq(wsIdInterleavedSubmissions)))
+    }
+  }
 }