Bonfire
Bonfire copied to clipboard
Difference between Site Email and System Email
As mentioned by @JbalTero on the forums, the difference between Site Email and System Email is not clear.
There may be some instances of the users module using the Site Email when it should be using the System Email (as the 'from' address in emails sent by the system), and the description of the Site Email should be changed to reflect that the Site Email is intended for display to end users (while the System Email is the email address used in the 'from' address for emails sent by the system).
While the two can be the same, and probably were at one point, we want people to be able to use one email for display on the site (a contact address), and a different email for the from address on emails (for example, a noreply@ address that doesn't exist on the mail server).
I encountered an error when I try to "Send New Email" on a freshly installed Bonfire.
Failure creating emails: One or more required fields are missing.
The error persist until I set the Sytem Email on Email settings.
That's why I got that question on the forum.
I think the best solution for that, in addition to the items above, would be to change the Emailer library's send() method to fall back to settings_item('site.system_email') if $data['from'] is empty and settings_item('sender_email') == false, which would require changing the following lines: https://github.com/ci-bonfire/Bonfire/blob/0.8-dev/bonfire/modules/emailer/libraries/Emailer.php#L120 https://github.com/ci-bonfire/Bonfire/blob/0.8-dev/bonfire/modules/emailer/libraries/Emailer.php#L130
For example, it might look something like this:
if (empty($data['to'])
|| ! isset($data['message'])
|| ! isset($data['subject'])
|| (empty($data['from'])
&& settings_item('sender_email') == false
&& settings_item('site.system_email') == false
)
) {
$this->error = lang('emailer_missing_data');
return false;
}
// ... $to, $subject, etc.
$from = empty($data['from']) ? (settings_item('sender_email') ?: settings_item('site.system_email')) : $data['from'];
or, to remove the nesting in the if() statement and perform one last check on the $from value:
public function send($data = array(), $queueOverride = null)
{
// Ensure the required information is supplied.
$from = empty($data['from']) ? (settings_item('sender_email') ?: settings_item('site.system_email')) : $data['from'];
if (empty($data['to'])
|| ! isset($data['message'])
|| ! isset($data['subject'])
|| empty($from)
) {
$this->error = lang('emailer_missing_data');
return false;
}
// ... $to, $subject, $message, etc. ...
sender_email
and site.system_email
is still confusing. I think sender_email
should be something like email.system_email
and site.system_email
should be site.site_email
though I think this refactoring needs to check every referenced string.
Your solution above is good enough whenever sender_email
is not provided.
And we should change the description for sender_email
and site.system_email
to avoid confusion and also with documentation.
oh nevermind my refactoring suggestion. That would confuse more users especially upgrading users.
It's a good idea, but even if we were to go down that road, it would be a compatibility break and would require some time between the decision to make the change and the change itself (marking it deprecated in the documentation, maintaining the new and old names in the system side-by-side for a while, etc.).
I have the same problem @JbalTero but in my case is the limitation of password length, I open a case #1181.
@andrebellafronte That's a completely different problem, but I've given you a potential fix over there. If that works, we can make the change in the module, as I don't see any additional problems with that.
I've pushed the fallback to Site Email and clarified the wording on the settings view (though we'll need some updates to the translations). I haven't taken a look at the documentation for these settings, yet. I'm still considering whether we should work on the names of the settings.