remove code to handle pending free plan i.e. friend

membership-attribute-service/app/components/TouchpointComponents.scala

@@ -133,7 +133,7 @@ class TouchpointComponents(
     ),
   )
 
-  lazy val futureCatalog: Future[CatalogMap] = catalogService.catalog
+  private lazy val futureCatalog: Future[CatalogMap] = catalogService.catalog
     .map(_.fold[CatalogMap](error => { logger.errorNoPrefix(scrub"error: ${error.list.toList.mkString}"); Map() }, _.map))
     .recover { case error =>
       logger.errorNoPrefix(scrub"Failed to load the product catalog from Zuora due to: $error")

membership-attribute-service/app/services/GuardianPatronService.scala

@@ -85,7 +85,6 @@ class GuardianPatronService(
         casActivationDate = None,
         promoCode = None,
         isCancelled = subscription.isCancelled,
-        hasPendingFreePlan = false,
         plans = CovariantNonEmptyList(
           SubscriptionPlan(
             id = RatePlanId(guardianPatronProductRatePlanId),
@@ -125,7 +124,6 @@ class GuardianPatronService(
         nextPaymentDate = subscription.nextPaymentDate,
         remainingTrialLength = 0,
         pendingCancellation = subscription.isPastDue,
-        pendingAmendment = false,
         paymentMethod = paymentDetails.cardStripeList.data.headOption.map(card =>
           PaymentCard(
             isReferenceTransaction = false,

membership-attribute-service/app/services/PaymentDetailsForSubscription.scala

@@ -32,7 +32,6 @@ class PaymentDetailsForSubscription(paymentService: PaymentService) extends Safe
     lastPaymentDate = None,
     nextPaymentDate = None,
     termEndDate = giftSubscription.termEndDate,
-    pendingAmendment = false,
     paymentMethod = None,
     plan = PersonalPlan(
       name = giftSubscription.plan.productName,

membership-attribute-service/test/acceptance/data/TestSubscription.scala

@@ -18,7 +18,6 @@ object TestSubscription {
       casActivationDate: Option[DateTime] = None,
       promoCode: Option[PromoCode] = None,
       isCancelled: Boolean = false,
-      hasPendingFreePlan: Boolean = false,
       plans: CovariantNonEmptyList[SubscriptionPlan] = CovariantNonEmptyList(TestPaidSubscriptionPlan(), Nil),
       readerType: ReaderType = ReaderType.Direct,
       gifteeIdentityId: Option[String] = None,
@@ -35,7 +34,6 @@ object TestSubscription {
       casActivationDate,
       promoCode,
       isCancelled,
-      hasPendingFreePlan,
       plans,
       readerType,
       gifteeIdentityId,

membership-attribute-service/test/services/PaymentDetailsForSubscriptionTest.scala

@@ -41,7 +41,6 @@ class PaymentDetailsForSubscriptionTest(implicit ee: ExecutionEnv) extends Speci
             lastPaymentDate = None,
             nextPaymentDate = None,
             termEndDate = digipackGift.termEndDate,
-            pendingAmendment = false,
             paymentMethod = None,
             plan = PersonalPlan(
               name = "Digital Pack",

membership-attribute-service/test/testdata/SubscriptionTestData.scala

@@ -169,7 +169,6 @@ trait SubscriptionTestData {
       casActivationDate = None,
       promoCode = None,
       isCancelled = isCancelled,
-      hasPendingFreePlan = false,
       plans = CovariantNonEmptyList(plans.head, plans.tail.toList),
       readerType = ReaderType.Direct,
       gifteeIdentityId = None,

membership-common/src/main/scala/com/gu/memsub/subsv2/Subscription.scala

@@ -28,7 +28,6 @@ case class Subscription(
     casActivationDate: Option[DateTime],
     promoCode: Option[PromoCode],
     isCancelled: Boolean,
-    hasPendingFreePlan: Boolean,
     plans: CovariantNonEmptyList[SubscriptionPlan],
     readerType: ReaderType,
     gifteeIdentityId: Option[String],
@@ -110,7 +109,7 @@ object GetCurrentPlans {
 }
 
 object Subscription {
-  def partial(hasPendingFreePlan: Boolean)(
+  def partial(
       id: memsub.Subscription.Id,
       name: memsub.Subscription.Name,
       accountId: memsub.Subscription.AccountId,
@@ -136,7 +135,6 @@ object Subscription {
       casActivationDate = casActivationDate,
       promoCode = promoCode,
       isCancelled = isCancelled,
-      hasPendingFreePlan = hasPendingFreePlan,
       plans = CovariantNonEmptyList(plans.head, plans.tail.toList),
       readerType = readerType,
       gifteeIdentityId = gifteeIdentityId,

membership-common/src/main/scala/com/gu/memsub/subsv2/reads/SubJsonReads.scala

@@ -92,27 +92,12 @@ object SubJsonReads {
     }
   }
 
-  implicit val lenientDateTimeReader: Reads[DateTime] =
+  private val lenientDateTimeReader: Reads[DateTime] =
     JodaReads.DefaultJodaDateTimeReads orElse Reads.IsoDateReads.map(new DateTime(_))
 
-  def subscriptionReads(
-      now: LocalDate, /*TODO get rid when we fix the below*/
-  ): Reads[NonEmptyList[SubscriptionPlan] => Subscription] = new Reads[NonEmptyList[SubscriptionPlan] => Subscription] {
+  val subscriptionReads: Reads[NonEmptyList[SubscriptionPlan] => Subscription] = new Reads[NonEmptyList[SubscriptionPlan] => Subscription] {
     override def reads(json: JsValue): JsResult[NonEmptyList[SubscriptionPlan] => Subscription] = {
 
-      // ideally we'd use the plans list
-      // on the main subscription model, but this is a quick fix.
-      val hasPendingFreePlan: Boolean = json \ "ratePlans" match {
-        case JsDefined(JsArray(plans)) =>
-          plans.exists { plan =>
-            val prices = (plan \\ "price").flatMap(_.asOpt[Float])
-            val chargeStartDates = (plan \\ "effectiveStartDate").flatMap(_.asOpt[LocalDate])
-            val planAddedThroughAmendment = (plan \ "lastChangeType").asOpt[String].contains("Add")
-            chargeStartDates.forall(_ >= now) && prices.forall(_ == 0f) && planAddedThroughAmendment
-          }
-        case _ => false
-      }
-
       json match {
         case o: JsObject =>
           (
@@ -129,7 +114,7 @@ object SubJsonReads {
               (__ \ "ReaderType__c").readNullable[String].map(ReaderType.apply) and
               (__ \ "GifteeIdentityId__c").readNullable[String] and
               (__ \ "autoRenew").read[Boolean]
-          )(memsub.subsv2.Subscription.partial(hasPendingFreePlan) _).reads(o)
+          )(memsub.subsv2.Subscription.partial _).reads(o)
         case e => JsError(s"Needed a JsObject, got ${e.getClass.getSimpleName}")
       }
     }

membership-common/src/main/scala/com/gu/memsub/subsv2/services/SubscriptionService.scala

@@ -2,14 +2,11 @@ package com.gu.memsub.subsv2.services
 
 import com.gu.memsub
 import com.gu.memsub.Subscription.{AccountId, ProductRatePlanId, RatePlanId}
-import com.gu.memsub.subsv2.SubscriptionPlan._
+import com.gu.memsub.promo.LogImplicit.Loggable
 import com.gu.memsub.subsv2._
 import com.gu.memsub.subsv2.reads.ChargeListReads.ProductIds
 import com.gu.memsub.subsv2.reads.SubJsonReads._
-import com.gu.memsub.subsv2.reads.SubPlanReads
 import com.gu.memsub.subsv2.services.SubscriptionService.CatalogMap
-import com.gu.memsub.subsv2.services.SubscriptionTransform.getRecentlyCancelledSubscriptions
-import com.gu.memsub.subsv2.services.Trace.Traceable
 import com.gu.monitoring.SafeLogger.LogPrefix
 import com.gu.monitoring.SafeLogging
 import com.gu.salesforce.ContactId
@@ -20,8 +17,6 @@ import play.api.libs.json.{Reads => JsReads, _}
 import scalaz._
 import scalaz.syntax.all._
 
-import scala.util.Try
-
 case class SubIds(ratePlanId: RatePlanId, productRatePlanId: ProductRatePlanId)
 
 object SubscriptionService {
@@ -68,7 +63,7 @@ class SubscriptionService[M[_]: Monad](pids: ProductIds, futureCatalog: => M[Cat
     extends SafeLogging {
   type EitherTM[A] = EitherT[String, M, A]
 
-  private implicit val idReads = new JsReads[JsValue] {
+  private val idReads = new JsReads[JsValue] {
     override def reads(json: JsValue): JsResult[JsValue] = JsSuccess(json)
   }
 
@@ -95,53 +90,46 @@ class SubscriptionService[M[_]: Monad](pids: ProductIds, futureCatalog: => M[Cat
     * @see
     *   https://community.zuora.com/t5/Admin-Settings-Ideas/Get-current-active-subscription-rate-plans/idi-p/19049
     */
-  def get(
-      name: memsub.Subscription.Name,
-      isActiveToday: Boolean = false,
-  )(implicit logPrefix: LogPrefix): M[Option[Subscription]] = {
+  def get(name: memsub.Subscription.Name, isActiveToday: Boolean = false)(implicit logPrefix: LogPrefix): M[Option[Subscription]] = {
 
     val url =
       if (isActiveToday)
         s"subscriptions/${name.get}?charge-detail=current-segment" // (effectiveStartDate <= today’s date < effectiveEndDate).
       else
         s"subscriptions/${name.get}" // FIXME: equivalent to ?charge-detail=last-segment which returns even removed historical charges. We should not have this as default.
 
-    val futureSubJson = rest.get[JsValue](url)(idReads, logPrefix)
-
-    futureSubJson.flatMap { subJson =>
-      futureCatalog.map { catalog =>
-        // FIXME: Why naming indicates multiple subscriptions? There should be only one sub per provided name.
-        val allSubscriptionsForSubscriberName = subJson.flatMap { jsValue =>
-          SubscriptionTransform.getSubscription(catalog, pids)(jsValue).withTrace("getAllValidSubscriptionsFromJson")
-        }
-        allSubscriptionsForSubscriberName.leftMap(error => logger.warn(s"Error from sub service for $name: $error")).toOption
-
-      }
-    }
+    for {
+      subJson <- rest.get[JsValue](url)(idReads, logPrefix)
+      catalog <- futureCatalog
+    } yield for {
+      jsValue <- subJson.withLogging(s"get subscription $name from zuora").toOption
+      subscription <- SubscriptionTransform
+        .getSubscription(catalog, pids)(jsValue)
+        .withLogging(s"getAllValidSubscriptionsFromJson for $name")
+        .toOption
+    } yield subscription
   }
 
   /** Using fromContact above fetch all the subscriptions for a given contact
     */
   private def subscriptionsForContact(
       transform: SubscriptionTransform.TimeRelativeSubTransformer,
-  )(contact: ContactId)(implicit logPrefix: LogPrefix): M[List[Subscription]] = {
-    val subJsonsFuture = jsonSubscriptionsFromContact(contact)
-
-    subJsonsFuture.flatMap { subJsonsEither =>
-      futureCatalog.map { catalog =>
-        val highLevelSubscriptions = subJsonsEither.map { subJsons =>
-          transform(catalog, pids)(subJsons)
-            .leftMap(e => logger.warn(s"Error from sub service for contact $contact: $e"))
-            .toList
-            .flatMap(_.list.toList) // returns an empty list if there's an error
-        }
-        highLevelSubscriptions
-          .leftMap(e => logger.warn(s"Error from sub service for contact $contact: $e"))
-          .toList
-          .flatten // returns an empty list if there's an error
+  )(contact: ContactId)(implicit logPrefix: LogPrefix): M[List[Subscription]] =
+    for {
+      subJsonsEither <- jsonSubscriptionsFromContact(contact)
+      catalog <- futureCatalog
+    } yield {
+      val highLevelSubscriptions = for {
+        subJsons <- subJsonsEither.withLogging("sub service - get for contact")
+        subscriptions <- transform(catalog, pids)(subJsons).withLogging("sub transform for json")
+      } yield subscriptions
+      highLevelSubscriptions.toEither match {
+        case Left(error) =>
+          logger.warn(s"Error from sub service for contact $contact: $error")
+          List.empty // returns an empty list if there's an error
+        case Right(nel) => nel.toList
       }
     }
-  }
 
   def current(contact: ContactId)(implicit logPrefix: LogPrefix): M[List[Subscription]] =
     subscriptionsForContact(SubscriptionTransform.getCurrentSubscriptions)(contact)
@@ -153,38 +141,34 @@ class SubscriptionService[M[_]: Monad](pids: ProductIds, futureCatalog: => M[Cat
       contact: ContactId,
       today: LocalDate = LocalDate.now(),
       lastNMonths: Int = 3, // cancelled in the last N months
-  )(implicit logPrefix: LogPrefix): M[String \/ List[Subscription]] = {
+  )(implicit logPrefix: LogPrefix): M[String \/ List[Subscription]] =
     (for {
       catalog <- EitherT(futureCatalog.map(\/.right[String, CatalogMap]))
       jsonSubs <- EitherT(jsonSubscriptionsFromContact(contact))
-      subs <- EitherT(Monad[M].pure(getRecentlyCancelledSubscriptions(today, lastNMonths, catalog, pids, jsonSubs)))
+      subs <- EitherT(Monad[M].pure(SubscriptionTransform.getRecentlyCancelledSubscriptions(today, lastNMonths, catalog, pids, jsonSubs)))
     } yield subs).run
-  }
 
   def subscriptionsForAccountId(
       accountId: AccountId,
-  )(implicit logPrefix: LogPrefix): M[Disjunction[String, List[Subscription]]] = {
-    val subsAsJson = jsonSubscriptionsFromAccount(accountId)
-
-    subsAsJson.flatMap { subJsonsEither =>
-      futureCatalog.map { catalog =>
-        subJsonsEither.rightMap { subJsons =>
-          SubscriptionTransform.getCurrentSubscriptions(catalog, pids)(subJsons).toList.flatMap(_.list.toList)
-        }
+  )(implicit logPrefix: LogPrefix): M[Disjunction[String, List[Subscription]]] =
+    for {
+      subJsonsEither <- jsonSubscriptionsFromAccount(accountId)
+      catalog <- futureCatalog
+    } yield {
+      subJsonsEither.rightMap { subJsons =>
+        SubscriptionTransform.getCurrentSubscriptions(catalog, pids)(subJsons).toList.flatMap(_.list.toList)
       }
     }
-  }
 
-  def jsonSubscriptionsFromContact(contact: ContactId)(implicit logPrefix: LogPrefix): M[Disjunction[String, List[JsValue]]] = {
+  private def jsonSubscriptionsFromContact(contact: ContactId)(implicit logPrefix: LogPrefix): M[Disjunction[String, List[JsValue]]] =
     (for {
       account <- ListT[EitherTM, AccountId](
         EitherT[String, M, IList[AccountId]](soap.getAccountIds(contact).map(l => \/.r[String](IList.fromSeq(l)))),
       )
       subJson <- ListT[EitherTM, JsValue](EitherT(jsonSubscriptionsFromAccount(account)).map(IList.fromSeq))
     } yield subJson).toList.run
-  }
 
-  def jsonSubscriptionsFromAccount(accountId: AccountId)(implicit logPrefix: LogPrefix): M[Disjunction[String, List[JsValue]]] =
+  private def jsonSubscriptionsFromAccount(accountId: AccountId)(implicit logPrefix: LogPrefix): M[Disjunction[String, List[JsValue]]] =
     rest.get[List[JsValue]](s"subscriptions/accounts/${accountId.get}")(multiSubJsonReads, implicitly)
 
   /** fetched with /v1/subscription/{key}?charge-detail=current-segment which zeroes out all the non-active charges

membership-common/src/main/scala/com/gu/memsub/subsv2/services/SubscriptionTransform.scala

@@ -91,11 +91,10 @@ object SubscriptionTransform extends SafeLogging {
   def getSubscription(
       catalog: CatalogMap,
       pids: ProductIds,
-      now: () => LocalDate = LocalDate.now, /*now only needed for pending friend downgrade*/
   )(subJson: JsValue): Disjunction[String, Subscription] = {
     import Trace.Traceable
     val planToSubscriptionFunction =
-      subscriptionReads(now()).reads(subJson).asEither.toDisjunction.leftMap(_.mkString(" ")).withTrace("planToSubscriptionFunction")
+      subscriptionReads.reads(subJson).asEither.toDisjunction.leftMap(_.mkString(" ")).withTrace("planToSubscriptionFunction")
 
     val lowLevelPlans = subJson
       .validate[List[SubscriptionZuoraPlan]](subZuoraPlanListReads)

membership-common/src/main/scala/com/gu/services/model/PaymentDetails.scala

@@ -18,7 +18,6 @@ case class PaymentDetails(
     nextPaymentDate: Option[LocalDate],
     remainingTrialLength: Int,
     pendingCancellation: Boolean,
-    pendingAmendment: Boolean,
     paymentMethod: Option[PaymentMethod],
     plan: PersonalPlan,
 )
@@ -47,7 +46,6 @@ object PaymentDetails {
       lastPaymentDate = lastPaymentDate,
       nextPaymentDate = nextPayment.map(_.date),
       termEndDate = sub.termEndDate,
-      pendingAmendment = sub.hasPendingFreePlan,
       paymentMethod = paymentMethod,
       plan = PersonalPlan.paid(sub),
       subscriberId = sub.name.get,

membership-common/src/test/scala/com/gu/memsub/subsv2/reads/SubReadsTest.scala

@@ -2,7 +2,7 @@ package com.gu.memsub.subsv2.reads
 
 import com.gu.i18n.Currency._
 import com.gu.lib.DateDSL._
-import com.gu.memsub.Benefit.{Partner, Weekly}
+import com.gu.memsub.Benefit.Weekly
 import com.gu.memsub.BillingPeriod.Quarter
 import com.gu.memsub.Product.WeeklyDomestic
 import com.gu.memsub.Subscription.{ProductRatePlanChargeId, ProductRatePlanId, RatePlanId, SubscriptionRatePlanChargeId}
@@ -79,7 +79,7 @@ class SubReadsTest extends Specification {
         end = now,
       )
       val subscription =
-        SubJsonReads.subscriptionReads(now).reads(json).get(NonEmptyList(plan))
+        SubJsonReads.subscriptionReads.reads(json).get(NonEmptyList(plan))
 
       subscription.readerType mustEqual Patron
     }

membership-common/src/test/scala/com/gu/memsub/subsv2/services/SubscriptionServiceTest.scala

@@ -273,7 +273,6 @@ class SubscriptionServiceTest extends Specification {
         casActivationDate = None,
         promoCode = None,
         isCancelled = isCancelled,
-        hasPendingFreePlan = false,
         plans = CovariantNonEmptyList(plans.head, plans.tail.toList),
         readerType = ReaderType.Direct,
         gifteeIdentityId = None,

membership-common/src/test/scala/com/gu/memsub/subsv2/services/SubscriptionTransformTest.scala

@@ -68,7 +68,7 @@ class SubscriptionTransformTest extends AnyFlatSpec {
 
     val json: JsValue = Resource.getJson("rest/plans/WeeklyOneYear.json")
     val result: Disjunction[String, V2Subscription] =
-      SubscriptionTransform.getSubscription(cat, Fixtures.productIds, () => 3 May 2017)(json)
+      SubscriptionTransform.getSubscription(cat, Fixtures.productIds)(json)
     val expected: String \/ V2Subscription = \/-(
       V2Subscription(
         id = Id("2c92c0f85bae511e015bcead968f69e0"),
@@ -81,7 +81,6 @@ class SubscriptionTransformTest extends AnyFlatSpec {
         casActivationDate = None,
         promoCode = None,
         isCancelled = false,
-        hasPendingFreePlan = false,
         plans = CovariantNonEmptyList(
           SubscriptionPlan(
             id = RatePlanId("2c92c0f85bae511e015bcead96a569e5"),
@@ -118,7 +117,7 @@ class SubscriptionTransformTest extends AnyFlatSpec {
 
     val json: JsValue = Resource.getJson("rest/plans/WeeklySixMonths.json")
     val result: Disjunction[String, V2Subscription] =
-      SubscriptionTransform.getSubscription(cat, Fixtures.productIds, () => 3 May 2017)(json)
+      SubscriptionTransform.getSubscription(cat, Fixtures.productIds)(json)
 
     val expected: Disjunction[String, V2Subscription] = \/-(
       V2Subscription(
@@ -132,7 +131,6 @@ class SubscriptionTransformTest extends AnyFlatSpec {
         casActivationDate = None,
         promoCode = None,
         isCancelled = false,
-        hasPendingFreePlan = false,
         plans = CovariantNonEmptyList(
           SubscriptionPlan(
             id = RatePlanId("2c92c0f95bae6218015bceaf24520fa9"),
@@ -170,7 +168,7 @@ class SubscriptionTransformTest extends AnyFlatSpec {
 
     val json: JsValue = Resource.getJson("rest/plans/WeeklySixMonthly.json")
     val result: Disjunction[String, V2Subscription] =
-      SubscriptionTransform.getSubscription(cat, Fixtures.productIds, () => 3 May 2017)(json)
+      SubscriptionTransform.getSubscription(cat, Fixtures.productIds)(json)
     val expected: Disjunction[String, V2Subscription] = \/-(
       V2Subscription(
         id = Id("2c92c0f85be67835015be8f374217f86"),
@@ -183,7 +181,6 @@ class SubscriptionTransformTest extends AnyFlatSpec {
         casActivationDate = None,
         promoCode = None,
         isCancelled = false,
-        hasPendingFreePlan = false,
         plans = CovariantNonEmptyList(
           SubscriptionPlan(
             id = RatePlanId("2c92c0f85be67835015be8f3743a7f8e"),