Skip to content

Commit

Permalink
Remove ATB from default params (#2430)
Browse files Browse the repository at this point in the history
Task/Issue URL: https://app.asana.com/0/414235014887631/1206435379975124/f
Tech Design URL:
CC:

Description:

This PR updates the Pixel and DailyPixel fire functions to change the default parameters.
  • Loading branch information
samsymons committed Apr 30, 2024
1 parent 5e52e8d commit 022fd60
Show file tree
Hide file tree
Showing 10 changed files with 51 additions and 26 deletions.
4 changes: 2 additions & 2 deletions Core/DailyPixel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public final class DailyPixel {
public static func fire(pixel: Pixel.Event,
error: Swift.Error? = nil,
withAdditionalParameters params: [String: String] = [:],
includedParameters: [Pixel.QueryParameters] = [.atb, .appVersion],
includedParameters: [Pixel.QueryParameters] = [.appVersion],
onComplete: @escaping (Swift.Error?) -> Void = { _ in }) {
var key: String = pixel.name

Expand Down Expand Up @@ -79,7 +79,7 @@ public final class DailyPixel {
public static func fireDailyAndCount(pixel: Pixel.Event,
error: Swift.Error? = nil,
withAdditionalParameters params: [String: String] = [:],
includedParameters: [Pixel.QueryParameters] = [.atb, .appVersion],
includedParameters: [Pixel.QueryParameters] = [.appVersion],
onDailyComplete: @escaping (Swift.Error?) -> Void = { _ in },
onCountComplete: @escaping (Swift.Error?) -> Void = { _ in }) {
let key: String = pixel.name
Expand Down
4 changes: 2 additions & 2 deletions Core/Pixel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ public class Pixel {
withAdditionalParameters params: [String: String] = [:],
allowedQueryReservedCharacters: CharacterSet? = nil,
withHeaders headers: APIRequest.Headers = APIRequest.Headers(),
includedParameters: [QueryParameters] = [.atb, .appVersion],
includedParameters: [QueryParameters] = [.appVersion],
onComplete: @escaping (Error?) -> Void = { _ in },
debounce: Int = 0) {

Expand Down Expand Up @@ -209,7 +209,7 @@ public class Pixel {
withAdditionalParameters params: [String: String] = [:],
allowedQueryReservedCharacters: CharacterSet? = nil,
withHeaders headers: APIRequest.Headers = APIRequest.Headers(),
includedParameters: [QueryParameters] = [.atb, .appVersion],
includedParameters: [QueryParameters] = [.appVersion],
onComplete: @escaping (Error?) -> Void = { _ in }) {
var newParams = params
if includedParameters.contains(.appVersion) {
Expand Down
3 changes: 2 additions & 1 deletion Core/UniquePixel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,15 @@ public final class UniquePixel {
/// This requires the pixel name to end with `_u`
public static func fire(pixel: Pixel.Event,
withAdditionalParameters params: [String: String] = [:],
includedParameters: [Pixel.QueryParameters] = [.appVersion],
onComplete: @escaping (Swift.Error?) -> Void = { _ in }) {
guard pixel.name.hasSuffix("_u") else {
assertionFailure("Unique pixel: must end with _u")
return
}

if !pixel.hasBeenFiredEver(uniquePixelStorage: storage) {
Pixel.fire(pixel: pixel, withAdditionalParameters: params, onComplete: onComplete)
Pixel.fire(pixel: pixel, withAdditionalParameters: params, includedParameters: includedParameters, onComplete: onComplete)
storage.set(Date(), forKey: pixel.name)
} else {
onComplete(Error.alreadyFired)
Expand Down
12 changes: 8 additions & 4 deletions DuckDuckGo/AdAttribution/AdAttributionPixelReporter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import Foundation
import Core

protocol PixelFiring {
static func fire(pixel: Pixel.Event, withAdditionalParameters params: [String: String]) async throws
static func fire(pixel: Pixel.Event, withAdditionalParameters params: [String: String], includedParameters: [Pixel.QueryParameters]) async throws
}

final class AdAttributionPixelReporter {
Expand Down Expand Up @@ -52,7 +52,11 @@ final class AdAttributionPixelReporter {
if attributionData.attribution {
let parameters = self.pixelParametersForAttribution(attributionData)
do {
try await pixelFiring.fire(pixel: .appleAdAttribution, withAdditionalParameters: parameters)
try await pixelFiring.fire(
pixel: .appleAdAttribution,
withAdditionalParameters: parameters,
includedParameters: [.appVersion, .atb]
)
} catch {
return false
}
Expand Down Expand Up @@ -83,10 +87,10 @@ final class AdAttributionPixelReporter {
}

extension Pixel: PixelFiring {
static func fire(pixel: Event, withAdditionalParameters params: [String: String]) async throws {
static func fire(pixel: Event, withAdditionalParameters params: [String: String], includedParameters: [QueryParameters]) async throws {

try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Void, Error>) in
Pixel.fire(pixel: pixel, withAdditionalParameters: params) { error in
Pixel.fire(pixel: pixel, withAdditionalParameters: params, includedParameters: includedParameters) { error in
if let error {
continuation.resume(throwing: error)
} else {
Expand Down
4 changes: 2 additions & 2 deletions DuckDuckGo/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
PixelParameters.widgetError: "1",
PixelParameters.widgetErrorCode: "\((error as NSError).code)",
PixelParameters.widgetErrorDomain: (error as NSError).domain
])
], includedParameters: [.appVersion, .atb])

case .success(let widgetInfo):
let params = widgetInfo.reduce([String: String]()) {
Expand All @@ -594,7 +594,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
}
return result
}
Pixel.fire(pixel: .appLaunch, withAdditionalParameters: params)
Pixel.fire(pixel: .appLaunch, withAdditionalParameters: params, includedParameters: [.appVersion, .atb])
}

}
Expand Down
2 changes: 1 addition & 1 deletion DuckDuckGo/Feedback/VPNFeedbackSender.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ struct DefaultVPNFeedbackSender: VPNFeedbackSender {
"breakageCategory": category.rawValue,
"breakageDescription": encodedUserText,
"breakageMetadata": metadata.toBase64(),
]) { error in
], includedParameters: [.appVersion, .atb]) { error in
if let error {
continuation.resume(throwing: error)
} else {
Expand Down
8 changes: 4 additions & 4 deletions DuckDuckGo/NetworkProtectionTunnelController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,16 @@ final class NetworkProtectionTunnelController: TunnelController {
/// Starts the VPN connection used for Network Protection
///
func start() async {
Pixel.fire(pixel: .networkProtectionControllerStartAttempt)
Pixel.fire(pixel: .networkProtectionControllerStartAttempt, includedParameters: [.appVersion, .atb])

do {
try await startWithError()
Pixel.fire(pixel: .networkProtectionControllerStartSuccess)
Pixel.fire(pixel: .networkProtectionControllerStartSuccess, includedParameters: [.appVersion, .atb])
} catch {
if case StartError.configSystemPermissionsDenied = error {
return
}
Pixel.fire(pixel: .networkProtectionControllerStartFailure, error: error)
Pixel.fire(pixel: .networkProtectionControllerStartFailure, error: error, includedParameters: [.appVersion, .atb])

#if DEBUG
errorStore.lastErrorMessage = error.localizedDescription
Expand Down Expand Up @@ -190,7 +190,7 @@ final class NetworkProtectionTunnelController: TunnelController {

do {
try tunnelManager.connection.startVPNTunnel(options: options)
UniquePixel.fire(pixel: .networkProtectionNewUser) { error in
UniquePixel.fire(pixel: .networkProtectionNewUser, includedParameters: [.appVersion, .atb]) { error in
guard error != nil else { return }
UserDefaults.networkProtectionGroupDefaults.vpnFirstEnabled = Pixel.Event.networkProtectionNewUser.lastFireDate(
uniquePixelStorage: UniquePixel.storage
Expand Down
2 changes: 1 addition & 1 deletion DuckDuckGo/TabViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1186,7 +1186,7 @@ extension TabViewController: WKNavigationDelegate {

#if NETWORK_PROTECTION
if webView.url?.isDuckDuckGoSearch == true, case .connected = netPConnectionStatus {
DailyPixel.fireDailyAndCount(pixel: .networkProtectionEnabledOnSearch)
DailyPixel.fireDailyAndCount(pixel: .networkProtectionEnabledOnSearch, includedParameters: [.appVersion, .atb])
}
#endif
}
Expand Down
18 changes: 17 additions & 1 deletion DuckDuckGoTests/AdAttributionPixelReporterTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,17 @@ final class AdAttributionPixelReporterTests: XCTestCase {
XCTAssertEqual(pixelAttributes["ad_id"], "5")
}

func testPixelAdditionalParameters() async throws {
let sut = createSUT()
attributionFetcher.fetchResponse = AdServicesAttributionResponse(attribution: true)

await sut.reportAttributionIfNeeded()

let pixelAttributes = try XCTUnwrap(PixelFiringMock.lastIncludedParams)

XCTAssertEqual(pixelAttributes, [.appVersion, .atb])
}

func testPixelAttributes_WhenPartialAttributionData() async throws {
let sut = createSUT()
attributionFetcher.fetchResponse = AdServicesAttributionResponse(
Expand Down Expand Up @@ -172,9 +183,13 @@ final actor PixelFiringMock: PixelFiring {

static var lastParams: [String: String]?
static var lastPixel: Pixel.Event?
static func fire(pixel: Pixel.Event, withAdditionalParameters params: [String: String]) async throws {
static var lastIncludedParams: [Pixel.QueryParameters]?
static func fire(pixel: Pixel.Event,
withAdditionalParameters params: [String: String],
includedParameters: [Pixel.QueryParameters]) async throws {
lastParams = params
lastPixel = pixel
lastIncludedParams = includedParameters

if let expectedFireError {
throw expectedFireError
Expand All @@ -184,6 +199,7 @@ final actor PixelFiringMock: PixelFiring {
static func tearDown() {
lastParams = nil
lastPixel = nil
lastIncludedParams = nil
expectedFireError = nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,35 +43,39 @@ final class NetworkProtectionPacketTunnelProvider: PacketTunnelProvider {
switch event {
case .userBecameActive:
DailyPixel.fire(pixel: .networkProtectionActiveUser,
withAdditionalParameters: [PixelParameters.vpnCohort: UniquePixel.cohort(from: defaults.vpnFirstEnabled)])
withAdditionalParameters: [PixelParameters.vpnCohort: UniquePixel.cohort(from: defaults.vpnFirstEnabled)],
includedParameters: [.appVersion, .atb])
case .reportConnectionAttempt(attempt: let attempt):
switch attempt {
case .connecting:
DailyPixel.fireDailyAndCount(pixel: .networkProtectionEnableAttemptConnecting)
DailyPixel.fireDailyAndCount(pixel: .networkProtectionEnableAttemptConnecting,
includedParameters: [.appVersion, .atb])
case .success:
let versionStore = NetworkProtectionLastVersionRunStore(userDefaults: .networkProtectionGroupDefaults)
versionStore.lastExtensionVersionRun = AppVersion.shared.versionAndBuildNumber

DailyPixel.fireDailyAndCount(pixel: .networkProtectionEnableAttemptSuccess)
DailyPixel.fireDailyAndCount(pixel: .networkProtectionEnableAttemptSuccess,
includedParameters: [.appVersion, .atb])
case .failure:
DailyPixel.fireDailyAndCount(pixel: .networkProtectionEnableAttemptFailure)
DailyPixel.fireDailyAndCount(pixel: .networkProtectionEnableAttemptFailure,
includedParameters: [.appVersion, .atb])
}
case .reportTunnelFailure(result: let result):
switch result {
case .failureDetected:
DailyPixel.fireDailyAndCount(pixel: .networkProtectionTunnelFailureDetected)
DailyPixel.fireDailyAndCount(pixel: .networkProtectionTunnelFailureDetected, includedParameters: [.appVersion, .atb])
case .failureRecovered:
DailyPixel.fireDailyAndCount(pixel: .networkProtectionTunnelFailureRecovered)
DailyPixel.fireDailyAndCount(pixel: .networkProtectionTunnelFailureRecovered, includedParameters: [.appVersion, .atb])
case .networkPathChanged(let newPath):
defaults.updateNetworkPath(with: newPath)
}
case .reportLatency(result: let result):
switch result {
case .error:
DailyPixel.fire(pixel: .networkProtectionLatencyError)
DailyPixel.fire(pixel: .networkProtectionLatencyError, includedParameters: [.appVersion, .atb])
case .quality(let quality):
guard quality != .unknown else { return }
DailyPixel.fireDailyAndCount(pixel: .networkProtectionLatency(quality: quality))
DailyPixel.fireDailyAndCount(pixel: .networkProtectionLatency(quality: quality), includedParameters: [.appVersion, .atb])
}
case .rekeyAttempt(let step):
switch step {
Expand Down

0 comments on commit 022fd60

Please sign in to comment.