-
Notifications
You must be signed in to change notification settings - Fork 219
Add support for extensions to filter express payment methods #4774
Conversation
Size Change: -25 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
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.
This works as advertised 👏🏼 Just a query about the tests. From simply reading the tests, it's not super clear what we're testing, or why we're expecting certain things... I would love it if you added inline comments with an explanation about why we're expecting the result.
assets/js/blocks-registry/payment-methods/payment-method-config-helper.ts
Show resolved
Hide resolved
args.features, | ||
args.paymentMethodName | ||
)( canMakePaymentArgument ); | ||
expect( canMakePayment ).toBe( falseCallback() ); |
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.
Doesn't falseCallback()
evaluate to simply false
, so you're testing here expect( canMakePayment ).toBe( false );
or am I misunderstanding?
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.
I've updated the test in 630a15e, hope this makes it more clear
d5b7a4a
to
832814d
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.
Thanks for making the changes and describing the tests better! 🙌🏼
This PR adds support for extensions to also filter express payment methods.
Fixes #4733
Testing
Automated Tests
Manual Testing
How to test the changes in this Pull Request:
Changelog