Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ protocol PointOfSaleSearchingItemsControllerProtocol: PointOfSaleItemsController
init(itemProvider: PointOfSaleItemServiceProtocol,
itemFetchStrategyFactory: PointOfSaleItemFetchStrategyFactoryProtocol,
initialState: ItemsViewState = ItemsViewState(containerState: .loading,
itemsStack: ItemsStackState(root: .loading([]),
itemsStack: ItemsStackState(root: .initial,
itemStates: [:])),
analyticsProvider: POSAnalyticsProviding) {
self.itemProvider = itemProvider
Expand Down Expand Up @@ -213,8 +213,12 @@ private extension PointOfSaleItemsController {
func setRootLoadingState() {
let items = itemsViewState.itemsStack.root.items

let isInitialState = itemsViewState.containerState == .loading
if !isInitialState {
let isInitialState = itemsViewState.containerState == .loading && itemsViewState.itemsStack.root == .initial
if isInitialState {
// Transition from initial to loading on first load
itemsViewState.itemsStack.root = .loading([])
} else {
// Preserve items during refresh
itemsViewState.itemsStack.root = .loading(items)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
import Foundation
import Observation
import class WooFoundation.CurrencySettings
import enum Yosemite.POSItem
import protocol Yosemite.POSObservableDataSourceProtocol
import struct Yosemite.POSVariableParentProduct
import class Yosemite.GRDBObservableDataSource
import protocol Storage.GRDBManagerProtocol

/// Controller that wraps an observable data source for POS items
/// Uses computed state based on data source observations for automatic UI updates
@Observable
final class PointOfSaleObservableItemsController: PointOfSaleItemsControllerProtocol {
private let dataSource: POSObservableDataSourceProtocol

// Track which items have been loaded at least once
private var hasLoadedProducts = false
private var hasLoadedVariationsForCurrentParent = false

// Track current parent for variation state mapping
private var currentParentItem: POSItem?

var itemsViewState: ItemsViewState {
ItemsViewState(
containerState: containerState,
itemsStack: ItemsStackState(
root: rootState,
itemStates: variationStates
)
)
}

init(siteID: Int64,
grdbManager: GRDBManagerProtocol,
currencySettings: CurrencySettings) {
self.dataSource = GRDBObservableDataSource(
siteID: siteID,
grdbManager: grdbManager,
currencySettings: currencySettings
)
}

// periphery:ignore - used by tests
init(dataSource: POSObservableDataSourceProtocol) {
self.dataSource = dataSource
}

func loadItems(base: ItemListBaseItem) async {
switch base {
case .root:
hasLoadedProducts = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we delay it until the items are loaded? Same with other similar flags.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 2bdf4ab

dataSource.loadProducts()
case .parent(let parent):
guard case .variableParentProduct(let parentProduct) = parent else {
assertionFailure("Unsupported parent type for loading items: \(parent)")
return
}

// If switching to a different parent, reset the loaded flag
if currentParentItem != parent {
currentParentItem = parent
hasLoadedVariationsForCurrentParent = false
}

hasLoadedVariationsForCurrentParent = true
dataSource.loadVariations(for: parentProduct)
}
}

func refreshItems(base: ItemListBaseItem) async {
switch base {
case .root:
dataSource.refresh()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this is not implemented. Is the idea to trigger an incremental sync when refresh is called?

Copy link
Contributor Author

@joshheald joshheald Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's right

case .parent(let parent):
guard case .variableParentProduct(let parentProduct) = parent else {
assertionFailure("Unsupported parent type for refreshing items: \(parent)")
return
}
dataSource.loadVariations(for: parentProduct)
}
}

func loadNextItems(base: ItemListBaseItem) async {
switch base {
case .root:
dataSource.loadMoreProducts()
case .parent:
dataSource.loadMoreVariations()
}
}
}

// MARK: - State Computation
private extension PointOfSaleObservableItemsController {
var containerState: ItemsContainerState {
// Use .loading during initial load, .content otherwise
if !hasLoadedProducts && dataSource.isLoadingProducts {
return .loading
}
return .content
}

var rootState: ItemListState {
let items = dataSource.productItems

// Initial state - not yet loaded
if !hasLoadedProducts {
return .initial
}

// Loading state - preserve existing items
if dataSource.isLoadingProducts {
return .loading(items)
}

// Error state
if let error = dataSource.error, items.isEmpty {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What kind of errors can we expect here? Since this comes from database, only something super-unexpected I suppose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be extremely rare – these are really simple select queries...

  • Database corruption
  • Programming errors during development, e.g. relationships missing, or incorrect query setup – but those shouldn't ever happen in production

Errors are more likely to appear when we sync the data, because we're relying on it coming down with the correct schema and logically consistent. These will happen when we write stuff to the DB, and will prevent the whole write transacion if they're not handled in other code... so they won't get as far as being shown here. It's primarily included for completeness with the existing code, and to make it easier to spot bugs in dev.

I noticed that we only have one error property, shared between variations and products, which could result in an error state being shown for the wrong screen. I've separated them out. 46b634a

return .error(.errorOnLoadingProducts(error: error))
}

// Empty state
if items.isEmpty {
return .empty
}

// Loaded state
return .loaded(items, hasMoreItems: dataSource.hasMoreProducts)
}

var variationStates: [POSItem: ItemListState] {
guard let parentItem = currentParentItem else {
return [:]
}

let items = dataSource.variationItems

// Initial state - not yet loaded
if !hasLoadedVariationsForCurrentParent {
return [parentItem: .initial]
}

// Loading state - preserve existing items
if dataSource.isLoadingVariations {
return [parentItem: .loading(items)]
}

// Error state
if let error = dataSource.error, items.isEmpty {
return [parentItem: .error(.errorOnLoadingVariations(error: error))]
}

// Empty state
if items.isEmpty {
return [parentItem: .empty]
}

// Loaded state
return [parentItem: .loaded(items, hasMoreItems: dataSource.hasMoreVariations)]
}
}
3 changes: 2 additions & 1 deletion Modules/Sources/PointOfSale/Models/ItemListState.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import enum Yosemite.POSItem

enum ItemListState {
case initial
Copy link
Contributor Author

@joshheald joshheald Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this state for more accurate modelling of what's happening.

My intention is to delay showing the loading indicators when we open variations with GRDB – it causes a slight flicker at the moment. This is some groundwork for that.

https://linear.app/a8c/issue/WOOMOB-1494/woo-poslocal-catalog-add-a-delay-before-showing-loading-indicators

case loading(_ currentItems: [POSItem])
case loaded(_ items: [POSItem], hasMoreItems: Bool)
case inlineError(_ items: [POSItem], error: PointOfSaleErrorState, context: InlineErrorContext)
Expand Down Expand Up @@ -38,7 +39,7 @@ extension ItemListState {
.loaded(let items, _),
.inlineError(let items, _, _):
return items
case .error, .empty:
case .initial, .error, .empty:
return []
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ struct ChildItemList: View {
var body: some View {
VStack {
switch state {
case .loaded([], _):
case .initial, .loaded([], _):
emptyView
case .loading, .loaded, .inlineError:
listView
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ struct ItemList<HeaderView: View>: View {
await itemsController.loadNextItems(base: node)
}
})
case .loaded, .error, .empty, .none, .inlineError(_, _, .refresh):
case .initial, .loaded, .error, .empty, .none, .inlineError(_, _, .refresh):
EmptyView()
}
}
Expand All @@ -136,7 +136,7 @@ struct ItemList<HeaderView: View>: View {
await itemsController.loadItems(base: .root)
}
})
case .loaded, .error, .empty, .none, .loading, .inlineError(_, _, .pagination):
case .initial, .loaded, .error, .empty, .none, .loading, .inlineError(_, _, .pagination):
EmptyView()
}
}
Expand Down
3 changes: 2 additions & 1 deletion Modules/Sources/PointOfSale/Presentation/ItemListView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@ struct ItemListView: View {
@ViewBuilder
private func itemListContent(_ itemListType: ItemListType) -> some View {
switch itemListState(itemListType) {
case .loading,
case .initial,
.loading,
.loaded,
.inlineError:
listView(itemListType: itemListType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,23 @@ public struct PointOfSaleEntryPointView: View {
services: POSDependencyProviding) {
self.onPointOfSaleModeActiveStateChange = onPointOfSaleModeActiveStateChange

self.itemsController = PointOfSaleItemsController(
itemProvider: PointOfSaleItemService(currencySettings: services.currency.currencySettings),
itemFetchStrategyFactory: itemFetchStrategyFactory,
analyticsProvider: services.analytics
)
// Use observable controller with GRDB if available and feature flag is enabled, otherwise fall back to standard controller
// Note: We check feature flag here for eligibility. Once eligibility checking is
// refactored to be more centralized, this check can be simplified.
let isGRDBEnabled = services.featureFlags.isFeatureFlagEnabled(.pointOfSaleLocalCatalogi1)
if let grdbManager = grdbManager, catalogSyncCoordinator != nil, isGRDBEnabled {
self.itemsController = PointOfSaleObservableItemsController(
siteID: siteID,
grdbManager: grdbManager,
currencySettings: services.currency.currencySettings
)
} else {
self.itemsController = PointOfSaleItemsController(
itemProvider: PointOfSaleItemService(currencySettings: services.currency.currencySettings),
itemFetchStrategyFactory: itemFetchStrategyFactory,
analyticsProvider: services.analytics
)
}
self.purchasableItemsSearchController = PointOfSaleItemsController(
itemProvider: PointOfSaleItemService(currencySettings: services.currency.currencySettings),
itemFetchStrategyFactory: itemFetchStrategyFactory,
Expand Down
Loading