Skip to content

Commit

Permalink
Deprecate PixelKit daily pixel suffixes (#1060)
Browse files Browse the repository at this point in the history
Required:

Task/Issue URL: https://app.asana.com/0/0/1208695427490034/f
iOS PR: duckduckgo/iOS#3534
macOS PR: duckduckgo/macos-browser#3509
What kind of version bump will this require?: Major

Description:

This PR renames the existing dailyAndCount PixelKit type to legacyDailyAndCount.

A new dailyAndCount case has been added, but it uses new suffixes.
  • Loading branch information
samsymons authored Nov 6, 2024
1 parent 64a5d8d commit 14594b6
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 6 deletions.
23 changes: 21 additions & 2 deletions Sources/PixelKit/PixelKit.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,14 @@ public final class PixelKit {
/// Sent once per day. The last timestamp for this pixel is stored and compared to the current date. Pixels of this type will have `_d` appended to their name.
case daily

/// Sent once per day with a `_d` suffix, in addition to every time it is called with a `_c` suffix.
/// [Legacy] Sent once per day with a `_d` suffix, in addition to every time it is called with a `_c` suffix.
/// This means a pixel will get sent twice the first time it is called per-day, and subsequent calls that day will only send the `_c` variant.
/// This is useful in situations where pixels receive spikes in volume, as the daily pixel can be used to determine how many users are actually affected.
case legacyDailyAndCount

/// Sent once per day with a `_daily` suffix, in addition to every time it is called with a `_count` suffix.
/// This means a pixel will get sent twice the first time it is called per-day, and subsequent calls that day will only send the `_count` variant.
/// This is useful in situations where pixels receive spikes in volume, as the daily pixel can be used to determine how many users are actually affected.
case dailyAndCount

fileprivate var description: String {
Expand All @@ -58,6 +63,8 @@ public final class PixelKit {
"Legacy Daily"
case .daily:
"Daily"
case .legacyDailyAndCount:
"Legacy Daily and Count"
case .dailyAndCount:
"Daily and Count"
}
Expand Down Expand Up @@ -233,7 +240,7 @@ public final class PixelKit {
} else {
printDebugInfo(pixelName: pixelName + "_d", frequency: frequency, parameters: newParams, skipped: true)
}
case .dailyAndCount:
case .legacyDailyAndCount:
reportErrorIf(pixel: pixelName, endsWith: "_u")
reportErrorIf(pixel: pixelName, endsWith: "_d") // Because is added automatically
reportErrorIf(pixel: pixelName, endsWith: "_c") // Because is added automatically
Expand All @@ -245,6 +252,18 @@ public final class PixelKit {
}

fireRequestWrapper(pixelName + "_c", headers, newParams, allowedQueryReservedCharacters, true, frequency, onComplete)
case .dailyAndCount:
reportErrorIf(pixel: pixelName, endsWith: "_u")
reportErrorIf(pixel: pixelName, endsWith: "_daily") // Because is added automatically
reportErrorIf(pixel: pixelName, endsWith: "_count") // Because is added automatically
if !pixelHasBeenFiredToday(pixelName) {
fireRequestWrapper(pixelName + "_daily", headers, newParams, allowedQueryReservedCharacters, true, frequency, onComplete)
updatePixelLastFireDate(pixelName: pixelName)
} else {
printDebugInfo(pixelName: pixelName + "_daily", frequency: frequency, parameters: newParams, skipped: true)
}

fireRequestWrapper(pixelName + "_count", headers, newParams, allowedQueryReservedCharacters, true, frequency, onComplete)
}
}

Expand Down
9 changes: 6 additions & 3 deletions Sources/PixelKitTestingUtilities/XCTestCase+PixelKit.swift
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public extension XCTestCase {
let knownExpectedParameters = knownExpectedParameters(for: event)
let callbackExecutedExpectation = expectation(description: "The PixelKit callback has been executed")

if frequency == .dailyAndCount {
if frequency == .legacyDailyAndCount {
callbackExecutedExpectation.expectedFulfillmentCount = 2
}

Expand All @@ -123,7 +123,7 @@ public extension XCTestCase {
firedParameters[key] == value
}, file: file, line: line)

if frequency == .dailyAndCount {
if frequency == .legacyDailyAndCount {
XCTAssertTrue(firedPixelName.hasPrefix(expectations.pixelName))
XCTAssertTrue(firedPixelName.hasSuffix("_c") || firedPixelName.hasSuffix("_d"))
XCTAssertEqual(firedPixelName.count, expectations.pixelName.count + 2)
Expand Down Expand Up @@ -155,9 +155,12 @@ public extension XCTestCase {
expectedPixelNames.append(originalName)
case .daily:
expectedPixelNames.append(originalName.appending("_d"))
case .dailyAndCount:
case .legacyDailyAndCount:
expectedPixelNames.append(originalName.appending("_d"))
expectedPixelNames.append(originalName.appending("_c"))
case .dailyAndCount:
expectedPixelNames.append(originalName.appending("_daily"))
expectedPixelNames.append(originalName.appending("_count"))
}
return expectedPixelNames
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/PixelKitTests/PixelKitTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ final class PixelKitTests: XCTestCase {
case .dailyEvent, .dailyEventWithoutParameters:
return .daily
case .dailyAndContinuousEvent, .dailyAndContinuousEventWithoutParameters:
return .dailyAndCount
return .legacyDailyAndCount
}
}
}
Expand Down

0 comments on commit 14594b6

Please sign in to comment.