Hi,
There is a bug when adding additional form validators to the order form (admin UI).
Here are the steps I took to get to this issue:
- Create a new order
- Add a new product
- Set the quantity to 2
- Press 'Save order' button - custom error triggered (maximum is 1)
- Set quantity to 1
- Press 'Save order' button
- Quantity still says 2
- Inconvenient fix - setting the quantity again to 1 and hitting 'Save order' fixes the issue
Reason
When an error is triggered, the quantity (2) of the line item is saved. When the correct quantity (1) is added and the form is posted, the old line_item object is retrieved from $element['line_items'][$line_item_id]['line_item']['#value']. This returned object contains the old quantity (1), and when the new cloned line_item quantity is set to 1. The old object quantity is checked against the new object and as they're both equal, the new line_item object isn't saved in the DB.
So when you load the order, the saved quantity (2) is retrieved from the DB.
I've attached a patch to fix this issue, please review.
Cheers,
David
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | load_updated_line_item-2108263-4.patch | 785 bytes | mglaman |
| commerce_line_item_manager_validate.patch | 544 bytes | evomase |
Comments
Comment #1
evomase commentedComment #2
rszrama commentedHey David, I'm having a hard time reproducing this bug because I don't know how you're attaching the validate handler to your form or what the check is that you're making. For example, given that the line item manager widget validates / submits data in its own validate handler, if you're interacting with the order form to add validation, why not load the line item ID referenced in $form_state['values'] and do the comparison there?
Really, in your case you want to prevent the quantity from changing to an invalid value, and I don't know if that's actually going to be possible. The best case solution for you will be to actually load the line item after it's been saved with the invalid value and set it back to a valid value I think.
Am I missing something?
Comment #3
rszrama commentedTagging for http://contribkanban.com/#/board/commerce/7.x-1.x.
There are a handful of present or recent-past issues that address the reloading of objects from the database. I'm not sure if this one was resolved in any of those or not.
Comment #4
mglamanThere error here, it seems, is that we're running commerce_line_item_save() during validation and not checking if form errors were set, only if the line item changed causing a discrepancy in data that evomase reported.
Comment #5
jsacksick commentedI successfully tested the patch from the comment #4 (The issue described is indeed fixed after applying the patch).
Comment #8
mglamanThanks! Committed.