PostReceiptOperation: added ability to also post AdServices token#2549
Conversation
PostReceiptOperation: added ability to also post AdServices tokenPostReceiptOperation: added ability to also post AdServices token
ed50d30 to
bf57b6a
Compare
aboedo
left a comment
There was a problem hiding this comment.
looks great! Left a couple of small comments
There was a problem hiding this comment.
feels odd for subs attributes manager to be in charge for this - there's no subs attribute involved, right?
There was a problem hiding this comment.
I realize that this is piping through deviceCache anyway, but I feel like going directly might be more appropriate in this case - there's no subs attribute involved, but if I were reading this code with no context, I'd assume that the token is stored as a subs attribute
If we want to share the code, I think this could be either a part of DeviceCache itself or its own object, it just feels arbitrary to add it to subs attributes manager since it's not really related
There was a problem hiding this comment.
Hmm I agree. That's what I had originally, I think I moved this so I could mock it, let me see.
There was a problem hiding this comment.
It's mainly because we don't mock AttributionPoster, and I wanted to verify this is called, but I'll see if I can do it in another way.
aboedo
left a comment
There was a problem hiding this comment.
looks great! Left a couple of small comments
bf57b6a to
25f59d7
Compare
- Added `aadAttributionToken` to `PostReceiptDataOperation` - Exposed `AttributionPoster.adServicesTokenToPostIfNeeded` - Added snapshot test to verify it's sent - Added `PurchasesOrchestrator` tests (SK1/SK2) for sending the attribution token - Added missing `PurchasesOrchestrator` tests for sending attributes - Added log when marking `AdServices` token as synced - Exposed `Purchases.attribution` for custom entitlement computation framework (and added to API tester) - Exposed `Purchases.enableAdServicesAttributionTokenCollection` for custom entitlement computation framework (and added to API tester)
25f59d7 to
530dc1b
Compare
|
Updated this to point to |
Codecov Report
@@ Coverage Diff @@
## release/4.19.0 #2549 +/- ##
==================================================
+ Coverage 87.70% 87.77% +0.07%
==================================================
Files 197 197
Lines 13278 13332 +54
==================================================
+ Hits 11645 11702 +57
+ Misses 1633 1630 -3
|
|
|
||
| guard let attributionToken = attributionFetcher.adServicesToken else { | ||
| return | ||
| guard self.latestNetworkIdAndAdvertisingIdentifierSent(network: .adServices) == nil else { |
There was a problem hiding this comment.
I've not been involved in the specs so this might be correct, but with this, only the first purchase for a user will send the token right? As in, if a user unsubscribes, the purchase expires and then resubscribes, the second time it won't be sent. Not sure if this token can change, so it might be ok to just send it the first time, but I thought I would mention it.
There was a problem hiding this comment.
yeah, that should be fine. That's the same behavior we have for adservices in other places.
And in practice, attribution tokens expire quickly, and a user will only be attributed once
aboedo
left a comment
There was a problem hiding this comment.
thanks for the fast work here!
…2551) Because `AAAttribution.attributionToken()` isn't available on simulators, we currently had no coverage for this entire part of the codebase. By mocking the token (on simulator + debug + integration tests only), we can verify posting to the server works correctly. As a follow-up, we can also verify that posting it as part of the receipt with #2549 works correctly as well.
|
Post-refactor version: #2566. |
…#2566) Cherry-picked #2549 post refactors (#2540 and #2542). This becomes a lot simpler since #2542, thanks to the fact that we can put this new parameter in `PurchasedTransactionData`. ### Changes: - Added `aadAttributionToken` to `PurchasedTransactionData` - Exposed `AttributionPoster.adServicesTokenToPostIfNeeded` - Added snapshot test to verify it's sent - Added `PurchasesOrchestrator` tests (SK1/SK2) for sending the attribution token - Added missing `PurchasesOrchestrator` tests for sending attributes - Added log when marking `AdServices` token as synced - Exposed `Purchases.attribution` for custom entitlement computation framework (and added to API tester) - Exposed `Purchases.enableAdServicesAttributionTokenCollection` for custom entitlement computation framework (and added to API tester)
Changes:
aadAttributionTokentoPostReceiptDataOperationAttributionPoster.adServicesTokenToPostIfNeededPurchasesOrchestratortests (SK1/SK2) for sending the attribution tokenPurchasesOrchestratortests for sending attributesAdServicestoken as syncedPurchases.attributionfor custom entitlement computation framework (and added to API tester)Purchases.enableAdServicesAttributionTokenCollectionfor custom entitlement computation framework (and added to API tester)