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

Add Introductory Price to InAppBillingProduct #126

Merged
merged 4 commits into from
Mar 22, 2018

Conversation

PhilippeCreytens
Copy link

Please take a moment to fill out the following:

Fixes #123 .

Changes Proposed in this pull request:

  • Add introductory price to Android
  • Add introductory price to iOS
  • Implement boolean to check if an introductory price is available

Copy link
Owner

@jamesmontemagno jamesmontemagno left a comment

Choose a reason for hiding this comment

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

For iOS you will need to handle operating systems as this was introduced in 11.2. Additionally, for Android what version of the API was this added in? Is it even in the API that I bind?

@PhilippeCreytens
Copy link
Author

Hey James,
I have updated the pull request to only add Introductory price since iOS 11.2.
Regarding the android, yes this is available in the binding you have made, if an introductory price is available then this is part of the json that is being processed. An introductory price is available from the Google Play Store version 3.5, the earliest version now available that supports subscriptions.

@mavaa
Copy link

mavaa commented Mar 21, 2018

Going to test this for our app, as we need this before we can do a release. Is there no way around needing iOS 11.2 to get these prices?
We are supposed to support iOS 8, but might have to reconsider if we can't get the prices to show up correctly.

@jamesmontemagno
Copy link
Owner

@marza91 they added the api in 11.2 so it is impossible to have it before any release before that.

There really isn't a reason to support anything older than iOS 10: https://data.apteligent.com/ios/

Description = p.LocalizedDescription,
CurrencyCode = p.PriceLocale?.CurrencyCode ?? string.Empty,
LocalizedIntroductoryPrice = operatingSystemHasIntroductoryPrice && p.IntroductoryPrice != null ? p.IntroductoryPrice.LocalizedPrice() : "",
MicrosIntroductoryPrice = operatingSystemHasIntroductoryPrice && p.IntroductoryPrice != null ? (long)(p.Price.DoubleValue * 1000000d) : 0
Copy link
Owner

Choose a reason for hiding this comment

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

shouldn't this be looking at p.IntroductoryPrice.DoubleValue?

Copy link
Owner

Choose a reason for hiding this comment

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

maybe we should refactor this into a common method?

Description = p.LocalizedDescription,
CurrencyCode = p.PriceLocale?.CurrencyCode ?? string.Empty
});
var operatingSystemHasIntroductoryPrice = NSProcessInfo.ProcessInfo.IsOperatingSystemAtLeastVersion(new NSOperatingSystemVersion(11, 2, 0));
Copy link
Owner

Choose a reason for hiding this comment

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

This is wrong. Put it at the class level:

bool IsiOS112 => UIDevice.CurrentDevice.CheckSystemVersion (11,2);

@jamesmontemagno
Copy link
Owner

Added a few more required tweaks to this one

@PhilippeCreytens
Copy link
Author

Hey James,

I have updated the PR, an extension method is now used to check whether an introduction price can be used and I have updated the DoubleValue to target the correct value.

@jamesmontemagno
Copy link
Owner

Any changes or anything needed for UWP?

@PhilippeCreytens
Copy link
Author

No changes are needed for UWP, the InAppBillingProduct will say no IntroductoryPrice is available, according to the microsoft documentation I do not see any reference to an introductory price, only if the item is on sale or not but this is not the same as an introductory price.

@jamesmontemagno jamesmontemagno merged commit 7a08bbe into jamesmontemagno:master Mar 22, 2018
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.

3 participants