Skip to content

Commit

Permalink
Merge pull request #976 from guardian/ph-20240205-1955-amendments-can…
Browse files Browse the repository at this point in the history
…cellations

Introduce RateplansProbe and pre Notification subscription check
  • Loading branch information
shtukas authored Feb 14, 2024
2 parents d71fcc3 + a1cb0ee commit fdfc3d6
Show file tree
Hide file tree
Showing 15 changed files with 7,966 additions and 217 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,6 @@ object AmendmentHandler extends CohortHandler {
val result = ExpiringSubscriptionResult(item.subscriptionName)
CohortTable.update(CohortItem.fromExpiringSubscriptionResult(result)).as(result)
}
case _: IncompatibleAmendmentHistoryFailure => {
// 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 =>
Expand Down Expand Up @@ -107,38 +97,6 @@ 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(
IncompatibleAmendmentHistoryFailure(
s"[4f7589ea] 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,
Expand All @@ -148,13 +106,6 @@ object AmendmentHandler extends CohortHandler {
for {
subscriptionBeforeUpdate <- fetchSubscription(item)

_ <-
if (subscriptionBeforeUpdate.version > 1) {
checkMigrationRelevanceBasedOnLastAmendment(item).debug("check relevance")
} else {
ZIO.succeed(())
}

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"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import pricemigrationengine.services._
import zio.{Clock, ZIO}
import com.gu.i18n
import pricemigrationengine.migrations.newspaper2024Migration
import pricemigrationengine.model.RateplansProbe

import java.time.LocalDate
import java.time.{LocalDate, ZoneId}
import java.time.format.DateTimeFormatter

object NotificationHandler extends CohortHandler {
Expand All @@ -22,18 +23,20 @@ object NotificationHandler extends CohortHandler {
EnvConfig.salesforce.layer,
EnvConfig.cohortTable.layer,
EnvConfig.emailSender.layer,
EnvConfig.zuora.layer,
EnvConfig.stage.layer,
DynamoDBClientLive.impl,
DynamoDBZIOLive.impl,
CohortTableLive.impl(input),
SalesforceClientLive.impl,
EmailSenderLive.impl
EmailSenderLive.impl,
ZuoraLive.impl
)
}

def main(
cohortSpec: CohortSpec
): ZIO[Logging with CohortTable with SalesforceClient with EmailSender, Failure, HandlerOutput] = {
): ZIO[Logging with CohortTable with SalesforceClient with EmailSender with Zuora, Failure, HandlerOutput] = {
for {
today <- Clock.currentDateTime.map(_.toLocalDate)
count <- CohortTable
Expand All @@ -47,14 +50,15 @@ object NotificationHandler extends CohortHandler {
def sendNotification(cohortSpec: CohortSpec)(
cohortItem: CohortItem,
today: LocalDate
): ZIO[EmailSender with SalesforceClient with CohortTable with Logging, Failure, Int] = {
): ZIO[EmailSender with SalesforceClient with CohortTable with Logging with Zuora, Failure, Int] = {

// We are starting with a simple check. That the item's startDate is at least minLeadTime(cohortSpec) days away
// from the current day. This will avoid headaches caused by letters not being sent early enough relatively to
// previously computed start dates, which can happen if, for argument sake, the engine is down for a few days.

if (thereIsEnoughNotificationLeadTime(cohortSpec, today, cohortItem)) {
val result = for {
_ <- cohortItemRatePlansChecks(cohortItem)
sfSubscription <-
SalesforceClient
.getSubscriptionByName(cohortItem.subscriptionName)
Expand Down Expand Up @@ -151,6 +155,48 @@ object NotificationHandler extends CohortHandler {
_ <- updateCohortItemStatus(cohortItem.subscriptionName, NotificationSendComplete)
} yield Successful

// -------------------------------------------------------------------
// Subscription Rate Plan Checks

private def fetchSubscription(item: CohortItem): ZIO[Zuora, Failure, ZuoraSubscription] =
Zuora
.fetchSubscription(item.subscriptionName)
.filterOrFail(_.status != "Cancelled")(CancelledSubscriptionFailure(item.subscriptionName))

private def subscriptionRatePlansCheck(
item: CohortItem,
subscription: ZuoraSubscription,
date: LocalDate
): ZIO[CohortTable with Zuora, Failure, Unit] = {
RateplansProbe.probe(subscription: ZuoraSubscription, date) match {
case ShouldProceed => ZIO.succeed(())
case ShouldCancel => {
val result = CancelledAmendmentResult(item.subscriptionName)
ZIO.succeed(CohortTable.update(CohortItem.fromCancelledAmendmentResult(result)).as(result))
}
case IndeterminateConclusion =>
ZIO.fail(
RatePlansProbeFailure(
s"[4f7589ea] NotificationHandler probeRatePlans could not determine a probe outcome for cohort item ${item}. Please investigate."
)
)
}
}

private def cohortItemRatePlansChecks(item: CohortItem): ZIO[CohortTable with Zuora, Failure, Unit] = {
for {
subscription <- fetchSubscription(item: CohortItem)
estimationInstant <- ZIO
.fromOption(item.whenEstimationDone)
.mapError(ex => AmendmentDataFailure(s"[3026515c] Could not extract whenEstimationDone from item ${item}"))
result <- subscriptionRatePlansCheck(
item,
subscription,
estimationInstant.atZone(ZoneId.of("Europe/London")).toLocalDate
)
} yield result
}

// -------------------------------------------------------------------
// Notification Windows

Expand Down Expand Up @@ -186,7 +232,7 @@ object NotificationHandler extends CohortHandler {
}

def thereIsEnoughNotificationLeadTime(cohortSpec: CohortSpec, today: LocalDate, cohortItem: CohortItem): Boolean = {
// To help with backward compatibility with existing tests, we apply this condition from 1st Dec 2022.
// To help with backward compatibility with existing tests, we apply this condition from 1st Dec 2020.
if (today.isBefore(LocalDate.of(2020, 12, 1))) {
true
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +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 IncompatibleAmendmentHistoryFailure(reason: String) extends Failure
case class RatePlansProbeFailure(reason: String) extends Failure

case class SalesforcePriceRiseWriteFailure(reason: String) extends Failure
case class SalesforceClientFailure(reason: String) extends Failure
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package pricemigrationengine.model

import java.time.LocalDate

sealed trait RateplansProbeResult
object ShouldProceed extends RateplansProbeResult
object ShouldCancel extends RateplansProbeResult
object IndeterminateConclusion extends RateplansProbeResult

object RateplansProbe {

/*
Date: 07th February 2024
This object implements the logic that decides whether a subscription
that is just about to be notified should indeed be notified and amended and that no
migration cancelling events have occurred on the subscription since it was estimated.
This check is necessary because migration cohort items can sleep for a long time (months) and
the engine is not kept up to date with any possible changes that could have occurred on the subscription
In effect we are replacing the work that was done here:
"Introduce last amendment check": https://github.com/guardian/price-migration-engine/pull/925
We are also going to slightly change the moment this check is performed. Since subscriptions
sleep before the notification step (and not before the amendment step), we should perform that check before the
Notification step is performed
The main logic used in RateplansProbe is that since we cannot have programmatic (API) access
to the list of all amendments of a subscription, then we are going to work off the subscription
itself by probing its rate plans.
( Zuora offers the retrieval of the last one, but not the entire list;
see: https://www.zuora.com/developer/api-references/older-api/operation/GET_AmendmentsBySubscriptionID/ )
During testing we discovered that in fact all the information we need is carried by the subscription
itself and that a relatively simple lookup of the rate plans should enable us to make the
same determination we would have been able to make if we could analyse the set of amendments.
Last but not least, by design, the current implementation is not perfect, in the sense that
if the code sees a subscription that contains a rate plan that is not one of the type it know
how to deal with, then the probe function will return IndeterminateConclusion causing the
engine to immediately fail. The engine operators should update the code to handle the missing case.
We did a separate extensive testing on a large sample of subscriptions to ensure we would be automatically
covering most cases.
*/

// All the functions of this object are pure and strive to work without side effects

def ratePlanDate(ratePlan: ZuoraRatePlan): Option[LocalDate] = {
// This function takes a rate plan and tries and determine its creation date
// The logic came from the work done here: https://github.com/guardian/price-migration-engine/pull/974
ratePlan.ratePlanCharges.headOption.flatMap(ratePlanCharge => ratePlanCharge.originalOrderDate)
}

def addedRatePlansAfterDate(subscription: ZuoraSubscription, date: LocalDate): List[ZuoraRatePlan] = {
// This function takes a subscription and return the list of rate plans added to the subscription
// after a given date. (In normal circumstances that date is intended to be the date the subscription
// has been estimated.)
subscription.ratePlans
.filter(ratePlan => ratePlan.lastChangeType.fold(false)(_ == "Add"))
.filter(ratePlan => ratePlanDate(ratePlan: ZuoraRatePlan).fold(false)(_.isAfter(date)))
}

def selectNonTrivialRatePlans(ratePlans: List[ZuoraRatePlan]): List[ZuoraRatePlan] = {
// This function should be used in conjunction with addedRatePlansAfterDate
// The intent is to remove from a list of rate plans the rate plans that we know
// are harmless and do not change the readiness of a subscription for price migration
// We refer to those harmless rate plans as "trivial"

// AT the moment, we are filtering away "Discounts"
ratePlans.filter(ratePlan => ratePlan.productName != "Discounts")
}

def probe(subscription: ZuoraSubscription, date: LocalDate): RateplansProbeResult = {
// This function takes a subscription (in intended circumstances that would be a subscription that has slept for
// a while and is about to be notified+amended) and decides whether or not to pursue. The return value is not a Boolean
// but a RateplansProbeResult. The interesting case is IndeterminateConclusion which will cause the engine to fail
// This is on purpose and by design. Upon such a failure the engine operators should look up the subscription,
// identify the type of rate plan that filtered away by selectNonTrivialRatePlans and decide the correct
// course of action.

if (subscription.status == "Cancelled") {
ShouldCancel
} else {
if (selectNonTrivialRatePlans(addedRatePlansAfterDate(subscription, date)).isEmpty) {
ShouldProceed
} else {
IndeterminateConclusion
}
}
}
}

This file was deleted.

13 changes: 0 additions & 13 deletions lambda/src/main/scala/pricemigrationengine/services/Zuora.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ trait Zuora {
): ZIO[Any, ZuoraUpdateFailure, ZuoraSubscriptionId]

def renewSubscription(subscriptionNumber: String): ZIO[Any, ZuoraRenewalFailure, Unit]

def fetchLastSubscriptionAmendment(
subscriptionId: ZuoraSubscriptionId
): ZIO[Any, ZuoraFetchFailure, ZuoraSubscriptionAmendment]
}

object Zuora {
Expand All @@ -49,13 +45,4 @@ 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))
}
Loading

0 comments on commit fdfc3d6

Please sign in to comment.