We are using the smtp module to connect to sendgrid. We only want to use it when sending newsletters. Since we are paying for each mail using this third party smtp we dont want to use it for all mails. So we believe adding the smtp mail class is a better option than setting it for the whole site by default.

function smtp_enable() {
  $systems = array(
    'default-system' => 'DefaultMailSystem'
    'smtp-system' => 'SmtpMailSystem'
  )
  variable_set('mail_system', array('default-system' => 'SmtpMailSystem'));
}
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Fabianx’s picture

Status: Active » Needs work

Could you please provide a patch instead?

ParisLiakos’s picture

Status: Needs work » Needs review
FileSize
792 bytes

No,don't clear the mail_system variable and then set it:/
this is a more elegant way:

Then,domidc you could manipulate this variable as you wish without smtp or any other module interfering.
Also smtp wont screw other modules settings this way

ParisLiakos’s picture

Title: Add the mail class in hook enable instead of replacing the default one » Don't reset mail system variable
Category: feature » bug

And this is a bug.

longwave’s picture

Priority: Normal » Major

This bug breaks any other contrib module that provides their own MailSystem and that was installed before SMTP. This affects at least Ubercart: #1364612: Ubercart don't work with smtp module?

ParisLiakos’s picture

Newsletter too.

the workaround till now is enabling smtp before installing any other module:/

TR’s picture

For a verbose explanation of why the SMTP module is doing the wrong thing, see http://drupal.org/node/900794#comment-4160574

pobster’s picture

Yeah thanks for this nice gotcha... How about you commit this fix and roll another release?

Pobster

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

i guess we can rtbc it then^^
and then wait for maintainers to commit

wundo’s picture

Status: Reviewed & tested by the community » Needs work

Could you please reroll it?

ParisLiakos’s picture

Status: Needs work » Needs review
FileSize
792 bytes

sure...seems pretty much identical to me though?
doesnt #2 apply anymore?

wundo’s picture

Status: Needs review » Fixed

Hmm never mind, it applied now.
I guess my HEAD was dirty, sorry about that.

Commited! :)

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.