Group privacy as 2nd layer of content access control

This isn't so much a feature request as a suggestion. Currently OG already supports one layer of group content access control, at the per-node level, and it works well. The site I'm working on required a group-level public vs. private setting for viewing group content by non-members, and I've finally been able to implement that. Because I'm sure lots of other people are working on this issue right now, I'm wanting to share what I've got now before an official solution is added to OG, in case it's useful. And if anyone sees any problems with what I've done, suggestions are welcome.

My code makes it so that groups have 2 layers of content privacy: group-level and the existing node-level, with group-level taking priority. If a group is public, all it's content is viewable regardless of a node's privacy setting. If a group is private, nodes that are set as public are still viewable, while nodes that are not set public are not. I have not addressed the issue of group home page privacy yet. That's a whole other can of worms.

My site only allows nodes to be affiliated with a single group, so I may be missing something subtle in my solution to this. I'm also still missing any admin settings to control this behavior, but I'll be working on that for my site eventually. I don't have time to make a patch at the moment because there's too much other custom code in my og.module and I'm busy at work, so for now I'll just put the new code here.

Here's the group form control:

<?php
  $form['og_privacy'] = array('#type' => 'radios', '#title' => t('Group Content Privacy'), '#default_value' => intval($node->og_privacy), '#weight' => -3,
                              '#options' => array(t('Public - All group content can be viewed by non-members'), t('Private - Only group members can view group content')),
                              '#description' => t('Should all of your group\'s content be readable by non-members?'));
?>

The additions to og_load_group(), og_insert_group(), and og_update_group() are straightforward (just a new column in {og}) so I won't print them here.

Here's the one place where this behavior is controlled. I found I only needed to modify og_node_access_records(), and implementing hook_access() should not be necessary (or even useful).

<?php
function og_node_access_records($node) {
  // don't write records if og access control is disabled or the node type is omitted or node is a group
  if (og_is_omitted_type($node->type) || !variable_get('og_enabled', FALSE)) {
    return;
  }

  if (og_is_group_type($node->type)) {
    // this grant allows group admins to manage stuff
    $grants[] = array('realm' => 'og_subscriber', 'gid' => $node->nid, 'grant_view' => 1, 'grant_update' => 1, 'grant_delete' => 1);
    // this one lets everyone see group homepage. see 'private groups' issue if you don't want this. we need help.
    $grants[] = array('realm' => 'og_public', 'gid' => 0, 'grant_view' => 1, 'grant_update' => 0, 'grant_delete' => 0);
  }
  elseif (is_array($node->og_groups)) {
    // applies to non group nodes

    //Added logic here to add an og_public grant if at least one of this node's associated
    // groups is set to be public at the group-level. It seemed to be necessary that
    // og_public is entered in the database before any og_subscriber grants.
    $any_public_groups = false;
    foreach ($node->og_groups as $gid) {
      $any_public_groups = $any_public_groups || 0 == (int)db_result(db_query("SELECT privacy FROM {og} WHERE nid = %d", $gid));
    }
    if ($node->og_public || $any_public_groups) {
      $grants[] = array('realm' => 'og_public', 'gid' => 0, 'grant_view' => 1, 'grant_update' => 0, 'grant_delete' => 0);
    }
    foreach ($node->og_groups as $gid) {
      // we write a broad grant here but og_node_grants() only issues it selectively.
      $grants[] = array('realm' => 'og_subscriber', 'gid' => $gid, 'grant_view' => 1, 'grant_update' => 1, 'grant_delete' => 1);
    }
  }
  return $grants;
}
?>

Unfortunately, if someone changes a group's privacy setting, the access grants for every node affiliated with the group must be manually updated. Since that's probably a rare case in the real world, I wasn't too concerned with the performance of this. Here's my changes to og_nodeapi():

<?php
    case 'update':
      if (og_is_group_type($node->type)) {
        //Added code to compare the new and old "og_privacy" value for the group,
        // and if the privacy setting changed, manually update the access grants
        // for each node affiliated with the group.

        //Get the previous value of the group's og_privacy field
        $old_privacy = (int) db_result(db_query("SELECT privacy FROM {og} WHERE nid = %d", $node->nid));

        og_update_group($node);
        // make sure the node owner is a full powered subscriber
        og_save_subscription($node->nid, $node->uid, array('is_active' => 1, 'is_admin' => 1));

        if ($old_privacy != $node->og_privacy) {
          $result = db_query("SELECT n.nid FROM {node} n INNER JOIN {og_ancestry} oga ON n.nid = oga.nid WHERE oga.group_nid = %d", $node->nid);
          while ($owned_nid = db_fetch_array($result)) {
            node_access_acquire_grants(node_load($owned_nid['nid']));	//manually update the node's access grants
          }
        }
      }
      elseif (!og_is_omitted_type($node->type)) {
        og_save_ancestry($node);
      }
      break;
?>

Any feedback will be greatly appreciated. And I'm also interested in knowing if this is the direction the OG developers were already leaning in or not.