drapergem / draper

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

issue with using draper outside of controller/view context within a rake task or (active)job #926

Closed timdiggins closed 2 months ago

timdiggins commented 1 year ago

If you use (by mistake? or on purpose?) draper outside of a view context (controller/view) then use some route helpers, this seems to work - indeed it does work wherever you test it, except for a few weird places - like in a Rake task or in a Sidekiq/Delayed Job runner.

But when you test those using an automated test or from rails c / rails r, they will work.

I've narrowed this down to Draper::ViewContext::BuildStrategy::Full#controller which (if called outside of a controller/view context) will:

1) infer a controller, and https://github.com/drapergem/draper/blob/89ec4b70f443bd6db526eddf73bddac52d6ca75b/lib/draper/view_context/build_strategy.rb#L39

2) manufacture a request (using a test request) but only if ActionController::TestRequest is defined. https://github.com/drapergem/draper/blob/89ec4b70f443bd6db526eddf73bddac52d6ca75b/lib/draper/view_context/build_strategy.rb#L41

I appreciate, using decorate outside of view context is unusual, but not beyond the bounds of possibility.

It turns out ActionController::TestRequest is possibly deprecated (maybe this is why it's not autoloaded in some cases?) https://github.com/rails/rails/blob/main/actionpack/lib/action_controller/test_case.rb#L32-L34

But in any case could one just replace it with ActionDispatch::TestRequest

I've done this and it seems to pass the test suite (except for the tests that specifically check the ActionController::TestRequest class). However I think it's not worth supporting EOLd rails in it. I'll send through a PR and can discuss there if there's any interest in merging this.


examples:

assume you have generated SomeModel, e.g.

rails g scaffold user email:string

and you have a rake task such as:

# lib/test_decoration.rake
task :test_request1 => :environment do
  p(defined:  defined?(ActionController::TestRequest))
end

task :test_request2 => :environment do
  p(defined:  defined?(ActionDispatch::TestRequest))
end

task :test_decorator => :environment do
  User.new.decorate.h.users_path
end

I've chosen development here, but works the same with test, production, staging (with typical rails defaults)

RAILS_ENV=development DISABLE_SPRING=1 rails r "p(defined: defined?(ActionController::TestRequest))"
{:defined=>"constant"}

RAILS_ENV=development DISABLE_SPRING=1 rails r "p(users_path: User.new.decorate.h.users_path)"
{:users_path=>"/users"}

RAILS_ENV=development DISABLE_SPRING=1 rake test_request1
{:defined=>nil}

#but: 
RAILS_ENV=development DISABLE_SPRING=1 rake test_request2
{:defined=>"constant"}

RAILS_ENV=development DISABLE_SPRING=1 rake test_decorator
NoMethodError: undefined method `host' for nil:NilClass
/path/gems/actionpack-6.1.7.4/lib/action_controller/metal/url_for.rb:32:in `url_options'
/path/gems/actionview-6.1.7.4/lib/action_view/routing_url_for.rb:123:in `url_options'
/path/gems/actionpack-6.1.7.4/lib/action_dispatch/routing/route_set.rb:262:in `call'
/path/gems/actionpack-6.1.7.4/lib/action_dispatch/routing/route_set.rb:213:in `call'
/path/gems/actionpack-6.1.7.4/lib/action_dispatch/routing/route_set.rb:326:in `block in define_url_helper'
/path/gems/draper-4.0.2/lib/draper/helper_proxy.rb:32:in `block in define_proxy'
/path/gems/draper-4.0.2/lib/draper/helper_proxy.rb:13:in `method_missing'
/app/lib/tasks/test_decoration.rake:8:in `block in <main>'
/app/Rakefile:12:in `block in execute_with_benchmark'
/app/Rakefile:12:in `execute_with_benchmark'

A similar situation will happen if you execute a ActiveJob such as:

# app/jobs/some_job.rb
class SomeJob < ApplicationJob
  queue_as :default

  def perform
    p(defined:  defined?(ActionController::TestRequest))
    User.new.decorate.h.users_path
  end
end

SomeJob.perform_later (raises error in sidekiq/background job runner)