https://drupal.org/sandbox/VincenzoGambino/2031291

This module provides a list of email domains that could potentially be Accepted or not during registration or on user profile update.

Further features:

  • List of all the email fields and enable or disable the default resctriction funcitonality
  • Customize the restriction functionality for each email field.
  • Show or not show domain list to the user.
CommentFileSizeAuthor
Schermata 2013-06-29 a 17.03.32.png39.61 KBvincenzo gambino

Comments

jyokum’s picture

jyokum commented
Status: Needs review » Needs work

Code needs some cleanup to meet standards. http://ventral.org/pareview/httpgitdrupalorgsandboxvincenzogambino203129...

Spelling mistake in .info file "description = Allor or deny"

cravecode’s picture

cravecode commented

This module is a neat idea, congrats!

My suggestions:

  • Should change your Git repository to version specific branches i.e.: 7.x-1.x (D7 dev).
  • Try not to use HTML in your t() functions. Consider concatenating them or theme the text.
vincenzo gambino’s picture

vincenzo gambino commented

Thanks,
I'm gonna go through it very soon.

jyokum, creavecode, thanks a lot for your review.

vincenzo gambino’s picture

vincenzo gambino commented
Status: Needs work » Needs review

Code cleaned up http://ventral.org/pareview/httpgitdrupalorgsandboxvincenzogambino203129...

Created 7.x-1.x branch

Deleted master branch

Fixed spelling mistake

Removed t() functions

Created README.txt

PA robot’s picture

PA robot commented

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

pere orga’s picture

pere orga
Primary language Catalan
commented

Hi!

Looks good.

IMHO you should use t() inside descriptions and wherever you can, but without including html markup. For example:
'#description' => t('Enter the domains...') . "<br />" . t('example.com') . "<br />" . t('example2.com'),

Minor: Inside email_domain_restriction_user_register_validate() you check if $domains is empty, but you don't need to, because if it's empty it won't enter the foreach(). So as long as $domains is an array you can drop the if().

petu’s picture

petu commented
Status: Reviewed & tested by the community » Needs work

Hello Vincenzo!
My recomendations:

Put in t() wrapper:

  1. all the descriptions in function email_domain_restriction_admin_form($form, &$form_state)

I am not a native English speakers, but IMHO the correct description should be

Allows or deny email addresses of particular domains.

This string exists in both *.info and *.module files.

vincenzo gambino’s picture

vincenzo gambino commented

Hi netol, hi petu,

thank you very much for your review.

I've wrapped in a t() function all the description in the format netol suggested and removed if($domain) statement into email_domain_restriction_user_register_validate()

Changed email_domain_restriction_user_register_validate() function name to a general one email_domain_restriction_user_email_domain_validate()

I've fixed the spelling as petu suggested.

On Ventral everything it's ok.

Ready to be reviewed.

petu’s picture

petu commented
Status: Needs review » Reviewed & tested by the community
vincenzo gambino’s picture

vincenzo gambino commented
Status: Needs work » Reviewed & tested by the community

jyokum, creavecode, netol and petu,

I would like to thank you with your reviews.

Now let's wait.

Thanks a lot.

vincenzo gambino’s picture

vincenzo gambino commented

Added hook_permission, moved under admin/config/people and fixed spelling mistakes.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Assigned: Unassigned » klausi

I'll look at this now in the Project applications sprint

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Assigned: klausi » Unassigned
Status: Reviewed & tested by the community » Postponed (maintainer needs more info)

https://drupal.org/project/useremaildomain

This sounds like a feature that should live in the existing PROJECT_NAME project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the PROJECT_NAME issue queue to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process.

If that fails for whatever reason please get back to us and set this back to "needs review".

vincenzo gambino’s picture

vincenzo gambino commented

Hi klausi,

thanks for your review,

I will try to reach the maintainer of the other project to not create a "duplicate projects".

On the other hands, my module does not depends on any other projects (just the core one). This makes it lighter and easier to install.
You can allow or disallow domains on registration and on profile update. So you can give access just to example.com or not give access to example.com
It uses drupal default email checker for syntax.
It will be extended to comments email and to all the email fields (webform or other forms with the chance to enable it for single fields).
Further features are different from the other projects and drush installation will be added.

PA robot’s picture

PA robot commented
Issue summary: View changes
Status: Postponed (maintainer needs more info) » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.