Automattic / woocommerce-services

WooCommerce Services is a feature plugin that integrates hosted services into WooCommerce (3.0+), and currently includes automated tax rates and the ability to purchase and print USPS shipping labels.
GNU General Public License v2.0
107 stars 20 forks source link

Payment methods are not fetched again when we buy shipping label #2585

Open harriswong opened 1 year ago

harriswong commented 1 year ago

Description

When the user interacts with the "Buy shipping label" button, we proceed with the purchase by retrieving the payment method. This payment method is stored in the options table under the wc_connect_options name. image.

While on this page, if the user deletes their payment method from their WP account and then proceeds with purchasing the label without refreshing the modal, then WCS&T will continue to use the payment method from wc_connect_options. The modal does not recognize the account has changed. This can result in the client purchasing labels with a deleted card.

This situation will not happen if the merchant refreshes the order page or loads the WCS&T settings page (ie. http://localhost/wp-admin/admin.php?page=wc-settings&tab=shipping&section=woocommerce-services-settings). The reason is that refreshing the order page will trigger WCS&T to check if we should show the WCS&T meta_box. This check automatically pulls the latest payment method info and will refresh the data. When we visit the order settings page, we would trigger an AP call to the connect settings endpoint (ie. http://localhost/wp-json/wc/v1/connect/account/settings) which would also refresh the data.

Looking at the code, it seems like using the locally stored connect options is the expected behaviour. The question is, do we want to continue using the connect option to cache these payment methods?

Possible solutions

  1. Won't fix. Update documentation to ask users to refresh the account settings pages before buying label.
  2. Update code so that every time we click "Buy shipping label", we pull the latest payment method from WP. This will ensure we are always up-to-date with the user's WP account payment method. We will increase calls to the WP and may impact performance.
  3. (not sure if this is possible) Update code so that we stop caching payment method in WCS&T, instead, move the logic of "finding the right payment method" into the connect server. Update connect server to always use/find the latest payment method. This way, WCS&T is decoupled from the payment method logic and updating payment methods in WP accounts will not impact WCS&T.
bartech commented 1 year ago

IMHO solution nr 2 looks most straight forward. Solution nr 3 is similar or even exactly the same in terms of logic just on another env.

We have to perform:

And good place to do it is wp/woo itself. We have all methods and tools to refresh payment method, check payment method validity and display nice notifications. Doing validation closest to the user is often the best.

As to concerns to do extra query. We already sending purchase label action to WCS&T, we just have to add payment method validation before we perform purchase action. No additional call to wp is required. And this is done only by store owners, so there won't be many of those performed anyway. If we are not worried doing a call to wordpress.com to get live payment method every time we load order edit screen then we should be even less worried when we do same call on label purchase action.

If we want to solve this on the connect server (option 3) it's only viable if whole chain of validating to displaying error message in Woo store is already provided.

harriswong commented 1 year ago

+1 on fixing this issue with 2).

That said, I do wanna add some more comments to elaborate on why I think 3) could be a good choice.

WCS&T has the functionality to allow merchants to switch to their preferred payment method when purchasing the labels. image

This feature makes storing WordPress.com's payment method ID a requirement. We need to store the ID either in WCS&T or the connect server.

Currently, WCS&T purchases a label from connect server by passing through the label details and the payment details. With 3), what we could do is to pass in just the label details. Since payment details aren't controlled by individually installed WCS&T, connect server can have more control over when to refresh the stored payment method ID. This could also open up opportunities to leverage potential webhooks (if any) from WordPress.com to update payment methods when a card deletion action is detected, etc. The turnaround time between connect server and WordPress.com should also be less compared to a hosting provider with WordPress.com. This can be another topic inside the connect server.

Besides the preferred payment method feature I mentioned above, I can't think of any other reasons why we want to store the payment method IDs inside WCS&T. If there is no need for this, 3) could be a good option where we don't need to implement a "payment store" inside WCS&T and connect server can have more control over label's payment method instead.