Provide example mail module on api.drupal.org

Moving to our new examples module project.

Yes freaking please. Ugh.

I'll see if I can whip up something when I finally figure it out. :P

Yoink!

I talked this over with ilo and rfay in IRC, and I think this is what we're going to do:

1. Create a simple form that sends an e-mail when submitted. Probably basically take http://drupal.org/node/197122 but clean it way up.

2. Also add a hook_mail_alter() implementation that adjusts an existing mail (maybe the user registration mail or something) to demonstrate that.

I was going to get into defining settings pages to define mail templates, but that's over an beyond just the basic "I need to *&@#ing send a (*&@#ing email" so we'll keep it simple for now.

Some other references:

http://api.drupal.org/api/function/contact_mail/6 seems to be the most non-cracked-out hook_mail() implementation in core.

rfay also posted http://drupalbin.com/12224 as a working example.

Ok, here's a first stab at a "simple" example. I wavered on whether to split the form functions into a separate file, but in the end decided it's much easier to grok this when all the code is in one place, and it's hard enough to grok already. :P

I've no idea if the comments here are way overkill or not, but figured I'd err on the side of being overly verbose, since it's real easy to delete text, but not as easy to invent it. ;)

Note that I haven't implemented hook_mail_alter() yet. I'm stuck trying to think of a good use case for it. I was kinda thinking maybe forcing all e-mails to be sent as HTML, but I don't really have time to dig into how to do that.

Oh, btw, I need someone to help test this since I don't have SMTP set up locally. :\

Thanks! Long needed!

And oh the delight of getting to review a webchick patch. :-)

Thanks webchick! :)
rfay, remember, comments no longer than 80 cols, periods at the end! don't let anyone pass!

Hahaha. :) Be careful not to go mad with power now, you two! ;)

As no movement has been here for 4 days I take this and build up a patch in the d6 version for now.

I apologize I haven't reviewed this or others, Ilo - I've been out of town.

On this, it was submitted by webchick, so go ahead and review, revise, and commit when you think it's good.

here we go.. Still needs review, I've included hook_mail_alter and simpletest code. Have to say.. was too hard to review webchick's submission.. ;)

Haha! I, on the other hand, have no qualms about reviewing your patch! ;)

(This is all meant with the highest degree of respect, btw, and I *really* appreciate you finishing what I started! I just think it's important to be extra nit-picky on these, since they are examples that new developers will learn from. Consistency is key, so that they learn good practices.)

+++ email_example/email_example.module Locally New
@@ -0,0 +1,180 @@
+/**
+ * Other email functionalities provided by Drupal
+ *
+ * Although they are not required to successfully use Drupal mail system, they
+ * still provide an enhanced control over emails being prepared or sent using
+ * the Drupal's email system.
+ */

I'm not sure I'd add this here. If anything, stick a sentence or so to the effect of "hook_mail_alter() is yet another example of what can be done with mail API" in the @file section at the top.

+++ email_example/email_example.module Locally New
@@ -0,0 +1,180 @@
+function email_example_mail_alter(&$message) {

I would suggest moving this up further, beneath email_example_mail_send(), so that all of the mail-related stuff is in one "hunk", and all of the non-email stuff comes below. I was thinking we might also want to put a divider of some kind like ///// Supporting functions //// between the two hunks, that so people know that the stuff before that is about mail, and the stuff after it is just set-up.

+++ email_example/email_example.module Locally New
@@ -0,0 +1,180 @@
+  // Hook_mail_alter() provides an interface to alter any aspect of email sent
+  // by Drupal. You can use this hook to add a common site footer to all
+  // outgoing email, add extra header fields, and/or modify the email in any
+  // way. HTML-izing the outgoing email is one possibility.
+

I would actually move this into the PHPDoc section, since it's about hook_mail_alter() itself, and not the code within.

+++ email_example/email_example.module Locally New
@@ -0,0 +1,180 @@
+  // For the purpose of this examlpe, modify all the outgoing messages and

"example" :)

+++ email_example/email_example.module Locally New
@@ -0,0 +1,180 @@
+  // the message was built.

I'd do "in which the message was built." The current sentence isn't quite proper English.

+++ email_example/email_example.test Locally New
@@ -0,0 +1,101 @@
+class EmailExampleTestCase extends DrupalWebTestCase {

We should have a summary line of PHPDoc here.

+++ email_example/email_example.test Locally New
@@ -0,0 +1,101 @@
+  /**
+   * Implementation of getInfo().
+   */
...
+  /**
+   * Implementation of setUp().
+   */

We don't actually put PHPDoc on top of these functions since they are inherited from the base class.

+++ email_example/email_example.test Locally New
@@ -0,0 +1,101 @@
+      'name' => t('Email example'),
+      'description' => t('Verify the email submission using the contact form.'),
+      'group' => t('Examples'),

These do not actually get put into t(), since they're not strings that ever need to be translated.

+++ email_example/email_example.test Locally New
@@ -0,0 +1,101 @@
+     $email_options['email'] = $this->randomName() .'@'. $this->randomName() .'.drupal';

For concatenation, you should use " . " (space on either side) -- this is a new standard added in D7, but I think it makes sense to pre-emptively back-port it.

+++ email_example/email_example.test Locally New
@@ -0,0 +1,101 @@
+     $email  = $this->drupalGetMails(array('email' => $email_options['email']));

There's an extra space between email and =

I'm on crack. Are you, too?

Hm.. Thank you very much for your comments, Id tried to keep as close to them as possible ;)

So..
- I've detailed that two examples are provided by the email example module in the @file header.
- moved the hook_mail_alter to the email functions hunk.
- included the //// Support functions //// separator.
- Fixed the English grammar (Sorry, this one I can't do better)
- Added the PHPdoc header to the testing class.
- Remove the headers for setUp() and getInfo()
- cross my fingers

:)

Of course, I forgot to mark as nr, can't do that with fingers crossed.

Missed the string issues in the patch, so I've reviewed all the suggestions again. Rerolled. If everybody agree with webchick's suggestions I guess this could be finished so we can start to work on d7 port.

OK, I committed this one to D6 with minor changes (attached). The .diff file shows the items that were changed by me in the commit. The .patch is what was committed in http://drupal.org/cvs?commit=286346

The changes I made:

• Small comment improvements
• the hook_mail_alter() example used the $language object as if it were a code; the code had to be pulled out of the object.
• The result check on drupal_mail() was incorrect. @webchick, if you provided this to your client, you may want to update it.

Now on to D7! Thanks to both of you for the excellent work on this.

Wow. Now that's what I call team work! Awesome! I'm so happy to finally have an example for this to give to folks. :D

This Examples project is such a great idea. Thanks so much for your hard work on this, guys! I'll see where else I can contribute here as I come across client needs and/or time.

Oh.. tricky part to get this patch file.. my cvs tools conspired against this issue :)

Briefing:
- minor changes for t() (2nd argument should be array() and third argument is now an array with 'langcode' key..
- updated .info file
- signatures to "Implement "

The test on hook_email_alter fails on this one, although it works fine in a live test.

Email signature successfully verified. Other email_example.test 97 EmailExampleTestCase->testContactForm() Fail

The added debug() in the test shows that the signature is not there. It is there on a live test.

I think this one is going to be a core bug. Could you pursue it?

BTW: Only tiny changes in this patch, but I had to recreate the .test file by hand, as the patch was invalid.

THANKS!

I'll not provide a new patch as we both are facing problem for each other files. Just would like to notice you what I've found..

This is a header of the email:
[Content-Type] => text/plain; charset=UTF-8; format=flowed; delsp=yes
This is the body of the email using $this->randomString(128) to generate the body:
s136221k:7cFs_]f.uX*i>*kjzr`S?-^\'f-p8q1x)rGP=O@G(.zFYRB2tQ/|8ot(gK....

As long as the string contains random html tags, <, >, and so, a bad-formed html body is transformed by drupal_html_to_text(), and part of the email body (sometimes the signature) is removed as being part of a html tag. The signature is in fact inserted, but gets removed when formatting the message body.

This is a bug (probably a fix should be backported for the testcase of the D6 version) introduced by trying to send bogus html as body of text messages in the testcase. The fix, replace randomStr() with randomName() so no html tags are used in the body.

Can you fix that yourself in your own local copy and do some tests before committing, I did verify this with a change in the testcase to send 1000 emails using randomName() (0 errors) and randomStr() ~30% of messages fail.

Thanks for the review, rfay!

Committed the attached to head http://drupal.org/cvs?commit=289908

Thanks for the nice work on this.

Let's follow up on the issue you correctly described in #20.

Thanks rfay!

The issue is #634618: email body gets truncated on character '<' followed by any special tag: example <b .