Problem/Motivation

The current 7.x branch of this module is completely unusable. It just plain doesn't work. I need this module for a project, and have decided to fix it up. Note that I haven't cleaned up the existing code, and some of my new work could probably be re-factored - suggestions are welcome.

I make no garantees, but I shall attempt to fix any bugs others encounter as well.

User interface changes

In addition to the D6 functionality, I've added an additional permission: "Administer User Permissions". When a user has this, they are given access to the User Permissions tab on the user page. There, they can access this module's functionality.

Users with this can only grant permissions that they themselves already have - a user with "Administer Blocks" can give this access to other users, but not grant themselves additional access.

CommentFileSizeAuthor
#3 small_fixes-1363246-3.patch3.95 KBthreewestwinds
#2 small_fixes-1363246-2.patch4.07 KBthreewestwinds
port-1.patch9.7 KBthreewestwinds

Comments

preventingchaos’s picture

preventingchaos commented

Thank you for posting this patch. I finally updated the 7.x version of the User Permissions module. I looked at your patch to help me catch up on some of the changes in Drupal 7 since I last touched this module, so part of my update is based on your patch. In the future I may add the additional functionality you put in your patch as well. Thanks again!

threewestwinds’s picture

threewestwinds commented
StatusFileSize
new small_fixes-1363246-2.patch4.07 KB

Ok, here's a simple followup. This patch fixes a few minor things (mostly code cleanup).

1) Your commit used tabs instead of spaces in places
2) Is still missing a version number from the .info file
3) Still a reference to user_admin_perm instead of user_admin_permissions in one of the comments.

threewestwinds’s picture

threewestwinds commented
StatusFileSize
new small_fixes-1363246-3.patch3.95 KB

Woops, added one whitespace error while fixing the rest. Here's a cleaner patch.

threewestwinds’s picture

threewestwinds commented

One more thing I noticed - you do user_role_revoke_permissions($role, $old) followed by user_role_grant_permissions($role, $new).

Instead, it's better to just user user_role_change_permissions($role, $new) - it does the exact same thing as the above two lines put together. Near the comment "// Modifying existing user permissions", you can save several lines like so:

  else {
    // Modifying existing user permissions
    user_role_change_permissions($rid, $perms);
    if(empty($perms)) {
      // If $perms is empty, this deletes all permission/role information
      // related to this role to reduce database clutter
      user_role_delete($rid);
    }
  }
preventingchaos’s picture

preventingchaos commented

Good catches! I think I have emacs setup to handle the whitespace mostly correctly now (I used this: http://drupal.org/node/59868), so tabs and extra trailing spaces shouldn't be an issue in the future.

I'll start making commits, probably targeting one thing at a time, starting with the whitespace.

Also, the 'version' number in the .info file is (supposed to be) automatically added when drupal.org creates the tar.gz/zip files, and setting it manually is actually discouraged according to this page: http://drupal.org/node/542202

threewestwinds’s picture

threewestwinds commented

Oh, so it is. I was pulling directly off git, so it wasn't being added. Thanks for the pointer.

preventingchaos’s picture

preventingchaos commented

I fixed the whitespace (I hope) and fixed the comment.

However, after looking at the data in $form_state['input'][$rid], I found it was only giving data for the boxes that were checked in the User Permissions form. This meant that all of the array_filter() calls I was making weren't doing anything, so I removed them. It also means that there are no "false" values for user_role_change_permissions() to use for revoking permissions that were unchecked. At the moment it appears to me that the simplest way to update the permissions is to continue calling user_role_revoke_permissions() followed by user_role_grant_permissions(). But I did clean up and reorganize that bit of code in the user_permissions_profile_permissions_form_submit() function.

I pushed the commits for these changes so far.

threewestwinds’s picture

threewestwinds commented
Status: Needs review » Closed (fixed)