allow group admins define default role for new members

stella - July 15, 2009 - 10:02
Project:OG User Roles
Version:6.x-4.x-dev
Component:Code
Category:feature request
Priority:normal
Assigned:Unassigned
Status:closed
Description

First of all thanks for the ogur module rewrite, the 4.x version looks great and has all the core functionality that I really need, well except for one thing :)

In the older versions of ogur, it was possible to allow group admins to define the default role for new members within their group. This feature was removed in the 4.x clean-up, but I believe it is still a central enough feature that should be also be included in the 4.x version.

I've attached that provides this feature. Changes include:

  • Adds a new og_users_roles_group_settings table to store the extra settings. The table name could be improved though. Currently it just stores the group id and the default role for new group members. It could be extended to add other per-group settings if needed however. I decided on a new table rather than using the variable table because when you've got thousands of groups on your site, it's just too much to store in that table and can have a negative impact on performance.
  • Adds a new option to admin/og/og_user_roles to turn on / off this feature.
  • If the feature is enabled, a default roles drop-down will appear on the node edit form for group nodes.
  • Implements hook_content_extra_fields() so the node edit form element can have its weight changed via cck.
  • When a new member joins a group and the option is enabled, the per-group default setting is used if available, otherwise it falls back to the site-wide default group setting.

Currently it does not do the following, but it may be desired in the future:

  • Allow group admins to override the 'default admin role' setting - I just couldn't think of a use case for this, but if needed a new column could just be added to the og_users_roles_group_settings table. Currently only the 'default role for new members' can be overridden by group admins.
  • At present it is only possible to turn on / off this new feature on a site-wide basis, i.e. it's not possible to configure this setting at a per group/node types level. However the existing ogur default role settings also aren't configurable at this level, so I didn't add it. It should possibly be dealt with separately if this is needed.

Cheers,
Stella

AttachmentSize
default_role_per_group.patch7.25 KB

#1

sun - July 16, 2009 - 01:09

Re-rolled this patch with some minor changes.

  $form['og_user_roles_defaults']['og_user_roles_admin_set_default_role'] = array(
    '#type' => 'checkbox',
    '#title' => t('Allow group admins set default role for new members'),
    '#description' => t('If enabled, group admins can override the default role for new group members when editing a group node.'),
    '#default_value' => variable_get('og_user_roles_admin_set_default_role', 0),
  );

I'm not sure whether this shouldn't be a access permission instead...?

If not, or rather in either case, we already have the variable 'og_user_roles_default_role', so renaming this variable to 'og_user_roles_default_role_override' or 'og_user_roles_default_role_per_node', or similar would make sense.

    case 'user update':
      $default_admin_role = variable_get('og_user_roles_default_admin_role', 0);
      $default_role = variable_get('og_user_roles_default_role', 0);
      if ($default_admin_role > 0 && $default_admin_role != $default_role) {

In og_user_roles_og(), we have some additional processing for 'user insert' now, but 'user update' still relies on the old variable only. Looking at that code, we might need a new helper function to determine the default role for a group.

        '#weight' => module_exists('content') ? content_extra_field_weight($node->type, 'og_user_roles_default_role') : 0,

I'm not sure whether this is really required. IIRC, content module automatically tries to apply a #weight to all elements/fields exposed. I could be wrong though.

Last, but probably not least, since the functionality of this module is quite critical from a security perspective, we also want to add/advance on the tests.

AttachmentSize
og_user_roles-HEAD.default.patch 7.53 KB

#2

stella - July 16, 2009 - 14:20

Patch re-rolled with the following changes:

  • Removed the setting from the admin settings page and replaced it with an "override group default role" permission.
  • Added tests for the new functionality.
  • Removed call to content_extra_field_weight(), turns out you don't need it after all :)
  • Added a new function og_user_roles_get_group_default_role() to get the default role for a particular group, which is then used on user insert and update.
AttachmentSize
519794.patch 10.8 KB

#3

sun - July 31, 2009 - 03:29

We want to keep the per-group setting optional, defaulting to the global configuration setting.

I was about to commit this, but now the tests are horribly failing and I need to get some sleep :-/

AttachmentSize
og_user_roles-HEAD.per-group-default.patch 11.05 KB

#4

stella - August 6, 2009 - 10:21

The attached patch fixes the problem with the tests.

I do get one test exception, but this happens regardless of whether of the patch is applied or not:

Table 'd6_site.simpletest535284imagecache_preset' doesn't exist query: SELECT * FROM simpletest535284imagecache_preset ORDER BY presetname
User warning
database.mysqli.inc  128
_db_query()

AttachmentSize
519794.patch 11.07 KB

#5

sun - August 14, 2009 - 12:00
Status:needs review» fixed

+++ og_user_roles.module 6 Aug 2009 10:18:55 -0000
@@ -444,3 +471,64 @@ function og_user_roles_role_delete($gid,
+function og_user_roles_get_group_roles($node_type) {
+  $roles = array();
...
+    $roles[$rid] = $user_roles[$rid];

Wow! So just declaring the array upfront made it work... I can only guess that happened, because $rid is an integer and one can only leave out the array declaration for string keys.

That, I didn't know. :-/

I'm on crack. Are you, too?

Thanks for reporting, reviewing, and testing! Committed to 4.x.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

#6

stella - August 14, 2009 - 12:10

w00t! thanks!

#7

System Message - August 28, 2009 - 12:20
Status:fixed» closed

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

 
 

Drupal is a registered trademark of Dries Buytaert.