I believe a $node object needs to be passed to og_is_group_admin and not $nid. og_is_group_admin is expecting to test $node->nid and $node->type.


function og_announce_validate(&$form, &$form_state) {
  // Allow only group admins to post to announcement only groups.
  foreach ($form_state['values']['og_initial_groups'] as $key => $nid) {
    if (og_announce_is_announce($nid) && !og_is_group_admin($nid)) {
      form_set_error('og_groups', t('Only group administrators may post to this group.'));
    }
  }
}

The og_is_group_admin function:

function og_is_group_admin($node, $account = NULL) {
  if (empty($account)) {
    global $user;
    $account = drupal_clone($user);
  }

  // Modules can cause us to arrive here before og_init() has fired.
  // See http://drupal.org/node/285696
  if ($account->uid && !isset($account->og_groups)) {
    // Reload the user object.
    $account = user_load($account->uid);
  }

  return og_is_group_type($node->type) && (user_access('administer nodes', $account) || !empty($account->og_groups[$node->nid]['is_admin']));
}
CommentFileSizeAuthor
#2 og_announce-6.x-1.x-1298256-3.patch2.03 KBseandunaway

Comments

seandunaway’s picture

You're right... Trying to think of the best way to fix this. I don't really want to node load potentially hundreds of times.

I guess options are to build a dummy node object with only type/nid, or do a custom query, or maybe use the global user object to check for is_admin...

Thoughts?

seandunaway’s picture

StatusFileSize
new2.03 KB

Quick fix that seems to be working okay in my sandbox with hundreds of groups.

seandunaway’s picture

Status: Active » Needs review
seandunaway’s picture

Title: Incorrect validation test? » Incorrect validation test in og_is_group_admin().
Status: Needs review » Fixed

Committed that. Works okay until we can find something better.

[6.x-1.x a0c0603] Issue #1298256 by rump: Incorrect validation test in og_is_group_admin(). Thanks jason.fisher!
1 files changed, 23 insertions(+), 20 deletions(-)

jason.fisher’s picture

FYI I ended up making an empty $node stdClass with only ->nid defined, and passing that.. ;)

seandunaway’s picture

Post the diff, lets get it in. :)

Status: Fixed » Closed (fixed)

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