Skip to content
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

[Custom Payment UI] IPP refactoring — collective PR (Parts 1-5) #13014

Merged
merged 182 commits into from
Nov 29, 2024

Conversation

samiuelson
Copy link
Collaborator

@samiuelson samiuelson commented Nov 27, 2024

This is a collective PR intending to merge IPP refactoring (part of Custom Payment UI project) into trunk.

This PR consists of the 5 PRs:

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

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

💡 The above PRs have been reviewed and thoroughly tested, including an internal call for testing (pdfdoF-5Rg-p2) that verified that refactoring doesn't introduce regression to existing IPP flow.

Description

As explained in the PRs linked above, the goal of the refactoring is to allow observing the payment state inside the POS screen (without showing the existing dialog-based UI), in order to allow displaying custom payment UI directly in the POS Totals screen.

Steps to reproduce

Testing information

The testing plan is to verify the refactoring doesn't introduce regression to the existing IPP flows, both in store management and POS parts of the app. This was done when smoke testing the partial PRs listed above and during the call for testing.

The tests that have been performed

  1. Manual testing of the existing IPP Payment flow, in different configurations, during the call for testing (pdfdoF-5Rg-p2)
  2. Manual testing of the Payment flow in POS

Images/gif

This is purely refactoring work; no visual changes were made.

  • 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.

samiuelson and others added 30 commits October 30, 2024 19:37
…tController`

The goal is to remove viewStateData property and map paymentState to ViewState in the target [ViewModel]
The goal is to make payment controller return UI-agnostic payment state and map the payment state to UI state in the target VM.
@samiuelson samiuelson added this to the 21.3 milestone Nov 27, 2024
@dangermattic
Copy link
Collaborator

dangermattic commented Nov 27, 2024

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

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 27, 2024

📲 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
FlavorJalapeno
Build TypeDebug
Commit0d4a874
Direct Downloadwoocommerce-wear-prototype-build-pr13014-0d4a874.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 27, 2024

📲 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
FlavorJalapeno
Build TypeDebug
Commit0d4a874
Direct Downloadwoocommerce-prototype-build-pr13014-0d4a874.apk

@samiuelson samiuelson changed the title Feature/custom payment UI [Custom Payment UI] IPP refactoring — collective PR Nov 27, 2024
@samiuelson samiuelson added the feature: mobile payments Related to mobile payments / card present payments / Woo Payments. label Nov 27, 2024
@samiuelson samiuelson added the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Nov 27, 2024
@samiuelson samiuelson marked this pull request as ready for review November 27, 2024 12:17
@samiuelson samiuelson changed the title [Custom Payment UI] IPP refactoring — collective PR [Custom Payment UI] IPP refactoring — collective PR (Parts 1-5) Nov 27, 2024
[POS] Custom payment UI — Part 5 | Unit tests clean up
@samiuelson samiuelson removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Nov 28, 2024
Copy link
Contributor

@kidinov kidinov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested IPP with real and simulated reader. TTP with simulated reader. No issues found

@codecov-commenter
Copy link

codecov-commenter commented Nov 29, 2024

Codecov Report

Attention: Patch coverage is 95.35334% with 48 lines in your changes missing coverage. Please review.

Project coverage is 40.13%. Comparing base (6062426) to head (0d4a874).
Report is 183 commits behind head on trunk.

Files with missing lines Patch % Lines
.../payment/controller/CardReaderPaymentController.kt 93.11% 14 Missing and 21 partials ⚠️
...s/cardreader/payment/CardReaderPaymentViewModel.kt 84.31% 7 Missing and 1 partial ⚠️
...payment/CardReaderPaymentStateToViewStateMapper.kt 97.89% 2 Missing and 2 partials ⚠️
...eader/payment/controller/CardReaderPaymentEvent.kt 88.88% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #13014      +/-   ##
============================================
+ Coverage     39.74%   40.13%   +0.38%     
- Complexity     6044     6119      +75     
============================================
  Files          1274     1280       +6     
  Lines         73485    74006     +521     
  Branches      10080    10123      +43     
============================================
+ Hits          29207    29701     +494     
- Misses        41699    41725      +26     
- Partials       2579     2580       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@samiuelson samiuelson merged commit 4833cd3 into trunk Nov 29, 2024
15 checks passed
@samiuelson samiuelson deleted the feature/custom-payment-ui branch November 29, 2024 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: mobile payments Related to mobile payments / card present payments / Woo Payments. unit-tests-exemption
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants