Rails Error When Trying To Mail to Multiple Users

I am trying to use the Whenever plugin for rails to perform a model process at certain times.

When I try to use the mail_out process in my User model, I get the following error. Can someone please point me in the right direction of what is going wrong?

/var/lib/gems/1.8/gems/rails-2.3.5/lib/commands/runner.rb:48: /var/lib/gems/1.8/gems/activesupport-2.3.5/lib/active_support/dependencies.rb:380:in `load_without_new_constant_marking': /home/tnederlof/Dropbox/Ruby/daily_trailer/app/models/user.rb:9: syntax error, unexpected tIDENTIFIER, expecting ')' (SyntaxError) @users = find(:all, :conditions => "#{weekday}sub = "t"")

My schedule.rb is as follows:

   every 1.day, :at => '5:30 am' do
    runner "User.mail_out"
  end

My User model is:

class User < ActiveRecord::Base

  acts_as_authentic

  def self.mail_out

    weekday = Date.today.strftime('%A').downcase

    @users = find(:all, :conditions => "#{weekday}sub = t")


    @users.each { |u| UserMailer.deliver_mail_out(u)}   


  end

end

My User_mailer is:

class UserMailer < ActionMailer::Base
    def mail_out(users)
    @recipients = { }
    users.each do |user|
      @recipients[user.email] = { :name => user.name }
    end


    from        "no-reply@dailytrailer.net"
    subject     "Check out the trailer of the day!"
    body        :user => user
  end

end

Migration:

  create_table "users", :force => true do |t|
    t.string   "email"
    t.date     "birthday"
    t.string   "gender"
    t.string   "zipcode"
    t.datetime "created_at"
    t.datetime "updated_at"
    t.string   "crypted_password"
    t.string   "password_salt"
    t.string   "persistence_token"
    t.string   "mondaysub",         :default => "f", :null => false
    t.string   "tuesdaysub",        :default => "f", :null => false
    t.string   开发者_C百科"wednesdaysub",      :default => "f", :null => false
    t.string   "thursdaysub",       :default => "f", :null => false
    t.string   "fridaysub",         :default => "f", :null => false
    t.string   "saturdaysub",       :default => "f", :null => false
    t.string   "sundaysub",         :default => "f", :null => false
  end

Change this line

@users = find(:all, :conditions => "#{weekday}sub = t")

to

@users = find(:all, :conditions => ["#{weekday}sub = t"])

The conditions key for find method takes either an array or hash as the value. If you are using array then the first element of the array straightaway converted into a "where" clause sql statement if you dont have any question marks in it and if you have then those question marks are substituted with the further elements of the array. For instance, changing the above to question mark notation will become

@users = find(:all, :conditions => ["#{weekday}sub = ?", 't'])

when 't' is a string

Its better to use question mark notation because it is more secure as you don't do string interpolation within your sql string which can be very harmful for you app data.

You can also write your query to hash conditions like so

@users = find(:all, :conditions => {"#{weekday}sub".to_sym => 't'})

Most people prefer this over array notation in other words consider this as the rails way of doing it.

And another point, looking at your UserMailer#mailout method, you don't need to do

@users.each { |u| UserMailer.deliver_mail_out(u)}

in your User.mailout method, you can just do

UserMailer.deliver_mail_out(@users)

because you are already extracting each user object from the users array in your UserMailer#mailout and adding them to the recipients.