Address feedback

LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Model/DataBroker.swift

@@ -67,6 +67,14 @@ struct MirrorSite: Codable, Sendable {
         removedAt = try? container.decode(Date.self, forKey: .removedAt)
 
     }
+
+    func wasRemoved(since: Date = Date()) -> Bool {
+        guard let removedAt = self.removedAt else {
+            return false
+        }
+
+        return removedAt < since
+    }
 }
 
 public enum DataBrokerHierarchy: Int {

LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Pixels/DataBrokerProtectionPixels.swift

@@ -436,38 +436,24 @@ extension DataBrokerProtectionPixels: PixelKitEvent {
             return [Consts.durationInMs: String(duration), Consts.hasError: hasError.description, Consts.brokerURL: brokerURL, Consts.sleepDuration: String(sleepDuration)]
         case .initialScanPreStartDuration(let duration):
             return [Consts.durationInMs: String(duration)]
-        case .globalMetricsWeeklyStats(let profilesFound, let optOutsInProgress, let successfulOptOuts, let failedOptOuts, let durationOfFirstOptOut, let numberOfNewRecordsFound):
-            return [Consts.numberOfRecordsFound: String(profilesFound),
-                    Consts.numberOfOptOutsInProgress: String(optOutsInProgress),
-                    Consts.numberOfSucessfulOptOuts: String(successfulOptOuts),
-                    Consts.numberOfOptOutsFailure: String(failedOptOuts),
-                    Consts.durationOfFirstOptOut: String(durationOfFirstOptOut),
-                    Consts.numberOfNewRecordsFound: String(numberOfNewRecordsFound)]
-        case .globalMetricsMonthlyStats(let profilesFound, let optOutsInProgress, let successfulOptOuts, let failedOptOuts, let durationOfFirstOptOut, let numberOfNewRecordsFound):
-            return [Consts.numberOfRecordsFound: String(profilesFound),
-                    Consts.numberOfOptOutsInProgress: String(optOutsInProgress),
-                    Consts.numberOfSucessfulOptOuts: String(successfulOptOuts),
-                    Consts.numberOfOptOutsFailure: String(failedOptOuts),
-                    Consts.durationOfFirstOptOut: String(durationOfFirstOptOut),
-                    Consts.numberOfNewRecordsFound: String(numberOfNewRecordsFound)]
-        case .dataBrokerMetricsWeeklyStats(let dataBrokerURL, let profilesFound, let optOutsInProgress, let successfulOptOuts, let failedOptOuts, let durationOfFirstOptOut, let numberOfNewRecordsFound, let numberOfReappereances):
-            return [Consts.dataBrokerParamKey: dataBrokerURL,
-                    Consts.numberOfRecordsFound: String(profilesFound),
-                    Consts.numberOfOptOutsInProgress: String(optOutsInProgress),
-                    Consts.numberOfSucessfulOptOuts: String(successfulOptOuts),
-                    Consts.numberOfOptOutsFailure: String(failedOptOuts),
-                    Consts.durationOfFirstOptOut: String(durationOfFirstOptOut),
-                    Consts.numberOfNewRecordsFound: String(numberOfNewRecordsFound),
-                    Consts.numberOfReappereances: String(numberOfReappereances)]
-        case .dataBrokerMetricsMonthlyStats(let dataBrokerURL, let profilesFound, let optOutsInProgress, let successfulOptOuts, let failedOptOuts, let durationOfFirstOptOut, let numberOfNewRecordsFound, let numberOfReappereances):
-            return [Consts.dataBrokerParamKey: dataBrokerURL,
-                    Consts.numberOfRecordsFound: String(profilesFound),
-                    Consts.numberOfOptOutsInProgress: String(optOutsInProgress),
-                    Consts.numberOfSucessfulOptOuts: String(successfulOptOuts),
-                    Consts.numberOfOptOutsFailure: String(failedOptOuts),
-                    Consts.durationOfFirstOptOut: String(durationOfFirstOptOut),
-                    Consts.numberOfNewRecordsFound: String(numberOfNewRecordsFound),
-                    Consts.numberOfReappereances: String(numberOfReappereances)]
+        case .globalMetricsWeeklyStats(let profilesFound, let optOutsInProgress, let successfulOptOuts, let failedOptOuts, let durationOfFirstOptOut, let numberOfNewRecordsFound),
+                        .globalMetricsMonthlyStats(let profilesFound, let optOutsInProgress, let successfulOptOuts, let failedOptOuts, let durationOfFirstOptOut, let numberOfNewRecordsFound):
+                    return [Consts.numberOfRecordsFound: String(profilesFound),
+                            Consts.numberOfOptOutsInProgress: String(optOutsInProgress),
+                            Consts.numberOfSucessfulOptOuts: String(successfulOptOuts),
+                            Consts.numberOfOptOutsFailure: String(failedOptOuts),
+                            Consts.durationOfFirstOptOut: String(durationOfFirstOptOut),
+                            Consts.numberOfNewRecordsFound: String(numberOfNewRecordsFound)]
+        case .dataBrokerMetricsWeeklyStats(let dataBrokerURL, let profilesFound, let optOutsInProgress, let successfulOptOuts, let failedOptOuts, let durationOfFirstOptOut, let numberOfNewRecordsFound, let numberOfReappereances),
+                     .dataBrokerMetricsMonthlyStats(let dataBrokerURL, let profilesFound, let optOutsInProgress, let successfulOptOuts, let failedOptOuts, let durationOfFirstOptOut, let numberOfNewRecordsFound, let numberOfReappereances):
+                   return [Consts.dataBrokerParamKey: dataBrokerURL,
+                           Consts.numberOfRecordsFound: String(profilesFound),
+                           Consts.numberOfOptOutsInProgress: String(optOutsInProgress),
+                           Consts.numberOfSucessfulOptOuts: String(successfulOptOuts),
+                           Consts.numberOfOptOutsFailure: String(failedOptOuts),
+                           Consts.durationOfFirstOptOut: String(durationOfFirstOptOut),
+                           Consts.numberOfNewRecordsFound: String(numberOfNewRecordsFound),
+                           Consts.numberOfReappereances: String(numberOfReappereances)]
         }
     }
 }

LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Pixels/DataBrokerProtectionPixelsUtilities.swift

@@ -28,14 +28,14 @@ final class DataBrokerProtectionPixelsUtilities {
     private static let calendar = Calendar.current
 
     static func shouldWeFirePixel(startDate: Date, endDate: Date, daysDifference: Frequency) -> Bool {
-        if let differenceBetweenDates = differenceBetweenDates(startDate: startDate, endDate: endDate) {
+        if let differenceBetweenDates = numberOfDaysFrom(startDate: startDate, endDate: endDate) {
             return differenceBetweenDates >= daysDifference.rawValue
         }
 
         return false
     }
 
-    static func differenceBetweenDates(startDate: Date, endDate: Date) -> Int? {
+    static func numberOfDaysFrom(startDate: Date, endDate: Date) -> Int? {
         let components = calendar.dateComponents([.day], from: startDate, to: endDate)
 
         return components.day

LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Pixels/DataBrokerProtectionStatsPixels.swift

@@ -147,12 +147,16 @@ final class DataBrokerProtectionStatsPixels {
         let dateOfFirstScan = dateOfFirstScan(brokerProfileQueryData)
 
         if shouldFireWeeklyStats(dateOfFirstScan: dateOfFirstScan) {
-            firePixels(for: brokerProfileQueryData, frequency: .weekly)
+            firePixels(for: brokerProfileQueryData,
+                       frequency: .weekly,
+                       dateSinceLastSubmission: repository.getLatestStatsWeeklyPixelDate())
             repository.markStatsWeeklyPixelDate()
         }
 
         if shouldFireMonthlyStats(dateOfFirstScan: dateOfFirstScan) {
-            firePixels(for: brokerProfileQueryData, frequency: .monthly)
+            firePixels(for: brokerProfileQueryData,
+                       frequency: .monthly,
+                       dateSinceLastSubmission: repository.getLatestStatsMonthlyPixelDate())
             repository.markStatsMonthlyPixelDate()
         }
     }
@@ -187,17 +191,17 @@ final class DataBrokerProtectionStatsPixels {
         }
     }
 
-    private func firePixels(for brokerProfileQueryData: [BrokerProfileQueryData], frequency: Frequency) {
-        let statsByBroker = calculateStatsByBroker(brokerProfileQueryData)
+    private func firePixels(for brokerProfileQueryData: [BrokerProfileQueryData], frequency: Frequency, dateSinceLastSubmission: Date? = nil) {
+        let statsByBroker = calculateStatsByBroker(brokerProfileQueryData, dateSinceLastSubmission: dateSinceLastSubmission)
 
         fireGlobalStats(statsByBroker, brokerProfileQueryData: brokerProfileQueryData, frequency: frequency)
         fireStatsByBroker(statsByBroker, frequency: frequency)
     }
 
-    private func calculateStatsByBroker(_ brokerProfileQueryData: [BrokerProfileQueryData]) -> [StatsByBroker] {
+    private func calculateStatsByBroker(_ brokerProfileQueryData: [BrokerProfileQueryData], dateSinceLastSubmission: Date? = nil) -> [StatsByBroker] {
         let profileQueriesGroupedByBroker = Dictionary(grouping: brokerProfileQueryData, by: { $0.dataBroker })
         let statsByBroker = profileQueriesGroupedByBroker.map { (key: DataBroker, value: [BrokerProfileQueryData]) in
-            calculateByBroker(key, data: value)
+            calculateByBroker(key, data: value, dateSinceLastSubmission: dateSinceLastSubmission)
         }
 
         return statsByBroker
@@ -229,8 +233,8 @@ final class DataBrokerProtectionStatsPixels {
     }
 
     /// internal for testing purposes
-    func calculateByBroker(_ broker: DataBroker, data: [BrokerProfileQueryData]) -> StatsByBroker {
-        let mirrorSitesSize = broker.mirrorSites.count
+    func calculateByBroker(_ broker: DataBroker, data: [BrokerProfileQueryData], dateSinceLastSubmission: Date? = nil) -> StatsByBroker {
+        let mirrorSitesSize = broker.mirrorSites.filter { !$0.wasRemoved() }.count
         var numberOfProfilesFound = 0 // Number of unique matching profiles found since the beginning.
         var numberOfOptOutsInProgress = 0 // Number of opt-outs in progress since the beginning.
         var numberOfSuccessfulOptOuts = 0 // Number of successfull opt-outs since the beginning
@@ -264,7 +268,7 @@ final class DataBrokerProtectionStatsPixels {
 
         let numberOfFailureOptOuts = numberOfProfilesFound - numberOfOptOutsInProgress - numberOfSuccessfulOptOuts
         let numberOfNewMatchesFound = calculateNumberOfNewMatchesFound(data)
-        let durationOfFirstOptOut = calculateDurationOfFirstOptOut(data)
+        let durationOfFirstOptOut = calculateDurationOfFirstOptOut(data, from: dateSinceLastSubmission)
 
         return StatsByBroker(dataBrokerURL: broker.url,
                              numberOfProfilesFound: numberOfProfilesFound,
@@ -286,7 +290,7 @@ final class DataBrokerProtectionStatsPixels {
         let profileQueriesGroupedByBroker = Dictionary(grouping: brokerProfileQueryDataWithAMatch, by: { $0.dataBroker })
 
         profileQueriesGroupedByBroker.forEach { (key: DataBroker, value: [BrokerProfileQueryData]) in
-            let mirrorSitesCount = key.mirrorSites.count
+            let mirrorSitesCount = key.mirrorSites.filter { !$0.wasRemoved() }.count
 
             for query in value {
                 let matchesFoundEvents = query.scanJobData.historyEvents
@@ -318,7 +322,8 @@ final class DataBrokerProtectionStatsPixels {
     /// If an opt-out wasn't submitted yet, we return 0.
     ///
     /// internal for testing purposes
-    func calculateDurationOfFirstOptOut(_ brokerProfileQueryData: [BrokerProfileQueryData]) -> Int {        guard let dateOfFirstScan = dateOfFirstScan(brokerProfileQueryData),
+    func calculateDurationOfFirstOptOut(_ brokerProfileQueryData: [BrokerProfileQueryData], from: Date? = nil) -> Int {
+        guard let dateOfFirstScan = dateOfFirstScan(brokerProfileQueryData),
               let dateOfFirstSubmittedOptOut = dateOfFirstSubmittedOptOut(brokerProfileQueryData) else {
             return 0
         }
@@ -327,7 +332,7 @@ final class DataBrokerProtectionStatsPixels {
             return 0
         }
 
-        guard let differenceInDays = DataBrokerProtectionPixelsUtilities.differenceBetweenDates(startDate: dateOfFirstScan, endDate: dateOfFirstSubmittedOptOut) else {
+        guard let differenceInDays = DataBrokerProtectionPixelsUtilities.numberOfDaysFrom(startDate: dateOfFirstScan, endDate: dateOfFirstSubmittedOptOut) else {
             return 0
         }
 
@@ -339,26 +344,33 @@ final class DataBrokerProtectionStatsPixels {
         return differenceInDays
     }
 
-    /// Returns the date of the first scan
-    private func dateOfFirstScan(_ brokerProfileQueryData: [BrokerProfileQueryData]) -> Date? {
+    /// Returns the date of the first scan since the beginning if not from Date is provided
+    private func dateOfFirstScan(_ brokerProfileQueryData: [BrokerProfileQueryData], from: Date? = nil) -> Date? {
         let allScanOperations = brokerProfileQueryData.map { $0.scanJobData }
         let allScanHistoryEvents = allScanOperations.flatMap { $0.historyEvents }
         let scanStartedEventsSortedByDate = allScanHistoryEvents
             .filter { $0.type == .scanStarted }
             .sorted { $0.date < $1.date }
 
-        return scanStartedEventsSortedByDate.first?.date
+        if let from = from {
+            return scanStartedEventsSortedByDate.filter { from < $0.date }.first?.date
+        } else {
+            return scanStartedEventsSortedByDate.first?.date
+        }
     }
 
-    /// Returns the date of the first sumbitted opt-out
-    private func dateOfFirstSubmittedOptOut(_ brokerProfileQueryData: [BrokerProfileQueryData]) -> Date? {
+    /// Returns the date of the first sumbitted opt-out. If no from date is provided, we return it from the beginning.
+    private func dateOfFirstSubmittedOptOut(_ brokerProfileQueryData: [BrokerProfileQueryData], from: Date? = nil) -> Date? {
         let firstOptOutSubmittedEvent = brokerProfileQueryData
             .flatMap { $0.optOutJobData }
             .flatMap { $0.historyEvents }
             .filter { $0.type == .optOutRequested }
             .sorted { $0.date < $1.date }
-            .first
 
-        return firstOptOutSubmittedEvent?.date
+        if let from = from {
+            return firstOptOutSubmittedEvent.filter { from < $0.date }.first?.date
+        } else {
+            return firstOptOutSubmittedEvent.first?.date
+        }
     }
 }

LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionStatsPixelsTests.swift

@@ -142,7 +142,7 @@ final class DataBrokerProtectionStatsPixelsTests: XCTestCase {
     }
 
     /// This test data has the following parameters
-    ///  - A broker that has two mirror sites
+    ///  - A broker that has two mirror sites but one was removed
     ///  - Four matches found
     ///  - One match was removed
     ///  - Two matches are in progress of being removed (this means we submitted the opt-out)
@@ -151,7 +151,7 @@ final class DataBrokerProtectionStatsPixelsTests: XCTestCase {
     func testStatsByBroker_hasCorrectParams() {
         let mirrorSites: [MirrorSite] = [
             .init(name: "Mirror #1", url: "url.com", addedAt: Date()),
-            .init(name: "Mirror #2", url: "url.com", addedAt: Date())
+            .init(name: "Mirror #2", url: "url.com", addedAt: Date(), removedAt: Date().yesterday)
         ]
         let broker: DataBroker = .mockWith(mirroSites: mirrorSites)
         let historyEventsForFirstOptOutOperation: [HistoryEvent] = [
@@ -191,12 +191,12 @@ final class DataBrokerProtectionStatsPixelsTests: XCTestCase {
 
         let result = sut.calculateByBroker(broker, data: [brokerProfileQueryData])
 
-        XCTAssertEqual(result.numberOfProfilesFound, 12)
-        XCTAssertEqual(result.numberOfOptOutsInProgress, 6)
-        XCTAssertEqual(result.numberOfSuccessfulOptOuts, 3)
-        XCTAssertEqual(result.numberOfFailureOptOuts, 3)
-        XCTAssertEqual(result.numberOfNewMatchesFound, 3)
-        XCTAssertEqual(result.numberOfReAppereances, 3)
+        XCTAssertEqual(result.numberOfProfilesFound, 8)
+        XCTAssertEqual(result.numberOfOptOutsInProgress, 4)
+        XCTAssertEqual(result.numberOfSuccessfulOptOuts, 2)
+        XCTAssertEqual(result.numberOfFailureOptOuts, 2)
+        XCTAssertEqual(result.numberOfNewMatchesFound, 2)
+        XCTAssertEqual(result.numberOfReAppereances, 2)
     }
 
     /// This test data has the following parameters