When going to the Shipping quotes admin page at admin/store/settings/quotes

Notice: Undefined index: quote in uc_quote_method_settings() (line 177 of ...shipping/uc_quote/uc_quote.admin.inc).

Here's the problem code:

    $shipping_methods = module_invoke_all('uc_shipping_method');
    $method_types = array();
    foreach ($shipping_methods as $method) {
      $method_types[$method['quote']['type']][] = $method['title'];  // Error on this line.

The problem is that I have a shipment only module installed. This module doesn't do quotes, so there's no 'quote' element of that particular $method array. See http://api.ubercart.me/api/drupal/ubercart!shipping!uc_quote!uc_quote.ap... and note that the 'quote' element is optional.

#1 1780904.patch778 bytesDan Z
PASSED: [[SimpleTest]]: [MySQL] 2,455 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more


Dan Z’s picture

Status: Active » Needs review
778 bytes
PASSED: [[SimpleTest]]: [MySQL] 2,455 pass(es). View

This patch seems to fix the issue. It might be worth going through the rest of the quote module to check for this same false assumption. There are four invocations of hook_uc_shipping_method() in there.

longwave’s picture

Status: Needs review » Fixed

Committed, thanks.

There are many calls to module_invoke_all('uc_shipping_method') spread throughout the code; some or all of these should all be switched out for calls to uc_quote_methods(), but that's for another issue.

Dan Z’s picture

Status: Fixed » Closed (won't fix)

So, instead of the fix in my patch, I should fix this bug by finding all the module_invoke_all('uc_shipping_method') calls that can be switched to uc_quote_methods(), and switch them?

Since it sound liks that's what you mean, I'll switch this to "won't fix".

longwave’s picture

Status: Closed (won't fix) » Fixed

No, "committed" means I reviewed your patch and pushed it to the repository, and even gave you author credit: http://drupalcode.org/project/ubercart.git/commitdiff/e011f73 - so this particular bug is fixed and this change will appear in the next release.

My second paragraph was referring to "It might be worth going through the rest of the quote module", which is possibly true, but should be handled in a separate issue. Unless it's actually causing a problem, I don't want to go changing code for the sake of it; however it would be an improvement to use the API properly rather than calling module_invoke_all() from many different places. I'm not suggesting you should necessarily be the one to take this on, but if you do, please start a new issue for it.

Dan Z’s picture

I would be a lot less embarrassed right now if I hadn't accidentally missed reading the first two words of your comment #2.

I created #1785146: Convert calls to module_invoke_all('uc_shipping_method') to uc_quote_methods(). for the API cleanup as you suggested and assigned myself to it. However, it's about #7 on my priorities right now, so it will be months, and someone might beat me to it.

Status: Fixed » Closed (fixed)

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