Feature/8182 iap single subcription requirement #8206

JorgeMucientes · 2023-01-17T16:41:18Z

Description

Adds a check before triggering store creation flow, to verify that the user doesn't have an active IAP subscription to WooCommerce.

Testing instructions

Trigger store creation flow (compile vanila debug flavour for this, and have a Google Play account located in the US)
Complete the flow creating a new site and getting an active subscription for IAP
Trigger store creation flow again and see how an error dialog is shown stating that you can't have more than 1 active subscription linked to your current Google account.

You are welcome to provide any feedback for a better error message.

I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

wpmobilebot · 2023-01-17T16:57:08Z

You can test the changes on this Pull Request by downloading an installable build, or scanning this QR code:

peril-woocommerce · 2023-01-18T16:30:05Z

	Warnings
⚠️	PR is not assigned to a milestone.

Generated by 🚫 dangerJS

kidinov · 2023-01-19T08:52:23Z

...rc/main/kotlin/com/woocommerce/android/ui/login/storecreation/iap/IapEligibilityViewModel.kt

+            is IAPError.Billing.ServiceUnavailable,
+            is IAPError.Billing.ItemNotOwned,
+            is IAPError.Billing.UserCancelled,
+            is IAPError.RemoteCommunication.Network -> R.string.store_creation_iap_eligibility_check_generic_error


Would it make sense to use offline_error string for the RemoteCommunication.Network error? To be a bit more specific?

Hmm, I'm not sure about displaying any non-generic message here.
From what I've seen in the code is that IAPError.RemoteCommunication.Network is returned when BaseRequest.GenericErrorType.TIMEOUT is returned. But that doesn't necessarily mean the user is offline. In fact there is another type of error for that BaseRequest.GenericErrorType.NO_CONNECTION, which is mapped to CreateOrderErrorType.API_ERROR

private fun WPComGsonRequest.WPComGsonNetworkError.toCreateOrderError() = when (type) { BaseRequest.GenericErrorType.TIMEOUT -> CreateOrderErrorType.TIMEOUT BaseRequest.GenericErrorType.NO_CONNECTION, BaseRequest.GenericErrorType.SERVER_ERROR, BaseRequest.GenericErrorType.INVALID_SSL_CERTIFICATE, BaseRequest.GenericErrorType.NETWORK_ERROR -> CreateOrderErrorType.API_ERROR BaseRequest.GenericErrorType.PARSE_ERROR, BaseRequest.GenericErrorType.NOT_FOUND, BaseRequest.GenericErrorType.CENSORED, BaseRequest.GenericErrorType.INVALID_RESPONSE -> CreateOrderErrorType.INVALID_RESPONSE BaseRequest.GenericErrorType.HTTP_AUTH_ERROR, BaseRequest.GenericErrorType.AUTHORIZATION_REQUIRED, BaseRequest.GenericErrorType.NOT_AUTHENTICATED -> CreateOrderErrorType.AUTH_ERROR BaseRequest.GenericErrorType.UNKNOWN -> CreateOrderErrorType.GENERIC_ERROR null -> CreateOrderErrorType.GENERIC_ERROR }

I wonder if ``BaseRequest.GenericErrorType.NO_CONNECTION, should be mapped instead to CreateOrderErrorType.TIMEOUT ? Wdyt?

Just judging by Volley's naming of the errors:

NoConnectionError
Error indicating that no connection could be established when performing a Volley request.

It might refer to the case when a TCP handshake could not be established. This can happen when the device is offline, some package loss, or any connection-related issue

TimeoutError
Indicates that the connection or the socket timed out.

The request was sent, but the response didn't come back during the timeout period. The reason can be exactly the same as the first error + the backend may just didn't answer on time

But that doesn't necessarily mean the user is offline.

True, but I guess the probability that something with a connection is high in these cases

I wonder if ``BaseRequest.GenericErrorType.NO_CONNECTION, should be mapped instead to CreateOrderErrorType.TIMEOUT? Wdyt?

Yeah, seems to be a bug in the implementation there. I'll create for myself a ticket to fix that

We might use less assertive message

<string name="error_generic_network">A network error occurred. Please check your connection and try again.</string>

in this case, wdyt?

kidinov

Thank you for this improvement! It looks and works well.

I left a comment which I think is worth thinking about. Please let me know your ideas regarding this

kidinov · 2023-01-19T09:16:39Z

...rc/main/kotlin/com/woocommerce/android/ui/login/storecreation/iap/IapEligibilityViewModel.kt

+                is WPComIsPurchasedResult.Success -> {
+                    when (result.purchaseStatus) {
+                        PurchaseStatus.PURCHASED_AND_ACKNOWLEDGED,
+                        PurchaseStatus.PURCHASED -> onUserNotEligibleForIAP(


I was thinking about this case. So the item is purchased but not acknowledged by our backend, which means that the WP plan is not given to a user as well. We could call purchaseWPComPlan and it internally will call the backend and the backend should acknowledge the purchase on the GP side, but at this moment we actually not sure if this is the same site a user wanted to purchase a subscription to at the begging 🤔

On other hand, it's the user's decision and we should hope that they know what they are doing, so maybe we still should purchaseWPComPlan in this case? I am not saying to call it here, but let a user go further, and then when the website is picked use an existing GP subscription in order to give a WP plan to the picked site

Another option would be to at least show to a user that yes we have your purchase, please contact support so they will link the GP subscription to a site that needs an upgrade

Wdyt?

Hey @kidinov I'm not sure we can do what you suggest. First of all I'd like to ask under which circumstance the purchase can be placed but not acknowledged?

Regarding this:

We could call purchaseWPComPlan and it internally will call the backend and the backend should acknowledge the purchase on the GP side, but at this moment we actually not sure if this is the same site a user wanted to purchase a subscription to at the begging

We do not have a siteId available at this point and we have no clue about the site that has the active IAP WooCommerce subscription, as this subscription is tied to Google account. Keep in mind these checks run when user clicks on "create a new store", so the user hasn't even entered a site name or anything yet.

I agree we could redirect them to support to troubleshoot the problem. I'll submit a curated message for that scenario with the appropriate CTA to create a Zendesk ticket

First of all I'd like to ask under which circumstance the purchase can be placed but not acknowledged?

The IAP becomes purchased after the user from the app makes a purchase. It gets acknowledged by the backend after we do the create_order call. So any network issue between the mobile and the backend, or between the backend and google play will lead to the purchased and not acknowledged state. Also, any error on the backend or the user kills the app at a specific moment will lead to the same result. So there are many places where it can go wrong.

Having said that, after some time not acknowledged purchase will get refunded, so it's not the end of the world

We do not have a siteId available at this point and we have no clue about the site that has the active IAP WooCommerce subscription, as this subscription is tied to Google account.

Yep, that's why I mentioned:

I am not saying to call it here, but let a user go further, and then when the website is picked use an existing GP subscription in order to give a WP plan to the picked site

So you could just let a user go further, and let them click the "purchase" button and then the library will do the rest of the work, but thinking about this now, it would feel weird for a user, to buy the same thing for the second time. It's not like the "try again" button, so I guess it is not something that people will do, and saying that to talk to support is probably the best approach in this case

Thanks for clarifying the scenarios when purchase is done but couldn't be acknoledged.

but thinking about this now, it would feel weird for a user, to buy the same thing for the second time

I agree with that. The user would be taken through a flow that expects a new site at all times. So it would be super confusing for them to have to enter all the same information again (in some cases they wouldn't be able to do this as the domain for example would be already assigned to their previous site), and then be requested to pay again for something they had already paid. Offer a "Retry" option for this case would require an entirely different flow where we request the site address the site they have purchased an eCommerce subscription for but is not acknowledged.

Once more question on this. Do HE have an straightforward way to check if users have an active IAP eCommerce, and if they do, link the subscription to their site?

Adding this for reference @kidinov
If the purchase fails to be acknowledged it won't matter if we retry to confirm the purchase as the associated purchaseToken expires.
Full context on this discussion p1674662344895449/1673441948.587159-slack-C025A8VV728

kidinov · 2023-01-19T09:21:17Z

WooCommerce/src/main/res/values/strings.xml

@@ -2766,7 +2766,9 @@
    <string name="store_creation_iap_eligibility_loading_title">We are getting ready for your store creation</string>
    <string name="store_creation_iap_eligibility_loading_subtitle">Please remain connected.</string>
    <string name="store_creation_iap_eligibility_check_error_title">Store Creation Unavailable </string>
-    <string name="store_creation_iap_eligibility_check_error_description">Store creation from the mobile app is currently unavailable for your country.</string>
+    <string name="store_creation_iap_eligibility_check_error_not_available_for_country">Store creation from the mobile app is currently unavailable for your country.</string>
+    <string name="store_creation_iap_eligibility_check_error_existing_subscription">You have an active WooCommerce subscription linked to your Google account. Only one WooCommerce subscription per Google account is allowed.</string>


What if we will give an option for what to do next to a user, something like:

Currently you are only able to purchase one eCommerce plan from the mobile app. For additional options, please visit our website

Yeah, makes sense. I've updated the message. However we can't provide a link to the actual store creation flow as that would not be compliant with Google Play terms.

codecov-commenter · 2023-01-19T11:07:24Z

Codecov Report

Base: 43.45% // Head: 43.50% // Increases project coverage by +0.05% 🎉

Coverage data is based on head (1d3c232) compared to base (9ec076a).
Patch coverage: 60.65% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk    #8206      +/-   ##
============================================
+ Coverage     43.45%   43.50%   +0.05%     
- Complexity     3532     3535       +3     
============================================
  Files           712      711       -1     
  Lines         37933    37958      +25     
  Branches       5010     5018       +8     
============================================
+ Hits          16482    16513      +31     
+ Misses        19980    19962      -18     
- Partials       1471     1483      +12

Impacted Files	Coverage Δ
...login/storecreation/iap/IapMobilePayApiProvider.kt	`0.00% <0.00%> (ø)`
...login/storecreation/iap/IapEligibilityViewModel.kt	`70.45% <62.71%> (-17.36%)`	⬇️
...ce/android/iap/pub/model/WPComIsPurchasedResult.kt	`100.00% <0.00%> (+100.00%)`	⬆️

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

…tion

…ment # Conflicts: # build.gradle

…edged

JorgeMucientes · 2023-01-20T14:33:37Z

Hey @kidinov thank you for the review and all the great feedback. I'll summarize the latest changes applied after you last comments.

From this comments I've refactored error codes in this commit as suggested, providing a specific error message for them. This also required updating mapping inside IapMobilePayApiProvider where I've also taken the opportunity to remove duplicated code: commit
Added a curated error message for the case where purchase has not been acknowledged, providing a CTA to open support:

contectSuppportIAP.mp4

Ready for another round of review.

…ment # Conflicts: # build.gradle

kidinov

LGTM!

Before merging, maybe we should integrate this fluxc change though?

…ment # Conflicts: # WooCommerce/src/debug/kotlin/com.woocommerce.android/iapshowcase/purchase/IAPShowcaseMobilePayAPIProvider.kt # build.gradle

JorgeMucientes added 4 commits January 17, 2023 15:39

Ensure user has no active IAP subscription

d0474b4

Refactor error handling

389cd22

Fix and add tests

b7aea27

Fix detekt indentation issues

bc1e515

Add missing assertion

583e447

JorgeMucientes added type: enhancement A request for an enhancement. feature: store creation Store creation flow labels Jan 18, 2023

JorgeMucientes requested a review from kidinov January 18, 2023 09:14

JorgeMucientes marked this pull request as ready for review January 18, 2023 09:14

JorgeMucientes added this to the 12.0 milestone Jan 18, 2023

kidinov self-assigned this Jan 19, 2023

kidinov reviewed Jan 19, 2023

View reviewed changes

JorgeMucientes removed this from the 12.0 milestone Jan 19, 2023

JorgeMucientes added 8 commits January 19, 2023 18:20

Update message for use case where user has an active IAP Woo subscrip…

00989fc

…tion

Add new error handling for RemoteCommunication IAP errors

ea8d561

Update FluxC version to include new CreateOrderErrorType network error

4485f34

Remove duplicated implementations of IAPMobilePayAPI

0fac0d0

Merge branch 'trunk' into feature/8182-iap-single-subcription-require…

a061571

…ment # Conflicts: # build.gradle

Add curated message for cases where plan is purchased but not acknowl…

5b5d96a

…edged

Fix detekt indentation issue

b131154

Fix and add unit tests

68ebb47

JorgeMucientes added 2 commits January 23, 2023 10:01

Merge branch 'trunk' into feature/8182-iap-single-subcription-require…

ea981c8

…ment # Conflicts: # build.gradle

Add FluxC latest trunk changeset to fix compile issues

1d3c232

kidinov approved these changes Feb 1, 2023

View reviewed changes

Merge branch 'trunk' into feature/8182-iap-single-subcription-require…

0ec9cb1

…ment # Conflicts: # WooCommerce/src/debug/kotlin/com.woocommerce.android/iapshowcase/purchase/IAPShowcaseMobilePayAPIProvider.kt # build.gradle

JorgeMucientes added 2 commits February 1, 2023 16:10

Fix error mapping after bad merge

49b9389

Remove empty file after merge

50e07db

JorgeMucientes enabled auto-merge February 1, 2023 15:33

JorgeMucientes merged commit ab19239 into trunk Feb 1, 2023

JorgeMucientes deleted the feature/8182-iap-single-subcription-requirement branch February 1, 2023 15:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/8182 iap single subcription requirement #8206

Feature/8182 iap single subcription requirement #8206

JorgeMucientes commented Jan 17, 2023 •

edited

Loading

wpmobilebot commented Jan 17, 2023 •

edited

Loading

peril-woocommerce bot commented Jan 18, 2023 •

edited

Loading

kidinov Jan 19, 2023 •

edited

Loading

JorgeMucientes Jan 19, 2023

kidinov Jan 20, 2023

kidinov left a comment

kidinov Jan 19, 2023

JorgeMucientes Jan 19, 2023

kidinov Jan 20, 2023

JorgeMucientes Jan 20, 2023

JorgeMucientes Jan 25, 2023

kidinov Jan 19, 2023

JorgeMucientes Jan 19, 2023 •

edited

Loading

codecov-commenter commented Jan 19, 2023 •

edited

Loading

JorgeMucientes commented Jan 20, 2023

kidinov left a comment

Feature/8182 iap single subcription requirement #8206

Feature/8182 iap single subcription requirement #8206

Conversation

JorgeMucientes commented Jan 17, 2023 • edited Loading

Description

Testing instructions

wpmobilebot commented Jan 17, 2023 • edited Loading

peril-woocommerce bot commented Jan 18, 2023 • edited Loading

kidinov Jan 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kidinov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JorgeMucientes Jan 19, 2023 • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented Jan 19, 2023 • edited Loading

Codecov Report

JorgeMucientes commented Jan 20, 2023

kidinov left a comment

Choose a reason for hiding this comment

JorgeMucientes commented Jan 17, 2023 •

edited

Loading

wpmobilebot commented Jan 17, 2023 •

edited

Loading

peril-woocommerce bot commented Jan 18, 2023 •

edited

Loading

kidinov Jan 19, 2023 •

edited

Loading

JorgeMucientes Jan 19, 2023 •

edited

Loading

codecov-commenter commented Jan 19, 2023 •

edited

Loading