Improve price per month accuracy for weekly subscriptions#1504
Conversation
|
looks like snapshots are failing but I can't see them, I imagine we had a value that was calculated off of the weekly in the snapshot? |
| class PeriodTest { | ||
| companion object { | ||
| private val MAX_OFFSET = Offset.offset(0.0000001) | ||
| private val MAX_OFFSET = Offset.offset(0.0001) |
There was a problem hiding this comment.
don't love reducing accuracy, but having a value this precise would probably risk our tests failing from floating point precision errors
vegaro
left a comment
There was a problem hiding this comment.
Makes sense! Just that little question
| private const val DAYS_PER_MONTH = 30.0 | ||
| private const val DAYS_PER_YEAR = 365.0 | ||
| private const val WEEKS_PER_MONTH = 4.0 | ||
| private const val WEEKS_PER_YEAR = 52.14 |
There was a problem hiding this comment.
Should we also calculate this one so we have the same precision?
There was a problem hiding this comment.
I was shooting for "least amount of changes possible" but we could.
It'd be nice to update them all really to also take into account leap years nvmd not sure how that'd even work since this is a "per year" value not "specifically this year" value
There was a problem hiding this comment.
Will make that change separately.
@aboedo I generated snapshots here and noticed that the weekly price was actually more expensive than the others with this new calculation, which caused a changed in the discount percentages. I thought to merge the new snapshots, but for now I decided to slightly lower the price of the weekly product so it's not more expensive than the monthly one. I pushed that change in this branch. Lmk if you would prefer to update the snapshots and we can do that. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1504 +/- ##
=======================================
Coverage 84.46% 84.46%
=======================================
Files 217 217
Lines 7196 7196
Branches 1004 1004
=======================================
Hits 6078 6078
Misses 730 730
Partials 388 388 ☔ View full report in Codecov by Sentry. |
### Description Followup to #1504 This slightly improves precision when calculating price per year for weekly subscriptions.
**This is an automatic release.** ### RevenueCatUI * Paywalls: Add `PaywallFooterView` (#1509) via Toni Rico (@tonidero) * Paywalls: Remove `PaywallActivity` theme to pickup application's theme by default (#1511) via Toni Rico (@tonidero) * Paywalls: Auto-close paywall activity if restore grants required entitlement identifier (#1507) via Toni Rico (@tonidero) ### Bugfixes * Improve pricePerYear price calculation precision (#1515) via Toni Rico (@tonidero) * Improve price per month accuracy for weekly subscriptions (#1504) via Andy Boedo (@aboedo) ### Dependency Updates * Bump danger from 9.4.0 to 9.4.1 (#1512) via dependabot[bot] (@dependabot[bot]) ### Other Changes * Remove unnecessary appInBackground parameters (#1508) via Cesar de la Vega (@vegaro) * Create `PurchasesStateProvider` (#1502) via Cesar de la Vega (@vegaro) Co-authored-by: revenuecat-ops <ops@revenuecat.com>
We were calculating the price per month for weekly products as price * 4, but there are on average 4.345 weeks in a month, so this updates the value accordingly.