-
Notifications
You must be signed in to change notification settings - Fork 118
[Local Catalog] Show items from GRDB in POS #16235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: issue/woomob-1088-use-GRDB-in-itemlist
Are you sure you want to change the base?
Changes from all commits
43cbb65
7a35f04
e1f994b
2609ec0
64ef020
eba2394
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
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() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? |
||
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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? |
||
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)] | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
import enum Yosemite.POSItem | ||
|
||
enum ItemListState { | ||
case initial | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
case loading(_ currentItems: [POSItem]) | ||
case loaded(_ items: [POSItem], hasMoreItems: Bool) | ||
case inlineError(_ items: [POSItem], error: PointOfSaleErrorState, context: InlineErrorContext) | ||
|
@@ -38,7 +39,7 @@ extension ItemListState { | |
.loaded(let items, _), | ||
.inlineError(let items, _, _): | ||
return items | ||
case .error, .empty: | ||
case .initial, .error, .empty: | ||
return [] | ||
} | ||
} | ||
|
There was a problem hiding this comment.
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.