Allow site admins to grant any role to group context

What's the "restricted roles" functionality?

Not sure I fully understand. Like,

Currently, additionally roles within a group context are granted by group admins individually for each group member. The site admin globally configures, which roles can be assigned by group admins.

You want:

Additionally, site admins should be able to do the same as group admins (already the case?). But a site admin is able to grant any role, not only those that can be granted by group admins.

Visually just meaning that everything stays the same for group admins. Site admins, unlike group admins, will see all available (or a globally configurable list of) user roles on og/users/%/roles.

Technically, requires to

- make group user role assignments relative instead of absolute. When group admins are changing a user's roles, there are not checkboxes for all possible roles in the form. Before revoking a role, form handling needs to check whether the role could be selected in the form in the first place.

- either add a globally configured list of roles available to grant for site admins, or alternatively and perhaps cleaner, add new permissions in the kind of "grant %name role". Perhaps the permission idea should be a separate issue, but I find it interesting, because it might be able to replace the current global selection of available roles per group content type.

Why a separate tab?

Do we need to display other roles for group admins? I don't think so. What I effectively have in mind is pretty simple:

1) You are group admin. On og/users/%/roles, you see the user list and can grant group-specific roles to individual users. You can grant the available/shown roles only.

2) You are site admin. On og/users/%/roles, you see the user list and can grant group-specific roles to individual users. You see all roles that exist and can grant any roles.

Though really not sure about the "all roles". We'd have to limit to certain globally configured ones.

--

Now that we know what we want and can be done, I have to amend that I won't have time to work on this feature. However, happy to review and commit patches.

Cool, ok. If you're new to Drupal development, I'd highly recommend to "post early post often" patches. I.e., instead of ramping up a giant patch after days of work, post your steps here and ask/wait for feedback on whether the changes are way to go. :)

Thanks!

+++ og_user_roles-LOCAL/og_user_roles.module	2010-08-19 11:21:09.000000000 -0400
@@ -31,6 +31,7 @@ function og_user_roles_perm() {
+    'grant any role for group',

Do we need a new permission for this? Currently, the global administration options are accessible through the "administer organic groups" permission (of OG) only. I'd prefer to re-use that permission for this enhancement, unless your use-case requirements actually demand for a separate permission.

+++ og_user_roles-LOCAL/og_user_roles.pages.inc	2010-08-19 15:29:11.000000000 -0400
@@ -1,4 +1,5 @@
 <?php
+
 // $Id: og_user_roles.pages.inc,v 1.5 2009/12/06 19:26:37 sun Exp $

Please revert :)

+++ og_user_roles-LOCAL/og_user_roles.pages.inc	2010-08-19 15:29:11.000000000 -0400
@@ -76,7 +77,21 @@ function og_user_roles_admin_settings() 
+    $roles = user_roles(true);

All Booleans (TRUE/FALSE/NULL) are written all-uppercase in Drupal.

+++ og_user_roles-LOCAL/og_user_roles.pages.inc	2010-08-19 15:29:11.000000000 -0400
@@ -76,7 +77,21 @@ function og_user_roles_admin_settings() 
+    // Remove roles that do not belong in the list
+    $default_member = og_user_roles_get_group_default_role($node->nid);
+    $default_admin = null;
+    foreach ($roles as $rid => $name) {
+      if ($rid == DRUPAL_AUTHENTICATED_RID || (($rid == $default_member || $rid == $default_admin) && !user_access('override group default role'))) {

I'm not sure whether I understand the logic applied here (the comment doesn't help much either [but should]), and, it seems like $default_admin will be always be NULL?

Additionally, I think that the "override group default role" permission only applies to the default role for members, not for group admins.

Powered by Dreditor.

Thanks for working on this issue, pgillis!

I think we are pretty close already. Did you test this manually, using different users with varying group admin privileges? That said, do you know how to write simpletests? OGUR already implements some first tests in ./tests/og_user_roles.test, and normally, every new feature has to come along with corresponding tests... writing them is pretty easy. If existing test code is not sufficient, then there are plenty of docs here: http://drupal.org/simpletest

+++ og_user_roles/og_user_roles.pages.inc	2010-08-23 11:59:50.000000000 -0400
@@ -76,7 +76,20 @@ function og_user_roles_admin_settings() 
+  //If the user can administer orgranic groups, give them access to more roles
...
+    // Filter out special system roles along with the default group role, unless override is enabled

Minor stylistic issues, as mentioned on http://drupal.org/node/1354:

1) There should be a single space after //

2) Comments should not exceed 80 chars. (Except of phpDoc function summaries.)

3) All code comments should form proper sentences, i.e., end with a period (full-stop).

4) Typo in "orgranic".

Major issue:

5) Reading these comments, they merely explain what the code more or less tells me, too. Code comments should explain the non-obvious and non-trivial only:

- Why are we doing this here? (What's the goal after all?)

- Why only for users with "administer organic groups" permission?

- Why do we filter out the default group role?

- Why do we NOT filter out the default group role if override is enabled? Also, what kind of override, and where?

+++ og_user_roles/og_user_roles.pages.inc	2010-08-23 11:59:50.000000000 -0400
@@ -76,7 +76,20 @@ function og_user_roles_admin_settings() 
+      if ($rid == DRUPAL_AUTHENTICATED_RID || ($rid == $default_member && !user_access('override group default role'))) {

Hm. This feature has to presume that not all roles may be contained in the form that is submitted. In turn, we can only change roles relatively.

Here, the default member role is excluded from the roles that can be assigned. What happens if the default member role is changed? --- Just a unverified consideration, which we should double-check.

Powered by Dreditor.

Default role handling is a pretty complex topic, which is discussed over in #614024: Default roles are not applied to existing group members/admins currently.

For this patch, we just need to ensure that we don't break nothing of the current default role functionality. (which I'd have to review in detail myself first -- thus my question for ensuring our expectations via tests ;)

subscribing