Closed (fixed)
Project:
Commerce Core
Version:
8.x-2.x-dev
Component:
Cart
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
20 Mar 2017 at 01:14 UTC
Updated:
4 Apr 2018 at 18:04 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
joe_f commentedHere’s a possible patch that removes the order item if the quantity is empty when updated.
Comment #3
vasikeComment #4
harings_rob commentedHi,
Could you create a pull request on github? That allows the automatic tests to run.
I did a quick test and it seems to do what you want it to do however:
- I am not sure if == '' is the right thing to do
- Maybe we should also take care of a 0 value as that still leaves a validation error
Also provide test coverage for the these scenario's.
(To input 0, you have to disable html validation, not sure what the view on this is or how we could avoid this).
Comment #5
harings_rob commentedComment #6
joe_f commentedOK will do.
Ah my mistake, I'll use empty() instead. This should handle the 0 value as well, but I don't think a 0 value will ever get through as even with html validation turned off it'll always hit the server side validation first before it gets to the submit handler function.
Thanks Rob.
Comment #7
joe_f commentedhttps://github.com/drupalcommerce/commerce/pull/685
Comment #8
joe_f commentedComment #9
bojanz commentedI checked 1.x, we want a validation error on empty string, and to remove the order item if the quantity is 0.
Comment #10
shabana.navas commentedAdded validation error when quantity is empty as per comments in #9.
Comment #11
pavelculacov commentedApply patch, worked, thanks
Comment #12
trigdog commentedempty() considers string "0" to be true so this patch never removes 0 as it display the field empty form Validation error. Updated patch.
Comment #13
trigdog commentedSorry, corrupt patch in #12. This one applies.
Comment #14
sorabh.v6Comment #15
sorabh.v6Not sure if we need to check against a "0", because the field type is number which doesn't allow a string as can be seen in below screenshot -
However, patch at #13 does gives a error message when quantity field is empty.
Thanks All
Comment #16
sorabh.v6Comment #17
drugan commentedRemove order item if the quantity is empty string or 0 or "0" or less than 0.
If you still need invalidate empty string quantity you can easily do this by making this field required:
Comment #18
ch2309gj commented#13 is working for me to fix the 'update' functionality in the cart.
I'd like to raise that this is also a problem for the add to cart form. Setting quantity to 0 on the add to cart form allows the item to be added to the cart, with quantity = 0. Setting quantity to blank on the add to cart form throws the following exception for a product priced $10.00:
InvalidArgumentException: The provided value "" is not a numeric value. in Drupal\commerce_price\Calculator::assertNumberFormat() (line 261 of modules/contrib/commerce/modules/price/src/Calculator.php).
Drupal\commerce_price\Calculator::multiply('10.000000', '') (Line: 138)
Drupal\commerce_price\Price->multiply('') (Line: 217)
Drupal\commerce_order\Entity\OrderItem->recalculateTotalPrice() (Line: 125)
Drupal\commerce_order\Entity\OrderItem->setUnitPrice(Object) (Line: 231)
Drupal\commerce_cart\Form\AddToCartForm->buildEntity(Array, Object) (Line: 178)
Drupal\Core\Entity\ContentEntityForm->validateForm(Array, Object)
I haven't tested this with a fresh install or anything.
Comment #19
dandaman commentedThe latest patch does work. In the cart, when quantity is changed to zero or emptied out, it is removed.
Although, at least according to Bojanz, it seems that we want it to validate it is a number, which was removed from the latest patch.
This code does not address one other issue. On an "Add to Cart" form the quantity can still be set to "0" and if they never hit the "Update Cart" button the zero quantity line item could still be ordered. I'd think we'd like to make "0" not an option for the "Quantity" of "Add to Cart" forms as well.
Comment #20
drugan commented@dandaman
Actually, checking if user input on the quantity which is itself of a number type is redundant. Just because modern browsers should not allow user to enter anything except number on such fields. They will be rejected even if it is the number but has a wrong step on the input. Please, check this behaviour in your browser and share if you have some different result. See more:
https://github.com/drupalcommerce/commerce/blob/8.x-2.x/modules/cart/src...
Without hitting the "Update Cart" button the quantity of the line item will never change. That's what this button was intended for. Otherwise it will be removed from the UI.
Empty string quantity. Some people think that this should be invalidated but I am not agree with them. It is an intuitive behaviour of a customer when they want to rid of an item: "Please, I don't want this, that is why I've emptied the field!". A warning that field can not be empty may make them angry and go out from the site forever.
Comment #21
dandaman commented@drugan,
In my tests, if I am on a product page and set the number "0" in the quantity field, I can then click "Add to Cart" and it will be added to my cart. If I immediately checkout, I think I'll be able to checkout with that 0-quantity line item. The code in this patch only changes that if the "Update Cart" button is pressed, I believe?
I guess what I'm saying is that we should add code to also disallow adding zero-quantity items to the cart.
Comment #22
drugan commented@dandaman
OMG, sorry, I've totally forgotten that you are using vanilla Commerce quantity field without any customizations.
Yes, for now the default minimum quantity is 0 and it can't be changed from the UI. It is both on the Add to cart form and edit quantity field on the cart page.
https://github.com/drupalcommerce/commerce/blob/8.x-2.x/modules/order/sr...
https://github.com/drupalcommerce/commerce/blob/8.x-2.x/modules/cart/src...
So, there is a workaround:
First, revert the #17.
Second, apply the patch from this issue:
#2794909: The quantity "step" must be configurable on the widget level
Flush caches.
Go admin/commerce/config/order-item-types/YOUR_ORDER_ITEM/edit/form-display/add_to_cart and change the quantity widget for Commerce number. Set up the widget for your needs. Particularly, set the minimum value to 1 (or more if you want to force customers to buy more at one go!). Do not forget to set a proper step value.
Repeat the same for the default form display mode: admin/commerce/config/order-item-types/YOUR_ORDER_ITEM/edit/form-display
Go to your product page and check if you can add 0 quantity to the cart. Go to cart page and try to update the cart with 0 or empty string quantity.
Comment #23
dandaman commentedAhh that makes sense. I'll check that out, thanks.
Comment #24
dandaman commentedI think I'd rather keep my site as vanilla Commerce as possible, so I think I'll go with #17 and adding this custom code:
In my opinion, it doesn't make sense to have line items of quantity 0. I think the patch proposed in this should be applied to make putting "0" in the quantity of the cart area to remove the line item. And I think the minimum quantity should be 1 by default. Maybe @drugan's rejected pull request is the best way to do this, but until then, I'll just use the custom code above. (I'm sure it gets more complicated with the decimal quantities and have not tested how best to resolve this.)
Comment #25
drugan commentedThis issue is now fixed in the Commerce Extended Quantity module:
https://www.drupal.org/project/commerce_xquantity
Comment #26
gge commented@drugan: Do you plan to include some of the functionality of Commerce Extended Quantity in the Commerce core?
CEQ works great but in order to disallow zero quantity orders we have to install 3 modules (CEQ, Extended Number Field and README Help) which I think is a bit too much...
Thanks!
Comment #27
drugan commented@gge
Unfortunately, the CEQ functionality was requested to pull into DC but not accepted.
So, for now it is the only possibility. As for you concern about extra modules to install, I think you can take a lot from them along with the CEQ module. For example, you may use the same Xnumber widget for all numeric fields on the system, not only CEQ. Also, I'd recommend to use xinteger, xdecimal or xfloat field types when creating new numeric fields for any of the entities. Notice, the HTML5 number element is the powerful one but somehow missed by the lead Drupal 8 maintainers. The Extended Number Field module fixes this issue.
Readme Help. With this module you also can find a lot of use cases on your site. Let's say you have a custom module and want to save some meta data for the module which could be quickly written, accessed and read in a more suitable format for your needs. It's easy. As the README Help module already installed open your_module.info.yml file and add readmehelp dependency for it. Then create or edit README file in the module's root folder and visit the admin/help/your_module page. That's it. The link also will be displayed on the admin/help even if you don't implemented hook_help() in your_module.module file. The same might be done for any of the system modules. You may notice that native help pages for the modules are mostly poor and actually don't provide any useful help. That's because writing the markup in the hook_help is quite tedious and therefore ignored by developers. The same with README files. You need to open the file in a text editor to look what they have inside. Again, mostly nothing. Just blah-blah. Readme Help makes module's help in one place, so you don't need to waist your time for a useless and ineffective work. Also, one additional useful feature of the module which you, as developer should not miss is that the README file is rendered almost the same on your module's Github repository, as on the admin/help/your_module page. Think of it as quick way to create a module's home page with screenshots, links and whatever you can create using markdown syntax and a subset of HTML tags.
Comment #28
gge commented@drugan
Thank you for the detailed reply and I will use your modules. The funny thing is that didn't pay attention to the README Help but now after reading your post I find it very interesting too :)
Thanks again and have a good day!
Comment #29
bojanz commentedWe need to do the following:
1) Add #required => TRUE to the quantity input field, so that the browser guards against "" inputs.
2) In the submit function, ignore invalid input (skip rows where quantity is negative or ""). This protects us if someone removes #required or #min for esthetic reasons. It also matches Shopify's behavior (Matt wanted me to check).
3) Add a test confirming that zero quantity removes the order item.
Comment #31
bojanz commentedThanks, everyone!