Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
timdiggins opened this issue Aug 1, 2023 · 0 comments · May be fixed by #927
Open

Comments

@timdiggins
Copy link

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

    Draper::ViewContext.controller ||= Draper.default_controller.new

  2. manufacture a request (using a test request) but only if ActionController::TestRequest is defined.

    controller.request ||= new_test_request controller if defined?(ActionController::TestRequest)

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)

timdiggins added a commit to timdiggins/draper that referenced this issue Aug 1, 2023
fixes drapergem#926

however the current PR also drops support for rails < 6.0
open to discussion around this if there's any engagement on this PR.
@timdiggins timdiggins linked a pull request Aug 1, 2023 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant