Merge branch 'master' into improve-delegate_all-performance

Gemfile

@@ -15,6 +15,11 @@ platforms :jruby do
   gem "activerecord-jdbcsqlite3-adapter"
 end
 
+if RUBY_VERSION >= "2.6.0"
+  gem "turbo-rails"
+  gem "redis", "~> 4.0"
+end
+
 if RUBY_VERSION >= "2.5.0"
   gem "rails", "~> 6.0"
   gem 'webrick'

README.md

@@ -131,6 +131,17 @@ class ArticleDecorator < Draper::Decorator
 end
 ```
 
+
+To decorate a model in a namespace e.g. `Admin::Catalogue` place the decorator under the
+directory `app/decorators/admin` in the same way you would with views and models.
+
+```ruby
+# app/decorators/admin/catalogue_decorator.rb
+class Admin::CatalogueDecorator < Draper::Decorator
+# ...
+end
+```
+
 ### Generators
 
 To create an `ApplicationDecorator` that all generated decorators inherit from, run...
@@ -155,6 +166,15 @@ rails generate decorator Article
 
 ...to create the `ArticleDecorator`.
 
+If you don't want Rails to generate decorator files when generating a new controller,
+you can add the following configuration to your `config/application.rb` file:
+
+```ruby
+config.generators do |g|
+  g.decorator false
+end
+```
+
 ### Accessing Helpers
 
 Normal Rails helpers are still useful for lots of tasks. Both Rails' provided
@@ -652,9 +672,8 @@ you can include this module manually.
 [Active Job](http://edgeguides.rubyonrails.org/active_job_basics.html) allows you to pass ActiveRecord
 objects to background tasks directly and performs the necessary serialization and deserialization. In
 order to do this, arguments to a background job must implement [Global ID](https://github.com/rails/globalid).
-Decorated objects implement Global ID by delegating to the object they are decorating. This means
-you can pass decorated objects to background jobs, however, the object won't be decorated when it is
-deserialized.
+Decorators implement Global ID.
+This means you can pass decorated objects to background jobs, and get them just as decorated when deserialized.
 
 ## Contributors
 

draper.gemspec

@@ -5,7 +5,7 @@ Gem::Specification.new do |s|
   s.version     = Draper::VERSION
   s.authors     = ["Jeff Casimir", "Steve Klabnik"]
   s.email       = ["[email protected]", "[email protected]"]
-  s.homepage    = "http://github.com/drapergem/draper"
+  s.homepage    = "https://github.com/drapergem/draper"
   s.summary     = "View Models for Rails"
   s.description = "Draper adds an object-oriented layer of presentation logic to your Rails apps."
   s.license     = "MIT"

lib/draper/compatibility/broadcastable.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module Draper
+  module Compatibility
+    # It would look consistent to use decorated objects inside templates broadcasted with
+    # Turbo::Broadcastable.
+    #
+    # This compatibility patch fixes the issue by overriding the original defaults to decorate the
+    # object, that's passed to the partial in a local variable.
+    module Broadcastable
+      private
+
+      def broadcast_rendering_with_defaults(options)
+        return super unless decorator_class?
+
+        # Add the decorated current instance into the locals (see original method for details).
+        options[:locals] =
+          (options[:locals] || {}).reverse_merge!(model_name.element.to_sym => decorate)
+
+        super
+      end
+    end
+  end
+end

lib/draper/compatibility/global_id.rb

@@ -5,17 +5,21 @@ module Compatibility
     # and deserialization. In order to do this, arguments to a background job must implement
     # [Global ID](https://github.com/rails/globalid).
     #
-    # This compatibility patch implements Global ID for decorated objects by delegating to the object
-    # that is decorated. This means you can pass decorated objects to background jobs, but 
-    # the object won't be decorated when it is deserialized. This patch is meant as an intermediate
-    # fix until we can find a way to deserialize the decorated object correctly.
+    # This compatibility patch implements Global ID for decorated objects by defining `.find(id)`
+    # class method that uses the original one and decorates the result.
+    # This means you can pass decorated objects to background jobs and they will be decorated when
+    # deserialized.
     module GlobalID
       extend ActiveSupport::Concern
 
       included do
         include ::GlobalID::Identification
+      end
 
-        delegate :to_global_id, :to_signed_global_id, to: :object
+      class_methods do
+        def find(*args)
+          object_class.find(*args).decorate
+        end
       end
     end
   end

lib/draper/decoratable.rb

@@ -1,4 +1,5 @@
 require 'draper/decoratable/equality'
+require 'draper/compatibility/broadcastable'
 
 module Draper
   # Provides shortcuts to decorate objects directly, so you can do
@@ -11,6 +12,10 @@ module Decoratable
     extend ActiveSupport::Concern
     include Draper::Decoratable::Equality
 
+    included do
+      prepend Draper::Compatibility::Broadcastable if defined? Turbo::Broadcastable
+    end
+
     # Decorates the object using the inferred {#decorator_class}.
     # @param [Hash] options
     #   see {Decorator#initialize}
@@ -87,8 +92,6 @@ def decorator_class(called_on = self)
       def ===(other)
         super || (other.is_a?(Draper::Decorator) && super(other.object))
       end
-
     end
-
   end
 end

lib/draper/query_methods.rb

@@ -10,7 +10,7 @@ module QueryMethods
     end
 
     def respond_to_missing?(method, include_private = false)
-      strategy.allowed?(method) || super
+      object.respond_to?(method) && strategy.allowed?(method) || super
     end
 
     private

lib/draper/view_context/build_strategy.rb

@@ -38,17 +38,9 @@ def call
         def controller
           Draper::ViewContext.controller ||= Draper.default_controller.new
           Draper::ViewContext.controller.tap do |controller|
-            controller.request ||= new_test_request controller if defined?(ActionController::TestRequest)
+            controller.request ||= ActionDispatch::TestRequest.create
           end
         end
-
-        def new_test_request(controller)
-          is_above_rails_5_1 ? ActionController::TestRequest.create(controller) : ActionController::TestRequest.create
-        end
-
-        def is_above_rails_5_1
-          ActionController::TestRequest.method(:create).parameters.first == [:req, :controller_class]
-        end
       end
     end
   end

spec/draper/query_methods_spec.rb

@@ -53,14 +53,24 @@ module Draper
       context 'when strategy allows collection to call the method' do
         before do
           allow(fake_strategy).to receive(:allowed?).with(:some_query_method).and_return(true)
+          allow(collection).to receive(:respond_to?).with(:some_query_method).and_return(true)
         end
 
         it { is_expected.to eq(true) }
+
+        context 'and collection does not implement the method' do
+          before do
+            allow(collection).to receive(:respond_to?).with(:some_query_method).and_return(false)
+          end
+
+          it { is_expected.to eq(false) }
+        end
       end
 
       context 'when strategy does not allow collection to call the method' do
         before do
           allow(fake_strategy).to receive(:allowed?).with(:some_query_method).and_return(false)
+          allow(collection).to receive(:respond_to?).with(:some_query_method).and_return(true)
         end
 
         it { is_expected.to eq(false) }

spec/draper/view_context/build_strategy_spec.rb

@@ -37,31 +37,14 @@ module Draper
 
         expect(controller.request).to be_nil
         strategy.call
-        expect(controller.request).to be_an ActionController::TestRequest
+        expect(controller.request).to be_an ActionDispatch::TestRequest
         expect(controller.params).to be_empty
 
         # sanity checks
         expect(controller.view_context.request).to be controller.request
         expect(controller.view_context.params).to be controller.params
       end
 
-      it "compatible with rails 5.1 change on ActionController::TestRequest.create method" do
-        ActionController::TestRequest.class_eval do
-          if ActionController::TestRequest.method(:create).parameters.first == []
-            def create controller_class
-              create
-            end
-          end
-        end
-        controller = Class.new(ActionController::Base).new
-        allow(ViewContext).to receive_messages controller: controller
-        strategy = ViewContext::BuildStrategy::Full.new
-
-        expect(controller.request).to be_nil
-        strategy.call
-        expect(controller.request).to be_an ActionController::TestRequest
-      end
-
       it "adds methods to the view context from the constructor block" do
         allow(ViewContext).to receive(:controller).and_return(fake_controller)
         strategy = ViewContext::BuildStrategy::Full.new do

spec/dummy/app/models/post.rb

@@ -1,3 +1,7 @@
+require 'turbo/broadcastable' if defined? Turbo::Broadcastable # HACK: looks weird, but works
+
 class Post < ApplicationRecord
   # attr_accessible :title, :body
+
+  broadcasts if defined? Turbo::Broadcastable
 end

spec/dummy/config/application.rb

@@ -9,6 +9,7 @@ def attempt_require(file)
 require 'draper'
 attempt_require 'mongoid'
 attempt_require 'devise'
+attempt_require 'turbo-rails'
 require 'active_model_serializers'
 
 module Dummy

spec/dummy/config/cable.yml

@@ -0,0 +1,8 @@
+# production:
+#   url: redis://redis.example.com:6379
+
+local: &local
+  url: redis://localhost:6379
+
+development: *local
+test:        *local

spec/dummy/spec/decorators/post_decorator_spec.rb

@@ -61,4 +61,13 @@
   it "uses a test view context from BaseController" do
     expect(Draper::ViewContext.current.controller).to be_an BaseController
   end
+
+  describe 'Global ID' do
+    it { expect(GlobalID::Locator.locate decorator.to_gid).to eq decorator }
+    it { expect(GlobalID::Locator.locate decorator.to_gid).to be_decorated }
+    it { expect(GlobalID::Locator.locate object.to_gid).not_to be_decorated }
+    it { expect(GlobalID::Locator.locate_signed decorator.to_sgid).to eq decorator }
+    it { expect(GlobalID::Locator.locate_signed decorator.to_sgid).to be_decorated }
+    it { expect(GlobalID::Locator.locate_signed object.to_sgid).not_to be_decorated }
+  end
 end

spec/dummy/spec/jobs/publish_post_job_spec.rb

@@ -4,6 +4,8 @@
   subject(:job) { described_class.perform_later(post) }
 
   it 'queues the job' do
-    expect { job }.to have_enqueued_job(described_class).with(post.object)
+    expect { job }.to have_enqueued_job(described_class).with { |post|
+      expect(post).to be_decorated
+    }
   end
 end

spec/dummy/spec/models/post_spec.rb

@@ -6,10 +6,16 @@
 
   it { should be_a ApplicationRecord }
 
-  describe '#to_global_id' do
-    let(:post) { Post.create }
-    subject { post.to_global_id }
+  describe 'broadcasts' do
+    let(:modification) { described_class.create! }
 
-    it { is_expected.to eq post.decorate.to_global_id }
-  end
+    it 'passes a decorated object for rendering' do
+      expect do
+        modification
+      end.to have_enqueued_job(Turbo::Streams::ActionBroadcastJob).with { |stream, action:, target:, **rendering|
+        expect(rendering[:locals]).to include :post
+        expect(rendering[:locals][:post]).to be_decorated
+      }
+    end
+  end if defined? Turbo::Broadcastable
 end