This is a Drupal 7 module that allows rules to work with mail edit. This is still not addressed in 7, http://drupal.org/node/1238416. Rules implements drupal_mail(). Mail edit is designed to manage email template for any emails sent with drupal_mail(). But rules has a special case for tokens. This module creates new mail actions for rules then, implements the mail_edit hooks for those rules.

http://drupal.org/sandbox/_KASH_/1635740

http://git.drupal.org/sandbox/_KASH_/1635740.git

Comments

cbeier’s picture

cbeier commented

Hi and welcome,

There are some coding style issues. You can use this online tool to detect the issues automatically: http://ventral.org/pareview/httpgitdrupalorgsandboxkash1635740git-7x-1x

Please make your contribution compatible with the Drupal coding standards. These can be viewed here: http://drupal.org/coding-standards

rules_mail_edit.rules.inc, line 113: The $rids value should be given as an attribute to the query.

db_query('SELECT DISTINCT u.mail, u.language FROM {users} u INNER JOIN {users_roles} r ON u.uid = r.uid WHERE r.rid IN (:rids)', array(':rids' => $rids);

The same applies for line 107.

Regards, Christian

_kash_’s picture

_kash_ commented

Thanks for the feedback :)

lotyrin’s picture

lotyrin commented

Updated Review of the 7.x-1.x branch:

  • ./rules_mail_edit.module: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
    function rules_mailkeys() {

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

Source: http://ventral.org/pareview - PAReview.sh online service

It's clean with Codesniffer/Coder now but there's a few more issues:

There are many places where strings are using double quotes when they don't need to.

I'm not sure if the define()s are really needed (string <-> string mapping), and when it's possible to break things by changing them and that necessitates a warning comment, I'd lean against them.

_kash_’s picture

_kash_ commented

Thanks lotyrin. The rules functions are implemented on behalf of the rules module as required by mail edit.

The defined strings could safely be altered before adding any rules templates. After adding templates, changing the strings will cause the existing templates to be lost. I'm open to alternative solutions to keeping string consistent without the defined constants.

I'll look at updating the quotes.

Thank you for the help and feed back :)

mitchell’s picture

mitchell commented

Thanks for contributing, _KASH_. I'll be commenting over at #1238416: Rules support for Mail Editor.

_kash_’s picture

_kash_ commented
Status: Needs work » Needs review
dSero’s picture

dSero commented
Status: Needs review » Needs work

Hi,

It looks like a very nice project that probably will help many drupal users.

Please notice that there are minor issues in your automatic report: http://ventral.org/pareview/httpgitdrupalorgsandboxkash1635740git

Please notice several issues that you may consider to take care of:
1. You may want to expand your README file to include all recommended sections
2. rules_mail_edit_tokens: from security point of view it's better to perform if () return rather than canonical if

Consider doing http://drupal.org/node/1410826 to accelerate your acceptance process

Best Regards,
Moshe

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs work » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

If you reopen this please keep in mind that we are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

ahmad abbad’s picture

ahmad abbad commented
Issue summary: View changes

?