14693 Separate container states from list states

woocommerce · Dec 18, 2024 · 6bdb9f3 · 6bdb9f3
1 parent 7f8bd39
commit 6bdb9f3
Show file tree

Hide file tree

Showing 17 changed files with 198 additions and 73 deletions.
diff --git a/Fakes/Fakes/Fake.swift b/Fakes/Fakes/Fake.swift
@@ -119,3 +119,10 @@ extension NSRange {
         .init()
     }
 }
+
+extension UUID {
+    /// Returns a default UUID
+    static func fake() -> Self {
+        .init()
+    }
+}
diff --git a/Fakes/Fakes/Yosemite.generated.swift b/Fakes/Fakes/Yosemite.generated.swift
@@ -33,6 +33,22 @@ extension Yosemite.JustInTimeMessageTemplate {
         .banner
     }
 }
+extension Yosemite.POSSimpleProduct {
+    /// Returns a "ready to use" type filled with fake values.
+    ///
+    public static func fake() -> Yosemite.POSSimpleProduct {
+        .init(
+            id: .fake(),
+            name: .fake(),
+            formattedPrice: .fake(),
+            productImageSource: .fake(),
+            productID: .fake(),
+            price: .fake(),
+            productType: .fake(),
+            bundledItems: .fake()
+        )
+    }
+}
 extension Yosemite.ProductReviewFromNoteParcel {
     /// Returns a "ready to use" type filled with fake values.
     ///

diff --git a/Networking/Networking/Remote/ProductsRemote.swift b/Networking/Networking/Remote/ProductsRemote.swift
@@ -645,7 +645,7 @@ public extension ProductsRemote {
 
 private extension ProductsRemote {
     enum POSConstants {
-        static let productsPerPage = "100"
+        static let productsPerPage = "5"
         static let productType = "simple"
         static let productStatus = "publish"
     }

diff --git a/WooCommerce/Classes/Copiable/Models+Copiable.generated.swift b/WooCommerce/Classes/Copiable/Models+Copiable.generated.swift
@@ -50,6 +50,21 @@ extension WooCommerce.AggregateOrderItem {
     }
 }
 
+extension WooCommerce.ItemsViewState {
+    func copy(
+        containerState: CopiableProp<ItemsContainerState> = .copy,
+        itemsStack: CopiableProp<[ItemsNavigationNode: ItemListState]> = .copy
+    ) -> WooCommerce.ItemsViewState {
+        let containerState = containerState ?? self.containerState
+        let itemsStack = itemsStack ?? self.itemsStack
+
+        return WooCommerce.ItemsViewState(
+            containerState: containerState,
+            itemsStack: itemsStack
+        )
+    }
+}
+
 extension WooCommerce.ShippingLabelSelectedRate {
     func copy(
         packageID: CopiableProp<String> = .copy,

diff --git a/WooCommerce/Classes/POS/Controllers/PointOfSaleItemsController.swift b/WooCommerce/Classes/POS/Controllers/PointOfSaleItemsController.swift
@@ -5,88 +5,85 @@ import protocol Yosemite.PointOfSaleItemServiceProtocol
 import enum Yosemite.PointOfSaleProductServiceError
 
 protocol PointOfSaleItemsControllerProtocol {
-    var itemListStatePublisher: any Publisher<ItemListState, Never> { get }
+    var itemsViewStatePublisher: any Publisher<ItemsViewState, Never> { get }
     func loadInitialItems() async
     func loadNextItems() async
     func reload() async
 }
 
 class PointOfSaleItemsController: PointOfSaleItemsControllerProtocol {
-    private(set) var itemListStatePublisher: any Publisher<ItemListState, Never>
-    private var itemListStateSubject: PassthroughSubject<ItemListState, Never> = .init()
-    private var allItems: [POSItem] = []
-    private var isInitialLoading: Bool = true
-    private let paginationTracker: PaginationTracker
+    private(set) var itemsViewStatePublisher: any Publisher<ItemsViewState, Never>
+    private var itemsViewStateSubject: PassthroughSubject<ItemsViewState, Never> = .init()
+    private var itemsViewState: ItemsViewState = .init(containerState: .loading, itemsStack: [:]) {
+        didSet {
+            itemsViewStateSubject.send(itemsViewState)
+        }
+    }
+    private let paginationTracker: PaginationTracker = PaginationTracker()
     private let itemProvider: PointOfSaleItemServiceProtocol
 
     init(itemProvider: PointOfSaleItemServiceProtocol) {
         self.itemProvider = itemProvider
-        self.paginationTracker = .init(pageFirstIndex: Constants.initialPage)
-        itemListStatePublisher = itemListStateSubject.eraseToAnyPublisher()
+        itemsViewStatePublisher = itemsViewStateSubject.eraseToAnyPublisher()
 
         paginationTracker.delegate = self
     }
 
     @MainActor
     func loadInitialItems() async {
+        itemsViewState = ItemsViewState(containerState: .loading, itemsStack: [:])
         paginationTracker.syncFirstPage()
     }
 
     @MainActor
     func loadNextItems() async {
+        let currentItems = itemsViewState.itemsStack[.root]?.items ?? []
+        itemsViewState = ItemsViewState(containerState: .content, itemsStack: [.root: .loading(currentItems)])
         paginationTracker.ensureNextPageIsSynced()
     }
 
     @MainActor
     func reload() async {
-        allItems.removeAll()
+        itemsViewState = ItemsViewState(containerState: .content, itemsStack: [.root: .loading([])])
         paginationTracker.resync()
     }
 
-    /// <#Description#>
-    /// - Parameter pageNumber: <#pageNumber description#>
-    /// - 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 ?? []
         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: [.root: .loaded(allItems)])
         return hasNextPage
     }
+}
 
-    private func updateItemListStateAfterLoadAttempt() {
-        if allItems.isEmpty {
-            itemListStateSubject.send(.empty)
-        } else {
-            itemListStateSubject.send(.loaded(allItems))
+private extension ItemListState {
+    var items: [POSItem] {
+        switch self {
+        case .loading(let items),
+                .loaded(let items):
+            return items
+        case .error:
+            return []
         }
     }
-
-    private enum Constants {
-        static let initialPage: Int = 1
-    }
 }
 
 extension PointOfSaleItemsController: PaginationTrackerDelegate {
     func sync(pageNumber: Int, pageSize: Int, reason: String?, onCompletion: SyncCompletion?) {
-        if isInitialLoading {
-            isInitialLoading = false
-            itemListStateSubject.send(.initialLoading)
-        } else {
-            itemListStateSubject.send(.loading(allItems))
-        }
         Task { @MainActor in
             do {
                 let hasNextPage = try await fetchItems(pageNumber: pageNumber)
-                updateItemListStateAfterLoadAttempt()
                 onCompletion?(.success(hasNextPage))
-            } catch PointOfSaleProductServiceError.pageOutOfRange {
-                updateItemListStateAfterLoadAttempt()
-                onCompletion?(.failure(PointOfSaleProductServiceError.pageOutOfRange))
             } catch {
-                itemListStateSubject.send(.error(PointOfSaleErrorState.errorOnLoadingProducts()))
+                itemsViewStateSubject.send(ItemsViewState(containerState: .error(PointOfSaleErrorState.errorOnLoadingProducts()),
+                                                          itemsStack: [:]))
                 onCompletion?(.failure(error))
             }
         }

diff --git a/WooCommerce/Classes/POS/Models/ItemListState.swift b/WooCommerce/Classes/POS/Models/ItemListState.swift
@@ -1,14 +1,12 @@
 import enum Yosemite.POSItem
-import protocol Yosemite.POSOrderableItem
+import Codegen
 
-enum ItemListState: Equatable {
-    case empty
-    case initialLoading
+enum ItemListState {
     case loading(_ currentItems: [POSItem])
     case loaded(_ items: [POSItem])
     case error(PointOfSaleErrorState)
 
-    var isLoadingAfterInitialLoad: Bool {
+    var isLoading: Bool {
         switch self {
         case .loading:
             return true
@@ -17,3 +15,5 @@ enum ItemListState: Equatable {
         }
     }
 }
+
+extension ItemListState: Equatable, GeneratedCopiable {}
diff --git a/WooCommerce/Classes/POS/Models/ItemsContainerState.swift b/WooCommerce/Classes/POS/Models/ItemsContainerState.swift
@@ -0,0 +1,10 @@
+import Foundation
+
+enum ItemsContainerState {
+    case loading
+    case empty
+    case error(PointOfSaleErrorState)
+    case content
+}
+
+extension ItemsContainerState: Equatable {}
diff --git a/WooCommerce/Classes/POS/Models/ItemsNavigationNode.swift b/WooCommerce/Classes/POS/Models/ItemsNavigationNode.swift
@@ -0,0 +1,9 @@
+import Foundation
+import enum Yosemite.POSItem
+
+enum ItemsNavigationNode {
+    case root
+    case item(POSItem)
+}
+
+extension ItemsNavigationNode: Hashable, Equatable {}
diff --git a/WooCommerce/Classes/POS/Models/ItemsViewState.swift b/WooCommerce/Classes/POS/Models/ItemsViewState.swift
@@ -0,0 +1,9 @@
+import Foundation
+import Codegen
+
+struct ItemsViewState {
+    let containerState: ItemsContainerState
+    let itemsStack: [ItemsNavigationNode: ItemListState]
+}
+
+extension ItemsViewState: GeneratedCopiable, Equatable {}
diff --git a/WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift b/WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift
@@ -21,7 +21,7 @@ protocol PointOfSaleAggregateModelProtocol {
     func cancelCardPaymentsOnboarding()
     func trackCardPaymentsOnboardingShown()
 
-    var itemListState: ItemListState { get }
+    var itemsViewState: ItemsViewState { get }
     func loadInitialItems() async
     func loadNextItems() async
     func reload() async
@@ -47,7 +47,8 @@ class PointOfSaleAggregateModel: ObservableObject, PointOfSaleAggregateModelProt
     @Published var cardPresentPaymentOnboardingViewModel: CardPresentPaymentsOnboardingViewModel?
     private var onOnboardingCancellation: (() -> Void)?
 
-    @Published private(set) var itemListState: ItemListState = .initialLoading
+    @Published private(set) var itemsViewState: ItemsViewState = ItemsViewState(containerState: .loading,
+                                                                                itemsStack: [:])
 
     @Published private(set) var cart: [CartItem] = []
 
@@ -74,7 +75,7 @@ class PointOfSaleAggregateModel: ObservableObject, PointOfSaleAggregateModelProt
         self.orderController = orderController
         self.analytics = analytics
         self.paymentState = paymentState
-        publishItemListState()
+        publishItemsViewState()
         publishCardReaderConnectionStatus()
         publishPaymentMessages()
         publishOrderState()
@@ -84,8 +85,8 @@ class PointOfSaleAggregateModel: ObservableObject, PointOfSaleAggregateModelProt
 
 // MARK: - ItemList
 extension PointOfSaleAggregateModel {
-    private func publishItemListState() {
-        itemsController.itemListStatePublisher.assign(to: &$itemListState)
+    private func publishItemsViewState() {
+        itemsController.itemsViewStatePublisher.assign(to: &$itemsViewState)
     }
 
     @MainActor

diff --git a/WooCommerce/Classes/POS/Presentation/ItemListView.swift b/WooCommerce/Classes/POS/Presentation/ItemListView.swift
@@ -10,20 +10,22 @@ struct ItemListView: View {
 
     @State private var lastScrollPosition: CGFloat = 0
     @State private var showSimpleProductsModal: Bool = false
+    let itemListState: ItemListState
 
     @AppStorage(BannerState.isSimpleProductsOnlyBannerDismissedKey)
     private var isHeaderBannerDismissed: Bool = false
 
     var body: some View {
         VStack {
             headerView
-            switch posModel.itemListState {
-            case .initialLoading, .empty, .error:
-                // These cases are handled directly in the dashboard, we do not render
-                // a specific view within the ItemListView to handle them
-                EmptyView()
-            case .loading(let items), .loaded(let items):
+            switch itemListState {
+            case .loading(let items),
+                    .loaded(let items):
                 listView(items)
+            case .error(let pointOfSaleErrorState):
+                // Currently unused, but this will show errors that are displayed inline with previously
+                // loaded items, e.g. when loading a new page or refreshing.
+                EmptyView()
             }
         }
         .refreshable {
@@ -134,15 +136,15 @@ private extension ItemListView {
                     listRow(item: item)
                 }
                 GhostItemCardView()
-                    .renderedIf(posModel.itemListState.isLoadingAfterInitialLoad)
+                    .renderedIf(itemListState.isLoading)
             }
             .frame(maxWidth: .infinity)
             .padding(.bottom, floatingControlAreaSize.height)
             .padding(.horizontal, Constants.itemListPadding)
             .background(GeometryReader { proxy in
                 Color.clear
                     .onChange(of: proxy.frame(in: .global).maxY) { maxY in
-                        if posModel.itemListState.isLoadingAfterInitialLoad {
+                        if itemListState.isLoading {
                             return
                         }
                         let threshold = Constants.viewHeight * Constants.scrollThresholdMultiplier
@@ -172,7 +174,7 @@ private extension ItemListView {
 
 private extension ItemListView {
     var shouldShowHeaderBanner: Bool {
-        posModel.itemListState.eligibleToShowSimpleProductsBanner && !isHeaderBannerDismissed
+        itemListState.eligibleToShowSimpleProductsBanner && !isHeaderBannerDismissed
     }
 }
 
@@ -182,9 +184,7 @@ private extension ItemListState {
         case .loading,
                 .loaded:
             return true
-        case .empty,
-            .initialLoading,
-            .error:
+        case .error:
             return false
         }
     }
@@ -287,7 +287,14 @@ private extension ItemListView {
 }
 
 #if DEBUG
+import struct Yosemite.POSSimpleProduct
+
 #Preview {
-    ItemListView()
+    let simpleProduct = POSSimpleProduct(id: UUID(),
+                                         name: "A simple product",
+                                         formattedPrice: "$5.00",
+                                         productID: 2,
+                                         price: "5.00")
+    ItemListView(itemListState: ItemListState.loaded([.simpleProduct(simpleProduct)]))
 }
 #endif