Merge pull request #8427 from woocommerce/issue/8173-sitesummarystats-dashboard-display

rachelmcr · web-flow · commit d71c1cf14ab9 · 2022-12-16T12:06:11.000Z
[My Store] Use stored site summary stats for visitors and conversion rate
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
@@ -3,6 +3,7 @@
 11.7
 -----
 - [**] Analytics Hub: Now you can select custom date ranges. [https://github.com/woocommerce/woocommerce-ios/pull/8414]
+- [*] My Store: We fixed an issue with Visitors and Conversion stats where sometimes visitors could be counted more than once in the selected period. [https://github.com/woocommerce/woocommerce-ios/pull/8427]
 
 
 11.6
diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Factories/StatsDataTextFormatter.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Factories/StatsDataTextFormatter.swift
@@ -100,22 +100,24 @@ struct StatsDataTextFormatter {
 
     // MARK: Views and Visitors Stats
 
-    /// Creates the text to display for the visitor count.
+    /// Creates the text to display for the visitor count based on SiteVisitStats data and a given interval.
     ///
-    static func createVisitorCountText(siteStats: SiteVisitStats?, selectedIntervalIndex: Int?) -> String {
+    static func createVisitorCountText(siteStats: SiteVisitStats?, selectedIntervalIndex: Int) -> String {
         if let visitorCount = visitorCount(at: selectedIntervalIndex, siteStats: siteStats) {
             return Double(visitorCount).humanReadableString()
         } else {
             return Constants.placeholderText
         }
     }
 
-    /// Creates the text to display for the visitor count delta.
+    /// Creates the text to display for the visitor count based on SiteSummaryStats data.
     ///
-    static func createVisitorCountDelta(from previousPeriod: SiteVisitStats?, to currentPeriod: SiteVisitStats?) -> DeltaPercentage {
-        let previousCount = visitorCount(at: nil, siteStats: previousPeriod)
-        let currentCount = visitorCount(at: nil, siteStats: currentPeriod)
-        return createDeltaPercentage(from: previousCount, to: currentCount)
+    static func createVisitorCountText(siteStats: SiteSummaryStats?) -> String {
+        guard let visitorCount = siteStats?.visitors else {
+            return Constants.placeholderText
+        }
+
+        return Double(visitorCount).humanReadableString()
     }
 
     /// Creates the text to display for the views count.
@@ -130,9 +132,9 @@ struct StatsDataTextFormatter {
 
     // MARK: Conversion Stats
 
-    /// Creates the text to display for the conversion rate.
+    /// Creates the text to display for the conversion rate based on SiteVisitStats data and a given interval.
     ///
-    static func createConversionRateText(orderStats: OrderStatsV4?, siteStats: SiteVisitStats?, selectedIntervalIndex: Int?) -> String {
+    static func createConversionRateText(orderStats: OrderStatsV4?, siteStats: SiteVisitStats?, selectedIntervalIndex: Int) -> String {
         guard let visitors = visitorCount(at: selectedIntervalIndex, siteStats: siteStats),
               let orders = orderCount(at: selectedIntervalIndex, orderStats: orderStats) else {
             return Constants.placeholderText
@@ -218,16 +220,14 @@ private extension StatsDataTextFormatter {
         return numberFormatter
     }()
 
-    /// Retrieves the visitor count for the provided order stats and, optionally, a specific interval.
+    /// Retrieves the visitor count for the provided site stats and a specific interval.
     ///
-    static func visitorCount(at selectedIndex: Int?, siteStats: SiteVisitStats?) -> Double? {
+    static func visitorCount(at selectedIndex: Int, siteStats: SiteVisitStats?) -> Double? {
         let siteStatsItems = siteStats?.items?.sorted(by: { (lhs, rhs) -> Bool in
             return lhs.period < rhs.period
         }) ?? []
-        if let selectedIndex, selectedIndex < siteStatsItems.count {
+        if selectedIndex < siteStatsItems.count {
             return Double(siteStatsItems[selectedIndex].visitors)
-        } else if let siteStats {
-            return Double(siteStats.totalVisitors)
         } else {
             return nil
         }
diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsAndTopPerformersPeriodViewController.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsAndTopPerformersPeriodViewController.swift
@@ -78,7 +78,10 @@ final class StoreStatsAndTopPerformersPeriodViewController: UIViewController {
     // MARK: Child View Controllers
 
     private lazy var storeStatsPeriodViewController: StoreStatsV4PeriodViewController = {
-        StoreStatsV4PeriodViewController(siteID: siteID, timeRange: timeRange, usageTracksEventEmitter: usageTracksEventEmitter)
+        StoreStatsV4PeriodViewController(siteID: siteID,
+                                         timeRange: timeRange,
+                                         currentDate: currentDate,
+                                         usageTracksEventEmitter: usageTracksEventEmitter)
     }()
 
     private lazy var inAppFeedbackCardViewController = InAppFeedbackCardViewController()
diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsPeriodViewModel.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsPeriodViewModel.swift
@@ -41,20 +41,31 @@ final class StoreStatsPeriodViewModel {
 
     /// Emits visitor stats text values based on site visit stats and selected time interval.
     private(set) lazy var visitorStatsText: AnyPublisher<String, Never> =
-    Publishers.CombineLatest($siteStats.eraseToAnyPublisher(), $selectedIntervalIndex.eraseToAnyPublisher())
-        .compactMap { siteStats, selectedIntervalIndex in
-            StatsDataTextFormatter.createVisitorCountText(siteStats: siteStats, selectedIntervalIndex: selectedIntervalIndex)
+    Publishers.CombineLatest3($siteStats.eraseToAnyPublisher(), $selectedIntervalIndex.eraseToAnyPublisher(), $summaryStats.eraseToAnyPublisher())
+        .compactMap { siteStats, selectedIntervalIndex, summaryStats in
+            if let selectedIntervalIndex {
+                return StatsDataTextFormatter.createVisitorCountText(siteStats: siteStats, selectedIntervalIndex: selectedIntervalIndex)
+            } else {
+                return StatsDataTextFormatter.createVisitorCountText(siteStats: summaryStats)
+            }
         }
         .removeDuplicates()
         .eraseToAnyPublisher()
 
     /// Emits conversion stats text values based on order stats, site visit stats, and selected time interval.
     private(set) lazy var conversionStatsText: AnyPublisher<String, Never> =
-    Publishers.CombineLatest3($orderStatsData.eraseToAnyPublisher(), $siteStats.eraseToAnyPublisher(), $selectedIntervalIndex.eraseToAnyPublisher())
-        .compactMap { orderStatsData, siteStats, selectedIntervalIndex in
-            StatsDataTextFormatter.createConversionRateText(orderStats: orderStatsData.stats,
-                                                            siteStats: siteStats,
-                                                            selectedIntervalIndex: selectedIntervalIndex)
+    Publishers.CombineLatest4($orderStatsData.eraseToAnyPublisher(),
+                              $siteStats.eraseToAnyPublisher(),
+                              $selectedIntervalIndex.eraseToAnyPublisher(),
+                              $summaryStats.eraseToAnyPublisher())
+        .compactMap { orderStatsData, siteStats, selectedIntervalIndex, summaryStats in
+            if let selectedIntervalIndex {
+                return StatsDataTextFormatter.createConversionRateText(orderStats: orderStatsData.stats,
+                                                                       siteStats: siteStats,
+                                                                       selectedIntervalIndex: selectedIntervalIndex)
+            } else {
+                return StatsDataTextFormatter.createConversionRateText(orderStats: orderStatsData.stats, siteStats: summaryStats)
+            }
         }
         .removeDuplicates()
         .eraseToAnyPublisher()
@@ -106,6 +117,7 @@ final class StoreStatsPeriodViewModel {
     // MARK: - Private data
 
     @Published private var siteStats: SiteVisitStats?
+    @Published private var summaryStats: SiteSummaryStats?
 
     typealias OrderStatsData = (stats: OrderStatsV4?, intervals: [OrderStatsV4Interval])
     @Published private var orderStatsData: OrderStatsData = (nil, [])
@@ -128,13 +140,27 @@ final class StoreStatsPeriodViewModel {
         return ResultsController(storageManager: storageManager, matching: predicate, sortedBy: [])
     }()
 
+    /// SiteSummaryStats ResultsController: Loads site summary stats from the Storage Layer
+    private lazy var summaryStatsResultsController: ResultsController<StorageSiteSummaryStats> = {
+        let formattedDateString: String = {
+            let date = timeRange.latestDate(currentDate: currentDate, siteTimezone: siteTimezone)
+            return StatsStoreV4.buildDateString(from: date, with: .day)
+        }()
+        let predicate = NSPredicate(format: "siteID = %ld AND period == %@ AND date == %@",
+                                    siteID,
+                                    timeRange.summaryStatsGranularity.rawValue,
+                                    formattedDateString)
+        return ResultsController(storageManager: storageManager, matching: predicate, sortedBy: [])
+    }()
+
     // MARK: - Configurations
 
     /// Updated externally when reloading data.
     var siteTimezone: TimeZone
 
     private let siteID: Int64
     private let timeRange: StatsTimeRangeV4
+    private let currentDate: Date
     private let currencyFormatter: CurrencyFormatter
     private let storageManager: StorageManagerType
     private let currencySettings: CurrencySettings
@@ -144,12 +170,14 @@ final class StoreStatsPeriodViewModel {
     init(siteID: Int64,
          timeRange: StatsTimeRangeV4,
          siteTimezone: TimeZone,
+         currentDate: Date,
          currencyFormatter: CurrencyFormatter,
          currencySettings: CurrencySettings,
          storageManager: StorageManagerType = ServiceLocator.storageManager) {
         self.siteID = siteID
         self.timeRange = timeRange
         self.siteTimezone = siteTimezone
+        self.currentDate = currentDate
         self.currencyFormatter = currencyFormatter
         self.currencySettings = currencySettings
         self.storageManager = storageManager
@@ -239,6 +267,7 @@ private extension StoreStatsPeriodViewModel {
     func configureResultsControllers() {
         configureSiteStatsResultsController()
         configureOrderStatsResultsController()
+        configureSummaryStatsResultsController()
     }
 
     func configureOrderStatsResultsController() {
@@ -260,6 +289,16 @@ private extension StoreStatsPeriodViewModel {
         }
         try? siteStatsResultsController.performFetch()
     }
+
+    func configureSummaryStatsResultsController() {
+        summaryStatsResultsController.onDidChangeContent = { [weak self] in
+            self?.updateSiteSummaryDataIfNeeded()
+        }
+        summaryStatsResultsController.onDidResetContent = { [weak self] in
+            self?.updateSiteSummaryDataIfNeeded()
+        }
+        try? summaryStatsResultsController.performFetch()
+    }
 }
 
 // MARK: - Private Helpers
@@ -269,6 +308,10 @@ private extension StoreStatsPeriodViewModel {
         siteStats = siteStatsResultsController.fetchedObjects.first
     }
 
+    func updateSiteSummaryDataIfNeeded() {
+        summaryStats = summaryStatsResultsController.fetchedObjects.first
+    }
+
     func updateOrderDataIfNeeded() {
         let orderStats = orderStatsResultsController.fetchedObjects.first
         let intervals = StatsIntervalDataParser.sortOrderStatsIntervals(from: orderStats)
diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsV4PeriodViewController.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsV4PeriodViewController.swift
@@ -128,6 +128,7 @@ final class StoreStatsV4PeriodViewController: UIViewController {
     ///
     init(siteID: Int64,
          timeRange: StatsTimeRangeV4,
+         currentDate: Date,
          currencyFormatter: CurrencyFormatter = CurrencyFormatter(currencySettings: ServiceLocator.currencySettings),
          currencySettings: CurrencySettings = ServiceLocator.currencySettings,
          usageTracksEventEmitter: StoreStatsUsageTracksEventEmitter) {
@@ -136,6 +137,7 @@ final class StoreStatsV4PeriodViewController: UIViewController {
         self.viewModel = StoreStatsPeriodViewModel(siteID: siteID,
                                                    timeRange: timeRange,
                                                    siteTimezone: siteTimezone,
+                                                   currentDate: currentDate,
                                                    currencyFormatter: currencyFormatter,
                                                    currencySettings: currencySettings)
         self.usageTracksEventEmitter = usageTracksEventEmitter
diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift
@@ -292,21 +292,18 @@ final class StatsDataTextFormatterTests: XCTestCase {
 
     // MARK: Views and Visitors Stats
 
-    // This test reflects the current method for computing total visitor count.
-    // It needs to be updated once this issue is fixed: https://github.com/woocommerce/woocommerce-ios/issues/8173
-    func test_createVisitorCountText_returns_expected_visitor_stats() {
+    func test_createVisitorCountText_for_SiteSummaryStats_returns_expected_visitor_stats() {
         // Given
-        let siteVisitStats = Yosemite.SiteVisitStats.fake().copy(items: [.fake().copy(period: "1", visitors: 17),
-                                                                         .fake().copy(period: "0", visitors: 5)])
+        let siteSummaryStats = Yosemite.SiteSummaryStats.fake().copy(visitors: 20)
 
         // When
-        let visitorCount = StatsDataTextFormatter.createVisitorCountText(siteStats: siteVisitStats, selectedIntervalIndex: nil)
+        let visitorCount = StatsDataTextFormatter.createVisitorCountText(siteStats: siteSummaryStats)
 
         // Then
-        XCTAssertEqual(visitorCount, "22")
+        XCTAssertEqual(visitorCount, "20")
     }
 
-    func test_createVisitorCountText_returns_expected_text_for_selected_interval() {
+    func test_createVisitorCountText_for_SiteVisitStats_returns_expected_text_for_selected_interval() {
         // Given
         let siteVisitStats = Yosemite.SiteVisitStats.fake().copy(items: [.fake().copy(period: "1", visitors: 17),
                                                                          .fake().copy(period: "0", visitors: 5)])
@@ -320,19 +317,6 @@ final class StatsDataTextFormatterTests: XCTestCase {
         XCTAssertEqual(visitorCount, "17")
     }
 
-    func test_createVisitorCountDelta_returns_expected_delta() {
-        // Given
-        let previousSiteStats = SiteVisitStats.fake().copy(items: [.fake().copy(period: "0", visitors: 10)])
-        let currentSiteStats = SiteVisitStats.fake().copy(items: [.fake().copy(period: "0", visitors: 15)])
-
-        // When
-        let visitorCountDelta = StatsDataTextFormatter.createVisitorCountDelta(from: previousSiteStats, to: currentSiteStats)
-
-        // Then
-        XCTAssertEqual(visitorCountDelta.string, "+50%")
-        XCTAssertEqual(visitorCountDelta.direction, .positive)
-    }
-
     func test_createViewsCountText_returns_expected_views_stats() {
         // Given
         let siteVisitStats = SiteSummaryStats.fake().copy(views: 250)
@@ -352,7 +336,7 @@ final class StatsDataTextFormatterTests: XCTestCase {
         let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 3))
 
         // When
-        let conversionRate = StatsDataTextFormatter.createConversionRateText(orderStats: orderStats, siteStats: siteVisitStats, selectedIntervalIndex: nil)
+        let conversionRate = StatsDataTextFormatter.createConversionRateText(orderStats: orderStats, siteStats: siteVisitStats, selectedIntervalIndex: 0)
 
         // Then
         XCTAssertEqual(conversionRate, "0%")
@@ -364,7 +348,7 @@ final class StatsDataTextFormatterTests: XCTestCase {
         let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 3557))
 
         // When
-        let conversionRate = StatsDataTextFormatter.createConversionRateText(orderStats: orderStats, siteStats: siteVisitStats, selectedIntervalIndex: nil)
+        let conversionRate = StatsDataTextFormatter.createConversionRateText(orderStats: orderStats, siteStats: siteVisitStats, selectedIntervalIndex: 0)
 
         // Then
         XCTAssertEqual(conversionRate, "35.6%") // order count: 3557, visitor count: 10000 => 0.3557 (35.57%)
@@ -375,24 +359,11 @@ final class StatsDataTextFormatterTests: XCTestCase {
         let siteVisitStats = Yosemite.SiteVisitStats.fake().copy(items: [.fake().copy(visitors: 10)])
         let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 3))
 
-        // When
-        let conversionRate = StatsDataTextFormatter.createConversionRateText(orderStats: orderStats, siteStats: siteVisitStats, selectedIntervalIndex: nil)
-
-        // Then
-        XCTAssertEqual(conversionRate, "30%") // order count: 3, visitor count: 10 => 0.3 (30%)
-    }
-
-    func test_createConversionRateText_for_SiteVisitStats_returns_expected_text_for_selected_interval() {
-        // Given
-        let siteVisitStats = Yosemite.SiteVisitStats.fake().copy(items: [.fake().copy(visitors: 10)])
-        let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 2),
-                                                  intervals: [.fake().copy(subtotals: .fake().copy(totalOrders: 1))])
-
         // When
         let conversionRate = StatsDataTextFormatter.createConversionRateText(orderStats: orderStats, siteStats: siteVisitStats, selectedIntervalIndex: 0)
 
         // Then
-        XCTAssertEqual(conversionRate, "10%")
+        XCTAssertEqual(conversionRate, "30%") // order count: 3, visitor count: 10 => 0.3 (30%)
     }
 
     func test_createConversionRateText_for_SiteSummaryStats_returns_placeholder_when_visitor_count_is_zero() {
diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Stats V4/StoreStatsPeriodViewModelTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Stats V4/StoreStatsPeriodViewModelTests.swift
diff --git a/Yosemite/Yosemite/Model/Model.swift b/Yosemite/Yosemite/Model/Model.swift
