Posted by bojanz on January 22, 2013 at 3:41pm
4 followers
| Project: | Drupal Commerce |
| Version: | 7.x-1.x-dev |
| Component: | Cart |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
Delete buttons are not supposed to trigger form validation.
However, the Delete button provided by Commerce doesn't set #limit_validation_errors, leading to possible validation errors on delete (more frequent when there are additional custom form elements in the cart).
Comments
#1
Here's a oneline patch.
#2
The last submitted patch, 1895682-cart-delete-button-validation.patch, failed testing.
#3
Why shouldn't it trigger validation? It's a submit button on the form, and if someone changes a quantity on another line item I wouldn't want their value to be lost by removing something else in the cart. So I'd say this is by design, but if you had a rationale for the change I'd be happy to entertain it.
Obviously, if this were just a delete button at the bottom of a line item entity form, we'd want it to limit validation errors. But as just one element of many on a form, I don't see why we would. I may be misunderstanding the form, though - perhaps it's using a button level submit handler so quantity values wouldn't be saved anyways?
#4
Exactly.
EDIT: Hm, I was wrong. Right now clicking Delete will save other quantities. However, I consider that to be wrong. If you didn't update your cart, Delete shouldn't be doing that for you.
#5
Ahh, interesting. It's how the "Checkout" button works, too.
#6
Here is a smarter patch.
Effects:
1) Delete doesn't trigger validation errors. It just removes the row. Quantities are not updated.
2) Quantities are updated when one of the main buttons is clicked (Update cart, Checkout)
#7
Rerolled patch against dev.