Hiya,

I've just installed og_forums-5.x-2.0 for my drupal 5.2 (Apache2, MySQL5, PHP5) and when logged in as normal user (not admin), I am getting this error when I try to post a forum topic:

warning: Invalid argument supplied for foreach() in /var/www/drupal/sites/all/modules/og_forum/og_forum.module on line 656.

I'm not getting any luck searching for an answer so thought I'd post here and see if anyone knew what the problem was...

TIA!

Comments

rconstantine’s picture

This may be a silly question, but do you have a forum vocabulary defined? You should have one here: admin/content/taxonomy that says "Forums".

The line you refer to is this one: foreach ($form['taxonomy'][$vid]['#options'] as $key => $the_rest) {

So if the Forum vocab exists, perhaps you haven't defined any forums inside it, or you haven't run the og_forum retro-active forum creator at admin/og/og_forum where there's a button marked "Update old groups".

Anonymous’s picture

You're right, I didn't have a vocab. I defined a one called 'forums' for the content type forums, set it to freetagging and required as my other vocabs are. I then clicked the retroactivat button just in case that would help but as all the forums already created it just said 0 forums created, and I still have the error when I try to post. I guess my next step is to test it out on a vanilla drupal install and see if creating the vocab first helps...

rconstantine’s picture

You can't create a vocabulary for forums manually. That should be in the Drupal handbook about forums. The way it's created is either: http://drupal.org/node/24475, or by creating a new organic group (which creates a container and one forum for it), or by retroactively creating forums for existing groups via the og_forum admin page. It either is always, or almost always, vocab ID 1. You should delete anything you made manually and try one of the above methods. You should get a Drupal message that says forum vocabulary created or something like that.

Again, read up on the regular forum docs. This module just extends behavior of them, but doesn't override too much.

Anonymous’s picture

I've rebuilt my site from scratch so not getting this error any more.

I did do the retro add forums, but it didn't make the category for me.

rconstantine’s picture

Glad to hear it's working. Sorry you had to redo your site. Feel free to open another issue regarding documentation if you'd like. I know the README needs to totally be redone. List specific things you'd like to see there. It should probably mention that the standard forum module docs should be read first.

Did you get a chance to read them? Did you find what I was talking about - the trigger that creates the forum vocab automatically? If you found it, give me a link so I can include it here and in the docs.

Paul Natsuo Kishimoto’s picture

Status: Active » Needs review
StatusFileSize
new0 bytes

I'm also experiencing this bug. Unfortunately, a wipe-and-restart isn't an option because I've got on the order of 300 forum threads and 12000 comments on a live site.

The attached patch seems to fix the problem. I'm not sure if the corrected behaviour is as expected... the container title doesn't seem to be in the "Forum:" drop-down. I'm usually logged in as an admin user, so maybe that's always the case for a general user (should it be?).

Also, all the files in the online browseable CVS are marked as "dead." I'm pretty sure this is wrong.

Paul Natsuo Kishimoto’s picture

Assigned: Unassigned » Paul Natsuo Kishimoto
Category: support » bug

I guess this was actually a bug, and I can assign it to myself.

rconstantine’s picture

I'll take a look at your code. Thanks for your efforts. I believe HEAD is dead, but the other branches should have files in them. They better! Yeah, just checked. They're all there.

rconstantine’s picture

The file was empty.

rconstantine’s picture

Oh, and make the patch against 2.x-dev if you can.

Paul Natsuo Kishimoto’s picture

StatusFileSize
new1.18 KB

Maybe the filename has to be .patch.txt?

rconstantine’s picture

Okay, I'll look at it. BTW, the container should not be listed when inside a group because the group is the container and containers can't hold posts, only forums can.

rconstantine’s picture

This is strange. First of all, I don't get the error message as listed in the original post at all. Second, I am able to post messages just fine as a user with only just enough permissions to do so - i.e. not given ANY other permissions.

I added some printr's to the section of code you patched without adding your patch to see what the data looks like. If you'd like to, you can run this for yourself:

if (!user_access('administer forums')) {
        drupal_set_message('<pre>All results: ' .print_r($all_results, TRUE). '</pre>');
        foreach ($form['taxonomy'][$vid]['#options'] as $key => $the_rest) {
          $option_key = key($the_rest->option);
          drupal_set_message('<pre>Option Key: ' .print_r($option_key, TRUE). '</pre>');
          drupal_set_message('<pre>Key: ' .print_r($key, TRUE). '</pre>');
          drupal_set_message('<pre>The rest: ' .print_r($the_rest, TRUE). '</pre>');
          if (!array_key_exists($option_key, $all_results)) {
            unset($form['taxonomy'][$vid]['#options'][$key]);
          }
          elseif (array_key_exists($option_key, $all_results) && $all_results[$option_key] == 0) {
            unset($form['taxonomy'][$vid]['#options'][$key]);
          }
        }
        $form['og_nodeapi']['visible']['og_groups']['#value'] = $form['og_nodeapi']['visible']['og_groups']['#default_value'];
        unset($form['og_nodeapi']['visible']['og_groups']['#default_value']);
        $form['og_nodeapi']['visible']['#access'] = 0;
      }

From what I see, the numbers are fine, as is the behavior. It's all just as expected.

What is not expected is when I go to view a post as that same user. I get a different error message:

Fatal error: Cannot use string offset as an array in D:\webserver\wamp\www\Drupal\includes\theme.inc on line 555

So I'll be looking into that, but the patch you've supplied doesn't seem necessary to me. Well, not just that, but it would give erroneous results as well. Let's look at a portion of the printr stuff that I see:

All results: Array
(
[69] => 0
[74] => 69
)

Option Key: 69

Key: 0

The rest: stdClass Object
(
[option] => Array
(
[69] => A group about Bob.
)

)

Option Key: 74

Key: 1

The rest: stdClass Object
(
[option] => Array
(
[74] => -Turkey dinner
)

)

Option Key: 8

Key: 2

The rest: stdClass Object
(
[option] => Array
(
[8] => Bob's Group
)

)

Option Key: 9

Key: 3

The rest: stdClass Object
(
[option] => Array
(
[9] => -General discussion
)

)

Here, you can see the forum->parent pairs in $all_results, followed by the first several iterations through the foreach, including $option_key, $key, and $the_rest. You can see that $key is simply a generic index from 0 and going up which has nothing to do with the forum/container ID number and so they'll never be array keys inside the $all_results array. So in your changed code, you would test to see if 0, 1, 2, or 3 are in the set 69, 74, which they are not. All four would be removed from the drop down list and in this case, there wouldn't be any selectable items in the list and the forum topic would be unpostable. Using my numbers, I test to see if 69, 74, 8 & 9 are in the set 69, 74. Two of them are. Those two are kept, while 8 & 9 are unset.

Moving to the second portion you changed, again, you compare to see if the value held in $all_results is 0. Since 0, 1, 2 & 3 are not keys in the array in the first place, they can't have values equal to 0, so this code won't fire in your case. In my case, I check to see if keys 69 & 74 have values of 0 in the $all_results array. 69 does, so it is unset, leaving just the forum I started in and initiated a post to. The post will go to 'Turkey dinner'.

So, this code is correct and I won't be changing it. But I wanted you to know why, and hopefully understand what the code is doing a bit more so that you can hunt down your problem better. I'll be hunting down my problem as well. I don't know if they are related or even the same. But let me tell you, I do appreciate your initiative and willingness to help. People like you are very helpful.

rconstantine’s picture

Nevermind on my error. I realized I had been screwing with mode.tpl.php last night just before I went to bed and didn't pick that up this morning.

So I have no errors. Sorry that doesn't help you.

Paul Natsuo Kishimoto’s picture

Here's my corresponding output:

All results: Array
(
  [1] => 0
  [2] => 1
  [5] => 1
  [6] => 1
)

Option Key: 
Key: 11
The rest: -2007 Veterans

Option Key: 
Key: 2
The rest: -Chit Chat

Option Key: 
Key: 5
The rest: -Car Pool

Option Key: 
Key: 6
The rest: -Car Wash Trading

Option Key: 
Key: 21
The rest: -Video Critique

along with error messages like:

# warning: key() [function.key]: Passed variable is not an array or object in /mnt/raid/www/home/dragonb/public_html/drupal/sites/all/modules/og_forum/og_forum.module on line 659.
# warning: array_key_exists() [function.array-key-exists]: The first argument should be either a string or an integer in /mnt/raid/www/home/dragonb/public_html/drupal/sites/all/modules/og_forum/og_forum.module on line 663.
... (repeats)

If I instead use a line like:

drupal_set_message("<pre>\$form['taxonomy'][$vid]['#options']\n" . print_r($form['taxonomy'][$vid], TRUE) . "</pre>");

I get:

$form['taxonomy'][1]['#options']
Array
(
    [#type] => select
    [#title] => Forums
    [#default_value] => Array
        (
            [0] => 2
        )

    [#options] => Array
        (
            [11] => -2007 Veterans
            [2] => -Chit Chat
            [5] => -Car Pool
            [6] => -Car Wash Trading
            [21] => -Video Critique
        )

    [#description] => 
    [#multiple] => 0
    [#size] => 0
    [#weight] => -10
    [#theme] => taxonomy_term_select
    [#required] => 1
)

With this it's apparent why the line removed in the patch would cause errors. I guess it's meaningless to discuss further unless I'm also running the 5.x-2.x-dev version (currently running 5.x-2.0... if the dev version is fixed, why not make another release?) Despite my reluctance to do it on a production site, I'll install the 5.x-2.x-dev and see if the error is fixed.

Paul Natsuo Kishimoto’s picture

With 5.x-2.x-dev the warning: key() and warning: array_key_exists() messages appear again. If I make the same changes as in the patch, they disappear and the form works properly.

rconstantine’s picture

What the???

Why are your "Option key:"s blank? And why aren't your "The rest"s fully defined objects like mine? Did you just not copy the full output? If your underlying structure is not like mine, then no wonder the standard code isn't working. How did you create the forum structures? This is really strange.

Um, let me think. Okay, the taxonomy or regular forum module is creating the drop down for forum selection, not this module. This module is just supposed to remove irrelevant entries from that list. So, are you using some other module that overrides or hacks either the taxonomy or forum modules besides this one?

Also, did you create your forum vocabulary from scratch, or did you use the normal methods? At some point you should have gotten a message stating that the forum vocabulary had been created without your having to create a vocab called 'forum'.

As a side note, in my modules, I use dev for bugfixes and when a number of them accumulate, I release an official version. This is because I update my modules much more often the the average module maintainer. Otherwise I'd be at version 100 with this probably. Also, there usually won't be experimental (i.e. non-functioning) code in them.

Paul Natsuo Kishimoto’s picture

It is really strange. If you look at the last <code> block I included in #15, you can see the structure of $form['taxonomy'][1] that's being manipulated. The value keyed by '#options' appears to be an array with integer keys and string values... no stdObjects to access.

Yes, the forum vocabularies were created in the normal way. The only possible culprit I can see is forum_access or its dependency acl. Other installed modules are cck, custom_breadcrumbs, date, image, nodeteaser, og, og_galleries, privatemsg, smileys, token, update_status and views.

I'll take a look at forum_access and see if I can find it modifying #options arrays anywhere.

Paul Natsuo Kishimoto’s picture

Status: Needs review » Active

Alright, it turns out forum_access was the culprit. I think I fixed the problem: #175957.

However, because forum_access iterates through the contents of the $the_rest->option before og_forum gets ahold of it, the internal array pointer is at the end of the array and key() returns NULL, with the result that the "Forum:" drop down on the form is empty. More information at http://www.php.net/key, see comments by "php_notes at ionws dot com" and "mkeller at gmx dot de" (at least, I think this is what's happening)

So, I had to modify:

          $option_key = key($the_rest->option);

to

          end($the_rest->option);
          $option_key = key($the_rest->option);

around l.679. The form now works properly. I realize it's a compatibility issue and not a problem with og_forum per se, but I don't think it would hurt to add that line to the code. Then I wouldn't have to patch og_forum just to make it work with forum_access :)

Also, the forum_access code in #175957 makes it seem like there could be multiple elements in $the_rest->option. I don't know if that ever happens, but og_forum might give unexpected results if it does.

rconstantine’s picture

Category: bug » feature

Thanks for your diligence in tracing this error. Although access modules play nicer with each other than ever before, there are obviously still issues with other module interaction. I am changing this from a bug report to a feature request.

Why are you using end() instead of reset()? That just doesn't seem right to me, though I haven't tried it. Could you try it with reset instead? As I'm sure you know, reset points the to the first element of an array whereas end points to the last element.

rconstantine’s picture

Status: Active » Fixed

I lost somewhere in reading what this thread is supposed to be about. Could someone that still has a concern open a new post regarding whatever they are concerned about please?

Marking this as fixed since I believe I fixed something somewhere.

Paul Natsuo Kishimoto’s picture

Re: #20, reset() works just as well. I simply chose end() because it was in the examples from the PHP manual that I linked. However, my last paragraph in #19 still applies.

Either way I'd like to see this added to the code for compatibility with forum_access, but I agree the original issue is fixed. Thanks for the help!

rconstantine’s picture

Okay, so open a new issue so I don't lose your request.

Anonymous’s picture

Status: Fixed » Closed (fixed)