drapergem / draper

Decorators/View-Models for Rails Applications
MIT License
5.22k stars 527 forks source link

`Undefined Method for Nil:NilClass` when using draper to send email from rake task #272

Closed jfeldstein closed 11 years ago

jfeldstein commented 12 years ago

My invoice decorator is such:

class InvoiceDecorator < ApplicationDecorator
  decorates :invoice

  def total
    h.number_as_currency model.total
  end
end

I'm generating and sending an email from a rake task (runs monthly), and want to use a decorator to render the email's partial.

Is there a hack that makes this possible? Currently, I get:

undefined method `number_with_delimiter' for nil:NilClass
steveklabnik commented 12 years ago

Hm. What version of Draper are you using? This should work. What's your rake task and mailer look like?

jfeldstein commented 12 years ago

Draper v0.12.0

Rake task

namespace :app do
  desc "Generate and deliver invoices to each Agency, for each Campaign they had running in the previous month."
  task "generate-invoices".to_sym => :environment do

    last_month = 1.month.ago

    Campaign.campaigns_to_invoice_for_month(last_month).each do |campaign|
      new_invoice = Invoice.create_for_campaign_for_month campaign, last_month
    end
  end
end

Mailer

class InvoiceMailer < ActionMailer::Base
  def deliver_to_agency( invoice )
    @invoice = InvoiceDecorator.decorate invoice

    mail ({
          :to => "#{@invoice.agency.name} <#{@invoice.agency.contact_email}>",
          :subject => "#{@invoice.month} invoice for '#{@invoice.campaign.name}'"
        })
  end
end
steveklabnik commented 12 years ago

Hmm, yeah, 0.12.0 is quite old. Can you at least try on 0.12.3 for me, and is there anything stopping you from upgrading to any of the newer releases? We've had 5 major ones since then, it may be fixed.

jfeldstein commented 12 years ago

Upgraded to 0.12.3, but h. is still nil.

Running into conflicts upgrading any further:

Bundler could not find compatible versions for gem "activesupport":
  In Gemfile:
    rails (~> 3.1.1) ruby depends on
      activesupport (= 3.1.1) ruby

    draper (>= 0.13.0) ruby depends on
      activesupport (3.2.0)
steveklabnik commented 12 years ago

Right. Okay. I will investigate; I bet I can backport the fix.

steveklabnik commented 12 years ago

Okay. Can you please try to use the v0.12-stable branch, please? Add this to your gemfile:

gem "draper", :github => "jcasimir/draper", :branch => "v0.12-stable"

And then bundle update.

I believe that the commit I've added (6887ade) should fix your problem. If it does, I'll cut a 0.12.4 for you. If not, back to the drawing board.

jfeldstein commented 12 years ago

Okay, this time I'm getting Stack level too deeps

jfeldstein commented 12 years ago

In case it's any help, this isn't just effecting decorators used within mailers, but specifically mailers that are called during rake tasks.

Rake task -> Mailer -> Decorated Model -> Failure.

steveklabnik commented 12 years ago

Oh no!

I'll try to check it out.

That may help, thanks.

jfeldstein commented 12 years ago

I've also discovered that that makes rails console crash with the following:

DEPRECATION WARNING: ActiveAdmin::Dashboard is deprecated and will be removed in the next version
/Users/Jordan/.rvm/gems/ruby-1.9.2-p290@sway/bundler/gems/draper-6887ade43c43/lib/draper/railtie.rb:51:in `block in <class:Railtie>': undefined method `set_current_view_context' for #<ApplicationController:0x00000104f6c9a8> (NoMethodError)
  from /Users/Jordan/.rvm/gems/ruby-1.9.2-p290@sway/gems/railties-3.1.8/lib/rails/railtie.rb:178:in `call'
  from /Users/Jordan/.rvm/gems/ruby-1.9.2-p290@sway/gems/railties-3.1.8/lib/rails/railtie.rb:178:in `block in load_console'
  from /Users/Jordan/.rvm/gems/ruby-1.9.2-p290@sway/gems/railties-3.1.8/lib/rails/railtie.rb:178:in `each'
  from /Users/Jordan/.rvm/gems/ruby-1.9.2-p290@sway/gems/railties-3.1.8/lib/rails/railtie.rb:178:in `load_console'
  from /Users/Jordan/.rvm/gems/ruby-1.9.2-p290@sway/gems/railties-3.1.8/lib/rails/engine.rb:407:in `block in load_console'
  from /Users/Jordan/.rvm/gems/ruby-1.9.2-p290@sway/gems/railties-3.1.8/lib/rails/application/railties.rb:8:in `each'
  from /Users/Jordan/.rvm/gems/ruby-1.9.2-p290@sway/gems/railties-3.1.8/lib/rails/application/railties.rb:8:in `all'
  from /Users/Jordan/.rvm/gems/ruby-1.9.2-p290@sway/gems/railties-3.1.8/lib/rails/engine.rb:407:in `load_console'
  from /Users/Jordan/.rvm/gems/ruby-1.9.2-p290@sway/gems/railties-3.1.8/lib/rails/application.rb:115:in `load_console'
  from /Users/Jordan/.rvm/gems/ruby-1.9.2-p290@sway/gems/railties-3.1.8/lib/rails/commands/console.rb:27:in `start'
  from /Users/Jordan/.rvm/gems/ruby-1.9.2-p290@sway/gems/railties-3.1.8/lib/rails/commands/console.rb:8:in `start'
  from /Users/Jordan/.rvm/gems/ruby-1.9.2-p290@sway/gems/railties-3.1.8/lib/rails/commands.rb:40:in `<top (required)>'
  from script/rails:6:in `require'
  from script/rails:6:in `<main>'
jfeldstein commented 12 years ago

Thanks, I appreciate it. I'm trying to use draper in the new version of BuyAds.com but ended up tripping over this glitch at the very last moment.

Any idea if its smart for us to update rails just to stay current? I'm not quite sure why bundle update leaves me with such an old version of everything anyway.

~J

(via phone) On Sep 8, 2012 2:07 PM, "Steve Klabnik" notifications@github.com wrote:

Oh no!

I'll try to check it out.

That may help, thanks.

— Reply to this email directly or view it on GitHubhttps://github.com/jcasimir/draper/issues/272#issuecomment-8397951.

steveklabnik commented 12 years ago

You should absolutely update Rails, 3.2 is the only version that's receiving bugfixes. 3.1, 3.0 only receive security stuff, and 2.x doesn't get updates at all.

The reason you're left with old versions is that we rely on new Rails features in new versions, and therefore have a dependency on Rails 3.2.

jfeldstein commented 12 years ago

Okay, just gona update and go from there. If you don't hear from me again, it's because the latest version fixed the issue.

Thanks for your help in the mean time. :^)

~ J

On Sun, Sep 9, 2012 at 12:50 PM, Steve Klabnik notifications@github.comwrote:

You should absolutely update Rails, 3.2 is the only version that's receiving bugfixes. 3.1, 3.0 only receive security stuff, and 2.x doesn't get updates at all.

The reason you're left with old versions is that we rely on new Rails features in new versions, and therefore have a dependency on Rails 3.2.

— Reply to this email directly or view it on GitHubhttps://github.com/jcasimir/draper/issues/272#issuecomment-8408490.

jfeldstein commented 12 years ago

I seem to have jumped the gun there. Upgrading is a pretty heft endeavor on it's own, so I can't get into it just yet. Back to rails 3.1 for me.

If you do end up fixing this in 0.12.3/.4 please let me know.

~ J

On Sun, Sep 9, 2012 at 12:55 PM, Jordan Feldstein jfeldstein@gmail.comwrote:

Okay, just gona update and go from there. If you don't hear from me again, it's because the latest version fixed the issue.

Thanks for your help in the mean time. :^)

~ J

On Sun, Sep 9, 2012 at 12:50 PM, Steve Klabnik notifications@github.comwrote:

You should absolutely update Rails, 3.2 is the only version that's receiving bugfixes. 3.1, 3.0 only receive security stuff, and 2.x doesn't get updates at all.

The reason you're left with old versions is that we rely on new Rails features in new versions, and therefore have a dependency on Rails 3.2.

— Reply to this email directly or view it on GitHubhttps://github.com/jcasimir/draper/issues/272#issuecomment-8408490.

steveklabnik commented 12 years ago

If you don't hear from me again, it's because the latest version fixed the issue.

If that's true, would you mind coming back here and closing it, please? And I'll be checking this out sometime tomorrow; hopefully I can find a fix without too much effort. I'm still not 100% sure where the bug is.

jfeldstein commented 12 years ago

Just to clarify, upgrading ended up being more trouble than I wanted to get into just yet, so I'm still in this with you.

If you need me to test anything else, just post and I'll get the emails.

If we do end up upgrading before there's a fix, or I get a fix that works, I'll touch back here and close the issue.

~ J

On Sun, Sep 9, 2012 at 1:10 PM, Steve Klabnik notifications@github.comwrote:

If you don't hear from me again, it's because the latest version fixed the issue.

If that's true, would you mind coming back here and closing it, please? And I'll be checking this out sometime tomorrow; hopefully I can find a fix without too much effort. I'm still not 100% sure where the bug is.

— Reply to this email directly or view it on GitHubhttps://github.com/jcasimir/draper/issues/272#issuecomment-8408730.

steveklabnik commented 12 years ago

Roger. I'll try to dig into this and fix it tomorrow.

jfeldstein commented 12 years ago

Thanks. Let me know when I can try a new version.

smlsml commented 12 years ago

Just giving a +1 for legacy rails / ruby (ree 1.8.7) support on fixes like this. I'm sure we're not the only ones with huge legacy apps that won't be updated but have to be maintained ;)

houen commented 12 years ago

It seems that this error is related to when one decorator instantiates another, when called by a mailer. At least in my code, this is the only time it manifests

houen commented 12 years ago

Actually, it manifests when a decorator is used in an interpolation argument, and tries to use h.whatever. Here is a stack trace:

NoMethodError in Badmin::CampaignAdminMembershipsController#create _undefined method `currentregion' for #<#Class:0x007fd87d302ed0:0x007fd87d319040>

app/decorators/application_decorator.rb:41:in at' app/decorators/campaign_decorator.rb:21:ingeneral_name' app/decorators/campaign_decorator.rb:17:in name' app/domain/translations/interpolations/campaign_interpolations.rb:9:inblock in get_campaign_interpolations' i18n (0.6.1) lib/i18n/interpolate/ruby.rb:25:in call' i18n (0.6.1) lib/i18n/interpolate/ruby.rb:25:inblock in interpolate_hash' i18n (0.6.1) lib/i18n/interpolate/ruby.rb:19:in gsub' i18n (0.6.1) lib/i18n/interpolate/ruby.rb:19:ininterpolate_hash' i18n (0.6.1) lib/i18n/interpolate/ruby.rb:15:in interpolate' i18n (0.6.1) lib/i18n/backend/base.rb:144:ininterpolate' i18n (0.6.1) lib/i18n/backend/base.rb:41:in translate' i18n (0.6.1) lib/i18n/backend/chain.rb:46:inblock (2 levels) in translate' i18n (0.6.1) lib/i18n/backend/chain.rb:44:in catch' i18n (0.6.1) lib/i18n/backend/chain.rb:44:inblock in translate' i18n (0.6.1) lib/i18n/backend/chain.rb:43:in each' i18n (0.6.1) lib/i18n/backend/chain.rb:43:intranslate' i18n (0.6.1) lib/i18n.rb:156:in block in translate' i18n (0.6.1) lib/i18n.rb:152:incatch' i18n (0.6.1) lib/i18n.rb:152:in translate' app/classes/ai18n.rb:29:intranslate' app/classes/ai18n.rb:13:in t2' app/mailers/mailers/notification_mailer.rb:24:innotification_mail' actionpack (3.1.3) lib/abstract_controller/base.rb:167:in process_action' actionpack (3.1.3) lib/abstract_controller/base.rb:121:inprocess' actionpack (3.1.3) lib/abstract_controller/rendering.rb:45:in process' actionmailer (3.1.3) lib/action_mailer/old_api.rb:65:inprocess' actionmailer (3.1.3) lib/action_mailer/base.rb:474:in process' actionmailer (3.1.3) lib/action_mailer/base.rb:469:ininitialize' actionmailer (3.1.3) lib/action_mailer/base.rb:456:in new' actionmailer (3.1.3) lib/action_mailer/base.rb:456:inmethod_missing' app/domain/notifications/sender.rb:280:in send_mail_to_user' app/domain/notifications/sender.rb:272:incheck_filter_send_mail_and_add_to_filter' app/domain/notifications/sender.rb:266:in send_to_campaign_membership' app/domain/notifications/sender.rb:210:insend_welcome_project_admin' app/domain/notifications/queuer.rb:136:in `queue_welcome_project_admin'

@steveklabnik Any idea what is causing this? I am available to get my hands dirty looking for a fix, if you can point me in the right direction. This happens for me also in 3.2 (I managed to get just enough working to test it, but, like @smlsml and @jfeldstein my app is too legacy to upgrade quickly...

Sorry there are no tests, I'm new to this :-)

houen commented 12 years ago

Okay, I found an ugly fix that works for me:

I have a custom "current_region" helper method, that Draper couldn't find before, so I check for the presence of the viewcontext, and that it responds to that method, and init a new if not. Ugly, but functional. @steveklabnik hope it helps, at least a little :D

@smlsml and @jfeldstein let me know if this works for you too (I think we have kinda the same problem)

unless Draper::ViewContext.current && Draper::ViewContext.current.respond_to?(:current_region) Draper::ViewContext.current = ApplicationController.new.view_context end

steveklabnik commented 12 years ago

@houen thanks. Hmmmmm. I'd like to pull this up into mainline draper...

houen commented 12 years ago

Awesome - glad I could help! :-) I guess checking for a common helper method would do the trick

steveklabnik commented 12 years ago

Well, see, the thing is, we do what you do, basically: https://github.com/jcasimir/draper/blob/v0.12-stable/lib/draper/view_context.rb

That's why I'm so confused...

houen commented 12 years ago

Yes, that piece of code helped me find my solution - but the problem is that in this case the Thread.current[:current_view_context] is actually not empty, just doesn't respond to the helper method. It feels like it's being wrapped in something. I'll post an inspect of it in 2 secs , to see if that might help.

steveklabnik commented 12 years ago

:heart:

houen commented 12 years ago

Ok, so unfortunately I cant post the entire thing, but here's the start (at is my helper method, and this code works perfectly in any "regular" view):

unless Draper::ViewContext.current && Draper::ViewContext.current.respond_to?(:at)

Draper::ViewContext.current = ApplicationController.new.view_context

  raise Draper::ViewContext.current.inspect
end

<#Class:0x007fd873da6350:0x007fd86827ef78 @_config={}, @view_renderer=#<ActionView::Renderer:0x007fd8683e28b0 @lookup_context=#<ActionView::LookupContext:0x007fd86c83dc80 @details_key=nil, @details={:handlers=>[:erb, :builder, :arb, :coffee, :haml, :rabl], :formats=>[:html, :text, :js, :css, :ics, :csv, :xml, :rss, :atom, :yaml, :multipart_form, :url_encoded_form, :json, :mobile], :locale=>[:"se-se"]}, @skip_default_locale=true, @frozen_formats=false, @cache=true, @prefixes=["mailers/notification_mailer", "mailers/better_now_mailer"], @view_paths=[/Users/houen/code/betternow/BNRailsWebsite/app/views, /Users/houen/.rvm/gems/ruby-1.9.3-p125@betternow/gems/activeadmin-0.4.2/app/views, /Users/houen/.rvm/gems/ruby-1.9.3-p125@betternow/gems/kaminari-0.14.1/app/views, /Users/houen/.rvm/gems/ruby-1.9.3-p125@betternow/gems/devise-2.1.2/app/views]>>, @_routes=nil, @_message=#<Mail::Message:70283756061960, Multipart: false, Headers: >, @mailer_name="mailers/notification_mailer", @_assigns={"_routes"=>nil, "_message"=>#<Mail::Message:70283756061960, Multipart: false, Headers: >, "mailer_name"=>"mailers/notification_mailer"}, @_controller=#<Mailers::NotificationMailer:0x007fd86ca08b50 @_routes=nil, @_action_has_layout=true, @_view_context_class=nil, @_message=#<Mail::Message:70283756061960, Multipart: false, Headers: >, @_prefixes=["mailers/notification_mailer", "mailers/better_now_mailer"], @_lookup_context=#<ActionView::LookupContext:0x007fd86c83dc80 @details_key=nil, @details={:handlers=>[:erb, :builder, :arb, :coffee, :haml, :rabl], :formats=>[:html, :text, :js, :css, :ics, :csv, :xml, :rss, :atom, :yaml, :multipart_form, :url_encoded_form, :json, :mobile], :locale=>[:"se-se"]}, @skip_default_locale=true, @frozen_formats=false, @cache=true,

houen commented 12 years ago

Hmm - of course not the code I posted, I mean the h.at method works perfectly :P

houen commented 12 years ago

Crap - ok so the fix isnt complete. I still get this when trying to use a link in an interpolation variable: Class#notification_mail failed with ActionView::Template::Error: undefined method `parameters' for nil:NilClass - 1 failed attempts

Any ideas?

steveklabnik commented 11 years ago

Can you please try again? I've added a54e3f3.

danshultz commented 11 years ago

@houen I've deployed the the code from a54e3f3 and the mailers seem to be working as expected - can you confirm this on your end?

steveklabnik commented 11 years ago

Since I haven't heard about this in a month, and @danshultz seems to think it's working, I'm giving it a close. Let's get a release with this code out soon.

@houen if you still see a problem and can provide a reproduction, please let me know and I'll re-open.

houen commented 11 years ago

@danshultz Sadly my code is very outdated (I am on 0.10.0@89126e48d64a3211526187021802264a0eb069c4) with a very long upgrade path. I have hotfixes in place now that make it work for me, and no time to experiment with it. Sorry. When we do get to the upgrades, I'll check it out right away!