Skip to content

Commit 353dc6e

Browse files
authored
[Order Details] Mitigate crash on shipping lines due async operations (#15808)
2 parents a124980 + 9ffe90b commit 353dc6e

File tree

5 files changed

+249
-129
lines changed

5 files changed

+249
-129
lines changed

WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ final class OrderDetailsDataSource: NSObject {
113113
///
114114
var orderHasLocalReceipt: Bool = false
115115

116+
/// Whether the order is eligible for backend receipt generation.
117+
/// This is calculated during sync to avoid async calls during section building.
118+
///
119+
var isEligibleForBackendReceipt: Bool = false
120+
116121
/// Closure to be executed when the cell was tapped.
117122
///
118123
var onCellAction: ((CellActionType, IndexPath?) -> Void)?
@@ -238,9 +243,7 @@ final class OrderDetailsDataSource: NSObject {
238243
///
239244
var orderSubscriptions: [Subscription] = [] {
240245
didSet {
241-
Task { @MainActor in
242-
await reloadSections()
243-
}
246+
reloadSections()
244247
}
245248
}
246249

@@ -869,8 +872,19 @@ private extension OrderDetailsDataSource {
869872

870873
private func configureAggregateOrderItem(cell: ProductDetailsTableViewCell, at indexPath: IndexPath) {
871874
cell.selectionStyle = .default
875+
guard let aggregateItem = aggregateOrderItems[safe: indexPath.row] else {
876+
ServiceLocator.crashLogging.logMessage(
877+
"Invalid aggregate order item index in OrderDetailsDataSource",
878+
properties: [
879+
"row": indexPath.row,
880+
"section": indexPath.section,
881+
"availableAggregateItemsCount": aggregateOrderItems.count
882+
],
883+
level: .error
884+
)
885+
return
886+
}
872887

873-
let aggregateItem = aggregateOrderItems[indexPath.row]
874888
let imageURL: URL? = {
875889
guard let imageURLString = aggregateItem.variationID != 0 ?
876890
lookUpProductVariation(productID: aggregateItem.productID, variationID: aggregateItem.variationID)?.image?.src:
@@ -1046,7 +1060,18 @@ private extension OrderDetailsDataSource {
10461060
}
10471061

10481062
private func configureShippingLine(cell: HostingConfigurationTableViewCell<ShippingLineRowView>, at indexPath: IndexPath) {
1049-
let shippingLine = shippingLines[indexPath.row]
1063+
guard let shippingLine = shippingLines[safe: indexPath.row] else {
1064+
ServiceLocator.crashLogging.logMessage(
1065+
"Invalid shipping line index in OrderDetailsDataSource",
1066+
properties: [
1067+
"row": indexPath.row,
1068+
"section": indexPath.section,
1069+
"availableShippingLinesCount": shippingLines.count
1070+
],
1071+
level: .error
1072+
)
1073+
return
1074+
}
10501075
let viewModel = ShippingLineRowViewModel(shippingLine: shippingLine, currency: order.currency, shippingMethods: siteShippingMethods, editable: false)
10511076
let view = ShippingLineRowView(viewModel: viewModel)
10521077

@@ -1182,8 +1207,7 @@ extension OrderDetailsDataSource {
11821207
/// When: Customer Note == nil >>> Hide Customer Note
11831208
/// When: Shipping == nil >>> Display: Shipping = "No address specified"
11841209
///
1185-
@MainActor
1186-
func reloadSections() async {
1210+
func reloadSections() {
11871211
// Freezes any data that require lookup after the sections are reloaded, in case the data from a ResultsController changes before the next reload.
11881212
refunds = resultsControllers.refunds
11891213
customAmounts = resultsControllers.feeLines
@@ -1202,7 +1226,7 @@ extension OrderDetailsDataSource {
12021226
)
12031227

12041228
var sections = buildStaticSections().compactMap { $0 }
1205-
let paymentSection = await createPaymentSection()
1229+
let paymentSection = createPaymentSection()
12061230

12071231
// Finds the position between shippingLines and customerInformation to inject the payment section,
12081232
// otherwise will always appear last because of being async
@@ -1506,8 +1530,7 @@ extension OrderDetailsDataSource {
15061530
]
15071531
}
15081532

1509-
@MainActor
1510-
private func createPaymentSection() async -> Section {
1533+
private func createPaymentSection() -> Section {
15111534
var rows: [Row] = [.payment, .customerPaid]
15121535
if condensedRefunds.isNotEmpty {
15131536
rows.append(contentsOf: Array(repeating: .refund, count: condensedRefunds.count))
@@ -1518,7 +1541,7 @@ extension OrderDetailsDataSource {
15181541
}
15191542
if orderHasLocalReceipt {
15201543
rows.append(.seeLegacyReceipt)
1521-
} else if await isEligibleForBackendReceipt() {
1544+
} else if isEligibleForBackendReceipt {
15221545
rows.append(.seeReceipt)
15231546
}
15241547
if isEligibleForRefund {
@@ -1531,15 +1554,6 @@ extension OrderDetailsDataSource {
15311554
)
15321555
}
15331556

1534-
@MainActor
1535-
private func isEligibleForBackendReceipt() async -> Bool {
1536-
return await withCheckedContinuation { continuation in
1537-
receiptEligibilityUseCase.isEligibleForReceipt(order.status, onCompletion: { isEligible in
1538-
continuation.resume(returning: isEligible)
1539-
})
1540-
}
1541-
}
1542-
15431557
func refund(at indexPath: IndexPath) -> Refund? {
15441558
let index = indexPath.row - Constants.paymentCell - Constants.paidByCustomerCell
15451559
let condensedRefund = condensedRefunds[index]

WooCommerce/Classes/ViewModels/Order Details/OrderDetailsViewModel.swift

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,7 @@ final class OrderDetailsViewModel {
126126
var orderNotes: [OrderNote] = [] {
127127
didSet {
128128
dataSource.orderNotes = orderNotes
129-
Task { @MainActor in
130-
await dataSource.reloadSections()
131-
}
129+
dataSource.reloadSections()
132130
}
133131
}
134132

@@ -321,6 +319,16 @@ extension OrderDetailsViewModel {
321319
group.leave()
322320
}
323321

322+
// Receipt eligibility need to be synced after the order but before we complete the order sync group,
323+
// otherwise we risk to crash due out of bounds when rendering the rest of the rows that require reloading sections.
324+
group.enter()
325+
Task { @MainActor in
326+
defer {
327+
group.leave()
328+
}
329+
dataSource.isEligibleForBackendReceipt = await isEligibleForBackendReceipt()
330+
}
331+
324332
group.enter()
325333
checkOrderAddOnFeatureSwitchState {
326334
onReloadSections?()
@@ -462,8 +470,8 @@ extension OrderDetailsViewModel {
462470

463471

464472
extension OrderDetailsViewModel {
465-
func reloadSections() async {
466-
await dataSource.reloadSections()
473+
func reloadSections() {
474+
dataSource.reloadSections()
467475
}
468476
}
469477

@@ -1026,6 +1034,15 @@ extension OrderDetailsViewModel {
10261034
status: .pending, onCompletion: { _ in })
10271035
stores.dispatch(action)
10281036
}
1037+
1038+
@MainActor
1039+
private func isEligibleForBackendReceipt() async -> Bool {
1040+
return await withCheckedContinuation { continuation in
1041+
receiptEligibilityUseCase.isEligibleForReceipt(order.status) { isEligible in
1042+
continuation.resume(returning: isEligible)
1043+
}
1044+
}
1045+
}
10291046
}
10301047

10311048
// MARK: - Notices

WooCommerce/Classes/ViewRelated/Orders/Order Details/OrderDetailsViewController.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -265,9 +265,7 @@ private extension OrderDetailsViewController {
265265
private extension OrderDetailsViewController {
266266

267267
func reloadSections() {
268-
Task {
269-
await viewModel.reloadSections()
270-
}
268+
viewModel.reloadSections()
271269
}
272270
}
273271

0 commit comments

Comments
 (0)