I have a "Group Event" content type. When a user with "Group Event: edit any content" permissions tries to edit a Group Event node, s/he can't save the node back to the original group if they're not a member of that group. This is because the Groups Audience field only shows groups that the current user is a member of.

If I understand correctly, the change in the following patch was supposed to address this use case. But even with the latest dev version of og and entityreference, I can't see a full list of groups in the Groups Audience field, even if I'm logged in as user #1.
http://drupal.org/node/1541672

Is there some other way I'm supposed to set things up to allow certain users with higher-level roles to edit group content even when they're not members? Thanks!

Files: 
CommentFileSizeAuthor
#20 og-forceoptional-1580814-20.patch11.1 KBamitaibu
PASSED: [[SimpleTest]]: [MySQL] 929 pass(es).
[ View ]
#18 og-forceoptional-1580814-5.patch11.38 KBaasarava
PASSED: [[SimpleTest]]: [MySQL] 921 pass(es).
[ View ]
#13 og-forceoptional-1580814-4.patch11.26 KBaasarava
PASSED: [[SimpleTest]]: [MySQL] 924 pass(es).
[ View ]
#11 og-forceoptional-1580814-3.patch3.05 KBaasarava
PASSED: [[SimpleTest]]: [MySQL] 840 pass(es).
[ View ]
#9 og-forceoptional-1580814-2.patch2.97 KBaasarava
FAILED: [[SimpleTest]]: [MySQL] 840 pass(es), 0 fail(s), and 6 exception(s).
[ View ]
#5 og-forceoptional-1580814.patch2.97 KBaasarava
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch og-forceoptional-1580814.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

aasarava’s picture

I should add that I understand the "Other Groups" field is one potential solution. However, that requires the primary Groups Audience field to be optional+multi, correct? When you have a Groups Audience field that is required+single, the Other Groups field method throws an error when the value is pushed into the primary field because there is already a value in the Groups Audience field -- the admin just can't access it.

aasarava’s picture

A solution was proposed by Amitai here:
http://drupal.org/node/1541672#comment-6004086

We'll use this thread to continue the discussion.

aasarava’s picture

Amitai, I think we're going to need to do more than just make the Groups Audience field unrequired. There's also a permissions issue. You can only see the Other Groups field if you have "administer group permissions" access, right? But if the goal is allow for a site editor role, then "administer group permissions" is too much access to give that role. Do you want to refine the permissions a bit?

amitaibu’s picture

Do you want to refine the permissions a bit?

What kind of permission were you thinking? Anyway, you can start working on the code, and if needed we can easily change the permission.

aasarava’s picture

StatusFileSize
new2.97 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch og-forceoptional-1580814.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Ok, patch attached that will add a "Force Optional Primary Field" checkbox to the settings for the secondary field. When checked, this will cause the primary field to be optional (if needed) so that the Other Groups field can be used in conjuction by non-member editors.

Amitai, did I do the language handling right? Also, does my change to the og_form_secondary_field_validate() function make sense? I needed to ensure that selecting "None" in the primary field did not add to the cardinality count.

aasarava’s picture

But as mentioned in comment #3, this still requires the non-member editors to have "administer group" permissions -- i.e., in addition to being able to edit group content, they can also adjust group field settings and permissions.

Amitai, my suggestion/request is to create another permission called "administer group content". That permission should be responsible for determining whether a user can see the Other Groups field. That would really help complete this patch so you can have sites with editors/moderators who aren't superadmins.

aasarava’s picture

Status:Active» Needs review

Status:Needs review» Needs work

The last submitted patch, og-forceoptional-1580814.patch, failed testing.

aasarava’s picture

Status:Needs work» Needs review
StatusFileSize
new2.97 KB
FAILED: [[SimpleTest]]: [MySQL] 840 pass(es), 0 fail(s), and 6 exception(s).
[ View ]

Trying again with correctly rolled patch.

Status:Needs review» Needs work

The last submitted patch, og-forceoptional-1580814-2.patch, failed testing.

aasarava’s picture

Status:Needs work» Needs review
StatusFileSize
new3.05 KB
PASSED: [[SimpleTest]]: [MySQL] 840 pass(es).
[ View ]

Third time's a charm.

amitaibu’s picture

@aasarava,
Thanks. Any chance for a test to back it up?

aasarava’s picture

StatusFileSize
new11.26 KB
PASSED: [[SimpleTest]]: [MySQL] 924 pass(es).
[ View ]

@Amitaibu,
Ok, I've added a test.

I also re-rolled the patch against the latest dev. Some commits since June caused my previous patch to not work completely. The main thing was that there was an added check in og.module/og_field_attach_form() to make sure $primary_ids was not empty. However, there are cases where a site editor who is not a member of any groups could be editing a group content node.

Hopefully we're good to go with this!

amitaibu’s picture

Status:Needs review» Needs work

Thanks:

+++ b/og.testundefined
@@ -1,5 +1,27 @@
+class OGDrupalWebTestCase extends DrupalWebTestCase {

Let's remove this parent class, and move assertNoOption() and
assertOption under OgPrimarySecondaryField class.

+++ b/og.testundefined
@@ -1061,15 +1083,131 @@ class OgPrimarySecondaryField extends DrupalWebTestCase {
+   * Test primary/secondary fields on article node edit for admin

We can remove the word "article".

+++ b/og.testundefined
@@ -1061,15 +1083,131 @@ class OgPrimarySecondaryField extends DrupalWebTestCase {
+   * since the admin user is not a member of any.

of any. => of any group.

+++ b/og.testundefined
@@ -1061,15 +1083,131 @@ class OgPrimarySecondaryField extends DrupalWebTestCase {
+   *

Remove empty line.

+++ b/og.testundefined
@@ -1061,15 +1083,131 @@ class OgPrimarySecondaryField extends DrupalWebTestCase {
+    $this->pass('test');

Remove :)

+++ b/og.testundefined
@@ -1061,15 +1083,131 @@ class OgPrimarySecondaryField extends DrupalWebTestCase {
+    $field_id = drupal_html_id("edit-".OG_AUDIENCE_FIELD."-$langcode");

fix coding style to have space between " and .

+++ b/plugins/entityreference/selection/OgSelectionHandler.class.phpundefined
@@ -125,6 +126,19 @@ class OgSelectionHandler extends EntityReference_SelectionHandler_Generic {
+        '#description' => t("Force the primary field to be optional when the secondary field is shown. This can prevent problems when admins edit group content for groups they're not members of."),

Maybe instead of "prevent problems" we should explain it doesn't require you to select a field.

aasarava’s picture

Amitai, the assertOption() and assertNoOption() helpers are used in two classes, which is why I created the parent class that both subclasses could inherit. If we remove the parent class, we have to duplicate these functions in two places. You sure you want to do that? As the tests for OG grow, it seems like it'll be helpful to have one parent class to store all the helper functions you may have.

jhedstrom’s picture

Referencing this issue as a duplicate #1633116: Permission or configuration to reference groups they do not belong to, since this is the fix I was looking for.

amitaibu’s picture

which is why I created the parent class that both subclasses could inherit.

Ok, so lets keep the class, but change its name to OgAssertOptionsTestCase

aasarava’s picture

StatusFileSize
new11.38 KB
PASSED: [[SimpleTest]]: [MySQL] 921 pass(es).
[ View ]

Done! Patch with requested updates attached.

aasarava’s picture

Status:Needs work» Needs review
amitaibu’s picture

StatusFileSize
new11.1 KB
PASSED: [[SimpleTest]]: [MySQL] 929 pass(es).
[ View ]

Cleaned up a bit, but still needs little work

+    if (!is_array($new_ids)) {
+      // Entity metadta returned a single ID or no ID.
+      $new_ids = array($new_ids);
+    }

Can you explain why we add in this patch?

+ * See: http://drupal.org/node/1580814

This can be removed

aasarava’s picture

Can you explain why we add in this patch?

When the ID values are retrieved from the field, they may or may not come back as an array. If not, we need to explicitly convert to an array so that we don't get PHP errors when the foreach($new_ids as $id) statement is reached several lines later in the code.

amitaibu’s picture

Status:Needs review» Fixed

Committed, thanks!

Status:Fixed» Closed (fixed)

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

azinck’s picture

Status:Closed (fixed)» Active

I know I'm resuscitating an issue that's been somewhat resolved, but I see 2 problems that this solution doesn't fully address. If a discussion of these issues results in new actions that need to be taken then I'll take out new ones for that.

  1. By simply making Groups Audience not required you now lose the ability to enforce that an entity ALWAYS be associated with a group.
  2. I haven't dug too deeply into it, but I can't get entityreference_prepopulate to target the "other groups" field. So if I'm an editor (not a member of a group, but having permission to create content in that group) and I click a link to create content in the group, neither the Groups Audience nor the Other Groups field gets populated.

How about this for an idea?: right now the "reference" option for a Groups Audience field includes the options:
My groups
Other groups
All groups

Could we add an option "Permissible groups" or something similar which would then only list groups in which you have either the "create content" or "edit content" OG permission (according to whether or not you're on a node add or node edit form) for the content type in question? This would obviate the need for the "Other Groups" field and much of the confusion it causes at the expense of a potentially more awkward interface for users who have content permissions in many groups. It would also obviate the need to make the Groups Audience field only required on a conditional basis.

What do you think?

amitaibu’s picture

Renee S’s picture

Status:Closed (fixed)» Active

I don't think the "complex" widget addresses all of what azinck is saying, so I'd like to reopen this request for support.

There is no more "My groups" on/off checkbox on the field settings to allow all groups to be selected (off) or just the user's groups (on).

The other issue fixes it so that people with the administrate group permission can edit nodes that belong to not-their-groups, but it doesn't let others people assign content to groups who aren't administrators ("All groups"), which was the purpose of the My Groups checkbox. That functionality has disappeared!

Is there some solution I'm missing that allows users to assign content to other groups?

amitaibu’s picture

Status:Active» Closed (fixed)

@Renee S, you are probably talking about #1796914: Allow non members to create content in group

Renee S’s picture

I am! Thanks!

Renee S’s picture

Issue summary:View changes

add url to other issue