From 6cea0b6b179df1aa9a7d4d1866e384a649984eed Mon Sep 17 00:00:00 2001 From: Federico Cappelli Date: Wed, 8 May 2024 09:28:19 +0100 Subject: [PATCH] Pixelkit invalid name fix (#2751) Task/Issue URL: https://app.asana.com/0/1199230911884351/1207238594885159/f CC: @ayoy @diegoreymendez @loremattei **Description**: - Adds https://github.com/duckduckgo/BrowserServicesKit/pull/810 - Creates a new NonStandardPixel that uses NonStandardEvent - Names and frequencies fix from https://app.asana.com/0/30173902528854/1207202086855539/f and https://app.asana.com/0/30173902528854/1207236076008989/f `m_mac.` > `m_mac_` --- DuckDuckGo.xcodeproj/project.pbxproj | 8 ++- .../xcshareddata/swiftpm/Package.resolved | 4 +- DuckDuckGo/Application/AppDelegate.swift | 6 +-- .../Fire/ViewModel/FirePopoverViewModel.swift | 2 +- .../NavigationBar/View/MoreOptionsMenu.swift | 2 +- .../View/PrivacyDashboardViewController.swift | 2 +- DuckDuckGo/Statistics/GeneralPixel.swift | 39 ++++---------- DuckDuckGo/Statistics/NonStandardPixel.swift | 53 +++++++++++++++++++ .../DuckPlayerTabExtension.swift | 2 +- .../DataBrokerProtection/Package.swift | 2 +- .../NetworkProtectionMac/Package.swift | 2 +- LocalPackages/SubscriptionUI/Package.swift | 2 +- .../BrokenSiteReportingReferenceTests.swift | 2 +- .../WebsiteBreakageReportTests.swift | 13 ++++- 14 files changed, 95 insertions(+), 44 deletions(-) create mode 100644 DuckDuckGo/Statistics/NonStandardPixel.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index dc88f48c10..a5a233c122 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -2552,6 +2552,8 @@ F116A7C32BD1924B00F3FCF7 /* PixelKitTestingUtilities in Frameworks */ = {isa = PBXBuildFile; productRef = F116A7C22BD1924B00F3FCF7 /* PixelKitTestingUtilities */; }; F116A7C72BD1925500F3FCF7 /* PixelKitTestingUtilities in Frameworks */ = {isa = PBXBuildFile; productRef = F116A7C62BD1925500F3FCF7 /* PixelKitTestingUtilities */; }; F116A7C92BD1929000F3FCF7 /* PixelKitTestingUtilities in Frameworks */ = {isa = PBXBuildFile; productRef = F116A7C82BD1929000F3FCF7 /* PixelKitTestingUtilities */; }; + F118EA852BEACC7000F77634 /* NonStandardPixel.swift in Sources */ = {isa = PBXBuildFile; fileRef = F118EA842BEACC7000F77634 /* NonStandardPixel.swift */; }; + F118EA862BEACC7000F77634 /* NonStandardPixel.swift in Sources */ = {isa = PBXBuildFile; fileRef = F118EA842BEACC7000F77634 /* NonStandardPixel.swift */; }; F188267C2BBEB3AA00D9AC4F /* GeneralPixel.swift in Sources */ = {isa = PBXBuildFile; fileRef = F188267B2BBEB3AA00D9AC4F /* GeneralPixel.swift */; }; F188267D2BBEB3AA00D9AC4F /* GeneralPixel.swift in Sources */ = {isa = PBXBuildFile; fileRef = F188267B2BBEB3AA00D9AC4F /* GeneralPixel.swift */; }; F18826802BBEB58100D9AC4F /* PrivacyProPixel.swift in Sources */ = {isa = PBXBuildFile; fileRef = F188267F2BBEB58100D9AC4F /* PrivacyProPixel.swift */; }; @@ -4034,6 +4036,7 @@ EEDE50102BA360C80017F3C4 /* NetworkProtection+VPNAgentConvenienceInitializers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "NetworkProtection+VPNAgentConvenienceInitializers.swift"; sourceTree = ""; }; EEF12E6D2A2111880023E6BF /* MacPacketTunnelProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MacPacketTunnelProvider.swift; sourceTree = ""; }; EEF53E172950CED5002D78F4 /* JSAlertViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = JSAlertViewModelTests.swift; sourceTree = ""; }; + F118EA842BEACC7000F77634 /* NonStandardPixel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NonStandardPixel.swift; sourceTree = ""; }; F188267B2BBEB3AA00D9AC4F /* GeneralPixel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GeneralPixel.swift; sourceTree = ""; }; F188267F2BBEB58100D9AC4F /* PrivacyProPixel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrivacyProPixel.swift; sourceTree = ""; }; F18826832BBEE31700D9AC4F /* PixelKit+Assertion.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "PixelKit+Assertion.swift"; sourceTree = ""; }; @@ -7823,6 +7826,7 @@ 857E5AF32A79044900FC0FB4 /* Experiment */, B610F2BA27A145C500FCEBE9 /* RulesCompilationMonitor.swift */, F188267B2BBEB3AA00D9AC4F /* GeneralPixel.swift */, + F118EA842BEACC7000F77634 /* NonStandardPixel.swift */, F18826832BBEE31700D9AC4F /* PixelKit+Assertion.swift */, F188267F2BBEB58100D9AC4F /* PrivacyProPixel.swift */, ); @@ -10022,6 +10026,7 @@ 3706FC38293F65D500E42796 /* BundleExtension.swift in Sources */, 4B9DB04B2A983B24000927DB /* NotificationService.swift in Sources */, 3706FC3A293F65D500E42796 /* NSOpenPanelExtensions.swift in Sources */, + F118EA862BEACC7000F77634 /* NonStandardPixel.swift in Sources */, 3706FC3B293F65D500E42796 /* FirePopover.swift in Sources */, 4B4D60C12A0C848E00BCD287 /* NetworkProtectionControllerErrorStore.swift in Sources */, 3706FC3E293F65D500E42796 /* VariantManager.swift in Sources */, @@ -11232,6 +11237,7 @@ 4B9DB04A2A983B24000927DB /* NotificationService.swift in Sources */, 3775912D29AAC72700E26367 /* SyncPreferences.swift in Sources */, F1B33DF22BAD929D001128B3 /* SubscriptionAppStoreRestorer.swift in Sources */, + F118EA852BEACC7000F77634 /* NonStandardPixel.swift in Sources */, 1DB9618329F67F6200CF5568 /* FaviconNullStore.swift in Sources */, BB5789722B2CA70F0009DFE2 /* DataBrokerProtectionSubscriptionEventHandler.swift in Sources */, B693954F26F04BEB0015B914 /* PaddedImageButton.swift in Sources */, @@ -12772,7 +12778,7 @@ repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit"; requirement = { kind = exactVersion; - version = 144.0.0; + version = 144.0.1; }; }; 9FF521422BAA8FF300B9819B /* XCRemoteSwiftPackageReference "lottie-spm" */ = { diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 41fa2b0e67..dd9eae0f55 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { - "revision" : "9906b9464f6f12e94f3cc62456b5b5a9c1a43db8", - "version" : "144.0.0" + "revision" : "f34b0a63938df11ef471aa3301dcc0de09b0d31b", + "version" : "144.0.1" } }, { diff --git a/DuckDuckGo/Application/AppDelegate.swift b/DuckDuckGo/Application/AppDelegate.swift index 2f93c17ac4..3d3ad5d5d2 100644 --- a/DuckDuckGo/Application/AppDelegate.swift +++ b/DuckDuckGo/Application/AppDelegate.swift @@ -449,7 +449,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate { .filter { $0 } .asVoid() .sink { [weak syncService] in - PixelKit.fire(GeneralPixel.syncDaily, frequency: .daily) + PixelKit.fire(GeneralPixel.syncDaily, frequency: .legacyDaily) syncService?.syncDailyStats.sendStatsIfNeeded(handler: { params in PixelKit.fire(GeneralPixel.syncSuccessRateDaily, withAdditionalParameters: params) }) @@ -532,7 +532,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate { } private func emailDidSignInNotification(_ notification: Notification) { - PixelKit.fire(GeneralPixel.emailEnabled) + PixelKit.fire(NonStandardEvent(NonStandardPixel.emailEnabled)) if AppDelegate.isNewUser { PixelKit.fire(GeneralPixel.emailEnabledInitial, frequency: .legacyInitial) } @@ -543,7 +543,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate { } private func emailDidSignOutNotification(_ notification: Notification) { - PixelKit.fire(GeneralPixel.emailDisabled) + PixelKit.fire(NonStandardEvent(NonStandardPixel.emailDisabled)) if let object = notification.object as? EmailManager, let emailManager = syncDataProviders.settingsAdapter.emailManager, object !== emailManager { syncService?.scheduler.notifyDataChanged() } diff --git a/DuckDuckGo/Fire/ViewModel/FirePopoverViewModel.swift b/DuckDuckGo/Fire/ViewModel/FirePopoverViewModel.swift index 1221497f5f..0d07e1f473 100644 --- a/DuckDuckGo/Fire/ViewModel/FirePopoverViewModel.swift +++ b/DuckDuckGo/Fire/ViewModel/FirePopoverViewModel.swift @@ -189,7 +189,7 @@ final class FirePopoverViewModel { // MARK: - Burning func burn() { - PixelKit.fire(GeneralPixel.fireButtonFirstBurn, frequency: .daily) + PixelKit.fire(GeneralPixel.fireButtonFirstBurn, frequency: .legacyDaily) switch (clearingOption, areAllSelected) { case (.currentTab, _): diff --git a/DuckDuckGo/NavigationBar/View/MoreOptionsMenu.swift b/DuckDuckGo/NavigationBar/View/MoreOptionsMenu.swift index 847452c63c..1b48e04049 100644 --- a/DuckDuckGo/NavigationBar/View/MoreOptionsMenu.swift +++ b/DuckDuckGo/NavigationBar/View/MoreOptionsMenu.swift @@ -524,7 +524,7 @@ final class EmailOptionsButtonSubMenu: NSMenu { let pixelParameters = self.emailManager.emailPixelParameters self.emailManager.updateLastUseDate() - PixelKit.fire(GeneralPixel.emailUserCreatedAlias, withAdditionalParameters: pixelParameters) + PixelKit.fire(NonStandardEvent(NonStandardPixel.emailUserCreatedAlias), withAdditionalParameters: pixelParameters) NSPasteboard.general.copy(address) NotificationCenter.default.post(name: NSNotification.Name.privateEmailCopiedToClipboard, object: nil) diff --git a/DuckDuckGo/PrivacyDashboard/View/PrivacyDashboardViewController.swift b/DuckDuckGo/PrivacyDashboard/View/PrivacyDashboardViewController.swift index befe61b87a..059652246d 100644 --- a/DuckDuckGo/PrivacyDashboard/View/PrivacyDashboardViewController.swift +++ b/DuckDuckGo/PrivacyDashboard/View/PrivacyDashboardViewController.swift @@ -45,7 +45,7 @@ final class PrivacyDashboardViewController: NSViewController { private let brokenSiteReporter: BrokenSiteReporter = { BrokenSiteReporter(pixelHandler: { parameters in - PixelKit.fire(GeneralPixel.brokenSiteReport, + PixelKit.fire(NonStandardEvent(NonStandardPixel.brokenSiteReport), withAdditionalParameters: parameters, allowedQueryReservedCharacters: BrokenSiteReport.allowedQueryReservedCharacters) }, keyValueStoring: UserDefaults.standard) diff --git a/DuckDuckGo/Statistics/GeneralPixel.swift b/DuckDuckGo/Statistics/GeneralPixel.swift index e2eeb8d192..f4b375fb66 100644 --- a/DuckDuckGo/Statistics/GeneralPixel.swift +++ b/DuckDuckGo/Statistics/GeneralPixel.swift @@ -26,7 +26,6 @@ import Configuration enum GeneralPixel: PixelKitEventV2 { case crash - case brokenSiteReport case compileRulesWait(onboardingShown: OnboardingShown, waitTime: CompileRulesWaitTime, result: WaitResult) case launchInitial(cohort: String) @@ -55,12 +54,6 @@ enum GeneralPixel: PixelKitEventV2 { case adClickAttributionActive case adClickAttributionPageLoads - case emailEnabled - case emailDisabled - case emailUserPressedUseAddress - case emailUserPressedUseAlias - case emailUserCreatedAlias - case jsPixel(_ pixel: AutofillUserScript.JSPixel) // Activation Points @@ -321,9 +314,6 @@ enum GeneralPixel: PixelKitEventV2 { case .crash: return "m_mac_crash" - case .brokenSiteReport: - return "epbf_macos_desktop" - case .compileRulesWait(onboardingShown: let onboardingShown, waitTime: let waitTime, result: let result): return "m_mac_cbr-wait_\(onboardingShown)_\(waitTime)_\(result)" @@ -371,13 +361,6 @@ enum GeneralPixel: PixelKitEventV2 { case .adClickAttributionPageLoads: return "m_mac_ad_click_page_loads" - // Deliberately omit the `m_mac_` prefix in order to format these pixels the same way as other platforms - case .emailEnabled: return "email_enabled_macos_desktop" - case .emailDisabled: return "email_disabled_macos_desktop" - case .emailUserPressedUseAddress: return "email_filled_main_macos_desktop" - case .emailUserPressedUseAlias: return "email_filled_random_macos_desktop" - case .emailUserCreatedAlias: return "email_generated_button_macos_desktop" - case .jsPixel(let pixel): // Email pixels deliberately avoid using the `m_mac_` prefix. if pixel.isEmailPixel { @@ -386,22 +369,22 @@ enum GeneralPixel: PixelKitEventV2 { return "m_mac_\(pixel.pixelName)" } case .emailEnabledInitial: - return "m_mac.enable-email-protection.initial" + return "m_mac_enable-email-protection_initial" case .watchInDuckPlayerInitial: - return "m_mac.watch-in-duckplayer.initial" + return "m_mac_watch-in-duckplayer_initial" case .setAsDefaultInitial: - return "m_mac.set-as-default.initial" + return "m_mac_set-as-default_initial" case .importDataInitial: - return "m_mac.import-data.initial" + return "m_mac_import-data_initial" case .newTabInitial: - return "m_mac.new-tab-opened.initial" + return "m_mac_new-tab-opened_initial" case .favoriteSectionHidden: - return "m_mac.favorite-section-hidden" + return "m_mac_favorite-section-hidden" case .recentActivitySectionHidden: - return "m_mac.recent-activity-section-hidden" + return "m_mac_recent-activity-section-hidden" case .continueSetUpSectionHidden: - return "m_mac.continue-setup-section-hidden" + return "m_mac_continue-setup-section-hidden" // Fire Button case .fireButtonFirstBurn: @@ -434,11 +417,11 @@ enum GeneralPixel: PixelKitEventV2 { return "m_mac_mp_wlr" case .launchInitial: - return "m.mac.first-launch" + return "m_mac_first-launch" case .serpInitial: - return "m.mac.navigation.first-search" + return "m_mac_navigation_first-search" case .serpDay21to27: - return "m.mac.search-day-21-27.initial" + return "m_mac_search-day-21-27_initial" case .vpnBreakageReport: return "m_mac_vpn_breakage_report" diff --git a/DuckDuckGo/Statistics/NonStandardPixel.swift b/DuckDuckGo/Statistics/NonStandardPixel.swift new file mode 100644 index 0000000000..b528aef0a1 --- /dev/null +++ b/DuckDuckGo/Statistics/NonStandardPixel.swift @@ -0,0 +1,53 @@ +// +// NonStandardPixel.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import PixelKit +import BrowserServicesKit +import DDGSync +import Configuration + +/// These pixels deliberately omit the `m_mac_` prefix in order to format these pixel the same way as other platforms, they are sent unchanged +enum NonStandardPixel: PixelKitEventV2 { + + case brokenSiteReport + case emailEnabled + case emailDisabled + case emailUserPressedUseAddress + case emailUserPressedUseAlias + case emailUserCreatedAlias + + var name: String { + switch self { + case .brokenSiteReport: return "epbf_macos_desktop" + case .emailEnabled: return "email_enabled_macos_desktop" + case .emailDisabled: return "email_disabled_macos_desktop" + case .emailUserPressedUseAddress: return "email_filled_main_macos_desktop" + case .emailUserPressedUseAlias: return "email_filled_random_macos_desktop" + case .emailUserCreatedAlias: return "email_generated_button_macos_desktop" + } + } + + var parameters: [String: String]? { + return nil + } + + var error: Error? { + return nil + } +} diff --git a/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift b/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift index ca6cec4c5a..c34ab6a88f 100644 --- a/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift +++ b/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift @@ -312,7 +312,7 @@ extension DuckPlayerTabExtension: NavigationResponder { return } if navigation.url.isDuckPlayer { - PixelKit.fire(GeneralPixel.duckPlayerDailyUniqueView, frequency: .daily) + PixelKit.fire(GeneralPixel.duckPlayerDailyUniqueView, frequency: .legacyDaily) } } diff --git a/LocalPackages/DataBrokerProtection/Package.swift b/LocalPackages/DataBrokerProtection/Package.swift index 19b8daa1e2..6b804973c4 100644 --- a/LocalPackages/DataBrokerProtection/Package.swift +++ b/LocalPackages/DataBrokerProtection/Package.swift @@ -29,7 +29,7 @@ let package = Package( targets: ["DataBrokerProtection"]) ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "144.0.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "144.0.1"), .package(path: "../SwiftUIExtensions"), .package(path: "../XPCHelper"), ], diff --git a/LocalPackages/NetworkProtectionMac/Package.swift b/LocalPackages/NetworkProtectionMac/Package.swift index a027d241cb..6aa9f1b8d2 100644 --- a/LocalPackages/NetworkProtectionMac/Package.swift +++ b/LocalPackages/NetworkProtectionMac/Package.swift @@ -31,7 +31,7 @@ let package = Package( .library(name: "NetworkProtectionUI", targets: ["NetworkProtectionUI"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "144.0.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "144.0.1"), .package(url: "https://github.com/airbnb/lottie-spm", exact: "4.4.1"), .package(path: "../XPCHelper"), .package(path: "../SwiftUIExtensions"), diff --git a/LocalPackages/SubscriptionUI/Package.swift b/LocalPackages/SubscriptionUI/Package.swift index 7eb918dbc2..0d69eb5246 100644 --- a/LocalPackages/SubscriptionUI/Package.swift +++ b/LocalPackages/SubscriptionUI/Package.swift @@ -12,7 +12,7 @@ let package = Package( targets: ["SubscriptionUI"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "144.0.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "144.0.1"), .package(path: "../SwiftUIExtensions") ], targets: [ diff --git a/UnitTests/PrivacyReferenceTests/BrokenSiteReportingReferenceTests.swift b/UnitTests/PrivacyReferenceTests/BrokenSiteReportingReferenceTests.swift index 1d38801f44..0621b9b2fa 100644 --- a/UnitTests/PrivacyReferenceTests/BrokenSiteReportingReferenceTests.swift +++ b/UnitTests/PrivacyReferenceTests/BrokenSiteReportingReferenceTests.swift @@ -51,7 +51,7 @@ final class BrokenSiteReportingReferenceTests: XCTestCase { APIRequest.Headers.setUserAgent("") var params = parameters params["test"] = "1" - let configuration = APIRequest.Configuration(url: URL.pixelUrl(forPixelNamed: GeneralPixel.brokenSiteReport.name), + let configuration = APIRequest.Configuration(url: URL.pixelUrl(forPixelNamed: NonStandardPixel.brokenSiteReport.name), queryParameters: params, allowedQueryReservedCharacters: BrokenSiteReport.allowedQueryReservedCharacters) return configuration.request diff --git a/UnitTests/WebsiteBreakageReport/WebsiteBreakageReportTests.swift b/UnitTests/WebsiteBreakageReport/WebsiteBreakageReportTests.swift index 1103a8f2eb..94f087a80d 100644 --- a/UnitTests/WebsiteBreakageReport/WebsiteBreakageReportTests.swift +++ b/UnitTests/WebsiteBreakageReport/WebsiteBreakageReportTests.swift @@ -18,12 +18,21 @@ import PrivacyDashboard import XCTest - +import PixelKit +import PixelKitTestingUtilities @testable import DuckDuckGo_Privacy_Browser @testable import Networking class WebsiteBreakageReportTests: XCTestCase { + func testReportBrokenSitePixel() { + fire(NonStandardEvent(NonStandardPixel.brokenSiteReport), + frequency: .standard, + and: .expect(pixelName: "epbf_macos_desktop"), + file: #filePath, + line: #line) + } + func testCommonSetOfFields() throws { let breakage = BrokenSiteReport( siteUrl: URL(string: "https://example.test/")!, @@ -129,7 +138,7 @@ class WebsiteBreakageReportTests: XCTestCase { APIRequest.Headers.setUserAgent("") var params = parameters params["test"] = "1" - let configuration = APIRequest.Configuration(url: URL.pixelUrl(forPixelNamed: GeneralPixel.brokenSiteReport.name), + let configuration = APIRequest.Configuration(url: URL.pixelUrl(forPixelNamed: NonStandardPixel.brokenSiteReport.name), queryParameters: params, allowedQueryReservedCharacters: BrokenSiteReport.allowedQueryReservedCharacters) return configuration.request