Update test cases in PointOfSaleItemsControllerTests based on the e…

…xpected behavior and the move of pagination tracking to PaginationTracker with fixes for issues found in the tests.
woocommerce · Dec 19, 2024 · 0242c2f · 0242c2f
1 parent f75a9bf
commit 0242c2f
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 50 deletions.
diff --git a/WooCommerce/Classes/POS/Controllers/PointOfSaleItemsController.swift b/WooCommerce/Classes/POS/Controllers/PointOfSaleItemsController.swift
@@ -54,7 +54,7 @@ class PointOfSaleItemsController: PointOfSaleItemsControllerProtocol {
             }
         } catch {
             // TODO: 14694 - Handle error from loading the next page, like showing an error UI at the end or as an overlay.
-            updateItemListStateAfterLoadAttempt()
+            itemListStateSubject.send(.error(PointOfSaleErrorState.errorOnLoadingProducts()))
         }
     }
 
@@ -70,6 +70,7 @@ class PointOfSaleItemsController: PointOfSaleItemsControllerProtocol {
             updateItemListStateAfterLoadAttempt()
         } catch {
             // TODO: 14694 - Handle error from pull-to-refresh, like showing an error UI at the beginning or as an overlay.
+            itemListStateSubject.send(.error(PointOfSaleErrorState.errorOnLoadingProducts()))
         }
     }
 

diff --git a/WooCommerce/Classes/Tools/InfiniteScroll/AsyncPaginationTracker.swift b/WooCommerce/Classes/Tools/InfiniteScroll/AsyncPaginationTracker.swift
@@ -57,8 +57,12 @@ final class AsyncPaginationTracker {
         guard !isPageBeingSynced(pageNumber: nextPage) else {
             return .syncing
         }
-        try await sync(pageNumber: nextPage, syncFunction: syncFunction)
-        return .synced
+        do {
+            try await sync(pageNumber: nextPage, syncFunction: syncFunction)
+            return .synced
+        } catch {
+            throw error
+        }
     }
 
     /// Resets internal states and resyncs the first page of results.
@@ -83,14 +87,15 @@ private extension AsyncPaginationTracker {
         markAsBeingSynced(pageNumber: pageNumber)
 
         do {
+            defer {
+                unmarkAsBeingSynced(pageNumber: pageNumber)
+            }
             let hasNextPage = try await syncFunction(pageNumber)
             self.hasNextPage = hasNextPage
             markAsSynced(pageNumber: pageNumber)
         } catch {
             throw error
         }
-
-        unmarkAsBeingSynced(pageNumber: pageNumber)
     }
 }
 

diff --git a/WooCommerce/WooCommerceTests/POS/Controllers/PointOfSaleItemsControllerTests.swift b/WooCommerce/WooCommerceTests/POS/Controllers/PointOfSaleItemsControllerTests.swift
@@ -125,6 +125,7 @@ final class PointOfSaleItemsControllerTests {
         let initialItems = MockPointOfSaleItemService.makeInitialItems()
         itemProvider.items = initialItems
         itemProvider.shouldSimulateTwoPages = true
+        await sut.loadInitialItems()
 
         // When
         await sut.loadNextItems()
@@ -140,6 +141,8 @@ final class PointOfSaleItemsControllerTests {
     @Test func loadNextItems_requests_second_page() async throws {
         // Given
         try #require(itemListState == .initialLoading)
+        itemProvider.shouldSimulateTwoPages = true
+        await sut.loadInitialItems()
 
         // When
         await sut.loadNextItems()
@@ -178,11 +181,15 @@ final class PointOfSaleItemsControllerTests {
 
     @Test func loadNextItems_when_itemProvider_throws_error_then_state_is_error() async throws {
         // Given
+        try #require(itemListState == .initialLoading)
+
+        itemProvider.shouldSimulateTwoPages = true
+        await sut.loadInitialItems()
+
         itemProvider.shouldThrowError = true
         let expectedError = PointOfSaleErrorState(title: "Error loading products",
                                                   subtitle: "Give it another go?",
                                                   buttonText: "Retry")
-        try #require(itemListState == .initialLoading)
 
         // When
         await sut.loadNextItems()
@@ -193,6 +200,9 @@ final class PointOfSaleItemsControllerTests {
 
     @Test func loadNextItems_after_itemProvider_throws_error_then_the_same_page_is_requested_next() async throws {
         // Given
+        itemProvider.shouldSimulateTwoPages = true
+        await sut.loadInitialItems()
+
         itemProvider.shouldThrowError = true
         await sut.loadNextItems()
         try #require(itemProvider.spyLastRequestedPageNumber == 2)
@@ -219,6 +229,9 @@ final class PointOfSaleItemsControllerTests {
 
     @Test func reload_requests_first_page() async throws {
         // Given
+        itemProvider.shouldSimulateTwoPages = true
+        await sut.loadInitialItems()
+
         await sut.loadNextItems()
         try #require(itemProvider.spyLastRequestedPageNumber == 2)
 
@@ -229,33 +242,26 @@ final class PointOfSaleItemsControllerTests {
         #expect(itemProvider.spyLastRequestedPageNumber == 1)
     }
 
-    @Test func loadNextItems_when_next_page_is_out_of_range_then_receives_error() async throws {
+    @Test func loadNextItems_when_next_page_is_empty_then_state_is_loaded() async throws {
         // Given
         await sut.loadInitialItems()
         try #require(itemProvider.spyLastRequestedPageNumber == 1)
-        let expectedError = PointOfSaleErrorState(title: "Error loading products",
-                                                  subtitle: "Give it another go?",
-                                                  buttonText: "Retry")
 
         // When
-        itemProvider.simulateNextPageIsOutOfRange()
+        itemProvider.shouldReturnZeroItems = true
         await sut.loadNextItems()
 
         // Then
-        guard case .error = itemListState else {
-            Issue.record("Expected error state, but got \(itemListState)")
-            return
-        }
-        #expect(itemListState == .error(expectedError))
+        #expect(itemListState == .loaded(MockPointOfSaleItemService.makeInitialItems()))
     }
 
-    @Test func loadNextItems_when_next_page_is_out_of_range_then_the_same_page_is_requested_next() async throws {
+    @Test func loadNextItems_when_next_page_is_empty_then_the_same_page_is_requested_next() async throws {
         // Given
         await sut.loadInitialItems()
         try #require(itemProvider.spyLastRequestedPageNumber == 1)
 
         // When
-        itemProvider.simulateNextPageIsOutOfRange()
+        itemProvider.shouldReturnZeroItems = true
         await sut.loadNextItems()
 
         // Then

diff --git a/WooCommerce/WooCommerceTests/POS/Mocks/MockPOSItemProvider.swift b/WooCommerce/WooCommerceTests/POS/Mocks/MockPOSItemProvider.swift
@@ -9,13 +9,9 @@ final class MockPointOfSaleItemService: PointOfSaleItemServiceProtocol {
     var shouldThrowError = false
     var shouldReturnZeroItems = false
     var shouldSimulateTwoPages = false
-    private var isPageOutOfRange = false
 
     var spyLastRequestedPageNumber: Int?
     func providePointOfSaleItems(pageNumber: Int) async throws -> (items: [POSItem], hasNextPage: Bool) {
-        if isPageOutOfRange {
-            throw MockError.pageOutOfRange
-        }
         spyLastRequestedPageNumber = pageNumber
         if shouldThrowError {
             throw MockError.requestFailed
@@ -25,18 +21,9 @@ final class MockPointOfSaleItemService: PointOfSaleItemServiceProtocol {
         }
         if shouldSimulateTwoPages,
             pageNumber > 1 {
-            simulateFetchNextPage()
-            return (items, true)
+            return (MockPointOfSaleItemService.makeSecondPageItems(), false)
         }
-        return (MockPointOfSaleItemService.makeInitialItems(), false)
-    }
-
-    func simulateFetchNextPage() {
-        items.append(contentsOf: MockPointOfSaleItemService.makeSecondPageItems())
-    }
-
-    func simulateNextPageIsOutOfRange() {
-        isPageOutOfRange = true
+        return (MockPointOfSaleItemService.makeInitialItems(), shouldSimulateTwoPages)
     }
 }
 
@@ -79,6 +66,5 @@ extension MockPointOfSaleItemService {
 
     enum MockError: Error {
         case requestFailed
-        case pageOutOfRange
     }
 }
diff --git a/Yosemite/Yosemite/PointOfSale/PointOfSaleItemServiceProtocol.swift b/Yosemite/Yosemite/PointOfSale/PointOfSaleItemServiceProtocol.swift
@@ -51,7 +51,7 @@ public protocol PointOfSaleItemServiceProtocol {
 // Default implementation for convenience, so we do not need to pass the first page explicitly
 // if no pageNumber is given.
 extension PointOfSaleItemServiceProtocol {
-    func providePointOfSaleItems(pageNumber: Int = 1) async throws -> [POSItem] {
+    func providePointOfSaleItems(pageNumber: Int = 1) async throws -> (items: [POSItem], hasNextPage: Bool) {
         try await providePointOfSaleItems(pageNumber: pageNumber)
     }
 }
diff --git a/Yosemite/Yosemite/PointOfSale/PointOfSaleProductService.swift b/Yosemite/Yosemite/PointOfSale/PointOfSaleProductService.swift
@@ -7,7 +7,6 @@ import class WooFoundation.CurrencySettings
 
 public enum PointOfSaleProductServiceError: Error {
     case requestFailed
-    case pageOutOfRange
     case unknown
 }
 
@@ -47,7 +46,7 @@ public final class PointOfSaleProductService: PointOfSaleItemServiceProtocol {
         let (products, totalPagesCount) = try await productsRemote.loadProductsForPointOfSale(for: siteID, productTypes: productTypes, pageNumber: pageNumber)
 
         if pageNumber != 1 && products.count == 0 {
-            throw PointOfSaleProductServiceError.pageOutOfRange
+            return ([], false)
         }
 
         let eligibilityCriteria: [(Product) -> Bool] = [

diff --git a/Yosemite/YosemiteTests/PointOfSale/POSProductProviderTests.swift b/Yosemite/YosemiteTests/PointOfSale/POSProductProviderTests.swift
@@ -40,24 +40,21 @@ final class PointOfSaleProductServiceTests: XCTestCase {
         }
     }
 
-    func test_PointOfSaleItemServiceProtocol_when_fails_request_with_pageOutOfRange_then_throws_error() async throws {
-        let expectedError = PointOfSaleProductServiceError.pageOutOfRange
-        network.simulateError(requestUrlSuffix: "products", error: expectedError)
+    func test_PointOfSaleItemServiceProtocol_when_empty_data_for_non_first_page_then_returns_empty_items_and_no_next_page() async throws {
+        network.simulateResponse(requestUrlSuffix: "products", filename: "empty-data-array")
 
         // When
-        do {
-            _ = try await itemProvider.providePointOfSaleItems()
-            XCTFail("Expected an error, but got success.")
-        } catch {
-            // Then
-            XCTAssertEqual(error as? PointOfSaleProductServiceError, expectedError)
-        }
+        let (items, hasNextPage) = try await itemProvider.providePointOfSaleItems()
+
+        // Then
+        XCTAssertTrue(items.isEmpty)
+        XCTAssertFalse(hasNextPage)
     }
 
     func test_PointOfSaleItemServiceProtocol_provides_no_items_when_store_has_no_products() async throws {
         // Given/When
         network.simulateResponse(requestUrlSuffix: "products", filename: "empty-data-array")
-        let expectedItems = try await itemProvider.providePointOfSaleItems()
+        let (expectedItems, _) = try await itemProvider.providePointOfSaleItems()
 
         // Then
         XCTAssertTrue(expectedItems.isEmpty)
@@ -73,7 +70,7 @@ final class PointOfSaleProductServiceTests: XCTestCase {
 
         // When
         network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-all-type-simple")
-        let expectedItems = try await itemProvider.providePointOfSaleItems()
+        let (expectedItems, _) = try await itemProvider.providePointOfSaleItems()
 
         // Then
         guard let item = expectedItems.first,
@@ -94,7 +91,7 @@ final class PointOfSaleProductServiceTests: XCTestCase {
 
         // When
         network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-all-for-eligibility-criteria")
-        let expectedItems = try await itemProvider.providePointOfSaleItems()
+        let (expectedItems, _) = try await itemProvider.providePointOfSaleItems()
 
         // Then
         XCTAssertEqual(expectedItems.count, expectedNumberOfItems)