Paywalls: Add support for sub_period variable
#1264
Conversation
| R.string.annual -> "Annual" | ||
| R.string.semester -> "6 month" | ||
| R.string.quarter -> "3 month" | ||
| R.string.bimonthly -> "2 month" |
There was a problem hiding this comment.
I copied the strings from iOS. Though it felt a bit weird to not make this plural
| PackageType.WEEKLY -> R.string.weekly | ||
| PackageType.UNKNOWN, PackageType.CUSTOM -> null | ||
| } | ||
| return stringId?.let { applicationContext.getString(it) } ?: "" |
There was a problem hiding this comment.
I'm defaulting to an empty string, which I think is the same we do in iOS.
| PackageType.ANNUAL -> "Annual" | ||
| PackageType.MONTHLY -> "Monthly" | ||
| PackageType.WEEKLY -> "Weekly" | ||
| else -> error("Unknown package type ${rcPackage.packageType}") |
There was a problem hiding this comment.
I didn't like copying this here... But I thought it was better to kind of duplicate this logic than use the real code during the tests.
| @Test | ||
| fun `process variables processes custom period`() { | ||
| rcPackage = TestData.Packages.annual.copy(packageType = PackageType.CUSTOM) | ||
| expectVariablesResult("{{ sub_period }}", "") |
There was a problem hiding this comment.
Just want to confirm this is what we expect @NachoSoto
There was a problem hiding this comment.
Yeah 👍🏻 I don't have a test for this in iOS but this is fine.
The idea is to have validations in the frontend / backend to prevent users from using these with custom pacakges.
| @@ -13,45 +15,131 @@ import java.util.Locale | |||
| @RunWith(AndroidJUnit4::class) | |||
| class VariableProcessorTest { | |||
|
|
|||
| private val usLocale = Locale.US | |||
| private val esLocale = Locale("es", "ES") | |||
There was a problem hiding this comment.
Cool 👍🏻
Don't have to do it now, but I guess we're close to adding Spanish strings and testing them? :)
There was a problem hiding this comment.
Yeah, I can probably add them tomorrow morning so we can test it. However, note that since we are mocking the application context, we won't be using the localized strings. However, this can still be useful to test formatters and such
There was a problem hiding this comment.
I mean, we could potentially run instrumentation tests (that run on an actual device), but I'm not sure it's worth it for this since we need to run them on firebase test lab and are much slower.
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## paywalls #1264 +/- ##
=========================================
Coverage 85.46% 85.46%
=========================================
Files 189 189
Lines 6376 6376
Branches 929 929
=========================================
Hits 5449 5449
Misses 568 568
Partials 359 359 ☔ View full report in Codecov by Sentry. |
tonidero
force-pushed
the
toniricodiez/pwl-216-sub_period
branch
from
f6388a8 to
a42abfe
Compare
### Description This PR adds support for the `sub_period` variable. In order to do that, some refactors were made to make sure we could test the result.
Description
This PR adds support for the
sub_periodvariable. In order to do that, some refactors were made to make sure we could test the result.