Merge branch 'feat/14699-pagination-headers-products' into issue/1469…

…3-split-container-and-list-states-with-new-pagination
woocommerce · Dec 19, 2024 · ac570eb · ac570eb
2 parents 47f3e10 + 1fb8df0
commit ac570eb
Show file tree

Hide file tree

Showing 14 changed files with 304 additions and 99 deletions.
diff --git a/Networking/Networking/Network/MockNetwork.swift b/Networking/Networking/Network/MockNetwork.swift
@@ -27,6 +27,9 @@ class MockNetwork: Network {
     ///
     var requestsForResponseData = [URLRequestConvertible]()
 
+    /// Response headers to be returned with the response data.
+    var responseHeaders: [String: String]?
+
     /// Number of notification objects in notifications-load-all.json file.
     ///
     static let notificationLoadAllJSONCount = 46
@@ -79,8 +82,17 @@ class MockNetwork: Network {
     }
 
     func responseDataAndHeaders(for request: any URLRequestConvertible) async throws -> (Data, ResponseHeaders?) {
-        // TODO
-        throw NetworkError.notFound()
+        requestsForResponseData.append(request)
+
+        if let error = error(for: request) {
+            throw error
+        }
+
+        guard let name = filename(for: request), let data = Loader.contentsOf(name) else {
+            throw NetworkError.notFound()
+        }
+
+        return (data, responseHeaders)
     }
 
     func responseDataPublisher(for request: URLRequestConvertible) -> AnyPublisher<Swift.Result<Data, Error>, Never> {

diff --git a/Networking/Networking/Remote/ProductsRemote.swift b/Networking/Networking/Remote/ProductsRemote.swift
@@ -204,7 +204,9 @@ public final class ProductsRemote: Remote, ProductsRemoteProtocol {
     /// - productTypes: A list of product types to be included in the results.
     /// - pageNumber: Number of page that should be retrieved.
     ///
-    public func loadProductsForPointOfSale(for siteID: Int64, productTypes: [ProductType] = [.simple], pageNumber: Int = 1) async throws -> (products: [Product], totalPagesCount: Int?) {
+    public func loadProductsForPointOfSale(for siteID: Int64,
+                                           productTypes: [ProductType] = [.simple],
+                                           pageNumber: Int = 1) async throws -> PagedItems<Product> {
         let parameters = [
             ParameterKey.page: String(pageNumber),
             ParameterKey.perPage: POSConstants.productsPerPage,
@@ -229,8 +231,9 @@ public final class ProductsRemote: Remote, ProductsRemoteProtocol {
         // Response header names are case insensitive.
         let totalPages = responseHeaders?.first(where: { $0.key.lowercased() == Remote.PaginationHeaderKey.totalPagesCount.lowercased() })
             .flatMap { Int($0.value) }
+        let hasMorePages = totalPages.map { pageNumber < $0 } ?? true
 
-        return (products: products, totalPagesCount: totalPages)
+        return .init(items: products, hasMorePages: hasMorePages)
     }
 
     /// Retrieves a specific list of `Product`s by `productID`.

diff --git a/Networking/Networking/Remote/Remote.swift b/Networking/Networking/Remote/Remote.swift
@@ -366,6 +366,17 @@ private extension Remote {
     }
 }
 
+/// Contains the result of a paginated request.
+public struct PagedItems<T> {
+    public let items: [T]
+    public let hasMorePages: Bool
+
+    public init(items: [T], hasMorePages: Bool) {
+        self.items = items
+        self.hasMorePages = hasMorePages
+    }
+}
+
 // MARK: - Constants!
 //
 public extension Remote {

diff --git a/Networking/NetworkingTests/Remote/ProductsRemoteTests.swift b/Networking/NetworkingTests/Remote/ProductsRemoteTests.swift
@@ -940,9 +940,10 @@ final class ProductsRemoteTests: XCTestCase {
         // When
         network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-all-type-simple")
 
-        let (products, _) = try await remote.loadProductsForPointOfSale(for: sampleSiteID)
+        let pagedProducts = try await remote.loadProductsForPointOfSale(for: sampleSiteID)
 
         // Then
+        let products = pagedProducts.items
         XCTAssertEqual(products.count, expectedProductsFromResponse)
         for product in products {
             XCTAssertEqual(try XCTUnwrap(product).productType, .simple)
@@ -970,9 +971,10 @@ final class ProductsRemoteTests: XCTestCase {
         // When
         network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-all-type-simple")
 
-        let (products, _) = try await remote.loadProductsForPointOfSale(for: sampleSiteID, pageNumber: initialPageNumber)
+        let pagedProducts = try await remote.loadProductsForPointOfSale(for: sampleSiteID, pageNumber: initialPageNumber)
 
         // Then
+        let products = pagedProducts.items
         XCTAssertEqual(products.count, expectedProductsFromResponse)
         for product in products {
             XCTAssertEqual(try XCTUnwrap(product).productType, .simple)
@@ -988,10 +990,46 @@ final class ProductsRemoteTests: XCTestCase {
         // When
         network.simulateResponse(requestUrlSuffix: "products", filename: "empty-data-array")
 
-        let (products, _) = try await remote.loadProductsForPointOfSale(for: sampleSiteID, pageNumber: pageNumber)
+        let pagedProducts = try await remote.loadProductsForPointOfSale(for: sampleSiteID, pageNumber: pageNumber)
 
         // Then
-        XCTAssertEqual(products.count, expectedProductsFromResponse)
+        XCTAssertEqual(pagedProducts.items.count, expectedProductsFromResponse)
+    }
+
+    func test_loadProductsForPointOfSale_returns_hasMorePages_based_on_header_with_case_insensitive_name() async throws {
+        // Given
+        let remote = ProductsRemote(network: network)
+        network.responseHeaders = ["X-WP-TotalPages": "5"]
+        network.simulateResponse(requestUrlSuffix: "products", filename: "empty-data-array")
+
+        // When loading page 1 to 4
+        for pageNumber in 1...4 {
+            let pagedProducts = try await remote.loadProductsForPointOfSale(for: sampleSiteID, pageNumber: pageNumber)
+
+            // Then
+            XCTAssertTrue(pagedProducts.hasMorePages)
+        }
+
+        // When loading page 17
+        let pagedProducts = try await remote.loadProductsForPointOfSale(for: sampleSiteID, pageNumber: 5)
+
+        // Then
+        XCTAssertFalse(pagedProducts.hasMorePages)
+    }
+
+    func test_loadProductsForPointOfSale_returns_hasMorePages_true_when_header_is_not_set() async throws {
+        // Given
+        let remote = ProductsRemote(network: network)
+        network.responseHeaders = nil
+        network.simulateResponse(requestUrlSuffix: "products", filename: "empty-data-array")
+
+        // When loading the first 5 pages
+        for pageNumber in 1...5 {
+            let pagedProducts = try await remote.loadProductsForPointOfSale(for: sampleSiteID, pageNumber: pageNumber)
+
+            // Then
+            XCTAssertTrue(pagedProducts.hasMorePages)
+        }
     }
 }
 

diff --git a/WooCommerce/Classes/POS/Controllers/PointOfSaleItemsController.swift b/WooCommerce/Classes/POS/Controllers/PointOfSaleItemsController.swift
@@ -20,61 +20,84 @@ class PointOfSaleItemsController: PointOfSaleItemsControllerProtocol {
             itemsViewStateSubject.send(itemsViewState)
         }
     }
-    private let paginationTracker: PaginationTracker = PaginationTracker()
+    private let paginationTracker: AsyncPaginationTracker
     private let itemProvider: PointOfSaleItemServiceProtocol
 
     init(itemProvider: PointOfSaleItemServiceProtocol) {
         self.itemProvider = itemProvider
+        self.paginationTracker = .init()
         itemsViewStatePublisher = itemsViewStateSubject.eraseToAnyPublisher()
-
-        paginationTracker.delegate = self
     }
 
     @MainActor
     func loadInitialItems() async {
         itemsViewState = ItemsViewState(containerState: .loading, itemsStack: ItemsStackState(root: .loading([])))
-        await withCheckedContinuation { continuation in
-            paginationTracker.syncFirstPage {
-                continuation.resume()
+        do {
+            try await paginationTracker.syncFirstPage { [weak self] pageNumber in
+                guard let self else { return true }
+                return try await fetchItems(pageNumber: pageNumber)
             }
+        } catch {
+            itemsViewState = ItemsViewState(containerState: .error(PointOfSaleErrorState.errorOnLoadingProducts()),
+                                            itemsStack: ItemsStackState(root: .loaded([])))
         }
     }
 
     @MainActor
     func loadNextItems() async {
+        guard paginationTracker.hasNextPage else {
+            return
+        }
         let currentItems = itemsViewState.itemsStack.root.items
         itemsViewState = ItemsViewState(containerState: .content, itemsStack: ItemsStackState(root: .loading(currentItems)))
-        await withCheckedContinuation { continuation in
-            paginationTracker.ensureNextPageIsSynced {
-                continuation.resume()
+        do {
+            _ = try await paginationTracker.ensureNextPageIsSynced { [weak self] pageNumber in
+                guard let self else { return true }
+                return try await fetchItems(pageNumber: pageNumber)
             }
+        } catch {
+            // TODO: 14694 - Handle error from loading the next page, like showing an error UI at the end or as an overlay.
+            itemsViewState = ItemsViewState(containerState: .error(PointOfSaleErrorState.errorOnLoadingProducts()),
+                                            itemsStack: ItemsStackState(root: .loaded(currentItems)))
         }
-        let updatedItems = itemsViewState.itemsStack.root.items
-        itemsViewState = ItemsViewState(containerState: .content, itemsStack: ItemsStackState(root: .loaded(updatedItems)))
     }
 
     @MainActor
     func reload() async {
-        itemsViewState = ItemsViewState(containerState: .content, itemsStack: ItemsStackState(root: .loading([])))
-        await withCheckedContinuation { continuation in
-            paginationTracker.resync {
-                continuation.resume()
+        do {
+            try await paginationTracker.resync { [weak self] pageNumber in
+                guard let self else { return true }
+                return try await fetchItems(pageNumber: pageNumber, appendToExistingItems: false)
             }
+        } catch {
+            // TODO: 14694 - Handle error from pull-to-refresh, like showing an error UI at the beginning or as an overlay.
+            itemsViewState = ItemsViewState(containerState: .error(PointOfSaleErrorState.errorOnLoadingProducts()),
+                                            itemsStack: ItemsStackState(root: .loaded([])))
         }
     }
 
+    /// Fetches items given a page number and appends new unique items to the `allItems` array.
+    /// - Parameter pageNumber: Page number to fetch items from.
+    /// - Parameter appendToExistingItems: Default true – set this to false when refreshing to make the new page the only page.
+    /// - Returns: A boolean that indicates whether there is next page for the paginated items.
     @MainActor
-    private func fetchItems(pageNumber: Int) async throws -> Bool {
-        let (newItems, hasNextPage) = try await itemProvider.providePointOfSaleItems(pageNumber: pageNumber)
-        var allItems = itemsViewState.itemsStack.root.items
+    private func fetchItems(pageNumber: Int, appendToExistingItems: Bool = true) async throws -> Bool {
+        let pagedItems = try await itemProvider.providePointOfSaleItems(pageNumber: pageNumber)
+        let newItems = pagedItems.items
+        var allItems = appendToExistingItems ? itemsViewState.itemsStack.root.items : []
         let uniqueNewItems = newItems.filter { newItem in
             // Note that this uniquing won't currently work, as POSItem has a UUID.
             !allItems.contains(newItem)
         }
         allItems.append(contentsOf: uniqueNewItems)
-        itemsViewState = ItemsViewState(containerState: .content,
-                                        itemsStack: ItemsStackState(root: .loaded(allItems)))
-        return hasNextPage
+        if allItems.isEmpty {
+            itemsViewState = ItemsViewState(containerState: .empty,
+                                            itemsStack: ItemsStackState(root: .loaded([])))
+        } else {
+            itemsViewState = ItemsViewState(containerState: .content,
+                                            itemsStack: ItemsStackState(root: .loaded(allItems)))
+        }
+        return pagedItems.hasMorePages
     }
 }
 
@@ -89,18 +112,3 @@ private extension ItemListState {
         }
     }
 }
-
-extension PointOfSaleItemsController: PaginationTrackerDelegate {
-    func sync(pageNumber: Int, pageSize: Int, reason: String?, onCompletion: SyncCompletion?) {
-        Task { @MainActor in
-            do {
-                let hasNextPage = try await fetchItems(pageNumber: pageNumber)
-                onCompletion?(.success(hasNextPage))
-            } catch {
-                itemsViewState = ItemsViewState(containerState: .error(PointOfSaleErrorState.errorOnLoadingProducts()),
-                                                itemsStack: ItemsStackState(root: .loading([])))
-                onCompletion?(.failure(error))
-            }
-        }
-    }
-}
diff --git a/WooCommerce/Classes/POS/Utils/PreviewHelpers.swift b/WooCommerce/Classes/POS/Utils/PreviewHelpers.swift
@@ -8,6 +8,7 @@ import protocol Yosemite.POSOrderableItem
 import protocol Yosemite.OrderSyncProductTypeProtocol
 import struct Yosemite.OrderSyncProductInput
 import enum Yosemite.ProductType
+import struct Yosemite.PagedItems
 import struct Yosemite.ProductBundleItem
 import struct Yosemite.OrderItem
 import Combine
@@ -37,8 +38,8 @@ struct POSProductPreview: POSOrderableItem, Equatable {
 }
 
 final class PointOfSalePreviewItemService: PointOfSaleItemServiceProtocol {
-    func providePointOfSaleItems(pageNumber: Int) async throws -> (items: [POSItem], hasNextPage: Bool) {
-        (items: [], hasNextPage: true)
+    func providePointOfSaleItems(pageNumber: Int) async throws -> PagedItems<POSItem> {
+        .init(items: [], hasMorePages: true)
     }
 
     func providePointOfSaleItems() -> [POSItem] {