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

Testing round 1 - feedback #1

Closed
7 of 9 tasks
jorgeatorres opened this issue Sep 8, 2020 · 19 comments · Fixed by #2
Closed
7 of 9 tasks

Testing round 1 - feedback #1

jorgeatorres opened this issue Sep 8, 2020 · 19 comments · Fixed by #2

Comments

@jorgeatorres
Copy link
Member

jorgeatorres commented Sep 8, 2020

Hey everyone!

Thanks for all your hard work on this! 😸
I've been trying out the extension and have some observations, which you'll find below. Some of my teammates will be doing the same soon, so expect a more thorough report once they had a chance to test, but it would be great if we could start looking at/discussing these issues in the meantime.

Thanks!

UX

  • (Settings) Once the e-mail address is entered and saved, it’s no longer possible to enable or disable sandbox mode without performing a full reset. Question: In PPEC you could have separate sandbox/live credentials and switch between those at any point. Is this doable in PayPal for WooCommerce?
  • Question: Do you need a special setup/approval on your account’s side to enable “PayPal Card processing”? If so, can this be checked somehow before allowing this setting to be enabled? Locally, I get an empty box and some console errors when trying to use this option. See screencast.

Checkout

  • (PayPal Credit) Despite not being in the US, I can see the “PayPal CREDIT” button. That even though isEligible() returns false for paypal.FUNDING.CREDIT.
  • [ ] On the Checkout page, when there are other payment options available everytime “PayPal” is selected the page scrolls up for some reason. Tested on Chrome 85.0 (macOS). See screencast.
  • When “PayPal Card processing” is enabled, the form appears in a weird place, at least on the Storefront theme: screenshot.
  • If possible, when “Credit Cards” is selected we should focus on the first card field to make it clear to the user that a form appeared and they still have something to fill out.

Code / environment

  • Plugin author (“Plugin:” header) should be “WooCommerce”.
  • Extension can’t be activated on PHP 7.0 due to various fatal errors. WooCommerce core requires PHP 7.0 but recommends 7.2 (and even warns users about it). If we could support 7.0 that would be great, but 7.2+ is ok as long as activating on 7.0 (which is still valid for WC) doesn’t result in a fatal error but a warning of some kind.
  • Can we remove the repositories section from composer.json as it's linked to private packagist account?
  • There are still some violations of the WooCommerce standards, but code is in overall much better shape in this regard now. Only 3 or 4 source code files seem to require modifications at this point.
  • Namespaced code should probably go into the \WooCommerce or \WooCommerce\PayPal namespace directly, not \Inpsyde. Not critical or high priority.
  • PHP notice logged when activating the plugin on the WC > Settings > Payments screen:

    [04-Sep-2020 17:07:37 UTC] PHP Notice: Trying to get property 'id' of non-object in […]paypal-for-woocommerce/modules/ppcp-wc-gateway/src/class-wcgatewaymodule.php on line 182

cc @websupporter

@jorgeatorres
Copy link
Member Author

jorgeatorres commented Sep 9, 2020

Also:

  • Can we rename the plugin to "PayPal Payments for WooCommerce" (header, slugs, etc.)? This is to prevent people from confusing the extension with "PayPal for WooCommerce" from AngelEYE. We'll handle the repo itself later.

@Chaithi
Copy link
Collaborator

Chaithi commented Sep 10, 2020

Also:

  • Can we rename the plugin to "PayPal Payments for WooCommerce" (header, slugs, etc.)? This is to prevent people from confusing the extension with "PayPal for WooCommerce" from AngelEYE. We'll handle the repo itself later.

Confirmed with Nadia and PayPal team that "PayPal Payments for WooCommerce" is acceptable.

@websupporter websupporter mentioned this issue Sep 11, 2020
10 tasks
@websupporter
Copy link
Member

(PayPal Credit) Despite not being in the US, I can see the “PayPal CREDIT” button. That even though isEligible() returns false for paypal.FUNDING.CREDIT.

Could it be that you have WP_DEBUG enabled and your customer billing address is in the US? In order to check if APMs etc show up correctly, with WP_DEBUG enabled the plugin overwrites the auto-country detection here:

https://github.com/woocommerce/paypal-for-woocommerce/blob/master/modules/ppcp-button/src/Assets/class-smartbutton.php#L683-L688

@jorgeatorres
Copy link
Member Author

jorgeatorres commented Sep 11, 2020

@websupporter: I believe you're right. I didn't know WP_DEBUG had something to do with it, but this is actually a nice addition as we used to use proxies or manually edit the code to simulate our location. Thanks!

@websupporter
Copy link
Member

On the Checkout page, when there are other payment options available everytime “PayPal” is selected the page scrolls up for some reason. Tested on Chrome 85.0 (macOS).

In my tests, the problem does not seem to be the plugin. I activated other gateways and deactivated the PayPal plugin. All options except the first one produced such a "scroll" effect for some reason.

@websupporter
Copy link
Member

When “PayPal Card processing” is enabled, the form appears in a weird place

How about here?
image

@websupporter
Copy link
Member

In PPEC you could have separate sandbox/live credentials and switch between those at any point. Is this doable in PayPal for WooCommerce?

Not at this point. If it is wished for, it can be done though. My thought process was: The user goes through this onboarding process. Ideally, the user never touches the API key and secret (although she can). A simple switch between sandbox and live will therefore produce invalid secret keys at the moment you save. With the reset, the user is forced to go through the process which ensures the correct credentials.

@jorgeatorres
Copy link
Member Author

jorgeatorres commented Sep 11, 2020

@websupporter 👋: Thanks for getting back so quickly!

In my tests, the problem does not seem to be the plugin. I activated other gateways and deactivated the PayPal plugin. All options except the first one produced such a "scroll" effect for some reason.

Got it. I'll add it to my list to investigate further. From the looks of it, it's probably a WC issue then (?).

How about here?

I believe that's much better. Can we also display the PayPal buttons in a similar location for consistency? If that's not too much trouble, that is.

Not at this point. If it is wished for, it can be done though. My thought process was: The user goes through this onboarding process. Ideally, the user never touches the API key and secret (although she can). A simple switch between sandbox and live will therefore produce invalid secret keys at the moment you save. With the reset, the user is forced to go through the process which ensures the correct credentials.

Fair enough. In PayPal Checkout we use a dropdown instead of a checkbox, so at all times there are two "sets of credentials", but I see your point. I'll take note of the difference and we'll revisit later if needed.

@jorgeatorres
Copy link
Member Author

@websupporter: One more question (sorry!). While trying to build the extension from the source code recently we noticed a change in the dhii/containers dependency which forced us to make changes to the source code in order to get the build to succeed. Those changes seem to be related to the ContainerInterface interface.
Are you aware of this or are we missing something? Would you consider setting a specific version for this dependency in the composer.json file to avoid this kind of breakage in the future? (cc @chickenn00dle)

@Chaithi
Copy link
Collaborator

Chaithi commented Sep 11, 2020

Question: Do you need a special setup/approval on your account’s side to enable “PayPal Card processing”? If so, can this be checked somehow before allowing this setting to be enabled? Locally, I get an empty box and some console errors when trying to use this option. See screencast.

Yes, the merchant must be approved (a Webhook is fired when this is done). If they are not approved, then when you load the card fields, it runs an eligibility check and if ineligible, will not load the fields.

@websupporter
Copy link
Member

@Chaithi can the WooCommerce instance listen to those Webhooks or only the platform (which again brings us into the question whether persistence on the platform would be needed, as merchant ids <-> websites would need to be stored so the platform would need to inform the WooCommerce installation).

Currently, the Woo installation sets up its webhooks after receiving the client/secret. This could be too late maybe.

Is there an endpoint where I can get information about what the current client_id/secret is capable of?

Workaround proposal for the moment:
Hide Credit Card option on checkout when ! isEligible (so that a customer can't select it). Maybe an Ajax feedback to the server, so we can store ! isEligible server-side, give feedback to the merchant in admin and prevent the rendering in the first place, until isEligible === true?

@websupporter
Copy link
Member

Regarding the position of the credit card fields. I dug into it. I was not aware of the \WC_Payment_Gateway_CC class. I utilize now this class. This enables me also to rely more on theme styles as now I can expect the theme to have styles for DCC fields ready, which I can try as much as possible to integrate (DCC fields in PayPal are Iframes in the end). Imo, for storefront, this works quite nicely:
image

@mikaey
Copy link
Collaborator

mikaey commented Sep 14, 2020

@Chaithi can the WooCommerce instance listen to those Webhooks or only the platform (which again brings us into the question whether persistence on the platform would be needed, as merchant ids <-> websites would need to be stored so the platform would need to inform the WooCommerce installation).

For that particular webhook event -- that would get sent to the platform, not the merchant...so it would be up to the platform to somehow forward that info on to the merchant's website.

Currently, the Woo installation sets up its webhooks after receiving the client/secret. This could be too late maybe.

Is there an endpoint where I can get information about what the current client_id/secret is capable of?

Unfortunately I don't know of anything that you can call with the merchant's client ID/secret that will tell you whether PayPal Card Processing is enabled on the account. The only thing I do know of is the Show Seller Status API which will show you whether the merchant is subscribed to PayPal Card Processing product.

@websupporter
Copy link
Member

Hi @mikaey, thanks for your feedback! I was playing a bit around with the Show Seller Status API. It appears I can use it with the merchant client id and secret! But with the merchants cridentials I can only get my own information. I tried another merchant ID, but was not authorized. So to me, this seems like a good way, we could go.

{
    "merchant_id": "LYXJCVREHD8J4",
    "tracking_id": "[email protected]",
    "products": [
        {
            "name": "PPCP_STANDARD",
            "vetting_status": "SUBSCRIBED",
            "capabilities": [
                "ALT_PAY_PROCESSING",
                "PAYPAL_CREDIT_PROCESSING",
                "RECEIVE_MONEY",
                "SEND_MONEY",
                "STANDARD_CARD_PROCESSING",
                "VENMO_PAY_PROCESSING",
                "WITHDRAW_MONEY"
            ]
        },
        {
            "name": "PPCP_CUSTOM",
            "vetting_status": "SUBSCRIBED",
            "capabilities": [
                "AMEX_OPTBLUE",
                "CARD_PROCESSING_VIRTUAL_TERMINAL",
                "COMMERCIAL_ENTITY",
                "CUSTOM_CARD_PROCESSING",
                "DEBIT_CARD_SWITCH",
                "FRAUD_TOOL_ACCESS"
            ]
        }
    ],
    "capabilities": [
        {
            "name": "ALT_PAY_PROCESSING",
            "status": "ACTIVE"
        },
        {
            "name": "PAYPAL_CREDIT_PROCESSING",
            "status": "ACTIVE"
        },
        {
            "name": "RECEIVE_MONEY",
            "status": "ACTIVE"
        },
        {
            "name": "SEND_MONEY",
            "status": "ACTIVE"
        },
        {
            "name": "STANDARD_CARD_PROCESSING",
            "status": "ACTIVE"
        },
        {
            "name": "VENMO_PAY_PROCESSING",
            "status": "ACTIVE"
        },
        {
            "name": "WITHDRAW_MONEY",
            "status": "ACTIVE"
        },
        {
            "name": "AMEX_OPTBLUE",
            "status": "ACTIVE"
        },
        {
            "name": "CARD_PROCESSING_VIRTUAL_TERMINAL",
            "status": "ACTIVE"
        },
        {
            "name": "COMMERCIAL_ENTITY",
            "status": "ACTIVE"
        },
        {
            "name": "CUSTOM_CARD_PROCESSING",
            "status": "ACTIVE"
        },
        {
            "name": "DEBIT_CARD_SWITCH",
            "status": "ACTIVE"
        },
....
    ],
....
}

This is the example response of a merchant who can do DCC. My way to verify if I can use DCC is to loop through all products. Check if I am SUBSCRIBED|APPROVED and if so I would loop through the capabilites of the product to see whether CUSTOM_CARD_PROCESSING is entailed in the product or not. Is that a valid assumption?

@websupporter
Copy link
Member

Would you consider setting a specific version for this dependency in the composer.json file to avoid this kind of breakage in the future?

dhii/module-interface has been downgraded to 0.1 (also, 0.2 will not be php7.0) (commit ddd4f5b). The other dependencies are fixed now (b808427)

@websupporter
Copy link
Member

Integrated the sellers api and check if the PayPal account can process DCC before showing the DCC settings. If the account can't process DCC we show a message, otherwise the settings and the merchant can activate DCC for the checkout. (c09e314)

@jorgeatorres
Copy link
Member Author

@websupporter: Thanks for all your hard work on this. I'll be reviewing the PR tomorrow for merging, unless you'd like to add something to it. Let me know!

@websupporter
Copy link
Member

Hi @jorgeatorres, thanks! Ready for review. A lot of files changed (mostly because namespaceing). I tried to keep the commits problem specific and descriptive. I hope that helps in terms of code review :)

@mikaey
Copy link
Collaborator

mikaey commented Sep 17, 2020

Hey @websupporter:

This is the example response of a merchant who can do DCC. My way to verify if I can use DCC is to loop through all products. Check if I am SUBSCRIBED|APPROVED and if so I would loop through the capabilites of the product to see whether CUSTOM_CARD_PROCESSING is entailed in the product or not. Is that a valid assumption?

Yup -- this is absolutely what you'd want to do!

@Lerie82 Lerie82 mentioned this issue Feb 24, 2024
This was referenced Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants