Fix incorrect Android replacement mode#93
Conversation
📝 WalkthroughWalkthroughFixed incorrect integer mappings for Android replacement modes in the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libraries/flutter_inapp_purchase/lib/enums.dart`:
- Around line 170-173: The enum-to-integer mapping for AndroidReplacementMode
was swapped (AndroidReplacementMode.chargeFullPrice now returns 4 and
AndroidReplacementMode.deferred returns 5); find all dependent tests, examples,
and any serialization/deserialization code that reference the old numeric values
(search for literal usages of 4 and 5 tied to AndroidReplacementMode, usages of
AndroidReplacementMode.chargeFullPrice or AndroidReplacementMode.deferred in
tests/examples) and update them to match the new mapping so chargeFullPrice maps
to 4 and deferred maps to 5, ensuring assertions, fixtures, and sample data are
adjusted accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6bc5c1d1-5878-4f3d-a2c0-9138785d5753
📒 Files selected for processing (1)
libraries/flutter_inapp_purchase/lib/enums.dart
| case AndroidReplacementMode.chargeFullPrice: | ||
| return 4; | ||
| case AndroidReplacementMode.deferred: | ||
| return 5; |
There was a problem hiding this comment.
Sync dependent tests/examples with this new mapping to prevent drift.
Lines 170-173 switch chargeFullPrice/deferred values, but existing references still encode the previous mapping (chargeFullPrice=5, deferred=4). Please update those usages in the same PR to avoid failing tests and contradictory sample behavior.
Suggested follow-up updates
--- a/libraries/flutter_inapp_purchase/test/enums_unit_test.dart
+++ b/libraries/flutter_inapp_purchase/test/enums_unit_test.dart
@@
- expect(AndroidReplacementMode.chargeFullPrice.value, 5);
+ expect(AndroidReplacementMode.chargeFullPrice.value, 4);
+ expect(AndroidReplacementMode.deferred.value, 5);--- a/libraries/flutter_inapp_purchase/example/lib/src/screens/subscription_flow_screen.dart
+++ b/libraries/flutter_inapp_purchase/example/lib/src/screens/subscription_flow_screen.dart
@@
- 'Deferred': 4,
- 'Immediate and Charge Full Price': 5,
+ 'Immediate and Charge Full Price': 4,
+ 'Deferred': 5,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libraries/flutter_inapp_purchase/lib/enums.dart` around lines 170 - 173, The
enum-to-integer mapping for AndroidReplacementMode was swapped
(AndroidReplacementMode.chargeFullPrice now returns 4 and
AndroidReplacementMode.deferred returns 5); find all dependent tests, examples,
and any serialization/deserialization code that reference the old numeric values
(search for literal usages of 4 and 5 tied to AndroidReplacementMode, usages of
AndroidReplacementMode.chargeFullPrice or AndroidReplacementMode.deferred in
tests/examples) and update them to match the new mapping so chargeFullPrice maps
to 4 and deferred maps to 5, ensuring assertions, fixtures, and sample data are
adjusted accordingly.
There was a problem hiding this comment.
Code Review
This pull request modifies the AndroidReplacementModeValue extension in libraries/flutter_inapp_purchase/lib/enums.dart to swap the integer return values for AndroidReplacementMode.chargeFullPrice and AndroidReplacementMode.deferred. I have no feedback to provide as there were no review comments to evaluate.
|
Thanks for the report and PR! I looked into this carefully and the proposed values are not correct for this code path. Details below. Why the PR is wrongThe Flutter val replacementMode = androidArgs.replacementMode ?: 5 // Default to CHARGE_FULL_PRICE
@Suppress("DEPRECATION")
updateParamsBuilder.setSubscriptionReplacementMode(replacementMode)That legacy setter consumes values from I verified this against the shipped Billing Library 8.3.0 bytecode:
What the PR changes (before → after)
So this PR breaks the previously-correct It also breaks the existing unit test at expect(AndroidReplacementMode.chargeFullPrice.value, 5);Correct fixOnly case AndroidReplacementMode.deferred:
return 6;
case AndroidReplacementMode.chargeFullPrice:
return 5;Please also extend Side note: the Kotlin Could you update the PR accordingly? Happy to help review once pushed. |
|
Superseded by #96, which lands the corrected legacy values ( |
|
Superseded by #96 (merged). |
fix #92
Summary by CodeRabbit