lat9 / upsxml

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

Make state and zip required #9

Closed scottcwilson closed 5 years ago

scottcwilson commented 6 years ago

As discussed on the forum.

scottcwilson commented 6 years ago

This would mean an automatic error message as soon as someone pulls up the estimator. Are you sure?

scottcwilson commented 6 years ago

Would it be better to change the core to give a better message? Like changing includes/languages/english.php to say

define('CART_SHIPPING_OPTIONS_LOGIN', 'Please <a href="' . zen_href_link(FILENAME_LOGIN, '', 'SSL') . '"><span class="pseudolink">Log In</span></a>, or set your state and zip code, to display your personal shipping costs.');

(outside the scope of this mod.)

lat9 commented 6 years ago

I was thinking more a warning rather than error message, something along the lines of your proposal for that shopping-cart page message. That's no different that the UPS returning "I can't find any quotes for you" and lets customers know that UPS is an option, if they'd only supply the information needed.

scottcwilson commented 6 years ago

If I do this instead, nothing changes:

        if (($order->delivery['zone_id'] == '') ||
            ($order->delivery['postcode'] == '')) {
            return $this->quotes = array('module' => $this->title, 'error' => 'some error message');
        }
lat9 commented 6 years ago

Note that the change gets a tad more complicated, because not all countries have zones and/or postcodes.

lat9 commented 6 years ago

Looking at the UPS documentation, it appears that the state/zipcode requirement applies to shipping addresses in the US only.

I added the following snippet (after the global statement) to the top of the plugin's quote method and the shipping estimator no longer returned UPS quotes unless both state and zip were supplied.

        global $order, $shipping_weight, $shipping_num_boxes, $total_weight, $boxcount;

        if ($order->delivery['country']['id'] == 223 && (empty($order->delivery['zone_id']) || empty($order->delivery['postcode']))) {
            $this->quotes = array(
                'module' => $this->title,
                'error' => "Both the state and zipcode are required for accurate UPS quotes."
            );
            return $this->quotes;
        }

Note that the default shipping-estimator handling does not display shipping-related errors.

scottcwilson commented 5 years ago

Should this PR be closed? It sounds like you have fixed the issue.

lat9 commented 5 years ago

Scott, to keep me aware of to-be-released changes, I don't normally close issues/PRs out until I've corrected/released the associated code.