Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for getBuyIntentExtraParams() #287

Merged
merged 9 commits into from
Jul 24, 2017

Conversation

ratm
Copy link
Contributor

@ratm ratm commented Jul 18, 2017

This PR addresses issue #241, adding support for getBuyIntentExtraParams() as this is the recommended method by Google.
https://developer.android.com/google/play/billing/billing_reference.html#getBuyIntent

The method is a variant of the getBuyIntent() method, and takes an additional extraParams parameter.

More info here: https://developer.android.com/google/play/billing/billing_reference.html#getBuyIntentExtraParams

ratm added 4 commits July 18, 2017 11:46
Add support for getBuyIntentExtraParams() as this is the recommended method by Google
https://developer.android.com/google/play/billing/billing_reference.html#getBuyIntent

The method is a variant of the getBuyIntent() method, and takes an additional extraParams parameter.

More info here: https://developer.android.com/google/play/billing/billing_reference.html#getBuyIntentExtraParams
@serggl
Copy link
Member

serggl commented Jul 18, 2017

instead of adding a new parameter to existing function,lets just add another function

@ratm
Copy link
Contributor Author

ratm commented Jul 18, 2017

@serggl I'm not sure to understand your remark.
Could you elaborate? Thanks!

@ratm
Copy link
Contributor Author

ratm commented Jul 18, 2017

@serggl Please let me know if my changes properly addressed your feedback. Thanks!

* @return {@code false} if the billing system is not initialized, {@code productId} is empty
* or if an exception occurs. Will return {@code true} otherwise.
*/
public boolean purchaseWithExtraParams(Activity activity, String productId, String developerPayload, Bundle extraParams)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets name it also purchase for consistency

* @return {@code false} if the billing system is not initialized, {@code productId} is empty or if an exception occurs.
* Will return {@code true} otherwise.
*/
public boolean subscribeWithExtraParams(Activity activity, String productId, String developerPayload, Bundle extraParams)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets name it subscribe

* @return {@code false} if {@code oldProductIds} is not {@code null} AND change subscription
* is not supported.
*/
public boolean updateSubscriptionWithExtraParams(Activity activity, List<String> oldProductIds,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets name it updateSubscription

productId,
purchaseType,
purchasePayload);
int apiVersion = Constants.GOOGLE_API_EXTRA_PARAMS_SUPPORTED_VERSION;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im worried that this API might not be supported, but same time v5 API (GOOGLE_API_SUBSCRIPTION_CHANGE_VERSION) may be. So its kind of a breaking change. We have isSubscriptionUpdateSupported() method that was checking if v5 API is supported, but we dont have one to check the availability of this newly introduced API.
So I guess there should be something in this method that checks for API version and uses backward compatible method if needed

productId,
purchaseType,
purchasePayload);
bundle = billingService.getBuyIntentExtraParams(Constants.GOOGLE_API_EXTRA_PARAMS_SUPPORTED_VERSION,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here. if we change API version from v3 to v6 we must ensure we have a backward compatibility plan

Use isBillingSupportedExtraParams() to check if we can use the new getBuyIntentExtraParams() method and fallback to the previous method if that’s not the case.

As isBillingSupportedExtraParams() is only available starting with Billing API v7, we effectively limit the usage of getBuyIntentExtraParams() to Billing API v7+
@ratm
Copy link
Contributor Author

ratm commented Jul 19, 2017

@serggl I'm now using isBillingSupportedExtraParams() to use the new API only if available.

productId,
purchaseType,
purchasePayload);
if( extraParamsBundle == null) // API v3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove a leading whitespace here?

@@ -19,6 +19,8 @@
{
public static final int GOOGLE_API_VERSION = 3;
public static final int GOOGLE_API_SUBSCRIPTION_CHANGE_VERSION = 5;
public static final int GOOGLE_API_EXTRA_PARAMS_SUPPORTED_VERSION = 6;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seem like this constant is not used for right now

@@ -364,6 +413,50 @@ public boolean isSubscriptionUpdateSupported()
return isSubsUpdateSupported;
}

public boolean isSubscriptionWithExtraParamsSupported(Bundle extraParams)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a little Javadoc here would be 👍

* is not supported.
*/
public boolean updateSubscription(Activity activity, List<String> oldProductIds,
String productId, String developerPayload, Bundle extraParams)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argument list here seem to be off from the indentation

productId,
purchaseType,
purchasePayload);
if(extraParamsBundle == null) // API v3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Api v5 actually :)

@serggl
Copy link
Member

serggl commented Jul 21, 2017

This now looks good!
last thing I could think of is that the changes made should be somehow reflected in the README

@serggl
Copy link
Member

serggl commented Jul 22, 2017

Thank you 👍 Will release this early next week

@ratm
Copy link
Contributor Author

ratm commented Jul 22, 2017

Sounds great! Thank you @serggl!

@serggl serggl merged commit c8e0cd8 into anjlab:master Jul 24, 2017
serggl added a commit that referenced this pull request Jul 24, 2017
showdpro pushed a commit to showdpro/android-inapp-billing-v3 that referenced this pull request Jul 13, 2021
* Add support for getBuyIntentExtraParams()

Add support for getBuyIntentExtraParams() as this is the recommended method by Google
https://developer.android.com/google/play/billing/billing_reference.html#getBuyIntent

The method is a variant of the getBuyIntent() method, and takes an additional extraParams parameter.

More info here: https://developer.android.com/google/play/billing/billing_reference.html#getBuyIntentExtraParams

* Cleanup

* Address coding style issues

* Remove extra blank line

* As requested, create specific methods to send the extra params, instead of overriding existing ones

* Fix style

* Implement @serggl’s feedback

Use isBillingSupportedExtraParams() to check if we can use the new getBuyIntentExtraParams() method and fallback to the previous method if that’s not the case.

As isBillingSupportedExtraParams() is only available starting with Billing API v7, we effectively limit the usage of getBuyIntentExtraParams() to Billing API v7+

* Add missing javadoc + Fix style

* Update README to add new methods usage
showdpro pushed a commit to showdpro/android-inapp-billing-v3 that referenced this pull request Jul 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants