Skip to content

Commit ceb70bd

Browse files
joshhealdjoshheald
authored
[Woo POS] Hide cancellation errors when refreshing (#15077)
2 parents b399e13 + b896d25 commit ceb70bd

File tree

4 files changed

+212
-75
lines changed

4 files changed

+212
-75
lines changed

‎WooCommerce/Classes/POS/Controllers/PointOfSaleItemsController.swift‎

Lines changed: 47 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import Foundation
22
import Observation
33
import enum Yosemite.POSItem
44
import protocol Yosemite.PointOfSaleItemServiceProtocol
5+
import enum Yosemite.PointOfSaleItemServiceError
56
import struct Yosemite.POSVariableParentProduct
67
import class Yosemite.Store
78

@@ -162,26 +163,33 @@ private extension PointOfSaleItemsController {
162163
/// - Returns: A boolean that indicates whether there is next page for the paginated items.
163164
@MainActor
164165
func fetchItems(pageNumber: Int, appendToExistingItems: Bool = true) async throws -> Bool {
165-
let pagedItems = try await itemProvider.providePointOfSaleItems(pageNumber: pageNumber)
166-
let newItems = pagedItems.items
167-
var allItems = appendToExistingItems ? itemsViewState.itemsStack.root.items : []
168-
let uniqueNewItems = newItems.filter { newItem in
169-
// Note that this uniquing won't currently work, as POSItem has a UUID.
170-
!allItems.contains(newItem)
171-
}
172-
allItems.append(contentsOf: uniqueNewItems)
173-
if allItems.isEmpty {
174-
itemsViewState.containerState = .empty
175-
itemsViewState.itemsStack = ItemsStackState(root: .loaded([], hasMoreItems: false),
176-
itemStates: [:])
177-
} else {
178-
let itemStates = itemsViewState.itemsStack.itemStates
179-
.filter { allItems.contains($0.key) }
166+
do {
167+
let pagedItems = try await itemProvider.providePointOfSaleItems(pageNumber: pageNumber)
168+
let newItems = pagedItems.items
169+
var allItems = appendToExistingItems ? itemsViewState.itemsStack.root.items : []
170+
let uniqueNewItems = newItems.filter { newItem in
171+
// Note that this uniquing won't currently work, as POSItem has a UUID.
172+
!allItems.contains(newItem)
173+
}
174+
allItems.append(contentsOf: uniqueNewItems)
175+
if allItems.isEmpty {
176+
itemsViewState.containerState = .empty
177+
itemsViewState.itemsStack = ItemsStackState(root: .loaded([], hasMoreItems: false),
178+
itemStates: [:])
179+
} else {
180+
let itemStates = itemsViewState.itemsStack.itemStates
181+
.filter { allItems.contains($0.key) }
182+
itemsViewState.containerState = .content
183+
itemsViewState.itemsStack = ItemsStackState(root: .loaded(allItems, hasMoreItems: pagedItems.hasMorePages),
184+
itemStates: itemStates)
185+
}
186+
return pagedItems.hasMorePages
187+
} catch PointOfSaleItemServiceError.requestCancelled {
180188
itemsViewState.containerState = .content
181-
itemsViewState.itemsStack = ItemsStackState(root: .loaded(allItems, hasMoreItems: pagedItems.hasMorePages),
182-
itemStates: itemStates)
189+
itemsViewState.itemsStack.root = .loaded(itemsViewState.itemsStack.root.items, hasMoreItems: true)
190+
// Assume that we have more pages since we'd made a request, and it was cancelled
191+
return true
183192
}
184-
return pagedItems.hasMorePages
185193
}
186194

187195
/// Fetches variation items given a page number and appends new unique items to the existing items array.
@@ -192,20 +200,28 @@ private extension PointOfSaleItemsController {
192200
parentItem: POSItem,
193201
pageNumber: Int,
194202
appendToExistingItems: Bool = true) async throws -> Bool {
195-
let pagedItems = try await itemProvider.providePointOfSaleVariationItems(
196-
for: parentProduct,
197-
pageNumber: pageNumber
198-
)
199-
let newItems = pagedItems.items
200-
var allItems: [POSItem] = appendToExistingItems ? (itemsViewState.itemsStack.itemStates[parentItem]?.items ?? []) : []
201-
let uniqueNewItems = newItems.filter { newItem in
202-
// Note that this uniquing won't currently work, as POSItem has a UUID.
203-
!allItems.contains(newItem)
204-
}
205-
allItems.append(contentsOf: uniqueNewItems)
203+
do {
204+
let pagedItems = try await itemProvider.providePointOfSaleVariationItems(
205+
for: parentProduct,
206+
pageNumber: pageNumber
207+
)
208+
let newItems = pagedItems.items
209+
var allItems: [POSItem] = appendToExistingItems ? (itemsViewState.itemsStack.itemStates[parentItem]?.items ?? []) : []
210+
let uniqueNewItems = newItems.filter { newItem in
211+
// Note that this uniquing won't currently work, as POSItem has a UUID.
212+
!allItems.contains(newItem)
213+
}
214+
allItems.append(contentsOf: uniqueNewItems)
206215

207-
updateState(for: parentItem, to: .loaded(allItems, hasMoreItems: pagedItems.hasMorePages))
208-
return pagedItems.hasMorePages
216+
updateState(for: parentItem, to: .loaded(allItems, hasMoreItems: pagedItems.hasMorePages))
217+
return pagedItems.hasMorePages
218+
} catch PointOfSaleItemServiceError.requestCancelled {
219+
itemsViewState.containerState = .content
220+
updateState(for: parentItem, to: .loaded(itemsViewState.itemsStack.itemStates[parentItem]?.items ?? [],
221+
hasMoreItems: true))
222+
// Assume that we have more pages since we'd made a request, and it was cancelled
223+
return true
224+
}
209225
}
210226
}
211227

‎WooCommerce/WooCommerceTests/POS/Controllers/PointOfSaleItemsControllerTests.swift‎

Lines changed: 119 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import Foundation
33
@testable import WooCommerce
44
import struct Yosemite.POSVariableParentProduct
55
import enum Yosemite.POSItem
6+
import enum Yosemite.PointOfSaleItemServiceError
67
import Observation
78

89
final class PointOfSaleItemsControllerTests {
@@ -238,7 +239,7 @@ final class PointOfSaleItemsControllerTests {
238239
await sut.loadItems(base: .root)
239240
await sut.loadItems(base: baseItem)
240241

241-
itemProvider.shouldThrowError = true
242+
itemProvider.errorToThrow = MockError.requestFailed
242243

243244
// When
244245
await sut.loadNextItems(base: baseItem)
@@ -275,7 +276,7 @@ final class PointOfSaleItemsControllerTests {
275276
let itemProvider = MockPointOfSaleItemService()
276277
let sut = PointOfSaleItemsController(itemProvider: itemProvider)
277278

278-
itemProvider.shouldThrowError = true
279+
itemProvider.errorToThrow = MockError.requestFailed
279280
let expectedError = PointOfSaleErrorState(title: "Error loading products",
280281
subtitle: "Give it another go?",
281282
buttonText: "Retry")
@@ -299,7 +300,7 @@ final class PointOfSaleItemsControllerTests {
299300
itemProvider.shouldSimulateTwoPages = true
300301
await sut.loadItems(base: .root)
301302

302-
itemProvider.shouldThrowError = true
303+
itemProvider.errorToThrow = MockError.requestFailed
303304

304305
// When
305306
await sut.loadNextItems(base: .root)
@@ -315,6 +316,57 @@ final class PointOfSaleItemsControllerTests {
315316
#expect(errorState == PointOfSaleErrorState.errorOnLoadingProductsNextPage())
316317
}
317318

319+
@available(iOS 17.0, *)
320+
@Test func loadItems_when_request_is_cancelled_then_state_is_loaded() async throws {
321+
// Given
322+
let itemProvider = MockPointOfSaleItemService()
323+
let sut = PointOfSaleItemsController(itemProvider: itemProvider)
324+
325+
itemProvider.errorToThrow = PointOfSaleItemServiceError.requestCancelled
326+
try #require(sut.itemsViewState.containerState == .loading)
327+
328+
// When
329+
await sut.loadItems(base: .root)
330+
331+
// Then
332+
guard case .loaded(let items, let hasMoreItems) = sut.itemsViewState.itemsStack.root else {
333+
Issue.record("Expected loaded ItemList state, but got \(sut.itemsViewState.itemsStack.root)")
334+
return
335+
}
336+
#expect(items.count == 0)
337+
#expect(hasMoreItems)
338+
}
339+
340+
@available(iOS 17.0, *)
341+
@Test func loadNextItems_when_request_is_cancelled_then_state_is_loaded() async throws {
342+
// Given
343+
let itemProvider = MockPointOfSaleItemService()
344+
let sut = PointOfSaleItemsController(itemProvider: itemProvider)
345+
346+
itemProvider.shouldSimulateTwoPages = true
347+
await sut.loadItems(base: .root)
348+
349+
guard case .loaded = sut.itemsViewState.itemsStack.root else {
350+
Issue.record("Expected loaded ItemList state, but got \(sut.itemsViewState.itemsStack.root)")
351+
return
352+
}
353+
354+
itemProvider.errorToThrow = PointOfSaleItemServiceError.requestCancelled
355+
356+
// When
357+
await sut.loadNextItems(base: .root)
358+
359+
// Then
360+
#expect(sut.itemsViewState.containerState == .content)
361+
362+
guard case .loaded(let items, let hasMoreItems) = sut.itemsViewState.itemsStack.root else {
363+
Issue.record("Expected loaded ItemList state, but got \(sut.itemsViewState.itemsStack.root)")
364+
return
365+
}
366+
#expect(items.count == 2)
367+
#expect(hasMoreItems)
368+
}
369+
318370
@available(iOS 17.0, *)
319371
@Test func loadNextItems_after_itemProvider_throws_error_then_the_same_page_is_requested_next() async throws {
320372
// Given
@@ -324,7 +376,7 @@ final class PointOfSaleItemsControllerTests {
324376
itemProvider.shouldSimulateTwoPages = true
325377
await sut.loadItems(base: .root)
326378

327-
itemProvider.shouldThrowError = true
379+
itemProvider.errorToThrow = MockError.requestFailed
328380
await sut.loadNextItems(base: .root)
329381
try #require(itemProvider.spyLastRequestedPageNumber == 2)
330382
itemProvider.spyLastRequestedPageNumber = 0
@@ -505,4 +557,67 @@ final class PointOfSaleItemsControllerTests {
505557
await sut.loadItems(base: baseItem)
506558
}
507559
}
560+
561+
@available(iOS 17.0, *)
562+
@Test func loadChildItems_when_request_is_cancelled_then_state_is_loaded() async throws {
563+
// Given
564+
let itemProvider = MockPointOfSaleItemService()
565+
let sut = PointOfSaleItemsController(itemProvider: itemProvider)
566+
567+
let parentItem = POSItem.variableParentProduct(POSVariableParentProduct(id: UUID(),
568+
name: "Parent product",
569+
productImageSource: nil,
570+
productID: 125))
571+
let baseItem = ItemListBaseItem.parent(parentItem)
572+
573+
itemProvider.errorToThrow = PointOfSaleItemServiceError.requestCancelled
574+
try #require(sut.itemsViewState.containerState == .loading)
575+
576+
// When
577+
await sut.loadItems(base: baseItem)
578+
579+
// Then
580+
guard case .loaded(let items, let hasMoreItems) = sut.itemsViewState.itemsStack.itemStates[parentItem] else {
581+
Issue.record("Expected loaded ItemList state, but got \(String(describing: sut.itemsViewState.itemsStack.itemStates[parentItem]))")
582+
return
583+
}
584+
#expect(items.count == 0)
585+
#expect(hasMoreItems)
586+
}
587+
588+
@available(iOS 17.0, *)
589+
@Test func loadNextChildItems_when_request_is_cancelled_then_state_is_loaded() async throws {
590+
// Given
591+
let itemProvider = MockPointOfSaleItemService()
592+
let sut = PointOfSaleItemsController(itemProvider: itemProvider)
593+
594+
let parentItem = POSItem.variableParentProduct(POSVariableParentProduct(id: UUID(),
595+
name: "Parent product",
596+
productImageSource: nil,
597+
productID: 125))
598+
let baseItem = ItemListBaseItem.parent(parentItem)
599+
600+
itemProvider.shouldSimulateTwoPagesOfVariations = true
601+
await sut.loadItems(base: baseItem)
602+
603+
itemProvider.errorToThrow = PointOfSaleItemServiceError.requestCancelled
604+
605+
// When
606+
await sut.loadNextItems(base: baseItem)
607+
608+
// Then
609+
#expect(sut.itemsViewState.containerState == .content)
610+
611+
guard case .loaded(let items, let hasMoreItems) = sut.itemsViewState.itemsStack.itemStates[parentItem] else {
612+
Issue.record("Expected loaded ItemList state, but got \(String(describing: sut.itemsViewState.itemsStack.itemStates[parentItem]))")
613+
return
614+
}
615+
#expect(items.count == 2)
616+
#expect(hasMoreItems)
617+
}
618+
619+
620+
enum MockError: Error {
621+
case requestFailed
622+
}
508623
}

‎WooCommerce/WooCommerceTests/POS/Mocks/MockPOSItemProvider.swift‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,16 @@ import struct Yosemite.POSVariableParentProduct
1010
final class MockPointOfSaleItemService: PointOfSaleItemServiceProtocol {
1111
/// An array of pages of items, returned when other flags are not set.
1212
var itemPages: [[POSItem]] = []
13-
var shouldThrowError = false
13+
var errorToThrow: Error?
1414
var shouldReturnZeroItems = false
1515
var shouldSimulateTwoPages = false
1616
var shouldSimulateMorePages = false
1717

1818
var spyLastRequestedPageNumber: Int?
1919
func providePointOfSaleItems(pageNumber: Int) async throws -> PagedItems<POSItem> {
2020
spyLastRequestedPageNumber = pageNumber
21-
if shouldThrowError {
22-
throw MockError.requestFailed
21+
if let errorToThrow {
22+
throw errorToThrow
2323
}
2424
if shouldReturnZeroItems {
2525
return .init(items: [], hasMorePages: false)
@@ -35,8 +35,8 @@ final class MockPointOfSaleItemService: PointOfSaleItemServiceProtocol {
3535
var shouldSimulateTwoPagesOfVariations = false
3636
var shouldSimulateMorePagesOfVariations = false
3737
func providePointOfSaleVariationItems(for parentProduct: POSVariableParentProduct, pageNumber: Int) async throws -> PagedItems<POSItem> {
38-
if shouldThrowError {
39-
throw MockError.requestFailed
38+
if let errorToThrow {
39+
throw errorToThrow
4040
}
4141
if shouldSimulateTwoPagesOfVariations,
4242
pageNumber > 1 {
@@ -127,8 +127,4 @@ extension MockPointOfSaleItemService {
127127
parentProductName: "Ice cream")
128128
return [.variation(variation3), .variation(variation4)]
129129
}
130-
131-
enum MockError: Error {
132-
case requestFailed
133-
}
134130
}

0 commit comments

Comments
 (0)