A customer was having issues with items showing up in multiple packages, thus showing higher shipping rates. We found out that the uc_quote field has a 'phone number' but the node has no where to add it! This should be added.
He added however, that it'd be best not to add any alternate address, since everything comes from the default location. Therefore, I suggest the following code:
Line 211, uc_quote (Inside uc_quote_form_alter)
$form['shipping']['default_address']['disable_address'] = array('#type' => 'checkbox', '#title' => t('Use Default Address'), '#default_value' => 1);
Change line 123 to add the check for default address:
if (!$node->disable_address && $node->street1) {
I added some extra into the code to check for another bug:
1) This code will check to see if the 'default address / don't update fields' is enabled. If its unchecked, it will check for various address components. Before only $node->street1 was viewed. In most cases that would be sufficient, but I can think of two places that have only their Name, city, zone, and zip code (since they own the zip-code and have one central mailing depot). If they didn't have a street address, it would delete their alternate entry!
see attached patch for the changes
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | 405678-compare_address_backport_d6-14.patch | 2.7 KB | millenniumtree |
| #12 | 405678_compare_address.patch | 4.05 KB | Island Usurper |
| #11 | 405678_compare_address.patch | 3.88 KB | Island Usurper |
| #9 | multi-package.patch | 3.08 KB | beefzilla |
| uc_quote-DefaultAddress.patch | 2.4 KB | japerry |
Comments
Comment #1
cha0s commentedThis patch looks fine. Can we get it in?
Comment #2
rszrama commentedI'm not sure it's a very user-friendly fix. Perhaps the thing to do would be for quotes to simply not take phone number into account at all when generating the location - makes sense. The phone number has no bearing on a shipping quote anyways. : P
Comment #3
cha0s commentedAs far as I could tell the code here does 2 things:
I believe he was just requesting that the phone number form element be added to the product form; the patch doesn't reflect that change.
Comment #4
rszrama commentedIt sounds like 2 might be a worthwhile fix, but 1 happens anyways if the address is left blank. We'll just have to have Lyle look at this to determine if it's really necessary. I think I'd still say that when it comes to building packages for shipping quotes the phone number should just be ignored altogether. It's unnecessary extra information.
Comment #5
Island Usurper commentedOh boy. Well, phone numbers are sometimes necessary for UPS shipments, particularly international shipments, so I'm inclined to leave the field in there. Also, the disable_address checkbox isn't being stored anywhere, and I'm not inclined to make a database change for it at this time, either. That means it's not much better than the way things are right now.
Even in the cases where there's only one mailing address in a ZIP code, that address still has a street number, doesn't it? It just seems like such a rare case that the exceptions can just make up something if they really have to.
What I'd really like is a good, object-oriented address API. Something that had unique ids for different locations so I could compare them easily. Until then, adding a phone field to the product location should help. There's already a column for it in the database, so I don't know why it's not being used.
Comment #6
tr commented#596982: uc_quote missing phone field on product page contains a patch for phone number.
Comment #7
flapsjack commentedI agree with Ryan that phone numbers aren't necessary for getting a shipping quote. I can see situations where the phone number is helpful so I am not for removing it. However, I don't think the phone number should be part of the comparison of two addresses.
Comment #8
tr commentedLets reconsider this and fix in 7.x-3.x if needed.
Comment #9
beefzilla commentedI have a fix for this. Because products using the default address don't pick up the phone number and email, I check against the physical parts of the address.
And the function that pulls a product's address from the database wasn't returning a UcAddress object, which added to the problem.
See http://drupal.org/node/1237554#comment-4915896 for the .zip file to test.
I added a patch file that includes the changes to three files I made. Not sure if the patch file formatting is right for multiple files.
Comment #10
beefzilla commentedComment #11
Island Usurper commentedThat's really unfortunate that fetchObject() doesn't respect the fetch option on most databases. (SQLite apparently uses different code for that method that looks like it works.) Fortunately, there is a workaround.
I'm taking out the in_array() call there because it's just silly. The $addresses array should only be walked once per product, so if we get to the end of it without finding a matching address, then we have to make a new package. Also, the UPS module is very similar, so I've patched it as well.
Comment #12
Island Usurper commentedOof. Somehow I put isSamePhysicalLocation() in the wrong class there. Once I put that in the UcAddress class where it belongs, it started to work right.
Comment #14
millenniumtreeI discovered this problem in 6.x-2.x
If we entered a phone number in the default shipping address, we'd always get UPS quotes with 2 packages instead of 1.
Our products are loaded programmatically, so no records are written into {uc_quote_product_locations}, until you manually edit/save a product. Once that happens, and you add-to-cart a manually-edited product along with a programmatically-created product, Ubercart thinks the two addresses are different (no phone number is stored in {uc_quote_product_locations}).
So, I backported this patch to D6/UC2. Enjoy!