2 forms on same page, Sending using Pony in Sinatra, same email address

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

Solution

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.