renz45 / mandrill_mailer

A small gem for sending Mandrill template emails
260 stars 84 forks source link

Defaults set on a child class wipe out all defaults form the parent class #99

Closed olivierlacan closed 8 years ago

olivierlacan commented 9 years ago

Example:

class MailerBase < MandrillMailer::TemplateMailer
  default from: "email@example.com",
          from_name: "Example",
          view_content_link: true
end

class FunMailer < MailerBase
  default view_content_link: false
end

The FunMailer will now have from_name and from reset to the mandrill_mailer defaults, not the MailerBase. So basically we need to fix this: https://github.com/renz45/mandrill_mailer/blob/5cae59e0f91cf3aedba85c37afde310be09af1a8/lib/mandrill_mailer/core_mailer.rb#L154-L159

From this:

def self.default(args)
      @defaults ||= {}
      @defaults[:from] ||= 'example@email.com'
      @defaults[:merge_vars] ||= {}
      @defaults.merge!(args)
    end

To this:

def self.default(args)
      @defaults ||= {}
      @defaults[:from] ||= (super_defaults[:from] || 'example@email.com')
      @defaults[:merge_vars] ||= (super_defaults[:merge_vars]|| {})
      @defaults.merge!(args)
    end

Makes sense? @renz45 @katiedelfin

renz45 commented 9 years ago

Odd... usually using @ inside a class method will be local to the current top level class, while using @@ would pollute the entire inheritance chain. I wonder if its a newer version of ruby that changes this behavior?

olivierlacan commented 9 years ago

That would probably show up in the Travis build Matrix if it were recent Ruby related. No?

renz45 commented 9 years ago

Dumb little example:

class One
  def self.set_one
    @test = "this is one"
  end

  def self.one
    @test
  end
end

class Two < One
  def self.set_two
    @test = "this is two"
  end
end
One.one
> nil

One.set_one
One.one
> "this is one"

Two.one
> nil

Two.set_two
Two.one
> "this is two"

One.one
> "this is one"

works for me in 2.0.0-p353

renz45 commented 8 years ago

Going to close this, feel free to reopen if it pops back up.