Skip to content

Commit

Permalink
refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
@shtukas
shtukas committed Sep 8, 2024
1 parent 6bca7dd commit bb21502
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 54 deletions.
38 changes: 25 additions & 13 deletions lambda/src/main/scala/pricemigrationengine/handlers/NotificationHandler.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,13 @@ object NotificationHandler extends CohortHandler {

// 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))
sp2024ContributionAmount <- ZIO.fromEither(sp2024ContributionAmount(cohortSpec, subscription))
sp2024PreviousCombinedAmount <- ZIO.fromEither(sp2024PreviousCombinedAmount(cohortSpec, subscription))
sp2024NewCombinedAmount <- ZIO.fromEither(sp2024NewCombinedAmount(cohortSpec, subscription))

sp2024ContributionAmountWithCurrencySymbol = Some(s"${currencySymbol}${sp2024ContributionAmount}")
sp2024PreviousCombinedAmountWithCurrencySymbol = Some(s"${currencySymbol}${sp2024PreviousCombinedAmount}")
sp2024NewCombinedAmountWithCurrencySymbol = Some(s"${currencySymbol}${sp2024NewCombinedAmount}")

brazeName <- ZIO.fromEither(brazeName(cohortSpec, subscription))

Expand All @@ -148,16 +152,24 @@ object NotificationHandler extends CohortHandler {
billing_postal_code = postalCode,
billing_state = address.state,
billing_country = country,
payment_amount = priceWithOptionalCappingWithCurrencySymbol,
payment_amount = priceWithOptionalCappingWithCurrencySymbol, // [1]
next_payment_date = startDateConversion(startDate),
payment_frequency = paymentFrequency,
subscription_id = cohortItem.subscriptionName,
product_type = sfSubscription.Product_Type__c.getOrElse(""),

// -------------------------------------------------------------
// [1]
// (Comment group: 7992fa98)
// For SupporterPlus2024, we did not use that value. Instead we used the data provided by the
// extension below. That value was the new base price, but we needed a different data distribution
// to be able to fill the email template. That distribution is given by the next section.

// 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,
sp2024_contribution_amount = sp2024ContributionAmountWithCurrencySymbol,
sp2024_previous_combined_amount = sp2024PreviousCombinedAmountWithCurrencySymbol,
sp2024_new_combined_amount = sp2024NewCombinedAmountWithCurrencySymbol
// -------------------------------------------------------------
)
)
),
Expand Down Expand Up @@ -460,20 +472,20 @@ object NotificationHandler extends CohortHandler {
as part of the set up of the SupporterPlus 2024 migration (see Comment Group: 602514a6-5e53)
*/

def sp2024_contribution_amount(
def sp2024ContributionAmount(
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))
SupporterPlus2024Migration.contributionAmount(subscription).map(o => o.map(b => b.toString))
case _ => Right(None)
}
}

def sp2024_previous_combined_amount(
def sp2024PreviousCombinedAmount(
cohortSpec: CohortSpec,
subscription: ZuoraSubscription
): Either[Failure, Option[String]] = {
Expand All @@ -482,21 +494,21 @@ object NotificationHandler extends CohortHandler {
MigrationType(cohortSpec) match {
case SupporterPlus2024 =>
SupporterPlus2024Migration
.sp2024_previous_combined_amount(subscription)
.previousCombinedAmount(subscription)
.map(o => o.map(b => b.toString))
case _ => Right(None)
}
}

def sp2024_new_combined_amount(
def sp2024NewCombinedAmount(
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))
SupporterPlus2024Migration.newCombinedAmount(subscription).map(o => o.map(b => b.toString))
case _ => Right(None)
}
}
Expand Down
11 changes: 10 additions & 1 deletion lambda/src/main/scala/pricemigrationengine/handlers/SalesforcePriceRiseCreationHandler.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,18 @@ object SalesforcePriceRiseCreationHandler extends CohortHandler {
case DigiSubs2023 => estimatedNewPrice
case Newspaper2024 => estimatedNewPrice
case GW2024 => PriceCap.priceCapForNotification(oldPrice, estimatedNewPrice, GW2024Migration.priceCap)
case SupporterPlus2024 => estimatedNewPrice
case SupporterPlus2024 => estimatedNewPrice // [1]
case Legacy => PriceCap.priceCapLegacy(oldPrice, estimatedNewPrice)
}
// [1]
// (Comment group: 7992fa98)

// This value wasn't actually used because we did that step using a Ruby script (we did not run the
// SalesforcePriceRiseCreationHandler from the AWS step function).
// The problem was that from the CohortItem we only had the old base price and the new base price
// but not the contribution component, and therefore we could not compute the total which is what
// we need to send to Salesforce.

SalesforcePriceRise(
Some(subscription.Name),
Some(subscription.Buyer__c),
Expand Down
44 changes: 20 additions & 24 deletions lambda/src/main/scala/pricemigrationengine/migrations/SupporterPlus2024Migration.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,20 +148,20 @@ object SupporterPlus2024Migration {
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]] = {
def previousBaseAmount(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]] = {
def newBaseAmount(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)
oldBaseAmountOpt <- previousBaseAmount(subscription)
oldBaseAmount <- oldBaseAmountOpt.toRight(
AmendmentDataFailure(
s"(error: 164d8f1c-6dc6) could not extract base amount for subscription ${subscription.subscriptionNumber}"
Expand All @@ -179,17 +179,17 @@ object SupporterPlus2024Migration {
}
}

def sp2024_contribution_amount(subscription: ZuoraSubscription): Either[Failure, Option[BigDecimal]] = {
def contributionAmount(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]] = {
def previousCombinedAmount(subscription: ZuoraSubscription): Either[Failure, Option[BigDecimal]] = {
for {
contributionAmountOpt <- sp2024_contribution_amount(subscription)
previousBaseAmountOpt <- sp2024_previous_base_amount(subscription)
contributionAmountOpt <- contributionAmount(subscription)
previousBaseAmountOpt <- previousBaseAmount(subscription)
} yield (
for {
contributionAmount <- contributionAmountOpt
Expand All @@ -198,10 +198,10 @@ object SupporterPlus2024Migration {
)
}

def sp2024_new_combined_amount(subscription: ZuoraSubscription): Either[Failure, Option[BigDecimal]] = {
def newCombinedAmount(subscription: ZuoraSubscription): Either[Failure, Option[BigDecimal]] = {
for {
contributionAmountOpt <- sp2024_contribution_amount(subscription)
previousBaseAmountOpt <- sp2024_new_base_amount(subscription)
contributionAmountOpt <- contributionAmount(subscription)
previousBaseAmountOpt <- newBaseAmount(subscription)
} yield (
for {
contributionAmount <- contributionAmountOpt
Expand All @@ -212,7 +212,7 @@ object SupporterPlus2024Migration {

def hasNonTrivialContribution(subscription: ZuoraSubscription): Either[Failure, Boolean] = {
for {
amountOpt <- sp2024_contribution_amount(subscription: ZuoraSubscription)
amountOpt <- contributionAmount(subscription: ZuoraSubscription)
amount <- amountOpt.toRight(
AmendmentDataFailure(
s"(error: 232760f5) could not extract contribution amount for subscription ${subscription.subscriptionNumber}"
Expand Down Expand Up @@ -258,17 +258,13 @@ object SupporterPlus2024Migration {
effectiveDate: LocalDate,
): Either[AmendmentDataFailure, ZuoraSubscriptionUpdate] = {
for {
ratePlan <- supporterPlusV2RatePlan(subscription)
ratePlanChargeId <- ratePlan.ratePlanCharges
.map(rpc => rpc.productRatePlanChargeId)
.headOption
.toRight(
AmendmentDataFailure(
s"[1862ada1] Could not determine the billing period for subscription ${subscription.subscriptionNumber}"
)
)
existingRatePlan <- supporterPlusV2RatePlan(subscription)
existingBaseRatePlanCharge <- supporterPlusBaseRatePlanCharge(
subscription.subscriptionNumber,
existingRatePlan
)
billingPeriod <- ZuoraRatePlan
.ratePlanToBillingPeriod(ratePlan)
.ratePlanToBillingPeriod(existingRatePlan)
.toRight(
AmendmentDataFailure(
s"[17469705] Could not determine the billing period for subscription ${subscription.subscriptionNumber}"
Expand All @@ -278,11 +274,11 @@ object SupporterPlus2024Migration {
ZuoraSubscriptionUpdate(
add = List(
AddZuoraRatePlan(
productRatePlanId = ratePlan.productRatePlanId,
productRatePlanId = existingRatePlan.productRatePlanId,
contractEffectiveDate = effectiveDate,
chargeOverrides = List(
ChargeOverride(
productRatePlanChargeId = ratePlanChargeId,
productRatePlanChargeId = existingBaseRatePlanCharge.productRatePlanChargeId,
billingPeriod = BillingPeriod.toString(billingPeriod),
price = 120.0
)
Expand All @@ -291,7 +287,7 @@ object SupporterPlus2024Migration {
),
remove = List(
RemoveZuoraRatePlan(
ratePlanId = ratePlan.id,
ratePlanId = existingRatePlan.id,
contractEffectiveDate = effectiveDate
)
),
Expand Down
32 changes: 16 additions & 16 deletions lambda/src/test/scala/pricemigrationengine/migrations/SupporterPlus2024MigrationTest.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ class SupporterPlus2024MigrationTest extends munit.FunSuite {
test("sp2024_previous_base_amount (monthly)") {
val subscription = Fixtures.subscriptionFromJson("Migrations/SupporterPlus2024/monthly/subscription.json")
assertEquals(
SupporterPlus2024Migration.sp2024_previous_base_amount(subscription),
SupporterPlus2024Migration.previousBaseAmount(subscription),
Right(
Some(BigDecimal(10))
)
Expand All @@ -486,7 +486,7 @@ class SupporterPlus2024MigrationTest extends munit.FunSuite {
test("sp2024_previous_base_amount (annual)") {
val subscription = Fixtures.subscriptionFromJson("Migrations/SupporterPlus2024/annual/subscription.json")
assertEquals(
SupporterPlus2024Migration.sp2024_previous_base_amount(subscription),
SupporterPlus2024Migration.previousBaseAmount(subscription),
Right(
Some(BigDecimal(150))
)
Expand All @@ -496,7 +496,7 @@ class SupporterPlus2024MigrationTest extends munit.FunSuite {
val subscription =
Fixtures.subscriptionFromJson("Migrations/SupporterPlus2024/sub-without-LastChangeType/subscription.json")
assertEquals(
SupporterPlus2024Migration.sp2024_previous_base_amount(subscription),
SupporterPlus2024Migration.previousBaseAmount(subscription),
Right(
Some(BigDecimal(6))
)
Expand All @@ -506,7 +506,7 @@ class SupporterPlus2024MigrationTest extends munit.FunSuite {
test("sp2024_new_base_amount (monthly)") {
val subscription = Fixtures.subscriptionFromJson("Migrations/SupporterPlus2024/monthly/subscription.json")
assertEquals(
SupporterPlus2024Migration.sp2024_new_base_amount(subscription),
SupporterPlus2024Migration.newBaseAmount(subscription),
Right(
Some(BigDecimal(12))
)
Expand All @@ -515,7 +515,7 @@ class SupporterPlus2024MigrationTest extends munit.FunSuite {
test("sp2024_new_base_amount (annual)") {
val subscription = Fixtures.subscriptionFromJson("Migrations/SupporterPlus2024/annual/subscription.json")
assertEquals(
SupporterPlus2024Migration.sp2024_new_base_amount(subscription),
SupporterPlus2024Migration.newBaseAmount(subscription),
Right(
Some(
BigDecimal(150 * 1.27)
Expand All @@ -529,15 +529,15 @@ class SupporterPlus2024MigrationTest extends munit.FunSuite {
// Below we make it explicit that we expect a 27% charge to be applied to the base charge as part of the price rise
val newBasePrice = 6 * 1.27
assertEquals(
SupporterPlus2024Migration.sp2024_new_base_amount(subscription),
SupporterPlus2024Migration.newBaseAmount(subscription),
Right(Some(BigDecimal(newBasePrice)))
)
}

test("sp2024_contribution_amount (monthly)") {
val subscription = Fixtures.subscriptionFromJson("Migrations/SupporterPlus2024/monthly/subscription.json")
assertEquals(
SupporterPlus2024Migration.sp2024_contribution_amount(subscription),
SupporterPlus2024Migration.contributionAmount(subscription),
Right(
Some(BigDecimal(0))
)
Expand All @@ -546,7 +546,7 @@ class SupporterPlus2024MigrationTest extends munit.FunSuite {
test("sp2024_contribution_amount (annual)") {
val subscription = Fixtures.subscriptionFromJson("Migrations/SupporterPlus2024/annual/subscription.json")
assertEquals(
SupporterPlus2024Migration.sp2024_contribution_amount(subscription),
SupporterPlus2024Migration.contributionAmount(subscription),
Right(
Some(BigDecimal(340))
)
Expand All @@ -556,7 +556,7 @@ class SupporterPlus2024MigrationTest extends munit.FunSuite {
val subscription =
Fixtures.subscriptionFromJson("Migrations/SupporterPlus2024/sub-without-LastChangeType/subscription.json")
assertEquals(
SupporterPlus2024Migration.sp2024_contribution_amount(subscription),
SupporterPlus2024Migration.contributionAmount(subscription),
Right(
Some(BigDecimal(3))
)
Expand All @@ -566,7 +566,7 @@ class SupporterPlus2024MigrationTest extends munit.FunSuite {
test("sp2024_previous_combined_amount (monthly)") {
val subscription = Fixtures.subscriptionFromJson("Migrations/SupporterPlus2024/monthly/subscription.json")
assertEquals(
SupporterPlus2024Migration.sp2024_previous_combined_amount(subscription),
SupporterPlus2024Migration.previousCombinedAmount(subscription),
Right(
Some(BigDecimal(10))
)
Expand All @@ -575,7 +575,7 @@ class SupporterPlus2024MigrationTest extends munit.FunSuite {
test("sp2024_previous_combined_amount (annual)") {
val subscription = Fixtures.subscriptionFromJson("Migrations/SupporterPlus2024/annual/subscription.json")
assertEquals(
SupporterPlus2024Migration.sp2024_previous_combined_amount(subscription),
SupporterPlus2024Migration.previousCombinedAmount(subscription),
Right(
Some(BigDecimal(490))
)
Expand All @@ -585,7 +585,7 @@ class SupporterPlus2024MigrationTest extends munit.FunSuite {
val subscription =
Fixtures.subscriptionFromJson("Migrations/SupporterPlus2024/sub-without-LastChangeType/subscription.json")
assertEquals(
SupporterPlus2024Migration.sp2024_previous_combined_amount(subscription),
SupporterPlus2024Migration.previousCombinedAmount(subscription),
Right(
Some(BigDecimal(6 + 3))
)
Expand All @@ -595,7 +595,7 @@ class SupporterPlus2024MigrationTest extends munit.FunSuite {
test("sp2024_new_combined_amount (monthly)") {
val subscription = Fixtures.subscriptionFromJson("Migrations/SupporterPlus2024/monthly/subscription.json")
assertEquals(
SupporterPlus2024Migration.sp2024_new_combined_amount(subscription),
SupporterPlus2024Migration.newCombinedAmount(subscription),
Right(
Some(BigDecimal(12))
)
Expand All @@ -605,7 +605,7 @@ class SupporterPlus2024MigrationTest extends munit.FunSuite {
val subscription = Fixtures.subscriptionFromJson("Migrations/SupporterPlus2024/annual/subscription.json")
val newCombinedAmount = 150 * 1.27 + 340
assertEquals(
SupporterPlus2024Migration.sp2024_new_combined_amount(subscription),
SupporterPlus2024Migration.newCombinedAmount(subscription),
Right(
Some(BigDecimal(newCombinedAmount))
)
Expand All @@ -620,7 +620,7 @@ class SupporterPlus2024MigrationTest extends munit.FunSuite {
val newCombinedAmount = BigDecimal(6 * 1.27 + 3).setScale(2, BigDecimal.RoundingMode.HALF_UP)

assertEquals(
SupporterPlus2024Migration.sp2024_new_combined_amount(subscription),
SupporterPlus2024Migration.newCombinedAmount(subscription),
Right(
Some(newCombinedAmount)
)
Expand Down Expand Up @@ -741,7 +741,7 @@ class SupporterPlus2024MigrationTest extends munit.FunSuite {
contractEffectiveDate = LocalDate.of(2024, 9, 9),
chargeOverrides = List(
ChargeOverride(
productRatePlanChargeId = "8a128ed885fc6ded018602296af13eba",
productRatePlanChargeId = "8a128ed885fc6ded018602296af13eba", // base plan charge Id
billingPeriod = "Month",
price = 120.0
)
Expand Down

0 comments on commit bb21502

Please sign in to comment.