Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions WooCommerce/StoreWidgets/Homescreen/StoreInfoView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ private extension StoreInfoView {
static let viewMore = AppLocalizedString(
"storeWidgets.infoView.viewMore",
value: "View More",
comment: "Title for the button indicator to display more stats in the Today's Stat widget when using accessibility fonts."
comment: "Title for the button indicator to display more stats in the Stat widget when using accessibility fonts."
)
static func updatedAt(_ updatedTime: String) -> LocalizedString {
let format = AppLocalizedString("storeWidgets.infoView.updatedAt",
Expand All @@ -247,7 +247,7 @@ private extension NotLoggedInView {
enum Localization {
static let notLoggedIn = AppLocalizedString(
"storeWidgets.notLoggedInView.notLoggedIn",
value: "Log in to see today’s stats.",
value: "Log in to see store’s stats.",
comment: "Title label when the widget does not have a logged-in store."
)
static let login = AppLocalizedString(
Expand All @@ -269,7 +269,7 @@ private extension UnableToFetchView {
enum Localization {
static let unableToFetch = AppLocalizedString(
"storeWidgets.unableToFetchView.unableToFetch",
value: "Unable to fetch today's stats",
value: "Unable to fetch store's stats",
comment: "Title label when the widget can't fetch data."
)
}
Expand Down
97 changes: 97 additions & 0 deletions WooCommerce/StoreWidgets/StatsTimeRange.swift
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
import Foundation
import WooFoundation
import enum Networking.StatGranularity
import enum Networking.StatsGranularityV4

/// Represents the time range for an Order Stats v4 model.
/// This is a local property and not in the remote response.
///
Expand Down Expand Up @@ -25,4 +30,96 @@ extension StatsTimeRange {
self = .thisYear
}
}

/// The maximum number of stats intervals a time range could have.
var maxNumberOfIntervals: Int {
switch self {
case .today:
return 24
case .thisWeek:
return 7
case .thisMonth:
return 31
case .thisYear:
return 12
}
}
Comment on lines +33 to +46
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think all of these properties should be shared in a commonplace with the main stats code?
Since we are going to update the analytics dashboard soon, there is a chance that these go outdated.

Copy link
Contributor Author

@ealeksandrov ealeksandrov Oct 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about moving StatsTimeRangeV4 to WooFoundation, but it imports Networking models. Do you think it's ok to allow WooFoundation depend on Networking? Or can we move all related models to WF and make Networking import it?

Original StatsTimeRangeV4 currently lives in Yosemite. I copy-pasted the code to prevent the import of Yosemite+Storage (+ Networking but it's already imported to widgets target).

Copy link
Contributor

@Ecarrion Ecarrion Oct 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WooFoundation depend on Networking

Probs not. Maybe we can move that to an extension helper that is imported in both targets?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be WooFoundation? I don't like idea of creating another library for a few models.

move all related models to WF and make Networking import it?

  • move StatsTimeRangeV4 from Yosemite to WF
  • move StatGranularity from Networking to WF
  • import WF everywhere
  • remove copy-pasted StatsTimeRange and use StatsTimeRangeV4 in widgets from WF

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That also work, may involve more work tho!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I'll merge current implementation into the feature branch and we can experiment with dependencies later.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a chance that these go outdated

☝️ What are the chances of this happening? What is the risk? How many times is this duplicated?

Maybe we can also use the Rule of three as a guideline.

Another library is also fine to me, assuming the cost is smaller compared to trying to place it in WooFoundation.


/// Represents the period unit of the store stats using Stats v4 API given a time range.
var intervalGranularity: StatsGranularityV4 {
switch self {
case .today:
return .hourly
case .thisWeek:
return .daily
case .thisMonth:
return .daily
case .thisYear:
return .monthly
}
}

/// Represents the period unit of the site visit stats given a time range.
var siteVisitStatsGranularity: StatGranularity {
switch self {
case .today, .thisWeek, .thisMonth:
return .day
case .thisYear:
return .month
}
}

/// The number of intervals for site visit stats to fetch given a time range.
/// The interval unit is in `siteVisitStatsGranularity`.
func siteVisitStatsQuantity(date: Date, siteTimezone: TimeZone) -> Int {
switch self {
case .today:
return 1
case .thisWeek:
return 7
case .thisMonth:
var calendar = Calendar.current
calendar.timeZone = siteTimezone
let daysThisMonth = calendar.range(of: .day, in: .month, for: date)
return daysThisMonth?.count ?? 0
case .thisYear:
return 12
}
}

/// Returns the latest date to be shown for the time range, given the current date and site time zone
///
/// - Parameters:
/// - currentDate: the date which the latest date is based on
/// - siteTimezone: site time zone, which the stats data are based on
func latestDate(currentDate: Date, siteTimezone: TimeZone) -> Date {
switch self {
case .today:
return currentDate.endOfDay(timezone: siteTimezone)
case .thisWeek:
return currentDate.endOfWeek(timezone: siteTimezone)
case .thisMonth:
return currentDate.endOfMonth(timezone: siteTimezone)
case .thisYear:
return currentDate.endOfYear(timezone: siteTimezone)
}
}

/// Returns the earliest date to be shown for the time range, given the latest date and site time zone
///
/// - Parameters:
/// - latestDate: the date which the earliest date is based on
/// - siteTimezone: site time zone, which the stats data are based on
func earliestDate(latestDate: Date, siteTimezone: TimeZone) -> Date {
switch self {
case .today:
return latestDate.startOfDay(timezone: siteTimezone)
case .thisWeek:
return latestDate.startOfWeek(timezone: siteTimezone)
case .thisMonth:
return latestDate.startOfMonth(timezone: siteTimezone)
case .thisYear:
return latestDate.startOfYear(timezone: siteTimezone)
}
}
}
39 changes: 23 additions & 16 deletions WooCommerce/StoreWidgets/StoreInfoDataService.swift
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import Networking

/// Orchestrator class that fetches today store stats data.
/// Orchestrator class that fetches store stats data.
///
final class StoreInfoDataService {

/// Data extracted from networking types.
///
struct Stats {
let timeRange: StatsTimeRange
let revenue: Decimal
let totalOrders: Int
let totalVisitors: Int
Expand Down Expand Up @@ -35,15 +36,16 @@ final class StoreInfoDataService {
///
func fetchStats(for storeID: Int64, timeRange: StatsTimeRange) async throws -> Stats {
// Prepare them to run in parallel
async let revenueAndOrdersRequest = fetchTodaysRevenueAndOrders(for: storeID)
async let visitorsRequest = fetchTodaysVisitors(for: storeID)
async let revenueAndOrdersRequest = fetchRevenueAndOrders(for: storeID, timeRange: timeRange)
async let visitorsRequest = fetchVisitors(for: storeID, timeRange: timeRange)

// Wait for for response
let (revenueAndOrders, visitors) = try await (revenueAndOrdersRequest, visitorsRequest)

// Assemble stats data
let conversion = visitors.totalVisitors > 0 ? Double(revenueAndOrders.totals.totalOrders) / Double(visitors.totalVisitors) : 0
return Stats(revenue: revenueAndOrders.totals.grossRevenue,
return Stats(timeRange: timeRange,
revenue: revenueAndOrders.totals.grossRevenue,
totalOrders: revenueAndOrders.totals.totalOrders,
totalVisitors: visitors.totalVisitors,
conversion: min(conversion, 1))
Expand All @@ -54,34 +56,39 @@ final class StoreInfoDataService {
///
private extension StoreInfoDataService {

/// Async wrapper that fetches todays revenues & orders.
/// Async wrapper that fetches revenues & orders.
///
func fetchTodaysRevenueAndOrders(for storeID: Int64) async throws -> OrderStatsV4 {
func fetchRevenueAndOrders(for storeID: Int64, timeRange: StatsTimeRange) async throws -> OrderStatsV4 {
try await withCheckedThrowingContinuation { continuation in
// `WKWebView` is accessed internally, we are forced to dispatch the call in the main thread.
Task { @MainActor in
let earliestDateToInclude = timeRange.earliestDate(latestDate: Date(), siteTimezone: .current)
let latestDateToInclude = timeRange.latestDate(currentDate: Date(), siteTimezone: .current)
orderStatsRemoteV4.loadOrderStats(for: storeID,
unit: .hourly,
earliestDateToInclude: Date().startOfDay(timezone: .current),
latestDateToInclude: Date().endOfDay(timezone: .current),
quantity: 24,
unit: timeRange.intervalGranularity,
earliestDateToInclude: earliestDateToInclude,
latestDateToInclude: latestDateToInclude,
quantity: timeRange.maxNumberOfIntervals,
forceRefresh: true) { result in
continuation.resume(with: result)
}
}
}
}

/// Async wrapper that fetches todays visitors.
/// Async wrapper that fetches visitors.
///
func fetchTodaysVisitors(for storeID: Int64) async throws -> SiteVisitStats {
func fetchVisitors(for storeID: Int64, timeRange: StatsTimeRange) async throws -> SiteVisitStats {
try await withCheckedThrowingContinuation { continuation in
// `WKWebView` is accessed internally, we are foreced to dispatch the call in the main thread.
// `WKWebView` is accessed internally, we are forced to dispatch the call in the main thread.
Task { @MainActor in
let latestDateToInclude = timeRange.latestDate(currentDate: Date(), siteTimezone: .current)
let quantity = timeRange.siteVisitStatsQuantity(date: latestDateToInclude, siteTimezone: .current)

siteVisitStatsRemote.loadSiteVisitorStats(for: storeID,
unit: .day,
latestDateToInclude: Date().endOfDay(timezone: .current),
quantity: 1) { result in
unit: timeRange.siteVisitStatsGranularity,
latestDateToInclude: latestDateToInclude,
quantity: quantity) { result in
continuation.resume(with: result)
}
}
Expand Down
56 changes: 40 additions & 16 deletions WooCommerce/StoreWidgets/StoreInfoProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,14 @@ final class StoreInfoProvider: IntentTimelineProvider {
networkService = strongService
Task {
do {
let todayStats = try await strongService.fetchStats(for: dependencies.storeID, timeRange: StatsTimeRange(configuration.timeRange))
let entry = Self.dataEntry(for: todayStats, with: dependencies)
let stats = try await strongService.fetchStats(for: dependencies.storeID, timeRange: StatsTimeRange(configuration.timeRange))
let entry = Self.dataEntry(for: stats, with: dependencies)
let reloadDate = Date(timeIntervalSinceNow: reloadInterval)
let timeline = Timeline<StoreInfoEntry>(entries: [entry], policy: .after(reloadDate))
completion(timeline)
} catch {
// WooFoundation does not expose `DDLOG` types. Should we include them?
print("⛔️ Error fetching today's widget stats: \(error)")
print("⛔️ Error fetching widget stats: \(error)")

let reloadDate = Date(timeIntervalSinceNow: reloadInterval)
let timeline = Timeline<StoreInfoEntry>(entries: [.error], policy: .after(reloadDate))
Expand Down Expand Up @@ -143,7 +143,7 @@ private extension StoreInfoProvider {
/// Redacted entry with sample data. If dependencies are available - store name and currency settings will be used.
///
static func placeholderEntry(for dependencies: Dependencies?) -> StoreInfoEntry {
StoreInfoEntry.data(.init(range: Localization.today,
StoreInfoEntry.data(.init(range: Localization.periodString(from: .today),
name: dependencies?.storeName ?? Localization.myShop,
revenue: Self.formattedAmountString(for: 132.234, with: dependencies?.storeCurrencySettings),
revenueCompact: Self.formattedAmountCompactString(for: 132.234, with: dependencies?.storeCurrencySettings),
Expand All @@ -155,14 +155,14 @@ private extension StoreInfoProvider {

/// Real data entry.
///
static func dataEntry(for todayStats: StoreInfoDataService.Stats, with dependencies: Dependencies) -> StoreInfoEntry {
StoreInfoEntry.data(.init(range: Localization.today,
static func dataEntry(for stats: StoreInfoDataService.Stats, with dependencies: Dependencies) -> StoreInfoEntry {
StoreInfoEntry.data(.init(range: Localization.periodString(from: stats.timeRange),
name: dependencies.storeName,
revenue: Self.formattedAmountString(for: todayStats.revenue, with: dependencies.storeCurrencySettings),
revenueCompact: Self.formattedAmountCompactString(for: todayStats.revenue, with: dependencies.storeCurrencySettings),
visitors: "\(todayStats.totalVisitors)",
orders: "\(todayStats.totalOrders)",
conversion: Self.formattedConversionString(for: todayStats.conversion),
revenue: Self.formattedAmountString(for: stats.revenue, with: dependencies.storeCurrencySettings),
revenueCompact: Self.formattedAmountCompactString(for: stats.revenue, with: dependencies.storeCurrencySettings),
visitors: "\(stats.totalVisitors)",
orders: "\(stats.totalOrders)",
conversion: Self.formattedConversionString(for: stats.conversion),
updatedTime: Self.currentFormattedTime()))
}

Expand Down Expand Up @@ -206,10 +206,34 @@ private extension StoreInfoProvider {
value: "My Shop",
comment: "Generic store name for the store info widget preview"
)
static let today = AppLocalizedString(
"storeWidgets.infoProvider.today",
value: "Today",
comment: "Range title for the today store info widget"
)

static func periodString(from timeRange: StatsTimeRange) -> String {
switch timeRange {
case .today:
return AppLocalizedString(
"storeWidgets.timeRange.today",
value: "Today",
comment: "Range title for the store info widget"
)
case .thisWeek:
return AppLocalizedString(
"storeWidgets.timeRange.thisWeek",
value: "This Week",
comment: "Range title for the store info widget"
)
case .thisMonth:
return AppLocalizedString(
"storeWidgets.timeRange.thisMonth",
value: "This Month",
comment: "Range title for the store info widget"
)
case .thisYear:
return AppLocalizedString(
"storeWidgets.timeRange.thisYear",
value: "This Year",
comment: "Range title for the store info widget"
)
}
}
}
}
4 changes: 2 additions & 2 deletions WooCommerce/StoreWidgets/StoreInfoWidget.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,12 @@ private extension StoreInfoWidget {
enum Localization {
static let title = AppLocalizedString(
"storeWidgets.displayName",
value: "Today",
value: "Stats",
comment: "Widget title, displayed when selecting which widget to add"
)
static let description = AppLocalizedString(
"storeWidgets.description",
value: "WooCommerce Stats Today",
value: "WooCommerce Stats",
comment: "Widget description, displayed when selecting which widget to add"
)
}
Expand Down