Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Default email behaviour #1671

Closed
laurencei opened this Issue Jul 30, 2012 · 10 comments

Comments

Projects
None yet
6 participants

The current default email behavior may result in unexpected consequences.

Currently it stores the data inbetween emails, unless you specifically call the clear() function. The problem is this is not inherently obvious, and can lead to silent errors that cause issues.

For example:

 public function index()
 {
  $this->load->library('email');

  $this->email->from('test@example.com', 'Codetest');       
        $this->email->to('example@example.com');
        $this->email->cc('cc@example.com');
        $this->email->subject('Test subject');
        $this->email->message('This is a test email');
        $this->email->send();
        echo $this->email->print_debugger();

  $this->email->from('test@example.com', 'Codetest');       
        $this->email->to('another@example.com');
        $this->email->subject('Another test subject');
        $this->email->message('This is a test email');
        $this->email->send();
        echo $this->email->print_debugger();

 }  

At first glance, this code seems ok. However the second email will include the "cc" from the first email! The only way to fix is it include "clear" between the emails.

I think it would make more sense to include some sort of "auto clear" function in the email config file, and set it to true by default.

That way, anyone who wants to use the mass mailing functions of Codeigniter can simply set it to false - but people who start using CI wont fall into a trap of unexpected emails being sent out with old data.

Contributor

janosrusiczki commented Jul 30, 2012

While this is generally a good idea I'm not sure if the proposed change won't break existing applications. I have relied on the way it currently works to send out notifications to multiple recipients by first setting up the email with everything except the recipient and then setting each recipient and sending the mail inside a loop.

Kitsched - thats why I propose a config option. Whilst the default config should set the "clear" function to true, anyone who is upgrading can simply set that to "false" to maintain existing applications. It could be included in the Change logs as a point to check/do during the 2.1.2 -> 3.0 upgrade for example.

perhaps you could even do:

$this->email->send(BOOL)

where BOOL = clear or not

so

$this->email->send(TRUE) // clear the email buffer after send
$this->email->send(FALSE) // keep the buffer for mass mailing

Contributor

janosrusiczki commented Jul 30, 2012

Both proposals sound good but I think it would be better to set the default behavior to the way things used to work.

Contributor

it-can commented Jul 30, 2012

I think it should be default FALSE, so the change won't break any current applications...

If you set the default to 'false' - then the behavior will remain - and the whole point behind this change is the fact the default behavior is not inherently obvious.

But if you include this as part of the "change log" upgrade - people who want that "old" behavior can simply configure it to be false with one simple config line change, but meanwhile new projects, and new people to CI, wont suffer from this issue....

Contributor

alexbilbie commented Jul 31, 2012

My personal view is that if you're upgrading to a major version of something then you should expect some things to break and you should carefully go over the changelog.

Before we release 3.0 there will be an extensive run down of all of the new and fixed things but also a clear section that describes breaking behaviour.

cc/ @ericbarnes @philsturgeon @narfbg

Contributor

narfbg commented Jul 31, 2012

@alexbilbie With that in mind - you should've probably noted that change in the upgrade notes. :)

Contributor

alexbilbie commented Jul 31, 2012

@narfbg good point...I'll do it now :)

Contributor

philsturgeon commented Jul 31, 2012

Yay for the singleton.

@alexbilbie alexbilbie closed this Jul 31, 2012

nonchip pushed a commit to nonchip/CodeIgniter that referenced this issue Jun 29, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment