Problem/Motivation

Disabling Product UI when there are no other product types defined throws a bunch of warnings and notices like:

Notice: Undefined index: commerce_product in commerce_product_reference_field_extra_fields_display_alter() (line 92 of /var/www/sites/all/modules/commerce/modules/product_reference/commerce_product_reference.module).
Warning: Invalid argument supplied for foreach() in commerce_product_reference_field_extra_fields_display_alter() (line 92 of /var/www/sites/all/modules/commerce/modules/product_reference/commerce_product_reference.module).

Proposed resolution

Add an extra check in commerce_product_reference.module to see if there are any available product types.

CommentFileSizeAuthor
#1 1402866.patch1.52 KBamateescu

Comments

amateescu’s picture

amateescu commented
Status: Active » Needs review
StatusFileSize
new 1402866.patch1.52 KB

And the patch :)

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented
Status: Needs review » Needs work 
+++ b/modules/product_reference/commerce_product_reference.moduleundefined
@@ -42,6 +42,11 @@ function commerce_product_reference_field_extra_fields() {
+          continue;

I think the use of 'continue' is discouraged since it makes the program flow harder to follow. What about wrapping the offending code in an "if (isset($product_fields['commerce_product'])) {...}? Same goes for the return statement. This would also make the comment unnecessary since the if (isset ...) is self-explanatory.

amateescu’s picture

amateescu commented
Assigned: amateescu » Unassigned
Status: Needs work » Needs review

I did an 'if' initially, but the code is actually harder to read if indented too much, so I changed that to continue and return.

Also, 'continue' is already used a few lines above in this function, I'm guessing for the same code readability reasons:
http://drupalcode.org/project/commerce.git/blob/refs/heads/7.x-1.x:/modu...

amateescu’s picture

amateescu commented
Issue tags: +1.2 blocker

Tagging. It's an easy enough fix to not get it in 1.2 :)

It also makes the uninstalling experience from #1403972: Make uninstallation possible again much more pleasant.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented
Status: Needs review » Reviewed & tested by the community

Other than the remarks in #2 (you're right, I didn't spot the continue already there), the patch looks good and I can confirm that it fixes the notices.

rszrama’s picture

rszrama commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -1.2 blocker

Committed as is. Yay.

Status: Fixed » Closed (fixed)

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