Apply frj PR 417 comments

guardian · Sep 19, 2019 · 8006ff5 · 8006ff5
1 parent c171df5
commit 8006ff5
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 36 deletions.
diff --git a/lib/holiday-stops/src/main/scala/com/gu/holiday_stops/CurrentSundayVoucherSubscription.scala b/lib/holiday-stops/src/main/scala/com/gu/holiday_stops/CurrentSundayVoucherSubscription.scala
@@ -1,8 +1,7 @@
 package com.gu.holiday_stops
 
 import java.time.temporal.ChronoUnit
-
-import scala.util.Try
+import cats.implicits._
 
 object CurrentSundayVoucherSubscriptionPredicate {
   def ratePlanIsSundayVoucher(ratePlan: RatePlan, sundayVoucherProductRatePlanChargeId: String): Boolean =
@@ -11,11 +10,12 @@ object CurrentSundayVoucherSubscriptionPredicate {
     }
 
   def sundayVoucherRatePlanHasBeenInvoicedForOneMonth(ratePlan: RatePlan): Boolean = {
-    Try {
-      val fromInclusive = ratePlan.ratePlanCharges.head.processedThroughDate.get
-      val toExclusive = ratePlan.ratePlanCharges.head.chargedThroughDate.get
-      toExclusive.isAfter(fromInclusive) && ChronoUnit.MONTHS.between(fromInclusive, toExclusive) == 1
-    }.getOrElse(false)
+    (
+      ratePlan.ratePlanCharges.headOption.flatMap(_.processedThroughDate),
+      ratePlan.ratePlanCharges.headOption.flatMap(_.chargedThroughDate)
+    ).mapN { (fromInclusive, toExclusive) =>
+        toExclusive.isAfter(fromInclusive) && ChronoUnit.MONTHS.between(fromInclusive, toExclusive) == 1
+      }.getOrElse(false)
   }
 
   def sundayVoucherRatePlanHasExactlyOneCharge(ratePlan: RatePlan): Boolean = (ratePlan.ratePlanCharges.size == 1)
@@ -58,23 +58,25 @@ object CurrentSundayVoucherSubscription {
     sundayVoucherProductRatePlanChargeId: String
   ): Either[ZuoraHolidayWriteError, CurrentSundayVoucherSubscription] = {
 
-    findSundayVoucherRatePlan(subscription, sundayVoucherProductRatePlanChargeId)
-      .map { currentSundayVoucherRatePlan => // these ugly gets are safe due to above conditions
-        val currentSundayVoucherRatePlanRatePlanCharge = currentSundayVoucherRatePlan.ratePlanCharges.head
-        new CurrentSundayVoucherSubscription(
-          subscriptionNumber = subscription.subscriptionNumber,
-          billingPeriod = currentSundayVoucherRatePlanRatePlanCharge.billingPeriod.get,
-          price = currentSundayVoucherRatePlanRatePlanCharge.price,
-          invoicedPeriod = CurrentInvoicedPeriod(
-            startDateIncluding = currentSundayVoucherRatePlanRatePlanCharge.processedThroughDate.get,
-            endDateExcluding = currentSundayVoucherRatePlanRatePlanCharge.chargedThroughDate.get
-          ),
-          ratePlanId = currentSundayVoucherRatePlan.id,
-          productRatePlanId = currentSundayVoucherRatePlan.productRatePlanId,
-          productRatePlanChargeId = currentSundayVoucherRatePlanRatePlanCharge.productRatePlanChargeId
-        )
-      }
-      .toRight(ZuoraHolidayWriteError(s"Failed to determine Sunday Voucher Newspaper Guardian rate plan: $subscription"))
+    findSundayVoucherRatePlan(subscription, sundayVoucherProductRatePlanChargeId).flatMap { currentSundayVoucherRatePlan =>
+      for {
+        currentSundayVoucherRatePlanRatePlanCharge <- currentSundayVoucherRatePlan.ratePlanCharges.headOption
+        billingPeriod <- currentSundayVoucherRatePlanRatePlanCharge.billingPeriod
+        startDateIncluding <- currentSundayVoucherRatePlanRatePlanCharge.processedThroughDate
+        endDateExcluding <- currentSundayVoucherRatePlanRatePlanCharge.chargedThroughDate
+      } yield new CurrentSundayVoucherSubscription(
+        subscriptionNumber = subscription.subscriptionNumber,
+        billingPeriod = billingPeriod,
+        price = currentSundayVoucherRatePlanRatePlanCharge.price,
+        invoicedPeriod = CurrentInvoicedPeriod(
+          startDateIncluding = startDateIncluding,
+          endDateExcluding = endDateExcluding
+        ),
+        ratePlanId = currentSundayVoucherRatePlan.id,
+        productRatePlanId = currentSundayVoucherRatePlan.productRatePlanId,
+        productRatePlanChargeId = currentSundayVoucherRatePlanRatePlanCharge.productRatePlanChargeId
+      )
+    }.toRight(ZuoraHolidayWriteError(s"Failed to determine Sunday Voucher Newspaper Guardian rate plan: $subscription"))
 
   }
 }
diff --git a/lib/holiday-stops/src/test/scala/com/gu/holiday_stops/ActionCalculatorTest.scala b/lib/holiday-stops/src/test/scala/com/gu/holiday_stops/ActionCalculatorTest.scala
@@ -133,23 +133,23 @@ class ActionCalculatorTest extends FlatSpec with Matchers with EitherValues {
       toInclusive = LocalDate.of(2019, 6, 22),
       productRatePlanKey = sundayVoucherProductRatePlanKey
     ) shouldEqual Right(List(
-      LocalDate.of(2019, 5, 26),
-      LocalDate.of(2019, 6, 2),
-      LocalDate.of(2019, 6, 9),
-      LocalDate.of(2019, 6, 16)
-    ))
+        LocalDate.of(2019, 5, 26),
+        LocalDate.of(2019, 6, 2),
+        LocalDate.of(2019, 6, 9),
+        LocalDate.of(2019, 6, 16)
+      ))
 
     ActionCalculator.publicationDatesToBeStopped(
       fromInclusive = LocalDate.of(2019, 5, 20),
       toInclusive = LocalDate.of(2019, 6, 23),
       productRatePlanKey = sundayVoucherProductRatePlanKey
     ) shouldEqual Right(List(
-      LocalDate.of(2019, 5, 26),
-      LocalDate.of(2019, 6, 2),
-      LocalDate.of(2019, 6, 9),
-      LocalDate.of(2019, 6, 16),
-      LocalDate.of(2019, 6, 23)
-    ))
+        LocalDate.of(2019, 5, 26),
+        LocalDate.of(2019, 6, 2),
+        LocalDate.of(2019, 6, 9),
+        LocalDate.of(2019, 6, 16),
+        LocalDate.of(2019, 6, 23)
+      ))
   }
   it should "return an error for an unsupported product rate plan" in {
     val unsupportedProductRatePlanKey =
@@ -159,6 +159,6 @@ class ActionCalculatorTest extends FlatSpec with Matchers with EitherValues {
       fromInclusive = LocalDate.of(2019, 5, 20),
       toInclusive = LocalDate.of(2019, 6, 22),
       productRatePlanKey = unsupportedProductRatePlanKey
-    ) should be ('left)
+    ) should be('left)
   }
 }