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

fix: add missing conn for SK2 intro offers #2630

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

Shaw-Signaturize
Copy link
Contributor

Adding missing mapping for intro offers in storekit2 on existing type

@Shaw-Signaturize
Copy link
Contributor Author

Not sure who to ping for this but it seems the failing check is on a diff of swift files but none were changed in this PR.
Could this be an issue in the check logic or is there something i have neglected to do.

@LectricAvenue
Copy link

@Shaw-Signaturize, thanks so much for this, I ran into this myself and came up with a similar solution, but I wasn't sure if it was a result of not using RNIAP as intended.

The type for SubscriptionIOS looks like this:

export interface SubscriptionIOS extends ProductCommon {
  platform: SubscriptionPlatform.ios;
  type: 'subs';
  discounts?: Discount[];
  introductoryPrice?: string;
  introductoryPriceAsAmountIOS?: string;
  introductoryPricePaymentModeIOS?:
    | ''
    | 'FREETRIAL'
    | 'PAYASYOUGO'
    | 'PAYUPFRONT';
  introductoryPriceNumberOfPeriodsIOS?: string;
  introductoryPriceSubscriptionPeriodIOS?: SubscriptionIosPeriod;
  subscriptionPeriodNumberIOS?: string;
  subscriptionPeriodUnitIOS?: SubscriptionIosPeriod;
}

I think what you have assigned to introductoryPriceAsAmountIOS should actually be assigned to introductoryPrice.

introductoryPriceAsAmountIOS should instead be the value without the currency symbol in front.

I believe that would make the proper change the following:

{
  // unchanged fields above
  introductoryPrice: subscription?.introductoryOffer?.displayPrice,
  introductoryPriceAsAmountIOS: subscription?.introductoryOffer?.price.toString(),
  introductoryPricePaymentModeIOS:
    subscription?.introductoryOffer?.paymentMode.toUpperCase() as
      | ''
      | 'FREETRIAL'
      | 'PAYASYOUGO'
      | 'PAYUPFRONT',
  introductoryPriceNumberOfPeriodsIOS:
    subscription?.introductoryOffer?.period?.value?.toString(),
  introductoryPriceSubscriptionPeriodIOS: subscription?.introductoryOffer
    ?.period?.unit as SubscriptionIosPeriod,
}

Since the library also returns currency: '' with a note that the field isn't available with SK2. I used to get countryCode and discounts with SK1, so maybe it would be good to add those in with similar notes so that it is easier for people to find in the future.

@Shaw-Signaturize
Copy link
Contributor Author

Shaw-Signaturize commented Dec 5, 2023

@LectricAvenue
You could very well be right, i did have a look at the type before adding the fields but as some of the fields don't match exactly, it required a bit of creative license.
I was originally hoping to get a bit of guidance from the project maintainers to make sure the mappings were what they had envisioned as with them being on the type and on the sk2 response it just seemed like a slight oversight that they were missed from the mapping.

@LectricAvenue
Copy link

@Shaw-Signaturize of course! I agree that potentially they intended to remove these, although surely they didn't mean to leave out the intro offer details entirely.

The docs seem very outdated, but in the source code I see lots of documentation markup that doesn't seem to be available on the website?

Copy link
Owner

@hyochan hyochan left a comment

Choose a reason for hiding this comment

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

Could you kindly check ci failing?

@hyochan hyochan changed the title fix: Adding missing connection for SK2 intro offers fix: add missing conn for SK2 intro offers Dec 8, 2023
@hyochan hyochan added 📱 iOS Related to iOS 🛠 bugfix All kinds of bug fixes labels Dec 11, 2023
@Shaw-Signaturize
Copy link
Contributor Author

Could you kindly check ci failing?

@hyochan

As mentioned above the CI seems to be failing when verifying that certain swift files have not changed.
The CI is under the impression that file ios/RNIapIosSk2.swift has changed but no modifications have been made to it in this PR.
I was unsure if this was an issue with the CI or there was something i have neglected to do

Copy link
Owner

@hyochan hyochan left a comment

Choose a reason for hiding this comment

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

I've rebased codebase myself~
Let's see how this code works!

@hyochan hyochan merged commit ba1b7ef into hyochan:main Dec 15, 2023
3 checks passed
@Shaw-Signaturize
Copy link
Contributor Author

@hyochan

Thanks. The help is much appreciated

arthurgeron pushed a commit to arthurgeron/react-native-iap that referenced this pull request Jan 7, 2024
Adding missing mapping for intro offers in storekit2 on existing type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠 bugfix All kinds of bug fixes 📱 iOS Related to iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants