[POS] Custom Payment UI — Part 3 | Emitting UI–agnostic payment states in `CardReaderPaymentController`

samiuelson commented 2 weeks ago

Closes: #12825

This is part 3 of 5 PRs refactoring the Payment flow:

[POS] Custom payment UI — Part 1 | Separating payment collection management from Android Framework
[POS] Custom payment UI — Part 2 | Separating CardReaderPaymentController's events from MultiLiveEvent
[POS] Custom Payment UI – Part 3 | Emitting UI–agnostic payment states in CardReaderPaymentController
[POS] Custom payment UI — Part 4 | Switch from viewStateData to paymentState
[POS] Custom payment UI — Part 5 | Unit tests clean up

⚠️ Don't merge — the branch will be merged together with the above ones after additional testing of the whole refactor.

Description

This PR introduces a new class modeling a "payment state" — CardReaderPaymentOrRefundState. It is emitted by the paymentState: StateFlow<CardReaderPaymentOrRefundState> property in CardReaderPaymentController, allowing observing the payment state in POS.

The goal of the paymentState property is to replace the viewStateData: LiveData<ViewState> that emits "UI model" instances, designed specifically for the existing dialog-based payment flow UI. For now, the viewStateData is marked deprecated, and supposed to be used only by the existing IPP flow.

Coming up in the next PR:

Removing viewStateData from the controller
Mapping the controller's paymentState to ViewState in CardReaderPaymentViewModel
Covering controller with unit tests

Testing information

As a result of the refactoring done within this PR, the app should work without any change. It's crucial to test the IPP flow in the store management and POS modes against regression. It may be useful to base on the test plan (pdfdoF-5Jz-p2).

The tests that have been performed

This PR will be followed up by removing the viewStateData property from the controller, adding unit tests, and mapping the paymentState to ViewState. That said, this PR should not cause any regression in the existing IPP flow, however, the most important part to test is the code changes.

[ ] I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

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 big (tablet) and small (phone) in case of UI changes, and no regressions are added.

dangermattic commented 2 weeks ago

	3 Errors
:no_entry_sign:	Please add tests for class `CardReaderPaymentOrRefundState` (or add `unit-tests-exemption` label to ignore this).
:no_entry_sign:	Please add tests for class `CardReaderPaymentState` (or add `unit-tests-exemption` label to ignore this).
:no_entry_sign:	This PR is tagged with `status: do not merge` label(s).

	1 Warning
:warning:	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by :no_entry_sign: Danger

wpmobilebot commented 2 weeks ago

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

	App Name	WooCommerce-Wear Android
	Platform	⌚️ Wear OS
	Flavor	Jalapeno
	Build Type	Debug
	Commit	0c4e9ecd9955fdb198590ec6794774b89e46c450
	Direct Download	`woocommerce-wear-prototype-build-pr12877-0c4e9ec.apk`

wpmobilebot commented 2 weeks ago

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

	App Name	WooCommerce Android
	Platform	📱 Mobile
	Flavor	Jalapeno
	Build Type	Debug
	Commit	f75681960fa13d9abce0c6c511387cb6f39fbea4
	Direct Download	`woocommerce-prototype-build-pr12877-f756819.apk`

samiuelson commented 1 week ago

Thanks for the reviews @kidinov and @malinajirka 🙇🏼

As discussed, we're going to keep the callbacks in the CardReaderPaymentOrRefundState. Also, the mapping of CardReaderPaymentOrRefundState to ViewState, and removing viewStateData from controller (bringing the controller to the final shape) is ready for review in the next PR: [POS] Custom payment UI — Part 4 | Switch from viewStateData to paymentState.

In the current PR, I extracted nameForTracking from the payment state model (c26ed71, 7fe2991) and moved the logic of the ::trackCancelledFlow function into a use case class (44115aa).

Let me know what you think!

samiuelson commented 2 days ago

Thanks for review, @malinajirka!

I haven't run thorough tests since I believe we'll run them on the final PR of this chain of PRs anyway.

That's correct. The build used in the Call for testing (pdfdoF-5Rg-p2) is generated from the next PR that includes payment state to UI state mapping — [POS] Custom payment UI — Part 4 | Switching from viewStateData to paymentState.

woocommerce / woocommerce-android