Closed jorgeatorres closed 3 years ago
This is ready for review now 😼.
I didn't make any progress towards a less cluttered settings screen but that's something we can try to tackle in a different PR.
@jorgeatorres this is looking really good from my testing and code review 💯
Test the upgrade routine: After upgrading to this branch, PayPal Credit Messaging wasn't visible on the checkout which is good, but when I viewed the settings it shows the credit messaging as enabled? Is this correct?
Frontend display settings
I just tested a few combinations of these to make sure everything was fine, however, I noticed some things were off. I don't think these are issues with his PR and more on PayPal's end but I'd like to get your opinion :)
I've attached some screenshots of my results just in case you wanted to double-check them as well
[ ] Layout: Text
[x] Graphics
[x] Credit messaging with different styles/settings per context (i.e. turned on for product pages, turned off for cart page, different styles chosen for checkout vs product pages)
Checkout testing
vault
param is not set when a simple product is in cartamount
) even if the cart doesn't contain a subscription or viewing a simple productvault
script arg is set to true when a subscription is in cartvault
script is only set to true when viewing a subscription productSome possible issues I found that need confirmation
To replicate:
$script_args['buyer-country'] = 'US';
trunk
, view the same subscription and notice the credit option is available.credit-bnpl
branch and view the checkout with a subscription product in your cart and the checkout is showing the PayPal credit optionHi @mattallan 👋,
Thanks for the review and for testing all the various scenarios 😸 . Answers/comments below.
After upgrading to this branch, PayPal Credit Messaging wasn't visible on the checkout which is good, but when I viewed the settings it shows the credit messaging as enabled? Is this correct?
It's not, but I was unable to replicate 🤔 . Did you do the whole setting WC_GATEWAY_PPEC_VERSION
to 2.0.4
thing?
Reloading the page to trigger the upgrade routine afterwards should set the "global" and all context-specific "Enable PayPal Credit messages" settings to "no", so in theory it shouldn't be enabled on the settings screen at all.
If you already went through this step, could you try again just to confirm? You'd have to set wc_ppec_version
to 2.0.3
in the options table first; otherwise the upgrade routine won't run.
I just tested a few combinations of these to make sure everything was fine, however, I noticed some things were off. I don't think these are issues with his PR and more on PayPal's end but I'd like to get your opinion :)
I see the same as you but I'm positive this was working before. Maybe it's related to the rollout of the new "Buy Now, Pay Later" feature or maybe it's something they broke in the process 🤷. During these days of development I've seen things work on and off: errors logged to console, banners appear/disappear, isEligible()
checks return false
even though the BNPL or Credit buttons appear, etc. I'm going to report my findings to PayPal but I don't see how could we address this on our end.
That said, I believe I managed to "isolate" the problem with the logo position, or at least what's causing it. It seems to be related to the amount
attribute. I've created a very simple JSFiddle that illustrates the problem: the first banner (with an amount of 500.00) renders correctly while the second one (with an amount of 400.00) doesn't. The "configuration" is exactly the same for both. As soon as you change the amount to a big enough number, it renders properly. (See screencast).
If you were testing on product pages with relatively small prices or with Subscriptions active (remember that in this case amount=''
), that would "explain" the issue.
On this branch, when I view a subscription product page it doesn't show any credit/paylater button, but it does show on the checkout page when i have a subscription in the cart 🤔
This seems to be related to the commit=true
parameter that gets passed to the SDK script on checkout (but not on single product or cart pages). The SDK docs say this about this parameter: "set to true if the transaction is Pay Now, or false if the amount captured changes after the buyer returns to your site. Not applicable for subscriptions." So I believe on subscription product pages the combination of vault=true
and commit=false
results in Credit not being offered as an option. I'm going to ask PayPal about this, but vault=true
was a requirement from them. Maybe this behavior is "intended" 😕.
This is probably related to the above issue but when I view a simple WC product I see the Pay Later button (https://d.pr/i/02UHD2). But when I view the same product with a subscription in my cart, I see the Credit button (https://d.pr/i/ZbMVbH).
Similar to the above, it seems vault=true
implies you get the Credit button instead of the BNPL one. Asking PayPal about this too, but also seems outside of our control.
Once again, thanks for your review and testing 💯. I'll let you know as soon as I hear back from PayPal. In the meantime, do let me know if you see some other things we can improve or fix.
BTW I've rebased on trunk
to add to this branch the fixes that were recently merged.
Ok. After speaking with PayPal I have some answers:
vault=true
) at the moment so it is expected that when subscriptions are involved, the button is Credit not "Pay Later".Let me know if things are now looking more stable on your end @mattallan.
BTW I've rebased on trunk to add to this branch the fixes that were recently merged.
Thanks! I also did a rebase locally before testing yesterday as well 😅 💃
It's not, but I was unable to replicate Did you do the whole setting WC_GATEWAY_PPEC_VERSION to 2.0.4 thing?
Oh, I'm sorry!! I was testing this upgrade process wrong 😞 I've just re-checked with the correct process and it's looking all good from my end.
I've created a very simple JSFiddle that illustrates the problem. The problem with the logo style "alternative" and position "top" or "right" seems to be a bug
WOW! I don't know how you found this, but yes that "explains" the issue I'm seeing. Thanks for confirming this with PayPal and it's good to know they're aware of it :+1:
As for neither Credit nor Pay Later appearing on subscription products it also seems to have been a temporary glitch. I now see the "Credit" button as expected
Hmm okay, I've tried re-testing this and it's still not showing any PayPal Credit buttons on subscription product pages. But if it's working for you I'm okay to merge this. It definitely feels a bit of inconsistency on PayPals end with rolling things out, but I've double-checked the code and it looks all good from our end, so nothing we can really do about it.
It sounds like every issue I reported has a reason for it happening and it's either working as intended or needs fixing on PayPal's side.
So with that.. LGTM :D
Description
This PR adds PayPal Credit messaging support to the PayPal Checkout plugin.
These messages are displayed in the form of banners with text/graphics promoting special offers from PayPal Credit. They are meant to be displayed in several places including home and/or category pages*, as well as alongside PayPal (Credit) buttons on the product, cart and checkout pages.
These are some examples of how these banners can look like:
(*) Per pb0GrA-Ud-p2, this PR doesn't include support for displaying the banners on home/category pages.
Steps to test:
credit-bnpl
).Test basic functionality:
There should be a new "Enable PayPal Credit message" setting and a section of related settings (as in the screenshot below) both globally and per context (single product & checkout, if context specific settings are enabled).
woocommerce_ppec_paypal_settings
row in the options table, and save it in a safe place.wc_ppec_version
andwoocommerce_ppec_paypal_settings
from the options table.woocommerce_ppec_paypal_settings
in the options table with the value you saved before.WC_GATEWAY_PPEC_VERSION
inwoocommerce-gateway-paypal-express-checkout.php
to2.0.4
.Documentation