If we try to access the subproducts module settings page when we don't have any variations/attributes this error occurs:

warning: Invalid argument supplied for foreach() in subproducts.module on line 1821.

This patch checks if there is an array before doing the foreach loop.

CommentFileSizeAuthor
subproducts_settings.patch1.39 KBrecidive

Comments

darren oh’s picture

Assigned: Unassigned » darren oh

I'll try to get this reviewed by early next week.

darren oh’s picture

Issue 65342 is a duplicate of this issue.

nedjo’s picture

This is needed and looks right, but I haven't found time to test it.

recidive’s picture

This patch just wraps the foreach loop with a if checking if $variations[$ptype] is an array (the patch seems to do more, but it's just indentation). I've experienced others errors like this, but I can't remember where, and I don't know if they have been fixed. So we can just change the function subproducts_get_variations() to return an empty array() instead of FALSE when we don't have variations for a specific product type. What do you think?

darren oh’s picture

Version: master » 4.7.x-1.x-dev
Status: Needs review » Reviewed & tested by the community

I think we should commit the patch as is. I tried using an empty array, and the error still occurred. By the way, the error only occurs when I enable the apparel module. There were no more errors after I applied this patch.

darren oh’s picture

Assigned: darren oh » Unassigned

I'm de-assigning myself because I don't have CVS commit access. Someone else needs to commit this patch.

darren oh’s picture

Component: other » subproducts
brmassa’s picture

Status: Reviewed & tested by the community » Needs work

darren

even marked by you as duplicated, the code posted in http://drupal.org/node/77368 address 3 modifications:
1* subproducts.module need one correction (made by henrique recidive)
2* subproducts.inc needs two corrections (made by yourself)

(these two need an extra line, like
if ($variation->attributes != NULL) { (as recidive did)
if (is_array($variations[$ptype])) { (as you did)
if ($variation->attributes) { (as i did)
)

3* .inc forces you to chose at least one attribute of each variation. so there is a new correction.

they are different and needed. as long the http://drupal.org/node/77368 issue has no .patch file (since my subproducts files has many other mods), i ask you to make the new .patch and post it here. (for 4.7 and cvs)

regards,

massa

darren oh’s picture

Issue 77368 does address other issues besides this one. I would suggest changing its title to reflect the issues which are not duplicates.

darren oh’s picture

Status: Needs work » Needs review

As far as I can tell, brmassa has no objections to the corrections contained in this patch and was simply pointing out that there was more work to be done. Committing this patch would be progress.

darren oh’s picture

Status: Needs review » Reviewed & tested by the community

This patch has been thoroughly reviewed already.

neclimdul’s picture

Assigned: Unassigned » neclimdul

This has been tested a lot. Thanks. I know it works and this is somewhat unrelated but I'd like to see something added to this patch.
My reaction to getting the error and reviewing the patch was "How do I add variations"? I honestly don't know. I figured it out one day a couple months ago and can't remember. If I can't Joe Blow sight admin probably wont either. Can someone add a hook_help entry for the page or something with some information on how to add variations and setup subproducts in general.

darren oh’s picture

It's not possible to add variations without installing the apparel module. With the apparel module installed, variations can be created in the store settings menu: admin/store/settings/variation. Some of us are not happy with having to use one set of variations for all products in a product type (see issue 70519), but that's a different issue.

darren oh’s picture

Status: Reviewed & tested by the community » Fixed

I'm glad to report this problem has been fixed in the latest version.

Anonymous’s picture

Status: Fixed » Closed (fixed)