Merge pull request #925 from guardian/ph-20230911-2344-ZuoraSubscript…

…ionAmendment

Introduce last amendment check

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

@@ -5,7 +5,7 @@ import pricemigrationengine.model._
 import pricemigrationengine.services._
 import zio.{Clock, ZIO}
 
-import java.time.LocalDate
+import java.time.{LocalDate, LocalDateTime, ZoneOffset}
 
 /** Carries out price-rise amendments in Zuora.
   */
@@ -46,6 +46,16 @@ object AmendmentHandler extends CohortHandler {
           val result = ExpiringSubscriptionResult(item.subscriptionName)
           CohortTable.update(CohortItem.fromExpiringSubscriptionResult(result)).as(result)
         }
+        case _: IncompatibleAmendmentHistory => {
+          // See the preambule of the ZuoraSubscriptionAmendment case class for context about
+          // IncompatibleAmendmentHistory
+          // Note: At the moment we only have one CancelledAmendmentResult, it would be great one day
+          //       To indicate several cancellation conditions, for instance discriminate between
+          //       cancellations because the zuora subscription has been cancelled and cancellations
+          //       because of incompatible amendment history.s
+          val result = CancelledAmendmentResult(item.subscriptionName)
+          CohortTable.update(CohortItem.fromCancelledAmendmentResult(result)).as(result)
+        }
         case e => ZIO.fail(e)
       },
       success = { result =>
@@ -107,13 +117,52 @@ object AmendmentHandler extends CohortHandler {
     }
   }
 
+  def amendmentIsBeforeInstant(amendment: ZuoraSubscriptionAmendment, instant: java.time.Instant): Boolean = {
+    val amendmentDate = LocalDate.parse(amendment.bookingDate)
+    val estimationDate = LocalDateTime.ofInstant(instant, ZoneOffset.UTC).toLocalDate
+    amendmentDate.isBefore(estimationDate)
+  }
+
+  private def checkMigrationRelevanceBasedOnLastAmendment(item: CohortItem): ZIO[Zuora, Failure, Unit] = {
+    // See the preambule of the ZuoraSubscriptionAmendment case class for context
+
+    for {
+      amendment <- Zuora.fetchLastSubscriptionAmendment(item.subscriptionName) // $1
+      estimationInstant <- ZIO
+        .fromOption(item.whenEstimationDone)
+        .mapError(ex => AmendmentDataFailure(s"[3026515c] Could not extract whenEstimationDone from item ${item}"))
+      result <-
+        if (amendmentIsBeforeInstant(amendment, estimationInstant)) {
+          ZIO.succeed(())
+        } else {
+          // ZIO.fail(
+          //  IncompatibleAmendmentHistory(
+          //    s"[4f7589ea] Cohort item ${item} is being written for cancellation, during scheduled amendment, due to last amendment check failing"
+          //  )
+          ZIO.fail(
+            AmendmentDataFailure(
+              s"[77c13996] Cohort item ${item} is being written for cancellation, during scheduled amendment, due to last amendment check failing; amendment: ${amendment}"
+            )
+          )
+        }
+    } yield result
+
+    // $1: The Zuora documentation
+    // https://www.zuora.com/developer/api-references/older-api/operation/GET_AmendmentsBySubscriptionID/
+    // specifies that a subscriptionId is to be provided, but it also works with a subscription number
+    // (aka subscription name for a cohort item).
+  }
+
   private def doAmendment(
       cohortSpec: CohortSpec,
       catalogue: ZuoraProductCatalogue,
       item: CohortItem
   ): ZIO[Zuora, Failure, SuccessfulAmendmentResult] = {
 
     for {
+
+      _ <- checkMigrationRelevanceBasedOnLastAmendment(item).debug("check relevance")
+
       startDate <- ZIO.fromOption(item.startDate).orElseFail(AmendmentDataFailure(s"No start date in $item"))
 
       oldPrice <- ZIO.fromOption(item.oldPrice).orElseFail(AmendmentDataFailure(s"No old price in $item"))

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

@@ -27,6 +27,7 @@ case class ZuoraRenewalFailure(reason: String) extends Failure
 case class AmendmentDataFailure(reason: String) extends Failure
 case class CancelledSubscriptionFailure(reason: String) extends Failure
 case class ExpiringSubscriptionFailure(reason: String) extends Failure
+case class IncompatibleAmendmentHistory(reason: String) extends Failure
 
 case class SalesforcePriceRiseWriteFailure(reason: String) extends Failure
 case class SalesforceClientFailure(reason: String) extends Failure

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

@@ -0,0 +1,85 @@
+package pricemigrationengine.model
+
+import java.time.LocalDate
+
+import pricemigrationengine.model.OptionReader // not sure why this import is needed as should be visible implicitly
+import upickle.default._
+
+/*
+{
+  "success" : true,
+  "id" : "8a129e8f8abcd26f018acc9501e37fbc",
+  "code" : "A-AM08077754",
+  "name" : "remove",
+  "type" : "RemoveProduct",
+  "description" : null,
+  "status" : "Completed",
+  "contractEffectiveDate" : "2023-10-28",
+  "serviceActivationDate" : "2023-10-28",
+  "customerAcceptanceDate" : "2023-10-28",
+  "effectiveDate" : "2023-10-28",
+  "suspendDate" : null,
+  "resumeDate" : null,
+  "newSubscriptionId" : "8a129e8f8abcd26f018acc9502127fc1",
+  "baseSubscriptionId" : "8a129e8f8abcd26f018acc9500137f9b",
+  "bookingDate" : "2023-09-25",
+  "termType" : null,
+  "currentTerm" : null,
+  "currentTermPeriodType" : null,
+  "termStartDate" : null,
+  "renewalSetting" : null,
+  "renewalTerm" : null,
+  "renewalTermPeriodType" : null,
+  "autoRenew" : null,
+  "specificUpdateDate" : null,
+  "newRatePlanId" : "8a129e8f8abcd26f018acc9501cc7fb9",
+  "baseRatePlanId" : "8a129e8f8abcd26f018acc9500247fa2",
+  "destinationAccountId" : null,
+  "destinationInvoiceOwnerId" : null,
+  "subType" : null,
+  "effectivePolicy" : null,
+  "createdByOrder" : false
+}
+ */
+
+/*
+  Date: 17th Sept 2023
+
+  Class ZuoraSubscriptionAmendment was originally introduced to detect whether or not there has been an
+  amendment on a subscription between the estimation step and the amendment step.
+
+  The check was introduced to prevent a problem by which, at least for the digital products we have scheduled
+  for price rise in 2023, (where people would be notified by email, and where the  subscriptions can be managed online),
+  there was also possibility for user to perform user-triggered subscription mutations online (for instance product
+  changes), which would not be known to the engine and which would then be lost and/or overridden by the engine at
+  amendment step. Pascal then decided that the engine would not proceed with any price rise if there were amendments
+  on the subscription between the estimation step and the amendment step.
+
+  This logic solves the immediate problem, but such a simple check is too crude and too inefficient, as it will prevent
+  some subscriptions from being migrated if they undergo mutations (amendments) that have nothing to do with user
+  journeys. In an ideal world we would retrieve the entire collection of amendments, analyse the recent ones and
+  make a determination on whether or not a price rise can happen.
+
+  Unfortunately Zuora, as far as we know, only offers us the retrieval of the last amendment. This is enough for the
+  check since we can then compare the date of estimation with the effective date of the amendment (which seems to
+  coincide with the date the amendment was created). But, this is clearly not enough for the type of  thorough evaluation
+  that would lead to a better check (without false positives).
+
+  1. At the moment we only need the bookingDate. This is the reason why this first version of the class only carries
+     that one attribute.
+
+  2. It will be great if in the future the following is done
+     - 2.1 Find a way to retrieve more amendments, ideally the entire sequence of amendments of a subscription.
+     - 2.2 Implement an analysis of that sequence to provide a better answer to the question: Should the engine perform
+         the scheduled price rise on the corresponding subscription.
+
+  Together with ZuoraSubscriptionAmendment we introduce
+    - case class IncompatibleAmendmentHistory, and
+    - checkMigrationRelevanceBasedOnLastAmendment in the amendment handler
+ */
+
+case class ZuoraSubscriptionAmendment(bookingDate: String)
+
+object ZuoraSubscriptionAmendment {
+  implicit val rwSubscription: ReadWriter[ZuoraSubscriptionAmendment] = macroRW
+}

lambda/src/main/scala/pricemigrationengine/services/Zuora.scala

@@ -21,6 +21,10 @@ trait Zuora {
   ): ZIO[Any, ZuoraUpdateFailure, ZuoraSubscriptionId]
 
   def renewSubscription(subscriptionNumber: String): ZIO[Any, ZuoraRenewalFailure, Unit]
+
+  def fetchLastSubscriptionAmendment(
+      subscriptionId: ZuoraSubscriptionId
+  ): ZIO[Any, ZuoraFetchFailure, ZuoraSubscriptionAmendment]
 }
 
 object Zuora {
@@ -45,4 +49,13 @@ object Zuora {
 
   def renewSubscription(subscriptionNumber: String): ZIO[Zuora, ZuoraRenewalFailure, Unit] =
     ZIO.environmentWithZIO(_.get.renewSubscription(subscriptionNumber))
+
+  // Note: the Zuora documentation
+  // https://www.zuora.com/developer/api-references/older-api/operation/GET_AmendmentsBySubscriptionID/
+  // specifies that a subscriptionId is to be provided, but it also works with a subscription number
+  // (aka subscription name for a cohort item).
+  def fetchLastSubscriptionAmendment(
+      subscriptionId: ZuoraSubscriptionId
+  ): ZIO[Zuora, Failure, ZuoraSubscriptionAmendment] =
+    ZIO.environmentWithZIO(_.get.fetchLastSubscriptionAmendment(subscriptionId))
 }

lambda/src/main/scala/pricemigrationengine/services/ZuoraLive.scala

@@ -219,6 +219,18 @@ object ZuoraLive {
             )
           ) <* logging.info(s"renewed subscription ${subscriptionNumber}")
 
+        override def fetchLastSubscriptionAmendment(
+            subscriptionId: ZuoraSubscriptionId
+        ): ZIO[Any, ZuoraFetchFailure, ZuoraSubscriptionAmendment] =
+          get[ZuoraSubscriptionAmendment](s"amendments/subscriptions/$subscriptionId")
+            .mapError(e =>
+              ZuoraFetchFailure(s"ZuoraSubscriptionAmendment, subscriptionId: $subscriptionId: ${e.reason}")
+            )
+            .tapBoth(
+              e => logging.error(s"Failed to fetch ZuoraSubscriptionAmendment, subscriptionId: $subscriptionId: $e"),
+              _ => logging.info(s"Fetched ZuoraSubscriptionAmendment, subscriptionId: $subscriptionId")
+            )
+
       }
     )
 }

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

@@ -2,8 +2,9 @@ package pricemigrationengine.handlers
 
 import pricemigrationengine.model.{ZuoraRatePlanCharge, ZuoraSubscriptionUpdate, _}
 
-import java.time.LocalDate
+import java.time.{LocalDate, ZoneOffset}
 import pricemigrationengine.Fixtures
+import pricemigrationengine.handlers.AmendmentHandler.amendmentIsBeforeInstant
 import pricemigrationengine.model.CohortTableFilter.NotificationSendDateWrittenToSalesforce
 
 class AmendmentHandlerTest extends munit.FunSuite {
@@ -823,4 +824,35 @@ class AmendmentHandlerTest extends munit.FunSuite {
       )
     )
   }
+
+  test("compare amendments and instants") {
+    val amendment = ZuoraSubscriptionAmendment("2023-09-17")
+
+    val date = LocalDate.parse(amendment.bookingDate)
+    // Checking that LocalDate.parse works as intended on a "YYYY-MM-DD"
+    assertEquals(date, LocalDate.of(2023, 9, 17))
+
+    val estimationInstant1 =
+      LocalDate.parse("2023-09-16").atStartOfDay(ZoneOffset.UTC).toInstant() // the day before
+    val estimationInstant2 =
+      LocalDate.parse("2023-09-17").atStartOfDay(ZoneOffset.UTC).toInstant() // the same day
+    val estimationInstant3 =
+      LocalDate.parse("2023-09-18").atStartOfDay(ZoneOffset.UTC).toInstant() // the day after
+
+    assertEquals(
+      amendmentIsBeforeInstant(amendment, estimationInstant1),
+      false
+    ) // false because amendment is on the 17th and estimation on the 16th
+
+    assertEquals(
+      amendmentIsBeforeInstant(amendment, estimationInstant2),
+      false
+    ) // false because isBefore is interpreted in the strict sense, so returns false on equality
+
+    assertEquals(
+      amendmentIsBeforeInstant(amendment, estimationInstant3),
+      true
+    ) // true because amendment is on the 17th and estimation on the 18th
+
+  }
 }

lambda/src/test/scala/pricemigrationengine/service/MockZuora.scala

@@ -18,6 +18,8 @@ object MockZuora extends Mock[Zuora] {
 
   object RenewSubscription extends Effect[String, ZuoraRenewalFailure, Unit]
 
+  object FetchLastSubscriptionAmendment extends Effect[String, ZuoraFetchFailure, ZuoraSubscriptionAmendment]
+
   val compose: URLayer[Proxy, Zuora] = ZLayer.fromZIO(ZIO.service[Proxy].map { proxy =>
     new Zuora {
 
@@ -40,6 +42,10 @@ object MockZuora extends Mock[Zuora] {
 
       override def renewSubscription(subscriptionNumber: String): ZIO[Any, ZuoraRenewalFailure, Unit] =
         proxy(RenewSubscription, subscriptionNumber)
+
+      override def fetchLastSubscriptionAmendment(
+          subscriptionId: ZuoraSubscriptionId
+      ): ZIO[Any, ZuoraFetchFailure, ZuoraSubscriptionAmendment] = proxy(FetchLastSubscriptionAmendment, subscriptionId)
     }
   })
 }