Skip to content

Commit

Permalink
Merge pull request #1073 from guardian/jd-remove-paid-classes
Browse files Browse the repository at this point in the history
remove the paid/free distinction in the reads
  • Loading branch information
johnduffell committed May 21, 2024
2 parents bb3a55c + 965c9dc commit 23c89ba
Show file tree
Hide file tree
Showing 30 changed files with 242 additions and 633 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ class AccountController(
.map(subs => subscriptionSelector(subscriptionName, s"the sfUser $contact", subs)),
)
contributionPlan <- SimpleEitherT.fromEither(subscription.plan match {
case p: SubscriptionPlan.Contributor @unchecked /* extra guard needed due to type erasure --> */ if p.product == Contribution => Right(p)
case p if p.product == Contribution => Right(p)
case nc => Left(s"$subscriptionName plan is not a contribution: " + nc)
})
billingPeriod = contributionPlan.charges.billingPeriod.asInstanceOf[RecurringPeriod]
Expand Down
19 changes: 7 additions & 12 deletions membership-attribute-service/app/models/AccountDetails.scala
Original file line number Diff line number Diff line change
Expand Up @@ -109,18 +109,13 @@ object AccountDetails {
"end" -> plan.end,
// if the customer acceptance date is future dated (e.g. 6for6) then always display, otherwise only show if starting less than 30 days from today
"shouldBeVisible" -> (subscription.acceptanceDate.isAfter(now) || plan.start.isBefore(now.plusDays(30))),
) ++ (plan match {
case paidPlan: PaidSubscriptionPlan[_, _] =>
Json.obj(
"chargedThrough" -> paidPlan.chargedThrough,
"price" -> paidPlan.charges.price.prices.head.amount * 100,
"currency" -> paidPlan.charges.price.prices.head.currency.glyph,
"currencyISO" -> paidPlan.charges.price.prices.head.currency.iso,
"billingPeriod" -> paidPlan.charges.billingPeriod.noun,
"features" -> paidPlan.features.map(_.code.get).mkString(","),
)
case _ => Json.obj()
}) ++ (plan.charges match {
"chargedThrough" -> plan.chargedThrough,
"price" -> plan.charges.price.prices.head.amount * 100,
"currency" -> plan.charges.price.prices.head.currency.glyph,
"currencyISO" -> plan.charges.price.prices.head.currency.iso,
"billingPeriod" -> plan.charges.billingPeriod.noun,
"features" -> plan.features.map(_.code.get).mkString(","),
) ++ (plan.charges match {
case paperCharges: PaperCharges =>
Json.obj(
"daysOfWeek" ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import models.{AccountDetails, ContactAndSubscription, DeliveryAddress}
import monitoring.CreateMetrics
import scalaz.ListT
import scalaz.std.scalaFuture._
import services.DifferentiateSubscription.differentiateSubscription
import services.PaymentFailureAlerter.{accountHasMissedPayments, alertText, safeToAllowPaymentUpdate}
import services.salesforce.ContactRepository
import services.stripe.ChooseStripe
Expand Down Expand Up @@ -47,7 +46,6 @@ class AccountDetailsFromZuora(
): ListT[SimpleEitherT, AccountDetails] = {
for {
contactAndSubscription <- allCurrentSubscriptions(userId, filter)
isPaidSubscription = differentiateSubscription(contactAndSubscription).isRight
detailsResultsTriple <- ListTEither.single(getAccountDetailsParallel(contactAndSubscription))
(paymentDetails, accountSummary, effectiveCancellationDate) = detailsResultsTriple
country = accountSummary.billToContact.country
Expand All @@ -64,7 +62,7 @@ class AccountDetailsFromZuora(
paymentDetails = paymentDetails,
billingCountry = accountSummary.billToContact.country,
stripePublicKey = stripePublicKey.key,
accountHasMissedRecentPayments = isPaidSubscription &&
accountHasMissedRecentPayments =
accountHasMissedPayments(contactAndSubscription.subscription.accountId, accountSummary.invoices, accountSummary.payments),
safeToUpdatePaymentMethod = safeToAllowPaymentUpdate(contactAndSubscription.subscription.accountId, accountSummary.invoices),
isAutoRenew = isAutoRenew,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import com.gu.memsub.Product.GuardianPatron
import com.gu.memsub.Subscription._
import com.gu.memsub._
import com.gu.memsub.subsv2.ReaderType.Direct
import com.gu.memsub.subsv2.{CovariantNonEmptyList, PaidCharge, PaidSubscriptionPlan, Subscription}
import com.gu.memsub.subsv2.{CovariantNonEmptyList, SingleCharge, Subscription, SubscriptionPlan}
import com.gu.monitoring.SafeLogger.LogPrefix
import com.gu.services.model.PaymentDetails
import com.gu.services.model.PaymentDetails.PersonalPlan
Expand Down Expand Up @@ -87,7 +87,7 @@ class GuardianPatronService(
isCancelled = subscription.isCancelled,
hasPendingFreePlan = false,
plans = CovariantNonEmptyList(
PaidSubscriptionPlan(
SubscriptionPlan(
id = RatePlanId(guardianPatronProductRatePlanId),
productRatePlanId = ProductRatePlanId(guardianPatronProductRatePlanId),
name = subscription.plan.id,
Expand All @@ -97,7 +97,7 @@ class GuardianPatronService(
productType = "Membership",
product = GuardianPatron,
features = Nil,
charges = PaidCharge(
charges = SingleCharge(
benefit = Benefit.GuardianPatron,
billingPeriod = billingPeriodFromInterval(subscription.plan.interval),
price = PricingSummary(Map(subscription.plan.currency -> price)),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,40 +1,29 @@
package services

import com.gu.memsub.promo.LogImplicit.LoggableFuture
import com.gu.memsub.subsv2.{Subscription, SubscriptionPlan}
import com.gu.memsub.{BillingPeriod, Price}
import com.gu.monitoring.SafeLogger.LogPrefix
import com.gu.monitoring.SafeLogging
import com.gu.services.model.PaymentDetails
import com.gu.services.model.PaymentDetails.PersonalPlan
import models.ContactAndSubscription
import scalaz.\/
import services.DifferentiateSubscription.differentiateSubscription
import services.zuora.payment.PaymentService

import scala.concurrent.{ExecutionContext, Future}
import scala.util.{Failure, Success}

class PaymentDetailsForSubscription(paymentService: PaymentService) extends SafeLogging {
def getPaymentDetails(
contactAndSubscription: ContactAndSubscription,
)(implicit ec: ExecutionContext, logPrefix: LogPrefix): Future[PaymentDetails] = {
val isGiftRedemption = contactAndSubscription.isGiftRedemption
val differentiated = differentiateSubscription(contactAndSubscription)
differentiated match {
case Right(giftSub) if isGiftRedemption =>
Future.successful(giftPaymentDetailsFor(giftSub))
case Right(paidSub) =>
val paymentDetails = paymentService.paymentDetails(\/.fromEither(differentiated), defaultMandateIdIfApplicable = Some(""))
paymentDetails.onComplete {
case Failure(exception) => logger.error(scrub"Failed to get payment details for $paidSub: $exception")
case Success(_) => logger.info(s"Successfully got payment details for $paidSub")
}
paymentDetails
case Left(freeSub) => Future.successful(PaymentDetails(freeSub))
}
val subscription = contactAndSubscription.subscription
if (contactAndSubscription.isGiftRedemption)
Future.successful(giftPaymentDetailsFor(subscription))
else
paymentService.paymentDetails(subscription, defaultMandateIdIfApplicable = Some("")).withLogging(s"get payment details for $subscription")
}

private def giftPaymentDetailsFor(giftSubscription: Subscription[SubscriptionPlan.Paid]): PaymentDetails = PaymentDetails(
private def giftPaymentDetailsFor(giftSubscription: Subscription[SubscriptionPlan.AnyPlan]): PaymentDetails = PaymentDetails(
pendingCancellation = giftSubscription.isCancelled,
chargedThroughDate = None,
startDate = giftSubscription.startDate,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package services

import com.gu.memsub.Product
import com.gu.memsub.Product.{Contribution, Membership}
import com.gu.memsub.Subscription.AccountId
import com.gu.memsub.subsv2.SubscriptionPlan.AnyPlan
import com.gu.memsub.subsv2.{Subscription, SubscriptionPlan}
Expand Down Expand Up @@ -53,13 +54,14 @@ object PaymentFailureAlerter extends SafeLogging {
case None => Future.successful(None)
}

def getProductDescription(subscription: Subscription[SubscriptionPlan.AnyPlan]) = if (subscription.asMembership.isDefined) {
s"${subscription.plan.productName} membership"
} else if (subscription.asContribution.isDefined) {
"contribution"
} else {
subscription.plan.productName
}
def getProductDescription(subscription: Subscription[SubscriptionPlan.AnyPlan]) =
if (subscription.plans.head.product == Membership) {
s"${subscription.plan.productName} membership"
} else if (subscription.plans.head.product == Contribution) {
"contribution"
} else {
subscription.plan.productName
}

maybePaymentMethodLatestDate map { maybeDate: Option[DateTime] =>
maybeDate map { latestDate: DateTime =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,20 @@ import com.gu.zuora.ZuoraSoapService
import com.gu.zuora.soap.models.Queries
import com.gu.zuora.soap.models.Queries.Account
import com.gu.zuora.soap.models.Queries.PaymentMethod._
import scalaz.\/
import scalaz.std.option._
import scalaz.std.scalaFuture._
import scalaz.syntax.monad._
import scalaz.syntax.std.option._

import scala.concurrent.{ExecutionContext, Future}
import scala.util.{Failure, Success, Try}
import scala.util.Try

class PaymentService(zuoraService: ZuoraSoapService, planMap: Map[ProductRatePlanChargeId, Benefit])(implicit ec: ExecutionContext)
extends SafeLogging {

def paymentDetails(
sub: Subscription[SubscriptionPlan.Free] \/ Subscription[SubscriptionPlan.Paid],
sub: Subscription[SubscriptionPlan.AnyPlan],
defaultMandateIdIfApplicable: Option[String] = None,
)(implicit logPrefix: LogPrefix): Future[PaymentDetails] =
sub.fold(a => Future.successful(PaymentDetails(a)), paidPaymentDetails(defaultMandateIdIfApplicable))

private def paidPaymentDetails(
defaultMandateIdIfApplicable: Option[String],
)(sub: Subscription[SubscriptionPlan.Paid])(implicit logPrefix: LogPrefix): Future[PaymentDetails] = {
)(implicit logPrefix: LogPrefix): Future[PaymentDetails] = {
val currency = sub.plan.charges.currencies.head
// I am not convinced this function is very safe, hence the option
val eventualMaybeLastPaymentDate = zuoraService
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ class AccountControllerAcceptanceTest extends AcceptanceTest {

contactRepositoryMock.get("200067388")(any) returns Future(\/.right(Some(contact)))

val charge = TestPaidCharge()
val charge = TestSingleCharge()
val chargedThroughDate = new LocalDate(2023, 3, 11)
val plan = TestPaidSubscriptionPlan(
product = Contribution,
Expand All @@ -320,9 +320,9 @@ class AccountControllerAcceptanceTest extends AcceptanceTest {
val subscription = TestSubscription(
name = Subscription.Name(subscriptionId),
plans = CovariantNonEmptyList(plan, Nil),
).asInstanceOf[com.gu.memsub.subsv2.Subscription[SubscriptionPlan.Contributor]]
)

subscriptionServiceMock.current[SubscriptionPlan.Contributor](contact)(any, any) returns Future(List(subscription))
subscriptionServiceMock.current[SubscriptionPlan.AnyPlan](contact)(any, any) returns Future(List(subscription))

zuoraRestServiceMock.updateChargeAmount(
subscription.name,
Expand Down Expand Up @@ -350,7 +350,7 @@ class AccountControllerAcceptanceTest extends AcceptanceTest {
contactRepositoryMock.get("200067388")(any) was called

identityMockClientAndServer.verify(identityRequest)
subscriptionServiceMock.current[SubscriptionPlan.Contributor](contact)(any, any) was called
subscriptionServiceMock.current[SubscriptionPlan.AnyPlan](contact)(any, any) was called
zuoraRestServiceMock.updateChargeAmount(
subscription.name,
charge.subRatePlanChargeId,
Expand Down Expand Up @@ -465,7 +465,7 @@ class AccountControllerAcceptanceTest extends AcceptanceTest {
contactRepositoryMock.get("200067388")(any) was called

identityMockClientAndServer.verify(identityRequest)
subscriptionServiceMock.current[SubscriptionPlan.Contributor](contact)(any, any) was called
subscriptionServiceMock.current[SubscriptionPlan.AnyPlan](contact)(any, any) was called

subscriptionServiceMock.decideCancellationEffectiveDate[SubscriptionPlan.AnyPlan](Name(subscriptionId), any, any)(any, any) was called
subscriptionServiceMock.subscriptionsForAccountId[SubscriptionPlan.AnyPlan](subscription.accountId)(any, any) was called
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ class PaymentUpdateControllerAcceptanceTest extends AcceptanceTest {
httpResponse.getStatus shouldEqual 200

identityMockClientAndServer.verify(identityRequest)
subscriptionServiceMock.current[SubscriptionPlan.Contributor](contact)(any, any) was called
subscriptionServiceMock.current[SubscriptionPlan.AnyPlan](contact)(any, any) was called
contactRepositoryMock.get("200067388")(any) was called
catalogServiceMock.unsafeCatalog was called
zuoraSoapServiceMock.getAccount(subscription.accountId)(any) wasCalled twice
Expand Down Expand Up @@ -364,7 +364,7 @@ class PaymentUpdateControllerAcceptanceTest extends AcceptanceTest {
httpResponse.getStatus shouldEqual 200

identityMockClientAndServer.verify(identityRequest)
subscriptionServiceMock.current[SubscriptionPlan.Contributor](contact)(any, any) was called
subscriptionServiceMock.current[SubscriptionPlan.AnyPlan](contact)(any, any) was called
contactRepositoryMock.get("200067388")(any) was called
ukStripeServiceMock.createCustomerWithStripePaymentMethod("myStripePaymentMethodId")(any) was called
ukStripeServiceMock.paymentIntentsGateway was called
Expand Down
28 changes: 14 additions & 14 deletions membership-attribute-service/test/acceptance/data/TestCatalog.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ object TestCatalogPlan {
billingPeriod: BP,
name: String,
amount: Double,
): CatalogPlan[P, PaidCharge[B, BP], Current] =
TestCatalogPlan[P, PaidCharge[B, BP], Current](
): CatalogPlan[P, SingleCharge[B, BP], Current] =
TestCatalogPlan[P, SingleCharge[B, BP], Current](
product = product,
name = name + "Paid",
charges = TestPaidCharge[B, BP](benefit, billingPeriod, TestPricingSummary.gbp(amount)),
charges = TestSingleCharge[B, BP](benefit, billingPeriod, TestPricingSummary.gbp(amount)),
status = Status.current,
)

Expand All @@ -44,47 +44,47 @@ object TestCatalogPlan {
benefit: B,
name: String,
amount: Double,
): CatalogPlan[P, PaidCharge[B, Month.type], Current] =
): CatalogPlan[P, SingleCharge[B, Month.type], Current] =
paid(product, benefit, Month, name + "Monthly", amount)

def sixWeeksPaid[P <: Product, B <: Benefit](
product: P,
benefit: B,
name: String,
amount: Double,
): CatalogPlan[P, PaidCharge[B, SixWeeks.type], Current] =
): CatalogPlan[P, SingleCharge[B, SixWeeks.type], Current] =
paid(product, benefit, SixWeeks, name + "SixWeeks", amount)

def quarterlyPaid[P <: Product, B <: Benefit](
product: P,
benefit: B,
name: String,
amount: Double,
): CatalogPlan[P, PaidCharge[B, Quarter.type], Current] =
): CatalogPlan[P, SingleCharge[B, Quarter.type], Current] =
paid(product, benefit, Quarter, name + "Quarterly", amount)

def yearlyPaid[P <: Product, B <: Benefit](
product: P,
benefit: B,
name: String,
amount: Double,
): CatalogPlan[P, PaidCharge[B, Year.type], Current] =
): CatalogPlan[P, SingleCharge[B, Year.type], Current] =
paid(product, benefit, Year, name + "Yearly", amount)

def threeMonthsPaid[P <: Product, B <: Benefit](
product: P,
benefit: B,
name: String,
amount: Double,
): CatalogPlan[P, PaidCharge[B, ThreeMonths.type], Current] =
): CatalogPlan[P, SingleCharge[B, ThreeMonths.type], Current] =
paid(product, benefit, ThreeMonths, name + "ThreeMonths", amount)

def oneYearPaid[P <: Product, B <: Benefit](
product: P,
benefit: B,
name: String,
amount: Double,
): CatalogPlan[P, PaidCharge[B, OneYear.type], Current] =
): CatalogPlan[P, SingleCharge[B, OneYear.type], Current] =
paid(product, benefit, OneYear, name + "OneYear", amount)

def paperCharges[P <: Product, BP <: BillingPeriod](product: P, name: String): CatalogPlan[P, PaperCharges, Current] =
Expand Down Expand Up @@ -116,8 +116,8 @@ object TestPaperCharges {
}

object TestPlans {
def testPaidMembershipPlans[B <: Benefit](benefit: B): PaidMembershipPlans[B] = {
PaidMembershipPlans(
def testPaidMembershipPlans[B <: Benefit](benefit: B): MembershipPlans[B] = {
MembershipPlans(
monthlyPaid(Membership, benefit, "Membership", 10),
yearlyPaid(Membership, benefit, "Membership", 120),
)
Expand Down Expand Up @@ -187,9 +187,9 @@ object TestPlans {

object TestCatalog {
def apply(
supporter: PaidMembershipPlans[Supporter.type] = testPaidMembershipPlans(Supporter),
partner: PaidMembershipPlans[Partner.type] = testPaidMembershipPlans(Partner),
patron: PaidMembershipPlans[Patron.type] = testPaidMembershipPlans(Patron),
supporter: MembershipPlans[Supporter.type] = testPaidMembershipPlans(Supporter),
partner: MembershipPlans[Partner.type] = testPaidMembershipPlans(Partner),
patron: MembershipPlans[Patron.type] = testPaidMembershipPlans(Patron),
digipack: DigipackPlans = testDigipackPlans(),
supporterPlus: SupporterPlusPlans = testSupporterPlusPlans(),
contributor: CatalogPlan.Contributor = monthlyPaid(Product.Contribution, Benefit.Contributor, "Contributor", 12),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
package acceptance.data

import acceptance.data.Randoms.randomId
import com.gu.memsub
import com.gu.memsub.{Benefit, BillingPeriod, Product}
import com.gu.memsub.Product
import com.gu.memsub.Product.Membership
import com.gu.memsub.Subscription.{Feature, ProductRatePlanId, RatePlanId}
import com.gu.memsub.subsv2.{PaidChargeList, PaidSubscriptionPlan}
import com.gu.memsub.subsv2.{ChargeList, SubscriptionPlan}
import org.joda.time.LocalDate

import java.time.Month

object TestPaidSubscriptionPlan {
def apply[P <: Product, C <: PaidChargeList](
def apply[P <: Product, C <: ChargeList](
id: RatePlanId = RatePlanId(randomId("ratePlan")),
productRatePlanId: ProductRatePlanId = ProductRatePlanId(randomId("productRatePlan")),
name: String = randomId("paidSubscriptionPlanName"),
Expand All @@ -21,11 +18,11 @@ object TestPaidSubscriptionPlan {
productType: String = randomId("paidSubscriptionPlanProductType"),
product: P = Membership,
features: List[Feature] = Nil,
charges: C = TestPaidCharge(),
charges: C = TestSingleCharge(),
chargedThrough: Option[LocalDate] = None, // this is None if the sub hasn't been billed yet (on a free trial)
start: LocalDate = LocalDate.now().minusDays(13),
end: LocalDate = LocalDate.now().minusDays(13).plusYears(1),
): PaidSubscriptionPlan[P, C] = PaidSubscriptionPlan(
): SubscriptionPlan[P, C] = SubscriptionPlan(
id: RatePlanId,
productRatePlanId,
name,
Expand Down
Loading

0 comments on commit 23c89ba

Please sign in to comment.