Feature/iap payment flow #8053
Feature/iap payment flow #8053
Conversation
JorgeMucientes
force-pushed
the
feature/iap-payment-flow
branch
from
113f892 to
f270124
Compare
You can test the changes on this Pull Request by downloading an installable build, or scanning this QR code:
|
…-flow # Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/di/InAppPurchasesModule.kt # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/login/storecreation/iap/IapEligibilityViewModel.kt
JorgeMucientes
removed
the
status: do not merge
| init { | ||
| loadPlan() | ||
| trackStep(VALUE_STEP_PLAN_PURCHASE) | ||
| } | ||
|
|
||
| fun setIAPActivityWrapper(wrapper: IAPActivityWrapper) { | ||
| iapActivityWrapper = wrapper |
There was a problem hiding this comment.
Won't it be a memory leak when the activity is recreated?
There was a problem hiding this comment.
Good point! I've completely missed that. Fixed :)
There was a problem hiding this comment.
The way you've done probably will work, just wondering why just not pass it as a method param
fun onConfirmClicked(wrapper: IAPActivityWrapper) {
Seems that it will be the more straightforward approach
There was a problem hiding this comment.
I remember attempting that approach first. I had some code like:
val activity = LocalContext.current.getActivity()
fun Context.getActivity(): AppCompatActivity? = when (this) {
is AppCompatActivity -> this
is ContextWrapper -> baseContext.getActivity()
else -> null
}In one of the UI @Composable functions. But somehow I kept getting crashes because the retrieved activity was not AppCompatActivity. I'll revisit this again, to double check if I can find the way to make it work, as I agree that passing that as parameter is way more straightforward.
There was a problem hiding this comment.
That's weird. I briefly tried that and it worked fine for
There was a problem hiding this comment.
I think in my previous attempts I was trying to directly cast LocalContext.current as AppCompatActivity resulting in a crash because LocalContext.current is actually a FragmentContextWrapper. The key here is in the line is ContextWrapper -> baseContext.getActivity() which I don't think I had in my original implementation.
In any case, with the latest changes this is solved in a more explicit manner.
...merce/src/main/kotlin/com/woocommerce/android/ui/login/storecreation/plans/PlansViewModel.kt
Show resolved
Hide resolved
...merce/src/main/kotlin/com/woocommerce/android/ui/login/storecreation/plans/PlansViewModel.kt
Outdated
Show resolved
Hide resolved
...merce/src/main/kotlin/com/woocommerce/android/ui/login/storecreation/plans/PlansViewModel.kt
Show resolved
Hide resolved
...merce/src/main/kotlin/com/woocommerce/android/ui/login/storecreation/plans/PlansViewModel.kt
Outdated
Show resolved
Hide resolved
...merce/src/main/kotlin/com/woocommerce/android/ui/login/storecreation/plans/PlansViewModel.kt
Outdated
Show resolved
Hide resolved
Codecov ReportBase: 43.02% // Head: 43.17% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## trunk #8053 +/- ##
============================================
+ Coverage 43.02% 43.17% +0.15%
- Complexity 3522 3537 +15
============================================
Files 679 681 +2
Lines 36956 37015 +59
Branches 4790 4798 +8
============================================
+ Hits 15901 15983 +82
+ Misses 19629 19602 -27
- Partials 1426 1430 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| val reason = if (errorType is IAPError.RemoteCommunication.Server) { | ||
| errorType.reason | ||
| } else "" | ||
| WooLog.i(WooLog.T.IAP, "Error processing eCommerce plan: $errorType reason: $reason") |
There was a problem hiding this comment.
np: probably this logging here is redundant, we log what we send to the tracks anyway:
2022-12-27 11:07:09.460 5998-5998 WooCommerce-IAP com.woocommerce.android I Error processing eCommerce plan: UserCancelled reason:
2022-12-27 11:07:09.462 5998-5998 WooCommerce-UTILS com.woocommerce.android I 🔵 Tracked: site_creation_iap_purchase_error, Properties: {"error_type":"UserCancelled","error_description":"","is_debug":true}
|
Thank you for the careful review @kidinov! 🙏🏼 |
|
Hey @kidinov! I think the PR is ready for the last round review! I was finally able to get rid of the lifecycle callbacks in I've also created an issue to address the error handling when fetching IAP product details fails Once again thanks for the careful PR review 🙏🏼 |
|
I'll be AFK the next week. Since this PR was already approved by @kidinov and the only changes I've added were minor improvement after the latest suggestions I'll proceed with merging. Once again thank you for the detailed review and the great suggestions. |
Second part of: #7925
Description
Provides integration with In App Purchases (IAP) to purchase the eCommerce plan when creating a new store.
The flow works like this:
Testing instructions
Follow these instructions: PdfdoF-1Ej-p2 (Android section) to be able to use IAP. A Google Play US account is required for this.
Images/gif
Android_IAP_demot.mp4