lat9 / upsxml

UPS-XML for Zen Cart
3 stars 4 forks source link

do not require destStateProv for neg rates #35

Open proseLA opened 1 year ago

proseLA commented 1 year ago

34

lat9 commented 1 year ago

The original change (see #6) was in response to a forum post that indicated that (for not-logged-in customers) no rates showed up from upsxml in the shipping-estimator until the state was supplied. Could/would you validate the difference in display between the current implementation and the removal of the state/province from the check?

I'm thinking that perhaps that destStateProv check needs to be only for US-based shipments.

proseLA commented 1 year ago

ok, i have tested. i think UPS is lame (as our most carriers); and i see problems with this code.

it does seem for US shipments, negotiated rates will not be returned if you only enter a zipcode, but they are returned for published rates.

also, when looking at the method _parseResult, we can either get a string or an array returned.

another problem is now the shipping estimator will return completely different rates if the store owner is using negotiated rates for quoting shipping. sure rates come back when there is just a zip code; but air services tend to be significantly discounted when negotiated, and those numbers will be different on the estimator (with no state code) then when logged in and providing a state code.

the current solution in this code is to hard code what we think UPS wants in order that negotiated rates come back; which i think is an exercise in futility.

if we want to accomplish what you are currently doing my solution would be to change this:

https://github.com/lat9/upsxml/blob/c16570e87e9058e304678e2783409beca161126b/includes/modules/shipping/upsxml.php#L802

to:

        $result = $this->_parseResult($xmlResult);
        if (is_string($result) && !empty($this->upsShipperNumber)) {
            $this->upsShipperNumber = '';
            return $this->_upsGetQuote();
        }
        return $result;

it means a further delay as we are now going back to the UPS server, but that is what i would do.