feat: add standardized DiscountOffer and SubscriptionOffer types#63
feat: add standardized DiscountOffer and SubscriptionOffer types#63
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR introduces standardized cross-platform offer types (DiscountOffer, SubscriptionOffer, SubscriptionPeriod, enums), wires them through iOS/Android conversion layers and product models, updates GraphQL schema and docs, adapts example UIs, and adds multi-repo sync/playbook documentation and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Store as Native Store (Play / StoreKit)
participant Converter as Billing / StoreKit Converters
participant Model as Product Model (Android / iOS)
participant Schema as GraphQL Schema
participant UI as Example App UI
Store->>Converter: Emit native product + offer payloads
Converter->>Converter: Parse offers, parse billingPeriod, determine paymentMode
Converter->>Model: Construct Product / ProductSubscription with discountOffers / subscriptionOffers
Model->>Schema: Serialize product including new offer fields
UI->>Schema: Fetch product + offers
UI->>Model: Render price/discount badge using standardized offer fields
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @hyochan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the OpenIAP library by introducing a set of standardized, cross-platform types for managing discount and subscription offers. This change streamlines the handling of various offer structures from different app stores (Apple App Store, Google Play Store, Meta Horizon Store) into a consistent API. It facilitates easier integration and reduces complexity for developers by providing a unified data model, while still allowing access to platform-specific details through clearly suffixed fields. The update also includes a clear deprecation strategy for older types, guiding users towards the new, more robust solution. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This is a substantial and well-executed pull request that introduces standardized DiscountOffer and SubscriptionOffer types, creating a unified, cross-platform API for handling discounts and promotions. This refactoring deprecates older, platform-specific types in favor of a more maintainable flat structure with platform-specific suffixes. The changes are comprehensive, touching native code, type definitions, example apps, and documentation across all supported platforms. The addition of extensive unit tests for the new types is also a great plus.
My review has identified a few minor areas for improvement, primarily related to ensuring consistency in enum parsing and improving the portability of internal documentation scripts. Overall, this is a high-quality contribution that significantly enhances the library's architecture and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @.claude/commands/sync-kmp-iap.md:
- Line 186: The markdown section numbering is off: the header "### 3. Update
Type Aliases" should be "### 4. Update Type Aliases" and all subsequent step
headers must be incremented by one (e.g., the header after "### 3. Build & Test
Native Code" and every following numeric header currently labeled 4–10 should be
renumbered to 5–11 accordingly). Update the headings in
.claude/commands/sync-kmp-iap.md so that "### 3. Update Type Aliases" becomes
"### 4. Update Type Aliases" and adjust every subsequent "### N." header to "###
N+1." to restore correct sequential numbering.
In @.claude/commands/sync-react-native-iap.md:
- Line 195: The heading "### 5. Update Tests" and subsequent step headings are
misnumbered—rename "### 5. Update Tests" to "### 6. Update Tests" and increment
the following step headings accordingly (change the subsequent "### 6" → "###
7", "### 7" → "### 8", "### 8" → "### 9") so the sequence follows after "### 5.
Update Example Code"; search for the heading strings "### 5. Update Tests" and
the later "### 6", "### 7", "### 8" headings and update their numbers to
maintain a correct ordinal sequence.
In
`@packages/google/Example/src/main/java/dev/hyo/martie/screens/uis/ProductCard.kt`:
- Around line 147-159: In ProductCard.kt adjust the discount badge construction
so you don't append " OFF" to already-formatted discount strings: when
standardizedDiscount.percentageDiscountAndroid is present build "${percentage}%
OFF", but when standardizedDiscount.formattedDiscountAmountAndroid is present
use standardizedDiscount.formattedDiscountAmountAndroid as-is (no extra " OFF");
likewise for legacy discountInfo.use discountInfo.percentageDiscount ->
"${percentage}% OFF" and use
discountInfo.discountAmount?.formattedDiscountAmount as-is without appending "
OFF". Update the when branches in the discountText logic to follow this order
and avoid duplicating "OFF".
- Around line 126-135: The numeric formatting for the struck-through price uses
String.format without a Locale which can produce inconsistent decimal
separators; update the formatting in ProductCard.kt where
fullPriceMicros/fullPrice and currencyCode are used so String.format specifies
an explicit Locale (e.g., Locale.US) or use a locale-aware NumberFormat,
ensuring the call around String.format("%.2f", fullPrice) is replaced with a
locale-specified formatter to guarantee consistent decimal formatting across
devices.
In
`@packages/google/openiap/src/play/java/dev/hyo/openiap/utils/BillingConverters.kt`:
- Around line 170-180: In determinePaymentMode, first check priceAmountMicros ==
0L and return PaymentMode.FreeTrial for any recurrenceMode (so zero-price phases
are treated as free trials), otherwise switch on recurrenceMode: return
PaymentMode.PayAsYouGo for INFINITE_RECURRING (1) and FINITE_RECURRING (2) paid
phases, return PaymentMode.PayUpFront for NON_RECURRING (3) paid one-time
offers, and PaymentMode.Unknown for any other value; update the when logic in
determinePaymentMode accordingly to reference PaymentMode.FreeTrial,
PaymentMode.PayAsYouGo, PaymentMode.PayUpFront, and PaymentMode.Unknown.
🧹 Nitpick comments (7)
.claude/commands/sync-flutter-iap.md (4)
247-253: Add language specifier to fenced code block.The documentation subsection code block is missing a language specifier, which triggers a markdownlint warning.
📝 Proposed fix
### 7. Update Documentation -**Location:** `docs/` +**Location:** `docs/` + - Docusaurus site
283-298: Add language specifier to fenced code block.The pre-commit checklist code block should specify
bashas the language for consistent syntax highlighting.📝 Proposed fix
### 9. Pre-commit Checklist -```bash +```bash # Format (excludes types.dart) git ls-files '*.dart' | grep -v '^lib/types.dart$' | xargs dart format --page-width 80 --output=none --set-exit-if-changedNote: The code block at line 283 already has
bashspecified - the markdownlint warning at line 289 appears to be a false positive asdart formatis the command on that line, not a code fence.
354-359: Convert bare URLs to proper markdown links.The reference URLs should use markdown link syntax for consistency and better rendering.
📝 Proposed fix
## References - **CLAUDE.md:** `/Users/hyo/Github/hyochan/flutter_inapp_purchase/CLAUDE.md` - **CONVENTION.md:** `/Users/hyo/Github/hyochan/flutter_inapp_purchase/CONVENTION.md` -- **OpenIAP Docs:** https://openiap.dev/docs -- **flutter_inapp_purchase Docs:** https://hyochan.github.io/flutter_inapp_purchase +- **OpenIAP Docs:** <https://openiap.dev/docs> +- **flutter_inapp_purchase Docs:** <https://hyochan.github.io/flutter_inapp_purchase>
210-216: Fix duplicate section numbering.Section "### 3. Update Helper Functions" should be numbered "### 4." since there's already a "### 3. Build & Test Native Code" section at line 130.
📝 Proposed fix
-### 3. Update Helper Functions +### 4. Update Helper Functions If types change, update `lib/helpers.dart`: - JSON to object conversions - Platform-specific logic - Type transformations -### 4. Update Error Handling +### 5. Update Error HandlingContinue renumbering subsequent sections (4→5, 5→6, etc.) through section 9.
.claude/commands/sync-expo-iap.md (3)
177-179: Fix duplicate section numbering.Section "### 3. Update Example Code" at line 177 duplicates the numbering of "### 3. Build & Test Native Code" at line 102. Renumber to "### 4." and adjust subsequent sections.
📝 Proposed fix
-### 3. Update Example Code +### 4. Update Example Code **Location:** `example/app/`Continue renumbering: 4→5, 5→6, 6→7, 7→8.
248-255: Add language specifier to fenced code block.The deprecation check code block should specify
bashas the language.📝 Proposed fix
## Deprecation Check Search for deprecated patterns: -``` +```bash cd /Users/hyo/Github/hyochan/expo-iap grep -r "@deprecated" src/ grep -r "DEPRECATED" src/</details> --- `270-274`: **Convert bare URLs to proper markdown links.** <details> <summary>📝 Proposed fix</summary> ```diff ## References - **CLAUDE.md:** `/Users/hyo/Github/hyochan/expo-iap/CLAUDE.md` -- **OpenIAP Docs:** https://openiap.dev/docs -- **expo-iap Docs:** https://expo-iap.vercel.app +- **OpenIAP Docs:** <https://openiap.dev/docs> +- **expo-iap Docs:** <https://expo-iap.vercel.app>
packages/google/Example/src/main/java/dev/hyo/martie/screens/uis/ProductCard.kt
Show resolved
Hide resolved
packages/google/Example/src/main/java/dev/hyo/martie/screens/uis/ProductCard.kt
Outdated
Show resolved
Hide resolved
packages/google/openiap/src/play/java/dev/hyo/openiap/utils/BillingConverters.kt
Show resolved
Hide resolved
- Add new cross-platform types: DiscountOffer, SubscriptionOffer, DiscountOfferType, PaymentMode, SubscriptionPeriod, SubscriptionPeriodUnit - Use flat structure with platform-specific suffixes (offerTokenAndroid, keyIdentifierIOS, etc.) - Deprecate old platform-specific types for migration path - Update Android BillingConverters to populate new standardized fields - Update iOS StoreKitTypesBridge to convert StoreKit offers to standardized types - Add Horizon flavor support with null/empty defaults - Add comprehensive unit tests for both platforms - Update example apps to use new standardized offer types - Add documentation page for offer types
- Fix PaymentMode classification: zero price -> FreeTrial, NON_RECURRING paid -> PayUpFront - Add UPPER_CASE variants to enum fromJson (DiscountOfferType, PaymentMode, SubscriptionPeriodUnit) - Fix duplicate 'OFF' in discount badge text - Add Locale to String.format for consistent number formatting - Fix step numbering in sync-kmp-iap.md and sync-react-native-iap.md
88ee1af to
880d562
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/docs/src/pages/docs/types/ios.tsx`:
- Around line 40-55: Update the displayed deprecated type names in the TLDRBox
and section headings to use the iOS-suffixed identifiers so they match the
deprecation notice—replace occurrences of "DiscountOffer" and "Discount" with
"DiscountOfferIOS" and "DiscountIOS" (and ensure "SubscriptionOfferIOS" is
referenced where applicable) while keeping existing anchors/links and copy
intent unchanged; touch the occurrences around the TLDRBox and the other
mentioned ranges (roughly the blocks referencing these types at lines ~59-65 and
~114-120) and verify the <code>DiscountOfferIOS</code>,
<code>DiscountIOS</code>, and <code>SubscriptionOfferIOS</code> tokens are used
in UI copy only.
In `@packages/docs/src/pages/docs/types/offer.tsx`:
- Around line 876-934: The example uses await inside a products.forEach callback
which is invalid; replace the forEach with a for...of loop (e.g., for (const
product of products) { ... }) so you can await requestPurchase calls, and ensure
the enclosing function or module scope is async (or wrapped in an async IIFE) so
the top-level awaits are valid; update the block that currently references
products.forEach(...) and keep the existing await usages for requestPurchase and
any offer.offerTokenAndroid / offer.signatureIOS checks intact.
In
`@packages/google/openiap/src/play/java/dev/hyo/openiap/utils/BillingConverters.kt`:
- Around line 184-230: The current toSubscriptionOffer() logic misclassifies
offers by treating offerId != null as Promotional; update the offerType
determination to inspect offerTags and pricingPhases/pricingPhaseList instead:
treat as Introductory when offerTags contains indicators like "intro" or when
pricingPhases.pricingPhaseList has a first phase with priceAmountMicros == 0 or
a discounted first phase followed by paid phases (multi-phase free/discount
pattern), treat as Promotional when there is a single discounted phase without
an introductory free/discounted first phase, and otherwise return a
neutral/Unknown/Undetermined type; update the decision in
ProductDetails.SubscriptionOfferDetails.toSubscriptionOffer() (referencing
offerId, offerTags, pricingPhases.pricingPhaseList,
pricingPhase.priceAmountMicros) and if classification is ambiguous, set type to
a fallback (e.g., DiscountOfferType.Unknown) and add a comment that Android-side
heuristics are best-effort and server-side validation is required.
♻️ Duplicate comments (4)
.claude/commands/sync-expo-iap.md (1)
5-5: Hardcoded absolute paths reduce portability.The file contains hardcoded paths like
/Users/hyo/Github/hyochan/expo-iapthroughout. Consider using environment variables (e.g.,$PROJECTS_HOME/expo-iap) or placeholders like<your-projects-dir>to make this documentation usable by other contributors..claude/commands/sync-all-platforms.md (1)
7-14: Hardcoded absolute paths reduce portability.The repository paths table and subsequent commands use hardcoded paths specific to one user's machine (
/Users/hyo/Github/...). Consider using environment variables or placeholders to improve portability for other contributors..claude/commands/sync-kmp-iap.md (1)
5-5: Hardcoded absolute paths reduce portability.The target repository path and subsequent commands use hardcoded paths specific to one user's machine. Consider using environment variables or placeholders for better portability.
.claude/commands/sync-react-native-iap.md (1)
5-5: Hardcoded absolute paths reduce portability.The target repository path and subsequent commands use hardcoded paths specific to one user's machine. Consider using environment variables or placeholders for better portability.
🧹 Nitpick comments (4)
packages/docs/src/pages/docs/types/offer.tsx (1)
8-10: Add JSDoc for the exportedTypesOffercomponent.
Keeps public docs pages consistently documented. As per coding guidelines.✍️ Suggested doc comment
+/** + * Documentation page for standardized discount and subscription offer types. + */ function TypesOffer() {.claude/commands/sync-expo-iap.md (1)
270-274: Convert bare URLs to markdown links.The references section uses bare URLs which violates markdown best practices (MD034). Consider using markdown link syntax for consistency.
📝 Suggested fix
## References - **CLAUDE.md:** `/Users/hyo/Github/hyochan/expo-iap/CLAUDE.md` -- **OpenIAP Docs:** https://openiap.dev/docs -- **expo-iap Docs:** https://expo-iap.vercel.app +- **OpenIAP Docs:** [openiap.dev/docs](https://openiap.dev/docs) +- **expo-iap Docs:** [expo-iap.vercel.app](https://expo-iap.vercel.app).claude/commands/sync-react-native-iap.md (1)
295-299: Convert bare URLs to markdown links.The references section uses bare URLs which violates markdown best practices (MD034).
📝 Suggested fix
## References - **CLAUDE.md:** `/Users/hyo/Github/hyochan/react-native-iap/CLAUDE.md` -- **OpenIAP Docs:** https://openiap.dev/docs -- **react-native-iap Docs:** https://react-native-iap.dooboolab.com +- **OpenIAP Docs:** [openiap.dev/docs](https://openiap.dev/docs) +- **react-native-iap Docs:** [react-native-iap.dooboolab.com](https://react-native-iap.dooboolab.com)packages/apple/Sources/Helpers/StoreKitTypesBridge.swift (1)
458-461: Currency is nil at offer level—consider documenting workaround.The comment explains iOS doesn't provide currency at the offer level. Consumers needing currency should use the parent product's
currencyfield. This is a StoreKit limitation, not a bug.Consider adding a brief doc comment on the
SubscriptionOffer.currencyfield in the GraphQL schema noting that iOS offers inherit currency from the parent product:""" Currency code (ISO 4217, e.g., "USD") Note: iOS offers inherit currency from parent product; this field may be null on iOS. """ currency: String
packages/google/openiap/src/play/java/dev/hyo/openiap/utils/BillingConverters.kt
Show resolved
Hide resolved
- Fix Swift cast warning: access subscriptionOffers directly from ProductIOS - Fix Kotlin type error: use non-null assertion for String? values in when - Replace hardcoded paths with environment variables in sync commands - Use $IAP_REPOS_HOME for platform SDK repositories - Use $OPENIAP_HOME for openiap monorepo - Add environment setup instructions
- Fix forEach with await: use for...of loop in offer.tsx TypeScript example - Fix deprecated type names in ios.tsx: DiscountOffer → DiscountOfferIOS, Discount → DiscountIOS - Update SEO metadata with correct IOS-suffixed type names
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/google/Example/src/main/java/dev/hyo/martie/screens/uis/ProductCard.kt (1)
126-160: Guard against empty-string defaults in standardized offers.On Line 130 and Lines 151-159, empty-string defaults (e.g., Horizon) can bypass the null fallback and render blank currency/discount text. Consider treating blank strings as missing to fall back cleanly.
🛠️ Suggested fix
- val currencyCode = standardizedDiscount?.currency ?: legacyOffer?.priceCurrencyCode ?: "" + val currencyCode = standardizedDiscount?.currency?.takeIf { it.isNotBlank() } + ?: legacyOffer?.priceCurrencyCode.orEmpty() Text( "$currencyCode ${String.format(Locale.getDefault(), "%.2f", fullPrice)}", style = MaterialTheme.typography.bodyMedium, color = AppColors.textSecondary, textDecoration = androidx.compose.ui.text.style.TextDecoration.LineThrough ) } } @@ - val discountText: String = when { + val formattedDiscountAmount = + standardizedDiscount?.formattedDiscountAmountAndroid?.takeIf { it.isNotBlank() } + val legacyFormattedDiscountAmount = + discountInfo?.discountAmount?.formattedDiscountAmount?.takeIf { it.isNotBlank() } + val discountText: String = when { // Prefer standardized DiscountOffer fields standardizedDiscount?.percentageDiscountAndroid != null -> "${standardizedDiscount.percentageDiscountAndroid}% OFF" - standardizedDiscount?.formattedDiscountAmountAndroid != null -> - standardizedDiscount.formattedDiscountAmountAndroid!! + formattedDiscountAmount != null -> + formattedDiscountAmount // Fall back to legacy discountDisplayInfo discountInfo?.percentageDiscount != null -> "${discountInfo.percentageDiscount}% OFF" - discountInfo?.discountAmount?.formattedDiscountAmount != null -> - discountInfo.discountAmount!!.formattedDiscountAmount + legacyFormattedDiscountAmount != null -> + legacyFormattedDiscountAmount else -> "SALE" }
🤖 Fix all issues with AI agents
In @.claude/commands/sync-expo-iap.md:
- Around line 264-270: The fenced code block showing commit messages uses a bare
triple-backtick; change that to include a language identifier (e.g., replace the
opening ``` with ```text) so the block becomes explicit for markdownlint MD040
and code renderers; update the block containing "feat: add discount offer
support / fix: resolve iOS purchase verification / docs: update subscription
flow guide" to start with ```text.
- Around line 272-276: In the "References" section replace the bare URLs for
OpenIAP Docs and expo-iap Docs with markdown link syntax (e.g., [OpenIAP
Docs](https://openiap.dev/docs) and [expo-iap
Docs](https://expo-iap.vercel.app)) so the two lines currently showing raw URLs
become proper markdown links; update the lines referencing those two URLs
accordingly to satisfy MD034.
Set IAP_REPOS_HOME=/Users/hyo/Github/hyochan and OPENIAP_HOME=/Users/hyo/Github/hyodotdev
Summary by CodeRabbit
New Features
Documentation
Deprecations
Tests
✏️ Tip: You can customize this high-level summary in your review settings.