[in_app_purchase_android] Add showInAppMessages support (#107412)#11214
[in_app_purchase_android] Add showInAppMessages support (#107412)#11214fingerart wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for the showInAppMessages API from the Google Play Billing Library. This includes defining the new API in Pigeon, implementing it on the Android native side, adding Dart wrappers, and updating the example app to demonstrate its usage. Tests for the new functionality have also been added.
|
@stuartmorgan-g @GaryQian What else do I need to do? |
|
@fingerart You need to wait for a review, which as our docs explain takes up to a couple of weeks. |
|
There are 6 failed checks, what do I need to do about it? |
You can see the task output by going to In this case they are all analyzer failures, which should trivially show up locally in any IDE configured to read repo-local analysis options. |
8d3a9cc to
410fa60
Compare
|
I have fixed the error checking item. |
gmackall
left a comment
There was a problem hiding this comment.
Mostly lgtm, one question about future proofing re: the categories of message to show.
Also, it would be good to add a test to packages/in_app_purchase/in_app_purchase_android/test/in_app_purchase_android_platform_addition_test.dart as the other methods of in_app_purchase_android_platform_addition.dart are also tested there
something like
group('showInAppMessages', () {
test('showInAppMessages success', () async {
const expected = InAppMessageResultWrapper(
responseCode: InAppMessageResponse.subscriptionStatusUpdated,
purchaseToken: 'dummy purchase token',
);
when(mockApi.showInAppMessages()).thenAnswer(
(_) async => PlatformInAppMessageResult(
responseCode: PlatformInAppMessageResponse.subscriptionStatusUpdated,
purchaseToken: expected.purchaseToken,
),
);
final InAppMessageResultWrapper result =
await iapAndroidPlatformAddition.showInAppMessages();
expect(result, equals(expected));
});
});410fa60 to
3200139
Compare
|
I added the |
|
lgtm, @bparrishMines would you like to do the secondary review? |
bparrishMines
left a comment
There was a problem hiding this comment.
LGTM with a couple nits
c72c1f7 to
2e060b6
Compare
|
@fingerart This needs to merge in the main branch and handle merging the pigeon Kotlin update. This PR doesn't allow maintainers to make changes, so I can't do this for you. |
2e060b6 to
db93292
Compare
|
@bparrishMines I have updated to Kotlin. |
bparrishMines
left a comment
There was a problem hiding this comment.
This looks good. I think the version bump got messed up with the kotlin merge. The pubspec version needs to updated along with my comments in the changelog.
db93292 to
503ead0
Compare
22ae8f4 to
cdc35c7
Compare
|
This pull request is not mergeable in its current state, likely because of a merge conflict. Pre-submit CI jobs were not triggered. Pushing a new commit to this branch that resolves the issue will result in pre-submit jobs being scheduled. |
cdc35c7 to
ba97bc9
Compare
ba97bc9 to
8b3523e
Compare
8b3523e to
5eac845
Compare
5eac845 to
58b8b2c
Compare
58b8b2c to
ee3e06a
Compare
As mentioned in #107412, a new native feature (showInAppMessages) has been added.
Pre-Review Checklist
[shared_preferences]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2