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

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.

Comment	File	Size	Author
#1	1780904.patch	778 bytes	Dan Z
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Dan Z CreditAttribution: Dan Z commented 11 September 2012 at 06:57

Status:

Active

» Needs review

File	Size
1780904.patch	778 bytes

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.

Comment #2

longwave

he/him

English

CreditAttribution: longwave commented 14 September 2012 at 16:14

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.

Comment #3

Dan Z CreditAttribution: Dan Z commented 14 September 2012 at 18:43

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".

Comment #4

longwave

he/him

English

CreditAttribution: longwave commented 14 September 2012 at 19:53

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.

Comment #5

Dan Z CreditAttribution: Dan Z commented 14 September 2012 at 20:26

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.