Skip to content

Validate variation attributes in WC_Cart::add_to_cart() #27115

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.

Sign up for GitHub

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

Jump to bottom
Merged
claudiosanches merged 3 commits into from
Aug 17, 2020
Merged

Validate variation attributes in WC_Cart::add_to_cart() #27115

claudiosanches merged 3 commits into from
Aug 17, 2020

Conversation

@claudiosanches
Copy link
Contributor

@claudiosanches claudiosanches commented Jul 24, 2020 •
edited
Loading

All Submissions:

Changes proposed in this Pull Request:

Currently there's only validation of variation attributes inside WC_Form_Handler::add_to_cart_handler_variable() by checking $_REQUEST, but there's no validation if you are using AJAX or using WC()->cart->add_to_cart() directly, allowing inconsistent behavior while adding variable products to the cart.

I hope with those changes fix the issues reported in #25133.

This PR fixes also a small bug when using:

?add-to-cart=<variation_id>&attribute_color=&attribute_size=
?add-to-cart=<parent_product_id>&variation_id=<variation_id>&attribute_color=&attribute_size=

It allows products with any attributes to get included into the cart without asking for an attribute.

Closes #25133.

How to test the changes in this Pull Request:

  1. Download and import a sample variable product to test: issue-25133.zip
  2. Try to add one of the variations to the cart using ?add-to-cart=<variation_id>&attribute_size=
  3. Note that the variation got added to the cart with missing attributes.
  4. Now try with ?add-to-cart=<parent_product_id>&variation_id=<variation_id>&attribute_size=
  5. Note the missing Size attribute again.
  6. Now apply the code of this PR and try again.
  7. You should see a message that the "Size" attribute is missing.
  8. It's possible to also test using WC()->cart->add_to_cart( $product_id, $quantity, $variation_id, $attributes_list ).

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

Fix - Fixed validation of variation attributes while adding products to the cart.

@claudiosanches claudiosanches added the status: in progress This is being worked on. label Jul 24, 2020
@claudiosanches claudiosanches mentioned this pull request Jul 24, 2020
Closed
3 tasks
@claudiosanches claudiosanches added status: needs review and removed status: in progress This is being worked on. labels Jul 24, 2020
@claudiosanches claudiosanches marked this pull request as ready for review July 24, 2020 23:22
@claudiosanches claudiosanches added this to the 4.4.0 milestone Jul 27, 2020
@Konamiman Konamiman modified the milestones: 4.4.0, 4.5.0 Jul 28, 2020
jonathansadowski
jonathansadowski approved these changes Jul 28, 2020
View reviewed changes
Copy link
Contributor

@jonathansadowski jonathansadowski left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning up my previous fix so that it also works with ajax and using WC()->cart->add_to_cart(). I've verified that it now works in those cases (and still works with request variables), and the news tests / changes to the test make sense to me 👍

@Konamiman
Copy link
Contributor

Konamiman commented Aug 7, 2020

This is what I get when testing the urls:

  • Without the patch: Invalid value posted for Size
  • With the patch: Color is a required field

Am I doing something wrong or missing some additional setup?

@Konamiman Konamiman added the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Aug 11, 2020
@claudiosanches
Copy link
Contributor Author

claudiosanches commented Aug 11, 2020 •
edited
Loading

Am I doing something wrong or missing some additional setup?

It depends on your product, have you imported the product I submitted?

With the patch: Color is a required field

If you are using the same steps it suppose to never require a color, since the color isn't a "any" attribute, but if you have changed to any in the product, so this is correct.

Any attribute that is set doesn't require an attribute if you already is using the variation_id, yet it's required for those attributes using any.

@claudiosanches claudiosanches removed the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Aug 11, 2020
@Konamiman
Copy link
Contributor

Konamiman commented Aug 12, 2020

I'm testing as per your instructions, that's how the product gets imported:

image

Then if I try ?add-to-cart=61&attribute_size= I get the behavior I described.

rrennick
rrennick approved these changes Aug 14, 2020
View reviewed changes
Copy link
Contributor

@rrennick rrennick left a comment

Choose a reason for hiding this comment

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

@claudiosanches Tested great for me.

@rrennick
Copy link
Contributor

rrennick commented Aug 14, 2020

This is good to merge once conflicts are addressed.

@claudiosanches
Copy link
Contributor Author

claudiosanches commented Aug 17, 2020

@Konamiman You are testing correctly.

With the patch: Color is a required field

This message is correct with the patch. The color suppose to be required, since it's empty now, at least on value need to inserted.

@claudiosanches claudiosanches merged commit f968bc1 into master Aug 17, 2020
@claudiosanches claudiosanches deleted the fix/25133 branch August 17, 2020 18:02
@woocommercebot woocommercebot added release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Aug 17, 2020
@juliaamosova juliaamosova added status: testing instructions added and removed release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Aug 18, 2020
@Aljullu Aljullu mentioned this pull request Aug 24, 2020
Merged
@filipemmacedo
Copy link

filipemmacedo commented Aug 26, 2020

Hello.

I am also having this problem. Only need to add the piece of code in the files?

Thank you

@ObliviousHarmony ObliviousHarmony mentioned this pull request Sep 10, 2020
Merged
6 tasks
@TwistedAndy TwistedAndy mentioned this pull request Sep 30, 2020
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peterfabian peterfabian Awaiting requested review from peterfabian

2 more reviewers

@rrennick rrennick rrennick approved these changes

@jonathansadowski jonathansadowski jonathansadowski approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release: add changelog Mark all PRs that have not had their changelog entries added. [auto]

Projects

None yet

Milestone

4.5.0

Development

Successfully merging this pull request may close these issues.

Order details page not displaying variation/attribute name

8 participants

@claudiosanches @Konamiman @rrennick @filipemmacedo @jonathansadowski @juliaamosova @woocommercebot