diff --git a/phoenix-scala/phoenix/app/phoenix/models/coupon/IlluminatedCoupon.scala b/phoenix-scala/phoenix/app/phoenix/models/coupon/IlluminatedCoupon.scala index 5fcb933b30..aba5b49522 100644 --- a/phoenix-scala/phoenix/app/phoenix/models/coupon/IlluminatedCoupon.scala +++ b/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, diff --git a/phoenix-scala/phoenix/app/phoenix/services/Checkout.scala b/phoenix-scala/phoenix/app/phoenix/services/Checkout.scala index 87eb43afe8..bd4dcc6c1d 100644 --- a/phoenix-scala/phoenix/app/phoenix/services/Checkout.scala +++ b/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 {} diff --git a/phoenix-scala/phoenix/app/phoenix/services/coupon/CouponUsageService.scala b/phoenix-scala/phoenix/app/phoenix/services/coupon/CouponUsageService.scala index b7e220816f..daa32293c1 100644 --- a/phoenix-scala/phoenix/app/phoenix/services/coupon/CouponUsageService.scala +++ b/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 () } diff --git a/phoenix-scala/phoenix/app/phoenix/services/orders/OrderStateUpdater.scala b/phoenix-scala/phoenix/app/phoenix/services/orders/OrderStateUpdater.scala index 1780432f16..7be07c5bba 100644 --- a/phoenix-scala/phoenix/app/phoenix/services/orders/OrderStateUpdater.scala +++ b/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,6 +16,7 @@ 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 @@ -22,21 +24,32 @@ 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], diff --git a/phoenix-scala/phoenix/test/integration/CouponsIntegrationTest.scala b/phoenix-scala/phoenix/test/integration/CouponsIntegrationTest.scala index a2ecb16c3b..e487a1aaa5 100644 --- a/phoenix-scala/phoenix/test/integration/CouponsIntegrationTest.scala +++ b/phoenix-scala/phoenix/test/integration/CouponsIntegrationTest.scala @@ -9,13 +9,13 @@ 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 @@ -23,6 +23,17 @@ 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 diff --git a/phoenix-scala/phoenix/test/integration/testutils/fixtures/api/ApiFixtures.scala b/phoenix-scala/phoenix/test/integration/testutils/fixtures/api/ApiFixtures.scala index 8e26925707..ef8a432f91 100644 --- a/phoenix-scala/phoenix/test/integration/testutils/fixtures/api/ApiFixtures.scala +++ b/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 )