CODEWITHSUNDEEP
ruby-on-railsrubyrakerake-task
Mike Wiesenhart
Mike Wiesenhart

Reputation: 316

Issue with custom rake task in Rails 4

I am trying to create a Rake task that will send an email when a listing has not been updated in a month, but can't seem to get it to work.

I think the issue could be that I am trying to pull out the listing_agent email, but I have listing_agent_id in there.

update_listing_mailer.rb:

class UpdateListingMailer < ActionMailer::Base
  default from: "Help <[email protected]>"

  def listing_update_agent(agent)
    @agent = agent
    @listing = listing
    mail to: "#{agent.email}", subject: "Your listing hasn't been updated in a month!"
  end
end

listing_update_agent.html.erb:

Hiya, <br><br>

The listings below have not been updated in a month! Is it still on the market? If so, please update. If not, please remove from the market.<br><br>

<strong>Address:</strong> <%= @listing.address %>
<strong>Last Updated:</strong> <%= @listing.updated_at %>
<br><br>

Thanks,<br>
Management

listing_needs_update.rake:

namespace :listings do
  desc "Sends an email to the listing agent when that listing hasn't been updated in a month"
  task listing_update_agent: :enviroment do
    Listing.all.each do |listing|
      if listing.updated_at == Date.today - 30
        UpdateListingMailer.listing_update_agent(listing.listing_agent_id).deliver_now
      end
    end
  end
end

error:

enter image description here

trace:

enter image description here

Upvotes: 0

Views: 70

Answers (1)

Dario Barrionuevo
Dario Barrionuevo

Reputation: 3257

I'll answer here to make it more clear.

Your task is named listing_update_agent, meaning that you should invoke it as: 

rake listings:listing_update_agent

Rather than:

rake listings:listing_needs_update

I can also think on some improvements on your code:

1) Task do things, so it'd be more semantic to call your task something like notify_outdated_listings

2) It'd be much more efficient to fetch the listings that need to be notified and work only with those, rather than asking for each record in the table, like you're doing in the task. You can achieve this by creating a scope in the Listing model like:

class Listing < ActiveRecord::Base
  scope :not_updated_in_last_month, -> { where('updated_at >= ?', 1.month.ago) } 
end

Then in your task:

namespace :listings do
  desc "Sends an email to the listing agent when that listing hasn't been updated in a month"
  task listing_update_agent: :enviroment do
    Listing.not_updated_in_last_month.each do |listing|
      UpdateListingMailer.listing_update_agent(listing.listing_agent_id).deliver_now
    end
  end
end

Can you tell the difference in performance this change could make?

Upvotes: 1

Related Questions