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

Email Library #276

Closed
lonnieezell opened this issue Sep 24, 2016 · 24 comments
Closed

Email Library #276

lonnieezell opened this issue Sep 24, 2016 · 24 comments
Assignees
Labels
in progress new feature PRs for new features
Milestone

Comments

@lonnieezell
Copy link
Member

The email library is in drastic need of some refactoring. Here's my thoughts:

  • MUST support multiple drivers, including mail, sendmail, smtp, log (for simply saving the generated emails to the writable directory), and for a few services like MailTrap.io, MailGun, Postmark or Mandrill.
  • We can simplify things and make a Mailer class the single point to manage everything about that email, including which views are used, default from values, etc. This also gives us the power to really customize the emails if we need to by tapping into any part of the application to gather data and display it correctly, etc
  • MUST be able to easily queue the emails to be sent at a later date, and/or throttled. This would integrate with the Queue System that is already planned.

In your controllers, when you need to send an email, it would be as simple as:

(new App\Mail\NewUserWelcomeEmail())
    ->to($user->email, $user->name)
    ->send();
@lonnieezell lonnieezell added new feature PRs for new features in progress labels Sep 24, 2016
@lonnieezell lonnieezell added this to the Pre-Alpha 2 milestone Sep 24, 2016
@lonnieezell lonnieezell self-assigned this Sep 24, 2016
@sv3tli0
Copy link
Contributor

sv3tli0 commented Sep 24, 2016

Will it be possible to add SignedBy(encrypted) method to all other standard things?
I think that it may be helpful if an app can send signed mails in some cases ..

@lonnieezell
Copy link
Member Author

Will it be possible to add SignedBy(encrypted) method to all other standard things?
I think that it may be helpful if an app can send signed mails in some cases ..

No promises, but I can look into it.

@ivantcholakov
Copy link

bcit-ci/CodeIgniter#3626

@aa6my
Copy link

aa6my commented Nov 12, 2016

@lonnieezell should we use many service as you state? but we target simple. I think the better one

(new App\Mail\NewUserWelcomeEmail()) ->to($user->email, $user->name) ->send($service);

service we can make it null refer to default library. the rest depend on service. etc mailgun postmark ...

@lonnieezell
Copy link
Member Author

@aa6my Services are pretty simple, and they make customizing everything much easier. That said, I'm definitely NOT happy with the email library at the moment, which is part of why it's been sitting for a bit while I work on other parts. :)

The original intent was to do it much like a number of other services, where there's a helper method, so it would look like this:

mailer(new App\Mail\NewUserWelcomeEmail())
    ->to($user->email, $user->name)
    ->send();

So it's very similar to what you've specified. And the problem with passing the service in at that point, is that should you ever change who you're sending email with (which could happen relatively often throughout the lifespan of projects) you have to change it at every location. By making it a service, you only have to change it in the Services config file and everything works.

The goal for has always been to make App\Mail\NewUserWelcomeEmail() the one place where all of the truth for the message is, all of the headers, addresses, subject line, etc. Thinking on this a little more today, and on something that @sv3tli0 mentioned on the forums a little while back, I may have another idea worth exploring:

The mail file is still the location of truth. But we can have services you declare in it, like Mailable, Pushable, SMSable, which provide the delivery methods. So, if you want the message to be sent by email, you'd make sure the class used the Mailable trait which provides a mail() method. If you sometimes wanted to mail something and sometimes need to push it via a cellular push notification (like when the system has major issues) you could use both Mailable and Pushable and could use mail() and push() as possible delivery methods. And queue() should always be available to add it to a queue to be dealt with asynchronously.

@aa6my
Copy link

aa6my commented Nov 15, 2016

@lonnieezell thanks for explaining for me. Actually i like this part

The mail file is still the location of truth. But we can have services you declare in it, like Mailable, Pushable, SMSable, which provide the delivery methods. So, if you want the message to be sent by email, you'd make sure the class used the Mailable trait which provides a mail() method. If you sometimes wanted to mail something and sometimes need to push it via a cellular push notification (like when the system has major issues) you could use both Mailable and Pushable and could use mail() and push() as possible delivery methods. And queue() should always be available to add it to a queue to be dealt with asynchronously.

After you start code it, i'll try assist as i can,

@lonnieezell
Copy link
Member Author

I've tried planing this out, and it really doesn't work well. There's too many differences between what you would send as an email and a text/push message. I was getting a bit frustrated at that point, only to realize I had already solved much of this problem with SprintPHP. So, will be moving much of the methodology of that over and that should work out fairly nice.

@isantolin
Copy link

MUST support multiple drivers, including mail, sendmail, smtp, log (for simply saving the generated emails to the writable directory), and for a few services like MailTrap.io, MailGun, Postmark or Mandrill.

Amazon SES, Sendgrid too?

@ivantcholakov
Copy link

ivantcholakov commented Feb 11, 2017 via email

@ivantcholakov
Copy link

@isantolin

Oh, sorry. I've answered here by a mistake. Ignore my previous post.

@mpmont
Copy link
Contributor

mpmont commented Apr 4, 2017

Whats the general opinion on just using something like phpmailer/phpmailer?

@lonnieezell
Copy link
Member Author

Typically, the community's response has been that they prefer something that doesn't use outside dependencies. Though in this case I'd be half tempted to pull in phpmailer or swiftmailer and call it good. Getting a bit tired of wrangling this one. :)

@SmolSoftBoi
Copy link
Contributor

I've created my own wrapper around ZendMail for the time being.

@mpmont
Copy link
Contributor

mpmont commented Apr 4, 2017

@lonnieezell Just do it 👍 email is one of those things that I really don't see a reason for it being part of the core. When its more than proven by now that those two are the best ones around.

@SmolSoftBoi
Copy link
Contributor

@mpmont There are likely a lot of people the don't see a reason for it being part of the core, but there are also likely a lot of people that do see a reason for it being part of the core.

Personally, I'm happy with a wrapper around an external dependency, however, I really do want to see all the features available that were proposed.

@slax0rr
Copy link

slax0rr commented Apr 5, 2017

I really do not see any disadvantage for using an external dependency and creating a wrapper around it. Nor do I see the advantage of writing yet another email library. Anything that is not available in the external library, can be added through extension. It would save a lot of work and worry, imho.

@cybosoft
Copy link

How about directly using swift mailer as base library

@goosehub
Copy link

I actually do the see disadvantage of using external dependencies, however in the case of email, I think it's completely reasonable to use PHPMailer here. As a CodeIgniter power user, I'd rather have time dedicated to other features as I believe this can become a major time sink.

@codeliter
Copy link

I belong to the school of thought that see no reason for including Email in the core. In CI3 I only used it like once. I always used a wrapper around Amazon SES or PHPmailer and my worries come to an end.

@lonnieezell
Copy link
Member Author

Just to add more things to consider: PHPMailer is licensed with LGPL, which makes it incompatible with MIT licensing, I believe, so we couldn't distribute it with the core. Swiftmailer has a compatible license (Creative Commons Attribution-ShareAlike 3.0 Unported).

However, I've also got the entire thing rewritten already, it just needs to be debugged and verified a little bit before I'm good with it. Just needed some time away from it since it was a bit of a process. :)

And, yes, it supports drivers. Am thinking an Amazon SES would be a good one to include as an example driver and a good test case to make sure what I've done is as flexible as I think it it.

@dantman
Copy link

dantman commented May 2, 2017

@lonnieezell How is it incompatible? LGPL is not viral like GPL.

@Synchro
Copy link

Synchro commented May 2, 2017

Indeed, I don't think it's incompatible at all. If you don't make changes to the library itself (which you shouldn't need to do anyway), LGPL is indistinguishable from MIT in all practical respects, and has no impact on the host project.

@lonnieezell
Copy link
Member Author

@dantman You're right. I wasn't thinking straight.

@lonnieezell
Copy link
Member Author

For now have simply ported the existing mailer class over from CI3. Can revisit in the future, potentially.

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

No branches or pull requests