Introduce MigrationType

lambda/src/main/scala/pricemigrationengine/handlers/AmendmentHandler.scala

@@ -116,8 +116,8 @@ object AmendmentHandler extends CohortHandler {
       invoicePreviewBeforeUpdate <-
         Zuora.fetchInvoicePreview(subscriptionBeforeUpdate.accountId, invoicePreviewTargetDate)
 
-      update <-
-        if (CohortSpec.isMembershipPriceRiseMonthlies(cohortSpec)) {
+      update <- MigrationType(cohortSpec) match {
+        case Membership2023Monthlies =>
           ZIO.fromEither(
             ZuoraSubscriptionUpdate
               .updateOfRatePlansToCurrent_Membership2023_Monthlies(
@@ -126,7 +126,7 @@ object AmendmentHandler extends CohortHandler {
                 startDate
               )
           )
-        } else if (CohortSpec.isMembershipPriceRiseAnnuals(cohortSpec)) {
+        case Membership2023Annuals =>
           ZIO.fromEither(
             ZuoraSubscriptionUpdate
               .updateOfRatePlansToCurrent_Membership2023_Annuals(
@@ -135,7 +135,7 @@ object AmendmentHandler extends CohortHandler {
                 startDate
               )
           )
-        } else {
+        case _ =>
           ZIO.fromEither(
             ZuoraSubscriptionUpdate
               .updateOfRatePlansToCurrent(
@@ -147,7 +147,7 @@ object AmendmentHandler extends CohortHandler {
                 PriceCap.priceCorrectionFactorForPriceCap(oldPrice, estimatedNewPrice)
               )
           )
-        }
+      }
 
       newSubscriptionId <- Zuora.updateSubscription(subscriptionBeforeUpdate, update)
 
@@ -178,18 +178,18 @@ object AmendmentHandler extends CohortHandler {
   }
 
   def handle(input: CohortSpec): ZIO[Logging, Failure, HandlerOutput] = {
-    if (!CohortSpec.isMembershipPriceRiseAnnuals(input)) {
-      main(input).provideSome[Logging](
-        EnvConfig.cohortTable.layer,
-        EnvConfig.zuora.layer,
-        EnvConfig.stage.layer,
-        DynamoDBZIOLive.impl,
-        DynamoDBClientLive.impl,
-        CohortTableLive.impl(input),
-        ZuoraLive.impl
-      )
-    } else {
-      ZIO.succeed(HandlerOutput(isComplete = true))
+    MigrationType(input) match {
+      case Membership2023Annuals => ZIO.succeed(HandlerOutput(isComplete = true))
+      case _ =>
+        main(input).provideSome[Logging](
+          EnvConfig.cohortTable.layer,
+          EnvConfig.zuora.layer,
+          EnvConfig.stage.layer,
+          DynamoDBZIOLive.impl,
+          DynamoDBClientLive.impl,
+          CohortTableLive.impl(input),
+          ZuoraLive.impl
+        )
     }
   }
 }

lambda/src/main/scala/pricemigrationengine/handlers/EstimationHandler.scala

@@ -122,9 +122,13 @@ object EstimationHandler extends CohortHandler {
       invoicePreview: ZuoraInvoiceList,
       cohortSpec: CohortSpec
   ): Int = {
-    if (isMonthlySubscription(subscription, invoicePreview))
-      if (CohortSpec.isMembershipPriceRise(cohortSpec)) 1 else 3
-    else 1
+    if (isMonthlySubscription(subscription, invoicePreview)) {
+      MigrationType(cohortSpec) match {
+        case Membership2023Monthlies => 1
+        case Membership2023Annuals   => 1
+        case _                       => 3
+      }
+    } else 1
   }
 
   def decideStartDateLowerboundWithRandomAddition(

lambda/src/main/scala/pricemigrationengine/handlers/NotificationHandler.scala

@@ -19,28 +19,29 @@ object NotificationHandler extends CohortHandler {
   // For general information about the notification period see the docs/notification-periods.md
 
   // The standard notification period for letter products (where the notification is delivered by email)
-  // is -49 (included) to -35 (excluded) days.
-  val guLettersNotificationLeadTime = 49
-  private val engineLettersMinNotificationLeadTime = 35
+  // is -49 (included) to -35 (excluded) days. Legally the min is 30 days, but we set 35 days to alert if a
+  // subscription if exiting the notification window and needs to be investigated and repaired before the deadline
+  // of 30 days.
+
+  val letterMaxNotificationLeadTime = 49
+  private val letterMinNotificationLeadTime = 35
 
   // Membership migration
   // Notification period: -33 (included) to -31 (excluded) days
-  private val membershipPriceRiseNotificationLeadTime = 33
-  private val membershipMinNotificationLeadTime = 31
+  private val emailMaxNotificationLeadTime = 33
+  private val emailMinNotificationLeadTime = 31
 
   def maxLeadTime(cohortSpec: CohortSpec): Int = {
-    if (CohortSpec.isMembershipPriceRise(cohortSpec)) {
-      membershipPriceRiseNotificationLeadTime
-    } else {
-      guLettersNotificationLeadTime
+    MigrationType(cohortSpec) match {
+      case Legacy => letterMaxNotificationLeadTime
+      case _      => emailMaxNotificationLeadTime
     }
   }
 
   def minLeadTime(cohortSpec: CohortSpec): Int = {
-    if (CohortSpec.isMembershipPriceRise(cohortSpec)) {
-      membershipMinNotificationLeadTime
-    } else {
-      engineLettersMinNotificationLeadTime
+    MigrationType(cohortSpec) match {
+      case Legacy => letterMinNotificationLeadTime
+      case _      => emailMinNotificationLeadTime
     }
   }
 
@@ -135,19 +136,21 @@ object NotificationHandler extends CohortHandler {
       )
       lastName <- requiredField(contact.LastName, "Contact.LastName")
       address <- targetAddress(contact)
-      street <-
-        if (CohortSpec.isMembershipPriceRise(cohortSpec)) {
+      street <- MigrationType(cohortSpec) match {
+        case Membership2023Monthlies =>
           requiredField(address.street.fold(Some(""))(Some(_)), "Contact.OtherAddress.street")
-        } else {
-          requiredField(address.street, "Contact.OtherAddress.street")
-        }
+        case Membership2023Annuals =>
+          requiredField(address.street.fold(Some(""))(Some(_)), "Contact.OtherAddress.street")
+        case _ => requiredField(address.street, "Contact.OtherAddress.street")
+      }
       postalCode = address.postalCode.getOrElse("")
-      country <-
-        if (CohortSpec.isMembershipPriceRise(cohortSpec)) {
+      country <- MigrationType(cohortSpec) match {
+        case Membership2023Monthlies =>
           requiredField(address.country.fold(Some("United Kingdom"))(Some(_)), "Contact.OtherAddress.country")
-        } else {
-          requiredField(address.country, "Contact.OtherAddress.country")
-        }
+        case Membership2023Annuals =>
+          requiredField(address.country.fold(Some("United Kingdom"))(Some(_)), "Contact.OtherAddress.country")
+        case _ => requiredField(address.country, "Contact.OtherAddress.country")
+      }
       oldPrice <- requiredField(cohortItem.oldPrice, "CohortItem.oldPrice")
       estimatedNewPrice <- requiredField(cohortItem.estimatedNewPrice, "CohortItem.estimatedNewPrice")
       startDate <- requiredField(cohortItem.startDate.map(_.toString()), "CohortItem.startDate")
@@ -156,8 +159,13 @@ object NotificationHandler extends CohortHandler {
       currencyISOCode <- requiredField(cohortItem.currency, "CohortItem.currency")
       currencySymbol <- currencyISOtoSymbol(currencyISOCode)
 
-      // In the case of membership price rise, we need to not cap the price
-      cappedEstimatedNewPriceWithCurrencySymbol = s"${currencySymbol}${PriceCap.cappedPrice(oldPrice, estimatedNewPrice, CohortSpec.isMembershipPriceRise(cohortSpec))}"
+      forceEstimated = MigrationType(cohortSpec) match {
+        case Membership2023Monthlies => true
+        case Membership2023Annuals   => true
+        case _                       => false
+      }
+
+      cappedEstimatedNewPriceWithCurrencySymbol = s"${currencySymbol}${PriceCap.cappedPrice(oldPrice, estimatedNewPrice, forceEstimated)}"
 
       _ <- logMissingEmailAddress(cohortItem, contact)
 

lambda/src/main/scala/pricemigrationengine/handlers/SalesforcePriceRiseCreationHandler.scala

@@ -80,16 +80,21 @@ object SalesforcePriceRiseCreationHandler extends CohortHandler {
         ZIO
           .fromOption(cohortItem.startDate)
           .orElseFail(SalesforcePriceRiseWriteFailure(s"$cohortItem does not have a startDate"))
-    } yield SalesforcePriceRise(
-      Some(subscription.Name),
-      Some(subscription.Buyer__c),
-      Some(oldPrice),
-      Some(
-        PriceCap.cappedPrice(oldPrice, estimatedNewPrice, CohortSpec.isMembershipPriceRise(cohortSpec))
-      ), // In case of membership price rise, we override the capping
-      Some(priceRiseDate),
-      Some(subscription.Id)
-    )
+    } yield {
+      val forceEstimated = MigrationType(cohortSpec) match {
+        case Membership2023Monthlies => true
+        case Membership2023Annuals   => true
+        case _                       => false
+      }
+      SalesforcePriceRise(
+        Some(subscription.Name),
+        Some(subscription.Buyer__c),
+        Some(oldPrice),
+        Some(PriceCap.cappedPrice(oldPrice, estimatedNewPrice, forceEstimated)),
+        Some(priceRiseDate),
+        Some(subscription.Id)
+      )
+    }
   }
 
   def handle(input: CohortSpec): ZIO[Logging, Failure, HandlerOutput] =

lambda/src/main/scala/pricemigrationengine/model/AmendmentData.scala

@@ -90,10 +90,12 @@ object AmendmentData {
       moment, just a difference between regular price rises and membership (batch 1) will do.
      */
 
-    if (CohortSpec.isMembershipPriceRise(cohortSpec)) {
-      Membership2023.priceData(account, catalogue, subscription, invoiceList, nextServiceStartDate, cohortSpec)
-    } else {
-      priceDataWithRatePlanMatching(account, catalogue, subscription, invoiceList, nextServiceStartDate)
+    MigrationType(cohortSpec) match {
+      case Membership2023Monthlies =>
+        Membership2023.priceData(account, catalogue, subscription, invoiceList, nextServiceStartDate, cohortSpec)
+      case Membership2023Annuals =>
+        Membership2023.priceData(account, catalogue, subscription, invoiceList, nextServiceStartDate, cohortSpec)
+      case _ => priceDataWithRatePlanMatching(account, catalogue, subscription, invoiceList, nextServiceStartDate)
     }
   }
 

lambda/src/main/scala/pricemigrationengine/model/CohortSpec.scala

@@ -34,6 +34,31 @@ case class CohortSpec(
   def tableName(stage: String): String = s"PriceMigration-$stage-$normalisedCohortName"
 }
 
+/*
+  MigrationType.apply: CohortSpec -> MigrationType
+  was introduced to help remove the `if else if else if ... else` pattern that was showing up as we started to
+  have more migrations, notably the SupporterPlus 2023 migration after the membership annuals. Having defined a
+  sealed trait means that we can use a `match / case` layout, which makes the code more readable
+
+  MigrationType does not identity a migration (despite the fact that some migrations map to a unique migration type)
+  It simply helps identify common code used by possibly more than one migration. For instance all the pre 2023 migrations
+  map to `Legacy`
+ */
+
+sealed trait MigrationType
+object Legacy extends MigrationType // refers to all migrations before membership 2023 and supporter 2023
+object Membership2023Monthlies extends MigrationType
+object Membership2023Annuals extends MigrationType
+
+object MigrationType {
+  def apply(cohortSpec: CohortSpec): MigrationType = cohortSpec.cohortName match {
+    case "Membership2023_Batch1" => Membership2023Monthlies
+    case "Membership2023_Batch2" => Membership2023Monthlies
+    case "Membership2023_Batch3" => Membership2023Annuals
+    case _                       => Legacy
+  }
+}
+
 object CohortSpec {
 
   implicit val rw: ReadWriter[CohortSpec] = macroRW
@@ -62,16 +87,4 @@ object CohortSpec {
       earliestPriceMigrationStartDate,
       migrationCompleteDate
     )).left.map(e => CohortSpecFetchFailure(e))
-
-  def isMembershipPriceRiseBatch1(cohortSpec: CohortSpec): Boolean = cohortSpec.cohortName == "Membership2023_Batch1"
-  def isMembershipPriceRiseBatch2(cohortSpec: CohortSpec): Boolean = cohortSpec.cohortName == "Membership2023_Batch2"
-  def isMembershipPriceRiseBatch3(cohortSpec: CohortSpec): Boolean = cohortSpec.cohortName == "Membership2023_Batch3"
-
-  def isMembershipPriceRiseMonthlies(cohortSpec: CohortSpec) =
-    isMembershipPriceRiseBatch1(cohortSpec) || isMembershipPriceRiseBatch2(cohortSpec)
-
-  def isMembershipPriceRiseAnnuals(cohortSpec: CohortSpec) = isMembershipPriceRiseBatch3(cohortSpec)
-
-  def isMembershipPriceRise(cohortSpec: CohortSpec) =
-    isMembershipPriceRiseMonthlies(cohortSpec) || isMembershipPriceRiseAnnuals(cohortSpec)
 }

lambda/src/main/scala/pricemigrationengine/model/Membership2023.scala

@@ -90,24 +90,24 @@ object Membership2023 {
       }
     }
 
-    if (CohortSpec.isMembershipPriceRiseMonthlies(cohortSpec)) {
-      for {
-        ratePlan <- subscriptionRatePlan(subscription)
-        ratePlanCharge <- subscriptionRatePlanCharge(subscription, ratePlan)
-        currency = ratePlanCharge.currency
-        oldPrice <- getOldPrice(subscription, ratePlanCharge)
-        newPrice <- currencyToNewPriceMonthlies(currency: String)
-      } yield PriceData(currency, oldPrice, newPrice, "Month")
-    } else if (CohortSpec.isMembershipPriceRiseAnnuals(cohortSpec)) {
-      for {
-        ratePlan <- subscriptionRatePlan(subscription)
-        ratePlanCharge <- subscriptionRatePlanCharge(subscription, ratePlan)
-        currency = ratePlanCharge.currency
-        oldPrice <- getOldPrice(subscription, ratePlanCharge)
-        newPrice <- currencyToNewPriceAnnuals(currency: String)
-      } yield PriceData(currency, oldPrice, newPrice, "Annual")
-    } else {
-      Left(AmendmentDataFailure(s"(error: 7ba45f10) Incorrect cohort spec for this function: ${cohortSpec}"))
+    MigrationType(cohortSpec) match {
+      case Membership2023Monthlies =>
+        for {
+          ratePlan <- subscriptionRatePlan(subscription)
+          ratePlanCharge <- subscriptionRatePlanCharge(subscription, ratePlan)
+          currency = ratePlanCharge.currency
+          oldPrice <- getOldPrice(subscription, ratePlanCharge)
+          newPrice <- currencyToNewPriceMonthlies(currency: String)
+        } yield PriceData(currency, oldPrice, newPrice, "Month")
+      case Membership2023Annuals =>
+        for {
+          ratePlan <- subscriptionRatePlan(subscription)
+          ratePlanCharge <- subscriptionRatePlanCharge(subscription, ratePlan)
+          currency = ratePlanCharge.currency
+          oldPrice <- getOldPrice(subscription, ratePlanCharge)
+          newPrice <- currencyToNewPriceAnnuals(currency: String)
+        } yield PriceData(currency, oldPrice, newPrice, "Annual")
+      case _ => Left(AmendmentDataFailure(s"(error: 7ba45f10) Incorrect cohort spec for this function: ${cohortSpec}"))
     }
   }
 }

lambda/src/test/scala/pricemigrationengine/handlers/NotificationHandlerTest.scala

@@ -1,8 +1,8 @@
 package pricemigrationengine.handlers
 
 import pricemigrationengine.handlers.NotificationHandler.thereIsEnoughNotificationLeadTime
-import pricemigrationengine.handlers.NotificationHandler.guLettersNotificationLeadTime
-import pricemigrationengine.{TestLogging}
+import pricemigrationengine.handlers.NotificationHandler.letterMaxNotificationLeadTime
+import pricemigrationengine.TestLogging
 import pricemigrationengine.model.CohortTableFilter._
 import pricemigrationengine.model._
 import pricemigrationengine.model.membershipworkflow.EmailMessage
@@ -199,7 +199,7 @@ class NotificationHandlerTest extends munit.FunSuite {
   test("guLettersNotificationLeadTime should be at least 49 days") {
     // "Should be at least 49 days", but for invariance we test against the
     // usual value of exactly 49
-    assert(guLettersNotificationLeadTime == 49)
+    assert(letterMaxNotificationLeadTime == 49)
   }
 
   test("NotificationHandler should get records from cohort table and SF and send Email with the data") {