Free up coupons from cancelled orders

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

@@ -41,6 +41,7 @@ case class IlluminatedCoupon(id: Int, context: IlluminatedContext, attributes: J
     val usageRules = (attributes \ "usageRules" \ "v").extractOpt[CouponUsageRules]
 
     val validation = usageRules match {
+      // FIXME: why split the cases to later join them in CouponUsageService? @michalrus
       case Some(rules) if !rules.isUnlimitedPerCode && !rules.isUnlimitedPerCustomer ⇒
         CouponUsageService.mustBeUsableByCustomer(id,
                                                   code.id,

phoenix-scala/phoenix/app/phoenix/services/Checkout.scala

@@ -304,8 +304,8 @@ case class Checkout(
   private def updateCouponCountersForPromotion(customer: User)(implicit ctx: OC): DbResultT[Unit] =
     for {
       maybePromo ← * <~ OrderPromotions.filterByCordRef(cart.refNum).one
-      _ ← * <~ maybePromo.map { promo ⇒
-           CouponUsageService.updateUsageCounts(promo.couponCodeId, customer)
+      _ ← maybePromo.flatMap(_.couponCodeId).traverse { codeId ⇒
+           CouponUsageService.incrementUsageCounts(codeId, customer, incrementBy = 1)
          }
     } yield {}
 

phoenix-scala/phoenix/app/phoenix/services/coupon/CouponUsageService.scala

@@ -10,6 +10,8 @@ import core.db._
 
 object CouponUsageService {
 
+  // TODO: are we entirely sure that all these three denormalized tables for usage counts are absolutely necessary?… @michalrus
+
   private def couponUsageCount(couponFormId: Int, accountId: Int)(implicit ec: EC, db: DB): DBIO[Int] =
     for {
       coupon ← CouponCustomerUsages.filterByCouponAndAccount(couponFormId, accountId).one
@@ -47,48 +49,43 @@ object CouponUsageService {
       _ ← * <~ couponMustBeUsable(couponFormId, accountId, usesAvailableForCustomer, couponCode)
     } yield {}
 
-  def updateUsageCounts(couponCodeId: Option[Int],
-                        customer: User)(implicit ec: EC, db: DB, ctx: OC): DbResultT[Unit] =
-    couponCodeId match {
-      case Some(codeId) ⇒
-        for {
-          couponCode ← * <~ CouponCodes.findById(codeId).extract.one.safeGet
-          context    ← * <~ ObjectContexts.mustFindById400(ctx.id)
-          code       ← * <~ CouponCodes.mustFindById400(codeId)
-          coupon ← * <~ Coupons
-                    .filterByContextAndFormId(context.id, code.couponFormId)
-                    .one
-                    .mustFindOr(CouponNotFoundForContext(code.couponFormId, context.name))
-          form ← * <~ ObjectForms.mustFindById400(coupon.formId)
-          couponUsage ← * <~ CouponUsages
-                         .filterByCoupon(coupon.formId)
+  def incrementUsageCounts(codeId: Int, customer: User, incrementBy: Int)(implicit ec: EC,
+                                                                          db: DB,
+                                                                          ctx: OC): DbResultT[Unit] =
+    for {
+      couponCode ← * <~ CouponCodes.findById(codeId).extract.one.safeGet
+      context    ← * <~ ObjectContexts.mustFindById400(ctx.id)
+      code       ← * <~ CouponCodes.mustFindById400(codeId)
+      coupon ← * <~ Coupons
+                .filterByContextAndFormId(context.id, code.couponFormId)
+                .one
+                .mustFindOr(CouponNotFoundForContext(code.couponFormId, context.name))
+      form ← * <~ ObjectForms.mustFindById400(coupon.formId)
+      couponUsage ← * <~ CouponUsages
+                     .filterByCoupon(coupon.formId)
+                     .one
+                     .findOrCreate(CouponUsages.create(CouponUsage(couponFormId = coupon.formId, count = 1)))
+      couponCodeUsage ← * <~ CouponCodeUsages
+                         .filterByCouponAndCode(coupon.formId, couponCode.id)
                          .one
                          .findOrCreate(
-                           CouponUsages.create(CouponUsage(couponFormId = coupon.formId, count = 1)))
-          couponCodeUsage ← * <~ CouponCodeUsages
-                             .filterByCouponAndCode(coupon.formId, couponCode.id)
-                             .one
-                             .findOrCreate(
-                               CouponCodeUsages.create(
-                                 CouponCodeUsage(couponFormId = coupon.formId,
-                                                 couponCodeId = couponCode.id,
-                                                 count = 0)))
-          couponUsageByCustomer ← * <~ CouponCustomerUsages
-                                   .filterByCouponAndAccount(coupon.formId, customer.accountId)
-                                   .one
-                                   .findOrCreate(
-                                     CouponCustomerUsages.create(
-                                       CouponCustomerUsage(couponFormId = coupon.formId,
-                                                           accountId = customer.accountId,
-                                                           count = 0)))
-          _ ← * <~ CouponUsages.update(couponUsage, couponUsage.copy(count = couponUsage.count + 1))
-          _ ← * <~ CouponCodeUsages.update(couponCodeUsage,
-                                           couponCodeUsage.copy(count = couponCodeUsage.count + 1))
-          _ ← * <~ CouponCustomerUsages.update(
-               couponUsageByCustomer,
-               couponUsageByCustomer.copy(count = couponUsageByCustomer.count + 1))
-        } yield {}
-      case _ ⇒
-        DbResultT.unit
-    }
+                           CouponCodeUsages.create(
+                             CouponCodeUsage(couponFormId = coupon.formId,
+                                             couponCodeId = couponCode.id,
+                                             count = 0)))
+      couponUsageByCustomer ← * <~ CouponCustomerUsages
+                               .filterByCouponAndAccount(coupon.formId, customer.accountId)
+                               .one
+                               .findOrCreate(
+                                 CouponCustomerUsages.create(
+                                   CouponCustomerUsage(couponFormId = coupon.formId,
+                                                       accountId = customer.accountId,
+                                                       count = 0)))
+      _ ← * <~ CouponUsages.update(couponUsage, couponUsage.copy(count = couponUsage.count + incrementBy))
+      _ ← * <~ CouponCodeUsages.update(couponCodeUsage,
+                                       couponCodeUsage.copy(count = couponCodeUsage.count + incrementBy))
+      _ ← * <~ CouponCustomerUsages.update(
+           couponUsageByCustomer,
+           couponUsageByCustomer.copy(count = couponUsageByCustomer.count + incrementBy))
+    } yield ()
 }

phoenix-scala/phoenix/app/phoenix/services/orders/OrderStateUpdater.scala

@@ -1,5 +1,6 @@
 package phoenix.services.orders
 
+import cats.syntax._
 import cats.implicits._
 import core.db._
 import core.failures.NotFoundFailure400
@@ -15,28 +16,40 @@ import phoenix.models.payment.storecredit.StoreCreditAdjustments.scope._
 import phoenix.responses.cord.{AllOrders, OrderResponse}
 import phoenix.responses.{BatchMetadata, BatchMetadataSource}
 import phoenix.services.LogActivity
+import phoenix.services.coupon.CouponUsageService
 import phoenix.utils.aliases._
 import phoenix.utils.apis.Apis
 import responses.BatchResponse
 import slick.jdbc.PostgresProfile.api._
 
 object OrderStateUpdater {
 
-  def updateState(
-      admin: User,
-      refNum: String,
-      newState: Order.State)(implicit ec: EC, db: DB, ac: AC, apis: Apis, au: AU): DbResultT[OrderResponse] =
+  def updateState(admin: User, refNum: String, newState: Order.State)(implicit ec: EC,
+                                                                      db: DB,
+                                                                      ac: AC,
+                                                                      oc: OC,
+                                                                      apis: Apis,
+                                                                      au: AU): DbResultT[OrderResponse] =
     for {
       order   ← * <~ Orders.mustFindByRefNum(refNum)
       _       ← * <~ order.transitionState(newState)
       _       ← * <~ updateQueries(admin, Seq(refNum), newState)
       updated ← * <~ Orders.mustFindByRefNum(refNum)
       _ ← * <~ doOrMeh(updated.state == Order.Canceled,
-                       DbResultT.fromResult(apis.middlewarehouse.cancelHold(refNum)))
+                       DbResultT.fromResult(apis.middlewarehouse.cancelHold(refNum)) >> freeCoupon(order))
       response ← * <~ OrderResponse.fromOrder(updated, grouped = true)
       _        ← * <~ doOrMeh(order.state != newState, LogActivity().orderStateChanged(admin, response, order.state))
     } yield response
 
+  private def freeCoupon(order: Order)(implicit ec: EC, db: DB, oc: OC): DbResultT[Unit] =
+    for {
+      maybePromo ← * <~ OrderPromotions.filterByCordRef(order.refNum).one
+      customer   ← Users.mustFindByAccountId(order.accountId)
+      _ ← maybePromo.flatMap(_.couponCodeId).traverse { codeId ⇒
+           CouponUsageService.incrementUsageCounts(codeId, customer, incrementBy = -1)
+         }
+    } yield ()
+
   def updateStates(
       admin: User,
       refNumbers: Seq[String],

phoenix-scala/phoenix/test/integration/CouponsIntegrationTest.scala

@@ -9,20 +9,31 @@ import objectframework.models.ObjectContext
 import org.json4s.JsonAST._
 import phoenix.failures.CartFailures.OrderAlreadyPlaced
 import phoenix.failures.CouponFailures.CouponIsNotActive
-import phoenix.models.cord.{Carts, Orders}
-import phoenix.models.coupon.Coupon
+import phoenix.models.cord.{Carts, Order, Orders}
+import phoenix.models.coupon.{Coupon, CouponUsageRules}
 import phoenix.models.traits.IlluminatedModel
 import phoenix.payloads.CartPayloads.CreateCart
 import phoenix.payloads.LineItemPayloads.UpdateLineItemsPayload
 import phoenix.responses.CouponResponses.CouponResponse
-import phoenix.responses.cord.CartResponse
+import phoenix.responses.cord.{CartResponse, OrderResponse}
 import phoenix.utils.time.RichInstant
 import testutils._
 import testutils.apis.PhoenixAdminApi
 import testutils.fixtures.BakedFixtures
 import testutils.fixtures.api._
 import core.utils.Money._
 import core.db._
+import phoenix.failures.ShippingMethodFailures.ShippingMethodNotFoundByName
+import phoenix.models.Reasons
+import phoenix.models.location.Region
+import phoenix.models.shipping.{ShippingMethod, ShippingMethods}
+import phoenix.payloads.GiftCardPayloads.GiftCardCreateByCsr
+import phoenix.payloads.OrderPayloads.UpdateOrderPayload
+import phoenix.payloads.PaymentPayloads.GiftCardPayment
+import phoenix.payloads.UpdateShippingMethod
+import phoenix.responses.AddressResponse
+import phoenix.responses.giftcards.GiftCardResponse
+import phoenix.utils.seeds.Factories
 
 class CouponsIntegrationTest
     extends IntegrationTestBase
@@ -173,6 +184,64 @@ class CouponsIntegrationTest
     }
   }
 
+  "→ Coupon code from cancelled order can be reused" in new Coupon_AnyQualifier_PercentOff {
+    import slick.jdbc.PostgresProfile.api._
+
+    override def couponUsageRules = CouponUsageRules(
+      isUnlimitedPerCode = false,
+      usesPerCode = Some(1),
+      isUnlimitedPerCustomer = false,
+      usesPerCustomer = Some(1)
+    )
+
+    // TODO: extract CheckoutFixture and reuse it here (more refactoring will be needed for that) @michalrus
+
+    val (customer, customerLoginData) = api_newCustomerWithLogin()
+    val skuCode                       = ProductSku_ApiFixture().skuCode
+    val refNum                        = api_newCustomerCart(customer.id).referenceNumber
+
+    // Place the order.
+    {
+      val addressPayload = randomAddress(regionId = Region.californiaId)
+      val address = customersApi(customer.id).addresses
+        .create(addressPayload)
+        .as[AddressResponse]
+
+      val (shipMethod, reason) = (for {
+        _ ← * <~ ShippingMethods.createAll(Factories.shippingMethods)
+        shipMethodName = ShippingMethod.expressShippingNameForAdmin
+        shipMethod ← * <~ ShippingMethods
+                      .filter(_.adminDisplayName === shipMethodName)
+                      .mustFindOneOr(ShippingMethodNotFoundByName(shipMethodName))
+        reason ← * <~ Reasons.create(Factories.reason(defaultAdmin.id))
+      } yield (shipMethod, reason)).gimme
+
+      val cartApi = cartsApi(refNum)
+      cartApi.lineItems.add(Seq(UpdateLineItemsPayload(skuCode, 2))).mustBeOk()
+      cartApi.shippingAddress.updateFromAddress(address.id).mustBeOk()
+      cartApi.shippingMethod
+        .update(UpdateShippingMethod(shipMethod.id))
+        .asTheResult[CartResponse]
+      cartApi.coupon.add(couponCode).asTheResult[CartResponse]
+      val grandTotal = cartApi.get.asTheResult[CartResponse].totals.total
+      val gcCode = giftCardsApi
+        .create(GiftCardCreateByCsr(grandTotal, reason.id))
+        .as[GiftCardResponse]
+        .code
+      cartApi.payments.giftCard.add(GiftCardPayment(gcCode, grandTotal.some)).mustBeOk()
+      cartApi.checkout().as[OrderResponse]
+    }
+
+    // Cancel.
+    ordersApi(refNum).update(UpdateOrderPayload(Order.Canceled)).as[OrderResponse].orderState must === (
+      Order.Canceled)
+
+    // Try to reuse that same coupon.
+    val refNum2 = api_newCustomerCart(customer.id).referenceNumber
+    cartsApi(refNum2).lineItems.add(Seq(UpdateLineItemsPayload(skuCode, 2))).mustBeOk()
+    cartsApi(refNum2).coupon.add(couponCode).asTheResult[CartResponse]
+  }
+
   trait CartCouponFixture extends StoreAdmin_Seed with Coupon_TotalQualifier_PercentOff {
 
     val skuCode = ProductSku_ApiFixture(skuPrice = 3100).skuCode

phoenix-scala/phoenix/test/integration/testutils/fixtures/api/ApiFixtures.scala

@@ -10,6 +10,7 @@ import org.json4s.JsonAST.JNull
 import org.json4s.JsonDSL._
 import org.scalatest.SuiteMixin
 import phoenix.models.catalog.Catalog
+import phoenix.models.coupon.CouponUsageRules
 import phoenix.models.promotion.Promotion
 import phoenix.payloads.CouponPayloads.CreateCoupon
 import phoenix.payloads.ProductPayloads.CreateProductPayload
@@ -170,12 +171,14 @@ trait ApiFixtures extends SuiteMixin with HttpSupport with PhoenixAdminApi with
   trait CouponFixtureBase {
     def couponActiveFrom: Instant       = Instant.now.minus(1, DAYS)
     def couponActiveTo: Option[Instant] = None
+    def couponUsageRules: CouponUsageRules =
+      CouponUsageRules(isUnlimitedPerCode = true, isUnlimitedPerCustomer = true)
 
     def promotion: PromotionResponse
 
     lazy val coupon = couponsApi
       .create(
-        CreateCoupon(couponAttrs(couponActiveFrom, couponActiveTo),
+        CreateCoupon(couponAttrs(couponActiveFrom, couponActiveTo, couponUsageRules),
                      promotion.id,
                      singleCode = Some(Lorem.letterify("?????")),
                      generateCodes = None))(implicitly, defaultAdminAuth)
@@ -185,19 +188,17 @@ trait ApiFixtures extends SuiteMixin with HttpSupport with PhoenixAdminApi with
 
     lazy val couponCode = coupon.code
 
-    protected def couponAttrs(activeFrom: Instant, activeTo: Option[Instant]): Map[String, Json] = {
-      val usageRules = {
-        ("isExclusive"            → true) ~
-        ("isUnlimitedPerCode"     → true) ~
-        ("isUnlimitedPerCustomer" → true)
-      }.asShadowVal(t = "usageRules")
+    protected def couponAttrs(activeFrom: Instant,
+                              activeTo: Option[Instant],
+                              usageRules: CouponUsageRules): Map[String, Json] = {
+      import org.json4s.Extraction.decompose
 
       val commonAttrs = Map[String, Any](
         "name"           → "Order coupon",
         "storefrontName" → "Order coupon",
         "description"    → "Order coupon description",
         "details"        → "Order coupon details".richText,
-        "usageRules"     → usageRules,
+        "usageRules"     → decompose(usageRules).asShadowVal(t = "usageRules"),
         "activeFrom"     → activeFrom
       )