Currently, the Ship From address only comes from the admin settings variables. That makes it impossible to use multiple locations. It also makes it hard to use different countries because currently, it's hard coded for the U.S.

The Ship To address can only come from:

 $order_wrapper->{$field_name}->commerce_customer_address->value()

This patch wraps that logic and adds hook_..._alter support for both addresses. Making this change should help address some of the issues brought up by these issues:
#1734920: Shipping from multiple origins The new hook could be used to let you change the ship from address based on the order.
#1313318: Add ship from address outside the US The new hook could let you set a non-U.S. address for the ship from.
#1485894: Modify UPS account forms to allow different services The new hook could be used to let you change the ship from address based on the order.

CommentFileSizeAuthor
#11 2293431-11.patch7.15 KBjoshmiller
#7 commerce_ups-2293431-02-nohunk4.patch6.85 KBchristiansanders
#2 commerce_ups-2293431-02.patch7.52 KBbendiy
#1 commerce_ups-2293431-01.patch7.53 KBbendiy

Comments

bendiy’s picture

bendiy commented
Status: Active » Needs review
StatusFileSize
new commerce_ups-2293431-01.patch7.53 KB
bendiy’s picture

bendiy commented
StatusFileSize
new commerce_ups-2293431-02.patch7.52 KB

Update patch to use right function name.

bellagio’s picture

bellagio commented

Could you tell me what i should expect after applying this patch? Will this let me add different address or is this a patch to develop more coding? i applied the patch, but i can't tell what has been changed.

bendiy’s picture

bendiy commented

@bellagio It just adds the ability for module developers to write hooks that modify what the address is.

// Allow other modules to alter the Commerce UPS ship to address.
drupal_alter('commerce_ups_ship_to_address_from_order', $ship_to_address, $order_wrapper);

...

// Allow other modules to alter the Commerce UPS ship from address.
drupal_alter('commerce_ups_ship_from_address', $ship_from_address, $order_wrapper);

Nothing changes unless you have a code that makes use of those alter hooks and modifies the address. The related issues could use these new hooks to do some of the things they are trying to do.

ultimike’s picture

ultimike
Primary language English
Location Florida, USA
commented

@bendiy,

I really like this patch (thanks for making our variable names a bit more consistent as well).

I'm willing to commit this, let's just have one or two more people test is out and report back that all is well.

Thanks,
-mike

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Reviewed & tested by the community

This is great! I'd swap out property_exists() with isset() because it's more common and fast but that is super minor and drop the fancy = indenting because it's not so common for coding standards.

This allows for the ship_from address to changes based on order! That way we can set the other warehouse's address can be used depending on the destination and get a more accurate estimate!

@bendiy++

christiansanders’s picture

christiansanders commented
StatusFileSize
new commerce_ups-2293431-02-nohunk4.patch6.85 KB

We had some big problems trying to cleanly apply the patch to the current dev version of the commerce_ups_module. Hunk 4 of the .xml.inc file was the issue (the line being replaced was exactly the same...) so we removed the hunk and the patch applied cleanly. Here is new version of that patch.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

This needs a re-roll but I've got commit access so we can get this in.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

edit: cross posted myself:S

niklaz’s picture

niklaz commented

I can confirm that this works, however depending on measurement system when switching to another ship from address which uses different measurement system, UPS will return error instead of results.
example of error:
"This measurement system is not valid for the selected country"

joshmiller’s picture

joshmiller
he/him
Primary language English
Location Indianapolis, Indiana, USA
commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new 2293431-11.patch7.15 KB

Rerolled.

andyg5000’s picture

andyg5000
Primary language English
Location North Carolina, USA
commented
Status: Reviewed & tested by the community » Needs work

Wow, this is an old one :) If it's still needed, I'm fine committing, but a few things first..

The api documentation doesn't match the function implementation.

I like the api documentation better than the implementation. Can we pass the address as alterable and the order wrapper as context (not by reference). I know it's an object, but it's a bit cleaner so that other devs don't go to town altering the order wrapper :)