Skip to content

Commit

Permalink
update EmailPayloadSubscriberAttributes with attributes for Supporter…
Browse files Browse the repository at this point in the history
…Plus2024
  • Loading branch information
shtukas committed Sep 3, 2024
1 parent 14684fc commit aaf823e
Show file tree
Hide file tree
Showing 10 changed files with 610 additions and 32 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ The price migration engine is an orchestration engine used to perform controlled

- [An introduction to the general principles of price migrations](docs/price-migrations-from-first-principles.md)
- [The journey of a cohort item](docs/the-journey-of-a-cohort-item.md)
- [Coding conventions](docs/coding-conventions.md)
- [Coding directives](docs/coding-directives.md)
- [Notification windows](docs/notification-windows.md)
- [The art of computing start dates](docs/start-date-computation.md)
- [The art of the cap; or how to gracefully cap prices in the engine](docs/the-art-of-the-cap.md)
Expand Down
8 changes: 0 additions & 8 deletions docs/coding-conventions.md

This file was deleted.

8 changes: 8 additions & 0 deletions docs/coding-directives.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# Coding Conventions

The price migration engine doesn't have coding conventions per se. ZIO does a very good job at keeping sanity between pure and impure code, and putting adhoc code into migration specific objects (the so called "modern" migrations) helps separate the general engine logic from specific requests. We also rely on the coding expertise of contributors to do the right thing (including breaking rules when needed).

With that said, we have the following conventions

- Coding Directive #1: When using `MigrationType(cohortSpec)` to dispatch values or behaviour per migration, and unless exceptions (there are a couple in the code for when we handle exceptions or for exceptional circumstances), we will be explicit on what we want and declaring all the cases. If somebody is implementing a new migration and follows the steps Pascal presented during GW2024, then declaring a new case will happen during the [first step](https://github.com/guardian/price-migration-engine/pull/1012). The reason for this rule is that an inexperienced contributor could easily miss a place in the code where some thought should have been given on what a new migration should specify. If the code compiles without prompting that decision the contributor might miss it. And even if the decision is to go with the "default", this needs to be explicitly specified. This convention was introduced in this pull request [pull:1022](https://github.com/guardian/price-migration-engine/pull/1022).

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import pricemigrationengine.migrations.{
GW2024Migration,
Membership2023Migration,
newspaper2024Migration,
SupporterPlus2024Migration
SupporterPlus2024Migration,
}
import pricemigrationengine.model.RateplansProbe

Expand Down Expand Up @@ -91,7 +91,7 @@ object NotificationHandler extends CohortHandler {
cohortSpec: CohortSpec,
cohortItem: CohortItem,
sfSubscription: SalesforceSubscription
): ZIO[EmailSender with SalesforceClient with CohortTable with Logging, Failure, Unit] =
): ZIO[Zuora with EmailSender with SalesforceClient with CohortTable with Logging, Failure, Unit] =
for {
_ <- Logging.info(s"Processing subscription: ${cohortItem.subscriptionName}")
contact <- SalesforceClient.getContact(sfSubscription.Buyer__c)
Expand Down Expand Up @@ -123,6 +123,12 @@ object NotificationHandler extends CohortHandler {

_ <- logMissingEmailAddress(cohortItem, contact)

// Data for SupporterPlus2024
subscription <- Zuora.fetchSubscription(cohortItem.subscriptionName)
sp2024_contribution_amount <- ZIO.fromEither(sp2024_contribution_amount(cohortSpec, subscription))
sp2024_previous_combined_amount <- ZIO.fromEither(sp2024_previous_combined_amount(cohortSpec, subscription))
sp2024_new_combined_amount <- ZIO.fromEither(sp2024_new_combined_amount(cohortSpec, subscription))

_ <- EmailSender.sendEmail(
message = EmailMessage(
EmailPayload(
Expand All @@ -144,7 +150,12 @@ object NotificationHandler extends CohortHandler {
next_payment_date = startDateConversion(startDate),
payment_frequency = paymentFrequency,
subscription_id = cohortItem.subscriptionName,
product_type = sfSubscription.Product_Type__c.getOrElse("")
product_type = sfSubscription.Product_Type__c.getOrElse(""),

// SupporterPlus 2024 extension
sp2024_contribution_amount = sp2024_contribution_amount,
sp2024_previous_combined_amount = sp2024_previous_combined_amount,
sp2024_new_combined_amount = sp2024_new_combined_amount,
)
)
),
Expand Down Expand Up @@ -434,4 +445,57 @@ object NotificationHandler extends CohortHandler {
)
} yield ()
}

// -------------------------------------------------------------------
// Supporter Plus 2024 extra functions

/*
Date: September 2024
Author: Pascal
Comment Group: 602514a6-5e53
These functions have been added to implement the extra fields added to EmailPayloadSubscriberAttributes
as part of the set up of the SupporterPlus 2024 migration (see Comment Group: 602514a6-5e53)
*/

def sp2024_contribution_amount(
cohortSpec: CohortSpec,
subscription: ZuoraSubscription
): Either[Failure, Option[String]] = {
// nb: Going against Coding Directive #1, we do not need to over specify the match, because we are expecting
// a non trivial behavior only for SupporterPlus2024
MigrationType(cohortSpec) match {
case SupporterPlus2024 =>
SupporterPlus2024Migration.sp2024_contribution_amount(subscription).map(o => o.map(b => b.toString))
case _ => Right(None)
}
}

def sp2024_previous_combined_amount(
cohortSpec: CohortSpec,
subscription: ZuoraSubscription
): Either[Failure, Option[String]] = {
// nb: Going against Coding Directive #1, we do not need to over specify the match, because we are expecting
// a non trivial behavior only for SupporterPlus2024
MigrationType(cohortSpec) match {
case SupporterPlus2024 =>
SupporterPlus2024Migration
.sp2024_previous_combined_amount(subscription)
.map(o => o.map(b => b.toString))
case _ => Right(None)
}
}

def sp2024_new_combined_amount(
cohortSpec: CohortSpec,
subscription: ZuoraSubscription
): Either[Failure, Option[String]] = {
// nb: Going against Coding Directive #1, we do not need to over specify the match, because we are expecting
// a non trivial behavior only for SupporterPlus2024
MigrationType(cohortSpec) match {
case SupporterPlus2024 =>
SupporterPlus2024Migration.sp2024_new_combined_amount(subscription).map(o => o.map(b => b.toString))
case _ => Right(None)
}
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package pricemigrationengine.migrations
import pricemigrationengine.model.PriceCap
import pricemigrationengine.model.ZuoraRatePlan
import pricemigrationengine.model._
import pricemigrationengine.util._

import java.time.LocalDate

Expand Down Expand Up @@ -55,6 +53,7 @@ object SupporterPlus2024Migration {
// Data Functions
// ------------------------------------------------

// ------------------------------------------------
// Prices

def getOldPrice(billingPeriod: BillingPeriod, currency: String): Option[Double] = {
Expand All @@ -73,6 +72,7 @@ object SupporterPlus2024Migration {
}
}

// ------------------------------------------------
// Cancellation Saves

def cancellationSaveRatePlan(subscription: ZuoraSubscription): Option[ZuoraRatePlan] = {
Expand All @@ -91,6 +91,7 @@ object SupporterPlus2024Migration {
} yield date
}

// ------------------------------------------------
// Subscription Data

def supporterPlusV2RatePlan(subscription: ZuoraSubscription): Either[AmendmentDataFailure, ZuoraRatePlan] = {
Expand All @@ -114,13 +115,101 @@ object SupporterPlus2024Migration {
ratePlan.ratePlanCharges.find(rpc => rpc.name.contains("Supporter Plus")) match {
case None => {
Left(
AmendmentDataFailure(s"Subscription ${subscriptionNumber} has a rate plan, but with no charge")
AmendmentDataFailure(s"Subscription ${subscriptionNumber} has a rate plan (${ratePlan}), but with no charge")
)
}
case Some(ratePlanCharge) => Right(ratePlanCharge)
}
}

def supporterPlusContributionRatePlanCharge(
subscriptionNumber: String,
ratePlan: ZuoraRatePlan
): Either[AmendmentDataFailure, ZuoraRatePlanCharge] = {
ratePlan.ratePlanCharges.find(rpc => rpc.name.contains("Contribution")) match {
case None => {
Left(
AmendmentDataFailure(s"Subscription ${subscriptionNumber} has a rate plan (${ratePlan}), but with no charge")
)
}
case Some(ratePlanCharge) => Right(ratePlanCharge)
}
}

// ------------------------------------------------
// Notification helpers

/*
Date: September 2024
Author: Pascal
Comment Group: 602514a6-5e53
These functions have been added to implement the extra fields added to EmailPayloadSubscriberAttributes
as part of the set up of the SupporterPlus 2024 migration (see Comment Group: 602514a6-5e53)
*/

def sp2024_previous_base_amount(subscription: ZuoraSubscription): Either[Failure, Option[BigDecimal]] = {
for {
ratePlan <- supporterPlusV2RatePlan(subscription)
ratePlanCharge <- supporterPlusBaseRatePlanCharge(subscription.subscriptionNumber, ratePlan)
} yield ratePlanCharge.price
}

def sp2024_new_base_amount(subscription: ZuoraSubscription): Either[Failure, Option[BigDecimal]] = {
for {
ratePlan <- supporterPlusV2RatePlan(subscription)
billingPeriod <- ZuoraRatePlan.ratePlanToBillingPeriod(ratePlan).toRight(AmendmentDataFailure(""))
ratePlanCharge <- supporterPlusBaseRatePlanCharge(subscription.subscriptionNumber, ratePlan)
currency = ratePlanCharge.currency
oldBaseAmountOpt <- sp2024_previous_base_amount(subscription)
oldBaseAmount <- oldBaseAmountOpt.toRight(
AmendmentDataFailure(
s"(error: 164d8f1c-6dc6) could not extract base amount for subscription ${subscription.subscriptionNumber}"
)
)
newPriceFromPriceGrid <- getNewPrice(billingPeriod, currency)
.map(BigDecimal(_))
.toRight(
AmendmentDataFailure(
s"(error: 611aedea-0478) could not getNewPrice for (billingPeriod, currency) and (${billingPeriod}, ${currency})"
)
)
} yield {
Some((oldBaseAmount * BigDecimal(1.27)).min(newPriceFromPriceGrid))
}
}

def sp2024_contribution_amount(subscription: ZuoraSubscription): Either[Failure, Option[BigDecimal]] = {
for {
ratePlan <- supporterPlusV2RatePlan(subscription)
ratePlanCharge <- supporterPlusContributionRatePlanCharge(subscription.subscriptionNumber, ratePlan)
} yield ratePlanCharge.price
}

def sp2024_previous_combined_amount(subscription: ZuoraSubscription): Either[Failure, Option[BigDecimal]] = {
for {
contributionAmountOpt <- sp2024_contribution_amount(subscription)
previousBaseAmountOpt <- sp2024_previous_base_amount(subscription)
} yield (
for {
contributionAmount <- contributionAmountOpt
previousBaseAmount <- previousBaseAmountOpt
} yield contributionAmount + previousBaseAmount
)
}

def sp2024_new_combined_amount(subscription: ZuoraSubscription): Either[Failure, Option[BigDecimal]] = {
for {
contributionAmountOpt <- sp2024_contribution_amount(subscription)
previousBaseAmountOpt <- sp2024_new_base_amount(subscription)
} yield (
for {
contributionAmount <- contributionAmountOpt
previousBaseAmount <- previousBaseAmountOpt
} yield contributionAmount + previousBaseAmount
)
}

// ------------------------------------------------
// Primary Interface
// ------------------------------------------------
Expand Down
13 changes: 10 additions & 3 deletions lambda/src/main/scala/pricemigrationengine/model/PriceCap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,20 @@ object PriceCap {
// We have separate functions for determining the
// estimated new price and for computing the adjusted ZuoraSubscriptionUpdate. Unlike the now obsolete part 1
// We design those functions with the signature that is actually useful for price cap, where we will need to apply it
// 1. The notification handler, and
// 2. The SalesforcePriceRiseCreationHandler, and
// 3. The amendment handler
// 1. The SalesforcePriceRiseCreationHandler (priceCapForNotification), and
// 2. The notification handler (priceCapForNotification), and
// 3. The amendment handler (priceCapForAmendment)

// Note: We should not apply capping during the estimation step and capped prices should not be written in the
// migration dynamo tables !

// Note: These functions work better for simple price migrations where the subscription has one price
// that we update. In the case of SupporterPlus subscriptions, with their base price and extra optional
// contribution (see example of the SupporterPlus2024 price migration), the base price is price risen and
// capped, but the subscription price is the sum of that new price together with the contribution. In a
// case like that we just implemented the price capping in the migration code itself, without messing around
// with this code.

def priceCapForNotification(oldPrice: BigDecimal, newPrice: BigDecimal, cap: BigDecimal): BigDecimal = {
// The cap is the price cap expressed as a multiple of the old price. For instance for a price cap
// of 20%, we will use a cap equal to 1.2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,60 @@ case class EmailPayloadSubscriberAttributes(
next_payment_date: String,
payment_frequency: String,
subscription_id: String,
product_type: String
product_type: String,

// SupporterPlus 2024 extension (see comment below)
sp2024_contribution_amount: Option[String] = None,
sp2024_previous_combined_amount: Option[String] = None,
sp2024_new_combined_amount: Option[String] = None,
)

/*
Date: September 2024
Author: Pascal
Comment Group: 602514a6-5e53
This note describes an extension of the EmailPayloadSubscriberAttributes that we are introducing specifically
for the SupporterPlus 2024 migration.
So far in the history of the engine, we have communicated to the customers the new (post increase) price of their
subscription. This information is carried by payment_amount.
The SupporterPlus migration is making a price increase on subscriptions which have a base price and an optional
contribution that is at the discretion of the user. For instance some monthly user pay £[10, 0], which means that
the user pays the base price of £10 and no extra contribution, whereas another user would pay £[10, 15], which means
that the user pays the base price of £10 and an extra contribution of £15, leading to a total price of £25.
In the SupporterPlus2024 migration we are increasing the base price only, for GBP moving from 10 to 12.
We want to communicate to the user that only the base price is being price increased and not their extra
contribution. More exactly for customers paying no extra contribution we want to say
"""
Your new price will take effect at your next payment date, on or around {next_payment_date}, and your new payment amount will be {payment_amount} - {payment_frequency}.
"""
And for customers with an extra contribution with want to say
"""
Your new subscription price will take effect at your next payment date, on or around {Next Payment Date}.
Your {payment_frequency} contribution of {contribution_amount} remains unchanged, and so your total billing amount will now be {new_combined_amount} - {payment_frequency}.
"""
We are introducing three new fields
sp2024_contribution_amount
sp2024_previous_combined_amount
sp2024_new_combined_amount
Those fields are going to be used by the emails templates that we are going to use for S+ 2024 and
should not be used for any other migrations.
I am going to decommission them a bit after October 2025, after the end of the migration. I will
nevertheless permanently document this modification in case it is needed again in some remote future
if/when we decide to make another SupporterPlus price migration.
*/

object EmailPayloadSubscriberAttributes {
implicit val rw: ReadWriter[EmailPayloadSubscriberAttributes] = macroRW
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@
"mrr" : 13.333333333,
"dmrc" : 0.000000000,
"tcv" : 0.000000000,
"dtcv" : -160.000000000,
"dtcv" : -150.000000000,
"originalOrderDate" : "2023-10-09",
"amendedByOrderOn" : "2023-12-01",
"description" : "",
Expand All @@ -211,7 +211,7 @@
"tiers" : null,
"discountApplyDetails" : null,
"pricingSummary" : "AUD160",
"price" : 160.000000000,
"price" : 150.000000000,
"discountAmount" : null,
"discountPercentage" : null
}, {
Expand Down Expand Up @@ -354,8 +354,8 @@
"specificEndDate" : null,
"mrr" : 13.333333333,
"dmrc" : 13.333333333,
"tcv" : 160.000000000,
"dtcv" : 160.000000000,
"tcv" : 150.000000000,
"dtcv" : 150.000000000,
"originalOrderDate" : "2023-11-26",
"amendedByOrderOn" : null,
"description" : "",
Expand Down
Loading

0 comments on commit aaf823e

Please sign in to comment.