Get rid of some more custom functions

phoenix-scala/core/app/core/db/ExceptionWrapper.scala

@@ -11,7 +11,7 @@ object ExceptionWrapper {
     import scala.util.{Failure, Success}
 
     dbio.asTry.dbresult.flatMap {
-      case Success(value) ⇒ DbResultT.good(value)
+      case Success(value) ⇒ value.pure[DbResultT]
       case Failure(e)     ⇒ DbResultT.failure(DatabaseFailure(e.getMessage))
     }
   }

phoenix-scala/core/app/core/db/FoxTableQuery.scala

@@ -86,7 +86,7 @@ abstract class FoxTableQuery[M <: FoxModel[M], T <: FoxTable[M]](construct: Tag
   }
 
   def refresh(model: M)(implicit ec: EC): DBIO[M] =
-    findOneById(model.id).safeGet
+    findOneById(model.id).unsafeGet
 
   type QuerySeq = Query[T, M, Seq]
 

phoenix-scala/core/app/core/db/SearchTerms.scala

@@ -1,5 +1,6 @@
 package core.db
 
+import cats.implicits._
 import core.failures.{Failure, NotFoundFailure400, NotFoundFailure404}
 import core.utils.Strings._
 import slick.jdbc.PostgresProfile.api._
@@ -24,7 +25,7 @@ trait SearchById[M <: FoxModel[M], T <: FoxTable[M]] {
   private def mustFindById(id: M#Id, notFoundFailure: M#Id ⇒ Failure = notFound404K)(implicit ec: EC,
                                                                                      db: DB): DbResultT[M] =
     this.findOneById(id).dbresult.flatMap {
-      case Some(model) ⇒ DbResultT.good(model)
+      case Some(model) ⇒ model.pure[DbResultT]
       case None        ⇒ DbResultT.failure(notFoundFailure(id))
     }
 }
@@ -37,7 +38,7 @@ trait SearchByRefNum[M <: FoxModel[M], T <: FoxTable[M]] extends SearchById[M, T
       implicit ec: EC,
       db: DB): DbResultT[M] =
     findOneByRefNum(refNum).dbresult.flatMap {
-      case Some(model) ⇒ DbResultT.good(model)
+      case Some(model) ⇒ model.pure[DbResultT]
       case None        ⇒ DbResultT.failure(notFoundFailure(refNum))
     }
 }
@@ -49,7 +50,7 @@ trait SearchByCode[M <: FoxModel[M], T <: FoxTable[M]] extends SearchById[M, T]
   def mustFindByCode(code: String, notFoundFailure: String ⇒ Failure = notFound404K)(implicit ec: EC,
                                                                                      db: DB): DbResultT[M] =
     findOneByCode(code).dbresult.flatMap {
-      case Some(model) ⇒ DbResultT.good(model)
+      case Some(model) ⇒ model.pure[DbResultT]
       case None        ⇒ DbResultT.failure(notFoundFailure(code))
     }
 }
@@ -62,7 +63,7 @@ trait SearchByIdAndName[M <: FoxModel[M], T <: FoxTable[M]] extends SearchById[M
       implicit ec: EC,
       db: DB): DbResultT[M] =
     findOneByIdAndName(id, name).dbresult.flatMap {
-      case Some(model) ⇒ DbResultT.good(model)
+      case Some(model) ⇒ model.pure[DbResultT]
       case None        ⇒ DbResultT.failure(notFoundFailure(name))
     }
 }

phoenix-scala/core/app/core/db/Star.scala

@@ -17,17 +17,17 @@ object * extends LazyLogging {
   def <~[A](v: SqlAction[A, NoStream, Effect.All])(implicit ec: EC): DbResultT[A] =
     <~(v.map(Either.right))
 
-  def <~[A](v: DBIO[A])(implicit ec: EC): DbResultT[A] =
-    DbResultT.fromF(v)
+  def <~[A](fa: DBIO[A])(implicit ec: EC): DbResultT[A] =
+    DbResultT.fromF(fa)
 
-  def <~[A](v: Either[Failures, A])(implicit ec: EC): DbResultT[A] =
-    DbResultT.fromEither(v)
+  def <~[A](fa: Either[Failures, A])(implicit ec: EC): DbResultT[A] =
+    DbResultT.fromEither(fa)
 
-  def <~[A](v: Future[Either[Failures, A]])(implicit M1: Monad[DBIO], M2: Monad[Future]): DbResultT[A] =
-    DbResultT.fromResult(Result.fromFEither(v))
+  def <~[A](gfa: Future[Either[Failures, A]])(implicit M1: Monad[DBIO], M2: Monad[Future]): DbResultT[A] =
+    DbResultT.fromResult(Result.fromFEither(gfa))
 
-  def <~[A](v: Future[A])(implicit ec: EC): DbResultT[A] =
-    <~(v.map(Either.right(_)).recover {
+  def <~[A](fa: Future[A])(implicit ec: EC): DbResultT[A] =
+    <~(fa.map(Either.right).recover {
       case ex ⇒
         logger.error("A Future failed during conversion to DbResultT.", ex)
         Either.left(GeneralFailure(ex.getMessage).single)
@@ -36,22 +36,25 @@ object * extends LazyLogging {
   def <~[A](fa: Result[A])(implicit ec: EC): DbResultT[A] =
     DbResultT.fromResult(fa)
 
-  def <~[A](v: A)(implicit ec: EC): DbResultT[A] =
-    v.pure[DbResultT]
+  // TODO: Is this more readable than inlining? @michalrus
+  def <~[A](a: A)(implicit ec: EC): DbResultT[A] =
+    a.pure[DbResultT]
 
   def <~[A](v: Validated[Failures, A])(implicit ec: EC): DbResultT[A] =
     DbResultT.fromEither(v.toEither)
 
-  def <~[M[_]: TraverseFilter, A](v: M[DbResultT[A]])(implicit ec: EC): DbResultT[M[A]] =
-    DbResultT.seqCollectFailures(v)
+  def <~[M[_]: TraverseFilter, A](fas: M[DbResultT[A]])(implicit ec: EC): DbResultT[M[A]] =
+    DbResultT.seqCollectFailures(fas)
 
   // FIXME: Remove this function after switching all Seqs to List/Vector. Cats don’t have instances for Seq and Seq is unsound. PM me or @kjanosz for details. @michalrus
-  def <~[A](v: Seq[DbResultT[A]])(implicit ec: EC): DbResultT[List[A]] =
-    DbResultT.seqCollectFailures(v.toList)
+  def <~[A](fas: Seq[DbResultT[A]])(implicit ec: EC): DbResultT[List[A]] =
+    DbResultT.seqCollectFailures(fas.toList)
 
-  def <~[A](v: DbResultT[A]): DbResultT[A] =
-    v
+  // TODO: Is this more readable than inlining? @michalrus
+  def <~[A](fa: DbResultT[A]): DbResultT[A] =
+    fa
 
-  def <~[A](v: Option[DbResultT[A]])(implicit ec: EC): DbResultT[Option[A]] = // TODO: sequence? @michalrus - yes, please! @aafa
-    v.fold(DbResultT.none[A])(_.map(Some(_)))
+  // TODO: Is this more readable than inlining? @michalrus
+  def <~[A](ofa: Option[DbResultT[A]])(implicit ec: EC): DbResultT[Option[A]] =
+    ofa.sequence
 }

phoenix-scala/core/app/core/db/package.scala

@@ -86,9 +86,6 @@ package object db {
     def fold[B](ra: Failures ⇒ B, rb: A ⇒ B)(implicit F: Monad[F]): FoxyT[F, B] = // TODO: this is not fold… Find a better name or remove it? @michalrus
       fa.map(rb).handleError(ra)
 
-    def meh(implicit M: Monad[F]): FoxyT[F, Unit] =
-      fa.void // TODO: remove me? But it’s cute… @michalrus
-
     def failuresToWarnings(valueIfWasFailed: A)(pf: PartialFunction[Failure, Boolean])(
         implicit F: Monad[F]): FoxyT[F, A] =
       fa.handleErrorWith { fs ⇒
@@ -105,15 +102,6 @@ package object db {
   }
 
   trait FoxyTFunctions[F[_]] {
-    def good[A](a: A)(implicit F: Monad[F]): FoxyT[F, A] = // TODO: remove me @michalrus
-      a.pure[FoxyT[F, ?]]
-
-    def unit(implicit F: Monad[F]): FoxyT[F, Unit] =
-      ().pure[FoxyT[F, ?]] // TODO: remove me? @michalrus
-
-    def none[A](implicit F: Monad[F]): FoxyT[F, Option[A]] =
-      (None: Option[A]).pure[FoxyT[F, ?]] // TODO: remove me? @michalrus
-
     def uiWarning(f: Failure)(implicit F: Monad[F]): FoxyT[F, Unit] =
       StateT.modify(MetaResponse.Warning(f) :: _)
 
@@ -149,6 +137,8 @@ package object db {
             case _       ⇒ EitherT.right(F.pure((s, ())))
         })
 
+    // TODO: maybe move the quasi-`sequence`-s from Functions to Ops? @michalrus
+
     /** Just like ``sequence`` but—in case of a failure—unlawful, as it will join failures from all Foxies. */
     def seqCollectFailures[L[_], A](lfa: L[FoxyT[F, A]])(implicit L: TraverseFilter[L],
                                                          F: Monad[F]): FoxyT[F, L[A]] =
@@ -270,32 +260,40 @@ package object db {
   def appendForUpdate[A, B <: slick.dbio.NoStream](sql: SqlAction[A, B, Effect.Read]): DBIO[A] =
     sql.overrideStatements(sql.statements.map(_ + " for update"))
 
-  // TODO: I don’t know… does this help? @michalrus
+  // TODO: Is this more readable than inlining? @michalrus
   def ifElse[A](condition: Boolean, ifBranch: ⇒ DbResultT[A], elseBranch: ⇒ DbResultT[A]) =
     if (condition) ifBranch else elseBranch
 
-  def when[F[_]](p: Boolean, s: ⇒ F[Unit])(implicit F: Applicative[F]): F[Unit] =
-    if (p) s.void else F.pure(())
+  def when[F[_]: Applicative](p: Boolean, s: ⇒ F[Unit]): F[Unit] =
+    if (p) s else ().pure[F]
 
-  def doOrGood[A](condition: Boolean, action: ⇒ DbResultT[A], good: ⇒ A)(implicit ec: EC): DbResultT[A] =
-    if (condition) action else DbResultT.good(good)
+  // TODO: Is this more readable than inlining? @michalrus
+  def doOrGood[F[_]: Applicative, A](p: Boolean, action: ⇒ F[A], good: ⇒ A)(implicit ec: EC): F[A] =
+    if (p) action else good.pure[F]
 
-  def doOrFail[A](condition: Boolean, action: ⇒ DbResultT[A], failure: ⇒ Failure)(
-      implicit ec: EC): DbResultT[A] =
-    if (condition) action else DbResultT.failure(failure)
+  // TODO: Is this more readable than inlining? @michalrus
+  def doOrFail[F[_]: Monad, A](p: Boolean, action: ⇒ FoxyT[F, A], failure: ⇒ Failure)(
+      implicit ec: EC): FoxyT[F, A] =
+    if (p) action else FoxyT[F].failure(failure)
 
+  // TODO: Is this more readable than inlining? @michalrus
+  // FIXME: should be defined over FoxyT, but inference fails then… @michalrus
   def failIf(condition: Boolean, failure: ⇒ Failure)(implicit ec: EC): DbResultT[Unit] =
-    if (condition) DbResultT.failure(failure) else DbResultT.unit
+    if (condition) DbResultT.failure(failure) else ().pure[DbResultT]
 
+  // TODO: Is this more readable than inlining? @michalrus
+  // FIXME: should be defined over FoxyT, but inference fails then… @michalrus
   def failIfNot(condition: Boolean, failure: ⇒ Failure)(implicit ec: EC): DbResultT[Unit] =
     failIf(!condition, failure)
 
-  def failIfFailures(failures: Seq[Failure])(implicit ec: EC): DbResultT[Unit] =
+  // TODO: Is this more readable than inlining? @michalrus
+  // TODO: There’s only one usage in the whole codebase. @michalrus
+  def failIfFailures[F[_]: Monad](failures: Seq[Failure]): FoxyT[F, Unit] =
     failures match {
       case head :: tail ⇒
-        DbResultT.failures(NonEmptyList.of(head, tail: _*))
+        FoxyT[F].failures(NonEmptyList.of(head, tail: _*))
       case _ ⇒
-        DbResultT.unit
+        ().pure[FoxyT[F, ?]]
     }
 
   implicit class EnrichedSQLActionBuilder(val action: SQLActionBuilder) extends AnyVal {
@@ -329,33 +327,32 @@ package object db {
 
     def findOrCreate(r: DbResultT[R])(implicit ec: EC): DbResultT[R] =
       dbio.dbresult.flatMap {
-        case Some(model) ⇒ DbResultT.good(model)
+        case Some(model) ⇒ model.pure[DbResultT]
         case None        ⇒ r
       }
 
     // Last item in tuple determines if cart was created or not
     def findOrCreateExtended(r: DbResultT[R])(implicit ec: EC): DbResultT[(R, FoundOrCreated)] =
       dbio.dbresult.flatMap {
-        case Some(model) ⇒ DbResultT.good((model, Found))
+        case Some(model) ⇒ (model, Found: FoundOrCreated).pure[DbResultT]
         case _           ⇒ r.map(result ⇒ (result, Created))
       }
 
     def mustFindOr(notFoundFailure: Failure)(implicit ec: EC): DbResultT[R] =
       dbio.dbresult.flatMap {
-        case Some(model) ⇒ DbResultT.good(model)
+        case Some(model) ⇒ model.pure[DbResultT]
         case None        ⇒ DbResultT.failure(notFoundFailure)
       }
 
     def mustNotFindOr(shouldNotBeHere: Failure)(implicit ec: EC): DbResultT[Unit] =
       dbio.dbresult.flatMap {
-        case None    ⇒ DbResultT.unit
+        case None    ⇒ ().pure[DbResultT]
         case Some(_) ⇒ DbResultT.failure(shouldNotBeHere)
       }
 
-    // we only use this when we *know* we can call head safely on a query. (e.g., you've created a record which
+    // we only use this when we *know* we can call head unsafely on a query. (e.g., you've created a record which
     // has a FK constraint to another table and you then fetch that associated record -- we already *know* it must
     // exist.
-    // FIXME: if you know it, prove it. Or s/safe/unsafe/ in the name *AND* comment. @michalrus
-    def safeGet(implicit ec: EC): DBIO[R] = dbio.map(_.get)
+    def unsafeGet(implicit ec: EC): DBIO[R] = dbio.map(_.get)
   }
 }

phoenix-scala/objectframework/app/objectframework/IlluminateAlgorithm.scala

@@ -2,6 +2,7 @@ package objectframework
 
 import java.time.Instant
 
+import cats.implicits._
 import cats.data.NonEmptyList
 import com.networknt.schema.JsonSchemaFactory
 import com.typesafe.scalalogging.LazyLogging
@@ -34,7 +35,7 @@ object IlluminateAlgorithm extends LazyLogging {
       implicit ec: ExecutionContext): DbResultT[JValue] = {
     val illuminated = projectFlatAttributes(form.attributes, shadow.attributes)
     getInternalAttributes(schema).fold {
-      DbResultT.good(illuminated)
+      illuminated.pure[DbResultT]
     } { jsonSchema ⇒
       val jsonSchemaFactory = new JsonSchemaFactory
       val validator         = jsonSchemaFactory.getSchema(asJsonNode(jsonSchema))
@@ -45,7 +46,7 @@ object IlluminateAlgorithm extends LazyLogging {
         ObjectValidationFailure(form.kind, shadow.id, err.getMessage)
       } match {
         case head :: tail ⇒ DbResultT.failures[JValue](NonEmptyList(head, tail))
-        case Nil          ⇒ DbResultT.good(illuminated)
+        case Nil          ⇒ illuminated.pure[DbResultT]
       }
     }
   }

phoenix-scala/objectframework/app/objectframework/ObjectUtils.scala

@@ -184,7 +184,7 @@ object ObjectUtils {
                              .copy[T](form = updateResult.form, shadow = updateResult.shadow)
                            updateHead(newObject, commit.id)
                          case _ ⇒
-                           DbResultT.good(fullObject)
+                           fullObject.pure[DbResultT]
                        })
     } yield committedObject
 

phoenix-scala/objectframework/app/objectframework/models/ObjectHeadLinks.scala

@@ -84,13 +84,13 @@ object ObjectHeadLinks {
              case right if !linkedRightIds.contains(right.id) ⇒
                create(build(left, right))
            }
-      } yield {}
+      } yield ()
 
     def createIfNotExist(left: L, right: R)(implicit ec: EC, db: DB): DbResultT[Unit] =
       for {
         linkExists ← * <~ filterLeft(left).filter(_.rightId === right.id).exists.result
         _          ← * <~ when(!linkExists, create(build(left, right)).void)
-      } yield {}
+      } yield ()
 
     def build(left: L, right: R): M
   }

phoenix-scala/objectframework/app/objectframework/models/OrderedLinks.scala

@@ -1,5 +1,6 @@
 package objectframework.models
 
+import cats.implicits._
 import core.db.ExPostgresDriver.api._
 import core.db._
 import objectframework.ObjectFailures.{LinkAtPositionCannotBeFound, LinkCannotBeFound}
@@ -44,7 +45,7 @@ abstract class OrderedObjectHeadLinkQueries[M <: OrderedObjectHeadLink[M],
       replacedLink ← * <~ allLefts
                       .filter(_.position === newPosition)
                       .mustFindOneOr(LinkAtPositionCannotBeFound(baseTableRow.getClass, left.id, newPosition))
-      newLinks ← * <~ (if (link.position == newPosition) DbResultT.good((link, replacedLink))
+      newLinks ← * <~ (if (link.position == newPosition) (link, replacedLink).pure[DbResultT]
                        else swapLinkPositions(link, replacedLink))
       (updatedLink, _) = newLinks
     } yield updatedLink

phoenix-scala/objectframework/app/objectframework/payloads/ObjectSchemaValidation.scala

@@ -1,5 +1,6 @@
 package objectframework.payloads
 
+import cats.implicits._
 import cats.data.NonEmptyList
 import com.networknt.schema.JsonSchemaFactory
 import core.db._
@@ -35,7 +36,7 @@ object ObjectSchemaValidation {
         PayloadValidationFailure(error.getMessage)
       } match {
         case head :: tail ⇒ DbResultT.failures[M](NonEmptyList(head, tail))
-        case Nil          ⇒ DbResultT.good(payload)
+        case Nil          ⇒ payload.pure[DbResultT]
       }
 
     }

phoenix-scala/objectframework/app/objectframework/services/ContentManager.scala

@@ -52,7 +52,7 @@ object ContentManager {
 
   type FullContentRelations = Map[String, Seq[Content]]
   def getRelations(content: Content)(implicit ec: EC): DbResultT[FullContentRelations] = {
-    val empty = (Map.empty : FullContentRelations).pure[DbResultT]
+    val empty = (Map.empty: FullContentRelations).pure[DbResultT]
 
     content.relations.foldLeft(empty) { (accRelations, relation) ⇒
       accRelations.flatMap { relations ⇒
@@ -86,7 +86,7 @@ object ContentManager {
         acc :+ (for {
           actualIds ← * <~ ContentQueries.filterCommitIds(kind, expectedIds).result
           _         ← * <~ validateAllCommits(kind, expectedIds, actualIds)
-        } yield {})
+        } yield ())
     }
 
   private def validateAllCommits(kind: String,

phoenix-scala/phoenix/app/phoenix/models/account/User.scala

@@ -139,7 +139,7 @@ object Users extends FoxTableQuery[User, Users](new Users(_)) with ReturningId[U
     maybeEmail match {
       case Some(email) ⇒
         otherUserByEmail(email, accountId).one.mustNotFindOr(UserEmailNotUnique)
-      case None ⇒ DbResultT.unit
+      case None ⇒ ().pure[DbResultT]
     }
 
 }

phoenix-scala/phoenix/app/phoenix/models/activity/Dimension.scala

@@ -1,5 +1,6 @@
 package phoenix.models.activity
 
+import cats.implicits._
 import cats.data.ValidatedNel
 import core.db.ExPostgresDriver.api._
 import core.db._
@@ -58,7 +59,7 @@ object Dimensions
 
   def findOrCreateByName(name: String)(implicit ec: EC): DbResultT[Dimension] =
     findByName(name).one.dbresult.flatMap {
-      case Some(dimension) ⇒ DbResultT.good(dimension)
+      case Some(dimension) ⇒ dimension.pure[DbResultT]
       case None            ⇒ create(Dimension(name = name, description = name.capitalize))
     }
 }

phoenix-scala/phoenix/app/phoenix/models/coupon/IlluminatedCoupon.scala

@@ -20,6 +20,7 @@ case class IlluminatedCoupon(id: Int, context: IlluminatedContext, attributes: J
 
   implicit val formats = JsonFormatters.phoenixFormats
 
+  // FIXME: duplicated code with IlluminatedModel? @michalrus
   def mustBeActive: Either[Failures, IlluminatedCoupon] = {
     val activeFrom = (attributes \ "activeFrom" \ "v").extractOpt[Instant]
     val activeTo   = (attributes \ "activeTo" \ "v").extractOpt[Instant]
@@ -58,7 +59,7 @@ case class IlluminatedCoupon(id: Int, context: IlluminatedContext, attributes: J
           .couponMustBeUsable(id, accountId, rules.usesPerCustomer.getOrElse(0), code.code)
 
       case Some(rules) if rules.isUnlimitedPerCode && rules.isUnlimitedPerCustomer ⇒
-        DbResultT.unit
+        ().pure[DbResultT]
 
       case _ ⇒
         DbResultT.failure(CouponUsageRulesAreEmpty(code.code))