-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[in_app_purchase] Federated Android implementation #3841
[in_app_purchase] Federated Android implementation #3841
Conversation
|
|
I think we should add examples for this plugin as well, also have a readme to document well how to use only android implementations if people want to (since we advertised it in our readme already) We should also do the same for the iOS package. However, these are not priorities since most of the people will use the unified API. But still, I think this can be on our future roadmap. We should create an issue and add TODOs to track it. I created the issue: flutter/flutter#81695, do you mind adding TODO in this PR? |
packages/in_app_purchase/in_app_purchase_android/analysis_options.yaml
Outdated
Show resolved
Hide resolved
packages/in_app_purchase/in_app_purchase_android/lib/src/in_app_purchase_android_platform.dart
Outdated
Show resolved
Hide resolved
packages/in_app_purchase/in_app_purchase_android/lib/src/in_app_purchase_android_platform.dart
Outdated
Show resolved
Hide resolved
packages/in_app_purchase/in_app_purchase_android/lib/src/in_app_purchase_android_platform.dart
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,23 @@ | |||
/// Thrown to indicate that an action failed while interacting with the | |||
/// in_app_purchase plugin. | |||
class InAppPurchaseException implements Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this android only? Does it make sense to be in the platform_interface?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment it is only used in the Android package. On iOS we don't need this specific exception. However I can imagine the exception to be useful on other platforms in the future.
So I wasn't sure to add it now to the platform_interface or later if we really need it on multiple platform (by that time we might have forgotten that there is a similar class in the Android implementation).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think it belongs to the platform_interface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a separate PR (#3852) which adds the InAppPurchaseException
to the platform_interface. Once that is merged I will make sure it gets removed from this PR.
packages/in_app_purchase/in_app_purchase_android/lib/src/in_app_purchase_android_platform.dart
Outdated
Show resolved
Hide resolved
packages/in_app_purchase/in_app_purchase_android/lib/src/in_app_purchase_android_platform.dart
Outdated
Show resolved
Hide resolved
91f05e7
to
b9f9778
Compare
No problem at all, but I am not sure where I would add these TODOs (as in which files)? Since there is no example, where would it make the most sense to add a TODO so we encounter it in the future? |
In the README.md, we can add todo saying we need to add example apps. |
..._app_purchase/in_app_purchase_android/lib/src/in_app_purchase_android_platform_addition.dart
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,23 @@ | |||
/// Thrown to indicate that an action failed while interacting with the | |||
/// in_app_purchase plugin. | |||
class InAppPurchaseException implements Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think it belongs to the platform_interface
Done. I also added a similar comment to the in_app_purchase_ios README.md file as part of PR #3851. |
..._app_purchase/in_app_purchase_android/lib/src/in_app_purchase_android_platform_addition.dart
Show resolved
Hide resolved
..._app_purchase/in_app_purchase_android/lib/src/in_app_purchase_android_platform_addition.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Implement BillingClientWrapper and unit-tests * Android specific implementation * Moved Android specific methods into addition class * Added missing line end * Purchases status to restored after call restorePurchases * Don't force GooglePlayPurchaseParam * Implement registerPlatform method * Added TODO comment to add example * Fixed mistake in API documentation * Added additional assert to test enablePendingPurchase * Update Android implementation with latest platform_interface
This PR adds the Android implementation of the new "in_app_purchase_platform_interface" (see PR #3781).
Note: when running Flutter commands for the
in_app_purchase_android
package I keep getting the warning below. I think this is caused by the fact that thein_app_purchase_android
is missing aMainActivity.java
(since is contains pure plugin code and noexample
which will be part of thein_app_purchase
package), but I am not sure. If I made a configuration mistake I would be happy to fix it.Warning:
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
dart format
. See plugin_tool format)[shared_preferences]
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.