Integrate with og-7.x-2.x

Hi, I've recently started a project that uses subgroups for og-7.x-2.x. I'd like to help however I can. I added just a couple small fixes to the patch in #3.

The patch in #3 worked for the most part. However it was using OG_REQUIRED_CTOOLS_API which I og doesn't seem to use anymore, and in og_subgroups_get_reverse_hierarchy_tree_perm() $group hadn't been switched over to $entity.

I would like to test these patches, but maybe you can give an indication what version is being patched, since I don't see a 2.x version on the project page.
Also, contains the #4 patch all the other patches above?
thanks

The version of og_subgroups is 7.x-1.x-dev, to be used with og-7.x-2.x-dev. And yes, #4 does include all previous patched from this issue.

Hi guys, thanks for the work.

Let's remove all the TODOs, and the extra spaces, so it can go a proper review (the current patch looks it needs a lot of cleanup)

any progress? this would be great to have with og-7.x-2.x

I will be working on this over the next few weeks, so I will try to get a reviewable patch up shortly. There are some missing features I need as well, which I'll outline soon.

I wrote this mainly to clear my own head on what I was trying to do. Any feedback is welcome. I'm hoping to have this done by next weekend.

Since og-7.x-1.x and 2.x both have subgroup capability built in (building hierarchies), I think we just need to add a couple of features.

Proposed features:
1) Propagate 'view' node access to all subgroups. This is already implemented in previous patch (though I've revised it a bit).
2) Propagate 'og_membership' both up and down group tree when:
a) User is subscribed.
b) When group node is saved.
3) Add a user interface to control features in #2.

I already did the following:

1) Removed ctools and graph api dependency, since this seems like it should be included in a separate module.
2) Fixed og_subgroups_node_access_records_alter() to use og_access's node grant realm.
3) Removed og_subgroups_node_grants_alter(), since it's uneeded after #2 fix.
4) Removed unused functions.

Still need to:

5) Add UI for membership propagtion.
6) Add membership propagation functions.

I'll be submitting a patch here next week sometime.

Wow, kenianbel, that sounds great. I think that would make one of the most useful og modules that really makes og 'organic'. Great work, thanks! A user interface for managing the memberships in the tree makes it also very useful.

A new and improved membership admin UI would be nice (autocomplete, tag style add/ remove members to group?).
Maybe possible to build such a feature during your work at that points?

5) Add UI for membership propagtion.
6) Add membership propagation functions.

I removed the node grants code completely after implementing a user membership sync feature. I still haven't tested it thoroughly though, so please test and let me know what works and what doesn't.

Basically the patch does the following:

1) Allows propagating group membership of any group content entity (user/node/comment, etc) up or down a group entity membership tree when a membership is inserted or updated.

2) Adds a 'user sync' feature, which syncs subscribed users between group content and groups when a new membership is inserted or updated.

I still need to figure out how deleting memberships should work out for propagation.

Patch attached.

@JohnnyX

As for adding a new membership UI, I think that would need to be requested separately in the OG issue queue. I know for this project, I wouldn't want group admins to be able to use autocomplete for user names, since this gives them access to user information they don't normally have.

Seems nobody needs a improved membership UI, but it's necessary for a G+ circle like feature with OG. OG have all needed features, but no simple/ nice UI to add/ remove members to a group (=circle/ friend list).

I'm no module developer. So I have to wait until someone needs it to.

Thanks to write me a short reply! :)

#13: Should this patch be applied to the 7.x-1.x-dev version of Subgroups?

Great that this is being worked on by the way.. Thanks for the effort! :)

Yes, sorry for the confusion in naming, I figured I'd add 2.x since it seems like 1.x is for og-7.x-1.x. I've actually done some more work on it as well, as there were some issues with the above patch with user syncing adding user memberships to non-group content.

Currently I'm using this for a course management system, where user (student) membership is propagated to all subgroups. (Units and Lessons). I'm not using content (non-user) propagation at all, nor am I sure if there is a use-case for this functionality. If anybody can test this out it would be great!

New patch... I re-added the og_user_access_alter code, didn't realize it was needed for access propagation, since I assumed propagating membership would also propagate access roles. But if a member is made an admin in a parent group, this isn't propagated down. We may want to use a different method in the future that actually propagated roles when there is a role change, rather than using og_user_access_alter.

I also added some function name fixes and a fix to the problem discussed in #17. Please test!

NOTE: All my patches are patched against 7.x-1.x.

What would it take to get this moved into the dev branch? It would be nice so that issues can be generated and tracked outside of this thread (...I haven't actually found any).

Great job by the way!

I'd also like to get this moved into it's own 2.x-dev branch. I'm starting to test the current patch on my dev site and I'm coming across a few bugs that would be better discussed in a separate issue.

@Amitaibu: I don't know if you have time, but if you could look over my code I'd really appreciate it. I'm not completely familiar with the inner workings of og, other then skimming the api and og.module code.

Last patch I promise... This one adds membership deletion propagation, as well as cleaning up a bunch of code. As far as I can see it works perfectly for my use case, so I probably won't be adding anything else to it after this patch.

I think the only thing left to do is:
1) Add group_access and group_content_access field setting propagation.
2) Figure out how to prevent infinite recursion when an up and down direction is selected.

Since I'm only using it to propagate and sync user membership downward, recursion it isn't an issue for my project. Maybe someone else can jump in here?

@kenianbei,
Thanks, can you please clean the patch, so it follows Drupal's coding standards (no tabs, spaces were needed, etc'), otherwise it's hard to review.

Now passes coder review.

Haven't looked in logic details, but lets cleanup properly before an implementation review.

+++ b/og_subgroups.admin.incundefined
@@ -0,0 +1,108 @@
+ *   Administration settings for OG Subgroups.

Wrong indentation.

+++ b/og_subgroups.admin.incundefined
@@ -0,0 +1,108 @@
+ * Menu callback for admin settings form

Missing dot in end of line (an other places, won't go over each one).

+++ b/og_subgroups.admin.incundefined
@@ -0,0 +1,108 @@
+  $form['og_subgroups_settings']['notes'] = array(

Why do we call this "notes"?

+++ b/og_subgroups.admin.incundefined
@@ -0,0 +1,108 @@
+    '#prefix' => '<p>',
+    '#suffix' => '</p>',

Let's remove this.

+++ b/og_subgroups.admin.incundefined
@@ -0,0 +1,108 @@
+      $form['og_subgroups_settings']['#description'] = t('You currently have no group content types. You can add a group content type by visiting the !link page.', array('!link' => l('OG Field Settings', 'admin/config/group/fields')));

The text inside the l() is missing a t().

+++ b/og_subgroups.admin.incundefined
@@ -0,0 +1,108 @@
+          '#title' => $group_bundle_label . t(' Group Content Propagation'),

XSS... you need to use t() with @label

+++ b/og_subgroups.admin.incundefined
@@ -0,0 +1,108 @@
+            '#title' => $group_content_type . ' Entity Bundles',

XSS also here. And missing t(). Won't cover other places we have this.

+++ b/og_subgroups.admin.incundefined
@@ -0,0 +1,108 @@
+        }   ¶

Trailing space (and other places).

+++ b/og_subgroups.admin.incundefined
@@ -0,0 +1,108 @@
+  // Add a custom submit handler in case we want to add custom features later on.
+  $form['#submit'][] = 'og_subgroups_settings_submit';  ¶

Not needed.

+++ b/og_subgroups.admin.incundefined
@@ -0,0 +1,108 @@
+ * Custom submit handler for og_subgroups_settings().
+ */
+function og_subgroups_settings_submit($form, $form_state) {
+
+  // Save the form state values to a system variable.
+  variable_set('og_subgroups_settings', $form_state['values']['og_subgroups_settings']);
+}
+
+
+

Not needed, and please remove all empty lines.

+++ b/og_subgroups.moduleundefined
@@ -90,118 +57,422 @@ function og_subgroups_get_reverse_hierarchy_tree_perm($entity_groups, $string =
+ * without the -need for him to be a member in the groups.

-need (remove hyphen).

+++ b/og_subgroups.moduleundefined
@@ -90,118 +57,422 @@ function og_subgroups_get_reverse_hierarchy_tree_perm($entity_groups, $string =
+  $field = field_info_field(OG_AUDIENCE_FIELD);

In branch the field name isn't hard coded. Check og_get_group_audience_fields()

+++ b/og_subgroups.moduleundefined
@@ -90,118 +57,422 @@ function og_subgroups_get_reverse_hierarchy_tree_perm($entity_groups, $string =
+          $new_membership = og_membership_create($type, $gid, $og_membership->entity_type, $og_membership->etid, OG_AUDIENCE_FIELD);

Better use og_group()

+++ b/og_subgroups.moduleundefined
@@ -90,118 +57,422 @@ function og_subgroups_get_reverse_hierarchy_tree_perm($entity_groups, $string =
+    if ( !og_get_membership($og_membership->entity_type, $og_membership->etid, 'user', $uid) ) {

Coding style.

+++ b/og_subgroups.moduleundefined
@@ -90,118 +57,422 @@ function og_subgroups_get_reverse_hierarchy_tree_perm($entity_groups, $string =
+      if ( $new_membership->gid == $uid ) {

Coding style.

+++ b/og_subgroups.moduleundefined
@@ -90,118 +57,422 @@ function og_subgroups_get_reverse_hierarchy_tree_perm($entity_groups, $string =
+function og_subgroups_get_hierarchy($entity_type = 'node', $entity, $etid, $propagation_setting = array(), &$tree = array()) {

You can remove = 'node' form function signature.

+++ b/og_subgroups.moduleundefined
@@ -90,118 +57,422 @@ function og_subgroups_get_reverse_hierarchy_tree_perm($entity_groups, $string =
+function og_subgroups_get_users_group($entity_type = 'node', $entity = NULL, $states = array(OG_STATE_ACTIVE)) {

What's the difference between this an og_get_entity_groups() ?

+++ b/og_subgroups.moduleundefined
@@ -90,118 +57,422 @@ function og_subgroups_get_reverse_hierarchy_tree_perm($entity_groups, $string =
+  $query->entityCondition('entity_type', 'og_membership', '=');
+  $query->propertyCondition('entity_type', 'user', '=');
+  $query->propertyCondition('group_type', $entity_type, '=');
+  $query->propertyCondition('gid', $id, '=');

You can chain it together.

+++ b/og_subgroups.testundefined
@@ -1,5 +1,9 @@
+ *   Testing functions for OG Subgroups.

Wrong indentation.

+++ b/og_subgroups.testundefined
@@ -73,47 +74,49 @@ class OgSubgroupsAccessFromDescendanceTestCase extends DrupalWebTestCase {
-   // $permissions = array(
-   //   'delete own article content' => 1,
-   //   'administer group' => 0,
-  //  );
- //  og_role_change_permissions($rid, $permissions);
+    // $permissions = array(
+    //   'delete own article content' => 1,
+    //   'administer group' => 0,
+    //  );
+    //  og_role_change_permissions($rid, $permissions);
 
-  //  $this->assertTrue(og_user_access($group->gid, 'delete own article content', $user2), t('User still has "delete own article content" permission.'));
-  //  $this->assertFalse(og_user_access($group->gid, 'administer group', $user2), t('User no longer has "administer group" permission.'));
+    //  $this->assertTrue(og_user_access($group->gid, 'delete own article content', $user2), t('User still has "delete own article content" permission.'));
+    //  $this->assertFalse(og_user_access($group->gid, 'administer group', $user2), t('User no longer has "administer group" permission.'));

If commented out, you can remove -- or better - improve.

Using og-7.x-2.x and entityreference-7.x-1.x-dev (complete new drupal install & fresh database) and patch from #23 I get the following error when I add a user to a sub-group:

OgException: OG membership can not be created in entity user and bundle user using the field og_group_ref as the field does not exist. in OgMembership->save() (line 46 of [path]sites/all/modules/og/includes/og.membership.inc).

I had configured sub_groups so that "Parents: A User will be automatically subscribed to all parent groups of a [sub-group]" is checked.

After the error message, the user does become a member of the sub-group but not the parent group. I'm guessing, perhaps wrongly, that the field has not been created since I'm on a fresh install? If i disable og_subgroups then the user is added fine to the sub-group (obviously not the parent) - no error message.
Happy to test an updated patch.

Hi Amitaibu,
I took the patch from #23 and tried to fix all your remarks.

Fixing the hardcoded OG_AUDIENCE_FIELD fixed the error in #25 too.

About og_subgroups_get_users_group() vs og_get_entity_groups(), the first one returns the users of a groups, the second one returns the groups of a user.

Currently the patch mostly works. Roles are not propagated, while I think they should. But more generally, is duplicating memberships really the best approach?

Thanks,
Marco

@marco, I was not able to apply your patch against the current 7.x branch and tried multiple ways to apply it but no luck. Can you retest the patch to see if there is an issue and re-roll if need so that I can test it out - thanks!

@blainelang, hi, thanks for trying the patch, I'm sorry it didn't work.
I fixed the strip level, you should now be able to apply this patch either with git apply or with patch -p1.

Thank you!

@marco, the patch applied and just wanted to confirm my setup is correct and provide some feedback.

• Created a new content type called 'hierarchical group' and checked off the is group option
• Added the field 'audience' to the new content type 'hierarchical group' content type
• This then adds the 'is group content' option as well in the content type
• Added a new node of type 'hierarchical group' - called 'Group 1'
• Added a new node of type 'hierarchical group' - called 'Group sub 1.1' and set the 'Group Audience' field to 'Group 1' - it's parent. I believe this is how you setup the hierarchical group structure
• In the OG Sub Groups config settings - under the hierarchical group settings, enable the Parents option under 'User Entity Bundles' and the Sync option
• Tested adding a user to the sub group - works! I do see the user membership propagated to the parent(s) - tested with a 3 level hierarchy as well.

Tried enabling both options for the User entity and at one point, the site would hang and reset (no errors) when adding users to a group. Got a feeling it was in a loop and it never saves the group node edit - no errors logged.

If we are managing access to content via group membership, why or when would you use the settings in OG Subgroup config for the non user entity - as in enabling the sync on article for example?

Should you be able to propagate the users up and down the hierarchy?

Thanks!

@blainelang

I think I had mentioned this above in #21, infinite recursion happens when an entity is propagated both up and down the tree. Unfortunately I don't have time to fix this, since the project I wrote this for doesn't use the functionality.

Another issue I encountered is that when a group is removed, the synced users stay with the group. So when you add that same group to another group, all those users follow the group. I will try to update the current patch when I get time to fix this, by having the added group relinquish all other og memberships before adding the new memberships from parent groups.

Should you be able to propagate the users up and down the hierarchy?

I think this is already happening correctly, if I understand your question correctly. When you check sync users for a bundle, when an entity is added to that bundle type group, all users that group has is transferred to the new entity.

This definitely still needs a lot of work... I hope we can get more developers working on this!

Patch fixing the user sync issue mentioned in #30, as well as fixing an unneeded arg in og_subgroups_get_hierarchy().

@kenianbei, thanks and I may have some time to work on this once I better understand the inner workings. Thanks for your quick reply and I had read your note a few days earlier on the recursion issue - just forgot when I was summarizing my testing.

Above was missing og_subgroups.admin.inc.

Any chance we could convert this into a 7.x-2.x-dev release? I think more people would test and contribute if we did that. Also, it would be easier to test if we could just download a new release instead of re-patching.

I'm going to test this as soon as I re-patch — of course last night I was applying the patch from #28 at the same time new patches were being posted in #31 and #33! Oops.

@kenianbei: I was seeing an error when adding a sub group (not a top level) being triggered because of an empty array so I've added some validation in og_subgroups_og_membership_sync_users() and re-rolled the patch. I tested the patch using patch -p0 (for some reason git apply did not like it).

Did more testing and wondered if this should be the desired behaviour:

• Add a new sub-group and then add users to it
• Edit that sub-group and move it - change the Group Audience to another top level group
• The previous users are no longer members - good
• Add one or more different users to the group again
• Edit the group again and move it back to it's previous parent group
• Examine the group membership and the original users are now members again. So as per your note in #30 above, the users still stay with the group but only when moved back to their original parent.
• The parent group (since I was replicating to parent) still has the user as a member even after moving
• If I now move the sub-group back the 2nd parent, then the users I had added then are now members and not the first setup members (if that makes sense - but matches what happened in previous move test back to the original parent)

Should we remove users and any replicated relationships when moving folders? This may be fine in general but I wonder if membership to the replicated groups (as in parents) should not be removed or be an option.

Any feedback or insight to my earlier question?

If we are managing access to content via group membership, why or when would you use the settings in OG Subgroup config for the non user entity - as in enabling the sync on article for example?

Please convert this into a 7.x-2.x-dev release

Yes if someone would be willing to roll a full version that would be really helpful, even if it has to be attached to this issue for now (I'm not sure whether anyone who's working on this has maintainer rights!) I'm having trouble applying the patches (I think my patch tool doesn't like patches that create a whole new file and it's getting wonky); even though I've done a bunch of manual cleanup I'm not confident I've correctly incorporated all the changes.

I really want to test but I don't want to be waste people's time reporting issues that are just a result of an incorrectly applied patch, so I'd like to be confident I'm testing the code as it's intended. I so appreciate all the work everyone has done on this issue so I hate to ask someone to go to the trouble, but I do think you'd get more testers with a full dev version.

@blainelang

I'm not sure I understand the question, but I'll give it a go:

If we are managing access to content via group membership, why or when would you use the settings in OG Subgroup config for the non user entity - as in enabling the sync on article for example?

To give a use case: the original site that spawned the patch was a course management site. The group structure went like this: Course => Unit => Lesson. In the subgroups settings, under 'Unit' bundle, I set lesson so that when a lesson was added to a unit, it would also be added to the parent group: course. This way anyone who was subscribed to the course could see the new lesson.

I'm not sure if other people would have better use-cases than this, and actually later on I abandoned using the node propagation settings and switched to a completely different method that included the ability to infinitely nest lessons rather than using a 3-level structure (course-unit-lesson). This code is rather use-case specific so I didn't rewrite the patch.

Currently I'm only using the user sync feature of the current patch for this site.

I'm curious if anybody else has use-cases for non-user propagation, or if we should just get rid of it and focus on user propagation?

Unfortunately I didn't really have a plan when creating this patch, I was just adapting it to suit my needs at the time, and now I'm not even using the original code I wrote (except for user-syncing).

Should we remove users and any replicated relationships when moving folders? This may be fine in general but I wonder if membership to the replicated groups (as in parents) should not be removed or be an option.

Personally I think we should remove all user memberships when the group is unattached from a parent group. Otherwise you would remove the subgroup from the parent group, thinking that users who are subscribed to the parent group can't see the subgroup, but actually they would still have member permission with the subgroup.

Just as an afterthought, perhaps we can build in a more robust set of permissions and settings for controlling how user syncing and propagation works, currently I've just included code to sync users with the attached subgroup, there is no propagation up or down the tree, nor any way to fine tune the syncing/propagation.

Small fix in og_subgroups_og_user_access_alter() so that we are using {} for dynamic property.

+  if (isset($context['group']->{$field_name}[LANGUAGE_NONE])) {
+    foreach ($context['group']->{$field_name}[LANGUAGE_NONE] as $delta) {

@kaniabei: I agree that non-user propagation like infinitely nesting content is a very common way to use subgroups, I currently have in nearly all my og sites things like department>program>course>module>content etc, some of which are groups, but which all must be filtered by one or a combination of entity references.

I was just testing the content propagation option and understand it better now. I created a view to show the content for the user context of the group being viewed. So if viewing the 'Course' or 'unit' group, the content added to the 'unit' group was also visible in the content view - assuming the user was also a member of the parent group.

The propagation worked when the content node 'article' was initially created but if you edit a node and move it, then the content propagation is not updated.

Here are the results of my test:

• I am using the patch from #40.
• I have a preschool site with three types of groups: a "Community" which is a parent of the other two group types (there's only one node of this type), and "Classroom" and "Volunteer Team" groups which are children of the Community group.
• The Classroom and Volunteer Team groups are set to propagate their users to the parent group, so that everyone who's a member of a classroom or a team will automatically become a member of the community.
• There are no content propagation settings on the Community group and no node entity bundle propagation settings on any group.

When I create a new user as a member of one of the child groups that's set to propagate users to the parent, I get a "502 Bad Gateway" error when I save the user and the user is not created.

If I create a new user who's only a member of the parent group, or if I create a new user with OG Subgroups disabled, the user can be created successfully.

I'm running on a Pantheon development server FWIW.

Hi
I found it miss a hook_permission implementation

/**
 * Implements hook_permission().
 */
function og_subgroups_permission() {
  return array(
    'administer og subgroups' =>  array(
      'title' => t('Administer organic subgroups'),
      'description' => t('Administer membership propagation for group types'),
    ),
  );
}

and remove . in hook_menu

 'access arguments' => array('administer og subgroups<strong>.</strong>'),

Sorry, but I don't know how publish a patch...

I am testing a patch at #40 and seeing some odd things...
If I set Entity selection in one content type`s reference field it changes globally for all content types.

@johaziel, How did you find out about this missing hook permission, what problems would appear? Thanks

guys, may you provide us with one final patch which contains all bug fixes above?
thanks

@yuri : I found that because only the user 1 can have access to admin/config/group/subgroups and not another administrator...

@yuri yes @johaziel is right, cus other administrators got "Access denied" message on subgroups admin pages, but also in my case implementing hook_permission didn't fix it, i'm trying now some other work around, if anybody found a solution for that please provide us with..
thanks

Tested it with the hook_permission change on #40, and it seems to be working really well: also, I specifically created users on children, and it propogated to parents without any errors.

@POVYLAZZZ's I can't replicate your problem, but if you're using the same entityreference field in multiple places than this will happen, yes, because some field-settings are global. You need to make your content type a group, and then ADD another, different entityreference OG field to define the relationship.

@Amitaibu, can we get a 7.x-2.x version? :) Do you want a re-rolled patch for that?

It seems there's a sandbox now: http://drupal.org/sandbox/porciletto/1870206.

Yes, that's my sandbox, I'm investigating whether it's possible to have og_subgroups 7.x-2.x without replicating memberships.
Other features:
- I'm focusing on user inheritance only
- it uses a field to set which groups allow for their subgroups to inherit users
- it shows which users are inherited (and from which entities, and which roles) in the og admin page
- it detects loops
- it's integrated with the node access system

It uses just a function of the patch above. I'm still working on it and it's not ready yet (feel free to test it if you want).

I'm still not sure if replicating memberships is a good idea or not. On one hand some things are easier. On the other hand duplicating and keeping in sync can cause weird errors if anything goes wrong. Performancewise, traversing the tree on the fly should be heavier of course, but with the patch above the number of membership entities is about number of leaves * depth of tree, which could lead to big numbers. It all depends on the complexity of the structure.

Another thing to consider is how that would work with things like Views. Right now there isn't very much role integration into Views, but reusing roles would have to be specially accounted for.

@marco,
Can you attach a patch here, it's easier to review.

> I'm still not sure if replicating memberships is a good idea or not.

IMO, Probably not a good idea, even in the price of performance. (btw OG is now integrated with Entity cache).

I'm still not sure if replicating memberships is a good idea or not.

The reason I went that route with original submitted patch was due to the specific project's needs. I needed a way to have two groups that are connected indirectly to share the same members and perms. For example:

Course -> Unit are grouped.
Unit -> Lesson are grouped.

Course -> Lesson are NOT grouped.

I needed a way for Lesson to inherit Course's members and perms. That's the main reason I added membership replication.

@Amitaibu I attach the patch to 7.x-1.x
I recently added support for og_user_access()

To test:
- add a user inheritance field to the groups
- create groups and subgroups
- user access and permissions should be inherited

I know that patch is not ready yet, I'd just like to discuss whether this is the right strategy or not.
For example, as Renee S noted, there's no integration with Views or the Groups audience widget, or anything else that uses og_get_entity_groups() w/$user or og_get_groups_by_user(), for example, and I'm not sure how to solve this cleanly.

Thanks!

The attached patches are an experiment, I added two hooks to og_get_entity_groups() and og_get_groups_by_user() to add the inherited groups, and modified my patch to use them. I didn't test much and they might not work at all.

> I didn't test much and they might not work at all.

That's not a good pitch for a patch :) Better test it, before you want the community to review it.

Sorry, I should have explained better: the patches in #57 work, and for example now you can see the inherited groups in the audience widget, I'm just afraid of some unwanted side effect (and they don't have Views integration). The patch in #56 seems to work well (with permission and access system), but they miss integration with the widget, Views and probably more.

Generally speaking I'm not asking for a formal review right now, I wouldn't even have posted a patch if I wasn't asked to, I'm just looking for kind opinions on whether this is the right way to solve this: I'd just prefer not to work a lot in the wrong direction. But I understand time is valuable for everyone, and if you prefer not to look at experiments, please don't feel compelled to: things will eventually fall into place.

Thanks,
Marco

I would love to test this but with so many patches, not sure where to start.

Ugh, this issue is a bit heartbreaking. :(

So it looks like kenianbei spent a good chunk of time working on this, going through review process, etc. Then marco started a separate module at http://drupal.org/sandbox/porciletto/1870206 which as far as I can tell doesn't use any of kenianbei's code (is it written from scratch?). As marco notes, it does include things that are not in kenianbei's version, such as Views integration, which is good. But two separate efforts at the same problem are not. :(

I'm with the folks who are recommending just committing something to the 7.x-2.x branch so we can open up testing to a wider audience, and split subsequent bug fixes/feature requests out into their own issues, rather than cobbling together two competing mega-patches. kenianbei's code has had more review, so probably makes sense to start with #40 and then selectively pull in the changes from marco's sandbox that make sense. Maybe even granting them both co-maintainer access to the D7 version, since it seems there's been no commit activity on the D7 version of this module since 2011?

I think part of the problem is that og changed so much from 6.x to 7.x and then 7.x-1.x to 7.x-2.x, and so it was hard to know what functionality needed moved over, and what use cases we should be building for. When I submitted my original patch I wasn't really sure what use case to write it for, thus it contains a lot of functionality that probably isn't needed.

After having a brief look at marco's code it looks like his patch is a better match for my current use-case (a university course management site) and probably most other people's use-cases. I think if we commit something it should start from marco's code.

I plan on updating to marco's code on my dev site and see how that goes. I'll post any fixes/additions I come across. Hopefully we can get a 7.x-2.x branch soon!

Also I have no issues serving as a co-maintainer for this as well.

@marco: I tested your patch, some comments:

1) The configuration and UI is better (uses a field that is set up like other og fields).
2) Permissions propagate fine for viewing and updating.
3) Subgroups that inherit permissions don't show up in the og audience field when adding a new group.

#3 is a major issue for me. I checked and it looks like og audience field is using a class from entityreference, getReferencableEntities. Perhaps we can alter this behavior to include subgroups. Not sure the best way to go about this though, any thoughts on this?

Just to summarize the major differences between the 2 patches:

kenianbei's patch:
#1: Propagates user permissions by creating new og_membership entities.
#2: Configuration using a separate og_subgroups settings page.
#3: Allows automatic grouping of new entities to all subgroup or parent entities, e.g you add a subgroup to another subgroup, and it automatically is grouped with all parent groups, or all child groups.

marco's patch:
#1: Propagates user permissions using og_user_access_alter and node grants (no new entities).
#2: Configuration using og fields settings page.
#3: No automatic group feature.

Those are the major differences. I think with the inclusion of entity_prepopulate in og, #3 is not really needed. However if there are a lot of people who need the functionality perhaps we can keep that code from my patch.

It would be nice to have entity prepopulate. You should keep it.

I was able to use marco's patch with my site without any major errors/issues. I can submit a reviewable patch if I get the go ahead from any og_subgroup maintainer(Amitaibu) or webchick. Just don't want to put anymore work into cleaning it up until I'm sure this is the route we are going.

I'm not a maintainer but after reading 64 I think you should move forward, I'm happy to review your patch.

Cleaned up patch attached... i can haz 2.x branch now?

Awesome!! Thank you SO much for doing that!! :D I notice some things (like LICENSE.txt shouldn't be there), but they can all be cleaned up post-commit.

I think what probably makes sense at this point is to follow the Abandoned projects process... in brief, file an issue stating your intent to come on as a co-maintainer, point off to this patch as evidence of your work, and ping the various maintainers via IRC/contact form.

I tried the patch and it works, good job. Attached patch removes the license file and fixes some comments.

I can't seem to apply #70. From 7.x-1.x I do

curl http://drupal.org/files/i1536178-70.patch | git apply

error: og_subgroups.common.inc: No such file or directory
error: og_subgroups.views.inc: No such file or directory

can you try the patch from #68

issue opened: http://drupal.org/node/1938472

I have created a 7.x-2.x branch and committed the module with the patch from #68. I have also made kenianbei a maintainer.

YAY!!!!!!!!!!!!!!!!!!!

Thank you so much bschilt!!

Not sure the proper status of this now. Moving to needs review. :)

Hey, I'm trying to use this module but I am stuck.
When I create a group, there is no where to indicate that this is a subgroup. When I go to configurations and look under the ORGANIC GROUPS section, all I see is the following:
OG field settings
Configure Organic groups fields in bundles.
OG permissions overview
Overview of Organic groups permissions.
OG roles overview
Overview of organic groups roles.
OG settings
Administer OG settings.

I don't see any settings for subgroups.
Can anyone tell me how to use the module? Thanks.

I downloaded the 7.x-2.x-dev release from the date March 10, 2013.

I'll try to get a useful readme up when time permits, but the basic procedure is to go to OG Field Settings, then choose the bundle, then choose the field: OG Inheritance Field and save. Then you can go to the content type and change default settings in the edit tab. Hope this helps!

thanks kenianbei it worked.

Not sure if this helps but here is a summary

Can USERS in a Parent Group be inherited by a Sub-Group?
YES
Set the Organic Group settings for your subgroup Content Type as 'Group' and 'Group Content'.
Set the subgroup so the Parent Group is the Groups Audience
Set the Parent Group so "Group user inheritance" is "Yes - subgroups of this group will inherit its users Set the Parent"
OUTCOME
member of Parent Group has access to private content in Sub Group.
when a member is removed from Parent Group they lose that access.

Can CONTENT in a Parent Group be inherited by a Sub-Group?
UNKNOWN

Can CONTENT in a Parent Group be inherited by a Sub-Group?
UNKNOWN

Content in a parent isn't inherited, nor is content in a subgroup inherited upward to the parent. If there is a use-case for this we can add it, since I've written the code already. Just open an issue in the 7.x-2.x branch.

Do roles from parent transfer down to child? Like for example, do the admins of a parent group have admin rights in the child group?

The use case we were trying to model was as follows: there are a bunch of committees -each having their own OG where they can add Posts, events etc.

There is also an 'overview' group that should see all content from all of those OGs, but should not themselves show as members of the SubGroups nor be able to add content to the sub groups.

I had thought the approach would be to make the Overview group a Parent group and then have all content from the subgroups 'trickle up' with the hope that I could still prevent the Overview Group from adding comments etc to the child content.

But I may have a solution that works with it as it is, especially as my testing seems to suggest that the Roles do 'trickle down' - meaning that if I make the Members of the Parent Group such that they cannot make Posts, Events, etc in the Parent group, then they also cannot do that in the child group - which is kind of what we wanted. (nb we have set the Parent Group to override the default OG Permissions)

Also the 'inherited members' do not show on our Members Tabs in the subgroups - haven't looked at the View to see if that could be changed since it is the outcome we wanted.

Also haven't sorted the issue around permissions for OG Comments module but hopefully will take a look at that next week, as it seems that overriding OG Permissions isn't having any effect on OG Comment Permissions currently.

So for now, I won't push you to get the code for content inheritance out but also can't see any reason not to have it out there so we can try it and see what it adds.

Good to hear about these use cases, I will try to get something out in the next couple weeks. I'm currently travelling and won't be able to focus on this much until I get back. I plan on adding this, just need some time to go over the old code and make sure it fits with the new patch we started using of marco's.

Will update on status here: http://drupal.org/node/1956426

I'm in the thick of creating nested groups with the 7-2 dev branch, and I'm a little confused as to why user and content propagation only seems to go down from parent group to subgroup rather than the other way around (which is different than the screenshot on the module page which must be the D6 UI).

In my case, I've got groups that are Towns, and then subgroups that are Schools (there can be multiple schools in each town). The subgroup is meant to be smaller subset of the parent group - the School is just for members of a single school, while the Town is where the members of multiple schools (and non-school related users) and all their content rolls up. Put another way, the parent group should always have more members than any of its subgroups.

I guess I could use the current setup with the "Group user inheritance" field propagation down to subgroups by making the Schools the parent group and the Town the subgroup, but Towns need to be created first, and the town selected when a School is created...

@wmfinnegan: We rewrote this version of og_subgroups with a specific use-case in mind, i.e. a course-based LMS. For the people I've talked to who added input as this patch was developed, all/most user membership manipulations were happening at the top-most level, so it made sense to only propagate downward.

However, this week I've been working on implementing my original patch back into the patch that got committed, which will all user and content membership to populate both up and down. I'm not sure when I will get it finished though this week though, as it turned out to be more work than expected.

I'm in the same both as #84, we have a need to define sub-committees who's membership is then shared upwards to the committee. So whenever you get the patch mentioned done, we'd love to help out and test it.

Just to clarify, the use case is for having taxonomy that are attached to groups propagated up and down the group tree?

We're currently looking for just 'up' the tree. Our setup will look like:

-Organization
-->Committee A
---->Committee A Subcommittee 1
---->Committee A Subcommittee 2
-->Committee B
---->Committee B Subcommittee 1

where the members are added to "Committee A Subcommittee 1" or "Committee A Subcommittee 2" automatically become members of "Committee A", and members added to "Committee B Subcommittee 1" become members of "Committee B". Not all members of "Committee A" will necessarily be members of a subcommittee, but all members of a subcommittee will be members of the parent committee. And similarily all members of "Committee A" or "Committee B" will become members of the "Organization" group.

@jrreid: Cool, thanks for the use case. Once I add the entity propagation you should be able to do this with user memberships. I'm planning on adding another og field that sets whether the entity is propagated up or down, or possibly both if I can figure out the logic without having recursion issues. I tentatively plan to set some time this week to work on it.

This is now in 7.x-2.x so updated version

Been evaluating this to use it.

kaniabei: My suggestion is to close this issue as fixed and work on different issues in their own issue, else this issue would likely get confusing a long. For each new feature you have your mind, create an issue already, even w/o a patch, so those interested in can know what's happening and maybe contribute themselves (verbose in whatcha thinking to do and if working on it -- like using the assigned field -- so people don't end up working on a patch you're already in progress with).

@hefox: We are already opening separate issues for new feature requests, but good suggestion on closing this.