Official Drupal 7 release (code cleanup provided) [#1907424]

Thank you, Tim, for a great useful module!

I'm hoping you can roll an official Drupal 7 release.

I've included a code refresh of the Drupal 7 version.

The most major change i made was to use hook_menu_alter() rather than re-defining the entire 'admin/config/people/accounts' item in hook_menu().

Other than that the attached patch:

* uses lower-case permission names (and sentence-case titles) as is core's convention
* avoids passing a variable into the t() function (which would make it harder for translaters to find the right text)
* clarifies that help text a little
* uses the D7 standard "Implements hook..." consistently rather than the D6 "Implementation of..."

Comment	File	Size	Author
#10	1907424.patch	4.86 KB	tchurch
#10
#10	user_settings_access.install.txt	825 bytes	tchurch
#5	1907424.patch	4.86 KB	tchurch
#5
#5	user_settings_access.install.txt	670 bytes	tchurch
#1	usa-drupal-7-refresh.patch	4.29 KB	mlncn
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

mlncn CreditAttribution: mlncn commented 4 February 2013 at 15:11

File	Size
usa-drupal-7-refresh.patch	4.29 KB

(now actually attached)

Comment #2

mlncn CreditAttribution: mlncn commented 11 February 2013 at 18:04

Status:

Active

» Needs review

Comment #3

tchurch CreditAttribution: tchurch commented 12 February 2013 at 11:27

Assigned:

Unassigned

» tchurch

Thanks.
I will review and test.

Comment #4

tchurch CreditAttribution: tchurch commented 12 February 2013 at 12:52

Status:

Needs review

» Needs work

OK. First look. It doesn't work.

Because of the change in the permission names, an update is needed to the database for roles who already have the permission.

I also want to look at the uninstall too (to remove all permissions from roles).

I will look further at it.

Comment #5

tchurch CreditAttribution: tchurch commented 25 February 2013 at 11:05

Status:

Needs work

» Needs review

File	Size
user_settings_access.install.txt	670 bytes
1907424.patch	4.86 KB

OK. I've uploaded a patch (I think it might be the same as the previous one) but also an install file with an update for the permissions.
Can someone confirm it's OK.

Comment #6

maxilein CreditAttribution: maxilein commented 2 March 2013 at 14:51

I haven't tested your recent patch.

I would just like to mention that I think it is good practice to show the module name in the modules listing.
The module is named "Control Access to User Settings" while in the listing it shows "User Settings Access".
With all the many modules around ...
Thanks + BR.

Adding: I completely misunderstood the meaning of "user settings":

This module separates management of general account settings (/admin/config/people/accounts) from user management.

It would be great if there was a permission to manage user profile data only.

Comment #7

tchurch CreditAttribution: tchurch commented 3 March 2013 at 07:46

I could change the module name or the project name in Drupal.
It might be better to change the project name to "User Settings Access" so it matches the names of the files too.

What do you think?