-
Notifications
You must be signed in to change notification settings - Fork 316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CF-106] Fetch AdServices Token #1519
Changes from 9 commits
f3131f9
d322c7c
dbcdc4c
c557ba5
882a936
21aa633
cc93cdb
9706097
c98031a
03357e7
bcd58a2
5c957b7
4c6332c
4a952c4
3811408
e02c0d2
81f2628
47ecfff
d51c37f
f2014f8
8923389
e5dea3a
10a073b
352de9e
bae8e3c
d4a552a
c181dc7
f29aedb
d6d3dc7
67b3e7a
4d800ac
2b21373
61c28a3
26b02f7
a11742a
6a7697b
60db73b
1df28a0
3e3d383
cc98cbe
839bc8d
2c16386
42bbabc
3de0061
d9f757b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,10 @@ import UIKit | |
import WatchKit | ||
#endif | ||
|
||
#if os(iOS) | ||
beylmk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import AdServices | ||
#endif | ||
|
||
enum AttributionFetcherError: Error { | ||
|
||
case identifierForAdvertiserUnavailableForPlatform | ||
|
@@ -82,6 +86,21 @@ class AttributionFetcher { | |
#endif | ||
} | ||
|
||
@available(iOS 14.3, *) | ||
func adServicesToken(completion: @escaping (String?, Error?) -> Void) { | ||
// TODO check for library? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think |
||
#if os(iOS) | ||
do { | ||
let attributionToken = try AAAttribution.attributionToken() | ||
completion(attributionToken, nil) | ||
} catch let attributionTokenError { | ||
completion(nil, attributionTokenError) | ||
} | ||
#endif | ||
// todo make error | ||
completion(nil, nil) | ||
} | ||
|
||
var isAuthorizedToPostSearchAds: Bool { | ||
// Should match platforms that require permissions detailed in | ||
// https://developer.apple.com/app-store/user-privacy-and-data-use/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,8 @@ enum StoreKitStrings { | |
|
||
case sk1_no_known_product_type | ||
|
||
case unknown_sk2_product_discount_type(rawValue: String) | ||
|
||
} | ||
|
||
extension StoreKitStrings: CustomStringConvertible { | ||
|
@@ -71,6 +73,10 @@ extension StoreKitStrings: CustomStringConvertible { | |
case .sk1_no_known_product_type: | ||
return "This StoreProduct represents an SK1 product, the type of product cannot be determined, " + | ||
"the value will be undefined. Use `StoreProduct.productCategory` instead." | ||
|
||
case .unknown_sk2_product_discount_type(let rawValue): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍🏻 |
||
return "Failed to create StoreProductDiscount.DiscountType with unknown value: \(rawValue)" | ||
|
||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,9 +91,10 @@ public typealias StartPurchaseBlock = (@escaping PurchaseCompletedBlock) -> Void | |
private let operationDispatcher: OperationDispatcher | ||
|
||
/** | ||
* Enable automatic collection of Apple Search Ads attribution. Defaults to `false`. | ||
* Enable automatic collection of AdServices attribution token. Defaults to `false`. | ||
*/ | ||
@objc public static var automaticAppleSearchAdsAttributionCollection: Bool = false | ||
@available(iOS 14.3, *) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this will need the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep good call. xcode never suggests the full checks for us, do they... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope, only for the currently selected target unfortunately. |
||
@objc public static var automaticAdServicesAttributionTokenCollection: Bool = false | ||
|
||
/** | ||
* Used to set the log level. Useful for debugging issues with the lovely team @RevenueCat. | ||
|
@@ -417,6 +418,10 @@ public typealias StartPurchaseBlock = (@escaping PurchaseCompletedBlock) -> Void | |
attributionPoster.postPostponedAttributionDataIfNeeded() | ||
postAppleSearchAddsAttributionCollectionIfNeeded() | ||
|
||
if #available(iOS 14.3, *) { | ||
postAdServicesTokenIfNeeded() | ||
} | ||
|
||
self.customerInfoObservationDisposable = customerInfoManager.monitorChanges { [weak self] customerInfo in | ||
guard let self = self else { return } | ||
self.delegate?.purchases?(self, receivedUpdated: customerInfo) | ||
|
@@ -439,6 +444,10 @@ public typealias StartPurchaseBlock = (@escaping PurchaseCompletedBlock) -> Void | |
customerInfoObservationDisposable?() | ||
privateDelegate = nil | ||
Self.automaticAppleSearchAdsAttributionCollection = false | ||
|
||
if #available(iOS 14.3, *) { | ||
Self.automaticAdServicesAttributionTokenCollection = false | ||
} | ||
Self.proxyURL = nil | ||
} | ||
|
||
|
@@ -721,6 +730,14 @@ extension Purchases { | |
attributionPoster.postAppleSearchAdsAttributionIfNeeded() | ||
} | ||
|
||
@available(iOS 14.3, *) | ||
private func postAdServicesTokenIfNeeded() { | ||
guard Self.automaticAdServicesAttributionTokenCollection else { | ||
return | ||
} | ||
attributionPoster.postAdServicesTokenIfNeeded() | ||
} | ||
|
||
} | ||
|
||
// MARK: Identity | ||
|
@@ -1822,6 +1839,12 @@ extension Purchases: PurchasesOrchestratorDelegate { | |
|
||
public extension Purchases { | ||
|
||
/** | ||
* Enable automatic collection of Apple Search Ads attribution. Defaults to `false`. | ||
*/ | ||
@available(*, deprecated, message: "use Purchases.automaticAdServicesAttributionTokenCollection for iOS 14.3+ instead") | ||
@objc static var automaticAppleSearchAdsAttributionCollection: Bool = false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe move this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there a difference between methods in Deprecations.swift and those in the // MARK: Deprecated section of Purchases.swift? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I didn't realize we still had some there. The idea was to make the main file (which is already huge) smaller by only having the current API. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. makes sense to me! i can move the rest while i'm at it... |
||
|
||
/** | ||
* Enable debug logging. Useful for debugging issues with the lovely team @RevenueCat. | ||
*/ | ||
|
@@ -1908,6 +1931,10 @@ private extension Purchases { | |
updateAllCachesIfNeeded() | ||
dispatchSyncSubscriberAttributesIfNeeded() | ||
postAppleSearchAddsAttributionCollectionIfNeeded() | ||
|
||
if #available(iOS 14.3, *) { | ||
postAdServicesTokenIfNeeded() | ||
} | ||
} | ||
|
||
@objc func applicationWillResignActive(notification: Notification) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -259,7 +259,7 @@ extension StoreProductDiscount.DiscountType { | |
case SK2ProductDiscount.OfferType.promotional: | ||
return .promotional | ||
default: | ||
Logger.warn(Strings.attribution.unknown_sk2_product_discount_type(rawValue: sk2Discount.type.rawValue)) | ||
Logger.warn(Strings.storeKit.unknown_sk2_product_discount_type(rawValue: sk2Discount.type.rawValue)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. :+1 |
||
return nil | ||
} | ||
} | ||
|
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.
Should this be weakly linked?
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.
In case I don't get to review this again, just wanted to point out that this is probably my most critical comment. I believe this has to be weakly linked so we don't make all users of the SDK implicitly link
AdServices
.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.
+1 on this, this part is a very delicate change that we should research and test out thoroughly.
It might even be a red flag for privacy-minded folks who're looking into using our SDK. Perhaps we can do the extra work of calling this through the same "reflection" we did for iAd?
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.
are you both okay with me doing the linking research as the follow-up ticket i created? would definitely be before merging into main. this just feels like enough work to warrant a separate PR
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.
absolutely! It's only important to get the linking right before merging to main, but it can totally be done as a separate thing
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.
okay then that's the plan!