glFusion / glfusion

glFusion CMS - Advanced Content Management with Style
https://www.glfusion.org
GNU General Public License v2.0
3 stars 0 forks source link

Custom welcome_email.txt not being sent to imported users. #365

Closed leegarner closed 4 years ago

leegarner commented 5 years ago

In USER_createAndSendPassword(), when the welcome_email.txt template is present it is used to create the $mailtext variable. However, since $mailhtml is not also created the call do COM_mail() fails with Email Error: Message body empty.

One option would be to add $mailhtml = str_replace("\n", "<br />\n", $mailtext);

leegarner commented 5 years ago

Maybe another option (untested) would be to wrap the text output in <pre> tags.

walterrowe commented 5 years ago

The call to COM_mail assumes both $mailhtml and $mailtext are set, though as Lee points out the handler for welcome_email.txt does not set $mailhtml. Worse still is that the call to COM_mail explicitly sends HTML format by specifying “true” after the $from argument. Does the wiki discuss what format should be included in welcome_email.txt such that one could set both $mailtext and $mailhtml to the value of $template->get_var (‘output’)?

return COM_mail( $to, $msgData['subject'], $msgData['htmlmessage'], $from, true, 0,'', $msgData['textmessage'] );

leegarner commented 5 years ago

A good point. Now that I've thought about it, I think this might really be a non-issue. The best approach is probably to create custom versions of newuser_template_html.thtml and newuser_template_text.thtml instead. Though I like the convenience of one template, this is probably the right way do do it.

walterrowe commented 5 years ago

Does the name (.txt suffix) imply that welcome_email.txt is for plaintext email? If so, should the isHtml argument to COM_mail() be false instead of true when welcome_email.txt is present?

Neither mailhtml or mailtext are initialized to empty string in USER_createAndSendPassword() in lib-user.php. The call to COM_mail() hardcoded isHtml to true.

If isHtml were a variable with default to true in USER_createAndSendPassword() and set to false when welcome_email.txt is present, and mailtext and mailhtml were property initialized to empty string, would that solve the problem?

leegarner commented 5 years ago

It actually would, but to keep with the single exit point of return COM_mail(... it should use $mailhtml for the message body. This should also work (but personally I'm switching to customizing the existing newuser_email-* templates).


--- a/private/system/lib-user.php
+++ b/private/system/lib-user.php
@@ -278,7 +278,9 @@ function USER_createAndSendPassword ($username, $useremail, $uid, $passwd = '')
         $template->set_var ('password', $passwd);
         $template->set_var ('name', COM_getDisplayName ($uid));
         $template->parse ('output', 'mail');
-        $mailtext = $template->get_var ('output');
+        $mailhtml = $template->get_var ('output');  // actually text
+        $mailtext = '';         // not used
+        $isHTML = false;
     } else {
         $T = new Template($_CONF['path_layout'].'email/');
         $T->set_file(array(
@@ -326,6 +328,7 @@ function USER_createAndSendPassword ($username, $useremail, $uid, $passwd = '')

         $T->parse ('output', 'text_msg');
         $mailtext = $T->finish($T->get_var('output'));
+        $isHTML = true;
     }
     $msgData['htmlmessage'] = $mailhtml;
     $msgData['textmessage'] = $mailtext;
@@ -336,7 +339,7 @@ function USER_createAndSendPassword ($username, $useremail, $uid, $passwd = '')
     $from = COM_formatEmailAddress( $_CONF['site_name'], $_CONF['noreply_mail'] );
     $to = COM_formatEmailAddress('',$useremail);

-    return COM_mail( $to, $msgData['subject'], $msgData['htmlmessage'], $from, true, 0,'', $msgData['textmessage'] );
+    return COM_mail( $to, $msgData['subject'], $msgData['htmlmessage'], $from, $isHTML, 0,'', $msgData['textmessage'] );
 }```
walterrowe commented 5 years ago

That is actually what I meant. Glad to see we think alike. You don’t show any variable initialization. I would add that at the top for completeness.