Skip to content

Commit

Permalink
PixelKit adoption (#2557)
Browse files Browse the repository at this point in the history
Task/Issue
URL:https://app.asana.com/0/1205842942115003/1206999268603553/f
Tech Design
URL:https://app.asana.com/0/1205842942115003/1206989773504216/f
CC: @diegoreymendez 

**Description**:

Migrating the entire app to PixelKit changing pixel.fire() to PixelKit.fire() with
related name and frequency changes. The few logic changes are
concentrated in PixelKit.

- Old Pixel implementation removed
- PixelKit adopted for every Pixel
- PixelKit code improved
- Debug menu "remove daily pixels" changed in "Remove Pixels Storage"
(reset all unique and daily pixels)
  • Loading branch information
federicocappelli authored and diegoreymendez committed Apr 18, 2024
1 parent 1c4eb9a commit 8e97096
Show file tree
Hide file tree
Showing 149 changed files with 1,662 additions and 2,135 deletions.
132 changes: 46 additions & 86 deletions DuckDuckGo.xcodeproj/project.pbxproj

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@
<Test
Identifier = "CBRCompileTimeReporterTests">
</Test>
<Test
Identifier = "PixelExperimentTests">
</Test>
<Test
Identifier = "PixelStoreTests/testWhenValuesAreAddedThenCallbacksAreCalled()">
</Test>
Expand Down
70 changes: 34 additions & 36 deletions DuckDuckGo/Application/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
var updateController: UpdateController!
#endif

@UserDefaultsWrapper(key: .firstLaunchDate, defaultValue: Date.monthAgo)
static var firstLaunchDate: Date

static var isNewUser: Bool {
return firstLaunchDate >= Date.weekAgo
}

// swiftlint:disable:next function_body_length
override init() {
do {
Expand All @@ -109,22 +116,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
Expand All @@ -133,12 +134,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")")
}
Expand All @@ -152,12 +148,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")")
}
Expand Down Expand Up @@ -193,6 +184,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())
Expand Down Expand Up @@ -236,11 +235,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
// PixelExperiment.cleanup()
// }

if LocalStatisticsStore().atb == nil {
Pixel.firstLaunchDate = Date()
AppDelegate.firstLaunchDate = Date()
// MARK: Enable pixel experiments here
}
AtbAndVariantCleanup.cleanup()
Expand Down Expand Up @@ -384,7 +384,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)
Expand Down Expand Up @@ -442,9 +441,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)
})
}

Expand Down Expand Up @@ -525,9 +524,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 {
Expand All @@ -536,18 +535,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() {
Expand All @@ -559,7 +557,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)
}
}

Expand Down
11 changes: 6 additions & 5 deletions DuckDuckGo/Application/URLEventHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import Common
import Foundation
import AppKit
import PixelKit

import NetworkProtectionUI

Expand Down Expand Up @@ -61,15 +62,15 @@ final class URLEventHandler {
@objc func handleUrlEvent(event: NSAppleEventDescriptor, reply: NSAppleEventDescriptor) {
guard let stringValue = event.paramDescriptor(forKeyword: keyDirectObject)?.stringValue else {
os_log("UrlEventListener: unable to determine path", type: .error)
Pixel.fire(.debug(event: .appOpenURLFailed,
error: NSError(domain: "CouldNotGetPath", code: -1, userInfo: nil)))
let error = NSError(domain: "CouldNotGetPath", code: -1, userInfo: nil)
PixelKit.fire(DebugEvent(GeneralPixel.appOpenURLFailed, error: error))
return
}

guard let url = URL.makeURL(from: stringValue) else {
os_log("UrlEventListener: failed to construct URL from path %s", type: .error, stringValue)
Pixel.fire(.debug(event: .appOpenURLFailed,
error: NSError(domain: "CouldNotConstructURL", code: -1, userInfo: nil)))
let error = NSError(domain: "CouldNotConstructURL", code: -1, userInfo: nil)
PixelKit.fire(DebugEvent(GeneralPixel.appOpenURLFailed, error: error))
return
}

Expand Down Expand Up @@ -153,7 +154,7 @@ final class URLEventHandler {
#if SUBSCRIPTION
case AppLaunchCommand.showPrivacyPro.launchURL:
WindowControllersManager.shared.showTab(with: .subscription(.subscriptionPurchase))
Pixel.fire(.privacyProOfferScreenImpression)
PixelKit.fire(PrivacyProPixel.privacyProOfferScreenImpression)
#endif
#if !APPSTORE && !DEBUG
case AppLaunchCommand.moveAppToApplications.launchURL:
Expand Down
9 changes: 5 additions & 4 deletions DuckDuckGo/Application/UpdateController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import Combine
import Sparkle
import BrowserServicesKit
import SwiftUIExtensions
import PixelKit

#if SPARKLE

Expand Down Expand Up @@ -107,7 +108,7 @@ extension UpdateController: SPUUpdaterDelegate {
return
}

Pixel.fire(.debug(event: .updaterAborted, error: error))
PixelKit.fire(DebugEvent(GeneralPixel.updaterAborted, error: error))
}

func updater(_ updater: SPUUpdater,
Expand All @@ -116,11 +117,11 @@ extension UpdateController: SPUUpdaterDelegate {
state: SPUUserUpdateState) {
switch choice {
case .skip:
Pixel.fire(.debug(event: .userSelectedToSkipUpdate))
PixelKit.fire(DebugEvent(GeneralPixel.userSelectedToSkipUpdate))
case .install:
Pixel.fire(.debug(event: .userSelectedToInstallUpdate))
PixelKit.fire(DebugEvent(GeneralPixel.userSelectedToInstallUpdate))
case .dismiss:
Pixel.fire(.debug(event: .userSelectedToDismissUpdate))
PixelKit.fire(DebugEvent(GeneralPixel.userSelectedToDismissUpdate))
@unknown default:
break
}
Expand Down
3 changes: 2 additions & 1 deletion DuckDuckGo/Autofill/AutofillActionExecutor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import Foundation
import BrowserServicesKit
import DDGSync
import AppKit
import PixelKit

/// Conforming types provide an `execute` method which performs some action on autofill types (e.g delete all passwords)
protocol AutofillActionExecutor {
Expand Down Expand Up @@ -68,7 +69,7 @@ struct AutofillDeleteAllPasswordsExecutor: AutofillActionExecutor {
syncService.scheduler.notifyDataChanged()
onSuccess?()
} catch {
Pixel.fire(.debug(event: .secureVaultError, error: error))
PixelKit.fire(DebugEvent(GeneralPixel.secureVaultError(error: error)))
}

return
Expand Down
9 changes: 5 additions & 4 deletions DuckDuckGo/Autofill/ContentOverlayViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import Combine
import BrowserServicesKit
import SecureStorage
import Autofill
import PixelKit

@MainActor
public final class ContentOverlayViewController: NSViewController, EmailManagerRequestDelegate {
Expand Down Expand Up @@ -191,7 +192,7 @@ public final class ContentOverlayViewController: NSViewController, EmailManagerR
parameters["keychain_operation"] = "save"
}

Pixel.fire(.debug(event: .emailAutofillKeychainError, error: error), withAdditionalParameters: parameters)
PixelKit.fire(DebugEvent(GeneralPixel.emailAutofillKeychainError), withAdditionalParameters: parameters)
}

private enum Constants {
Expand Down Expand Up @@ -294,7 +295,7 @@ extension ContentOverlayViewController: SecureVaultManagerDelegate {
}

public func secureVaultManager(_: SecureVaultManager, didAutofill type: AutofillType, withObjectId objectId: String) {
Pixel.fire(.formAutofilled(kind: type.formAutofillKind))
PixelKit.fire(GeneralPixel.formAutofilled(kind: type.formAutofillKind))

if type.formAutofillKind == .password &&
passwordManagerCoordinator.isEnabled {
Expand All @@ -320,9 +321,9 @@ extension ContentOverlayViewController: SecureVaultManagerDelegate {

self.emailManager.updateLastUseDate()

Pixel.fire(.jsPixel(pixel), withAdditionalParameters: pixelParameters)
PixelKit.fire(GeneralPixel.jsPixel(pixel), withAdditionalParameters: pixelParameters)
} else {
Pixel.fire(.jsPixel(pixel), withAdditionalParameters: pixel.pixelParameters)
PixelKit.fire(GeneralPixel.jsPixel(pixel), withAdditionalParameters: pixel.pixelParameters)
}
}

Expand Down
5 changes: 3 additions & 2 deletions DuckDuckGo/Bookmarks/Legacy/LegacyBookmarkStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import Foundation
import CoreData
import Cocoa
import PixelKit

fileprivate extension UUID {

Expand Down Expand Up @@ -171,7 +172,7 @@ extension LegacyBookmarkStore {

try context.save()
} catch {
Pixel.fire(.debug(event: .bookmarksStoreRootFolderMigrationFailed, error: error))
PixelKit.fire(DebugEvent(GeneralPixel.bookmarksStoreRootFolderMigrationFailed, error: error))
}
}

Expand Down Expand Up @@ -211,7 +212,7 @@ extension LegacyBookmarkStore {
// 4. Save the migration:
try context.save()
} catch {
Pixel.fire(.debug(event: .bookmarksStoreRootFolderMigrationFailed, error: error))
PixelKit.fire(DebugEvent(GeneralPixel.bookmarksStoreRootFolderMigrationFailed, error: error))
}
}
}
Expand Down
13 changes: 7 additions & 6 deletions DuckDuckGo/Bookmarks/Legacy/LegacyBookmarksStoreMigration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import Foundation
import CoreData
import Bookmarks
import Persistence
import PixelKit

public class LegacyBookmarksStoreMigration {

Expand Down Expand Up @@ -55,7 +56,7 @@ public class LegacyBookmarksStoreMigration {
guard BookmarkUtils.fetchRootFolder(destination) == nil else {
// There should be no data left as migration has been done already
if !bookmarkRoots.isEmpty {
Pixel.fire(.debug(event: .bookmarksMigrationAlreadyPerformed))
PixelKit.fire(DebugEvent(GeneralPixel.bookmarksMigrationAlreadyPerformed))

cleanupOldData(in: source)
}
Expand All @@ -72,9 +73,9 @@ public class LegacyBookmarksStoreMigration {
let newFavoritesRoot = BookmarkUtils.fetchLegacyFavoritesFolder(destination) else {

if bookmarkRoots.isEmpty {
Pixel.fire(.debug(event: .bookmarksCouldNotPrepareDatabase))
PixelKit.fire(DebugEvent(GeneralPixel.bookmarksCouldNotPrepareDatabase))
} else {
Pixel.fire(.debug(event: .bookmarksMigrationCouldNotPrepareDatabase))
PixelKit.fire(DebugEvent(GeneralPixel.bookmarksMigrationCouldNotPrepareDatabase))
}

Thread.sleep(forTimeInterval: 2)
Expand Down Expand Up @@ -157,15 +158,15 @@ public class LegacyBookmarksStoreMigration {
}

do {
try destination.save(onErrorFire: .bookmarksMigrationFailed)
try destination.save(onErrorFire: GeneralPixel.bookmarksMigrationFailed)

cleanupOldData(in: source)
} catch {
destination.reset()

BookmarkUtils.prepareLegacyFoldersStructure(in: destination)
do {
try destination.save(onErrorFire: .bookmarksMigrationCouldNotPrepareDatabaseOnFailedMigration)
try destination.save(onErrorFire: GeneralPixel.bookmarksMigrationCouldNotPrepareDatabaseOnFailedMigration)
} catch {
Thread.sleep(forTimeInterval: 2)
fatalError("Could not write to Bookmarks DB")
Expand All @@ -181,7 +182,7 @@ public class LegacyBookmarksStoreMigration {
allObjects.forEach { context.delete($0) }

if context.hasChanges {
try? context.save(onErrorFire: .bookmarksMigrationCouldNotRemoveOldStore)
try? context.save(onErrorFire: GeneralPixel.bookmarksMigrationCouldNotRemoveOldStore)
}
}

Expand Down
Loading

0 comments on commit 8e97096

Please sign in to comment.