In #1536178-80: Integrate with og-7.x-2.x, @kenianbei says

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.

So here's a use case for content inheritance:
I have a site for a school, and each grade has their own organic group. The grade groups are sub-groups of an all-school group. It would be great if items from the all-school calendar could be visible on the calendar for each grade, so you don't have to look at two different calendars to get your child's schedule. It would also be good (I'd actually assume this is how it would work, but maybe this is one of those things that depends on whether you're using multiple group reference fields or not) if you could choose this by content type, so that, for instance, calendar items would inherit from the parent group, but newsletters wouldn't.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

BrightBold’s picture

Oops. Forgot to preview. That link should be #1536178-80: Integrate with og-7.x-2.x.

kenianbei’s picture

Assigned: Unassigned » kenianbei

I plan on adding this in the next couple of weeks, will update here when it's in.

BrightBold’s picture

Awesome, thanks. I'll look forward to it.

Brian MacKinney’s picture

kenianbei, I'm looking forward to this feature as well, because I think it will create og_vocab inheritance as well. Please tell me if my logic is correct:

Currently group types are:
College
-Program
--Course

In order for course elements (Topics, Assignments, Objectives, etc.) to be tagged by program-specific vocabulary terms, the Program level is where the og_vocab are stored, and every course element must be created as group content of the program as well. Without inheritance, this might eventually create problems sorting through all of the courses and programs when creating an assignment, for the OG vocab to be available to the content.

When content inheritance is added, my hope/assumption is that when a user creates a topic or assignment as group content for a course, it will inherit the program group_audience from the course as well and the vocabulary from the program will be available to the content.

Our other options are to make each of our 100+ programs a sub-site so that it has unique vocabularies and use OG for courses alone.

Hope to test it soon. Thanks.

kenianbei’s picture

@Brian: Yes, this is how it works, and how I originally had set up my LMS project. I hope to get this done this week... but I just moved so I can't make any promises :) ... also hoping to add a better install/config readme, these options can be confusing for new users of OG.

Brian MacKinney’s picture

I can help with the documentation.

kenianbei’s picture

FYI, I finally had time to work on this, I'm hoping to get something testable out sometime during DrupalCon Portland. I ended up using a propagation field that can be configured with default values. When an entity with the field is attached to a group, it will use the field settings to decide to propagate up or down the group.

I just need to figure out a more graceful way to avoid recursion when both up and down is selected.

Yuri’s picture

@kanianbei That sounds very exciting, I look forward to your solution. I tried to use Rules to propagate group content, in such a way that it adds values of the parent group's group audience field.

kenianbei’s picture

Hello all,

Sorry for the long delay in getting to this, I was moved to another project for several months. I'm now back on this project so I plan to keep active with this for the next few months.

I've been working with the code on our dev site and have found that the inheritance field code caused a huge performance issue with loading all subgroups on each call to hook_node_grants(). The more I thought about this solution, the more I've realized it's not a good way to propagate permissions, for several reasons:

  1. OG already has it's own permission system, which means adding our own on top of OG means we have to keep both in sync with og_user_access_alter(). Better to just let OG handle group permissions.
  2. Because we bypass OG's permission handling, we would also need to create a work-around for entity selection as per this issue: https://drupal.org/node/1995018.
  3. The current code causes many instances of long load times and infinite recursion as per this issue: https://drupal.org/node/2017955.
  4. The current code is quite complex and duplicates code that already exists in OG.

I've rewritten the current code to address these issues in the following ways:

  • Removed the OG inheritance field and all supporting code.
  • Added an OG propagation field that can be added to group content types. This will propagate a group content (all content types) entity's membership up and/or down the group tree based on field settings.
  • Simplified code for better maintainability.

Please review the patch, particularly testing infinite recursion issues.

If anyone feels strongly about not removing the inheritance field, please speak now, otherwise I'm planning on dropping the field. Sorry for making such a drastic change, but I've found on large group structures load times become unmanageable. Please comment in this issue.

Yuri’s picture

Thanks kenianbei for what I think will be a great improvement on the group content propagation.

After applying the patch and adding the propagation field to group types, the new propagation field appears:

Up - This entity will propagate it's membership to all parent groups.
Down - This entity will propagate it's membership to all child groups.
Note: Propagation will only follow multiple paths in one direction. In order to propagate to up and down multiple paths, select the 'All' option.

First comment here is that there is no 'All' option, but I guess checking both checkboxes would do that, didn't test that yet.

Now for testing: for a Group B which is member of group A, I selected 'Down', saved, and created a subgroup C. If I understood well, the group C should be automatically member of C also, right? However, the group membership does not show in the group audience field.
Maybe I'm missing how this should work, but as far as I understand propagation, current patch is not working in my site.

kenianbei’s picture

Thanks Yuri, I originally had three options but realized I just needed 2.

I've already found recursion issues with the patch I just uploaded, I'll be working on this today and tomorrow.

Make sure you attach the field to a 'group content' type. It won't work properly if attached to a 'group' type. We'll need to figure a way to restrict this field to group content types only (I don't think og has this feature).

Let me get another patch out before testing, will update soon.

hefox’s picture

Status: Active » Needs work

diff --git a/LICENSE.txt b/LICENSE.txt
index 2c095c8..0000000

index 2c095c8..0000000
--- a/LICENSE.txt

Removal of LICENSE.txt should likely be another issue

+++ b/og_subgroups.module
@@ -1,107 +1,295 @@
+  $allowed_values = array(
+    OG_PROPAGATION_UP => 'Up - This entity will propagate it\'s membership to all parent groups.',
+    OG_PROPAGATION_DOWN => 'Down - This entity will propagate it\'s membership to all child groups.',

t()?

+++ b/og_subgroups.module
@@ -1,107 +1,295 @@
+    'description' => t('Determine if group content will propagate it\'s membership.'),

Use double quotes instead of escpaing coma, see example https://api.drupal.org/api/drupal/includes!bootstrap.inc/function/t/7

+++ b/og_subgroups.module
@@ -1,107 +1,295 @@
+      'description' => t('Note: Propagation will only follow multiple paths in one direction. In order to propagate to up and down multiple paths, select the \'All\' option.'),

Remove?

+++ b/og_subgroups.module
@@ -1,107 +1,295 @@
+          'type' => 'options_onoff',

Is this correct formatter?

+++ b/og_subgroups.module
@@ -1,107 +1,295 @@
+  $cache = &drupal_static(__FUNCTION__, array());
+  if (in_array($og_membership->gid, $cache)) {
+    return;

What happens if two different memberships are done in the same page load to the same group?

+++ b/og_subgroups.module
@@ -1,107 +1,295 @@
+ */
+function og_subgroups_og_membership_create_propagate_down($og_membership) {
+  $groups = og_subgroups_get_associated_entities($og_membership->group_type, $og_membership->gid);
+  if (!empty($groups)) {

This never checks that these are groups returned, as far as I can tell.

=====

Isn't this the same basically the same way 6.x worked? Never used it, so guessing.

edit: I'm not a fan of this.

Consider the scenario that a user joins subgroup X, and then parent group Y, then leaves parent group Y.. do they loose membership in subgroup X?

hefox’s picture

My current thoughts are to reopen #1995012: Allow altering of membership/dynamic membership? but turn it into an issue to allow dynamic/altering membership information (and adding better caching, cache_set version). Altering membership at the specific points should allow getting rid of the access hooks in og_subgroups along with fixing the entity controller.

Should be able to cache a good amount of this information. For example, a user membership changes only 1) group is added (4 group owner) 2) group inheritance changed (for anyone part of the group hierarchy) 3) user membership changes (insert, update).

kenianbei’s picture

Removal of LICENSE.txt should likely be another issue

Agreed, though it should be removed since drupal packages the license now.

+++ b/og_subgroups.module
@@ -1,107 +1,295 @@
+ $allowed_values = array(
+ OG_PROPAGATION_UP => 'Up - This entity will propagate it\'s membership to all parent groups.',
+ OG_PROPAGATION_DOWN => 'Down - This entity will propagate it\'s membership to all child groups.',

t()?

Added.

+++ b/og_subgroups.module
@@ -1,107 +1,295 @@
+ 'description' => t('Determine if group content will propagate it\'s membership.'),

Use double quotes instead of escpaing coma, see example https://api.drupal.org/api/drupal/includes!bootstrap.inc/function/t/7

Reworded to be clearer, and incidentally removed apostrophes.

+++ b/og_subgroups.module
@@ -1,107 +1,295 @@
+ 'description' => t('Note: Propagation will only follow multiple paths in one direction. In order to propagate to up and down multiple paths, select the \'All\' option.'),

Remove?

Reworded to fit actual options and functionality.

+++ b/og_subgroups.module
@@ -1,107 +1,295 @@
+ 'type' => 'options_onoff',

Is this correct formatter?

Nope, fixed.

+++ b/og_subgroups.module
@@ -1,107 +1,295 @@
+ $cache = &drupal_static(__FUNCTION__, array());
+ if (in_array($og_membership->gid, $cache)) {
+ return;

What happens if two different memberships are done in the same page load to the same group?

Not sure... will need to test this. Is there a better way to check for infinite recursion between hook_insert/update calls?

+++ b/og_subgroups.module
@@ -1,107 +1,295 @@
+ */
+function og_subgroups_og_membership_create_propagate_down($og_membership) {
+ $groups = og_subgroups_get_associated_entities($og_membership->group_type, $og_membership->gid);
+ if (!empty($groups)) {

This never checks that these are groups returned, as far as I can tell.

I think this should work, since both og_subgroups_get_associated_entities() and og_get_entity_groups() return an empty array if no results.

Isn't this the same basically the same way 6.x worked? Never used it, so guessing.

I'm not sure either, but I think 6.x also relied on it's own hook_node_grants and access testing. I'd prefer to just let OG handle this.

edit: I'm not a fan of this.

I'm not sold either way... Unfortunately I am using this on a semi-live site and the current node_grant() code is extremely slow. I need something faster at least in the short-term.

Another possibility is to leave in the user inheritance field code and add the membership propagation field code on top of this. If you are interested in improving the performance issues with that code I'm willing to support it. However as it stands it isn't usable on large scale sites (500+ group/group content in a tree).

To reiterate about the new code:

  • Simpler code base.
  • More functionality (Includes content inheritance).
  • No need for extra access grants/alters.
  • No need for extra entity reference selection class.
  • Can propagate both up and down (current code only propagates down).

New patch attached. I also included fixes for recursion and erroneous grouping with non-group entities.

kenianbei’s picture

@hefox:

My current thoughts are to reopen #1995012: Allow altering of membership/dynamic membership? but turn it into an issue to allow dynamic/altering membership information (and adding better caching, cache_set version).

This issue is actually in the OG issue queue, so it shouldn't be reopened. We would need to write our own subgroups ER selection handler.

Also I'm not sure what you mean by "allow dynamic/altering membership information". Can you clarify? Do you mean showing of subgroups that are tagged using OG_USER_INHERITANCE_FIELD for a nonmember?

Altering membership at the specific points should allow getting rid of the access hooks in og_subgroups along with fixing the entity controller.

Yes, to a point. But we would still need to handle access for when nonmembers use CRUD ops on these groups/group content. The old code used OG_USER_INHERITANCE_FIELD to tag the entity and then check if they are a member of a parent group. If they were a member, it would use hook_node_grant() and og_user_access_alter() to give access.

I don't see anyway around this without using the new patch... can you?

hefox’s picture

I think you misunderstand my idea.

Find the right place to add an alter hook and add the inherited groups so og in general things the user is part of the group, w/o actually adding the user to the groups, which #1995012: Allow altering of membership/dynamic membership? is starting to do (though the issue was started about the selection handler, but could be reworked to be about that). Cache it well so minimal calculation is needed.

> + $groups = og_subgroups_get_associated_entities($og_membership->group_type, $og_membership->gid);
This gets content in groups as well as groups, if I recall correctly. But in current also.

The issue w/ adding user membership is
1) setting changes -- if the group membership setting changes, what happens?
2) origin of membership not retained -- what happens when a user joins a child group, then joins a parent group, then leaves the parent group?

kenianbei’s picture

I see what you mean now... Sorry I didn't look closely at your patch.

For now I will resubmit the last patch as just a feature addition instead of a complete rewrite. Then we can see about reworking the current user inheritance code so that it actually works.

I'd really like to get more people commenting on this who know OG well... I'm not sure which approach is really best. Maybe Amitaibu could offer some suggestions?

kenianbei’s picture

Status: Needs work » Needs review
FileSize
9.82 KB

This patch only adds content inheritance without removing/refactoring any existing functionality.

bblake’s picture

In my instance, we don't always want to propagate deletes up, only if a user doesn't have a special role in the parent group. Tweaked the patch in #18 a bit for that in case anyone has a similar circumstance.

kenianbei’s picture

This looks more like something that should live in a custom module. We can't make assumptions about how many og roles a user has and base deletion on that.

Yuri’s picture

The patch at #18 does not work any more with the latest dev.. output is:

Patching did not go smoothly.
This command was issued: /usr/bin/patch -p1 --verbose -d '/home/healthy/public_html/sites/all/modules/og_subgroups' -i '/home/healthy/public_html/sites/default/files/patches/og_subgroups-7.x-2.x-content_inheritance-1956426-18.patch'
This was the output from patch:
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/og_subgroups.module b/og_subgroups.module
|index 4d57056..39f32f6 100644
|--- a/og_subgroups.module
|+++ b/og_subgroups.module
--------------------------
Patching file og_subgroups.module using Plan A...
Hunk #1 succeeded at 5.
Hunk #2 succeeded at 17 with fuzz 1 (offset 4 lines).
Hunk #3 succeeded at 98 (offset 35 lines).
Hunk #4 FAILED at 149.
1 out of 4 hunks FAILED -- saving rejects to file og_subgroups.module.rej
done
BrightBold’s picture

Issue summary: View changes

Apologies for the cross-post, but if anyone who's following this issue is interested in working on some of these inheritance issues including access inheritance (e.g., a public subgroup of a private group should be public only to members of its private parent group, not public to the whole world), an organization I work with may have some funding to sponsor work on that issue. See [#8633899-30] for more details and contact me.

Edit: Not sure why issue link didn't work, but it's here: https://drupal.org/comment/8633899#comment-8633899

brad.bulger’s picture

I'm a bit confused, the initial description here implies that this is about the propagation of content, but about halfway through it turns into the propagation of memberships. Did it prove to be not feasible to have content propagate? Or am I misunderstanding what that phrase means? I was looking for something like, if I create node X as content in childgroup C, the node is also automatically content in parentgroup P.

sk2013’s picture

Hi,

I applied the patch #19 and got the following options in the node edit.
Option 1: "Up - This entity will propagate it's membership to all parent groups. "
Option 2: "Down - This entity will propagate it's membership to all child groups. "

I'm not sure on how to proceed further. Any pointer or reference help is highly appreciated.

I'm thankful for your time.

sk2013’s picture

Any Update?

BrightBold’s picture

@sk2013 — there's some work going on over in #824016: Subgroup visibility and access inheritance regarding the access inheritance issues I mentioned in #22. @liberatr wrote what right now is a sandbox module, but hopefully will get merged into Subgroups once we've worked out the kinks. You can check that out and see if it addresses your needs. If you test it, please let us know how it works for you and if it seems like it's doing the logical things you expect Subgroups to do.

rooby’s picture

Issue summary: View changes