fix(android): prevent product data overwrite in fetchProducts with type 'all'#3044
fix(android): prevent product data overwrite in fetchProducts with type 'all'#3044CacaoRick wants to merge 1 commit intohyochan:mainfrom
Conversation
…pe 'all' Previously, when fetching products with type 'all', the code manually queried InApp and Subs separately, then merged results using a map. This caused valid InApp products to be overwritten by empty/incomplete data from the Subs query when the same SKU was queried for both types. OpenIAP's fetchProducts already handles ProductQueryType.All correctly with duplicate prevention using processedIds. This commit removes the redundant manual implementation and directly uses OpenIAP's logic. Fixes issue where InApp products lose price information when mixed with subscription products in a single 'all' type query.
WalkthroughSimplifies fetchProducts in HybridRnIap.kt to a single-path flow using the computed queryType. Removes the previous All-case branching and aggregation, consolidates logging to a single payload and result, and retains updating productTypeBySku and returning converted NitroProduct lists. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant JS as RN Caller
participant IAP as HybridRnIap.fetchProducts
participant BC as BillingClient (query)
participant Map as productTypeBySku
JS->>IAP: fetchProducts(skus, queryType)
IAP->>IAP: compute effective queryType
IAP->>BC: queryProducts(queryType, skus)
BC-->>IAP: products[]
IAP->>Map: update per SKU type
IAP-->>JS: NitroProduct[]
note over IAP: Single-path flow replaces prior All-case branching
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)android/src/main/java/com/margelo/nitro/iap/**/*.kt📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (1)android/src/main/java/com/margelo/nitro/iap/HybridRnIap.kt (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
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 @CacaoRick, 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 resolves a critical bug in the Android Highlights
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 pull request effectively addresses the issue of product data being overwritten when fetching mixed product types. By removing the manual implementation for handling ProductQueryType.All and instead relying on the underlying OpenIapModule's fetchProducts method, the code is now much simpler and more robust. The change correctly fixes the bug where in-app product information was lost during mixed-type queries. This simplification is a great improvement, and the changes look solid.
|
Wait, I noticed that there is a bug in the cache of OpenIapModule's The first final line |
Closing This PRAfter further investigation, I've discovered that this issue stems from bugs in the underlying OpenIAP library that cannot be reliably worked around at the react-native-iap level. Root Cause AnalysisIssue 1: OpenIAP
|
|
@CacaoRick Hey good catch! I'll manage to fix this here hyodotdev/openiap-apple#16 and release to |
|
@CacaoRick Released |
|
Thank you for your maintenance. |
Problem
When using
fetchProductswithtype: 'all'to query mixed product types (e.g., both InApp and Subscription SKUs), InApp products lose their price information.Reproduction Steps
fetchProducts(['inapp_sku', 'subs_sku'], 'all')pricefield isnullanddisplayPrice,currencyfields are""Root Cause
The original implementation handled
ProductQueryType.Allby:inapp_skuwith complete price informationmap[sku] = productto unconditionally overwrite → valid InApp product data gets overwrittenSolution
OpenIAP's
fetchProductsmethod already correctly implementsProductQueryType.Alllogic:Uses processedIds to track already-processed product IDs
Prevents duplicate additions of the same product
Correctly handles mixed-type queries
Reference: OpenIAP fetchProducts implementation
This PR removes the redundant manual implementation and directly uses OpenIAP's functionality.
Testing
Tested in my app with the following scenarios:
Before this fix, InApp products would return
{ displayPrice:"", currency:"", price:null }when queried together with subscription products using type'all'. After this fix, all products maintain their complete data.Summary by CodeRabbit
New Features
Refactor
Performance
Stability
Observability