Description: Improves Drupal core basic behavior by notifying all users that have the administer users permission when a new user registers with pending approval.
Users get a setting to disable these notifications in their user profile form.
See _user_mail_notify()

Version: Drupal 7
Sandbox: http://drupal.org/sandbox/sebcorbin/1574742
Repository: git clone --recursive --branch master http://git.drupal.org/sandbox/sebcorbin/1574742.git

Review of other projects

Comments

SebCorbin’s picture

SebCorbin commented
Issue summary: View changes

Updated issue summary.

patrickd’s picture

patrickd commented
Status: Active » Needs review

welcome,

Nice to see you've already fixed all coding standard issues.

Please have a look at the project application workflow.

Actually this project is too short to approve you as git vetted user. We are currently discussing how much code we need, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed (regarding security and correct api usage). However, it seems to me that you're already a quite active maintainer of other modules, therefore I'd look over this fact.

As installation and usage instructions are quite important for us to review, please take a moment to make your project page follow the tips for a great project page.

We do really need more hands in the application queue and highly recommend to get a review bonus so we can come back to your application sooner.

regards

SebCorbin’s picture

SebCorbin commented

I see your point, but this is a very basic and plug-and-play module that just extends a "too simple" behavior in Drupal core.
I intended to keep this module's code very simple using all APIs/hooks that Drupal offers but I can understand if that is not enough.

I updated the project description in the sandbox as you said, even though the goal of this module is pretty straightforward :)

I will also make a D6 backport, if that can help on deciding the issue of this application.
I'll also look into some other project applications because you guys do a great job maintaining this constant queue.

SebCorbin’s picture

SebCorbin commented
Issue summary: View changes

Updated issue summary.

SebCorbin’s picture

SebCorbin commented
Issue tags: +PAreview: review bonus
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -PAreview: review bonus

manual review:

  1. mapan_form_user_profile_form_alter(): I think you should use user_access('administer users', $account); which is way more performant.
  2. "$edit['data']['notify_pending_approval'] = isset($edit['disable_notify_pending_approval']) ? !$edit['disable_notify_pending_approval'] : TRUE;": I think you could shorten that line to "$edit['data']['notify_pending_approval'] = !empty($edit['disable_notify_pending_approval']);" no?

Although this project on its own is too short to approve you as git vetted user, there are several other projects that you are involved in and that show your skills. So the git vetted user role should be fine.

Otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

SebCorbin’s picture

SebCorbin commented

Wow I feel dumb for user_access(), thanks!
And I thought that empty() would fire a notice, I get less and less dumb :)

patrickd’s picture

patrickd commented
Status: Reviewed & tested by the community » Fixed

Thanks for your contribution and welcome to the community of project contributors on drupal.org!

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Thanks to the dedicated reviewer(s) as well.

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Reviewed some applications