As part of the design for Drupal Commons, users are able to create nodes inside of public groups in a single reference field, regardless of whether or not the user is already a member of the group.

OG provides separate "your groups" and "other groups" fields with separate autocomplete, validation and submission handling.

Can you advise on how we could use the OG/Entityreference API to provide these as a single field?

Switching the field to a basic Entityreference field causes OG to show the error,

[field] is a group-audience but its Entity selection mode is not defined as "Organic groups" in the field settings page

Additionally, I believe that some of the default validation from the og_group_ref field is useful, so my sense is we don't want to simply suppress that message and use a default entity reference field.

Thanks!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

amitaibu’s picture

So in fact you want to use the "regular" ER field, but attach the OG widget behavior to it. I don't think it can be done right now, so I see two options:

1) Write your own ER-selection handler
2) Write a patch to OG to allow adding the behavior even when not using OG's widget.

Elin Yordanov’s picture

How about allowing the 'other groups' field widget to check ALL groups instead of other groups, and disable/unset 'your groups' field widget?

Then it would also make sense to rename the widget to 'all groups'

ezra-g’s picture

Title: Single field for your/other groups » og_group_ref field should respect og_user_access()
Category: support » bug
Status: Active » Needs review
FileSize
4.83 KB

Thanks for the input here.

After looking more closely at the situation, I'm retitling and reclassifying. Commons uses a hook_og_user_access_alter() implementation to allow users to post into public groups where the user is not already a member.

The OG Entityreference behavior only allows the user to reference groups where the user is already a member, even if og_user_access() would return TRUE for that user for create operations within that group. In other words, the field behavior doesn't honor the results from og_user_access().

This patch corrects this behavior. For standard, out-of-the-box OG implementations, there should be no functional difference from the current behavior of the field, since by default, og_user_access() will return TRUE only when the user is already a member of the groups.

The approach of the patch is to take some of the business logic from public function buildEntityFieldQuery() and move it into public function getReferencableEntities(), and no longer limit the EFQ by group NIDs where the user is a member.

Status: Needs review » Needs work

The last submitted patch, 1902086-og-ref-respect-og-user-access-3.patch, failed testing.

amitaibu’s picture

@ezra-g,
Thanks. Apart of the failing tests, can you please explain how this approach solves your problem?

ezra-g’s picture

Re @Amitaibu/#5:

I tried to explain this in comment in #3 - I'll try another time :).

Currently, the og_group_ref field behavior lets users reference groups that meet these criteria

1) Get a list of groups where current user is a member
2) Limit the list to groups whose titles partially match the text entered in the field
3) Limit the list of groups to those where the user has permission to create/update the relevant node type within the group, as determined by og_user_access()

The approach solves the problem of letting non-members post into public groups by eliminating item #1, which is actually addressed by item #3. I believe criteria #1 is unnecessary.

Happy to discuss further if you'd like to discuss in IRC :).

amitaibu’s picture

+++ b/plugins/entityreference/selection/OgSelectionHandler.class.phpundefined
@@ -12,6 +12,48 @@ class OgSelectionHandler extends EntityReference_SelectionHandler_Generic {
+      foreach ($results[$entity_type] as $gid => $values) {

My problem with the solution is that this result set might be huge, thus making this solution un-scalable. No?

ezra-g’s picture

The proposed patch should be at least as scalable as Entityreference's default behavior because it is *more restrictive* in finding the initial result set in the EFQ because it further restricts the result set to just items that are groups.

In any case, I would expect LIMIT() on the query to make the query reasonable, even with a large potential total result set.

bulldozer2003’s picture

@ezra-g
I believe I understand your use case. I have a similar use, if not nearly identical. Rather than using the other groups field, I solved the issue by using hook_entity_query_alter to populate the standard group audience field with the additional groups the user is allowed to post to. I have posted a patch in #1865944: Allow implementing modules to change the My/Other groups selection that allows OG to respect changes made to the group audience field by external modules.

If you look at the patch in comment 45 you will see a function og_test_entity_query_alter(), in that function the query that populates the group audience field is altered to display all groups of type node.

If you look at my sandbox project OG Subgroup Roles you will see a more complicated implementation of hook_entity_query_alter() which does all 3 items in your OL in comment 6.

Here is the code for the query alter in og_test:

/**
 * Implements hook_entity_query_alter().
 *
 * @see OgComplexWidgetTestCase::testAlteredAudienceField()
 */
function og_test_entity_query_alter($query) {
  $instance = !empty($query->metaData['entityreference_selection_handler']->instance) ? $query->metaData['entityreference_selection_handler']->instance : array();
  // Alter OG_AUDIENCE_FIELD . '_altered' to allow selection of all group nodes.
  if (!empty($query->metaData['field']['field_name']) && $query->metaData['field']['field_name'] == OG_AUDIENCE_FIELD . '_altered' && !empty($instance['field_mode']) && $instance['field_mode'] == 'default' && !empty($query->propertyConditions[0]['operator']) && $query->propertyConditions[0]['operator'] == 'IN') {
    // Set the property conditions to match all groups of type node.
    $query->propertyConditions[0]['value'] = og_get_all_group('node');
  }
}
amitaibu’s picture

Status: Needs work » Closed (duplicate)

Closing in favor of #1865944: Allow implementing modules to change the My/Other groups selection. Please re-open if think differently.

ezra-g’s picture

Status: Closed (duplicate) » Needs review

Thanks, guys.

Setting back to needs review because I don't believe #1865944 addresses our use case of having a single autocomplete field widget that lets users reference any group where they have access to post content.

I don't believe altering the query is a reliable approach because the proposed example query alter assumes:
A) The element of the reference query that restricts to a set of nids will be reliably keyed on query->propertyConditions[0]

The key changes to 1 when the user has entered text into an autocomplete field. I'm not aware of any way to reliably know which element in the array we want to alter, especially if other modules may be altering this query.

B) The element of the query that we want to override is an IN() query

The query can also be an equals query set to -1 when the user is not a member of any groups.

C) That we have a field that only references a single group type, since it uses og_get_all_group() which requires a type parameter.

Users may be members of groups of multiple entity types.

In my test implementation similar to what's suggested in #9, I wasn't able to alter the query sufficiently to allow non-members to reference a group within the single primary groups autocomplete widget.

In any case, it's not clear to me why we should need to do any altering in the first place, particularly at the query construction level, in order to make the OG API consistently respect og_user_access().

Are there specific objections/concerns with the patch in this issue that haven't been addressed? Anything I can clarify to help explain our motivation for the proposed change?

amitaibu’s picture

What about #7?

ezra-g’s picture

Was this not addressed in #8?

amitaibu’s picture

Sorry, missed #8. I'll need to re-read the patch. Tests should pass before, though.

bulldozer2003’s picture

I will post an in-depth response when I get a moment.

amitaibu’s picture

Status: Needs review » Needs work
+++ b/plugins/entityreference/selection/OgSelectionHandler.class.phpundefined
@@ -12,6 +12,48 @@ class OgSelectionHandler extends EntityReference_SelectionHandler_Generic {
+    $options = array();

I think you are missing the global $user and $group_type variables.

+++ b/plugins/entityreference/selection/OgSelectionHandler.class.phpundefined
@@ -12,6 +12,48 @@ class OgSelectionHandler extends EntityReference_SelectionHandler_Generic {
+    $node_type = $this->instance['bundle'];
+    $node = $this->entity;

Shouldn't be hardcoded to node.

+++ b/plugins/entityreference/selection/OgSelectionHandler.class.phpundefined
@@ -12,6 +12,48 @@ class OgSelectionHandler extends EntityReference_SelectionHandler_Generic {
+    // includes only gropu entities.

typo in group.

+++ b/plugins/entityreference/selection/OgSelectionHandler.class.phpundefined
@@ -12,6 +12,48 @@ class OgSelectionHandler extends EntityReference_SelectionHandler_Generic {
+    if ($limit > 0) {

Enough if ($limit);

amitaibu’s picture

The proposed patch should be at least as scalable as Entityreference's default behavior because it is *more restrictive* in finding the initial result set in the EFQ because it further restricts the result set to just items that are groups.

In any case, I would expect LIMIT() on the query to make the query reasonable, even with a large potential total result set.

I'm not sure about this statement. Lets say I want my users to see a select list with their groups. If I have 10K groups, OG will iterate over each one of those, which is wrong. (this is btw, the reason the "create" permission in the UI is disabled for non-members).

ezra-g’s picture

#17 is a solid point. My suggestion is that we keep the current behavior for select lists but adopt the presently proposed behavior for the autocomplete tags widget, which is better suited towards public posting into sites with tens of thousands of groups.

If you agree, I can revise the patch in that way.

amitaibu’s picture

Title: og_group_ref field should respect og_user_access() » Allow group-audience widget to allow adding new content to groups a user doesn't belong to

> but adopt the presently proposed behavior for the autocomplete tags widget,

That sounds a bit wrong, as site admins might use other widgets. We need to find a scalable way, to allow it to work on every widget type.
I had an idea that we can also use ER-prepopualte, and letting the widget add groups passed via url, that user has access to. Like this we don't need to iterate over each group. The problem is what to do with node edit...

amitaibu’s picture

Status: Needs work » Closed (duplicate)

Lets close on favor of #1865944: Allow implementing modules to change the My/Other groups selection

I'm also going to open a new issue for #19

Renee S’s picture

I agree with @ezra_g that these are different things.

I don't agree with the logic that closing off the non-member create permission because people might choose a less-scalable widget makes sense -- on sites with a zillion nodes making responsible widget choices is seen as par for the course, and the site manager is responsible for choosing something that makes sense for the amount of data. I'm not going to use "checkbox" widget if I have eight thousand nodes, either... I'll use autocomplete, and I don't think there *is* a scalable way for all widgets to handle large numbers of things, nor is that a realistic expectation.

grigorym’s picture

Status: Closed (duplicate) » Active

I cann't apply the patch of ezra-g and force it to work. It shows the full list of groups on node creation, if i'm a member of any one of them. I've got two ordinary groups, which allow members to write and disallow anyone else. Suppose I'm a member of group2 and NOT a member of group1.

It's the function getReferencableEntities($match = NULL, $match_operator = 'CONTAINS', $limit = 0) that makes me wonder. I debug this function and verify that:

og_user_access(node, 1, create post content)=FALSE
og_user_access(node, 2, create post content)=1

here:

      foreach ($results[$entity_type] as $gid => $values) {
        // Check if user has "create" permissions on those groups.
        // If the user doesn't have create permission, check if perhaps the
        // content already exists and the user has edit permission.
        if (og_user_access($entity_type, $gid, "create $node_type content")) {
          $ids[] = $gid;
        }
        elseif (!empty($node->nid){ ... }
      }
      if (!empty($ids)) {
        $entities = entity_load($entity_type, array_keys($results[$entity_type]));
        foreach ($entities as $entity_id => $entity) {
          list(,, $bundle) = entity_extract_ids($entity_type, $entity);
          $options[$bundle][$entity_id] = check_plain($this->getLabel($entity));
        }
      }

Aren't we building $ids in vain, since its local variable and we completely ignore it when building $options? Shouldn't we select only entities whos ids belong to $ids?

perhaps instead of

      if (!empty($ids)) {
        $entities = entity_load($entity_type, array_keys($results[$entity_type]));
        ...

we should write

      if (!empty($ids)) {
        $entities = entity_load($entity_type, $ids);
        ...

In my case, at least, it behaves exactly I want it: lists the groups I am a member of, unioned with the groups that allow 'create' for non-members. Those who have too many groups, may use the autocomplete widget instead of the list. One more hint - 'create' for non-members is still selectable only with entityreference_prepopulate, which is an unnecesssary link with my use case.
I suppose, such bevahior is much needed (or at least, enough needed), to have its own option in the settings?

amitaibu’s picture

Status: Active » Closed (duplicate)

No need to apply the patch , feature is already in the latest release. See the readme

grigorym’s picture

Yes, 7.x-2.2 populates the list with the group, taken from the url, is the user is allowed to post to it. Thank you for your work. However, for my case I find the solution of ezra-g more convenient: that the list is populated with all accessible groups, irrespective of path?og_group_ref=XXX. Could it, perhaps be available by config/settings in future releases?

Renee S’s picture

Status: Closed (duplicate) » Active

Seconded @grigorym's comment; prepopulate isn't a suitable solution for many use-cases, like for example when users may or may not decide to post into several groups, which isn't known when they click whatever link they're using to create content (which again isn't always going to be from the same place.)

Renee S’s picture

Category: bug » feature
amitaibu’s picture

Status: Active » Fixed

@Renee S ,

This can be done in contrib. For example see the work we are doing in commons which already uses OG as-is, but implements own Selection handler.
#1961296: Refactor og_group_ref field per OG API improvements

Renee S’s picture

Perfect. Thank you! Nicked =) I'll roll that into a standalone helper module for folks who desire this functionality without Commons.

grigorym’s picture

Please, Renee S, mention it here, when you do

Renee S’s picture

@grigorym, here it is, very rough. Extremely very rough. A direct copy of @Amitaibu and @ezra-g's stuff. It works (as in, enable it and set the Entity Selection -> Mode to the "OG non-member add" handler and non-members can post into the groups that field controls), but needs work, and settings. At the moment it doesn't do any checking for public groups vs. private; I want to leverage the "non-members can create" permission for that.

http://drupal.org/sandbox/Reinette/1985380

Status: Fixed » Closed (fixed)

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

Jon Pugh’s picture

Status: Closed (fixed) » Active

I'm still not entirely convinced this should't be possible in OG core...

I have an OG Entity Reference field for a node type called "Department", with a node type called "Invoice" that is group content.

On node/add/invoice, the Department OG reference field is empty, unless I check Create Invoice content: Member AND I am a member of at least one Department (only departments I am a member of show up in the list).

Based on the OG Node field permissions grid, I should be able to check Create Invoice content: Non-member to allow me to post an invoice "into" a department group i am not a member of, but it doesn't work that way.

Why is the "create xyz content" permission for "non-members" a possibility in the UI, but it doesn't seem to actually grant me that permission (as a non-member)?

Renee S’s picture

The permission is in the UI for things like Entityreference Prepopulate, which uses it to allow people with the right link to post into a group even if they're not a member.

@Amitaibu said in another thread that he didn't want to make the non-member-create behaviour possible because widgets might not allow it to scale (ie, somebody using a checkbox widget with a thousand groups). This is a philosophical disagreement; IMO there exist scalable widgets (eg, autocomplete) and it should be up to the site builder to choose the right one for their circumstances. But anyway ;)

Jon Pugh’s picture

The permission is in the UI for things like Entityreference Prepopulate

Are you talking about the permission for Entityreference Prepopulate? Can you point me at it? Can't seem to find anything but "Skip access permission" (for ER Prepopulate)

...because widgets might not allow it to scale (ie, somebody using a checkbox widget with a thousand groups).

Who is to say I am not a member of a thousand groups?

You are correct, the widget you choose is a totally use-case-specific decision. I agree that "Might not be scalable" is not a valid argument here. There are many factors that affect scalability (both technical and usability-wise), it's up to the site builder to mitigate them all.

amitaibu’s picture

Status: Active » Closed (fixed)

Please open a new issue

kclarkson’s picture

I have read a bunch of threads related to this and this one seems to go the longest and makes the most sense.

I also agree with @Renee S, @ezra, @jon pugh this feature is HUGE and IMHO should be build into OG. I just spent hours looking at permissions and other areas of my site trying to figure out: why can I not check the box for create content for non-member ? Why are my prepopulate links not adding the content to a group ?

As a site builder who is using OG for a University Site, and someone who is trying to use COD for multiple conferences, this feature seems like a no brainer especially when the UI tricks you to believe that it should be possible.

I don't understand the "scalable" thing, when you can set it on the default settings of the OG Group content type. How many sites have hundreds of different content types that are the groups. Depending on your needs you should be able to decide the default settings.

EDIT: As Renee stated below, there are other modules such as OG Context that require the OG widget to be used. So therefore the custom widget does not work. So I am using organic groups in combination with COD (Conference Organization Distribution) and I need people to be able to submit conference session proposals without being a part of the group.

PLEASE RECONSIDER !!!!

Renee S’s picture

Status: Closed (fixed) » Active

The concern that @kclarkson has raised is that often modules rely on the "OG Group" selector widget when making decisions. The work-around, which is to create a new widget, breaks a lot of functionality in those things that assume anything OG-related uses the OG widget.

We definitely need to have a way to let non-members select groups in widgets, and this is even being built into Commons, so it's definitely not an usual use case. I would love it to be reconsidered too, but in the meantime creating a separate widget is the only way to accomplish this.

rv0’s picture

@kclarkson
iirc custom widgets work just fine with the latest OG Menu.

kclarkson’s picture

@rv0

You are correct sir but OG Context does not. And I am using OG context to place the OG menu.

Tresler’s picture

I have another, simpler, use case that I'll throw on the pile here that is in direct response to the original issue.

I have a site with 10 groups. Everyone can post to all of those groups. I have a form where I want 6 checkboxes for the 6 main groups that I want to display on forms (the other 4 are only used in certain other forms).

OG currently makes it impossible to select group audience based on Entity Reference View without throwing an error every time the form is constructued.

First, if we want to limit the widgets that a form can use for a field, we should do that in the validation of the field settings, not every time the form is constructed.

Second, why should a widget *type* ever be tied directly to access control? Field *options* should be, but never type. Why does OG care if I want to use radio buttons, autocomplete, or a select list?

izmeez’s picture

Issue summary: View changes

This is a long issue but it seems the closest to what I would like to do #2344393: Changing audience og widget type..

As I read this thread it seems the biggest drawback to an audience selection widget that includes all "My groups" and "Other groups" is scalability.

However, for sites that do not have huge numbers of groups such a widget might be more convenient and easy to use.

Would it be possible to have two widgets where one is used up to a defined number of groups and fall back to auto complete approach when there are large numbers of groups?

jdleonard’s picture

I came across this today trying to figure out how to let a non-member add group content when the OG Field Permissions module is enabled (everything worked as expected with that module disabled). Changing the Group Audience widget appears to be the solution, but it seems to be unintuitive and undocumented. I'll echo #40 in asking why the type of widget affects access control (in my case, node creation) and also how could the OG Field Permissions module affect node creation access control?