From b71ed70ce9b0ef3ce51d4f96da0193ab70493944 Mon Sep 17 00:00:00 2001
From: Sabrina Tardio <44158575+SabrinaTardio@users.noreply.github.com>
Date: Tue, 17 Dec 2024 12:47:17 +0100
Subject: [PATCH] change api (#1133)

Task/Issue URL:
https://app.asana.com/0/1204186595873227/1208964427775425/f
iOS PR:  https://github.com/duckduckgo/iOS/pull/3732
macOS PR: https://github.com/duckduckgo/macos-browser/pull/3670
What kind of version bump will this require?: Minor
---
 .../PixelExperimentKit.swift                  |  96 ++++++++-----
 .../PixelExperimentKitTests.swift             | 130 ++++++++++++++----
 2 files changed, 171 insertions(+), 55 deletions(-)

diff --git a/Sources/PixelExperimentKit/PixelExperimentKit.swift b/Sources/PixelExperimentKit/PixelExperimentKit.swift
index d0962f791..ecfb35482 100644
--- a/Sources/PixelExperimentKit/PixelExperimentKit.swift
+++ b/Sources/PixelExperimentKit/PixelExperimentKit.swift
@@ -21,6 +21,7 @@ import BrowserServicesKit
 import Foundation
 
 public typealias ConversionWindow = ClosedRange<Int>
+public typealias NumberOfCalls = Int
 
 struct ExperimentEvent: PixelKitEvent {
     var name: String
@@ -72,7 +73,7 @@ extension PixelKit {
         ExperimentConfig.fireFunction(event, .uniqueByNameAndParameters, false)
     }
 
-    /// Fires a pixel for a specific action in an experiment, based on conversion window and value thresholds (if value is a number).
+    /// Fires a pixel for a specific action in an experiment, based on conversion window and value.
     /// - Parameters:
     ///   - subfeatureID: Identifier for the subfeature associated with the experiment.
     ///   - metric: The name of the metric being tracked (e.g., "searches").
@@ -82,7 +83,7 @@ extension PixelKit {
     /// This function:
     /// 1. Validates if the experiment is active.
     /// 2. Ensures the user is within the specified conversion window.
-    /// 3. Tracks actions performed and sends the pixel once the target value is reached (if applicable).
+    /// 3. Sends the pixel if not sent before (unique by name and parameter)
     public static func fireExperimentPixel(for subfeatureID: SubfeatureID,
                                            metric: String,
                                            conversionWindowDays: ConversionWindow,
@@ -94,11 +95,41 @@ extension PixelKit {
         }
         guard let experimentData = featureFlagger.getAllActiveExperiments()[subfeatureID] else { return }
 
+        // Check if within conversion window
+        guard isUserInConversionWindow(conversionWindowDays, enrollmentDate: experimentData.enrollmentDate) else { return }
+
+        // Define event
+        let event = event(for: subfeatureID, experimentData: experimentData, conversionWindowDays: conversionWindowDays, metric: metric, value: value)
+        ExperimentConfig.fireFunction(event, .uniqueByNameAndParameters, false)
+    }
+
+    /// Fires a pixel for a specific action in an experiment, based on conversion window and value thresholds.
+    /// - Parameters:
+    ///   - subfeatureID: Identifier for the subfeature associated with the experiment.
+    ///   - metric: The name of the metric being tracked (e.g., "searches").
+    ///   - conversionWindowDays: The range of days after enrollment during which the action is valid.
+    ///   - numberOfCalls: target number of actions required to fire the pixel.
+    ///
+    /// This function:
+    /// 1. Validates if the experiment is active.
+    /// 2. Ensures the user is within the specified conversion window.
+    /// 3. Tracks actions performed and sends the pixel once the target value is reached (if applicable).
+    public static func fireExperimentPixelIfThresholdReached(for subfeatureID: SubfeatureID,
+                                                             metric: String,
+                                                             conversionWindowDays: ConversionWindow,
+                                                             threshold: NumberOfCalls) {
+        // Check is active experiment for user
+        guard let featureFlagger = ExperimentConfig.featureFlagger else {
+            assertionFailure("PixelKit is not configured for experiments")
+            return
+        }
+        guard let experimentData = featureFlagger.getAllActiveExperiments()[subfeatureID] else { return }
+
         fireExperimentPixelForActiveExperiment(subfeatureID,
                                                experimentData: experimentData,
                                                metric: metric,
                                                conversionWindowDays: conversionWindowDays,
-                                               value: value)
+                                               numberOfCalls: threshold)
     }
 
     /// Fires search-related experiment pixels for all active experiments.
@@ -172,7 +203,7 @@ extension PixelKit {
                     experimentData: experimentData,
                     metric: metric,
                     conversionWindowDays: range,
-                    value: "\(value)"
+                    numberOfCalls: value
                 )
             }
         }
@@ -182,39 +213,24 @@ extension PixelKit {
                                                                experimentData: ExperimentData,
                                                                metric: String,
                                                                conversionWindowDays: ConversionWindow,
-                                                               value: String) {
+                                                               numberOfCalls: Int) {
         // Set parameters, event name, store key
-        let eventName = "\(Constants.metricsEventPrefix)_\(subfeatureID)_\(experimentData.cohortID)"
-        let conversionWindowValue = (conversionWindowDays.lowerBound != conversionWindowDays.upperBound) ?
-        "\(conversionWindowDays.lowerBound)-\(conversionWindowDays.upperBound)" :
-        "\(conversionWindowDays.lowerBound)"
-        let parameters: [String: String] = [
-            Constants.metricKey: metric,
-            Constants.conversionWindowDaysKey: conversionWindowValue,
-            Constants.valueKey: value,
-            Constants.enrollmentDateKey: experimentData.enrollmentDate.toYYYYMMDDInET()
-        ]
-        let event = ExperimentEvent(name: eventName, parameters: parameters)
-        let eventStoreKey = "\(eventName)_\(parameters.toString())"
+        let event = event(for: subfeatureID, experimentData: experimentData, conversionWindowDays: conversionWindowDays, metric: metric, value: String(numberOfCalls))
+        let parameters = parameters(metric: metric, conversionWindowDays: conversionWindowDays, value: String(numberOfCalls), experimentData: experimentData)
+        let eventStoreKey = "\(event.name)_\(parameters.toString())"
 
         // Determine if the user is within the conversion window
         let isInWindow = isUserInConversionWindow(conversionWindowDays, enrollmentDate: experimentData.enrollmentDate)
 
-        // Check if value is a number
-        if let numberOfAction = NumberOfActions(value), numberOfAction > 1 {
-            // Increment or remove based on conversion window status
-            let shouldSendPixel = ExperimentConfig.eventTracker.incrementAndCheckThreshold(
-                forKey: eventStoreKey,
-                threshold: numberOfAction,
-                isInWindow: isInWindow
-            )
+        // Increment or remove based on conversion window status
+        let shouldSendPixel = ExperimentConfig.eventTracker.incrementAndCheckThreshold(
+            forKey: eventStoreKey,
+            threshold: numberOfCalls,
+            isInWindow: isInWindow
+        )
 
-            // Send the pixel only if conditions are met
-            if shouldSendPixel {
-                ExperimentConfig.fireFunction(event, .uniqueByNameAndParameters, false)
-            }
-        } else if isInWindow {
-            // If value is not a number, send the pixel only if within the window
+        // Send the pixel only if conditions are met
+        if shouldSendPixel {
             ExperimentConfig.fireFunction(event, .uniqueByNameAndParameters, false)
         }
     }
@@ -233,6 +249,24 @@ extension PixelKit {
         return currentDate >= calendar.startOfDay(for: startOfWindow) &&
         currentDate <= calendar.startOfDay(for: endOfWindow)
     }
+
+    private static func event(for subfeatureID: SubfeatureID, experimentData: ExperimentData, conversionWindowDays: ConversionWindow, metric: String, value: String) -> ExperimentEvent{
+        let eventName = "\(Constants.metricsEventPrefix)_\(subfeatureID)_\(experimentData.cohortID)"
+        let parameters = parameters(metric: metric, conversionWindowDays: conversionWindowDays, value: value, experimentData: experimentData)
+        return ExperimentEvent(name: eventName, parameters: parameters)
+    }
+
+    private static func parameters(metric: String, conversionWindowDays: ConversionWindow, value: String, experimentData: ExperimentData) -> [String: String] {
+        let conversionWindowValue = (conversionWindowDays.lowerBound != conversionWindowDays.upperBound) ?
+        "\(conversionWindowDays.lowerBound)-\(conversionWindowDays.upperBound)" :
+        "\(conversionWindowDays.lowerBound)"
+        return [
+            Constants.metricKey: metric,
+            Constants.conversionWindowDaysKey: conversionWindowValue,
+            Constants.valueKey: value,
+            Constants.enrollmentDateKey: experimentData.enrollmentDate.toYYYYMMDDInET()
+        ]
+    }
 }
 
 extension Date {
diff --git a/Tests/PixelExperimentKitTests/PixelExperimentKitTests.swift b/Tests/PixelExperimentKitTests/PixelExperimentKitTests.swift
index 30334f6e6..545bc68ce 100644
--- a/Tests/PixelExperimentKitTests/PixelExperimentKitTests.swift
+++ b/Tests/PixelExperimentKitTests/PixelExperimentKitTests.swift
@@ -51,7 +51,7 @@ final class PixelExperimentKitTests: XCTestCase {
         firedIncludeAppVersion = []
     }
 
-    func testFireExperimentEnrollmentPixelSendsExpectedData() {
+    func testfireExperimentEnrollmentPixelPixelSendsExpectedData() {
         // GIVEN
         let subfeatureID = "testSubfeature"
         let cohort = "A"
@@ -70,7 +70,7 @@ final class PixelExperimentKitTests: XCTestCase {
         XCTAssertFalse(firedIncludeAppVersion[0])
     }
 
-    func testFireExperimentPixel_WithValidExperimentAndConversionWindowAndValueNotNumber() {
+    func testFireExperimentPixel_WithValidExperimentAndConversionWindow() {
         // GIVEN
 
         let subfeatureID = "credentialsSaving"
@@ -99,25 +99,82 @@ final class PixelExperimentKitTests: XCTestCase {
         XCTAssertEqual(mockPixelStore.store.count, 0)
     }
 
+    func testFireExperimentPixel_WithInvalidExperimentAndValidConversionWindow() {
+        // GIVEN
+        let subfeatureID = "credentialsSaving"
+        let conversionWindow = 3...7
+        let value = String(Int.random(in: 1...100))
+        mockFeatureFlagger.experiments = [:]
+
+        // WHEN
+        PixelKit.fireExperimentPixel(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, value: value)
+
+        // THEN
+        XCTAssertTrue(firedEvent.isEmpty)
+        XCTAssertTrue(firedFrequency.isEmpty)
+        XCTAssertTrue(firedIncludeAppVersion.isEmpty)
+        XCTAssertEqual(mockPixelStore.store.count, 0)
+    }
+
+    func testFireExperimentPixel_WithValidExperimentAndBeforeConversionWindow() {
+        // GIVEN
+        let subfeatureID = "credentialsSaving"
+        let cohort = "control"
+        let enrollmentDate = Date().addingTimeInterval(-7 * 24 * 60 * 60) // 7 days ago
+        let conversionWindow = 8...11
+        let value = "someValue"
+        let experimentData = ExperimentData(parentID: "autofill", cohortID: cohort, enrollmentDate: enrollmentDate)
+        mockFeatureFlagger.experiments = [subfeatureID: experimentData]
+
+        // WHEN
+        PixelKit.fireExperimentPixel(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, value: value)
+
+        // THEN
+        XCTAssertTrue(firedEvent.isEmpty)
+        XCTAssertTrue(firedFrequency.isEmpty)
+        XCTAssertTrue(firedIncludeAppVersion.isEmpty)
+        XCTAssertEqual(mockPixelStore.store.count, 0)
+    }
+
+    func testFireExperimentPixel_WithValidExperimentAndAfterConversionWindow() {
+        // GIVEN
+        let subfeatureID = "credentialsSaving"
+        let cohort = "control"
+        let enrollmentDate = Date().addingTimeInterval(-12 * 24 * 60 * 60) // 12 days ago
+        let conversionWindow = 8...11
+        let value = "someValue"
+        let experimentData = ExperimentData(parentID: "autofill", cohortID: cohort, enrollmentDate: enrollmentDate)
+        mockFeatureFlagger.experiments = [subfeatureID: experimentData]
+
+        // WHEN
+        PixelKit.fireExperimentPixel(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, value: value)
+
+        // THEN
+        XCTAssertTrue(firedEvent.isEmpty)
+        XCTAssertTrue(firedFrequency.isEmpty)
+        XCTAssertTrue(firedIncludeAppVersion.isEmpty)
+        XCTAssertEqual(mockPixelStore.store.count, 0)
+    }
+
     func testFireExperimentPixel_WithValidExperimentAndConversionWindowAndValue1() {
         // GIVEN
         let subfeatureID = "credentialsSaving"
         let cohort = "control"
         let enrollmentDate = Date().addingTimeInterval(-3 * 24 * 60 * 60) // 5 days ago
         let conversionWindow = 3...7
-        let value = "1"
+        let value = 1
         let expectedEventName = "experiment_metrics_\(subfeatureID)_\(cohort)"
         let expectedParameters = [
             "metric": "someMetric",
             "conversionWindowDays": "3-7",
-            "value": value,
+            "value": String(value),
             "enrollmentDate": enrollmentDate.toYYYYMMDDInET()
         ]
         let experimentData = ExperimentData(parentID: "autofill", cohortID: cohort, enrollmentDate: enrollmentDate)
         mockFeatureFlagger.experiments = [subfeatureID: experimentData]
 
         // WHEN
-        PixelKit.fireExperimentPixel(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, value: value)
+        PixelKit.fireExperimentPixelIfThresholdReached(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, threshold: value)
 
         // THEN
         XCTAssertEqual(firedEvent[0].name, expectedEventName)
@@ -127,27 +184,54 @@ final class PixelExperimentKitTests: XCTestCase {
         XCTAssertEqual(mockPixelStore.store.count, 0)
     }
 
-    func testFireExperimentPixel_WithValidExperimentAndConversionWindowAndValueN() {
+    func testFireExperimentPixelWhenReachingNumberOfCalls_WithValidExperimentAndConversionWindowAndValue1() {
+        // GIVEN
+        let subfeatureID = "credentialsSaving"
+        let cohort = "control"
+        let enrollmentDate = Date().addingTimeInterval(-3 * 24 * 60 * 60) // 5 days ago
+        let conversionWindow = 3...7
+        let value = 1
+        let expectedEventName = "experiment_metrics_\(subfeatureID)_\(cohort)"
+        let expectedParameters = [
+            "metric": "someMetric",
+            "conversionWindowDays": "3-7",
+            "value": String(value),
+            "enrollmentDate": enrollmentDate.toYYYYMMDDInET()
+        ]
+        let experimentData = ExperimentData(parentID: "autofill", cohortID: cohort, enrollmentDate: enrollmentDate)
+        mockFeatureFlagger.experiments = [subfeatureID: experimentData]
+
+        // WHEN
+        PixelKit.fireExperimentPixelIfThresholdReached(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, threshold: value)
+
+        // THEN
+        XCTAssertEqual(firedEvent[0].name, expectedEventName)
+        XCTAssertEqual(firedEvent[0].parameters, expectedParameters)
+        XCTAssertEqual(firedFrequency[0], .uniqueByNameAndParameters)
+        XCTAssertFalse(firedIncludeAppVersion[0])
+        XCTAssertEqual(mockPixelStore.store.count, 0)
+    }
+
+    func testFireExperimentPixelWhenReachingNumberOfCalls_WithValidExperimentAndConversionWindowAndValueN() {
         // GIVEN
         let subfeatureID = "credentialsSaving"
         let cohort = "control"
         let enrollmentDate = Date().addingTimeInterval(-7 * 24 * 60 * 60) // 5 days ago
         let conversionWindow = 3...7
-        let randomNumber = Int.random(in: 1...100)
-        let value = "\(randomNumber)"
+        let value = Int.random(in: 1...100)
         let expectedEventName = "experiment_metrics_\(subfeatureID)_\(cohort)"
         let expectedParameters = [
             "metric": "someMetric",
             "conversionWindowDays": "3-7",
-            "value": value,
+            "value": String(value),
             "enrollmentDate": enrollmentDate.toYYYYMMDDInET()
         ]
         let experimentData = ExperimentData(parentID: "autofill", cohortID: cohort, enrollmentDate: enrollmentDate)
         mockFeatureFlagger.experiments = [subfeatureID: experimentData]
 
         // WHEN calling fire before expected number of calls
-        for n in 1..<randomNumber {
-            PixelKit.fireExperimentPixel(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, value: value)
+        for n in 1..<value {
+            PixelKit.fireExperimentPixelIfThresholdReached(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, threshold: value)
             // THEN
             XCTAssertTrue(firedEvent.isEmpty)
             XCTAssertTrue(firedFrequency.isEmpty)
@@ -157,7 +241,7 @@ final class PixelExperimentKitTests: XCTestCase {
         }
 
         // WHEN calling fire at the right number of calls
-        PixelKit.fireExperimentPixel(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, value: value)
+        PixelKit.fireExperimentPixelIfThresholdReached(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, threshold: value)
 
         // THEN
         XCTAssertEqual(firedEvent[0].name, expectedEventName)
@@ -167,16 +251,15 @@ final class PixelExperimentKitTests: XCTestCase {
         XCTAssertEqual(mockPixelStore.store.count, 0)
     }
 
-    func testFireExperimentPixel_WithInvalidExperimentAndValidConversionWindowAndValue1() {
+    func testFireExperimentPixelWhenReachingNumberOfCalls_WithInvalidExperimentAndValidConversionWindowAndValue1() {
         // GIVEN
         let subfeatureID = "credentialsSaving"
         let conversionWindow = 3...7
-        let randomNumber = Int.random(in: 1...100)
-        let value = "\(randomNumber)"
+        let value = Int.random(in: 1...100)
         mockFeatureFlagger.experiments = [:]
 
         // WHEN
-        PixelKit.fireExperimentPixel(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, value: value)
+        PixelKit.fireExperimentPixelIfThresholdReached(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, threshold: value)
 
         // THEN
         XCTAssertTrue(firedEvent.isEmpty)
@@ -185,18 +268,18 @@ final class PixelExperimentKitTests: XCTestCase {
         XCTAssertEqual(mockPixelStore.store.count, 0)
     }
 
-    func testFireExperimentPixel_WithValidExperimentAndOutsideConversionWindowAndValueN() {
+    func testFireExperimentPixelWhenReachingNumberOfCalls_WithValidExperimentAndOutsideConversionWindowAndValueN() {
         // GIVEN
         let subfeatureID = "credentialsSaving"
         let cohort = "control"
         let enrollmentDate = Date().addingTimeInterval(-7 * 24 * 60 * 60) // 7 days ago
         let conversionWindow = 8...11
-        let value = "3"
+        let value = 3
         let experimentData = ExperimentData(parentID: "autofill", cohortID: cohort, enrollmentDate: enrollmentDate)
         mockFeatureFlagger.experiments = [subfeatureID: experimentData]
 
         // WHEN
-        PixelKit.fireExperimentPixel(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, value: value)
+        PixelKit.fireExperimentPixelIfThresholdReached(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, threshold: value)
 
         // THEN
         XCTAssertTrue(firedEvent.isEmpty)
@@ -205,28 +288,27 @@ final class PixelExperimentKitTests: XCTestCase {
         XCTAssertEqual(mockPixelStore.store.count, 0)
     }
 
-    func testFireExperimentPixel_WithValidExperimentAndAfterConversionWindowAndValueNAfterSomeCalledHappened() {
+    func testFireExperimentPixelWhenReachingNumberOfCalls_WithValidExperimentAndAfterConversionWindowAndValueNAfterSomeCalledHappened() {
         // GIVEN
         let subfeatureID = "credentialsSaving"
         let cohort = "control"
         let enrollmentDate = Date().addingTimeInterval(-6 * 24 * 60 * 60) // 6 days ago
-        print(enrollmentDate)
         let conversionWindow = 3...5
-        let value = "3"
+        let value = 3
         let experimentData = ExperimentData(parentID: "autofill", cohortID: cohort, enrollmentDate: enrollmentDate)
         mockFeatureFlagger.experiments = [subfeatureID: experimentData]
         let expectedEventName = "experiment_metrics_\(subfeatureID)_\(cohort)"
         let expectedParameters = [
             "metric": "someMetric",
             "conversionWindowDays": "3-5",
-            "value": value,
+            "value": String(value),
             "enrollmentDate": enrollmentDate.toYYYYMMDDInET()
         ]
         let eventStoreKey = expectedEventName + "_" + expectedParameters.toString()
         mockPixelStore.store = [eventStoreKey: 2]
 
         // WHEN
-        PixelKit.fireExperimentPixel(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, value: value)
+        PixelKit.fireExperimentPixelIfThresholdReached(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, threshold: value)
 
         // THEN
         XCTAssertTrue(firedEvent.isEmpty)