When I choose Saved shipping address calculation works only on 2nd time after pressing "Click to calculate shipping address" button.

If I enter shipping address myself everything works well and at 1st time calculate everything.

If I choose existing address, Its automatically calculate wrong values and after pressing "Click to calculate shipping address" button everything counts well and right.

I think its about javascript or ajax bug of ubercart.

Caan anybody explain what could be wrong?

Files: 
CommentFileSizeAuthor
#19 uc_addresses6-uc_quote-trigger-1454080-19.patch2.13 KBMegaChriz
PASSED: [[SimpleTest]]: [MySQL] 4,627 pass(es).
[ View ]
#17 uc_addresses6-uc_quote-trigger-1454080-17.patch642 bytesMegaChriz
FAILED: [[SimpleTest]]: [MySQL] 4,322 pass(es), 2 fail(s), and 4 exception(s).
[ View ]
#15 uc_addresses-uc_quote-adds-trigger-1454080-15.patch1.83 KBMegaChriz
PASSED: [[SimpleTest]]: [MySQL] 2,295 pass(es).
[ View ]
#10 uc_addresses-delay_pc_change_trigger-1454080-10.patch1.34 KBstewart.adam
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch uc_addresses-delay_pc_change_trigger-1454080-10.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

TR’s picture

Assigned:l33roy» Unassigned
Category:bug» support
Priority:Major» Normal
Status:Active» Postponed (maintainer needs more info)

I have never seen this behavior, and I can't reproduce it. Potentially this is a problem with your theme. Do you have the same problem with Garland? Does Firebug show and JavaScript errors on the checkout page? Do you have some other module installed like uc_addresses which might affect the checkout page? 6.x-2.4 is an old version - I strongly recommend you upgrade to the current version of Ubercart.

l33roy’s picture

No javascript errors. No modules that can might affect the checkout page.

Its a javascript problem. I have my own contributed shipping module, that calculates shipping.

Its ok with carts information and the products information pass well to my shipping calculation function.

But the delivery pane does not pass the values of its form fields to my function when choosing the saved address first time. Its only pass the values at second time to my function after pressing "Click to calculate shipping address" button everything counts well and right.

If I enter shipping address myself everything works well and at 1st time calculate everything.

If I choose existing address, Its automatically calculate wrong values and after pressing "Click to calculate shipping address" button everything counts well and right.

my fuction:
uc_cvshipping_quote($products, $details)

$details is object with zip, zone and country from delivery form.

Where to look at to solve my problem?

TR’s picture

Do you have the same problem with Garland?
Do you have the same problem with any other shipping module but your own?

l33roy’s picture

Its very difficult for me to check it. Its production web site.
I cant change the web site with such a big manipulations.

I set drupal_set_message function to my uc_cvshipping_quote($products, $details)

drupal_set_message('Zone:'.$shipto_zone.'; Country:'.$shipto_country, 'error');

And I see that the problem is exactly what I said before. Why you think that it can be the theme problem after it? How it can depend on it? Or it can depend on the shipping function? Explain me please.

$shipto_zone variable is null when choosing the saved address first time. Its not null at second time after pressing "Click to calculate shipping address" button everything counts well and right.

If I enter shipping address myself everything works well and at 1st time calculate everything.

If I choose existing address, Its automatically calculate wrong values and after pressing "Click to calculate shipping address" button everything counts well and right.

TR’s picture

Status:Postponed (maintainer needs more info)» Closed (cannot reproduce)
timgilmour’s picture

We have this same issue, though the interaction is between uc_addresses and the ajax shipping calculation.

As far as I can tell (i'm a producer not a developer), what is happening is a race condition between uc_addresses and the ajax shipping calculation. What is happening:

1) User selects an address from the saved addresses.
2) This populates fields, but the state/province field is an ajax lookup, pulling the relevant states/provinces from the files providing them, by country
3) the delay in populating this field, since it is trying to look up the relevant state/province, instead of just pushing the one stored for the address into the field, plays havoc with the calculate shipping button because...
4) the ajax shipping calc fires when the address fields are changed

and so as a result, you get a malformed shipping quote request, as the state/province field is not correctly populated. it then repeats the request, sometimes successful, sometimes not. the UI hides / shows various rates during this period, causing shipping option entries to appear and disappear

sometimes it ends up with the correct rate. sometimes not. but definitely a race condition between the two ajax events. what i would suggest is that when a saved address is loaded, the ajax lookup on the state/province field is disabled, and the value for that address book entry is directly loaded into state/province.

you are probably not seeing it because you don't have much javascript running on that page. the site we are working on had 550kb of js on the cart/checkout page, which we have since dropped to ~300kb -- and the problem has improved materially, yet is still there and an issue.

hope this helps (and is the correct issue to post into). we're seeing this behavior in ubercart 6.x-2.7 btw.

Daniel.Moberly’s picture

Status:Closed (cannot reproduce)» Needs work

This is still an ongoing issue, and is perfectly described by timgilmour in #6

TR’s picture

Project:Ubercart» Ubercart Addresses
Version:6.x-2.4» 6.x-2.x-dev
Component:Cart/checkout» Checkout
Status:Needs work» Active

@timgilmour posted to a closed thread. That information was unlikely to be seen by anybody. If you want an issue to be seen and acted upon, it's important to set the issue status correctly.

@Daniel.Moberly: I have to assume when you say your issue "is perfectly described by timgilmour in #6" that you are able to reproduce the issue and have come to the same conclusion that "what is happening is a race condition between uc_addresses and the ajax shipping calculation". In which case, this seems to be a problem that should be handled in the uc_addresses queue because no one has reproduced this behavior with just core Ubercart modules. I had to guess which version of uc_addresses you're using - please correct the issue version if necessary. Also note that "needs work" is only for issues that have a proposed patch that doesn't operate correctly and still needs work, otherwise this should be "active".

MegaChriz’s picture

Status:Active» Postponed (maintainer needs more info)

@Daniel.Moberly
Please provide the following information:

  1. Which version of Ubercart Addresses are you using?
  2. Do you have this issue only with Ubercart Addresses enabled or does it also occur when it's not enabled? Please check that by disabling the Ubercart Addresses module and checkout with an user with previous orders (because the default Ubercart behaviour is that addresses can be selected from previous orders, but only if there are previous orders of course!).
stewart.adam’s picture

Category:support» bug
Status:Postponed (maintainer needs more info)» Needs review
StatusFileSize
new1.34 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch uc_addresses-delay_pc_change_trigger-1454080-10.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Attached patch may not be the ideal solution but we needed a quick fix and this works for us.

The problem is that the quote button listens for changes in the postal code field and recalculates on the 'change' JS event. The issue is when setting all the fields at once, we enter a race condition where incorrect shipping quotes may be returned if (1) the counter is changing to something that isn't the store default and (2) the AJAX call to retrieve the new zones comes in after the AJAX call to get new quotes goes out.

Status:Needs review» Needs work

The last submitted patch, uc_addresses-delay_pc_change_trigger-1454080-10.patch, failed testing.

timgilmour’s picture

FYI my post above -- the module version was 6.x-1.0.

MegaChriz’s picture

@stewart.adam
Thanks for the patch! I have not yet tried to apply it (I also still need to find time to reproduce the issue first), but quick glance at it, it seems the problem can be solved if the postal code change is triggered after the zones are updated.
If this is true, then it's likely Ubercart has the same problem, because in the javascript function apply_address() in uc_cart/uc_cart.js, the change for postal code is triggered also before the zones are updated. This would also explain why the issue seemed to be in both Ubercart Addresses 6.x-1.x and 6.x-2.x, because the 6.x-1.x version uses the apply_address() function in uc_cart.js to fill in the address form.

Ubercart Addresses only issue or also Ubercart issue?

I would love to know if this is an Ubercart Addresses only issue or also an issue in Ubercart. If it is the latter, then fixes in both projects may be required.

@stewart.adam, @timgilmour, @Daniel.Moberly + anyone else facing this issue
Can you try if you have the same issue without Ubercart Addresses enabled? Ubercart has also an address selector implemented, but it is only shown when the active user has previously placed orders. As stewart.adam says in #10, the issue seems to happen only when the country changes and the zones need to update, so in order to try to reproduce the issue with Ubercart only, you need to login as a customer/user with previous orders that have a delivery country other then the store default country. You can follow these steps:

  1. Disable the Ubercart Addresses module.
  2. Create an order for a customer and set the delivery country to an other country then the store default. The chosen delivery country must trigger different shipping quotes.
  3. (Optional) Repeat step 2 a few times (each time with an other delivery country, triggering other shipping quotes), so you will get more addresses to choose from at checkout.
  4. Login as this customer.
  5. Add a product to the cart and go to checkout.
  6. Test if you face the same issue.

Alternatively, if you already have customers with orders where the delivery country differs from the store default country (and that have different shipping quotes), you can skip steps 2 and 3.

Why the patch didn't apply

@stewart.adam
The patch didn't apply because it was created at Drupal root instead of at the module folder. This would create a patch applyable by the drupal.org testbot:

cd path/to/uc_addresses
git diff > patch.patch

where "path/to/uc_addresses" would be the path where the uc_addresses files are located on your system.

timgilmour’s picture

i can try, but it won't be till after the holidays -- the project we had the issue on has been delivered (mainly) and we aren't in active development on it anymore.

MegaChriz’s picture

Status:Needs review» Needs work
StatusFileSize
new1.83 KB
PASSED: [[SimpleTest]]: [MySQL] 2,295 pass(es).
[ View ]

I have been able to reproduce the problem! When I select an address, the quotes are calculated earlier then when the address is updated.

Next, I have tried to reproduce the bug with the uc_addresses module turned off. So, I created two orders: first one with "United States" as delivery country, second one with "Canada" as delivery country. Then I added a product to the cart and went to checkout. The address selectors where there (with the addresses from the previous orders). I collapsed the billing pane fieldset, so I could see what happens in the quote pane when making changes in the delivery pane. At first, it seemed that the same error occurred. When I select an address, the shipping quotes are updated before the address values are filled in. But just a second later (or maybe even less than a second), the shipping quotes jump to the right rate.

I checked how uc_quote did this magic and I found a function called setQuoteCallbacks() in ubercart/shipping/uc_quote/uc_quote.js. That function adds event handlers to fields for updating the shipping quotes. I saw it's looking for 'delivery_address_select' for adding a trigger to the address select field, but Ubercart Addresses's one is called 'delivery_addressbook'. Crap. Changing the uc_quote.js code to look also for 'delivery_addressbook', seem to solve the problem.
The setQuoteCallbacks() function is called when Drupal.behaviors.getQuotes() gets called. I'm not sure when that happens, but seem to me that the easiest way to fix this, was just altering the contents of that javascript function.

The attached patch alters the javascript outputted at the checkout page by implementing hook_preprocess_page(). It will scan the scripts section of the page for Drupal.behaviors.getQuotes and then it adds a call to the new javascript function uc_addresses_setQuoteCallbacks(). This function is implemented in uc_addresses.js and what that function does is adding a trigger to the 'delivery_addressbook' field. The function uc_addresses_setQuoteCallbacks() will only be called whenever Drupal.behaviors.getQuotes() gets called.
I know this may not be neatest solution, I hope to find time later to seek for a nicer, less ugly solution.

MegaChriz’s picture

Status:Needs work» Needs review

Forget to set status.

MegaChriz’s picture

Status:Needs work» Needs review
StatusFileSize
new642 bytes
FAILED: [[SimpleTest]]: [MySQL] 4,322 pass(es), 2 fail(s), and 4 exception(s).
[ View ]

When looking at this again one half year later it may be as simple as renaming the address selector on the checkout page from 'addressbook' to 'address_select' as I said in #15 that uc_quote was looking for 'delivery_address_select'. I can't remember why the address selector should explicitly be called 'addressbook'.

The attached patch renames the address selector on the checkout page from 'addressbook' to 'address_select'.

Let me know if this patch works for you.

Status:Needs review» Needs work

The last submitted patch, uc_addresses6-uc_quote-trigger-1454080-17.patch, failed testing.

MegaChriz’s picture

Status:Needs work» Needs review
StatusFileSize
new2.13 KB
PASSED: [[SimpleTest]]: [MySQL] 4,627 pass(es).
[ View ]

In addition of the patch #17, this patch also renames the address selector in the tests. Let's see if that helps.