refactoring

guardian · Jan 22, 2024 · 0d67941 · 0d67941
1 parent fd74c4e
commit 0d67941
Show file tree

Hide file tree

Showing 9 changed files with 83 additions and 123 deletions.
diff --git a/lambda/src/main/scala/pricemigrationengine/handlers/AmendmentHandler.scala b/lambda/src/main/scala/pricemigrationengine/handlers/AmendmentHandler.scala
@@ -7,7 +7,7 @@ import pricemigrationengine.migrations._
 import pricemigrationengine.services._
 import zio.{Clock, ZIO}
 import java.time.{LocalDate, LocalDateTime, ZoneOffset}
-import pricemigrationengine.migrations.newspaper2024migration.Newspaper2024MigrationAmendment
+import pricemigrationengine.migrations.newspaper2024migration
 
 /** Carries out price-rise amendments in Zuora.
   */
@@ -211,7 +211,7 @@ object AmendmentHandler extends CohortHandler {
           )
         case Newspaper2024 =>
           ZIO.fromEither(
-            Newspaper2024MigrationAmendment.subscriptionToZuoraSubscriptionUpdate(
+            newspaper2024migration.Amendment.subscriptionToZuoraSubscriptionUpdate(
               subscriptionBeforeUpdate,
               startDate,
             )

diff --git a/lambda/src/main/scala/pricemigrationengine/handlers/EstimationHandler.scala b/lambda/src/main/scala/pricemigrationengine/handlers/EstimationHandler.scala
@@ -1,6 +1,6 @@
 package pricemigrationengine.handlers
 
-import pricemigrationengine.migrations.newspaper2024migration.Newspaper2024MigrationEstimation
+import pricemigrationengine.migrations.newspaper2024migration
 import pricemigrationengine.model.CohortTableFilter._
 import pricemigrationengine.model._
 import pricemigrationengine.services._
@@ -147,7 +147,7 @@ object EstimationHandler extends CohortHandler {
       MigrationType(cohortSpec) match {
         case Membership2023Monthlies => 1
         case Membership2023Annuals   => 1
-        case Newspaper2024           => Newspaper2024MigrationEstimation.startDateSpreadPeriod(subscription)
+        case Newspaper2024           => newspaper2024migration.Estimation.startDateSpreadPeriod(subscription)
         case _                       => 3
       }
     } else 1
@@ -168,8 +168,9 @@ object EstimationHandler extends CohortHandler {
     // nature of that migration), which also takes the subscription.
 
     val startDateLowerBound1 = MigrationType(cohortSpec) match {
-      case Newspaper2024 => Newspaper2024MigrationEstimation.startDateGeneralLowerbound(cohortSpec, today, subscription)
-      case _             => startDateGeneralLowerbound(cohortSpec, today)
+      case Newspaper2024 =>
+        newspaper2024migration.Estimation.startDateGeneralLowerbound(cohortSpec, today, subscription)
+      case _ => startDateGeneralLowerbound(cohortSpec, today)
     }
 
     // We now respect the policy of not increasing members during their first year

diff --git a/lambda/src/main/scala/pricemigrationengine/handlers/NotificationHandler.scala b/lambda/src/main/scala/pricemigrationengine/handlers/NotificationHandler.scala
@@ -6,7 +6,7 @@ import pricemigrationengine.model.membershipworkflow._
 import pricemigrationengine.services._
 import zio.{Clock, ZIO}
 import com.gu.i18n
-import pricemigrationengine.migrations.newspaper2024migration.Newspaper2024MigrationStaticData
+import pricemigrationengine.migrations.newspaper2024migration
 
 import java.time.LocalDate
 import java.time.format.DateTimeFormatter
@@ -34,7 +34,7 @@ object NotificationHandler extends CohortHandler {
       case Membership2023Annuals   => 33
       case SupporterPlus2023V1V2MA => 33
       case DigiSubs2023            => 33
-      case Newspaper2024           => Newspaper2024MigrationStaticData.maxLeadTime
+      case Newspaper2024           => newspaper2024migration.StaticData.maxLeadTime
       case Legacy                  => 49
     }
   }
@@ -45,7 +45,7 @@ object NotificationHandler extends CohortHandler {
       case Membership2023Annuals   => 31
       case SupporterPlus2023V1V2MA => 31
       case DigiSubs2023            => 31
-      case Newspaper2024           => Newspaper2024MigrationStaticData.minLeadTime
+      case Newspaper2024           => newspaper2024migration.StaticData.minLeadTime
       case Legacy                  => 35
     }
   }

diff --git a/...ion/Newspaper2024MigrationAmendment.scala → ...ns/Newspaper2024Migration/Amendment.scala b/...ion/Newspaper2024MigrationAmendment.scala → ...ns/Newspaper2024Migration/Amendment.scala
@@ -1,19 +1,19 @@
 package pricemigrationengine.migrations.newspaper2024migration
 import pricemigrationengine.model._
-import pricemigrationengine.migrations.newspaper2024migration.Newspaper2024MigrationStaticData._
-import pricemigrationengine.migrations.newspaper2024migration.Newspaper2024MigrationEstimation._
+import pricemigrationengine.migrations.newspaper2024migration.StaticData._
+import pricemigrationengine.migrations.newspaper2024migration.Estimation._
 
 import java.time.LocalDate
 
-object Newspaper2024MigrationAmendment {
+object Amendment {
 
   def subscriptionToNewChargeDistribution2024(subscription: ZuoraSubscription): Option[ChargeDistribution2024] = {
-    val priceCorrectionFactor = Newspaper2024MigrationPriceCapping.priceCorrectionFactor(subscription)
+    val priceCorrectionFactor = PriceCapping.priceCorrectionFactor(subscription)
     for {
-      data2024 <- Newspaper2024MigrationEstimation
+      data2024 <- Estimation
         .subscriptionToSubscriptionData2024(subscription)
         .toOption
-      priceDistribution <- Newspaper2024MigrationStaticData.priceDistributionLookup(
+      priceDistribution <- StaticData.priceDistributionLookup(
         data2024.productName,
         data2024.billingPeriod,
         data2024.ratePlanName
@@ -47,26 +47,9 @@ object Newspaper2024MigrationAmendment {
       subscription: ZuoraSubscription,
       effectiveDate: LocalDate,
   ): Either[AmendmentDataFailure, ZuoraSubscriptionUpdate] = {
-
-    def transform1[T](option: Option[T]): Either[AmendmentDataFailure, T] = {
-      option match {
-        case None                 => Left(AmendmentDataFailure("error"))
-        case Some(ratePlanCharge) => Right(ratePlanCharge)
-      }
-    }
-
-    def transform2[T](data: Either[String, T]): Either[AmendmentDataFailure, T] = {
-      data match {
-        case Left(string) => Left(AmendmentDataFailure(string))
-        case Right(t)     => Right(t)
-      }
-    }
-
     for {
-      data2024 <- transform2[SubscriptionData2024](
-        Newspaper2024MigrationEstimation.subscriptionToSubscriptionData2024(subscription)
-      )
-      chargeDistribution <- transform1[ChargeDistribution2024](subscriptionToNewChargeDistribution2024(subscription))
+      data2024 <- Estimation.subscriptionToSubscriptionData2024(subscription).left.map(AmendmentDataFailure)
+      chargeDistribution <- subscriptionToNewChargeDistribution2024(subscription).toRight(AmendmentDataFailure("error"))
     } yield ZuoraSubscriptionUpdate(
       add = List(
         AddZuoraRatePlan(

diff --git a/...on/Newspaper2024MigrationEstimation.scala → ...s/Newspaper2024Migration/Estimation.scala b/...on/Newspaper2024MigrationEstimation.scala → ...s/Newspaper2024Migration/Estimation.scala
@@ -3,7 +3,7 @@ import pricemigrationengine.model._
 
 import java.time.LocalDate
 
-object Newspaper2024MigrationEstimation {
+object Estimation {
 
   // --------------------------------------------------------------------------------------------
 
@@ -83,7 +83,7 @@ object Newspaper2024MigrationEstimation {
             (price: BigDecimal, ratePlanCharge: ZuoraRatePlanCharge) =>
               price + ratePlanCharge.price.getOrElse(BigDecimal(0))
           )
-          targetRatePlanId <- Newspaper2024MigrationStaticData.ratePlanIdLookUp(productName, ratePlan.ratePlanName)
+          targetRatePlanId <- StaticData.ratePlanIdLookUp(productName, ratePlan.ratePlanName)
         } yield SubscriptionData2024(
           productName = productName,
           ratePlan = ratePlan,
@@ -92,10 +92,7 @@ object Newspaper2024MigrationEstimation {
           currency = currency,
           currentPrice = currentPrice,
           targetRatePlanId = targetRatePlanId
-        )) match {
-          case Some(data) => Right(data)
-          case _ => Left(s"[error: 0e218c37] Could not determine SubscriptionData2024 for subscription ${subscription}")
-        }
+        )).toRight(s"[error: 0e218c37] Could not determine SubscriptionData2024 for subscription ${subscription}")
       }
       case _ =>
         Left(
@@ -120,10 +117,10 @@ object Newspaper2024MigrationEstimation {
     // The price correction factor is meant to be 1 except in the case of
     // subscriptions with capping in which case the correction factor will have been precomputed
     // from the data generated by Matt
-    val priceCorrectionFactor = Newspaper2024MigrationPriceCapping.priceCorrectionFactor(subscription)
+    val priceCorrectionFactor = PriceCapping.priceCorrectionFactor(subscription)
     for {
       data2024 <- subscriptionToSubscriptionData2024(subscription).toOption
-      price <- Newspaper2024MigrationStaticData.priceLookup(
+      price <- StaticData.priceLookup(
         data2024.productName,
         data2024.billingPeriod,
         data2024.ratePlanName
@@ -137,24 +134,10 @@ object Newspaper2024MigrationEstimation {
 
     // PriceData(currency: Currency, oldPrice: BigDecimal, newPrice: BigDecimal, billingPeriod: String)
 
-    def transform1[T](option: Option[T]): Either[AmendmentDataFailure, T] = {
-      option match {
-        case None                 => Left(AmendmentDataFailure("error"))
-        case Some(ratePlanCharge) => Right(ratePlanCharge)
-      }
-    }
-
-    def transform2[T](data: Either[String, T]): Either[AmendmentDataFailure, T] = {
-      data match {
-        case Left(string) => Left(AmendmentDataFailure(string))
-        case Right(t)     => Right(t)
-      }
-    }
-
     for {
-      data2024 <- transform2[SubscriptionData2024](subscriptionToSubscriptionData2024(subscription))
+      data2024 <- subscriptionToSubscriptionData2024(subscription).left.map(AmendmentDataFailure)
       oldPrice = data2024.currentPrice
-      newPrice <- transform1[BigDecimal](subscriptionToNewPrice(subscription))
+      newPrice <- subscriptionToNewPrice(subscription).toRight(AmendmentDataFailure("error"))
     } yield PriceData(
       data2024.currency,
       oldPrice,
@@ -234,7 +217,7 @@ object Newspaper2024MigrationEstimation {
     datesMax(
       earliestPriceMigrationStartDate,
       today.plusDays(
-        Newspaper2024MigrationStaticData.minLeadTime + 1
+        StaticData.minLeadTime + 1
       ) // +1 because we need to be strictly over minLeadTime days away. Exactly minLeadTime is not enough.
     )
   }

diff --git a/.../Newspaper2024MigrationPriceCapping.scala → ...Newspaper2024Migration/PriceCapping.scala b/.../Newspaper2024MigrationPriceCapping.scala → ...Newspaper2024Migration/PriceCapping.scala
@@ -3,7 +3,7 @@ import pricemigrationengine.model._
 
 import java.time.LocalDate
 
-object Newspaper2024MigrationPriceCapping {
+object PriceCapping {
 
   // The engine has a notion of capping, but it would not work with this migration
   // Instead, Matt S. provided us with a list of subscriptions which need to be capped

diff --git a/...on/Newspaper2024MigrationStaticData.scala → ...s/Newspaper2024Migration/StaticData.scala b/...on/Newspaper2024MigrationStaticData.scala → ...s/Newspaper2024Migration/StaticData.scala
@@ -3,7 +3,7 @@ import pricemigrationengine.model._
 
 import java.time.LocalDate
 
-object Newspaper2024MigrationStaticData {
+object StaticData {
 
   val maxLeadTime = 40
   val minLeadTime = 35

diff --git a/lambda/src/main/scala/pricemigrationengine/model/AmendmentData.scala b/lambda/src/main/scala/pricemigrationengine/model/AmendmentData.scala
@@ -1,6 +1,6 @@
 package pricemigrationengine.model
 
-import pricemigrationengine.migrations.newspaper2024migration.Newspaper2024MigrationEstimation
+import pricemigrationengine.migrations.newspaper2024migration
 import pricemigrationengine.migrations.{DigiSubs2023Migration, GuardianWeeklyMigration, Membership2023Migration}
 import pricemigrationengine.model.ZuoraProductCatalogue.{homeDeliveryRatePlans, productPricingMap}
 
@@ -339,7 +339,7 @@ object AmendmentData {
           subscription
         )
       case Newspaper2024 =>
-        Newspaper2024MigrationEstimation.priceData(
+        newspaper2024migration.Estimation.priceData(
           subscription
         )
       case Legacy => priceDataWithRatePlanMatching(account, catalogue, subscription, invoiceList, nextServiceStartDate)