Shopify / unity-buy-sdk

The Unity Buy SDK allows Unity developers to query and sell products from Shopify directly in Unity.
https://help.shopify.com/api/sdks/custom-storefront/unity-buy-sdk
MIT License
67 stars 23 forks source link

Adjustments to checkout functions #291

Open richardmonette opened 7 years ago

richardmonette commented 7 years ago

Summarizing our checkout functions dilemma.

Related https://github.com/Shopify/unity-buy-sdk/issues/283, https://github.com/Shopify/unity-buy-sdk/issues/269

Current state of the checkout functions

Currently, we have three ways of initiating checkout.

Firstly, we have the native pay api (ApplePay, and eventually AndroidPay will be triggered by calling this function. Since drawer ui opens, and controls the interaction, we are able to know whether the checkout results in success, cancellation or failure. In turn, we are able to offer callbacks for these events to the caller.

public void CheckoutWithNativePay(string key, CheckoutSuccessCallback success, CheckoutCancelCallback cancelled, CheckoutFailureCallback failure)

Next up, is the api for a "native" web view based checkout.

On iOS, this web view is most like a "native" checkout, it takes control of the users focus, and there are events for when the web view is dismissed. As a result of it not being possible to navigate away from the web view, and there being events about closing, we are able to watch for the web view close, check the status of the checkout and then call the appropriate callback.

On Android, the web view opens in a browser which does not capture the users behaviour, such that they user is able to navigate away from an in progress checkout, back to the game, or to any other app. As a result, when the user returns to the game, it is possible that the checkout browser window is actually still open, and we must consider that the checkout success callback might need to be triggered at a later point in time. Furthermore, since the browser window may still be open, it is not clear if the cancelled callback should be triggered, because its possible that they checkout may be called at a later point in time.

public void CheckoutWithNativeWebView(CheckoutSuccessCallback success, CheckoutCancelCallback cancelled, CheckoutFailureCallback failure)

Finally, we have the current procedure for launching a fully external browser to handle the checkout. This is the process which would be used on desktop, for example, or when demonstrating a checkout from the editor, where there is no other browser functionality available.

As it stands, we just have the function

public void GetWebCheckoutLink(GetWebCheckoutLinkSuccessCallback success, GetWebCheckoutLinkFailureCallback failure)

which fetches the checkout url. It is then the responsibility of the caller to do a second call on

Application.OpenURL passing that checkout url, in order to start the web browser.

We currently have no callback associated with this process, so it would be entirely up to the caller to handle any OnApplicationFocus behaviour, in order to check and see if the checkout succeeded.

Issues with the current checkout functions

  1. On Android, we add a GameObject so as to monitor the ApplicationOnFocus event, but we leave that monitor in place indefinitely, such that cancel callback might fire even if the checkout browser windows is still open (meaning the checkout wasn't really cancelled), and/or the success callback may fire at a later time (if the user goes back and completes the checkout), when it is no longer reasonable to trigger the checkout (for example, the user is now back in the game.)

  2. For fully external browser checkouts, we don't currently afford any method to start the checkout, nor do we afford any callbacks. These callbacks would be subject to the same issues as the Android browser based checkout - and would require consideration of how we solve those issues as well.

@Shopify/gaming @coreypollock

richardmonette commented 7 years ago

OK, so making a note here for the record on this, since we've shipped https://github.com/Shopify/unity-buy-sdk/pull/280 which changed CheckoutWithNativeWebView to CheckoutWithWebView and set things up so we use the UnityWebCheckout when in the editor or in the stand alone player.

The good news is, I believe this fixes issue 2, wherein we did not offer any functions for launching an external browser checkout. However, the bad news, is that it means that we have essentially duplicated issue 1, where we have the problem that ApplicationOnFocus is never removed, so the callbacks might fire at future, indeterminate times in the life cycle of the app.

richardmonette commented 7 years ago

So, for fixing issue 2, what I am thinking is that we modify https://github.com/Shopify/unity-buy-sdk/blob/beta/scripts/generator/graphql_generator/csharp/SDK/WebCheckoutMessageReceiver.cs.erb such that it makes a single attempt to handle the "focus" (OnApplicationFocus/OnNativeMessage) event. We could could a flag to prevent the attempt on the callback from happening twice.

This would mean that on the green path, the user triggers the checkout, completes it, comes back to the game and the callbacks fire as expected. If they cancelled, then one cancelled event will fire (the name of this event is a bit weird still, but that's sort of another sub issues again). If the user returns to an open checkout and completes it later, there won't by a success callback triggered, but I think this is definitely an improvement, if not an outright fix, since it wouldn't make sense to trigger success when the game is now in an unknown state.

tl;dr My proposal is that we only trigger the 'return to game' event callbacks to check for checkout success the first time the user returns to the game.

@dengjeffrey @sleroux If that sounds good to you guys, I'll go ahead and write the PR for this change.

dengjeffrey commented 7 years ago

Could you clarify as to what you mean by flag to prevent the attempt on the callback from happening twice.. Would this be as simple as removing the WebCheckoutMessageReceiver?

richardmonette commented 7 years ago

A couple options I can see:

  1. We actually remove WebCheckoutMessageReceiver, the GameObject from the Unity scene, which in turn means it won't be receiving any more events.

or,

  1. We leave the GameObject in the scene, but we add something like
if (eventFiredAlreadyDirtyFlag) {
    return;
}
eventFiredAlreadyDirtyFlag = true;

to OnNativeMessage and OnApplicationFocus, and we use that to control when the handlers fire. I'm open to either, that seems like more of a technical detail, assuming we are all in agreement that this is the correct life cycle to be implementing.

dengjeffrey commented 7 years ago

Using this solution, during an edge case the implementor will never know if the completion state of the local cart is synced until they hit an error.

The edge case being, they returned to the app, then back to the checkout to complete it.

But perhaps we are trying to handle too much, I'm okay with this path.

sleroux commented 7 years ago

Checking in onApplicationFocus only once sounds like the cleanest and most probable solution for now. This pretty much replicates our existing behaviour for our other web views for external browsers which is fine.