-
Notifications
You must be signed in to change notification settings - Fork 16
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
PixelKit adoption #2557
Merged
Merged
PixelKit adoption #2557
Changes from 15 commits
Commits
Show all changes
48 commits
Select commit
Hold shift + click to select a range
2490e15
DailyPixels moved to PixelKit
federicocappelli a08d287
many more pixels migrated to PixelKit
federicocappelli 227c2b5
all pixels migrated to PixelKit
federicocappelli afbb6a6
more pixels migrated and unit tests now build
federicocappelli c2b6453
Unit tests fixed and PixelExperiment removed
federicocappelli 6eb1b98
lint and comments added
federicocappelli a8a62ac
PixelKit API cleanup
federicocappelli 04de217
PixelKit safety checks
federicocappelli 2262709
Merge branch 'main' into fcappelli/PixelKit
federicocappelli 70849bc
Migrate attribution pixel to pixel kit (#2564)
alessandroboron 54c0cf0
clearFrequencyHistoryForAllPixels implemented with new "reset Pixels …
federicocappelli 32bbabe
Merge branch 'main' into fcappelli/PixelKit
federicocappelli 1e15a9b
Update sandbox-test-tool.xcscheme
federicocappelli 069183f
reverting unrelated changes
federicocappelli e6a73d6
Merge branch 'fcappelli/PixelKit' of https://github.com/duckduckgo/ma…
federicocappelli a452537
pixel logs fired even in dry mode
federicocappelli 746a29d
.legacyDaily pixel frequency added for support daily
federicocappelli 6de49c3
PixelKit Unit tests fixed
federicocappelli 3e8a24c
Merge branch 'main' into fcappelli/PixelKit
federicocappelli 245c603
Merge branch 'main' into fcappelli/PixelKit
federicocappelli 3784405
Merge branch 'main' into fcappelli/PixelKit
federicocappelli 88d0f27
unit tests fix
federicocappelli e75d9de
Merge branch 'main' into fcappelli/PixelKit
federicocappelli 5564fbe
Merge branch 'main' into fcappelli/PixelKit
federicocappelli 53bbb6b
source change
federicocappelli 442c0b7
Diego's review comments improvements
federicocappelli 158da93
Merge branch 'main' into fcappelli/PixelKit
federicocappelli 932c480
Merge branch 'main' into fcappelli/PixelKit
federicocappelli 1743e59
style fixed
federicocappelli c7f2f93
Merge branch 'main' into fcappelli/PixelKit
federicocappelli c46b890
unit tests fixed after bad merge
federicocappelli b7d31a6
sandbox tool version reverted
federicocappelli a21c5fa
PR comments addressed
federicocappelli 541f86d
switch default removed
federicocappelli f7f9795
Merge branch 'main' into fcappelli/PixelKit
federicocappelli ca0f0b5
Merge branch 'main' into fcappelli/PixelKit
federicocappelli 12fd8b9
logs params are now public
federicocappelli e1c789f
Merge branch 'main' into fcappelli/PixelKit
federicocappelli 6ff3e5e
Date code duplication removed
federicocappelli aa7adb4
Legacy Pixel class and related files removed
federicocappelli 2367796
list style fixed
federicocappelli 1ae905b
pixels unit tests improved
federicocappelli 47bc219
swift-snapshot-testing downgraded to previous version
federicocappelli 880aadb
Merge branch 'main' into fcappelli/PixelKit
federicocappelli a6c24d5
Merge branch 'main' into fcappelli/PixelKit
federicocappelli 98a2c7a
Merge branch 'main' into fcappelli/PixelKit
federicocappelli 6bbd209
unit tests fixed
federicocappelli 1c17e9a
reverting xcode 15.3 changes
federicocappelli File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -97,6 +97,15 @@ final class AppDelegate: NSObject, NSApplicationDelegate { | |
var updateController: UpdateController! | ||
#endif | ||
|
||
static private var aMonthAgo = Calendar.current.date(byAdding: .month, value: -1, to: Date())! // Temporary for init | ||
@UserDefaultsWrapper(key: .firstLaunchDate, defaultValue: aMonthAgo) | ||
static var firstLaunchDate: Date | ||
|
||
static var isNewUser: Bool { | ||
let oneWeekAgo = Calendar.current.date(byAdding: .weekOfYear, value: -1, to: Date())! | ||
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. same as above, use Date extension from 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. removed, thanks |
||
return firstLaunchDate >= oneWeekAgo | ||
} | ||
|
||
// swiftlint:disable:next function_body_length | ||
override init() { | ||
do { | ||
|
@@ -111,22 +120,16 @@ final class AppDelegate: NSObject, NSApplicationDelegate { | |
internalUserDecider = DefaultInternalUserDecider(store: internalUserDeciderStore) | ||
|
||
if NSApplication.runType.requiresEnvironment { | ||
#if DEBUG | ||
Pixel.setUp(dryRun: true) | ||
Self.setUpPixelKit(dryRun: true) | ||
#else | ||
Pixel.setUp() | ||
Self.setUpPixelKit(dryRun: false) | ||
#endif | ||
Self.configurePixelKit() | ||
|
||
Database.shared.loadStore { _, error in | ||
guard let error = error else { return } | ||
|
||
switch error { | ||
case CoreDataDatabase.Error.containerLocationCouldNotBePrepared(let underlyingError): | ||
Pixel.fire(.debug(event: .dbContainerInitializationError, error: underlyingError)) | ||
PixelKit.fire(DebugEvent(GeneralPixel.dbContainerInitializationError(error: underlyingError))) | ||
default: | ||
Pixel.fire(.debug(event: .dbInitializationError, error: error)) | ||
PixelKit.fire(DebugEvent(GeneralPixel.dbInitializationError(error: error))) | ||
} | ||
|
||
// Give Pixel a chance to be sent, but not too long | ||
|
@@ -135,12 +138,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate { | |
} | ||
|
||
let preMigrationErrorHandling = EventMapping<BookmarkFormFactorFavoritesMigration.MigrationErrors> { _, error, _, _ in | ||
if let error = error { | ||
Pixel.fire(.debug(event: .bookmarksCouldNotLoadDatabase, error: error)) | ||
} else { | ||
Pixel.fire(.debug(event: .bookmarksCouldNotLoadDatabase)) | ||
} | ||
|
||
PixelKit.fire(DebugEvent(GeneralPixel.bookmarksCouldNotLoadDatabase(error: error))) | ||
Thread.sleep(forTimeInterval: 1) | ||
fatalError("Could not create Bookmarks database stack: \(error?.localizedDescription ?? "err")") | ||
} | ||
|
@@ -154,12 +152,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate { | |
|
||
BookmarkDatabase.shared.db.loadStore { context, error in | ||
guard let context = context else { | ||
if let error = error { | ||
Pixel.fire(.debug(event: .bookmarksCouldNotLoadDatabase, error: error)) | ||
} else { | ||
Pixel.fire(.debug(event: .bookmarksCouldNotLoadDatabase)) | ||
} | ||
|
||
PixelKit.fire(DebugEvent(GeneralPixel.bookmarksCouldNotLoadDatabase(error: error))) | ||
Thread.sleep(forTimeInterval: 1) | ||
fatalError("Could not create Bookmarks database stack: \(error?.localizedDescription ?? "err")") | ||
} | ||
|
@@ -193,6 +186,14 @@ final class AppDelegate: NSObject, NSApplicationDelegate { | |
#endif | ||
} | ||
|
||
static func configurePixelKit() { | ||
#if DEBUG | ||
Self.setUpPixelKit(dryRun: true) | ||
#else | ||
Self.setUpPixelKit(dryRun: false) | ||
#endif | ||
} | ||
|
||
func applicationWillFinishLaunching(_ notification: Notification) { | ||
APIRequest.Headers.setUserAgent(UserAgent.duckDuckGoUserAgent()) | ||
Configuration.setURLProvider(AppConfigurationURLProvider()) | ||
|
@@ -230,11 +231,12 @@ final class AppDelegate: NSObject, NSApplicationDelegate { | |
_ = RecentlyClosedCoordinator.shared | ||
|
||
// Clean up previous experiment | ||
if PixelExperiment.allocatedCohortDoesNotMatchCurrentCohorts { | ||
PixelExperiment.cleanup() | ||
} | ||
// if PixelExperiment.allocatedCohortDoesNotMatchCurrentCohorts { // Re-implement https://app.asana.com/0/0/1207002879349166/f | ||
federicocappelli marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// PixelExperiment.cleanup() | ||
// } | ||
|
||
if LocalStatisticsStore().atb == nil { | ||
Pixel.firstLaunchDate = Date() | ||
AppDelegate.firstLaunchDate = Date() | ||
// MARK: Enable pixel experiments here | ||
} | ||
AtbAndVariantCleanup.cleanup() | ||
|
@@ -383,7 +385,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate { | |
appVersion: AppVersion.shared.versionNumber, | ||
source: source, | ||
defaultHeaders: [:], | ||
log: .networkProtectionPixel, | ||
defaults: .netP) { (pixelName: String, headers: [String: String], parameters: [String: String], _, _, onComplete: @escaping PixelKit.CompletionBlock) in | ||
|
||
let url = URL.pixelUrl(forPixelNamed: pixelName) | ||
|
@@ -442,9 +443,9 @@ final class AppDelegate: NSObject, NSApplicationDelegate { | |
.filter { $0 } | ||
.asVoid() | ||
.sink { [weak syncService] in | ||
Pixel.fire(.syncDaily, limitTo: .dailyFirst) | ||
PixelKit.fire(GeneralPixel.syncDaily, frequency: .daily) | ||
syncService?.syncDailyStats.sendStatsIfNeeded(handler: { params in | ||
Pixel.fire(.syncSuccessRateDaily, withAdditionalParameters: params) | ||
PixelKit.fire(GeneralPixel.syncSuccessRateDaily, withAdditionalParameters: params) | ||
}) | ||
} | ||
|
||
|
@@ -525,9 +526,9 @@ final class AppDelegate: NSObject, NSApplicationDelegate { | |
} | ||
|
||
private func emailDidSignInNotification(_ notification: Notification) { | ||
Pixel.fire(.emailEnabled) | ||
if Pixel.isNewUser { | ||
Pixel.fire(.emailEnabledInitial, limitTo: .initial) | ||
PixelKit.fire(GeneralPixel.emailEnabled) | ||
if AppDelegate.isNewUser { | ||
PixelKit.fire(GeneralPixel.emailEnabledInitial, frequency: .legacyInitial) | ||
} | ||
|
||
if let object = notification.object as? EmailManager, let emailManager = syncDataProviders.settingsAdapter.emailManager, object !== emailManager { | ||
|
@@ -536,18 +537,17 @@ final class AppDelegate: NSObject, NSApplicationDelegate { | |
} | ||
|
||
private func emailDidSignOutNotification(_ notification: Notification) { | ||
Pixel.fire(.emailDisabled) | ||
PixelKit.fire(GeneralPixel.emailDisabled) | ||
if let object = notification.object as? EmailManager, let emailManager = syncDataProviders.settingsAdapter.emailManager, object !== emailManager { | ||
syncService?.scheduler.notifyDataChanged() | ||
} | ||
} | ||
|
||
@objc private func dataImportCompleteNotification(_ notification: Notification) { | ||
if Pixel.isNewUser { | ||
Pixel.fire(.importDataInitial, limitTo: .initial) | ||
if AppDelegate.isNewUser { | ||
PixelKit.fire(GeneralPixel.importDataInitial, frequency: .legacyInitial) | ||
} | ||
} | ||
|
||
} | ||
|
||
func updateSubscriptionStatus() { | ||
|
@@ -559,7 +559,7 @@ func updateSubscriptionStatus() { | |
|
||
if case .success(let subscription) = await SubscriptionService.getSubscription(accessToken: token, cachePolicy: .reloadIgnoringLocalCacheData) { | ||
if subscription.isActive { | ||
DailyPixel.fire(pixel: .privacyProSubscriptionActive, frequency: .dailyOnly) | ||
PixelKit.fire(PrivacyProPixel.privacyProSubscriptionActive, frequency: .daily) | ||
} | ||
} | ||
|
||
|
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Code moved from Pixel.swift to AppDelegate, I'm sure this isn't the right place or approach but it's comparable to the previous one.
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.
we have
Date.monthAgo
inBSK.Common.DateExtensions
, worths removing thisThere 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.
Much better, thanks, fixed