From 3083afa25c796e44b5696bc949d1ec8fd2280456 Mon Sep 17 00:00:00 2001 From: Evgeny Aleksandrov Date: Tue, 13 Dec 2022 18:25:39 +0300 Subject: [PATCH 1/8] Add createViewsCountText --- .../Dashboard/Factories/StatsDataTextFormatter.swift | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Factories/StatsDataTextFormatter.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Factories/StatsDataTextFormatter.swift index 422e03d1acf..227175c811a 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Factories/StatsDataTextFormatter.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Factories/StatsDataTextFormatter.swift @@ -1,6 +1,7 @@ import Foundation import Yosemite import WooFoundation +import struct Networking.SiteSummaryStats /// Helpers for calculating and formatting stats data for display. /// @@ -118,6 +119,16 @@ struct StatsDataTextFormatter { return createDeltaPercentage(from: previousCount, to: currentCount) } + /// Creates the text to display for the views count. + /// + static func createViewsCountText(siteStats: SiteSummaryStats?) -> String { + guard let viewsCount = siteStats?.views else { + return Constants.placeholderText + } + + return Double(viewsCount).humanReadableString() + } + // MARK: Conversion Stats /// Creates the text to display for the conversion rate. From 2189baa60d19c9ab3a0a3fd1abcdd3750a951b87 Mon Sep 17 00:00:00 2001 From: Evgeny Aleksandrov Date: Tue, 13 Dec 2022 18:19:52 +0300 Subject: [PATCH 2/8] Add GeneratedCopiable, GeneratedFakeable to SiteSummaryStats --- Fakes/Fakes/Networking.generated.swift | 13 ++++++++++ .../Copiable/Models+Copiable.generated.swift | 24 +++++++++++++++++++ .../Model/Stats/SiteSummaryStats.swift | 2 +- 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/Fakes/Fakes/Networking.generated.swift b/Fakes/Fakes/Networking.generated.swift index 4daa7f8b7a0..e1f8e0f77fa 100644 --- a/Fakes/Fakes/Networking.generated.swift +++ b/Fakes/Fakes/Networking.generated.swift @@ -1593,6 +1593,19 @@ extension SiteSettingGroup { .general } } +extension Networking.SiteSummaryStats { + /// Returns a "ready to use" type filled with fake values. + /// + public static func fake() -> Networking.SiteSummaryStats { + .init( + siteID: .fake(), + date: .fake(), + period: .fake(), + visitors: .fake(), + views: .fake() + ) + } +} extension Networking.SiteVisitStats { /// Returns a "ready to use" type filled with fake values. /// diff --git a/Networking/Networking/Model/Copiable/Models+Copiable.generated.swift b/Networking/Networking/Model/Copiable/Models+Copiable.generated.swift index 8a6f03fe222..8368414410d 100644 --- a/Networking/Networking/Model/Copiable/Models+Copiable.generated.swift +++ b/Networking/Networking/Model/Copiable/Models+Copiable.generated.swift @@ -1912,6 +1912,30 @@ extension Networking.SiteSetting { } } +extension Networking.SiteSummaryStats { + public func copy( + siteID: CopiableProp = .copy, + date: CopiableProp = .copy, + period: CopiableProp = .copy, + visitors: CopiableProp = .copy, + views: CopiableProp = .copy + ) -> Networking.SiteSummaryStats { + let siteID = siteID ?? self.siteID + let date = date ?? self.date + let period = period ?? self.period + let visitors = visitors ?? self.visitors + let views = views ?? self.views + + return Networking.SiteSummaryStats( + siteID: siteID, + date: date, + period: period, + visitors: visitors, + views: views + ) + } +} + extension Networking.SiteVisitStats { public func copy( siteID: CopiableProp = .copy, diff --git a/Networking/Networking/Model/Stats/SiteSummaryStats.swift b/Networking/Networking/Model/Stats/SiteSummaryStats.swift index 20ee4f7dcb9..64897f89a8e 100644 --- a/Networking/Networking/Model/Stats/SiteSummaryStats.swift +++ b/Networking/Networking/Model/Stats/SiteSummaryStats.swift @@ -3,7 +3,7 @@ import Codegen /// Represents site summary stats for a specific period. /// -public struct SiteSummaryStats: Decodable { +public struct SiteSummaryStats: Decodable, GeneratedCopiable, GeneratedFakeable { public let siteID: Int64 public let date: String public let period: StatGranularity From be20da1ba698a7bfd811125e04012a164345270e Mon Sep 17 00:00:00 2001 From: Evgeny Aleksandrov Date: Tue, 13 Dec 2022 18:25:53 +0300 Subject: [PATCH 3/8] Add test --- .../Factories/StatsDataTextFormatterTests.swift | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift index 5fdb95f458f..d059a87563e 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift @@ -1,6 +1,7 @@ import XCTest import Yosemite import WooFoundation +import struct Networking.SiteSummaryStats @testable import WooCommerce /// `StatsDataTextFormatter` tests. @@ -333,6 +334,17 @@ final class StatsDataTextFormatterTests: XCTestCase { XCTAssertEqual(visitorCountDelta.direction, .positive) } + func test_createViewsCountText_returns_expected_views_stats() { + // Given + let siteVisitStats = Networking.SiteSummaryStats.fake().copy(views: 250) + + // When + let viewsCount = StatsDataTextFormatter.createViewsCountText(siteStats: siteVisitStats) + + // Then + XCTAssertEqual(viewsCount, "250") + } + // MARK: Conversion Stats func test_createConversionRateText_returns_placeholder_when_visitor_count_is_zero() { From 64033eb3bbd0857df2f72e15504b3dbc01f4c488 Mon Sep 17 00:00:00 2001 From: Evgeny Aleksandrov Date: Tue, 13 Dec 2022 18:26:34 +0300 Subject: [PATCH 4/8] Add createConversionRateText with SiteSummaryStats input --- .../Factories/StatsDataTextFormatter.swift | 40 ++++++++++++------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Factories/StatsDataTextFormatter.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Factories/StatsDataTextFormatter.swift index 227175c811a..36a152d679c 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Factories/StatsDataTextFormatter.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Factories/StatsDataTextFormatter.swift @@ -134,22 +134,22 @@ struct StatsDataTextFormatter { /// Creates the text to display for the conversion rate. /// static func createConversionRateText(orderStats: OrderStatsV4?, siteStats: SiteVisitStats?, selectedIntervalIndex: Int?) -> String { - let visitors = visitorCount(at: selectedIntervalIndex, siteStats: siteStats) - let orders = orderCount(at: selectedIntervalIndex, orderStats: orderStats) + guard let visitors = visitorCount(at: selectedIntervalIndex, siteStats: siteStats), + let orders = orderCount(at: selectedIntervalIndex, orderStats: orderStats) else { + return Constants.placeholderText + } - let numberFormatter = NumberFormatter() - numberFormatter.numberStyle = .percent - numberFormatter.minimumFractionDigits = 1 + return createConversionRateText(converted: orders, total: visitors) + } - if let visitors, let orders { - // Maximum conversion rate is 100%. - let conversionRate = visitors > 0 ? min(orders/visitors, 1): 0 - let minimumFractionDigits = floor(conversionRate * 100.0) == conversionRate * 100.0 ? 0: 1 - numberFormatter.minimumFractionDigits = minimumFractionDigits - return numberFormatter.string(from: conversionRate as NSNumber) ?? Constants.placeholderText - } else { + /// Creates the text to display for the conversion rate based on SiteSummaryStats data. + /// + static func createConversionRateText(orderStats: OrderStatsV4?, siteStats: SiteSummaryStats?) -> String { + guard let visitors = siteStats?.visitors, let orders = orderStats?.totals.totalOrders else { return Constants.placeholderText } + + return createConversionRateText(converted: Double(orders), total: Double(visitors)) } // MARK: Product Stats @@ -272,9 +272,21 @@ private extension StatsDataTextFormatter { } } + /// Creates the text to display for the conversion rate from 2 input values. + /// + static func createConversionRateText(converted: Double, total: Double) -> String { + let numberFormatter = NumberFormatter() + numberFormatter.numberStyle = .percent + numberFormatter.minimumFractionDigits = 1 + + // Maximum conversion rate is 100%. + let conversionRate = total > 0 ? min(converted/total, 1) : 0 + let minimumFractionDigits = floor(conversionRate * 100.0) == conversionRate * 100.0 ? 0 : 1 + numberFormatter.minimumFractionDigits = minimumFractionDigits + return numberFormatter.string(from: conversionRate as NSNumber) ?? Constants.placeholderText + } + enum Constants { static let placeholderText = "-" } - - } From 4227993ad1d24221abcb208c7020dcbfab43f212 Mon Sep 17 00:00:00 2001 From: Evgeny Aleksandrov Date: Tue, 13 Dec 2022 18:26:41 +0300 Subject: [PATCH 5/8] Add tests --- .../StatsDataTextFormatterTests.swift | 44 +++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift index d059a87563e..c9ca69302d3 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift @@ -347,7 +347,7 @@ final class StatsDataTextFormatterTests: XCTestCase { // MARK: Conversion Stats - func test_createConversionRateText_returns_placeholder_when_visitor_count_is_zero() { + func test_createConversionRateText_for_SiteVisitStats_returns_placeholder_when_visitor_count_is_zero() { // Given let siteVisitStats = Yosemite.SiteVisitStats.fake().copy(items: [.fake().copy(visitors: 0)]) let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 3)) @@ -359,7 +359,7 @@ final class StatsDataTextFormatterTests: XCTestCase { XCTAssertEqual(conversionRate, "0%") } - func test_createConversionRateText_returns_one_decimal_point_when_percentage_value_has_two_decimal_points() { + func test_createConversionRateText_for_SiteVisitStats_returns_one_decimal_point_when_percentage_value_has_two_decimal_points() { // Given let siteVisitStats = Yosemite.SiteVisitStats.fake().copy(items: [.fake().copy(visitors: 10000)]) let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 3557)) @@ -371,7 +371,7 @@ final class StatsDataTextFormatterTests: XCTestCase { XCTAssertEqual(conversionRate, "35.6%") // order count: 3557, visitor count: 10000 => 0.3557 (35.57%) } - func test_createConversionRateText_returns_no_decimal_point_when_percentage_value_is_integer() { + func test_createConversionRateText_for_SiteVisitStats_returns_no_decimal_point_when_percentage_value_is_integer() { // Given let siteVisitStats = Yosemite.SiteVisitStats.fake().copy(items: [.fake().copy(visitors: 10)]) let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 3)) @@ -383,7 +383,7 @@ final class StatsDataTextFormatterTests: XCTestCase { XCTAssertEqual(conversionRate, "30%") // order count: 3, visitor count: 10 => 0.3 (30%) } - func test_createConversionRateText_returns_expected_text_for_selected_interval() { + 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), @@ -396,6 +396,42 @@ final class StatsDataTextFormatterTests: XCTestCase { XCTAssertEqual(conversionRate, "10%") } + func test_createConversionRateText_for_SiteSummaryStats_returns_placeholder_when_visitor_count_is_zero() { + // Given + let siteVisitStats = Networking.SiteSummaryStats.fake().copy(visitors: 0) + let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 3)) + + // When + let conversionRate = StatsDataTextFormatter.createConversionRateText(orderStats: orderStats, siteStats: siteVisitStats) + + // Then + XCTAssertEqual(conversionRate, "0%") + } + + func test_createConversionRateText_for_SiteSummaryStats_returns_one_decimal_point_when_percentage_value_has_two_decimal_points() { + // Given + let siteVisitStats = Networking.SiteSummaryStats.fake().copy(visitors: 10000) + let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 3557)) + + // When + let conversionRate = StatsDataTextFormatter.createConversionRateText(orderStats: orderStats, siteStats: siteVisitStats) + + // Then + XCTAssertEqual(conversionRate, "35.6%") // order count: 3557, visitor count: 10000 => 0.3557 (35.57%) + } + + func test_createConversionRateText_for_SiteSummaryStats_returns_no_decimal_point_when_percentage_value_is_integer() { + // Given + let siteVisitStats = Networking.SiteSummaryStats.fake().copy(visitors: 10) + let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 3)) + + // When + let conversionRate = StatsDataTextFormatter.createConversionRateText(orderStats: orderStats, siteStats: siteVisitStats) + + // Then + XCTAssertEqual(conversionRate, "30%") // order count: 3, visitor count: 10 => 0.3 (30%) + } + // MARK: Delta Calculations func test_createDeltaPercentage_returns_expected_positive_delta() { From 39451de73f4ef3a7ba2077db6b195d664149fd6a Mon Sep 17 00:00:00 2001 From: Evgeny Aleksandrov Date: Tue, 13 Dec 2022 18:28:58 +0300 Subject: [PATCH 6/8] Add SiteSummaryStats typealias to Yosemite --- Yosemite/Yosemite/Model/Model.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Yosemite/Yosemite/Model/Model.swift b/Yosemite/Yosemite/Model/Model.swift index 8cd0cdce160..acd021c5547 100644 --- a/Yosemite/Yosemite/Model/Model.swift +++ b/Yosemite/Yosemite/Model/Model.swift @@ -125,6 +125,7 @@ public typealias SitePlugin = Networking.SitePlugin public typealias SitePluginStatusEnum = Networking.SitePluginStatusEnum public typealias SiteSetting = Networking.SiteSetting public typealias SiteSettingGroup = Networking.SiteSettingGroup +public typealias SiteSummaryStats = Networking.SiteSummaryStats public typealias SiteVisitStats = Networking.SiteVisitStats public typealias SiteVisitStatsItem = Networking.SiteVisitStatsItem public typealias StateOfACountry = Networking.StateOfACountry From a42a1654e079e1ce11a7c04614873154c68bb2c2 Mon Sep 17 00:00:00 2001 From: Evgeny Aleksandrov Date: Tue, 13 Dec 2022 18:29:50 +0300 Subject: [PATCH 7/8] Remove unnecessary Networking import --- .../Dashboard/Factories/StatsDataTextFormatter.swift | 1 - .../Factories/StatsDataTextFormatterTests.swift | 9 ++++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/WooCommerce/Classes/ViewRelated/Dashboard/Factories/StatsDataTextFormatter.swift b/WooCommerce/Classes/ViewRelated/Dashboard/Factories/StatsDataTextFormatter.swift index 36a152d679c..63a05847558 100644 --- a/WooCommerce/Classes/ViewRelated/Dashboard/Factories/StatsDataTextFormatter.swift +++ b/WooCommerce/Classes/ViewRelated/Dashboard/Factories/StatsDataTextFormatter.swift @@ -1,7 +1,6 @@ import Foundation import Yosemite import WooFoundation -import struct Networking.SiteSummaryStats /// Helpers for calculating and formatting stats data for display. /// diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift index c9ca69302d3..67c43bfe3f8 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift @@ -1,7 +1,6 @@ import XCTest import Yosemite import WooFoundation -import struct Networking.SiteSummaryStats @testable import WooCommerce /// `StatsDataTextFormatter` tests. @@ -336,7 +335,7 @@ final class StatsDataTextFormatterTests: XCTestCase { func test_createViewsCountText_returns_expected_views_stats() { // Given - let siteVisitStats = Networking.SiteSummaryStats.fake().copy(views: 250) + let siteVisitStats = SiteSummaryStats.fake().copy(views: 250) // When let viewsCount = StatsDataTextFormatter.createViewsCountText(siteStats: siteVisitStats) @@ -398,7 +397,7 @@ final class StatsDataTextFormatterTests: XCTestCase { func test_createConversionRateText_for_SiteSummaryStats_returns_placeholder_when_visitor_count_is_zero() { // Given - let siteVisitStats = Networking.SiteSummaryStats.fake().copy(visitors: 0) + let siteVisitStats = SiteSummaryStats.fake().copy(visitors: 0) let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 3)) // When @@ -410,7 +409,7 @@ final class StatsDataTextFormatterTests: XCTestCase { func test_createConversionRateText_for_SiteSummaryStats_returns_one_decimal_point_when_percentage_value_has_two_decimal_points() { // Given - let siteVisitStats = Networking.SiteSummaryStats.fake().copy(visitors: 10000) + let siteVisitStats = SiteSummaryStats.fake().copy(visitors: 10000) let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 3557)) // When @@ -422,7 +421,7 @@ final class StatsDataTextFormatterTests: XCTestCase { func test_createConversionRateText_for_SiteSummaryStats_returns_no_decimal_point_when_percentage_value_is_integer() { // Given - let siteVisitStats = Networking.SiteSummaryStats.fake().copy(visitors: 10) + let siteVisitStats = SiteSummaryStats.fake().copy(visitors: 10) let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 3)) // When From 7f6731e3838f1d7fbf8efc9714f6c3b50bd4eb63 Mon Sep 17 00:00:00 2001 From: Evgeny Aleksandrov Date: Tue, 13 Dec 2022 18:41:11 +0300 Subject: [PATCH 8/8] Rename variable in tests --- .../Factories/StatsDataTextFormatterTests.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift index 67c43bfe3f8..f99beff415c 100644 --- a/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift +++ b/WooCommerce/WooCommerceTests/ViewRelated/Dashboard/Factories/StatsDataTextFormatterTests.swift @@ -397,11 +397,11 @@ final class StatsDataTextFormatterTests: XCTestCase { func test_createConversionRateText_for_SiteSummaryStats_returns_placeholder_when_visitor_count_is_zero() { // Given - let siteVisitStats = SiteSummaryStats.fake().copy(visitors: 0) + let siteSummaryStats = SiteSummaryStats.fake().copy(visitors: 0) let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 3)) // When - let conversionRate = StatsDataTextFormatter.createConversionRateText(orderStats: orderStats, siteStats: siteVisitStats) + let conversionRate = StatsDataTextFormatter.createConversionRateText(orderStats: orderStats, siteStats: siteSummaryStats) // Then XCTAssertEqual(conversionRate, "0%") @@ -409,11 +409,11 @@ final class StatsDataTextFormatterTests: XCTestCase { func test_createConversionRateText_for_SiteSummaryStats_returns_one_decimal_point_when_percentage_value_has_two_decimal_points() { // Given - let siteVisitStats = SiteSummaryStats.fake().copy(visitors: 10000) + let siteSummaryStats = SiteSummaryStats.fake().copy(visitors: 10000) let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 3557)) // When - let conversionRate = StatsDataTextFormatter.createConversionRateText(orderStats: orderStats, siteStats: siteVisitStats) + let conversionRate = StatsDataTextFormatter.createConversionRateText(orderStats: orderStats, siteStats: siteSummaryStats) // Then XCTAssertEqual(conversionRate, "35.6%") // order count: 3557, visitor count: 10000 => 0.3557 (35.57%) @@ -421,11 +421,11 @@ final class StatsDataTextFormatterTests: XCTestCase { func test_createConversionRateText_for_SiteSummaryStats_returns_no_decimal_point_when_percentage_value_is_integer() { // Given - let siteVisitStats = SiteSummaryStats.fake().copy(visitors: 10) + let siteSummaryStats = SiteSummaryStats.fake().copy(visitors: 10) let orderStats = OrderStatsV4.fake().copy(totals: .fake().copy(totalOrders: 3)) // When - let conversionRate = StatsDataTextFormatter.createConversionRateText(orderStats: orderStats, siteStats: siteVisitStats) + let conversionRate = StatsDataTextFormatter.createConversionRateText(orderStats: orderStats, siteStats: siteSummaryStats) // Then XCTAssertEqual(conversionRate, "30%") // order count: 3, visitor count: 10 => 0.3 (30%)