Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi sending #75

Open
WinterSilence opened this issue Jun 2, 2019 · 6 comments
Open

Multi sending #75

WinterSilence opened this issue Jun 2, 2019 · 6 comments

Comments

@WinterSilence
Copy link

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:
    $cid = 'cid:'.md5(pathinfo($image_url, PATHINFO_BASENAME));

    '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
Copy link
Member

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
Copy link
Author

@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
Copy link
Member

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
Copy link
Author

WinterSilence commented Jun 3, 2019

@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
Copy link
Member

WanWizard commented Jun 3, 2019

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
Copy link
Author

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants