fuel / email

Fuel PHP Framework - Fuel v1.x Email library
60 stars 38 forks source link

Multi sending #75

Open WinterSilence opened 5 years ago

WinterSilence commented 5 years ago

Example:

$mailer = Email::forge();
$body = View::forge('email/template', $data);
$mailer->html_body($body);
foreach ($mailing_list as $user) {
   $body->set('user', $user);
   $mailer->to($user['email'])->send();
}

Code like this throw error or work slow because multi sending not supported. Rework:

  1. body/alt_body typed in Email_Driver::send(), not in setters
  2. Add method Email_Driver::before_send() called in Email_Driver::send() init properties only once
  3. Parse attachments from body is bad solution. Also: https://github.com/fuel/email/blob/5d0965258c001529beb1bab505260f4e5cb7c464/classes/email/driver.php#L233 'second/img.jpg' rewrite 'firtst/img.jpg'

Email\View_Body would solve it:

namespace Email;
use Fuel\Core\View;
class View_Body extends View
{
    protected $email;
    protected $attachments = [];

    public function __construct(Email_Driver $email, $file = null, $data = [], $filter = null)
    {
        $this->email = $email;
        $this->bind('email', $this->email);
        parent::__construct($file, $data, $filter);
    }

    public function set_filename($file, $reverse = false)
    {
        // TODO: add in config `'base_dir' => 'emails'` - directory with mail templates
        if ($dir = $this->email->get_config('email.defaults.base_dir'))
        {
            $file = $dir.DIRECTORY_SEPARATOR.$file;
        }

        return parent::set_filename($file, $reverse);
    }
    public function render($file = null)
    {
        // TODO: update method `clear_attachments` for optional clear
        $this->email->clear_attachments('inline');
        $this->attachments = [];

        return parent::render($file);
    }
    /**
     * Template helper.
     * @example `<img src="<?=$this->attach_inline($url)?>">`
     * @param string $path Path/URL to file
     * @return string CID
     */
    protected function attach_inline($path)
    {
        $id = 'cid:'.md5($path);
        if (!in_array($id, $this->attachments))
        {
            $this->email->attach($path, true, $id);
            $this->attachments[] = $id;
        }
        return $id;
    }
}
WanWizard commented 5 years ago

You have to be more specific, the Email package isn't written by me, I don't know all ins and outs... ;)

As to 3: I don't think the package should force the use of a view for the email body, which means attachments should still work if you pass a string to body().

WinterSilence commented 5 years ago

@WanWizard

isn't written by me, I don't know all ins and outs.

I don't focus on u. I analise mail libs and write my version.

$email->html_body(\View::forge('email/template', $email_data));

it's next step after this example :) Current parser use very easy algorithm(ex: rewrite code added in pre/code tags) and can't be disabled in config/method.

, which means attachments should still work if you pass a string to body()

I'm not understood you

WanWizard commented 5 years ago

You move code / functionality from the email classes to a View class extension. That creates a dependency, and it breaks existing code that calls html_body() with a string instead of a View_Body instance.

WinterSilence commented 5 years ago

@WanWizard

You move code / functionality from the email classes to a View class extension

where u find moved code / functionality?

That creates a dependency

View_Body created for this. I can transfer code into Email_Driver, but that class already not so SOLID.

breaks existing code that calls html_body() with a string instead of a View_Body instance.

Nope, class have __toString() method

You have any ideas how adds multi sending and don't break anything?

WanWizard commented 5 years ago

Ok, lets call it duplicating code then. Like attach_inline(), which should be inside the email classes.

Nope, class have __toString() method

You misunderstood me, inline images should still work for code NOT using a View based solution. Like all existing code already out there... Which means the existing code should be fixed, not worked around.

WinterSilence commented 5 years ago

@WanWizard

Like attach_inline(), which should be inside the email classes.

i add this wrapper because current "attach" methods not returns cid and this solution dont breaks existing code.

Which means the existing code should be fixed, not worked around

I dont have other ideas, replacing in string is worse solution