[Plans] Integrate Google Store calls, and synch IAP with backend - step one by daniloercoli · Pull Request #4078 · wordpress-mobile/WordPress-Android

daniloercoli · 2016-05-09T12:39:50Z

This PR integrates Google Store calls into the app, and syncs in-app purchases with the backend.

In details:

When the user taps on the Buy button the app starts the Google Store purchase Intent.
When the purchase is finished on the Store, the app gets back the result, and takes the user to the post IAP flow. If there is an error the Google Store already shows an error, no need to show anything here in the app, and the app keeps the user on the Plans UI.
Immediately after the purchase is made, the app syncs with the wpcom servers by running a refresh task.

The refresh task, is something that syncs all the IAP info with the backend immediately after a purchase is made, at app startup, and every 5 minutes. This is just a security check, so we don't need to store in the app if the latest upgrades were synched with the server with success or not. Also, the user can cancel the subscription in the Google Store App, and we don't know this fact until the app is launched again (or the backend runs the nightly checks).

This PR also disable Buy button if a paid plan is already available on the blog. Upgrades aren't handled in-app in this iteration.

Note: Backend is not yet ready. You will see 404 errors on those network calls ;)

…e user could have cancelled them on the PlayStore). Re-synch purchases when the user buys a subscription from the store.

…ss-Android into feature/plans-integrate-store-step-one

Add comments.

…ss-Android into feature/plans-integrate-store-step-one

…an subscription, and redirect the user to support. We're not going to enable upgrades whitin the apps.

…ss-Android into feature/plans-integrate-store-step-one # Conflicts: # WordPress/src/main/res/values/strings.xml

…lan attached to it.

…ss-Android into feature/plans-integrate-store-step-one

nbradbury · 2016-05-09T12:49:20Z

I just tried this PR on a Nexus 6 emulator with the Google APIs, and at startup I'm seeing this:

Caused by: java.lang.IllegalStateException: IAB helper is not set up. 
Can't perform operation: queryInventory
                                                                         at org.wordpress.android.ui.plans.util.IabHelper.checkSetupDone(IabHelper.java:808)
                                                                         at org.wordpress.android.ui.plans.util.IabHelper.queryInventory(IabHelper.java:568)
                                                                         at org.wordpress.android.ui.plans.UpdateIAPTask.listIAPs(UpdateIAPTask.java:88)
                                                                         at org.wordpress.android.ui.plans.UpdateIAPTask.doInBackground(UpdateIAPTask.java:78)
                                                                         at org.wordpress.android.ui.plans.UpdateIAPTask.doInBackground(UpdateIAPTask.java:31)
                                                                         at android.os.AsyncTask$2.call(AsyncTask.java:295)
                                                                         at java.util.concurrent.FutureTask.run(FutureTask.java:237)
                                                                         at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1113) 
                                                                         at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:588) 
                                                                         at java.lang.Thread.run(Thread.java:818)

daniloercoli · 2016-05-09T12:56:35Z

@nbradbury You should see a IAB failed with XXX error before that one you mentioned above.
Anyway, I probably missed to check if IAB was setup with success or not before starting the network calls.
Also thinking to disable the Buy button if for some reason IABHelper cannot be successfully setup on the device.

nbradbury · 2016-05-09T13:02:03Z

WordPress/src/main/java/org/wordpress/android/ui/plans/PlansActivity.java

+        final Plan plan = getPageAdapter().getPlan(position);
+
+        // No downgrade!
+        if (PlansUtils.isFreePlan(plan) || currentPlanProductId > plan.getProductID()) {


Rather than hard-code currentPlanProductId > plan.getProductID(), I think we should have a helper routine that checks if one plan is "greater" than another. We had this earlier on, and it was added to avoid sprinkling our code with assumptions that a greater plan ID meant a more expensive plan.

…on it.

Introduce a utlity method that check if a plan is "greater" than another.

Fix a typo in the method that returns if a passed plan object is a free plan.

nbradbury · 2016-05-09T21:54:28Z

I'm wondering if we can make this more efficient.

Every five minutes it makes a call to the IabHelper to get purchases (which by itself is fine since purchases are cached on the device), and then makes a separate POST to our backend for each purchase.

This is fine for users who never buy anything since no network calls will be made, but otherwise it seems wasteful.

I think we're okay decreasing the frequency of UpdateIAPTask. As long as it's called at startup, changing the interval to 30min or longer seems more reasonable. And it would be nice if the backend could accept multiple purchases in a single call.

Even better would be some way to determine whether we even need to call our backend to update purchases, since the vast majority of the time our backend will already have the latest purchase info.

This may seem like nitpicking since we're not talking about a huge waste of bandwidth, but over time I've seen us increase the number of network calls done at startup and at regular intervals, and I think we need to be more careful about unnecessary network usage.

nbradbury · 2016-05-09T22:16:09Z

WordPress/src/main/java/org/wordpress/android/ui/plans/PlansUtils.java

-        return plan.getProductSlug().equals("free_plan") ||
-               plan.getProductSlug().equals("jetpack_free");
+        return plan.getProductSlug().equals(PlansConstants.FREE_PLAN_SLUG) ||
+               plan.getProductSlug().equals(PlansConstants.JETPACK_FREE_PLAN_SLUG);


daniloercoli · 2016-05-10T09:44:49Z

I think we're okay decreasing the frequency of UpdateIAPTask. As long as it's called at startup, changing the interval to 30min or longer seems more reasonable.

Ok, will update the interval to 60 minutes, since it's already called at startup.

Even better would be some way to determine whether we even need to call our backend to update purchases, since the vast majority of the time our backend will already have the latest purchase info.

Yeah, I had thought to implement this kind of logic, but ended up with a code that was a bit clunky. We need to keep track if the purchase was already synched with the server, and keep track of server responses. Anyway, will try to rethink it.
It would not be a problem if our code that checks purchase isn't fail-proof, since the backend runs a nightly task that checks all the IAPs by contacting Google Store directly.

…not use scheduled updates, since the backend already synchs the IAPs nightly.

daniloercoli · 2016-05-11T15:27:07Z

@nbradbury - This is ready for another round of review.
IAPs are only synched when a purchase is made. We don't need to check them client side periodically, since the backend already synchs them on a nightly job.

nbradbury · 2016-05-12T13:08:50Z

IAPs are only synched when a purchase is made. We don't need to check them client side periodically, since the backend already synchs them on a nightly job.

~~Perhaps I'm misunderstanding, but if this is the case, how does the app know about purchases made elsewhere (on the web, or on another device, for example)?~~

Nevermind, I answered my own question by looking at your last commit.

nbradbury · 2016-05-12T13:17:25Z

WordPress/src/main/java/org/wordpress/android/ui/plans/UpdateIAPTask.java

+public class UpdateIAPTask extends AsyncTask<Void, Void, Void> {
+    private static final int GET_IAP_BINDER_TIMEOUT = 30000;
+    private static final String IAP_ENDPOINT = "/iap/validate";
+    protected Context mContext;


Minor nit, this can be private final rather than protected.

daniloercoli · 2016-05-12T14:54:33Z

@nbradbury - I think all the issues brought up during the 2nd review are now resolved.

nbradbury · 2016-05-12T17:33:26Z

WordPress/src/main/java/org/wordpress/android/ui/plans/util/IabHelper.java


    void logError(String msg) {
-        AppLog.d(AppLog.T.PLANS, "In-app billing error: " + msg);
+        AppLog.e(AppLog.T.PLANS, "In-app billing error: " + msg);


Both logError and logWarn are missing the mDebugLog check.

The original version doesn't have the mDebugLog check for w and e logging. I think it's ok not having it there, since w and e are important.

nbradbury · 2016-05-12T19:27:04Z

daniloercoli added 22 commits April 10, 2016 22:30

Use our logger in IabHelper

2341d26

Start integrating the store calls

9cda45d

Re-synch purchases with wpcom server on startup and periodically. (Th…

db846cf

…e user could have cancelled them on the PlayStore). Re-synch purchases when the user buys a subscription from the store.

Merge branch 'develop' of https://github.com/wordpress-mobile/WordPre…

eecf66f

…ss-Android into feature/plans-integrate-store-step-one

Remove unused imports.

443d963

Add comments.

Remove custom hardcoded SKUs since REST Endpoint has been updated.

8f553c9

Fix log message

2b9d5a6

Merge branch 'develop' of https://github.com/wordpress-mobile/WordPre…

63ef93b

…ss-Android into feature/plans-integrate-store-step-one

Merge branch 'develop' of https://github.com/wordpress-mobile/WordPre…

6be91fa

…ss-Android into feature/plans-integrate-store-step-one

Merge branch 'develop' of https://github.com/wordpress-mobile/WordPre…

3835ec3

…ss-Android into feature/plans-integrate-store-step-one

Use the new endpoint name, and remove unused parameters.

691e6a8

Merge branch 'develop' of https://github.com/wordpress-mobile/WordPre…

cc3c871

…ss-Android into feature/plans-integrate-store-step-one

Change the label to "upgrade" when the selected blog already has a pl…

569e2b0

…an subscription, and redirect the user to support. We're not going to enable upgrades whitin the apps.

Merge branch 'develop' of https://github.com/wordpress-mobile/WordPre…

9cfa151

…ss-Android into feature/plans-integrate-store-step-one # Conflicts: # WordPress/src/main/res/values/strings.xml

Remove links to unused subprojects used during development

20c7335

Revert unwanted commits

27765fb

Fix merge conflicts

ef9b80d

Fix merge conflicts - tentative #2

97baa8c

New plans strings

7c28243

Do not show the Upgrade|Purchase button when the blog already has a p…

90e71a4

…lan attached to it.

Merge branch 'develop' of https://github.com/wordpress-mobile/WordPre…

e6ce503

…ss-Android into feature/plans-integrate-store-step-one

Merge branch 'develop' of https://github.com/wordpress-mobile/WordPre…

877b460

…ss-Android into feature/plans-integrate-store-step-one

daniloercoli assigned nbradbury May 9, 2016

daniloercoli added Plans & Checkout [Type] Task labels May 9, 2016

daniloercoli changed the title ~~[Plans] Integrate google store and synch IAP with backend - step one~~ [Plans] Integrate Google Store calls, and synch IAP with backend - step one May 9, 2016

nbradbury reviewed May 9, 2016
View reviewed changes

daniloercoli added 3 commits May 9, 2016 15:25

Make sure mIabHelper has finished its setup before doing any calls …

bdba660

…on it.

Use constants defined in PlanConstants.java when checking for planID

948c6be

Introduce a utlity method that check if a plan is "greater" than another.

Remove unused constants.

795c213

Fix a typo in the method that returns if a passed plan object is a free plan.

nbradbury reviewed May 9, 2016
View reviewed changes

Synch IAPs with the backend when there is a pending request only. Do …

f48f9f7

…not use scheduled updates, since the backend already synchs the IAPs nightly.

nbradbury reviewed May 12, 2016
View reviewed changes

daniloercoli added 4 commits May 12, 2016 15:41

Change the visibilty of mContext to private in UpdateIAPTask

858cfa8

Drop unused variable

d0d361e

Added a comment that describe how the sync with wpcom works

70d4012

Revert changes made to IabHelper

84932ae

nbradbury reviewed May 12, 2016
View reviewed changes

nbradbury merged commit f963389 into develop May 12, 2016

nbradbury deleted the feature/plans-integrate-store-step-one branch May 12, 2016 19:27

Conversation

daniloercoli commented May 9, 2016

Uh oh!

nbradbury commented May 9, 2016

Uh oh!

daniloercoli commented May 9, 2016

Uh oh!

nbradbury May 9, 2016

Choose a reason for hiding this comment

Uh oh!

nbradbury commented May 9, 2016

Uh oh!

nbradbury May 9, 2016

Choose a reason for hiding this comment

Uh oh!

daniloercoli commented May 10, 2016

Uh oh!

daniloercoli commented May 11, 2016

Uh oh!

nbradbury commented May 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nbradbury May 12, 2016

Choose a reason for hiding this comment

Uh oh!

daniloercoli commented May 12, 2016

Uh oh!

nbradbury May 12, 2016

Choose a reason for hiding this comment

Uh oh!

daniloercoli May 12, 2016

Choose a reason for hiding this comment

Uh oh!

nbradbury commented May 12, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nbradbury commented May 12, 2016 •

edited

Loading