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

Already on GitHub? Sign in to your account

Wrap relation calls and scope calls in decorators #257

Merged
merged 3 commits into from Aug 23, 2012

Conversation

Projects
None yet
4 participants
Contributor

jhsu commented Aug 22, 2012

  • wrap model in decorator when scope is called on decorator
  • decorate named scope calls on decorators and enumerable proxies

for example,

ProductDecorator.where(name: "Something").first

will return an instance of ProductDecorator

also,

@product_decorator.some_relation

will return a decorated enumerable proxy

Related to issue #128 .

Wrap relation calls and scope calls in decorators
* wrap model in decorator when scope is called on decorator
* decorate named scope calls on decorators and enumerable proxies

This pull request passes (merged e4595d9 into beb14be).

This pull request passes (merged 0f02556 into beb14be).

@steveklabnik steveklabnik commented on an outdated diff Aug 22, 2012

lib/draper/base.rb
@@ -255,7 +255,12 @@ def method_missing(method, *args, &block)
if model.respond_to?(method)
self.class.send :define_method, method do |*args, &blokk|
- model.send method, *args, &blokk
+ result = model.send method, *args, &blokk
+ if result.class.name == "ActiveRecord::Relation"
@steveklabnik

steveklabnik Aug 22, 2012

Owner

this seems really awkward, can we at least use kind_of??

@steveklabnik steveklabnik commented on an outdated diff Aug 22, 2012

lib/draper/base.rb
@@ -269,10 +274,11 @@ def method_missing(method, *args, &block)
end
def self.method_missing(method, *args, &block)
- if method.to_s.match(/^find_((all_|last_)?by_|or_(initialize|create)_by_).*/)
- self.decorate(model_class.send(method, *args, &block), :context => args.dup.extract_options!)
+ model_result = model_class.send(method, *args, &block)
+ if model_result.is_a?(model_class) || model_result.class.name == 'ActiveRecord::Relation'

@steveklabnik steveklabnik commented on an outdated diff Aug 22, 2012

lib/draper/decorated_enumerable_proxy.rb
@@ -25,7 +25,24 @@ def find(ifnone_or_id = nil, &blk)
end
def method_missing (method, *args, &block)
- @wrapped_collection.send(method, *args, &block)
+ if @wrapped_collection.respond_to?(method)
+ self.class.send :define_method, method do |*args, &blokk|
+ scoped_result = @wrapped_collection.send(method, *args, &block)
+ if scoped_result.class.name == "ActiveRecord::Relation"
@steveklabnik

steveklabnik Aug 22, 2012

Owner

And here. I'll just stop commenting, grab them all.

Owner

steveklabnik commented Aug 22, 2012

Thanks for this! Other than those minor issues, looks good!

Contributor

jhsu commented Aug 23, 2012

Sweet! I'll get to it tonight. Do the tests look ok?

On Wednesday, August 22, 2012, Steve Klabnik wrote:

Thanks for this! Other than those minor issues, looks good!


Reply to this email directly or view it on GitHubhttps://github.com/jcasimir/draper/pull/257#issuecomment-7955778.

From,
Joseph Hsu
http://josephhsu.com | @jhsu

Owner

steveklabnik commented Aug 23, 2012

Yes. I'm a little concerned about defining AR::Relation in the test, but it's better than loading up and connecting to a DB and everything.

Contributor

jhsu commented Aug 23, 2012

Ah yeah.

Also, since ActiveRecord isn't currently a dependency of Draper, I didn't want to use #kind_of? to check class.

Owner

steveklabnik commented Aug 23, 2012

Roger. Makes sense. How about if defined?(ActiveRecord) && kind_of?(ActiveRecord::Relation) then?

Contributor

jhsu commented Aug 23, 2012

aha! that seems better.

Owner

steveklabnik commented Aug 23, 2012

Ideally, we wouldn't need to do this at all: kind_of? is a smell. But let's get it working first, and then fix things later :)

This pull request passes (merged c46b0fd into beb14be).

steveklabnik added a commit that referenced this pull request Aug 23, 2012

Merge pull request #257 from jhsu/draper-scope-wrap
Wrap relation calls and scope calls in decorators

@steveklabnik steveklabnik merged commit 99e45d3 into drapergem:master Aug 23, 2012

Owner

steveklabnik commented Aug 23, 2012

Woo! Thank you!

Contributor

kuraga commented Oct 15, 2012

Attention! PR has been reverted by 044a6e6.
P.S. Why GitHub doesn't help to check reverts?.. :-(
But it's cool when relations and scopes are decorated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment