When a customer has previously placed orders, there will be drop down called "Saved addresses" from which the customer can select an address.
At first, when selecting an address for delivery no shipping costs are calculated. But when selecting an other address, shipping costs *are* calculated, but these shipping costs are based on the previously selected address.

Example

Let's say whe have three flat rates:
1. $ 5.00 (valid when postal code is "11111")
2. $ 7.50 (valid when postal code is "22222")
3. $ 2.50 (valid when zone is "Alabama" (= 1))

The customer has placed three orders in the past, one with postal code = "11111", one with postal code = "22222" and one with zone = "Alabama".

The customer wants to checkout for the fourth time, he/she adds a product to the cart and goes to the checkout page.

At first, the customer selects the address with postal code 11111 from the saved address drop down. No shipping costs are calculated.
Second, the customer changes his/her mind and selects the address with postal code 22222 from the saved addresses drop down. Shipping costs are calculated and result into $ 5.00. This is wrong, because that rate may only be valid when the postal code is "11111" and the current postal code is "22222". So the shipping costs should have been $ 7.50.

Modules installed

- uc_cart
- uc_order
- uc_product
- uc_store
- uc_quote
- uc_flatrate
- rules
- rules_admin
(and modules were the above listed depend on)

Attachments

I have the following files attached:
- A file with the rules I had defined.
- A PHP file, to be executed in a Drupal environment, that adds three orders and three flat rates to the database. With this you can faster setup a test environment to reproduce the bug.

Images

Also attached are some images to clarify what's going wrong.
1. Customer enters checkout page. No shipping costs are calculated.
2. Customer selects address with postal code = "11111". No shipping costs are calculated.
3. Customer selects address with postal code = "22222". Shipping costs for postal code = "11111" are calculated.
4. Customer selects address with zone = "Alabama". Shipping costs for postal code = "22222" are calculated.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

longwave’s picture

Status: Active » Postponed (maintainer needs more info)

Is this still an issue with all the Ajax checkout changes that have gone into 7.x-3.x-dev?

MegaChriz’s picture

Assigned: Unassigned » MegaChriz

Not sure. I hope to find time to test this scenario soon.

Guess I forgot to mention in this issue this was a follow-up of #1448288: Shipping costs not triggered correctly on /cart and /cart/checkout. (See #23 and #24 of that issue).

MegaChriz’s picture

Version: 7.x-3.0 » 7.x-3.x-dev
Assigned: MegaChriz » Unassigned
Status: Postponed (maintainer needs more info) » Active

Yes, this is still an issue. I've retested the scenario posted in #0 with a clean install and Ubercart 7.x-3.2+72-dev. I executed the attached PHP-file in #0 to recreate the shipping rates. Then I imported the rules attached in #0 one by one. And finally I cleared all caches. As that somehow resulted in the rules to be reverted, I re-saved each shipping rate, imported the rules, re-saved the rules and cleared all caches again. This time the rules stayed in place.

Shipping quotes keeps being one step too late with calculating shipping costs; it calculates the costs for the situation that was valid before an other address got selected. I tried adjusting the ajax settings at admin/store/settings/checkout/ajax, but I saw that "Delivery: Saved addresses" was already set to update the shipping quotes pane.

FATALIST’s picture

I have a similar problem. When I change Country - shipping quotes calculates for a previously selected value.
I traced ajax call and found that the problem is in weight sorting at the end of _uc_checkout_pane_list().
In my situation quotes pane should be placed before delivery information pane, but shipping quotes can be properly calculated only if it's placed AFTER delivery address pane. I think there should be predefined panes processing priority, otherwise moving panes can cause errors like this.

MegaChriz’s picture

@FATALIST
Are you sure that shipping costs are calculated correctly when the quotes pane is placed after the delivery pane? When I tried to reproduce this bug the last time, I did had the quotes pane after the delivery pane. So I'm not sure if the root of this problem lies with the order (sequence) of the panes. It could also be a race condition as in #1454080: Shipping calculation not work if choosing existing address (Drupal 6 versions of Ubercart Addresses).

FATALIST’s picture

@MegaChriz
Yes. I tested both of combinations and I seen code.
In _uc_checkout_pane_list() Ubercart gets list of panes and sort it by weight. Next, in uc_cart_checkout_form() ubercart iterates over all panes and calls 'prepare' callback for each one. In calls chain script calls uc_quote_action_get_quote() and finally it calls quote callback (line 103 in uc_quote.pages.inc). In my case it been uc_global_quote_quote. Problem is that $details contains old delivery country (because delivery pane not processed yet) and callback return quote cost for old country. This is totally explains why quotes keeps being one step too late from delivery counrty dropdown value.

FATALIST’s picture

Update:
Problem is in callbacks sequence:
on ajax callback I get this sequence:

  1. uc_cart_checkout_pane_quotes (process)
  2. uc_cart_checkout_pane_address (process)
  3. uc_cart_checkout_pane_quotes (view)
  4. uc_cart_checkout_pane_address (view)

Problem is that quotes calculating on uc_cart_checkout_pane_quotes (process), but address fields updating in uc_cart_checkout_pane_address (process). It affects quotes that depends on delivery address (uc_global_quote in my case).

longwave’s picture

The 6.x-2.x equivalent of this issue was #1084086: Invalid option selected. Recalculate shipping quotes to continue., closed that as a duplicate.

MegaChriz’s picture

I'm not sure, longwave, if the other issue is exactly a duplicate. I had this issue also with the quotes pane placed after the delivery pane, but starting with at least three shipping rates. Maybe we talk about two different issues here, though with the same error results.
I'd like to write a test for this case, but I think it's quite hard (or maybe even not possible?) to do, because, as far as I know, you can't execute javascript during automated tests. I recently found out that it's possible to simulate an AJAX call in tests, but I think that will not be accurate enough for this issue.

mandreato’s picture

I had a similar issue due to pane position/weight: see this post.

SilviuChingaru’s picture

Status: Active » Needs review
FileSize
1.61 KB

Initial patch. The problem was that order was mixed saved at process level and at view level. The right order should be like this:
at process level:
The order is updated with selected address if any and values from copy address are updated also if needed.
at view level:
the shipping form must be re-prepared to use updated values instead of old ones. If it is re-prepared at view level shipping quote use updated order not partially updated order based on it's pane weight.

Status: Needs review » Needs work

The last submitted patch, Issue-1453306-fix-patch-1.patch, failed testing.

SilviuChingaru’s picture

Status: Needs work » Needs review
FileSize
1.73 KB

- Fixed PHP warning and notices for select_address.
- Quotes form is re-prepared only if it is an AJAX request because at first time form generation Uc already call the prepare.

longwave’s picture

@fiftyz: Thanks as always for working on this. I cleaned up the comments and code a little. It would be good to get some manual test results on this.

Status: Needs review » Needs work

The last submitted patch, 1453306-checkout-select-address-trigger-14.patch, failed testing.

longwave’s picture

Status: Needs work » Needs review
FileSize
3.07 KB

Oh, we use -1 as a magic value. I wonder if we can safely change that to the empty string "" instead?

SilviuChingaru’s picture

@longwave thank you for maintaining this great project. It's almost everything I need for my store, my store grew up using Uc and I intend to help developing everything it's missing ;-).

MegaChriz’s picture

I hope to test the patch next week.

longwave’s picture

Third time lucky. Changing uc_select_addresses() should be out of scope of this patch.

SilviuChingaru’s picture

There still is a issue with check if it is an ajax request. My original code is not right, it's checking only if trigger_element exists but i think it shoukd check if exists and #type is not button.
I didn't checked this too deep because I'm on phone now. I'll look deeper into it latter and post a patch if needed.

TR’s picture

@fiftyz: Did you ever get a chance to test @longwave's patch in #19? Does it work for you?

SilviuChingaru’s picture

Status: Needs review » Needs work

@TR The code is working fine if we chose an address from address bock with postal codes 11111 or 22222, the shipping method are hidden/displayed as expected BUT:

If we manually change the postal code and we enter anything else but thos 11111 or 22222 the method is not hidden as expected, even if there is a condition that mention to display the 11111 shipping method only when postal code is 11111.

Of course I put a trigger on delivery postal code for testing this and one on saved addres. The saved address is working as expected, the delivery postal code is not.

Edit: With patch from #14 is working as expected but with one from #19 it doesn't.

I don't see where the problem is in longwave patch and as far as I can see there are no changes in the code itself, only in comments.

I'll do more testing.

SilviuChingaru’s picture

Status: Needs work » Needs review
SilviuChingaru’s picture

Status: Needs review » Reviewed & tested by the community

I think there was something cached not right on my test environment. I repeated the tests today and everything work as expected.

How I've tested:
- I've created a weight quote of 5$ base price with condition that the postal code equal to 11111
- I've created a weight quote of 10$ base price with condition that the postal code equal to 22222
- I've setup uc ajax admin to recalculate the shipping on Delivery: Saved addresses and Delivery: Cod postal.

What I've tested:
I've created 2 orders with postal code 11111 and then 22222 and I've checked if methods are shown / hidden on entering those codes (when postal code textfield lose focus), also with one of this method selected and the changed the pc so the quote is hidden and the frist one is selected by default as expected.
After orders are created I've selected each of this address to see if quotes are displayed as expected and then retest above.

Everything worked fine.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 19: 1453306-checkout-select-address-trigger-19.patch, failed testing.

longwave’s picture

Status: Needs work » Needs review
longwave’s picture

Status: Needs review » Reviewed & tested by the community

Not sure why testbot retested this, back to RTBC as per #24

longwave’s picture

Version: 7.x-3.x-dev » 8.x-4.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed to 7.x-3.x, needs porting to 8.x-4.x.

SilviuChingaru’s picture

@longwave Would have been fair me to be mentioned as the "authored by" in the commit, don't you think?! This is not the first time when only you take credit for my work too... :-((

longwave’s picture

I gave you credit in the commit message, which is the method described in the maintainer documentation at https://drupal.org/node/52287

"Issue #1453306 by fiftyz, longwave: Selecting an address from the drop down at checkout doesn't trigger shipping costs or triggers incorrect shipping costs."

SilviuChingaru’s picture

I saw the comment, but in maintainer documentation is also mentioned:

You can also add a commit author in git to attribute authorship of what you're committing to another user.

which method credits the author as it should on all Drupal pages.

I don't see any credits for me in Committers for Ubercart for this issue for example, I only see that my last commit was 8 months ago, which is not fair.

Also, I don't see my commit in Your Commits on my profile page like I see for example on Views PHP on the same page.

I'm doing this for free, like you do, and I like my work to be at least credited as it should be, like you did for example on commits like:
37b3f34 (#2002748: uc_product_handler_field_price should render trough theme_uc_price),
b815b3d (#1785434: Ubercart is not implementing the foreign keys setting correctly in schema)
or
acfdcde (#1413372: Admin order shipping method not set and order data reset when shipping quote applied to order)

but you didn't on issues like:
#1408156: Catalog navigation not showing in path taxonomy_catalog terms in und-efined lang
#1410302: Notice: Undefined index: method and rate in uc_shipping_shipment_edit();
#1432532: Notice: Undefined index: last-name în uc_order_tokens()
#1492626: "Cart Items" triggers dont work
#1911278: Expose shipping data to views
#1910660: Multiple products cost for products with revisions

Thank you for understanding!

longwave’s picture

Setting the "authored by" line is complicated and I don't always remember to do it when I commit. Also, there can only be one "authored by" so if more than one person worked on the patch, it's not possible to give equivalent credit to both of them. The ones where I did do it, your patch was committed as-is and I remembered; the other ones, either I forgot to copy and paste the correct --author switch, or more than just you contributed to the final patch.

SilviuChingaru’s picture

I personally consider that is definitely less complicated to set the "authored by" than debugging the code. And, yes can only be one "authored by" and if there are more than one person that worked on the patch, I think that the one that actually fixed the code itself should be mentioned as author and not persons that commented the code or spell checked the comments (those should indeed be mentioned in the comment message).

By the way, for issues mentioned above, I was the one that fixed the code and others added some comments or fixed some codding standards at most. Like in this issue also, I fixed the code and you wrote the comments, and my name is mentioned only in commit message and it appear that my last contribution to Ubercart was 8 months ago and you appear as commiter and author witch is NOT fair.

Thank you very much for understanding and I hope that at least in the future you will set the "authored by" even if for this issue you don't rollback and commit again with correct "authored by" set like I think you should.

longwave’s picture

Reverted and recommitted. I will try to remember to use the --author switch in the future where appropriate; but apologies in advance if I forget.

SilviuChingaru’s picture

Thank you very much, you really made my day happier.

P.S.: No problem if you'll forget, I'll be here to remind you :-P :-).

TR’s picture

Issue tags: +beta blocker
longwave’s picture

Status: Patch (to be ported) » Needs review
FileSize
2.64 KB

Straight port to D8; difficult to fully test because we have no shipping quote rules yet.

longwave’s picture

Status: Needs review » Fixed

Committed.

  • longwave committed b0fe8f5 on 8.x-4.x
    Issue #1453306 by longwave, fiftyz: Selecting an address from the drop...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.