woocommerce-ios icon indicating copy to clipboard operation
woocommerce-ios copied to clipboard

[Woo POS] Technical: Consider presentation-layer architecture improvements

Open staskus opened this issue 6 months ago • 1 comments

Observations of some of the parts of POS architecture that could be improved after M2.

Description

POS presentation layer is currently split into:

  • PointOfSaleEntryPointView
  • PointOfSaleDashboard (PointOfSaleDashboardViewModel, PointOfSaleDashboardView)
  • Cart (CartViewModel, CartView)
  • ItemList (ItemListViewModel, ItemListView)
  • Checkout (TotalsViewModel, TotalsView)

Issues

Although this split looks sound in principle, we haven't decoupled these parts of POS in practiace.

Dependency on PointOfSaleDashboardViewModel

Although CartView and TotalsView rely on their own view models, they also rely on the main PointOfSaleDashboardViewModel which makes these views tightly coupled on a larger view model.

PointOfSaleDashboardViewModel depends on and exposes smaller view models

PointOfSaleDashboardViewModel depends on CartViewModel, ItemListViewModel, and TotalsViewModel logic. To make PointOfSaleDashboardViewModel testable, we created protocols for smaller view models and started injecting them into PointOfSaleDashboardViewModel.

PointOfSaleDashboardViewModel also exposes CartViewModel, ItemListViewModel, and TotalsViewModel as internal variables which makes part PointOfSaleDashboardView dependant on these particular view models.

Consider at least hiding the dependency of other view models in PointOfSaleDashboardViewModel from the outer world, and exposing any required functionality through its own variables or methods.

TotalsViewModel (Checkout) reuses the state between payments

TotalsViewModel is created at the entry point of POS, and then used in the PointOfSaleDashboardView and TotalsView.

It can create issues the user is presented a state of previous payment or previous order when entering TotalsView. We do some manual clean-up of the view model when beginning a new payment but it is still open to possible issues. We also subscribe to publishers that may return an event from a previous payment if we quickly cancel and start a new payment.

Consider creating a new TotalsViewModel when a new payment starts to indicate a completely fresh start of the payment flow.

staskus avatar Jul 29 '24 12:07 staskus