-
Notifications
You must be signed in to change notification settings - Fork 69
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
Streamline checkout classes #7869
Conversation
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
Size Change: 0 B Total Size: 1.27 MB ℹ️ View Unchanged
|
9239f77
to
c239163
Compare
… and increase test coverage
21d1935
to
a9eb6b2
Compare
2991b22
to
7ceff11
Compare
3cf2d34
to
ce1de59
Compare
bc4be8f
to
3ea8609
Compare
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.
Everything tests well per the instructions. Just one question regarding the removal of wcpayConfig
from the checkout class. Otherwise, it's really nice to see a single clean checkout class!
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 again for working on this! 🚢
Merging due to reduced team capacity, and the PR still has one approval. |
Closes #7783
Contributes to #6882 as per first bullet point here
Changes proposed in this Pull Request
This PR merges two checkout classes into one
Since the latest UPE rollout, only
WC_Payments_UPE_Checkout
was used. It also used parent class methods in some places.As a result, in this PR
WC_Payments_Checkout
which was used inWC_Payments_UPE_Checkout
was moved intoWC_Payments_UPE_Checkout
WC_Payments_Checkout
was removedWC_Payments_Checkout
was removed, as the previous two points led to this class being emptyWC_Payments_UPE_Checkout
was renamed toWC_Payments_Checkout
to reduce complexity and improve readabilityAnother significant step forward was the unit tests refactoring and extension. Multiple dependencies to the checkout class within the gateway test suite made the tests mock their own units under test. Checkout classes were used in unit tests for gateways, where we directly or indirectly tested their implementation from the gateway level, which is wrong. This PR improves this and to confirm this, you might try to search for
WC_Payments_Checkout
in test files and find that it's being used only in the test suite which is supposed to test checkout. Also a few new test cases were added totest-class-wc-payments-checkout.php
.Changes and testing instructions
save payment method
checkbox is displayed for logged in user for card PM and is not displayed for other, non reusable PMsnpm run changelog
to add a changelog file, choosepatch
to leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge