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

Improve data fetching in Order Details, to avoid I/O performance on the main thread #14999

Open
wants to merge 9 commits into
base: trunk
Choose a base branch
from
3 changes: 2 additions & 1 deletion Fakes/Fakes/Networking.generated.swift
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,8 @@ extension Networking.Order {
customFields: .fake(),
renewalSubscriptionID: .fake(),
appliedGiftCards: .fake(),
attributionInfo: .fake()
attributionInfo: .fake(),
shippingLabels: .fake()
)
}
}
Expand Down
10 changes: 10 additions & 0 deletions Networking/Networking.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,11 @@
453954D22C90D84200A3E64A /* meta-data-products-and-orders.json in Resources */ = {isa = PBXBuildFile; fileRef = 453954D12C90D84200A3E64A /* meta-data-products-and-orders.json */; };
453954D42C90E81300A3E64A /* MetaDataRemoteTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 453954D32C90E81300A3E64A /* MetaDataRemoteTests.swift */; };
453954D62C9193BC00A3E64A /* meta-data-products-orders-update.json in Resources */ = {isa = PBXBuildFile; fileRef = 453954D52C9193BC00A3E64A /* meta-data-products-orders-update.json */; };
453B33FD2D47F1F4005C05B0 /* ShippingLabel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02C254A7256373AB00A04423 /* ShippingLabel.swift */; };
453B33FE2D47F1FB005C05B0 /* ShippingLabelStatus.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02C254AB2563781800A04423 /* ShippingLabelStatus.swift */; };
453B33FF2D47F203005C05B0 /* ShippingLabelRefund.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02C2549F25636F6900A04423 /* ShippingLabelRefund.swift */; };
453B34002D47F20A005C05B0 /* ShippingLabelRefundStatus.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02C254AF256378D000A04423 /* ShippingLabelRefundStatus.swift */; };
453B34012D47F22C005C05B0 /* ShippingLabelAddress.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02C2549925636E1500A04423 /* ShippingLabelAddress.swift */; };
45551F122523E7F1007EF104 /* UserAgent.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45551F112523E7F1007EF104 /* UserAgent.swift */; };
45551F142523E7FF007EF104 /* UserAgentTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45551F132523E7FF007EF104 /* UserAgentTests.swift */; };
4568E2222459ADC60007E478 /* SitePostsRemote.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4568E2212459ADC60007E478 /* SitePostsRemote.swift */; };
Expand Down Expand Up @@ -4892,11 +4897,13 @@
2680B0D22BED5A4C00E7F1D8 /* AuthenticatedRESTRequest.swift in Sources */,
2680B0D02BED5A1900E7F1D8 /* WooConstants.swift in Sources */,
2680B0CF2BED59CE00E7F1D8 /* ApplicationPasswordStorage.swift in Sources */,
453B34002D47F20A005C05B0 /* ShippingLabelRefundStatus.swift in Sources */,
2680B0CB2BED58B200E7F1D8 /* ApplicationPasswordNameAndUUID.swift in Sources */,
26373E0A2BFCEAAF008E6735 /* OrderItemTax.swift in Sources */,
2680B0CC2BED58B200E7F1D8 /* ApplicationPassword.swift in Sources */,
26373E122BFCEDB7008E6735 /* OrderFeeLine.swift in Sources */,
26F2CADD2BED579100F9A5E7 /* ApplicationPasswordUseCase.swift in Sources */,
453B33FF2D47F203005C05B0 /* ShippingLabelRefund.swift in Sources */,
26F2CADA2BED578200F9A5E7 /* RequestProcessor.swift in Sources */,
26F2CADB2BED578200F9A5E7 /* RequestAuthenticator.swift in Sources */,
26F2CADC2BED578200F9A5E7 /* RequestConverter.swift in Sources */,
Expand All @@ -4909,6 +4916,7 @@
263A6DBC2BEAA49B00C292B2 /* AnyDecodable.swift in Sources */,
263A6DBD2BEAA49B00C292B2 /* AnyEncodable.swift in Sources */,
263A6DB92BEAA46F00C292B2 /* SiteSummaryStatsMapper.swift in Sources */,
453B33FD2D47F1F4005C05B0 /* ShippingLabel.swift in Sources */,
26373E112BFCEDAE008E6735 /* OrderRefundCondensed.swift in Sources */,
263A6DBA2BEAA46F00C292B2 /* SiteSummaryStats.swift in Sources */,
263A6DB62BEAA46000C292B2 /* SiteVisitStatsMapper.swift in Sources */,
Expand Down Expand Up @@ -4967,6 +4975,7 @@
26CFDEE12C0295EA005ABC31 /* MetaContainer.swift in Sources */,
26373E152BFCEDE9008E6735 /* OrderGiftCard.swift in Sources */,
269014CB2BEA9253006056E0 /* JetpackRequest.swift in Sources */,
453B34012D47F22C005C05B0 /* ShippingLabelAddress.swift in Sources */,
269014CA2BEA924B006056E0 /* NetworkError.swift in Sources */,
26373E092BFCEA90008E6735 /* OrderItemProductAddOn.swift in Sources */,
26373E162BFCEDFD008E6735 /* OrderAttributionInfo.swift in Sources */,
Expand All @@ -4979,6 +4988,7 @@
269014C32BEA9134006056E0 /* Mapper.swift in Sources */,
269014C22BEA912D006056E0 /* Request.swift in Sources */,
26373E182BFCEF79008E6735 /* KeyedDecodingContainer+Woo.swift in Sources */,
453B33FE2D47F1FB005C05B0 /* ShippingLabelStatus.swift in Sources */,
26373E202BFCF6A7008E6735 /* EntityDateModifiedMapper.swift in Sources */,
26CFDEE02C0295E1005ABC31 /* NoteRange.swift in Sources */,
26CFDEDE2C0295CB005ABC31 /* NoteBlock.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1348,7 +1348,8 @@ extension Networking.Order {
customFields: CopiableProp<[MetaData]> = .copy,
renewalSubscriptionID: NullableCopiableProp<String> = .copy,
appliedGiftCards: CopiableProp<[OrderGiftCard]> = .copy,
attributionInfo: NullableCopiableProp<OrderAttributionInfo> = .copy
attributionInfo: NullableCopiableProp<OrderAttributionInfo> = .copy,
shippingLabels: CopiableProp<[ShippingLabel]> = .copy
) -> Networking.Order {
let siteID = siteID ?? self.siteID
let orderID = orderID ?? self.orderID
Expand Down Expand Up @@ -1388,6 +1389,7 @@ extension Networking.Order {
let renewalSubscriptionID = renewalSubscriptionID ?? self.renewalSubscriptionID
let appliedGiftCards = appliedGiftCards ?? self.appliedGiftCards
let attributionInfo = attributionInfo ?? self.attributionInfo
let shippingLabels = shippingLabels ?? self.shippingLabels

return Networking.Order(
siteID: siteID,
Expand Down Expand Up @@ -1427,7 +1429,8 @@ extension Networking.Order {
customFields: customFields,
renewalSubscriptionID: renewalSubscriptionID,
appliedGiftCards: appliedGiftCards,
attributionInfo: attributionInfo
attributionInfo: attributionInfo,
shippingLabels: shippingLabels
)
}
}
Expand Down
23 changes: 19 additions & 4 deletions Networking/Networking/Model/Order.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ public struct Order: Decodable, Sendable, GeneratedCopiable, GeneratedFakeable {
///
public let attributionInfo: OrderAttributionInfo?

/// Shipping labels associated with the order
///
public let shippingLabels: [ShippingLabel]

/// Order struct initializer.
///
public init(siteID: Int64,
Expand Down Expand Up @@ -100,7 +104,8 @@ public struct Order: Decodable, Sendable, GeneratedCopiable, GeneratedFakeable {
customFields: [MetaData],
renewalSubscriptionID: String?,
appliedGiftCards: [OrderGiftCard],
attributionInfo: OrderAttributionInfo?) {
attributionInfo: OrderAttributionInfo?,
shippingLabels: [ShippingLabel]) {

self.siteID = siteID
self.orderID = orderID
Expand Down Expand Up @@ -145,6 +150,7 @@ public struct Order: Decodable, Sendable, GeneratedCopiable, GeneratedFakeable {
self.renewalSubscriptionID = renewalSubscriptionID
self.appliedGiftCards = appliedGiftCards
self.attributionInfo = attributionInfo
self.shippingLabels = shippingLabels
}


Expand Down Expand Up @@ -239,6 +245,11 @@ public struct Order: Decodable, Sendable, GeneratedCopiable, GeneratedFakeable {
return OrderAttributionInfo(metaData: allOrderMetaData)
}()

// Shipping labels
/// This will be an empty array by default because it's not directly parsed from the Order details, so it won't be decoded.
/// It's fetched with a specific API request, while at the same time it has a relationship in Core Data with Order.
let shippingLabels: [ShippingLabel] = []

self.init(siteID: siteID,
orderID: orderID,
parentID: parentID,
Expand Down Expand Up @@ -276,7 +287,8 @@ public struct Order: Decodable, Sendable, GeneratedCopiable, GeneratedFakeable {
customFields: customFields,
renewalSubscriptionID: renewalSubscriptionID,
appliedGiftCards: appliedGiftCards,
attributionInfo: attributionInfo)
attributionInfo: attributionInfo,
shippingLabels: shippingLabels)
}

public static var empty: Order {
Expand Down Expand Up @@ -317,7 +329,8 @@ public struct Order: Decodable, Sendable, GeneratedCopiable, GeneratedFakeable {
customFields: [],
renewalSubscriptionID: nil,
appliedGiftCards: [],
attributionInfo: nil)
attributionInfo: nil,
shippingLabels: [])
}
}

Expand Down Expand Up @@ -365,6 +378,7 @@ internal extension Order {
case taxLines = "tax_lines"
case metadata = "meta_data"
case giftCards = "gift_cards"
case shippingLabels = "shipping_labels"
}
}

Expand Down Expand Up @@ -407,7 +421,8 @@ extension Order: Equatable {
lhs.items.count == rhs.items.count &&
lhs.items.sorted() == rhs.items.sorted() &&
lhs.customerNote == rhs.customerNote &&
lhs.attributionInfo == rhs.attributionInfo
lhs.attributionInfo == rhs.attributionInfo &&
lhs.shippingLabels == rhs.shippingLabels
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import Foundation
import Codegen

/// The status of shipping label refund.
public enum ShippingLabelRefundStatus: GeneratedFakeable {
public enum ShippingLabelRefundStatus: Sendable, GeneratedFakeable {
case pending
case unknown
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import Foundation
import Codegen

/// The status of shipping label.
public enum ShippingLabelStatus: GeneratedFakeable {
public enum ShippingLabelStatus: Sendable, GeneratedFakeable {
case purchased
case purchaseError
case purchaseInProgress
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import Codegen

/// Represents a Shipping Label.
///
public struct ShippingLabel: Equatable, GeneratedCopiable, GeneratedFakeable {
public struct ShippingLabel: Equatable, Sendable, GeneratedCopiable, GeneratedFakeable {
/// The remote ID of the site that owns this shipping label.
public let siteID: Int64

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import Codegen

/// Represents a Shipping Label Address.
///
public struct ShippingLabelAddress: GeneratedCopiable, Equatable, GeneratedFakeable {
public struct ShippingLabelAddress: GeneratedCopiable, Equatable, Sendable, GeneratedFakeable {
/// The name of the company at the address.
public let company: String

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import Codegen

/// Represents a Shipping Label Refund.
///
public struct ShippingLabelRefund: Equatable, GeneratedFakeable {
public struct ShippingLabelRefund: Equatable, Sendable, GeneratedFakeable {
/// The date of refund request.
public let dateRequested: Date

Expand Down
2 changes: 2 additions & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@
- [*] Product Details: Display cover tag on the first product image [https://github.com/woocommerce/woocommerce-ios/pull/15041]
- [*] Payments: Update learn more links to open Stripe-specific docs when using that gateway [https://github.com/woocommerce/woocommerce-ios/pull/15035]
- [*] Now, usernames and emails in text fields across multiple login views are no longer capitalized. [https://github.com/woocommerce/woocommerce-ios/pull/15002]
- [internal] Improve data fetching in Order Details, to avoid I/O performance on the main thread. [https://github.com/woocommerce/woocommerce-ios/pull/14999]

21.6
-----
- [*] Payments: improve retry handling when card reader updates fail due to low battery [https://github.com/woocommerce/woocommerce-ios/pull/14990]
- [*] Payments: Improved error handling during card payments and refunds [https://github.com/woocommerce/woocommerce-ios/pull/14980]
- [***] Updated the app look and feel to match the new WooCommerce branding [https://github.com/woocommerce/woocommerce-ios/pull/14825]


21.5
-----
- [*] Blaze: if the AI is unable to provide suggestions in the creation form, the form will now automatically use product information to ensure all fields of the preview are populated. [https://github.com/woocommerce/woocommerce-ios/pull/14868]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,6 @@ final class OrderDetailsResultsControllers {
///
private lazy var productVariationResultsController: ResultsController<StorageProductVariation> = getProductVariationResultsController()

/// Fee lines Results Controller.
///
private lazy var feeLinesResultsController: ResultsController<StorageOrderFeeLine> = {
let predicate = NSPredicate(format: "order.orderID == %ld", order.orderID)
let descriptor = NSSortDescriptor(key: "feeID", ascending: true)

return ResultsController<StorageOrderFeeLine>(storageManager: storageManager, matching: predicate, sortedBy: [descriptor])
}()

/// Status Results Controller.
///
private lazy var statusResultsController: ResultsController<StorageOrderStatus> = {
Expand All @@ -63,17 +54,6 @@ final class OrderDetailsResultsControllers {
return ResultsController<StorageRefund>(storageManager: storageManager, matching: predicate, sortedBy: [descriptor])
}()

/// ShippingLabel Results Controller.
///
private lazy var shippingLabelResultsController: ResultsController<StorageShippingLabel> = {
let predicate = NSPredicate(format: "siteID = %ld AND orderID = %ld", order.siteID, order.orderID)
let dateCreatedDescriptor = NSSortDescriptor(keyPath: \StorageShippingLabel.dateCreated, ascending: false)
let shippingLabelIDDescriptor = NSSortDescriptor(keyPath: \StorageShippingLabel.shippingLabelID, ascending: false)
return ResultsController<StorageShippingLabel>(storageManager: storageManager,
matching: predicate,
sortedBy: [dateCreatedDescriptor, shippingLabelIDDescriptor])
}()

/// AddOnGroup ResultsController.
///
private lazy var addOnGroupResultsController: ResultsController<StorageAddOnGroup> = {
Expand Down Expand Up @@ -128,7 +108,7 @@ final class OrderDetailsResultsControllers {
/// Shipping labels for an Order
///
var shippingLabels: [ShippingLabel] {
return shippingLabelResultsController.fetchedObjects
return order.shippingLabels
}

/// Site's add-on groups.
Expand All @@ -142,7 +122,7 @@ final class OrderDetailsResultsControllers {
}

var feeLines: [OrderFeeLine] {
return feeLinesResultsController.fetchedObjects
return order.fees
}

/// Shipping methods list
Expand All @@ -169,10 +149,8 @@ final class OrderDetailsResultsControllers {
configureProductResultsController(onReload: onReload)
configureProductVariationResultsController(onReload: onReload)
configureRefundResultsController(onReload: onReload)
configureShippingLabelResultsController(onReload: onReload)
configureAddOnGroupResultsController(onReload: onReload)
configureSitePluginsResultsController(onReload: onReload)
configureFeeLinesResultsController(onReload: onReload)
configureShippingMethodsResultsController(onReload: onReload)
}

Expand Down Expand Up @@ -286,24 +264,6 @@ private extension OrderDetailsResultsControllers {
}
}

private func configureShippingLabelResultsController(onReload: @escaping () -> Void) {
shippingLabelResultsController.onDidChangeContent = {
onReload()
}

shippingLabelResultsController.onDidResetContent = { [weak self] in
guard let self = self else { return }
self.refetchAllResultsControllers()
onReload()
}

do {
try shippingLabelResultsController.performFetch()
} catch {
DDLogError("⛔️ Unable to fetch ShippingLabels for Site \(siteID) and Order \(order.orderID): \(error)")
}
}

private func configureAddOnGroupResultsController(onReload: @escaping () -> Void) {
addOnGroupResultsController.onDidChangeContent = {
onReload()
Expand Down Expand Up @@ -340,24 +300,6 @@ private extension OrderDetailsResultsControllers {
}
}

private func configureFeeLinesResultsController(onReload: @escaping () -> Void) {
feeLinesResultsController.onDidChangeContent = {
onReload()
}

feeLinesResultsController.onDidResetContent = { [weak self] in
guard let self = self else { return }
self.refetchAllResultsControllers()
onReload()
}

do {
try feeLinesResultsController.performFetch()
} catch {
DDLogError("⛔️ Unable to fetch Order Fee lines for Site \(siteID): \(error)")
}
}

private func configureShippingMethodsResultsController(onReload: @escaping () -> Void) {
shippingMethodsResultsController.onDidChangeContent = {
onReload()
Expand All @@ -384,7 +326,6 @@ private extension OrderDetailsResultsControllers {
try? refundResultsController.performFetch()
try? trackingResultsController.performFetch()
try? statusResultsController.performFetch()
try? shippingLabelResultsController.performFetch()
try? addOnGroupResultsController.performFetch()
try? sitePluginsResultsController.performFetch()
try? shippingMethodsResultsController.performFetch()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,8 @@ struct EditAddressForm_Previews: PreviewProvider {
customFields: [],
renewalSubscriptionID: nil,
appliedGiftCards: [],
attributionInfo: nil)
attributionInfo: nil,
shippingLabels: [])

static let sampleAddress = Address(firstName: "Johnny",
lastName: "Appleseed",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,8 @@ extension ShippingLabelPackagesFormViewModel {
customFields: [],
renewalSubscriptionID: nil,
appliedGiftCards: [],
attributionInfo: nil)
attributionInfo: nil,
shippingLabels: [])
}

static func sampleAddress() -> Address {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ enum ShippingLabelSampleData {
customFields: [],
renewalSubscriptionID: nil,
appliedGiftCards: [],
attributionInfo: nil)
attributionInfo: nil,
shippingLabels: [])
}

static func samplePackageDetails() -> ShippingLabelPackagesResponse {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ extension Order {
customFields: [],
renewalSubscriptionID: nil,
appliedGiftCards: [],
attributionInfo: nil)
attributionInfo: nil,
shippingLabels: [])
}
#endif
Loading