Skip to content
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

Sabrina/revert cards experiment #2119

Merged
merged 5 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions DuckDuckGo/Application/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, FileDownloadManagerDel
if LocalStatisticsStore().atb == nil {
Pixel.firstLaunchDate = Date()
// MARK: Enable pixel experiments here
PixelExperiment.install()
}
AtbAndVariantCleanup.cleanup()
DefaultVariantManager().assignVariantIfNeeded { _ in
Expand Down Expand Up @@ -489,7 +488,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, FileDownloadManagerDel
private func emailDidSignInNotification(_ notification: Notification) {
Pixel.fire(.emailEnabled)
if Pixel.isNewUser {
PixelExperiment.fireEmailProtectionEnabledPixel()
Pixel.fire(.emailEnabledInitial, limitTo: .initial)
}

if let object = notification.object as? EmailManager, let emailManager = syncDataProviders.settingsAdapter.emailManager, object !== emailManager {
Expand All @@ -506,7 +505,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, FileDownloadManagerDel

@objc private func dataImportCompleteNotification(_ notification: Notification) {
if Pixel.isNewUser {
PixelExperiment.fireImportDataInitialPixel()
Pixel.fire(.importDataInitial, limitTo: .initial)
}
}

Expand Down
10 changes: 2 additions & 8 deletions DuckDuckGo/HomePage/Model/HomePageContinueSetUpModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -356,19 +356,16 @@ extension HomePage.Models {
}

private var shouldMakeDefaultCardBeVisible: Bool {
!PixelExperiment.isNoCardsExperimentOn &&
shouldShowMakeDefaultSetting &&
!defaultBrowserProvider.isDefault
}

private var shouldImportCardBeVisible: Bool {
!PixelExperiment.isNoCardsExperimentOn &&
shouldShowImportSetting &&
!dataImportProvider.didImport
}

private var shouldDuckPlayerCardBeVisible: Bool {
!PixelExperiment.isNoCardsExperimentOn &&
shouldShowDuckPlayerSetting &&
duckPlayerPreferences.duckPlayerModeBool == nil &&
!duckPlayerPreferences.youtubeOverlayAnyButtonPressed
Expand All @@ -383,24 +380,21 @@ extension HomePage.Models {
}

private var shouldEmailProtectionCardBeVisible: Bool {
!PixelExperiment.isNoCardsExperimentOn &&
shouldShowEmailProtectionSetting &&
!emailManager.isSignedIn
}

private var shouldSurveyDay0BeVisible: Bool {
let oneDayAgo = Calendar.current.date(byAdding: .weekday, value: -1, to: Date())!
return !PixelExperiment.isNoCardsExperimentOn &&
isDay0SurveyEnabled &&
return isDay0SurveyEnabled &&
shouldShowSurveyDay0 &&
!userInteractedWithSurveyDay0 &&
firstLaunchDate > oneDayAgo
}

private var shouldSurveyDay7BeVisible: Bool {
let oneWeekAgo = Calendar.current.date(byAdding: .weekOfYear, value: -1, to: Date())!
return !PixelExperiment.isNoCardsExperimentOn &&
isDay7SurveyEnabled &&
return isDay7SurveyEnabled &&
shouldShowSurveyDay0 &&
shouldShowSurveyDay7 &&
!userInteractedWithSurveyDay0 &&
Expand Down
7 changes: 2 additions & 5 deletions DuckDuckGo/HomePage/View/HomePageView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ extension HomePage.Views {
if isBurner {
BurnerHomePageView()
} else {
regularHomePageView(includingContinueSetUpCards: model.isContinueSetUpAvailable, isNoCardExperimentOn: PixelExperiment.isNoCardsExperimentOn)
regularHomePageView(includingContinueSetUpCards: model.isContinueSetUpAvailable)
.contextMenu(ContextMenu(menuItems: {
if model.isContinueSetUpAvailable {
Toggle(UserText.newTabMenuItemShowContinuteSetUp, isOn: $model.isContinueSetUpVisible)
Expand All @@ -53,15 +53,12 @@ extension HomePage.Views {
}
}

func regularHomePageView(includingContinueSetUpCards: Bool, isNoCardExperimentOn: Bool) -> some View {
func regularHomePageView(includingContinueSetUpCards: Bool) -> some View {
ZStack(alignment: .top) {

ScrollView {
VStack(spacing: 0) {
Group {
if isNoCardExperimentOn {
DefaultBrowserPrompt()
}
if includingContinueSetUpCards {
ContinueSetUpView()
.padding(.top, 64)
Expand Down
6 changes: 2 additions & 4 deletions DuckDuckGo/HomePage/View/HomePageViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,8 @@ final class HomePageViewController: NSViewController {

override func viewWillAppear() {
super.viewWillAppear()
if !PixelExperiment.isExperimentInstalled {
if OnboardingViewModel.isOnboardingFinished && Pixel.isNewUser {
Pixel.fire(.newTabInitial(), limitTo: .initial)
}
if OnboardingViewModel.isOnboardingFinished && Pixel.isNewUser {
Pixel.fire(.newTabInitial, limitTo: .initial)
}
subscribeToHistory()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ final class DefaultBrowserPreferences: ObservableObject {
guard NSApp.runType.requiresEnvironment else { return }
#endif
if Pixel.isNewUser && isDefault {
PixelExperiment.fireSetAsDefaultInitialPixel()
Pixel.fire(.setAsDefaultInitial, limitTo: .initial)
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions DuckDuckGo/Statistics/ATB/StatisticsLoader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ final class StatisticsLoader {
completion()
}
}
PixelExperiment.fireFirstSerpPixel()
PixelExperiment.fireDay21To27SerpPixel()
Pixel.fire(.serp)
self.fireDailyOsVersionCounterPixel()
} else if !self.statisticsStore.isAppRetentionFiredToday {
Expand Down
64 changes: 3 additions & 61 deletions DuckDuckGo/Statistics/Experiment/PixelExperiment.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,6 @@ enum PixelExperiment: String, CaseIterable {
return logic.isInstalled
}

static var isNoCardsExperimentOn: Bool {
// This is to avoid the cohort is assigned before the user actually sees the new tab page (after the onboarding has been completed)
if isExperimentInstalled && !OnboardingViewModel.isOnboardingFinished {
return true
}
// Cohort are assigned the first time it's called
return Self.cohort == .noCards
}

static var allocatedCohortDoesNotMatchCurrentCohorts: Bool {
guard let allocatedCohort = logic.allocatedCohort else { return false }
if PixelExperiment(rawValue: allocatedCohort) == nil {
Expand All @@ -60,8 +51,7 @@ enum PixelExperiment: String, CaseIterable {

// These are the variants. Rename or add/remove them as needed. If you change the string value
// remember to keep it clear for privacy triage.
case control = "c"
case noCards = "d"
case control
}

/// These functions contain the business logic for determining if the pixel should be fired or not.
Expand All @@ -79,22 +69,6 @@ extension PixelExperiment {
logic.fireDay21To27SerpPixel()
}

static func fireSetAsDefaultInitialPixel() {
logic.fireSetAsDefaultInitialPixel()
}

static func fireImportDataInitialPixel() {
logic.fireImportDataInitialPixel()
}

static func fireWatchInDuckPlayerPixel() {
logic.fireWatchInDuckPlayerPixel()
}

static func fireEmailProtectionEnabledPixel() {
logic.fireEmailProtectionEnabledPixel()
}

}

final internal class PixelExperimentLogic {
Expand Down Expand Up @@ -157,15 +131,15 @@ final internal class PixelExperimentLogic {
// You'll need additional pixels for your experiment. Pass the cohort as a paramter.
func fireEnrollmentPixel() {
// You'll probably need this at least.
guard allocatedCohort != nil, let cohort else { return }
Pixel.fire(.newTabInitial(cohort: cohort.rawValue), limitTo: .initial, includeAppVersionParameter: false)
}

// Often used
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it was worth leaving those here since they will probably often be used although atm they will not be called

func fireFirstSerpPixel() {
guard allocatedCohort != nil, let cohort else { return }
Pixel.fire(.serpInitial(cohort: cohort.rawValue), limitTo: .initial, includeAppVersionParameter: false)
}

// Often used for retention experiments
func fireDay21To27SerpPixel() {
guard allocatedCohort != nil, let cohort else { return }

Expand All @@ -174,38 +148,6 @@ final internal class PixelExperimentLogic {
}
}

func fireSetAsDefaultInitialPixel() {
if allocatedCohort != nil, let cohort {
Pixel.fire(.setAsDefaultInitial(cohort: cohort.rawValue), limitTo: .initial)
} else {
Pixel.fire(.setAsDefaultInitial(), limitTo: .initial)
}
}

func fireImportDataInitialPixel() {
if allocatedCohort != nil, let cohort {
Pixel.fire(.importDataInitial(cohort: cohort.rawValue), limitTo: .initial)
} else {
Pixel.fire(.importDataInitial(), limitTo: .initial)
}
}

func fireWatchInDuckPlayerPixel() {
if allocatedCohort != nil, let cohort {
Pixel.fire(.watchInDuckPlayerInitial(cohort: cohort.rawValue), limitTo: .initial)
} else {
Pixel.fire(.watchInDuckPlayerInitial(), limitTo: .initial)
}
}

func fireEmailProtectionEnabledPixel() {
if allocatedCohort != nil, let cohort {
Pixel.fire(.emailEnabledInitial(cohort: cohort.rawValue), limitTo: .initial)
} else {
Pixel.fire(.emailEnabledInitial(), limitTo: .initial)
}
}

func cleanup() {
isInstalled = false
allocatedCohort = nil
Expand Down
11 changes: 5 additions & 6 deletions DuckDuckGo/Statistics/PixelEvent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,11 @@ extension Pixel {
case debug(event: Debug, error: Error? = nil)

// Activation Points
case newTabInitial(cohort: String? = nil)
case emailEnabledInitial(cohort: String? = nil)

case watchInDuckPlayerInitial(cohort: String? = nil)
case setAsDefaultInitial(cohort: String? = nil)
case importDataInitial(cohort: String? = nil)
case newTabInitial
case emailEnabledInitial
case watchInDuckPlayerInitial
case setAsDefaultInitial
case importDataInitial

// New Tab section removed
case favoriteSectionHidden
Expand Down
22 changes: 7 additions & 15 deletions DuckDuckGo/Statistics/PixelParameters.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,25 +51,12 @@ extension Pixel.Event {

case .launchInitial(let cohort):
return [PixelKit.Parameters.experimentCohort: cohort]

case .serpInitial(let cohort):
return [PixelKit.Parameters.experimentCohort: cohort]

case .serpDay21to27(let cohort):
return [PixelKit.Parameters.experimentCohort: cohort]
case .setAsDefaultInitial(let cohort):
guard let cohort else { return nil }
return [PixelKit.Parameters.experimentCohort: cohort]
case .newTabInitial(let cohort):
guard let cohort else { return nil }
return [PixelKit.Parameters.experimentCohort: cohort]
case .emailEnabledInitial(let cohort):
guard let cohort else { return nil }
return [PixelKit.Parameters.experimentCohort: cohort]
case .watchInDuckPlayerInitial(let cohort):
guard let cohort else { return nil }
return [PixelKit.Parameters.experimentCohort: cohort]
case .importDataInitial(let cohort):
guard let cohort else { return nil }
return [PixelKit.Parameters.experimentCohort: cohort]

case .dailyPixel(let pixel, isFirst: _):
return pixel.parameters
Expand Down Expand Up @@ -118,6 +105,11 @@ extension Pixel.Event {
.emailUserPressedUseAlias,
.emailUserPressedUseAddress,
.jsPixel,
.emailEnabledInitial,
.watchInDuckPlayerInitial,
.importDataInitial,
.newTabInitial,
.setAsDefaultInitial,
.favoriteSectionHidden,
.recentActivitySectionHidden,
.continueSetUpSectionHidden,
Expand Down
2 changes: 1 addition & 1 deletion DuckDuckGo/YoutubePlayer/YoutubeOverlayUserScript.swift
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ extension YoutubeOverlayUserScript {
return nil
}
if pixelName == "play.use" {
PixelExperiment.fireWatchInDuckPlayerPixel()
Pixel.fire(.watchInDuckPlayerInitial, limitTo: .initial)
}
return nil
}
Expand Down
Loading
Loading