Integrates the PET (Previewable E-mail Templates) module with Rules. Provides a single Rules condition to specify a PET name, actions can then be assigned to trigger when an e-mail is sent using that template.

This module is intended to be used with the PET, Rules and Token modules. If you pass a Node ID (nid) to a PET, all of that node's fields (including CCK fields) will be available as tokens to Rules actions. User object tokens are also available.

Sandbox Project Page: http://drupal.org/sandbox/wuh/1841460
GIT Location: git clone http://git.drupal.org:sandbox/wuh/1841460.git pet_rules
Drupal Version: 6.x

Reviews of other projects

http://drupal.org/node/1838880#comment-6735124

Comments

wuh’s picture

Issue summary: View changes

Including full git clone command

parwan005’s picture

Hi,

here are the details after manual review of your code: -

1) @file comment is missing from your .module file.
2) Doxygen commenting change : "Implementation of hook_requiredhook()" has been replaced with "Implements hook_requiredhook()"
3) Your .info file doesnt end with a new line.

Also make sure you finish your ventral issues here : http://ventral.org/pareview/httpgitdrupalorgsandboxwuh1841460git

Thanks
Parwan

indydas’s picture

Hi wuh,

Nice little module integration with PET and Rules here.

After checking your code out I did have a quick scan of your code. With the obvious missing @file comment as mentioned above the only thing I did notice is the return; statements. Possibly these could be changed to indicate it's returning something or nothing?

Is this module being extended out for multiple roles too? If so, it could be worth adding a hook_perm in the module file.

wuh’s picture

Thanks for the comments. Unfortunately I'm stuck trying to fix the issue with my git repository. The Ventral page says:

It appears you are working in the "heads/6.x-1.0" branch in git. You should really be working in a version specific branch.

I'm not 100% sure what this means, I've renamed my local HEAD to 6.x-1.x, but now I can't get rid of 6.x-1.0 from the remote repo at all. If I'm honest, I'm finding all this git stuff pretty confusing, despite having read as much as I could find! From what I gather, I don't want a master branch at all. I think that 6.x-1.0 is the master branch, but I'm not allowed to delete it.

I'm using git through the OSX application, Tower. I have tried running commands from the terminal window too - same problems.

If anyone can help with this, I'd be most grateful.

alex.sorokin.v’s picture

Status: Needs review » Needs work

Hi!

Today I had the same issue with Git.

But I didn't rename any branches. I added new branch and delete master branch.

I believe that this link is help you: http://drupal.org/node/1127732

Please don't forget about step "5. Set the appropriate default branch."
I had a lot of problem without this step :)

-regards

alex.sorokin.v’s picture

wuh’s picture

Status: Needs work » Needs review

Thanks to everyone who's commented and given feedback on this, and thank you Alex for your git help. I've now cleared all errors and warnings flagged by Ventral.

In answer to indydas, I presume you're talking about the return on line 39 of pet_rules.module. I don't think it's necessary to change this return, its only function is to make sure the main hook_submit in the PET module itself is also executed. I don't plan to expand this for roles at the moment, maybe I could look at that at a later date.

Hopefully this is good to go now, let's see!

vineet.osscube’s picture

Hi,
Manual review of your coding.

1). Option values pet_title pass into from t() function at line 59 into pet_rules.rules.inc file.
2). Wrong comment info at line 23 it is implementation of hook_rules_condition_info() not hook_condition_info(). into file pet_rules.rules.inc file.

Thanks,
osscube.

wuh’s picture

Hi osscube - I've fixed the second error you flagged, it should indeed have been hook_rules_condition_info.

The first thing however, I'm not sure why you're suggesting l() here as it's not a link? Maybe I've misunderstood you?

Thanks for taking the time to comment

vineet.osscube’s picture

Hi Wuh,
Sorry ! it is mistake, i suggesting for t() function. I am update it.
Thanks,

wuh’s picture

I won't make that change because titles are not wrapped in t() in the PET module (for which this is an addon).

For example, lines 29-33 in pet.admin.inc:

$rows[] = array(
  l($pet->name, 'pet/' . $pet->name),
  $pet->title,
  $ops,
 );
wuh’s picture

Priority: Normal » Critical

Changing priority to critical since I've received no new feedback for 4 weeks.

klausi’s picture

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 :-)

klausi’s picture

Priority: Critical » Normal
Status: Needs review » Reviewed & tested by the community

Sorry for the delay. Make sure to review more project applications and get a review bonus and this will get finished faster.

manual review:

  1. pet_rules_rules_event_info(): wrong doc block, this is a hook implementation and should be documented as such. See http://drupal.org/node/1354#hookimpl . Same for pet_rules_rules_condition_info() and possibly elsewhere, I think all doc blocks on all hooks are wrong in the module.
  2. pet_rules_form_alter(): use hook_form_FORM_ID_alter() instead.

But that are not critical application blockers, so I guess this is RTBC.

klausi’s picture

Status: Reviewed & tested by the community » Fixed

No objections for more than a week, so ...

Thanks for your contribution, wuh!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

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.

Thanks to the dedicated reviewer(s) as well.

wuh’s picture

Thanks very much Klausi. I have made the last few changes you recommended and promoted this to a full project: http://drupal.org/project/pet_rules

I'll try to make a Drupal 7 version of the module ASAP :)

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

Adding link to review of Metis project application