woocommerce / woocommerce-gateway-stripe

The official Stripe Payment Gateway for WooCommerce
https://wordpress.org/plugins/woocommerce-gateway-stripe/
234 stars 206 forks source link

Use cart totals for Payment Request Button in the product page #1563

Open ricardo opened 3 years ago

ricardo commented 3 years ago

Related issues for context:

Because we allow users to start the payment request flow from the product page, the total price and displayItems for the Payment Request Button are calculated in three different ways in WC_Stripe_Payment_Request. Two of these ways are for the product page.

This has some heavy downsides:

How to fix this:

Essentially, the idea to fix this is to calculate totals and labels after the PRB is clicked, through some async request after the product is added to the cart.

We can wait for the product to be added to the cart and then trigger another request for ajax_get_cart_details to always get the same price and labels, either from the product page or from the cart or checkout pages.

Of course, this extra request is a UX tradeoff, which we need to find a way to address. Here are some ideas:

  1. Add loading indicators and block the PRB once it's clicked, then make the request for the prices, then open the PR dialog.
  2. Find a way to add loading indicators to the native PR dialog instead of the button. For instance, Apple Pay already has a native loading state for the Apple Pay dialog right after you click the button, or before the payment is confirmed.

Other ideas we could include in this fix:

helgatheviking commented 3 years ago

👀 following

Since it's fairly easy to get the price for a simple product, and I believe that's the majority of the products, we could perhaps bypass this extra request for certain types of products.

FYI, for Name Your Price.. I would still need to get the price from the cart even for a simple product. (product addons could be impacted by this too)

For Mix and Match, getting the cart would possibly solve everything!

ricardo commented 3 years ago

@helgatheviking thanks for the note. We will probably need to get the price from the cart in any scenario.

asumaran commented 3 years ago

I'm going to split this issue in multiple parts. I'll submit separate PRs for every product type so it's easier to review and test.

I'll start testing with simple products and then the other product types we support (Variable Products, Subscriptions, etc). After that I'll start working on support Mix and Match, Composite, etc.

asumaran commented 3 years ago

Closed by mistake. Work is still pending to fully address this issue.

nawaz0705 commented 2 years ago

We got a user in Happychat #32673388 who is reporting the similar issue. They reported this issue back on January 28, 2021, and now they want to know if we have any update regarding this.

This is the bug report created in favor of them which is merged, but the user is still facing the issue on their website. They're using updated version of WooCommerce, WooCommerce Subscriptions and WooCommerce Stripe Gateway plugin. Any updated regarding this would be highly appreciated!

Updated Ticket: #4613324

@asumaran

aheckler commented 2 years ago

Also related: https://github.com/woocommerce/woocommerce-gateway-stripe/issues/1745

aaronsilber commented 2 years ago

Hi @asumaran - we are still experiencing this issue. Do you or the WC team have any update on when this issue is planned to be addressed?

helgatheviking commented 2 years ago

Since WooCommerce doesn't provide a way to retrieve the product price through JavaScript, we need to manually calculate the price based on quantity, variations and other variables on the front-end.

Thinking about this again... both my plugins calculate/determine the price on the front-end already. If there was a standardized way for us to tell the PRB script what that amount is we could handle compatibility on our side. I did this with the old WooCommerce PayPal Payments plugin.... which pulled price from some kind of data attribute. On my side, I updated that attribute when my product's config was valid and then PayPal knew what price to use.

In this way we could bypass needing a call to the Cart API (or some other async request).

Would need 2 things:

  1. ability to enable/disable the PRB as product is/not valid
  2. ability to dynamically accept a price... via data attribute or equivalent.

Downside is this would mean our plugins would need to add compat on our side, but this is doable if there's a standardized approach.

asumaran commented 2 years ago

@aaronsilber unfortunately, we won't be able to deliver this soon. We've found multiple issues while implementing this approach.

The original approach doesn't work on Safari and the workaround will require major implementations in WC core (like the ability to create multiple carts). So, to answer your question: No, there's no updates to share regarding this issue.

helgatheviking commented 2 years ago

@asumaran Rethinking this a bit today and I don't think we have to get as complex as separate carts. If we look at the current ajax add to cart.... I think the biggest barrier to compatibility is that only some form data is serialized and posted.... it looks like only the variation attributes and hard-coded support for product addons.

And in the ajax callback the 5th parameter for add_to_cart() is ignored... though I'm not sure how necessary this is if the form data is in the $_POST.

Both of my plugins are hooking in to the wc()->cart->add_to_cart() function (at woocommerce_add_cart_item_data ) and listening for my $_POST variables. If they existed in the $_POST I think my plugins my automatically set the right price here via wc()->cart->calculate_totals()

So I am thinking we should start by relaying all the form fields? Does that make sense to you?

helgatheviking commented 2 years ago

As an update... the above changes get the correct price into the cart! But in the gpay modal they are still displaying the wrong price.... 🤔

helgatheviking commented 2 years ago

Apologies for the slightly stream of consciousness... I have it about 50% of the way I think... see demo site

If you enter a price and then tab out or click outside (this part is not complete but based on current Name Your Price functionality where validation doesn't happen until the .change() event).

You should see the payment request buttons blocked and unblocked (and some stuff happening in the console). After that if you click on the request button the price and total is the price that was entered in the price input.

The main things that are happening are:

  1. Add an custom trigger that will fire wc_stripe_payment_request.getSelectedProductData() and update the payment request object. NYP then calls this via $( document.body ).trigger( 'wc_stripe_update_selected_product_data' ); when the form is valid.
  2. include all the form inputs in the data that is sent them to the ajax callback.
  3. Convert this back into the global $_POST in ajax_get_selected_product_data()
  4. Introduce a filter for the ajax_get_selected_product_data() json response

What's missing still is that (at least for Name Your Price), you can enter a price and then click on the request button and the ajax request update will not have fired yet. Might need a way to add my own error notices when the button is clicked... or a way to disable the button entirely until the NYP price is valid. I tried

$( document.body ).trigger( 'wc_stripe_block_payment_request_button' );

But it's not clear to me yet why that's not working... the request button is still clickable.

On the NYP side I have a filter to modify the request data the ajax function returns:

/**
 * Update request response.
 *
 * @param   array $data
 * @param   obj WC_Product
 * @return  array
 */
function wc_nyp_add_nyp_response( $data, $product ) {

    if ( class_exists( 'WC_Name_Your_Price_Helpers' ) && WC_Name_Your_Price_Helpers::is_nyp( $product ) ) {
        $posted_price = WC_Name_Your_Price_Helpers::get_posted_price( $product );
        $data['displayItems'][0]['amount'] = WC_Stripe_Helper::get_stripe_amount( $posted_price );
        $data['total']['amount'] =  WC_Stripe_Helper::get_stripe_amount( $posted_price );
    }
    return $data;
}
add_filter( 'wc_stripe_payment_request_get_selected_product_data', 'wc_nyp_add_nyp_response', 10, 2 );

lemme see if I can start a PR....

asumaran commented 2 years ago

@helgatheviking I was planning to respond with a much larger comment which I'll do anyway later/tomorrow. But for now I'd suggest you to try your demo on Safari. Also, the product data (amount, requires shipping?, etc) must be added to the cart and come from the backend that way we ensure that plugins that modify the product (e.g. the price) will work with PRBs.

helgatheviking commented 2 years ago

@asumaran I don't have Safari so that's going to be tough for me. But the product data will be coming from the backend.. and the price is reflected correctly in the cart if I cancel the checkout and go look at the cart.

By relaying the custom form fields in the global $_POST plugins can then filter the price and/or the response via the newly added filters. Also by using the $_POST the cart works semi-automatically since NYP was always listening to the posted data to set the cart price.

It's a work in progress. Hopefully a proposal that might jumpstart a fresh approach to this problem.

asumaran commented 2 years ago

@helgatheviking, sorry for the late response. I'll try to respond to your comments. Also, I will try to explain why as of today we failed to deliver this new approach.

Thinking about this again... both my plugins calculate/determine the price on the front-end already. If there was a standardized way for us to tell the PRB script what that amount is we could handle compatibility on our side.

With the new approach, we don't want to get the product's data from the frontend. There are many issues with getting the right amount the user has to pay using PRBs. We figured out that the best and only way to get the most accurate product information is to add the product to the Cart and get the payment information from there. This will allow us to take coupons, taxes, any product type, into consideration as well as 3rd party plugins that might update the Cart.

we could bypass needing a call to the Cart API (or some other async request).

We do want to make the async call to the backend (to the Store API most likely) for the reasons mentioned above.

If you enter a price and then tab out or click outside Name Your Price functionality where validation doesn't happen until the .change() event).

I think this is particular case this issue might not address. Although, I think there will be still issues with 3rd party plugins (e.g., with a plugin that applies a discount for $200 worth of products added to the Cart). Also, how do you apply taxes? discounts?

I think https://github.com/woocommerce/woocommerce-gateway-stripe/issues/1563#issuecomment-1144083690 is a legitimate need that can be addressed without the new approach we want to implement here.

--

Now, I will explain the problems we faced while working on this issue. I think some of them also apply to your suggestions on https://github.com/woocommerce/woocommerce-gateway-stripe/issues/1563#issuecomment-1144083690

The new approach is straightforward. It can be summarized as follows:

  1. User visits a product page.
  2. User clicks the PRB button.
  3. Stop event propagation, then make an async request to add the product to the Cart.
  4. Apply the product amount/info to the PRB.
  5. Programmatically open the payment sheet with the correct product information.
  6. User makes the payment.

The problem with this approach happens in step 3. This works fine on Chromium-based browsers without issues but fails on Safari (Apple Pay). We discovered that the maximum time the browser would wait until we programmatically open the payment sheet should not be over 1000ms (probably because of this). Past this time, Safari will throw an error.

We could not ensure the async request would take less than 1 second to finish. We decided not to follow this approach.

We tried multiple things, but the only acceptable workaround was the following approach (summarized):

  1. User visits a product page. 1.1 Product is added to the Cart. 1.2 Initialize the PRB button with information from the Cart.
  2. User clicks the PRB button.
  3. User makes the payment.

This was a clean fix and worked well in both Safari and Chrome. But there was just one problem: The Cart was reset every time the user visited a product page. If the user already had items on the Cart, it was reset automatically after visiting a product page. This was unacceptable from a UI/UX perspective.

This was the most robust approach. We just needed to figure out how not to mess up the Cart every time. We needed a way to add the product into a "temporary" cart that existed along with the regular Cart. Currently, that's not possible. Fortunately, thanks to @dechov, I just found out there's work well underway related to this. Hopefully, when it lands, we will be able to finish the implementation of the new approach without issues.

So, for this reason, and some others (like the ability to know if the product(s) will require shipping or not to be able to remove this section before opening the payment sheet), we need to make an async request to the backend to get the cart information. Without this, we wouldn't be able to improve the way PRBs work currently.

Here's a diagram that shows how PRBs will work if we are able to add the product to a "temporary" cart. Hopefully it can be implemented soon.

screen-shot-2021-09-06-at-11 46 29-pm

Happy to continue the discussion regarding this issue. Let me know if you have any question or suggestions.

helgatheviking commented 2 years ago

@asumaran Hi Alfredo. Thank you so much for this very thoughtful and detailed reply. I appreciate the effort. It does help clarify what some of the limitations are and it's clearly well beyond what I want to get into while just tinkering around. :) I guess I just have to stay patient then. Cool to see the Store API tackle the temp cart situation... they will also need to support adding custom product types to cart (which isn't possible via the Store API yet).

Though the part I am proposing about relaying all the inputs to $_POST... could that still be helpful? I do see the correct prices ending up in the cart. Or do you think it will all go via the Store API eventually?

asumaran commented 2 years ago

@helgatheviking, we are not actively working on this plugin right now. Although, we'll happily review any contribution that works for everyone and doesn't break compatibility with others plugins.

Though the part I am proposing about relaying all the inputs to $_POST... could that still be helpful? I do see the correct prices ending up in the cart. Or do you think it will all go via the Store API eventually?

I quickly looked at your PR and think it's very dangerous to populate super globals dynamically with user-provided data. It'd be best if you could create an issue with your requirements so we can discuss the details on how to achieve that there.

helgatheviking commented 2 years ago

Are there any plugins with request buttons that woo is working on? Seems like at least one plugin ought to be compatible with marketplace plugins... or at least extensible. I won't able to to do this on my own. It's way too big a project for me to take on as a non-employee.

My requirement has already been documented in https://github.com/woocommerce/woocommerce-gateway-stripe/issues/1515. I need to be able to send a dynamic price from the front-end because the database price is NOT the correct price in a lot of cases.

asumaran commented 2 years ago

Are there any plugins with request buttons that woo is working on?

WooCommerce Payments is actively supported and works with PRBs too, but they both likely share the same issue about PRBs on the product page – On the other hand, I just learned we would be back working on WCStripe soon. Hopefully, we can address this issue as it's required to fix multiple existing issues.

aaronsilber commented 2 years ago

@asumaran Glad to hear there may be development on this plugin from the WC team. Our client is still experiencing this issue breaking PRB for them, so we wanted to check in and see if there's any update here? Are you waiting on PR#5953 to be able to deliver this?

aaronsilber commented 2 years ago

Hi @asumaran - our client is still experiencing this issue and is wondering if there's any update or roadmap for resolving this issue? If not, how could a community contributor help out here?

asumaran commented 2 years ago

@aaronsilber, the PR that sets the grounds to implement this issue, has been merged recently.

It seems we'll be able to manage multiple carts and fix the issues we've been dealing with in Apple Pay/Safari.

Hopefully, we can get together a proof of concept and deliver this long-awaited approach soon. I'll try to give it a look next week. Although, most likely, another person will have to work on the implementation since I'll be on parental leave soon.

thenbrent commented 1 year ago

Given the number of issues addressed by this change (as seen in the initial description), I'm labeling it high priority so that we can get the prior work over the line to address these issues.