[Woo POS] Make POS aggregate model Observable

[joshheald]

Part of: #14869
Merge after: #15048

Description

This PR makes the POS Aggregate Model @Observable, and switches over to @Environment and @State to hold it.

Observation uses macros and property wrappers to keep track of which views depend on which properties, then only redraw the dependent views when the property's value changes.

Previously, with ObservableObject, any view which used the aggregate model would be redrawn whenever any of its @Published properties changed. This meant that all of the POS was redrawn whenever any published property changed, because the Dashboard view would be redrawn and that would cascade down the heirarchy.

There are no changes to expected behaviour in this PR, and the properties we observe are still mostly backed by Combine publishers.

It may be possible to observe some reduction in redraws of the whole PointOfSaleDashboardView, where redraws begin to localize to the views which depend on something that's changed. This is limited though, because state changes are still frequent and (for example) the Dashboard view still relies on the item list state, so any changes to that state will redraw everything.

Testing information

It's worth testing broad functionality of the POS, checking that it behaves as you expect. This testing doesn't need to be especially deep; there are more PRs to come where we'll get further in to the detail.

---

• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

• The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
• Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
• Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on all devices (phone/tablet) and no regressions are added.

[dangermattic]

	2 Warnings
⚠️	View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.
⚠️	This PR is assigned to the milestone 21.7. This milestone is due in less than 2 days. Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

[wpmobilebot]

📲 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	pr15059-8f44ba8
	Version	21.6
	Bundle ID	com.automattic.alpha.woocommerce
	Commit	8f44ba8
	App Center Build	WooCommerce - Prototype Builds #12810

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

[staskus]

Do you think some of the memory leak issues could get resolved this way? I treated assign as a syntaxic sugar to sink but some of our memory debugger observations are pointing to these assignments 🤔

[joshheald]

Possibly... but probably not. assign(to: on:) keeps a strong reference though, perhaps it's worth looking for that deeper in the codebase?

If you instead implemented MyModel with assign(to: lastUpdated, on: self), storing the returned AnyCancellable instance could cause a reference cycle, because the Subscribers.Assign subscriber would hold a strong reference to self. Using assign(to:) solves this problem.
– assign(to:) docs

[joshheald]

@staskus Replacing the publishers and associated sink calls with simple properties might help (see my other comment) but it probably won't be practical to break it for the Payments code, because that's used right down at the StripeCardReaderService level.

[joshheald]

@staskus @iamgabrielma @jaclync only requires one review, but I want to tag all of you in these changes for visibility.

Future work on the cards:

• [Woo POS] Use Observation to fix refresh issue #15060 removes the workaround for refreshing, without reinstating the error that it was put in place to solve.
• To follow that, I'll remove the .loading state from pull-to-refresh, which actually causes the issue in the first place. It doesn't make sense to show the ghost cell at the bottom of the list during PTR, in my opinion. That simplifies things further, but it's useful to have fixed it without that for our own learning.
• Finally, replace as many .sink blocks in the aggregate model as I can – we should just be able to use simple properties, and can in most cases remove the reliance on publishers.

[staskus]

Thanks! I'm happy will be using the most modern recommended APIs 👍

I read the migration docs and the changes follow the plan.

I also tested products, variables products, floating bar actions, simple product alert, reader connection/disconnection, card payment, cash payment, email receipts, and a couple of error states. It all works as expected. I only noticed the detail you mentioned about ghost cells visible when doing the pull to refresh.

[staskus]

This is limited though, because state changes are still frequent and (for example) the Dashboard view still relies on the item list state, so any changes to that state will redraw everything.

Do you think we should do anything about this? For example, rework the item list state so it could be observed more granularly. Or is it not worth it?

Edit: Answered in this PR #15059 as well. I'm glad we can use @Observable for it.