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.
| Comment | File | Size | Author |
|---|---|---|---|
| subproducts_settings.patch | 1.39 KB | recidive |
Comments
Comment #1
darren ohI'll try to get this reviewed by early next week.
Comment #2
darren ohIssue 65342 is a duplicate of this issue.
Comment #3
nedjoThis is needed and looks right, but I haven't found time to test it.
Comment #4
recidive commentedThis patch just wraps the
foreachloop with aifchecking 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 functionsubproducts_get_variations()to return an emptyarray()instead ofFALSEwhen we don't have variations for a specific product type. What do you think?Comment #5
darren ohI 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.
Comment #6
darren ohI'm de-assigning myself because I don't have CVS commit access. Someone else needs to commit this patch.
Comment #7
darren ohComment #8
brmassa commenteddarren
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
Comment #9
darren ohIssue 77368 does address other issues besides this one. I would suggest changing its title to reflect the issues which are not duplicates.
Comment #10
darren ohAs 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.
Comment #11
darren ohThis patch has been thoroughly reviewed already.
Comment #12
neclimdulThis 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.
Comment #13
darren ohIt'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.
Comment #14
darren ohI'm glad to report this problem has been fixed in the latest version.
Comment #15
(not verified) commented