Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
SabrinaTardio committed Dec 17, 2024
1 parent e5c0ba3 commit 9aa81e8
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 16 deletions.
17 changes: 8 additions & 9 deletions Sources/PixelExperimentKit/PixelExperimentKit.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import BrowserServicesKit
import Foundation

public typealias ConversionWindow = ClosedRange<Int>
public typealias NumberOfCalls = Int

struct ExperimentEvent: PixelKitEvent {
var name: String
Expand Down Expand Up @@ -94,14 +95,12 @@ 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)

// Send unique by name and parameter pixel if within conversion window
let isInWindow = isUserInConversionWindow(conversionWindowDays, enrollmentDate: experimentData.enrollmentDate)
if isInWindow {
ExperimentConfig.fireFunction(event, .uniqueByNameAndParameters, false)
}
ExperimentConfig.fireFunction(event, .uniqueByNameAndParameters, false)
}

/// Fires a pixel for a specific action in an experiment, based on conversion window and value thresholds.
Expand All @@ -115,10 +114,10 @@ extension PixelKit {
/// 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 fireExperimentPixelWhenReachingNumberOfCalls(for subfeatureID: SubfeatureID,
public static func fireExperimentPixelIfThresholdReached(for subfeatureID: SubfeatureID,
metric: String,

Check failure on line 118 in Sources/PixelExperimentKit/PixelExperimentKit.swift

View workflow job for this annotation

GitHub Actions / Run SwiftLint

Function parameters should be aligned vertically if they're in multiple lines in a declaration (vertical_parameter_alignment)
conversionWindowDays: ConversionWindow,

Check failure on line 119 in Sources/PixelExperimentKit/PixelExperimentKit.swift

View workflow job for this annotation

GitHub Actions / Run SwiftLint

Function parameters should be aligned vertically if they're in multiple lines in a declaration (vertical_parameter_alignment)
numberOfCalls: Int) {
threshold: NumberOfCalls) {

Check failure on line 120 in Sources/PixelExperimentKit/PixelExperimentKit.swift

View workflow job for this annotation

GitHub Actions / Run SwiftLint

Function parameters should be aligned vertically if they're in multiple lines in a declaration (vertical_parameter_alignment)
// Check is active experiment for user
guard let featureFlagger = ExperimentConfig.featureFlagger else {
assertionFailure("PixelKit is not configured for experiments")
Expand All @@ -130,7 +129,7 @@ extension PixelKit {
experimentData: experimentData,
metric: metric,
conversionWindowDays: conversionWindowDays,
numberOfCalls: numberOfCalls)
numberOfCalls: threshold)
}

/// Fires search-related experiment pixels for all active experiments.
Expand Down
14 changes: 7 additions & 7 deletions Tests/PixelExperimentKitTests/PixelExperimentKitTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ final class PixelExperimentKitTests: XCTestCase {
mockFeatureFlagger.experiments = [subfeatureID: experimentData]

// WHEN
PixelKit.fireExperimentPixelWhenReachingNumberOfCalls(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, numberOfCalls: value)
PixelKit.fireExperimentPixelIfThresholdReached(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, threshold: value)

// THEN
XCTAssertEqual(firedEvent[0].name, expectedEventName)
Expand Down Expand Up @@ -202,7 +202,7 @@ final class PixelExperimentKitTests: XCTestCase {
mockFeatureFlagger.experiments = [subfeatureID: experimentData]

// WHEN
PixelKit.fireExperimentPixelWhenReachingNumberOfCalls(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, numberOfCalls: value)
PixelKit.fireExperimentPixelIfThresholdReached(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, threshold: value)

// THEN
XCTAssertEqual(firedEvent[0].name, expectedEventName)
Expand Down Expand Up @@ -231,7 +231,7 @@ final class PixelExperimentKitTests: XCTestCase {

// WHEN calling fire before expected number of calls
for n in 1..<value {
PixelKit.fireExperimentPixelWhenReachingNumberOfCalls(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, numberOfCalls: value)
PixelKit.fireExperimentPixelIfThresholdReached(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, threshold: value)
// THEN
XCTAssertTrue(firedEvent.isEmpty)
XCTAssertTrue(firedFrequency.isEmpty)
Expand All @@ -241,7 +241,7 @@ final class PixelExperimentKitTests: XCTestCase {
}

// WHEN calling fire at the right number of calls
PixelKit.fireExperimentPixelWhenReachingNumberOfCalls(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, numberOfCalls: value)
PixelKit.fireExperimentPixelIfThresholdReached(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, threshold: value)

// THEN
XCTAssertEqual(firedEvent[0].name, expectedEventName)
Expand All @@ -259,7 +259,7 @@ final class PixelExperimentKitTests: XCTestCase {
mockFeatureFlagger.experiments = [:]

// WHEN
PixelKit.fireExperimentPixelWhenReachingNumberOfCalls(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, numberOfCalls: value)
PixelKit.fireExperimentPixelIfThresholdReached(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, threshold: value)

// THEN
XCTAssertTrue(firedEvent.isEmpty)
Expand All @@ -279,7 +279,7 @@ final class PixelExperimentKitTests: XCTestCase {
mockFeatureFlagger.experiments = [subfeatureID: experimentData]

// WHEN
PixelKit.fireExperimentPixelWhenReachingNumberOfCalls(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, numberOfCalls: value)
PixelKit.fireExperimentPixelIfThresholdReached(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, threshold: value)

// THEN
XCTAssertTrue(firedEvent.isEmpty)
Expand Down Expand Up @@ -308,7 +308,7 @@ final class PixelExperimentKitTests: XCTestCase {
mockPixelStore.store = [eventStoreKey: 2]

// WHEN
PixelKit.fireExperimentPixelWhenReachingNumberOfCalls(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, numberOfCalls: value)
PixelKit.fireExperimentPixelIfThresholdReached(for: subfeatureID, metric: "someMetric", conversionWindowDays: conversionWindow, threshold: value)

// THEN
XCTAssertTrue(firedEvent.isEmpty)
Expand Down

0 comments on commit 9aa81e8

Please sign in to comment.