-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds pricePerPeriod functions to PricingPhase #1789
Changes from all commits
4dede13
7dbcf15
b6d625e
0fe59ba
3a08ee0
63d8209
46b8d7a
5947b8b
155105c
605d09a
942a859
be7ec19
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,8 @@ package com.revenuecat.purchases.models | |
|
||
import android.os.Parcelable | ||
import com.revenuecat.purchases.utils.pricePerMonth | ||
import com.revenuecat.purchases.utils.pricePerWeek | ||
import com.revenuecat.purchases.utils.pricePerYear | ||
import kotlinx.parcelize.Parcelize | ||
import java.util.Locale | ||
|
||
|
@@ -54,14 +56,54 @@ data class PricingPhase( | |
} | ||
} | ||
|
||
/** | ||
* Gives the price of the [PricingPhase] in the given locale in a weekly recurrence. This means that for example, | ||
* if the period is monthly, the price will be divided by 4. It uses a currency formatter to format the price in | ||
* the given locale. Note that this value may be an approximation. | ||
* @param locale Locale to use for formatting the price. Default is the system default locale. | ||
*/ | ||
@JvmOverloads | ||
fun pricePerWeek(locale: Locale = Locale.getDefault()): Price = | ||
price.pricePerWeek(billingPeriod, locale) | ||
|
||
/** | ||
* Gives the price of the [PricingPhase] in the given locale in a monthly recurrence. This means that for example, | ||
* if the period is annual, the price will be divided by 12. It uses a currency formatter to format the price in | ||
* the given locale. Note that this value may be an approximation. | ||
* @param locale Locale to use for formatting the price. Default is the system default locale. | ||
*/ | ||
@JvmOverloads | ||
fun pricePerMonth(locale: Locale = Locale.getDefault()): Price = | ||
price.pricePerMonth(billingPeriod, locale) | ||
|
||
/** | ||
* Gives the price of the [PricingPhase] in the given locale in a yearly recurrence. This means that for example, | ||
* if the period is monthly, the price will be multiplied by 12. It uses a currency formatter to format the price in | ||
* the given locale. Note that this value may be an approximation. | ||
* @param locale Locale to use for formatting the price. Default is the system default locale. | ||
*/ | ||
@JvmOverloads | ||
fun pricePerYear(locale: Locale = Locale.getDefault()): Price = | ||
price.pricePerYear(billingPeriod, locale) | ||
|
||
/** | ||
* Gives the price of the [PricingPhase] in the given locale in a monthly recurrence. This means that for example, | ||
* if the period is annual, the price will be divided by 12. It uses a currency formatter to format the price in | ||
* the given locale. Note that this value may be an approximation. | ||
* | ||
* This is equivalent to: | ||
* ```kotlin | ||
* pricePerMonth(locale).formatted | ||
* ``` | ||
* | ||
* @param locale Locale to use for formatting the price. Default is the system default locale. | ||
*/ | ||
@Deprecated( | ||
message = "pricePerMonth() provides more price info", | ||
replaceWith = ReplaceWith("pricePerMonth(locale).formatted"), | ||
) | ||
@JvmOverloads | ||
fun formattedPriceInMonths(locale: Locale = Locale.getDefault()): String { | ||
return price.pricePerMonth(billingPeriod, locale).formatted | ||
return pricePerMonth(locale).formatted | ||
Comment on lines
106
to
+107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we deprecate There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I agree with that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright, done: 5947b8b |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
package com.revenuecat.purchases.common.models | ||
|
||
import com.revenuecat.purchases.models.Period | ||
import com.revenuecat.purchases.models.Price | ||
import com.revenuecat.purchases.models.PricingPhase | ||
import com.revenuecat.purchases.models.RecurrenceMode | ||
import org.assertj.core.api.Assertions.assertThat | ||
import org.junit.Test | ||
import org.junit.runner.RunWith | ||
import org.junit.runners.Parameterized | ||
import java.util.Locale | ||
|
||
@RunWith(Parameterized::class) | ||
class ParameterizedPricingPhaseTest( | ||
private val billingPeriodIso8601: String, | ||
private val expected: Expected, | ||
) { | ||
|
||
class Expected( | ||
val weekly: Price, | ||
val monthly: Price, | ||
val yearly: Price, | ||
) | ||
|
||
companion object { | ||
|
||
private val BASE_PRICE = Price( | ||
formatted = "$99.99", | ||
amountMicros = 99990000, | ||
currencyCode = "USD", | ||
) | ||
|
||
@Suppress("LongMethod") | ||
@JvmStatic | ||
@Parameterized.Parameters(name = "{0}") | ||
fun parameters(): Collection<*> = listOf( | ||
arrayOf( | ||
"P1W", | ||
Expected( | ||
weekly = BASE_PRICE, | ||
monthly = Price( | ||
formatted = "$434.48", | ||
amountMicros = 434480357, | ||
currencyCode = "USD" | ||
), | ||
yearly = Price( | ||
formatted = "$5,213.76", | ||
amountMicros = 5213764285, | ||
currencyCode = "USD", | ||
) | ||
) | ||
), | ||
arrayOf( | ||
"P1M", | ||
Expected( | ||
weekly = Price( | ||
formatted = "$23.01", | ||
amountMicros = 23011397, | ||
currencyCode = "USD", | ||
), | ||
monthly = BASE_PRICE, | ||
yearly = Price( | ||
formatted = "$1,199.88", | ||
amountMicros = 1199880000, | ||
currencyCode = "USD", | ||
) | ||
) | ||
), | ||
arrayOf( | ||
"P1Y", | ||
Expected( | ||
weekly = Price( | ||
formatted = "$1.92", | ||
amountMicros = 1917616, | ||
currencyCode = "USD", | ||
), | ||
monthly = Price( | ||
formatted = "$8.33", | ||
amountMicros = 8332500, | ||
currencyCode = "USD", | ||
), | ||
yearly = BASE_PRICE | ||
) | ||
), | ||
) | ||
} | ||
|
||
@Suppress("DEPRECATION") | ||
@Test | ||
fun `should correctly calculate periodic prices for billing period`() { | ||
// Arrange | ||
val phase = PricingPhase( | ||
billingPeriod = Period.create(billingPeriodIso8601), | ||
recurrenceMode = RecurrenceMode.FINITE_RECURRING, | ||
billingCycleCount = 2, | ||
price = BASE_PRICE | ||
) | ||
|
||
// Act | ||
val actualWeekly = phase.pricePerWeek(Locale.US) | ||
val actualMonthly = phase.pricePerMonth(Locale.US) | ||
val actualYearly = phase.pricePerYear(Locale.US) | ||
val actualWeeklyNoLocale = phase.pricePerWeek() | ||
val actualMonthlyNoLocale = phase.pricePerMonth() | ||
val actualYearlyNoLocale = phase.pricePerYear() | ||
val actualMonthlyFormatted = phase.formattedPriceInMonths(Locale.US) | ||
val actualMonthlyFormattedNoLocale = phase.formattedPriceInMonths() | ||
|
||
// Assert | ||
assertThat(actualWeekly).isEqualTo(expected.weekly) | ||
assertThat(actualMonthly).isEqualTo(expected.monthly) | ||
assertThat(actualYearly).isEqualTo(expected.yearly) | ||
assertThat(actualWeeklyNoLocale.amountMicros).isEqualTo(expected.weekly.amountMicros) | ||
assertThat(actualMonthlyNoLocale.amountMicros).isEqualTo(expected.monthly.amountMicros) | ||
assertThat(actualYearlyNoLocale.amountMicros).isEqualTo(expected.yearly.amountMicros) | ||
assertThat(actualMonthlyFormatted).isEqualTo(expected.monthly.formatted) | ||
// Hard to test anything else if the locale is not deterministic, besides formatting exactly like the | ||
// implementation does. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess we could mock the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes, true! I don't think it's super important either though, because this is deprecated, and the implementation it calls is fully tested. |
||
assertThat(actualMonthlyFormattedNoLocale).isNotBlank() | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised codecov says these are not tested when we are adding tests in this PR 🤔. Not sure what's happening though... Any ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's very weird. I'm investigating. 🔍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figured it out! I wasn't testing the overloads without
Locale
! 46b8d7aThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohhh makes sense. Thanks for debugging that!