Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Widgets: Data formatting #7708

Merged
merged 10 commits into from
Sep 13, 2022
1 change: 1 addition & 0 deletions WooCommerce/Classes/Extensions/UserDefaults+Woo.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ extension UserDefaults {
case defaultSiteAddress
case defaultStoreID
case defaultStoreName
case defaultStoreCurrencySettings
case defaultAnonymousID
case defaultRoles
case deviceID
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ extension SelectedSiteSettings {
fetchedObjects.forEach {
ServiceLocator.currencySettings.updateCurrencyOptions(with: $0)
}

UserDefaults.group?[.defaultStoreCurrencySettings] = try? JSONEncoder().encode(ServiceLocator.currencySettings)
}
}

Expand Down
4 changes: 2 additions & 2 deletions WooCommerce/StoreWidgets/StoreInfoDataService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ final class StoreInfoDataService {
let (revenueAndOrders, visitors) = try await (revenueAndOrdersRequest, visitorsRequest)

// Assemble stats data
let conversion = visitors.totalVisitors > 0 ? Double(revenueAndOrders.totals.totalOrders) / Double(visitors.totalVisitors) * 100 : 0
let conversion = visitors.totalVisitors > 0 ? Double(revenueAndOrders.totals.totalOrders) / Double(visitors.totalVisitors) : 0
return Stats(revenue: revenueAndOrders.totals.grossRevenue,
totalOrders: revenueAndOrders.totals.totalOrders,
totalVisitors: visitors.totalVisitors,
conversion: conversion)
conversion: min(conversion, 1))
}
}

Expand Down
50 changes: 40 additions & 10 deletions WooCommerce/StoreWidgets/StoreInfoProvider.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import WidgetKit
import WooFoundation
import KeychainAccess

/// Type that represents the all the possible Widget states.
Expand Down Expand Up @@ -53,16 +54,20 @@ final class StoreInfoProvider: TimelineProvider {
///
private var networkService: StoreInfoDataService?

/// Desired data reload interval provided to system = 30 minutes.
///
private let reloadInterval: TimeInterval = 30 * 60

/// Redacted entry with sample data.
///
func placeholder(in context: Context) -> StoreInfoEntry {
let dependencies = Self.fetchDependencies()
return StoreInfoEntry.data(.init(range: Localization.today,
name: dependencies?.storeName ?? Localization.myShop,
revenue: "$132.234",
revenue: Self.formattedAmountString(for: 132.234, with: dependencies?.storeCurrencySettings),
visitors: "67",
orders: "23",
conversion: "34%"))
conversion: Self.formattedConversionString(for: 23/67)))
}

/// Quick Snapshot. Required when previewing the widget.
Expand All @@ -72,7 +77,6 @@ final class StoreInfoProvider: TimelineProvider {
}

/// Real data widget.
/// TODO: Update with real data.
///
func getTimeline(in context: Context, completion: @escaping (Timeline<StoreInfoEntry>) -> Void) {
guard let dependencies = Self.fetchDependencies() else {
Expand All @@ -85,15 +89,14 @@ final class StoreInfoProvider: TimelineProvider {
do {
let todayStats = try await strongService.fetchTodayStats(for: dependencies.storeID)

// TODO: Use proper store formatting.
let entry = StoreInfoEntry.data(.init(range: Localization.today,
name: dependencies.storeName,
revenue: "$\(todayStats.revenue)",
revenue: Self.formattedAmountString(for: todayStats.revenue, with: dependencies.storeCurrencySettings),
visitors: "\(todayStats.totalVisitors)",
orders: "\(todayStats.totalOrders)",
conversion: "\(todayStats.conversion)%"))
conversion: Self.formattedConversionString(for: todayStats.conversion)))

let reloadDate = Date(timeIntervalSinceNow: 30 * 60) // Ask for a 15 minutes reload.
let reloadDate = Date(timeIntervalSinceNow: reloadInterval)
let timeline = Timeline<StoreInfoEntry>(entries: [entry], policy: .after(reloadDate))
completion(timeline)

Expand All @@ -102,7 +105,7 @@ final class StoreInfoProvider: TimelineProvider {
// WooFoundation does not expose `DDLOG` types. Should we include them?
print("⛔️ Error fetching today's widget stats: \(error)")

let reloadDate = Date(timeIntervalSinceNow: 30 * 60) // Ask for a 30 minutes reload.
let reloadDate = Date(timeIntervalSinceNow: reloadInterval)
let timeline = Timeline<StoreInfoEntry>(entries: [.error], policy: .after(reloadDate))
completion(timeline)
}
Expand All @@ -118,6 +121,7 @@ private extension StoreInfoProvider {
let authToken: String
let storeID: Int64
let storeName: String
let storeCurrencySettings: CurrencySettings
}

/// Fetches the required dependencies from the keychain and the shared users default.
Expand All @@ -126,14 +130,40 @@ private extension StoreInfoProvider {
let keychain = Keychain(service: WooConstants.keychainServiceName)
guard let authToken = keychain[WooConstants.authToken],
let storeID = UserDefaults.group?[.defaultStoreID] as? Int64,
let storeName = UserDefaults.group?[.defaultStoreName] as? String else {
let storeName = UserDefaults.group?[.defaultStoreName] as? String,
let storeCurrencySettingsData = UserDefaults.group?[.defaultStoreCurrencySettings] as? Data,
let storeCurrencySettings = try? JSONDecoder().decode(CurrencySettings.self, from: storeCurrencySettingsData) else {
return nil
}
return Dependencies(authToken: authToken, storeID: storeID, storeName: storeName)
return Dependencies(authToken: authToken,
storeID: storeID,
storeName: storeName,
storeCurrencySettings: storeCurrencySettings)
}
}

private extension StoreInfoProvider {

static func formattedAmountString(for amountValue: Decimal, with currencySettings: CurrencySettings?) -> String {
let currencyFormatter = CurrencyFormatter(currencySettings: currencySettings ?? CurrencySettings())
return currencyFormatter.formatAmount(amountValue) ?? Constants.valuePlaceholderText
}

static func formattedConversionString(for conversionRate: Double) -> String {
let numberFormatter = NumberFormatter()
numberFormatter.numberStyle = .percent
numberFormatter.minimumFractionDigits = 1

// do not add 0 fraction digit if the percentage is round
let minimumFractionDigits = floor(conversionRate * 100.0) == conversionRate * 100.0 ? 0 : 1
numberFormatter.minimumFractionDigits = minimumFractionDigits
return numberFormatter.string(from: conversionRate as NSNumber) ?? Constants.valuePlaceholderText
Comment on lines +158 to +160
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is a bit of an edge case here or in the conversion formula as because 100% is showing as 10.000%
IMG_718F09EAB50E-1

In retrospective, I should have added unit tests to these types 🙈

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, very obvious error, converted rate value to percentage twice 😄
Fixed in 72e118c.

}

enum Constants {
static let valuePlaceholderText = "-"
}

enum Localization {
static let myShop = NSLocalizedString("My Shop", comment: "Generic store name for the store info widget preview")
static let today = NSLocalizedString("Today", comment: "Range title for the today store info widget")
Expand Down
2 changes: 1 addition & 1 deletion WooFoundation/WooFoundation/Currency/CurrencyCode.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/// The 3-letter country code for supported currencies
///
public enum CurrencyCode: String, CaseIterable {
public enum CurrencyCode: String, CaseIterable, Codable {
// A
case AED, AFN, ALL, AMD, ANG, AOA, ARS, AUD, AWG, AZN,
// B
Expand Down
36 changes: 34 additions & 2 deletions WooFoundation/WooFoundation/Currency/CurrencySettings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ import Foundation

/// Site-wide settings for displaying prices/money
///
public class CurrencySettings {
public class CurrencySettings: Codable {

// MARK: - Enums

/// Designates where the currency symbol is located on a formatted price
///
public enum CurrencyPosition: String {
public enum CurrencyPosition: String, Codable {
case left = "left"
case right = "right"
case leftSpace = "left_space"
Expand Down Expand Up @@ -394,4 +394,36 @@ public class CurrencySettings {
return "ZK"
}
}

// MARK: - Codable implementation
// Used for serialization in UserDefaults to share settings between app and widgets extension
//
// No custom logic, but it is required because `@Published` property prevents automatic Codable synthesis
// (currencyCode type is Published<CurrencyCode> instead of CurrencyCode)

enum CodingKeys: CodingKey {
case currencyCode
case currencyPosition
case groupingSeparator
case decimalSeparator
case fractionDigits
}

public required init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
currencyCode = try container.decode(CurrencyCode.self, forKey: .currencyCode)
currencyPosition = try container.decode(CurrencyPosition.self, forKey: .currencyPosition)
groupingSeparator = try container.decode(String.self, forKey: .groupingSeparator)
decimalSeparator = try container.decode(String.self, forKey: .decimalSeparator)
fractionDigits = try container.decode(Int.self, forKey: .fractionDigits)
}

public func encode(to encoder: Encoder) throws {
var container = encoder.container(keyedBy: CodingKeys.self)
try container.encode(currencyCode, forKey: .currencyCode)
try container.encode(currencyPosition, forKey: .currencyPosition)
try container.encode(groupingSeparator, forKey: .groupingSeparator)
try container.encode(decimalSeparator, forKey: .decimalSeparator)
try container.encode(fractionDigits, forKey: .fractionDigits)
}
Comment on lines +404 to +428
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we have a custom encoding here, maybe its worth noting that it was added for the widget extension. So future devs will be more informed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's actually not custom, but required because of @Published property preventing auto-synthesized conformance.
Agree about a comment, added in 9abe891

}