Fix RevenueCatUI API issues and add API tests #1433
Conversation
| offering: Offering? = null, | ||
| fontProvider: ParcelizableFontProvider? = null, | ||
| requiredEntitlementIdentifier: String, |
There was a problem hiding this comment.
Honestly I wasn't sure about doing this now since it's a breaking change, but I think it's better now that the API is still experimental. If we don't put this mandatory parameter first, users are forced to use named parameters if they only want to set the requiredEntitlementIdentifier but not the optional parameters.
Lmk if you would prefer me to revert it.
There was a problem hiding this comment.
Yes, better now than never! I don't think there are that many users yet so let's fix these things while we have time.
tonidero
force-pushed
the
add-revenuecatui-api-tests
branch
from
3452e7a to
ada223a
Compare
| @@ -17,7 +17,7 @@ android { | |||
| } | |||
|
|
|||
| defaultConfig { | |||
| minSdkVersion 21 // Compose requires minSdkVersion 21 | |||
| minSdkVersion 24 // RevenueCat UI requires 24 | |||
There was a problem hiding this comment.
Had to update the api tests in order to test the revenuecatui module. I think it should be fine, since our API shouldn't depend on the sdk version
There was a problem hiding this comment.
Hmm but this helped guarantee that we didn't break this compatibility. Should we create a separate module for revenuecatui API tests?
There was a problem hiding this comment.
Hmm I see your point, but it's difficult to change the minSdkVersion accidentally 🤞, and the main sdk shouldn't compile already if there is any issue that would be caught by the API tester. So I think it's ok to keep it in the same module? cc @vegaro. In case you have other thoughts.
| */ | ||
| @TypeParceler<FontStyle, FontStyleParceler>() | ||
| val fontStyle: FontStyle = FontStyle.Normal, | ||
| val fontStyle: Int = FontStyle.Normal.value, |
There was a problem hiding this comment.
This is another breaking change in kotlin... Basically I'm doing this since it made it impossible to create instances of these classes in Java, since FontStyle is not available in Java.
tonidero
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1433 +/- ##
=======================================
Coverage 84.13% 84.13%
=======================================
Files 197 197
Lines 6649 6649
Branches 965 965
=======================================
Hits 5594 5594
Misses 684 684
Partials 371 371 ☔ View full report in Codecov by Sentry. |
| @@ -17,7 +17,7 @@ android { | |||
| } | |||
|
|
|||
| defaultConfig { | |||
| minSdkVersion 21 // Compose requires minSdkVersion 21 | |||
| minSdkVersion 24 // RevenueCat UI requires 24 | |||
There was a problem hiding this comment.
Hmm but this helped guarantee that we didn't break this compatibility. Should we create a separate module for revenuecatui API tests?
| offering: Offering? = null, | ||
| fontProvider: ParcelizableFontProvider? = null, | ||
| requiredEntitlementIdentifier: String, |
There was a problem hiding this comment.
Yes, better now than never! I don't think there are that many users yet so let's fix these things while we have time.
**This is an automatic release.** ### New Features * `StoreProduct`: new `pricePerWeek` and `pricePerYear` (#1426) via NachoSoto (@NachoSoto) ### RevenueCatUI * Fix RevenueCatUI API issues and add API tests (#1433) via Toni Rico (@tonidero) * Paywalls: Add initial snapshot testings for RevenueCatUI library (#1432) via Toni Rico (@tonidero) * `Paywalls`: new `{{ sub_price_per_week }}` variable (#1427) via NachoSoto (@NachoSoto) * `Paywalls`: new `{{ sub_relative_discount }}` variable (#1425) via NachoSoto (@NachoSoto) ### Dependency Updates * Bump fastlane-plugin-revenuecat_internal from `a297205` to `0ddee10` (#1431) via dependabot[bot] (@dependabot[bot]) ### Other Changes * `Offering`: restore constructor with no `PaywallData` (#1437) via NachoSoto (@NachoSoto) Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Description
This adds API tests for the paywalls module. In the process of writing those API tests I found 2 issues that I wanted to get fixed while the API is experimental:
requiredEntitlementIdentifierparameter in thePaywallActivityLauncher.launchIfNeededmethod was after some optional parameters, making it mandatory to use named parameters if you didn't want to pass the other optional parameters.PaywallFontsince it was using value classes that are not available in Java.