| Project: | Organic groups |
| Version: | 7.x-2.x-dev |
| Component: | og.module |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
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!
Comments
#1
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.
#2
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'
#3
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.
#4
The last submitted patch, 1902086-og-ref-respect-og-user-access-3.patch, failed testing.
#5
@ezra-g,
Thanks. Apart of the failing tests, can you please explain how this approach solves your problem?
#6
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 :).
#7
+++ 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?
#8
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.
#9
@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');
}
}
#10
Closing in favor of #1865944: Allow implementing modules to change the My/Other groups selection. Please re-open if think differently.
#11
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?
#12
What about #7?
#13
Was this not addressed in #8?
#14
Sorry, missed #8. I'll need to re-read the patch. Tests should pass before, though.
#15
I will post an in-depth response when I get a moment.
#16
+++ 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);#17
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).
#18
#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.
#19
> 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...
#20
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
#21
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.
#22
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:
<?phpforeach ($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
<?phpif (!empty($ids)) {
$entities = entity_load($entity_type, array_keys($results[$entity_type]));
...
?>
we should write
<?phpif (!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?
#23
No need to apply the patch , feature is already in the latest release. See the readme
#24
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?
#25
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.)
#26
#27
@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
#28
Perfect. Thank you! Nicked =) I'll roll that into a standalone helper module for folks who desire this functionality without Commons.
#29
Please, Renee S, mention it here, when you do
#30
@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
#31
Automatically closed -- issue fixed for 2 weeks with no activity.