Content type editing resets default assignments

Okay, I have a little more info on this bug, though I haven't yet decided how to go about fixing it. (I'm going to try though, when I get more time.)

The problem starts, as Paul points out, in _og_content_type_admin_rebuild_table(). When a node type is saved, _og_content_type_admin_rebuild_table() gets called. The problem starts on line 1238:

        if (!og_is_omitted_type($type->type) && !og_is_group_type($type->type)) {

This if statement must evaluate to TRUE for the content type in question if the content_type_admin settings are going to be preserved. (Later in that if clause, the content type must get added to the $new_allowed and $new_active arrays.)

Problem is, og_is_omitted_type() is returning TRUE when it shouldn't, causing the if statement to not execute.

This is because og_is_ommitted_type() checks the variable table for a variable named og_content_type_usage_[your-content-type-name-here]. If it doesn't exist, og_is_ommitted_type() assumes the default, which is that the content type is "ommitted", and returns TRUE. Problem is, if your content type is a valid allowed group post, there should be a variable in the variable table called og_content_type_usage_[your-content-type-name-here], with a value of "group_post_standard". If there is, then og_is_ommitted_type() will see that, and return FALSE.

So why, if your content type IS a valid group post, is there no entry in the variables table indicating this when the if statement on line 1238 executes?

Turns out, organic groups itself deletes that variable entirely and then re-saves it when any edits are made to the content type.

So, execution goes like this:

1. You edit and save the content type.
2. In og.module, og_node_type() executes:

function og_node_type($op, $info) {
  switch ($op) {
    case 'delete':
      variable_del('og_content_type_usage_'. $info->type);
      break;
    case 'update':
      $usage = variable_get('og_content_type_usage_'. $info->type, 'omitted');
      variable_set('og_content_type_usage_'. $info->type, $usage);
      if (isset($info->old_type)) {
        variable_del('og_content_type_usage_'. $info->old_type);
      }
  }
}

(note the variable_del() call)

3. Then _og_content_type_admin_rebuild_table() runs, failing because the variable is gone.
4. Then, eventually, the node module's node_type_form_submit() function (found in /modules/node/content_types.inc) runs, restoring the variable.

So we're just in the unlucky situation that the relevant variable we need for this all to work is temporarily gone while og_content_type_admin is trying to do its job.

My brain hurts from figuring this out (I've spent the last two hours running Xdebug on this), so I'm in no shape to figure out what the best solution to this might be. But it must be possible to make this execute before or after the organic groups module does its business, right? I've tried changing module weights, but that hasn't worked so far for me. Or maybe there's another way. I'll give it some thought.

Ah ha! I was wrong. I had NOT tried setting the module weight of the og_content_type_admin module to something LESS than the weight of the og module. OG is set to 0 by default. So I went into the system table and set the weight of og_content_type_admin to -1, and voila, the problem disappears.

Now...

The big question is, does this CAUSE any new problems? I'm still not sure. Testing now...

Okay, it's been a couple of weeks, and my fix of lowering the module weight seems to work fine - I've seen no adverse effects so far on my setup.

If the weight change is the proper fix for this issue, then that should be set by default when installing this module (and on re-enabling it), plus a comment about this in the code.

Changing the weight from the default install +3 to -1 solved the issue.
It might be time to committ
Thanks

Can this be committed?