[Woo POS] Extract cart logic to `CartViewModel` #13191

iamgabrielma · 2024-06-28T05:33:33Z

Description

This is the step 2 of #12998 , here we move Cart-related actions and its handling to a separate CartViewModel.

Its design follows the same pattern as with the product list, we'll observe changes in the model from the dashboard and use these to coordinate and communicate between different subviews. In general:

When the cart is submitted to the next stage of the flow via submitCart(_:), emits the cart items to its subscribers.
When the cart is modified to the previous stage of the flow via addMoreToCart(), emits a signal to its subscribers.
All cart-related logic (add/delete/clear/...) happens internally on the VM.

Changes

Created CartViewModel
Moved cart-related logic to this new VM
Added unit tests

Testing

Run the POS, and observe the product selector and cart interactions behave normally:
- Adding/removing/"clear all" items to the cart works normally
- Tapping on "checkout" submits the order for next stage (totals + payment screens)
- Tapping "add more" brings us back to the product list (after re-loading products from the network) and we can continue updating the order as needed.

There's 3 items that I left outside of this PR, since are not totally related to the refactor itself, and can be tackled separately:

Mocking CartViewModel for preview simplicity: [Woo POS] MockCartViewModel #13207
Rethink how we set the ID for CartItem: [Woo POS] Improve testability: Consider change unique ID for CartItem #13209
Dashboard tests: [Woo POS] Unit tests for PointOfSaleDashboardViewModel #13210

wpmobilebot · 2024-06-28T05:43:17Z

📲 You can test the changes from this Pull Request in WooCommerce iOS by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce iOS
	Build Number	pr13191-e15efe2
	Version	19.3
	Bundle ID	com.automattic.alpha.woocommerce
	Commit	`e15efe2`
	App Center Build	WooCommerce - Prototype Builds #9827

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

…s-extract-cart-vm * task/12998-pos-extract-item-list-vm: (46 commits) Nit updates. Allow setting a different title for the top performers list Update localized string keys and comments Freeze strings for localization Update metadata strings Update release_notes for 19.3 Update CHANGELOG for 19.3 Ensure Google Campaigns list contains top 5 campaigns by total sales Fix linting error Remove unused imports Add unit tests for Google Campaigns analytics card view model Hide campaign image placeholder when card is redacted Update total sales properties Release Notes: add new section for next version (19.4) Update draft release notes for 19.3. Bump version number Hide image placeholder on Google Campaigns card Add view model for Google Campaigns analytics card Add Google Ads Programs web report Update naming for analytics card to display a stat and list of top performers ...

…12998-pos-extract-cart-vm * task/12998-fix-items-not-set-for-order-sync: Fix order sync from passing empty items for the products. # Conflicts: # WooCommerce/Classes/POS/ViewModels/PointOfSaleDashboardViewModel.swift

iamgabrielma · 2024-07-01T08:53:21Z

Only one review is needed.

cc: @staskus just looping you in for awareness, but feel free to review/drop any comment as well 🙇

bozidarsevo · 2024-07-01T12:48:39Z

I see few more // TODO: 12998, will there be more extraction PRs for the PointOfSaleDashboardViewModel?

bozidarsevo

Just that question regarding TODOs 👍

jaclync · 2024-07-01T14:13:50Z

I see few more // TODO: 12998, will there be more extraction PRs for the PointOfSaleDashboardViewModel?

👋 I added those TODO comments, and yes they will be moved/extracted from PointOfSaleDashboardViewModel to a different view model in #13211.

jaclync

Thanks for updating the preview and tests! I added a test case and tested it for 2 transactions again, LGTM I'll merge it shortly to unblock #13211.

jaclync · 2024-07-01T14:10:57Z

WooCommerce/Classes/POS/Presentation/CartView.swift

+    let orderStageSubject = PassthroughSubject<PointOfSaleDashboardViewModel.OrderStage, Never>()
+    let orderStagePublisher = orderStageSubject.eraseToAnyPublisher()


In SwiftUI preview, we could also pass a constant value like passing Just< PointOfSaleDashboardViewModel.OrderStage >(.building) to CartViewModel(orderStage:). We can potentially have two previews, one for each order stage case.

Would like to see what you think about this change, I will merge the PR without updating this 🙂

Thanks for merging, appreciated! 🙇

In SwiftUI preview, we could also pass a constant value like passing Just< PointOfSaleDashboardViewModel.OrderStage >(.building) to CartViewModel(orderStage:). We can potentially have two previews, one for each order stage case.

That sounds good, I've updated #13207 for reference

jaclync · 2024-07-01T14:43:15Z

WooCommerce/WooCommerceTests/POS/ViewModels/CartViewModelTests.swift

+        /* TODO:
+         https://github.com/woocommerce/woocommerce-ios/issues/13209
+         The unique UUID for CartItem is set on init, but CartItem is only internal to addItemToCart()
+         We need to extract this to a separate function and assure that ID's are correct,
+         otherwise the UUID's for testing won't match
+         */


For this test case, I wonder if we could first add some items by using the cart item from itemsInCart. I added a simple test case in e15efe2, feel free to add other edge cases in the TODO issue.

Yes, initially I wanted to test only by id to assure that we remove the exact "line item" if a cart has multiple of the same productID, so if we happen to change the implementation, we wouldn't get a false positive from tests if we happen to have just another item with the same name or property in the list.

That said I think testing it like this and adding the followup is good enough for the moment (better than having no test case!). Thanks for tackling it 🙇

iamgabrielma added 4 commits June 28, 2024 11:12

Create CartViewModel

3a18a4d

Extract addItemToCart

36b6f5c

Extract removal methods

96d2150

Restore isCartCollapsed

5c2ab4e

iamgabrielma added type: task An internally driven task. feature: POS labels Jun 28, 2024

iamgabrielma added this to the 19.4 milestone Jun 28, 2024

Observe cart submissions

6f76016

Extract CartViewModel to own file

bfe2381

iamgabrielma mentioned this pull request Jun 28, 2024

[Woo POS] Split PointOfSaleDashboardViewModel responsibilities #12998

Closed

Base automatically changed from task/12998-pos-extract-item-list-vm to trunk June 28, 2024 19:38

jaclync mentioned this pull request Jun 28, 2024

[Woo POS] Fix order sync from passing empty items for the products #13205

Merged

1 task

jaclync added 4 commits June 28, 2024 16:40

Implement other cart observations to maintain the existing behavior.

d0f890c

Fix cart items still deletable in order finalizing stage.

ed2237b

Hide clear cart CTA in order finalizing stage.

e00e222

jaclync changed the base branch from trunk to task/12998-fix-items-not-set-for-order-sync June 28, 2024 20:50

Mark CartViewModel.orderStage as private.

8c8d6f7

Base automatically changed from task/12998-fix-items-not-set-for-order-sync to trunk July 1, 2024 03:52

iamgabrielma added 4 commits July 1, 2024 11:27

restore CartView preview

b9649d9

Unit tests for CartViewModel

f5b2bfd

Delete cart-related tests from dashboard test suite

a623662

lint

000442f

iamgabrielma marked this pull request as ready for review July 1, 2024 08:31

iamgabrielma requested review from jaclync and bozidarsevo July 1, 2024 08:52

bozidarsevo approved these changes Jul 1, 2024

View reviewed changes

Add a test case on CartViewModel.removeItemFromCart.

e15efe2

jaclync approved these changes Jul 1, 2024

View reviewed changes

jaclync enabled auto-merge July 1, 2024 14:46

jaclync merged commit 432ea08 into trunk Jul 1, 2024
19 of 22 checks passed

jaclync deleted the task/12998-pos-extract-cart-vm branch July 1, 2024 15:01

iamgabrielma mentioned this pull request Jul 10, 2024

[Woo POS] Extract CartViewModel and TotalsViewModel #13179

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Woo POS] Extract cart logic to `CartViewModel` #13191

[Woo POS] Extract cart logic to `CartViewModel` #13191

iamgabrielma commented Jun 28, 2024 •

edited

Loading

wpmobilebot commented Jun 28, 2024 •

edited

Loading

iamgabrielma commented Jul 1, 2024

bozidarsevo commented Jul 1, 2024

bozidarsevo left a comment

jaclync commented Jul 1, 2024

jaclync left a comment

jaclync Jul 1, 2024

iamgabrielma Jul 2, 2024

jaclync Jul 1, 2024

iamgabrielma Jul 2, 2024

		let orderStageSubject = PassthroughSubject<PointOfSaleDashboardViewModel.OrderStage, Never>()
		let orderStagePublisher = orderStageSubject.eraseToAnyPublisher()

[Woo POS] Extract cart logic to CartViewModel #13191

[Woo POS] Extract cart logic to CartViewModel #13191

Conversation

iamgabrielma commented Jun 28, 2024 • edited Loading

Description

Changes

Testing

wpmobilebot commented Jun 28, 2024 • edited Loading

iamgabrielma commented Jul 1, 2024

bozidarsevo commented Jul 1, 2024

bozidarsevo left a comment

Choose a reason for hiding this comment

jaclync commented Jul 1, 2024

jaclync left a comment

Choose a reason for hiding this comment

jaclync Jul 1, 2024

Choose a reason for hiding this comment

iamgabrielma Jul 2, 2024

Choose a reason for hiding this comment

jaclync Jul 1, 2024

Choose a reason for hiding this comment

iamgabrielma Jul 2, 2024

Choose a reason for hiding this comment

[Woo POS] Extract cart logic to `CartViewModel` #13191

[Woo POS] Extract cart logic to `CartViewModel` #13191

iamgabrielma commented Jun 28, 2024 •

edited

Loading

wpmobilebot commented Jun 28, 2024 •

edited

Loading