1

i am using Pony.mail to send mail within Sinatra, what i have now is two forms, one that only sends an email address for subscription to newsletter and the second form is a contact form, both are going through the same action.

What I am trying to achieve is if the subscription field is completed then only send those params or if the contact form is completed and sent then send those params

Heres what i come up with so far, but getting undefined method nil

 post '/' do
  require 'pony'
 Pony.mail(
 :from => params[:name] || params[:subscribe],
 :to => 'myemailaddress',
 :subject => params[:name] + " has contacted you via the Website" ||  params[:subscribe] + " has subscribed to the newsletter",
 :body => params[:email] + params[:comment],
 :via => :smtp,
 :via_options => {
 :address              => 'smtp.gmail.com',
 :port                 => '587',
 :enable_starttls_auto => true,
 :user_name            => 'myemailaddress',
 :password             => 'mypassword',
 :authentication       => :plain, 
 :domain               => "localhost.localdomain" 
 })
  redirect '/success' 
 end

is this even possible or would each form have to be dealt with individually?

Thanks

Richlewis
  • 15,070
  • 37
  • 122
  • 283

1 Answers1

4

There are several stages I'd go through to refactor this code.

1. Extract the things that are changing (and make them more Rubyish)

post '/' do
  require 'pony'

  from = params[:name] || params[:subscribe]
  subject = "#{params[:name]} has contacted you via the Website" ||
            "#{params[:subscribe]} has subscribed to the newsletter"
  body = "#{params[:email]}#{params[:comment]}"

  Pony.mail(
    :from => from,
    :to => 'myemailaddress',
    :subject => subject,
    :body => body,
    :via => :smtp,
    :via_options => {
    :address              => 'smtp.gmail.com',
    :port                 => '587',
    :enable_starttls_auto => true,
    :user_name            => 'myemailaddress',
    :password             => 'mypassword',
    :authentication       => :plain, 
    :domain               => "localhost.localdomain" 
  })
  redirect '/success' 
end

2. Make clear your intentions

in this case, that there are two branches through the code.

post '/' do
  require 'pony'

  if params[:name] # contact form
    from = params[:name]
    subject = "#{params[:name]} has contacted you via the Website"
  else # subscription form
    from = params[:subscribe]
    subject = "#{params[:subscribe]} has subscribed to the newsletter"
  end

  body = "#{params[:email]}#{params[:comment]}"

  Pony.mail(
    :from => from,
    :to => 'myemailaddress',
    :subject => subject,
    :body => body,
    :via => :smtp,
    :via_options => {
    :address              => 'smtp.gmail.com',
    :port                 => '587',
    :enable_starttls_auto => true,
    :user_name            => 'myemailaddress',
    :password             => 'mypassword',
    :authentication       => :plain, 
    :domain               => "localhost.localdomain" 
  })
  redirect '/success' 
end

(I'm not a big fan of setting local vars within conditional branches, but we'll ignore that for clarity. I'd probably create a hash before the conditional with the keys already done, and then populate it in the branches but YMMV.)

3. Extract what doesn't change from what does.

Sinatra has a configure block just for this kind of thing.

require 'pony'

configure :development do
  set :email_options, {
    :via => :smtp,
    :via_options => {
    :address              => 'smtp.gmail.com',
    :port                 => '587',
    :enable_starttls_auto => true,
    :user_name            => 'myemailaddress',
    :password             => 'mypassword',
    :authentication       => :plain, 
    :domain               => "localhost.localdomain" 
  }
end

Pony.options = settings.email_options

Notice I've added :development as you may want to set it up differently for production.

Now your route is a lot cleaner and easier to debug:

post '/' do

  if params[:name] # contact form
    from = params[:name]
    subject = "#{params[:name]} has contacted you via the Website"
  else # subscription form
    from = params[:subscribe]
    subject = "#{params[:subscribe]} has subscribed to the newsletter"
  end

  body = "#{params[:email]}#{params[:comment]}"

  Pony.mail
    :from => from,
    :to => 'myemailaddress',
    :subject => subject,
    :body => body,

  redirect '/success' 
end

My last tip, would be to put as many of those Pony options into ENV vars, which will not only keep things like passwords out of source control but also allow you to change the settings a lot easier. Perhaps put them in a Rakefile and load different environments for different contexts etc.


To use environment variables, I do the following:

# Rakefile

# in this method set up some env vars
def basic_environment
  # I load them in from a YAML file that is *not* in source control
  # but you could just specify them here
  # e.g. ENV["EMAIL_A"] = "me@example.com"
end

namespace :app do

  desc "Set up the environment locally"
  task :environment do
    warn "Entering :app:environment"
    basic_environment()
  end

  desc "Run the app locally"
  task :run_local => "app:environment" do
    exec "bin/rackup config.ru -p 4630"
  end
end

# from the command line, I'd run
`bin/rake app:run_local`

# in the Sinatra app file
configure :production do
    # these are actual settings I use for a Heroku app using Sendgrid
    set "email_options", {      
      :from => ENV["EMAIL_FROM"],
      :via => :smtp,
      :via_options => {
        :address => 'smtp.sendgrid.net',
        :port => '587',
        :domain => 'heroku.com',
        :user_name => ENV['SENDGRID_USERNAME'],
        :password => ENV['SENDGRID_PASSWORD'],
        :authentication => :plain,
        :enable_starttls_auto => true
      },
    }
end

# then a block with slightly different settings for development
configure :development do
  # local settings…
    set "email_options", {
      :via => :smtp,
      :via_options => {
        :address              => 'smtp.gmail.com',
        :port                 => '587',
        :enable_starttls_auto => true,
        :user_name            => ENV["EMAIL_A"],
        :password             => ENV["EMAIL_P"], 
        :authentication       => :plain,
        :domain               => "localhost.localdomain"
      }
    }
end

I usually keep most of these setting in a YAML file locally for development, but add these to the production server directly. There are lots of ways to handle this, YMMV.

ian
  • 12,003
  • 9
  • 51
  • 107
  • Thank you so much, there is much to take in from this which is great.I have tried setting ENV variables as i dont want to hardcode my credentials into the mail config, though I get SMTP validation errors, though when i hardcode the credentials it works, very strange on that one – Richlewis Feb 07 '13 at 14:10
  • also what is the benefit of say params[:name] compared to #{params[:name]}, you are using interpolarization here yes? – Richlewis Feb 07 '13 at 14:12
  • @Richlewis I've updated the answer with some stuff about env vars. _Interpolation_, yes, and it's the way that most string munging is done in The Ruby Style™. For me, it reads better, and I believe it performs better but don't rely on that piece of memory! :) – ian Feb 07 '13 at 16:36