I've noticed that the estimator appears to rely solely on the configured default destination type resulting in and estimate (and charge to customer) that may not reflect the actual amount charged by UPS. When setting up a package and shipment, however, the destination address type (commercial or residential) appears to determined dynamically (By the UPS rating tool?)

The end result is that if I set the default destination type to "Commercial" then I consistently undercharge my customers when shipments go a residential address.

There is a tool in the UPS APIs called "Address Validation - Street Level" which determines the destination type and is apparently being used in the shipping process but not in the estimation process. This is illustrated by the fact that, for the same order with a single package the amount in uc_order_line_items.amount (filled from the shipping estimate?) <> the amount in uc_shipments.cost when the destination type does not match the configured default destination type.

see also
http://www.ubercart.org/forum/support/16343/ups_shipping_cost_different_...

CommentFileSizeAuthor
#10 1155274-10-D7.patch10.99 KBTR
#5 ubercart-ups-destination-type-D7-fix-1155274-5.patch5.4 KBamorsent
#5 ubercart-ups-destination-type-D6-fix-1155274-5.patch5.08 KBamorsent
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

TR’s picture

TR CreditAttribution: TR commented
Issue tags: -shipping costs charges

This is entirely possible to do - I do it in my uc_fedex module for instance - but it will require someone to contribute a patch to make it happen. If this is important to you, please consider doing it yourself or hiring someone to do it, then contribute the code back to the community.

TR’s picture

TR CreditAttribution: TR commented
Status: Active » Postponed

Changing status to reflect the fact that the maintainers aren't going to be working on this. We will happily put it into Ubercart if code is provided.

TR’s picture

TR CreditAttribution: TR commented
Version: 6.x-2.x-dev » 7.x-3.x-dev

This will have to go in 7.x-3.x first, if it's implemented.

matthand’s picture

matthand
he/him
Primary language English
Location Maryland
CreditAttribution: matthand commented

I'd like to sponsor the development of this feature. Are there any developers that can quote it for me?

amorsent’s picture

amorsent CreditAttribution: amorsent commented
Issue summary: View changes
FileSize
ubercart-ups-destination-type-D6-fix-1155274-5.patch5.08 KB
ubercart-ups-destination-type-D7-fix-1155274-5.patch5.4 KB

I've attached two patches. One is for D6, and one is for D7. I have tested the D6 patch, but not the D7 patch.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented
Status: Postponed » Needs review

Thank you for posting this. Hopefully someone who needs this feature can test the patches.

Status: Needs review » Needs work

The last submitted patch, 5: ubercart-ups-destination-type-D7-fix-1155274-5.patch, failed testing.

The last submitted patch, 5: ubercart-ups-destination-type-D6-fix-1155274-5.patch, failed testing.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented
Status: Needs work » Needs review
TR’s picture

TR CreditAttribution: TR commented
FileSize
1155274-10-D7.patch10.99 KB

Thanks for getting this started. I found it easier to rewrite the patch than to list all the things I thought needed to be done. This version makes the UPS code consistent with the way it was done in uc_fedex, including admin and shipping options, default values, vertical tab summary, debugging information, and comments.

amorsent’s picture

amorsent CreditAttribution: amorsent commented

Looks good to me, and a bit cleaner than mine.

amorsent’s picture

amorsent CreditAttribution: amorsent commented

One thing ... I see you added to the uninstall:
variable_del('uc_ups_label_lifetime')
I don't see that variable added anywhere else in this patch. Slip in by mistake?

TR’s picture

TR CreditAttribution: TR commented

No, it wasn't a mistake, I just noticed that variable wasn't being deleted so I thought I would correct the situation at the same time I was adding the code to delete the address validation variable.

shazamfu’s picture

shazamfu CreditAttribution: shazamfu commented

I applied TR's patch but my UPS rates still weren't changing based on the destination type. I found that I needed to change this (lines 222 & 223 of the patch):

if (isset($response->AddressClassification->Code)) {
switch ($response->AddressClassification->Code) {

to this:

if (isset($response->AddressKeyFormat->AddressClassification->Code)) {
switch ($response->AddressKeyFormat->AddressClassification->Code) {

If the patch doesn't work for you, I'd suggest using devel to display the $response object & see if you may need to adjust the path to the Code value.

TR queued 10: 1155274-10-D7.patch for re-testing.

jaynhook’s picture

jaynhook CreditAttribution: jaynhook commented

Hi, I noticed on line 496 of the module, it says:

$schema .= "<ResidentialAddressIndicator/>

But the UPS API states that to indicate a Residential address type the <ResidentialAddress/> element should be used. So I think somehow the extra word "Indicator" ended up in the code (or the UPS API was updated)...

I'm trying to determine if this module correctly calculates the rate and determines the address code for Residential vs. Commercial packages. Thanks,

-Jay