Hi there,
I've been stuck on a pretty slow internet connection a few times recently, and I've noticed that if I click on the edit tab of an Ubercart order, the products listing and credit card info areas do not get loaded immediately -- I assume there are some AJAX requests that go out and load these pieces separately. This is all fine if I actually wait for these areas to load before saving the order, but I ran into a problem recently where I clicked 'save' on the order before either area had loaded.
Doing so removed all products from the order and all stored credit card info, leaving me unable to process the order.
It seems that this should not be the default behavior -- there should be some mechanism to prevent an empty product list and blank CC info from being saved if those fields have not been dynamically loaded yet. Perhaps a hidden form element whose value was set after the loading was complete would do the trick. Before saving an order, Ubercart could check the value of this field, and if it indicated that the dynamic info had not yet been loaded, then Ubercart would ignore the save.
I think this is a fairly major issue, because it can lead to accidental loss of order information.
As a final note -- you can replicate this by simply disabling Javascript in your browser, editing an order, and saving it.
Let me know if you need any more information,
Ben
-- EDIT --
Though the fix in rev 957 works for the 'Submit changes' button, it does *not* work for the 'Add line' button. If this button is clicked while the payment info or products are still loading, the payment info / product info is lost.
Same problem origin, I believe. I don't know if there are other buttons that also result in the order being saved, but for any that exist or are added in the future, this will also be an issue.
Re: Order products & credit card info lost during order editing
Hmm... good point. That's a nasty bug.
I suppose my quick fix will be to simply disable the submit button and activate it using Javascript once the various panes have loaded their necessary information. It's a little hackish but will prevent wiping order data (which is always no good)!
Re: Re: Order products & credit card info lost during order edit
Made the submit button disabled by default and exposed a couple JS functions for panes to put "holds" on the button. When all holds are removed, the button will be enabled for saving. Let's hope I didn't break normal button execution!
May not be fully fixed
I'm not sure if this is related, but we've lost another set of payment information on my store even after applying this fix (revision 957, I believe). I have not yet been able to replicate it, but I am working on it. It may be an unrelated issue, and thankfully we are not losing product information anymore.
Re: May not be fully fixed
Bah, I was wrong. We lost the product info as well. Still trying to reproduce
Re: Order products & credit card info lost during order editing
Reopening, because though the submit button version of this problem is fixed, but it seems the 'Add line' button also causes the order to be saved, and can still be clicked before the payment information gets loaded (see the edit to the original problem description above).
Patch w/simple fix
I've attached a patch against the latest bzr that contains a simple fix. Instead of using $('#edit-submit-changes') to select the submit button (which only selects the main submit button at the bottom of the page, it uses $(#uc-order-edit-form input[@type="submit"]'), which selects all submit buttons for the form. I've also set the 'Add line' button to be disabled by default.
Ben
Re: Patch w/simple fix
Hmm... main problem is there are other submit buttons on the page that may affect this. I think the "Apply shipping quote" button is in there, too. Can you confirm and maybe re-roll the patch to make sure it covers all the submits?
Re: Re: Patch w/simple fix
The patch above should disable and re-enable all submit buttons in the form with JavaScript. That is the purpose of the change I made to the jQuery selector.
The only issue with other buttons is that there will be a very short amount of time before the page initialization JS runs where they will not yet be disabled. In order to get around this, I set '#disabled' to true when building their form element for the 'Add Item' submit button.
I could just go add this attribute to the 'Apply shipping quote' button (and every other submit button) when it is generated as a form element. However, it seems a better way would be to walk over the order edit form once it was all generated and set '#disabled' = true for all elements of type '#submit'.
Which would you prefer in a patch?
Re: Re: Re: Patch w/simple fix
Hmm, the problem I realized when I was making the first change is that someone w/ JS turned off will never have those buttons disabled by the ready function. So we'll have to have any submit button disabled by default that way folks are forced to turn on JS. I think I'll update the product pane's loading message to mention that JS is required for order edit pages. I also wonder if instead of adding #disabled to every submit element I should walk through the form array and manually add it. This would take the burden off developers to remember to add it to every single order pane (although I guess I'm the main person screwing with the order pane code).
Thoughts?
Re: Re: Re: Re: Patch w/simple fix
Yeah, that seems like a good idea to be on the safe side (walking over the edit form array and disabling any submit buttons before the form is rendered). I've been sucked up into finals week, but I should have some time to create a patch for this soon, unless you happen to get to it first.
Cheers,
Ben
Ben, After an initial
Ben,
After an initial attempt, I decided it wasn't worth it to walk through the array like that. Instead I'm adding and will document the need for others to add the following lines to any submit buttons on their order panes that will effect an order save prior to some other action. Any buttons with this effect should should up disabled and will be recognized by their class for disabling once the page loads and/or after all holds are removed:
<?php'#attributes' => array('class' => 'save-button'),
'#disabled' => TRUE,
?>
Maybe this can be made a little smoother in the future, especially once D6 allows us to attach submit handlers to individual buttons. For now, this should work.
Re: Ben, After an initial
Sorry for the slow response -- just got back from a trip to Death Valley w/out internet.
This solution looks fine to me -- I started to try walking through the form array too, and realized it was probably going to be more trouble than it was worth. Thanks for looking into this and fixing it!
Ben