Reward Verification APIs#6696
Merged
Merged
Conversation
7736b9a to
f6d079b
Compare
6158e97 to
4cb910d
Compare
polmiro
commented
Apr 24, 2026
c0ed817 to
64067af
Compare
Member
Author
|
@RCGitBot please test |
Member
Author
|
@RCGitBot please test |
Member
Author
|
@RCGitBot please test |
38bdafa to
c9e8e0a
Compare
Contributor
|
This change is part of the following stack: Change managed by git-spice. |
6e871e7 to
988a8f2
Compare
c9e8e0a to
b096c5e
Compare
2 tasks
b096c5e to
6e4e9da
Compare
988a8f2 to
e1207e3
Compare
ajpallares
reviewed
May 7, 2026
ajpallares
left a comment
Member
There was a problem hiding this comment.
Some final comments, mostly to simplify the code even further
Move invalid virtual-currency handling into VerifiedReward with assertion + fallback, and centralize the assertion string while simplifying mapping logic.
Make unsupportedReward the canonical public representation and remove the transient unknown naming from the adapter API surface.
Simplify handler control flow by gating state checks on a non-nil result callback and invoke `.failed` when state is unexpectedly missing so production callers receive a terminal outcome.
Unify the reward-earned handler into a single closure path that invokes the start callback once and handles missing verification state with a failed result while preserving debug assertions and logging.
Move RewardVerification mapping helpers next to their only call site and remove the now-redundant Present.swift file.
Remove the dedicated placement-override helper and assign the tracked placement directly at the show-time call sites to keep the flow simpler with unchanged behavior.
Drop the test that only re-validates delegate store round-tripping so the suite stays focused on show-time placement override behavior.
…tion-public-api # Conflicts: # AdapterSDKs/RevenueCatAdMob/Sources/RevenueCatAdMob/RewardVerification/Strings.swift
Relocate rewarded ad reward-verification extensions from Tracking into RewardVerification and keep placement wiring in Tracking through a small show-time placement bridge.
Emit a production error log before asserting when reward verification receives a non-positive virtual currency amount, while still falling back to unsupportedReward.
Emit the warning before the debug assert so callback misuse diagnostics are visible in all build configurations.
ajpallares
reviewed
May 7, 2026
ajpallares
left a comment
Member
There was a problem hiding this comment.
I think this is almost ready! I still have some comments before approval.
Also, I think the PR description is stale? E.g. Present.swift is still referenced
Drop the test that only validates delegate-store round-tripping and keep coverage focused on explicit nil placement override behavior.
Remove not-nil assertions that do not add value while keeping the API symbol references in place for signature coverage.
Mark rewardVerificationStarted callbacks as MainActor in public present overloads and helper signatures to make callback threading guarantees explicit and consistent.
Clarify when rewardVerificationStarted and rewardVerificationResult callbacks execute in all reward verification present overloads.
Clarify that verification starts when AdMob invokes the reward callback while keeping result timing wording implementation-agnostic.
ajpallares
approved these changes
May 7, 2026
ajpallares
left a comment
Member
There was a problem hiding this comment.
Thanks for all the iterations on this! I think this is ready.
I still think we should separate the API tests from the unit tests, but we can definitely tackle that separately 👍
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 785db5c. Configure here.
Update API surface method reference annotations so tests fail if MainActor is removed from rewardVerificationStarted callback parameters.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
purchases-androidand hybridsMotivation
Ship the first experimental AdMob reward-verification surface so apps can enable verification per loaded rewarded ad and receive a single, structured verification result after presentation, without coupling verification to
loadAndTrack.Description
enableRewardVerification()onGoogleMobileAds.RewardedAdandRewardedInterstitialAd, delegating toRewardVerification.Setup.install(on:).presentoverloads:present(from:rewardVerificationStarted:rewardVerificationResult:)present(from:placement:rewardVerificationStarted:rewardVerificationResult:)@MainActorand documents callback timing explicitly.RewardedAds+RewardVerification.swift(Outcome->RewardVerificationResult, internalVerifiedReward-> adapterVerifiedReward)..unsupportedRewardVerifiedReward(non-enum reward payload surface)RewardVerificationResult(verified/failed, plusverifiedRewardwhen present)APISurfaceTestsPresentMappingTestsPresentRewardVerificationTestsRewardVerificationResultTestsRewardVerificationPlacementOverrideTestsTests/UnitTests/Ads/RewardVerification/VerifiedRewardTestsNote
Medium Risk
Adds new experimental public APIs around rewarded-ad presentation and reward verification, plus new result types and state checks; mistakes could affect reward delivery callbacks and analytics placement attribution. Changes are scoped to the AdMob adapter and covered by new unit tests, but touch async dispatch and per-ad state/placement handling.
Overview
Introduces an experimental reward-verification API surface for AdMob rewarded formats:
enableRewardVerification()onRewardedAd/RewardedInterstitialAdand newpresent(...)overloads that optionally emitrewardVerificationStartedand a single structuredRewardVerificationResultafter polling completes.Adds public result payload types
VerifiedRewardandRewardVerificationResult, plus internal mapping from RevenueCat’sVerifiedReward/Outcomeinto the adapter types; includes new logging + debug assertions for misuse (result callback without enabling) and invalid virtual-currency amounts (assert + fallback to.unsupportedReward).Updates tracking to allow show-time placement overrides via
Tracking.setShowTimePlacement(...)(used by the newpresent(from:placement:...)overloads), and expands tests (including Nimble in SPM) to cover API availability, mapping, callback ordering, assertion behavior, and placement override semantics.Reviewed by Cursor Bugbot for commit 1cb4d4f. Bugbot is set up for automated code reviews on this repo. Configure here.