diff --git a/lambda/src/main/scala/pricemigrationengine/handlers/AmendmentHandler.scala b/lambda/src/main/scala/pricemigrationengine/handlers/AmendmentHandler.scala index 9f7c0a35..20a5bedd 100644 --- a/lambda/src/main/scala/pricemigrationengine/handlers/AmendmentHandler.scala +++ b/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 + ) } } } diff --git a/lambda/src/main/scala/pricemigrationengine/handlers/EstimationHandler.scala b/lambda/src/main/scala/pricemigrationengine/handlers/EstimationHandler.scala index 05b752bc..d7ab8ca2 100644 --- a/lambda/src/main/scala/pricemigrationengine/handlers/EstimationHandler.scala +++ b/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( diff --git a/lambda/src/main/scala/pricemigrationengine/handlers/NotificationHandler.scala b/lambda/src/main/scala/pricemigrationengine/handlers/NotificationHandler.scala index 22e05849..3e5efdf9 100644 --- a/lambda/src/main/scala/pricemigrationengine/handlers/NotificationHandler.scala +++ b/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) diff --git a/lambda/src/main/scala/pricemigrationengine/handlers/SalesforcePriceRiseCreationHandler.scala b/lambda/src/main/scala/pricemigrationengine/handlers/SalesforcePriceRiseCreationHandler.scala index 75eb62b5..7b915e4c 100644 --- a/lambda/src/main/scala/pricemigrationengine/handlers/SalesforcePriceRiseCreationHandler.scala +++ b/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] = diff --git a/lambda/src/main/scala/pricemigrationengine/model/AmendmentData.scala b/lambda/src/main/scala/pricemigrationengine/model/AmendmentData.scala index 807b4ecd..7c8107e5 100644 --- a/lambda/src/main/scala/pricemigrationengine/model/AmendmentData.scala +++ b/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) } } diff --git a/lambda/src/main/scala/pricemigrationengine/model/CohortSpec.scala b/lambda/src/main/scala/pricemigrationengine/model/CohortSpec.scala index 9aa88fc9..ee7ddbfd 100644 --- a/lambda/src/main/scala/pricemigrationengine/model/CohortSpec.scala +++ b/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) } diff --git a/lambda/src/main/scala/pricemigrationengine/model/Membership2023.scala b/lambda/src/main/scala/pricemigrationengine/model/Membership2023.scala index b2bc2ce5..14ce1685 100644 --- a/lambda/src/main/scala/pricemigrationengine/model/Membership2023.scala +++ b/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}")) } } } diff --git a/lambda/src/test/scala/pricemigrationengine/handlers/NotificationHandlerTest.scala b/lambda/src/test/scala/pricemigrationengine/handlers/NotificationHandlerTest.scala index 2f159d1b..ba2ea4fd 100644 --- a/lambda/src/test/scala/pricemigrationengine/handlers/NotificationHandlerTest.scala +++ b/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") {