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

Remove ATB from default params #2430

Merged
merged 12 commits into from
Apr 30, 2024
Merged
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 @@ -170,7 +170,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 @@ -200,7 +200,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],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this to the UniquePixel API so that the caller can control the parameters, before this it was relying on the default parameters.

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
4 changes: 2 additions & 2 deletions DuckDuckGo/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,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 @@ -609,7 +609,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
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ 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:
Expand All @@ -59,16 +60,16 @@ final class NetworkProtectionPacketTunnelProvider: PacketTunnelProvider {
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))
quanganhdo marked this conversation as resolved.
Show resolved Hide resolved
Expand Down