Don't do runtime delegation, delegate explicitly (#789)

* Don't do runtime delegation, delegate explicitly

If someone accidentally calls `super` instead of `object` inside a decorator,
all hell breaks loose.

```rb
class Post < ActiveRecord::Base
  def title
    "title"
  end
end

class PostDecorator < Draper::Decorator
  delegate_all

  def title
    super and "overriden title"
  end
end

post = PostDecorator.new(Post.new)
post.title #=> "overriden title"
post.title #=> "title"
```

This patch makes both `super` and `object` work. I'm not sure why
runtime delegation was added in the first place, but it seems very wrong
to add methods on the fly.

* Add documentation and more tests.

* Consistent line spacing in test.
This commit is contained in:
Cliff Braton 2017-04-03 11:24:01 -05:00 committed by GitHub
parent 05b107b131
commit 31fa4793db
3 changed files with 45 additions and 9 deletions

View File

@ -496,7 +496,10 @@ end
When your decorator calls `delegate_all`, any method called on the decorator not
defined in the decorator itself will be delegated to the decorated object. This
is a very permissive interface.
includes calling `super` from within the decorator. A call to `super` from within
the decorator will first try to call the method on the parent decorator class. If
the method does not exist on the parent decorator class, it will then try to call
the method on the decorated `object`. This is a very permissive interface.
If you want to strictly control which methods are called within views, you can
choose to only delegate certain methods from the decorator to the source model:

View File

@ -2,12 +2,14 @@ module Draper
module AutomaticDelegation
extend ActiveSupport::Concern
# Delegates missing instance methods to the source object.
# Delegates missing instance methods to the source object. Note: This will delegate `super`
# method calls to `object` as well. Calling `super` will first try to call the method on
# the parent decorator class. If no method exists on the parent class, it will then try
# to call the method on the `object`.
def method_missing(method, *args, &block)
return super unless delegatable?(method)
self.singleton_class.delegate method
send(method, *args, &block)
object.send(method, *args, &block)
end
# Checks if the decorator responds to an instance method, or is able to

View File

@ -572,12 +572,43 @@ module Draper
expect(decorator.hello_world).to be :delegated
end
it "adds delegated methods to the decorator when they are used" do
decorator = Decorator.new(double(hello_world: :delegated))
it 'delegates `super` to parent class first' do
parent_decorator_class = Class.new(Decorator) do
def hello_world
"parent#hello_world"
end
end
expect(decorator.methods).not_to include :hello_world
decorator.hello_world
expect(decorator.methods).to include :hello_world
child_decorator_class = Class.new(parent_decorator_class) do
def hello_world
super
end
end
decorator = child_decorator_class.new(double(hello_world: 'object#hello_world'))
expect(decorator.hello_world).to eq 'parent#hello_world'
end
it 'delegates `super` to object if method does not exist on parent class' do
decorator_class = Class.new(Decorator) do
def hello_world
super
end
end
decorator = decorator_class.new(double(hello_world: 'object#hello_world'))
expect(decorator.hello_world).to eq 'object#hello_world'
end
it 'raises `NoMethodError` when `super` is called on for method that does not exist' do
decorator_class = Class.new(Decorator) do
def hello_world
super
end
end
decorator = decorator_class.new(double)
expect{decorator.hello_world}.to raise_error NoMethodError
end
it "allows decorator to decorate different classes of objects" do