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

Fix the pricing issue with woo product add-ons ext #780

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

wigglemuff
Copy link
Contributor

@wigglemuff wigglemuff commented Jun 25, 2020

Summary

Closes #606.

Before PPEC only picked up the price of the last checked add-on in the total price calculation. Now it picks up the prices of all the checked add-ons in the total price.

Steps to test:

  1. Install Woo + PPEC + Woo Product Add-Ons
  2. Set up add-ons by creating a simple product > going to product-data > add-ons > adding a checkbox field with 3 options with different flat prices. Example: https://d.pr/i/dGeblB (also available as a inline screenshot-1 below).
  3. Switch to the master / main branch
  4. Go to the product page (having add-ons), try purchasing the product via PPEC button with 2 checked add-ons.
  5. Current behavior: The total price = main product price + last add-on price. It does NOT include prices for every checked add-on in the total price. Expected behavior: The total should include prices for every checked add-on.
  6. Switch to the branch issue_606
  7. Try purchasing a product via PPEC with 2 checked add-ons.
  8. The total price = main product price + prices of ALL the checked add-ons.

Documentation

This doesn't need documentation. The customer does not see any change.

Tests for PPEC

  • Checkout a single product with no add-ons
  • Checkout a single product with add-ons, by selecting at least 2 or more add-ons (expected behavior: total price = main product price + price of each checked add-on).
  • Checkout from cart page
  • Checkout from checkout page

Screenshot

Screenshot-1
Screenshot 2020-06-25 at 4 47 58 PM

Questions

Previously the data sent to PayPal only contained one add-on item (example: c) even when the checked items were more (example: b and c).

Screenshot 2020-06-25 at 4 18 00 PM

In this PR, I have made the field name's unique, such that each checked item is included in the data sent to PayPal. Is this the correct approach?

Closes #606.

Before ppec only picked up the last checked add-on in the total price calculation. Now it picks up all the checked add-ons in the final price.
This commit updates generate_cart function, replaces serialized data with FormData object.
Using FormData fixes all the price related issues between PPEC and Woo Product Add-ons plugin.
Tested for simple and variable products with Woo Product add-on plugin.
Copy link
Contributor

@achyuthajoy achyuthajoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the bug report, there are 2 problems that needs to be fixed.

  1. Multi-option check boxes uses only the last checked option value when passed to PayPal
  2. File field type values not passed to PayPal

Problem 1

I've tried applying the PR and noticed that the problem with check boxes are solved. However, explicit renaming of add-on fields caused problems the other field types and values for the same are not passed to PayPal.

Steps to recreate

  • Apply PR
  • Add different field types to products (Short Text, Quantity, Custom Defined Price etc.) including check boxes with multiple values.
  • From Single Product page, submit to PayPal and notice that only the values for check boxes along with the base product price are passed to PayPal.

Problem 2

However, the second problem related to file field type value not passed to PayPal is still not solved. I've managed to figure out from where the problem happens (Line 92).

var field_pairs = form.serializeArray();

jQuery serializeArray() skips the file input field and hence the values are not added to the data array - https://api.jquery.com/serializeArray/

assets/js/wc-gateway-ppec-generate-cart.js Outdated Show resolved Hide resolved
@wigglemuff
Copy link
Contributor Author

Revisions

cb0a78e fixes both these issues:

Based on the bug report, there are 2 problems that needs to be fixed.
(a) Multi-option check boxes uses only the last checked option value when passed to PayPal
(b) File field type values not passed to PayPal

Manual Tests

  • Variable Product
    • Buy a variable product with no add-ons
    • Buy a variable product with two checked add-ons
    • Buy a variable product with following woo-add-on fields: quantity, customer defined price, file upload
  • Simple Product
    • Same tests as above for simple product

Debugging

To debug, insert the code below on line 118:

for (var key of formData.entries()) {
    console.log(key);
}

The Magic

This fixes all the issues because each key is unique in formData:

Screenshot 2020-06-26 at 7 14 10 PM

// Save attributes in a nested array,
// so that `attributes` can be used later on when adding a variable product to cart.
if ( -1 !== formParams[ i ].name.indexOf( 'attribute_' ) ) {
formData.append( "attributes[" + formParams[ i ].name + "]" , formParams[ i ].value );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to do this because formData does not support nested values!

$.each( $( tag )[ 0 ].files, function( i, file ) {
formData.append( tag.name, file );
} );
} );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helps us include the file upload field prices from Woo Product add-on in the final calculation.

@jorgeatorres jorgeatorres self-requested a review July 16, 2020 13:43
@jorgeatorres jorgeatorres self-assigned this Jul 31, 2020
@chickenn00dle chickenn00dle changed the base branch from master to main August 17, 2020 07:35
@chickenn00dle chickenn00dle changed the base branch from main to trunk August 26, 2020 05:06
@jorgeatorres jorgeatorres removed their assignment Feb 10, 2022
@jorgeatorres jorgeatorres removed their request for review February 10, 2022 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PayPal Express checkout on single product doesn't include add ons
3 participants