-
Notifications
You must be signed in to change notification settings - Fork 136
[POS] Custom payment UI — Part 4 | Switching from viewStateData to paymentState
#12898
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
Conversation
Generated by 🚫 Danger |
viewStateData to paymentStateviewStateData to paymentState
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/custom-payment-ui #12898 +/- ##
===============================================================
- Coverage 40.65% 40.16% -0.50%
- Complexity 5808 6073 +265
===============================================================
Files 1251 1274 +23
Lines 71210 73783 +2573
Branches 9937 10089 +152
===============================================================
+ Hits 28952 29635 +683
- Misses 39624 41580 +1956
+ Partials 2634 2568 -66 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
viewStateData to paymentStateviewStateData to paymentState
...oocommerce/android/ui/payments/cardreader/payment/CardReaderPaymentStateToViewStateMapper.kt
Outdated
Show resolved
Hide resolved
malinajirka
left a comment
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.
Thanks @samiuelson ! Overall, it LGTM. I've left a couple questions. As with the previous PR, I just smoke tested the flow - I haven't ran thorough tests as I assume we'll run those anyway on the final PR of this chain of PRs.
This way we can avoid not-null assertion operator (!!) in prod code.
|
Version |
This way we can avoid not-null assertion operator (!!) in prod code.
Switching from
viewStateDatatopaymentStateCloses: #12825
Description
This PR:
val viewStateData: LiveData<ViewState>fromCardReaderPaymentControllerCardReaderPaymentController.paymentStatetoViewStateinCardReaderPaymentViewModel, using theCardReaderPaymentStateToViewStateMapperuse case classThe goal of that PR is to make the payment controller independent of the
ViewStateclass, and extract mapping of theCardReaderPaymentOrRefundStateemitted by controller toViewState(UI state of the existing IPP flow) in the target VM. Thanks to such separation, the controller will allow flexible reuse of the payment flow in the POS.This is part 4 of 5 PRs refactoring the Payment flow:
Next steps
I'll add unit tests for
CardReaderPaymentController, in the next PR.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
I tested the payment collection flow in both POS and store management modes, using card-present payment and TTP; verified that the IPP flow works and is not changed.
I wasn't able to test Interac payments/refunds.
The build from this PR will be used for call for testing – extra manual testing focused on regression.
RELEASE-NOTES.txtif 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: