Lots of code in the shipping quote modules similar to:

  $enabled = variable_get('uc_quote_enabled', array());
  $weight = variable_get('uc_quote_method_weight', array());

  $result = db_query("SELECT mid, title, label, base_rate, product_rate FROM {uc_flatrate_methods}");
  foreach ($result as $method) {
    // Ensure default values are set.
    $enabled += array('flatrate_' . $method->mid => TRUE);
    $weight += array('flatrate_' . $method->mid => 0);

Setting $enabled and $weight on a method should be up to uc_quote as it owns the configuration for these fields. Shipping methods should be able to suggest defaults, but not need to read their own settings.

CommentFileSizeAuthor
#7 1308692-enabled-quote-methods.patch19.62 KBIsland Usurper
#5 1308692-enabled-quote-methods.patch19.42 KBlongwave
#3 1308692-enabled-quote-methods.patch8.12 KBlongwave

Comments

tr’s picture

tr commented

Not sure what you're getting at here. Flatrate and Weightquote are different than USPS and UPS, because the latter two know exactly what methods they need to define in hook_uc_shipping_method(), while the former don't - flat rate and weight rate methods are dynamically defined by the store owner. So in order to return a well-constructed $method array from uc_flatrate_uc_shipping_method(), above code is reading from the database to see what flat rate methods have been defined and what their enabled state and weight is, then appending an array for those methods to the existing $enabled and $weight arrays. That code does *not* enable or set the weight of the flat rate methods. I don't see what you would want to change here.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Why does any shipping method need to know if it is enabled? hook_uc_shipping_method() should just return a list of all possible methods, including the *default* enabled and weight values. uc_quote should then be responsible for filtering the list according to the user's chosen configuration (or use the defaults if the user hasn't saved a selection), and then only calling the relevant enabled methods when requested.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs work
StatusFileSize
new 1308692-enabled-quote-methods.patch8.12 KB

Attached patch is a proof of concept for this idea, currently implemented for flatrate only.

It adds a new uc_quote_methods() function which is responsible for invoking hook_uc_shipping_method and setting enabled/weight according to user preferences, then filtering this list to enabled methods only if requested.

tr’s picture

tr commented

I see - you're saying that "enabled" and "weight" shouldn't appear at all in the array returned by hook_uc_shipping_method(). OK, that makes sense. I don't know why they're there in the first place. There is a very small use case for allowing hook_uc_shipping_method() to return weight, because a module may define several shipping methods and may want to specify relative weights (e.g. USPS and FedEx do this), but I don't see any real problem in losing that as long as initial ordering of sub-methods is controlled by the order that those sub-methods are defined in hook_uc_shipping_method() (which I think it is, by default). For enabled, I guess having it there allows a module to enable its own methods upon install, which might be more intuitive than the admin having to enable the shipping method module first at admin/modules then enable the shipping method itself at admin/store/settings/quotes/methods. But in practice, no quote module does this, so it's no loss.

The description of hook_uc_shipping_method() uc_quote.api.php should be modified as part of the patch.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
StatusFileSize
new 1308692-enabled-quote-methods.patch19.42 KB

This patch includes changes for all four core shipping modules, and improves the documentation for hook_uc_shipping_method().

Flat rate and weight quote have 'enabled' => TRUE so they are automatically enabled on creation as before. USPS international quotes have 'weight' => 1 so they are automatically weighted below the domestic quotes by default.

tr’s picture

tr commented

#5: 1308692-enabled-quote-methods.patch queued for re-testing.

Island Usurper’s picture

Island Usurper commented
Status: Needs review » Fixed
StatusFileSize
new 1308692-enabled-quote-methods.patch19.62 KB

Oh yes, this makes more sense. Had to reroll the patch to get it to apply, but it's committed. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.