Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Extract decorated associations #332

Merged
merged 5 commits into from

2 participants

@haines
Collaborator

Continuing on the refactoring warpath, the next target is Decorator#decorates_association.

I've extracted it into its own class, and simplified the logic. It was previously using reflect_on_association when available, which was unnecessary since we had already fetched the associated object and can thus infer the decorator directly.

I'm not 100% happy with

def collection?
  source.respond_to?(:first)
end

but that was how it worked before, so it doesn't break anything. I'm tempted to file this under "good enough for now", unless there are any ideas to improve it.

haines added some commits
@haines haines Remove deprecated lib/rspec_integration 2cb56c2
@haines haines Move performance into spec 7fd06ea
@haines haines Extract DecoratedAssociation f0a1be3
@haines haines Simplify association decoration
We don't actually need to reflect on the association because we have
already fetched the associated object - we can just infer the
appropriate decorator from it directly.
609123f
@haines haines Remove redundant spec
The `:polymorphic` option is now redundant, because the decorator
is always inferred from the associated object.
966d9cc
@steveklabnik
Owner

Awesome! :heart:

I'm tempted to file this under "good enough for now"

Me too, especially since that's how it worked before. If we find edge cases we can change it.

@steveklabnik steveklabnik merged commit a9dedcd into drapergem:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 8, 2012
  1. @haines
  2. @haines

    Move performance into spec

    haines authored
  3. @haines

    Extract DecoratedAssociation

    haines authored
  4. @haines

    Simplify association decoration

    haines authored
    We don't actually need to reflect on the association because we have
    already fetched the associated object - we can just infer the
    appropriate decorator from it directly.
  5. @haines

    Remove redundant spec

    haines authored
    The `:polymorphic` option is now redundant, because the decorator
    is always inferred from the associated object.
This page is out of date. Refresh to see the latest.
View
1  lib/draper.rb
@@ -8,6 +8,7 @@
require 'draper/helper_proxy'
require 'draper/lazy_helpers'
require 'draper/decoratable'
+require 'draper/decorated_association'
require 'draper/security'
require 'draper/helper_support'
require 'draper/view_context'
View
55 lib/draper/decorated_association.rb
@@ -0,0 +1,55 @@
+module Draper
+ class DecoratedAssociation
+
+ attr_reader :source, :association, :options
+
+ def initialize(source, association, options)
+ @source = source
+ @association = association
+ @options = options
+ end
+
+ def call
+ return undecorated if undecorated.nil? || undecorated == []
+ decorate
+ end
+
+ private
+
+ def undecorated
+ @undecorated ||= begin
+ associated = source.send(association)
+ associated = associated.send(options[:scope]) if options[:scope]
+ associated
+ end
+ end
+
+ def decorate
+ @decorated ||= decorator_class.send(decorate_method, undecorated, options)
+ end
+
+ def decorate_method
+ if collection? && decorator_class.respond_to?(:decorate_collection)
+ :decorate_collection
+ else
+ :decorate
+ end
+ end
+
+ def collection?
+ undecorated.respond_to?(:first)
+ end
+
+ def decorator_class
+ return options[:with] if options[:with]
+
+ if collection?
+ options[:with] = :infer
+ Draper::CollectionDecorator
+ else
+ undecorated.decorator_class
+ end
+ end
+
+ end
+end
View
56 lib/draper/decorator.rb
@@ -50,49 +50,25 @@ def self.has_finders(options = {})
# Typically called within a decorator definition, this method causes
# the assocation to be decorated when it is retrieved.
#
- # @param [Symbol] association_symbol name of association to decorate, like `:products`
- # @option options [Hash] :with The decorator to decorate the association with
- # :scope The scope to apply to the association
- def self.decorates_association(association_symbol, options = {})
- define_method(association_symbol) do
- orig_association = source.send(association_symbol)
-
- return orig_association if orig_association.nil? || orig_association == []
- return decorated_associations[association_symbol] if decorated_associations[association_symbol]
-
- orig_association = orig_association.send(options[:scope]) if options[:scope]
-
- return options[:with].decorate(orig_association) if options[:with]
-
- collection = orig_association.respond_to?(:first)
-
- klass = if options[:polymorphic]
- orig_association.class
- elsif association_reflection = find_association_reflection(association_symbol)
- association_reflection.klass
- elsif collection
- orig_association.first.class
- else
- orig_association.class
- end
-
- decorator_class = "#{klass}Decorator".constantize
-
- if collection
- decorated_associations[association_symbol] = decorator_class.decorate_collection(orig_association, options)
- else
- decorated_associations[association_symbol] = decorator_class.decorate(orig_association, options)
- end
+ # @param [Symbol] association name of association to decorate, like `:products`
+ # @option options [Class] :with the decorator to apply to the association
+ # @option options [Symbol] :scope a scope to apply when fetching the association
+ def self.decorates_association(association, options = {})
+ define_method(association) do
+ decorated_associations[association] ||= Draper::DecoratedAssociation.new(source, association, options)
+ decorated_associations[association].call
end
end
# A convenience method for decorating multiple associations. Calls
# decorates_association on each of the given symbols.
#
- # @param [Symbols*] association_symbols name of associations to decorate
- def self.decorates_associations(*association_symbols)
- options = association_symbols.extract_options!
- association_symbols.each{ |sym| decorates_association(sym, options) }
+ # @param [Symbols*] associations name of associations to decorate
+ def self.decorates_associations(*associations)
+ options = associations.extract_options!
+ associations.each do |association|
+ decorates_association(association, options)
+ end
end
# Specifies a black list of methods which may *not* be proxied to
@@ -227,12 +203,6 @@ def handle_multiple_decoration
end
end
- def find_association_reflection(association)
- if source.class.respond_to?(:reflect_on_association)
- source.class.reflect_on_association(association)
- end
- end
-
def decorated_associations
@decorated_associations ||= {}
end
View
2  lib/draper/rspec_integration.rb
@@ -1,2 +0,0 @@
-warn 'DEPRECATION WARNING -- use `require "draper/test/rspec_integration"` instead of `require "draper/rspec_integration"`'
-require 'draper/test/rspec_integration'
View
130 spec/draper/decorated_association_spec.rb
@@ -0,0 +1,130 @@
+require 'spec_helper'
+
+describe Draper::DecoratedAssociation do
+ let(:decorated_association) { Draper::DecoratedAssociation.new(source, association, options) }
+ let(:source) { Product.new }
+ let(:options) { {} }
+
+ describe "#call" do
+ subject { decorated_association.call }
+
+ context "for an ActiveModel collection association" do
+ let(:association) { :similar_products }
+
+ context "when the association is not empty" do
+ it "decorates the collection" do
+ subject.should be_a Draper::CollectionDecorator
+ end
+
+ it "infers the decorator" do
+ subject.decorator_class.should be :infer
+ end
+ end
+
+ context "when the association is empty" do
+ it "doesn't decorate the collection" do
+ source.stub(:similar_products).and_return([])
+ subject.should_not be_a Draper::CollectionDecorator
+ subject.should be_empty
+ end
+ end
+ end
+
+ context "for non-ActiveModel collection associations" do
+ let(:association) { :poro_similar_products }
+
+ context "when the association is not empty" do
+ it "decorates the collection" do
+ subject.should be_a Draper::CollectionDecorator
+ end
+
+ it "infers the decorator" do
+ subject.decorator_class.should be :infer
+ end
+ end
+
+ context "when the association is empty" do
+ it "doesn't decorate the collection" do
+ source.stub(:poro_similar_products).and_return([])
+ subject.should_not be_a Draper::CollectionDecorator
+ subject.should be_empty
+ end
+ end
+ end
+
+ context "for an ActiveModel singular association" do
+ let(:association) { :previous_version }
+
+ context "when the association is present" do
+ it "decorates the association" do
+ subject.should be_decorated_with ProductDecorator
+ end
+ end
+
+ context "when the association is absent" do
+ it "doesn't decorate the association" do
+ source.stub(:previous_version).and_return(nil)
+ subject.should be_nil
+ end
+ end
+ end
+
+ context "for a non-ActiveModel singular association" do
+ let(:association) { :poro_previous_version }
+
+ context "when the association is present" do
+ it "decorates the association" do
+ subject.should be_decorated_with ProductDecorator
+ end
+ end
+
+ context "when the association is absent" do
+ it "doesn't decorate the association" do
+ source.stub(:poro_previous_version).and_return(nil)
+ subject.should be_nil
+ end
+ end
+ end
+
+ context "when a decorator is specified" do
+ let(:options) { {with: SpecificProductDecorator} }
+
+ context "for a singular association" do
+ let(:association) { :previous_version }
+
+ it "decorates with the specified decorator" do
+ subject.should be_decorated_with SpecificProductDecorator
+ end
+ end
+
+ context "for a collection association" do
+ let(:association) { :similar_products}
+
+ it "decorates with a collection of the specifed decorators" do
+ subject.should be_a Draper::CollectionDecorator
+ subject.decorator_class.should be SpecificProductDecorator
+ end
+ end
+ end
+
+ context "when a collection decorator is specified" do
+ let(:association) { :similar_products }
+ let(:options) { {with: ProductsDecorator} }
+
+ it "decorates with the specified decorator" do
+ subject.should be_a ProductsDecorator
+ end
+ end
+
+ context "with a scope" do
+ let(:association) { :thing }
+ let(:options) { {scope: :foo} }
+
+ it "applies the scope before decoration" do
+ scoped = [SomeThing.new]
+ SomeThing.any_instance.should_receive(:foo).and_return(scoped)
+ subject.source.should be scoped
+ end
+ end
+ end
+end
View
102 spec/draper/decorator_spec.rb
@@ -126,100 +126,26 @@
end
describe ".decorates_association" do
- context "for ActiveModel collection associations" do
- before { subject.class.decorates_association :similar_products }
+ before { subject.class.decorates_association :similar_products, with: ProductDecorator }
- context "when the association is not empty" do
- it "decorates the collection" do
- subject.similar_products.should be_a Draper::CollectionDecorator
- subject.similar_products.each {|item| item.should be_decorated_with ProductDecorator }
- end
- end
-
- context "when the association is empty" do
- it "doesn't decorate the collection" do
- source.stub(:similar_products).and_return([])
- subject.similar_products.should_not be_a Draper::CollectionDecorator
- subject.similar_products.should be_empty
- end
- end
- end
-
- context "for Plain Old Ruby Object collection associations" do
- before { subject.class.decorates_association :poro_similar_products }
-
- context "when the association is not empty" do
- it "decorates the collection" do
- subject.poro_similar_products.should be_a Draper::CollectionDecorator
- subject.poro_similar_products.each {|item| item.should be_decorated_with ProductDecorator }
- end
- end
+ describe "overridden association method" do
+ let(:decorated_association) { ->{} }
- context "when the association is empty" do
- it "doesn't decorate the collection" do
- source.stub(:poro_similar_products).and_return([])
- subject.poro_similar_products.should_not be_a Draper::CollectionDecorator
- subject.poro_similar_products.should be_empty
- end
+ it "creates a DecoratedAssociation" do
+ Draper::DecoratedAssociation.should_receive(:new).with(source, :similar_products, {with: ProductDecorator}).and_return(decorated_association)
+ subject.similar_products
end
- end
- context "for an ActiveModel singular association" do
- before { subject.class.decorates_association :previous_version }
-
- context "when the association is present" do
- it "decorates the association" do
- subject.previous_version.should be_decorated_with ProductDecorator
- end
+ it "memoizes the DecoratedAssociation" do
+ Draper::DecoratedAssociation.should_receive(:new).once.and_return(decorated_association)
+ subject.similar_products
+ subject.similar_products
end
- context "when the association is absent" do
- it "doesn't decorate the association" do
- source.stub(:previous_version).and_return(nil)
- subject.previous_version.should be_nil
- end
- end
- end
-
- context "for an ActiveModel singular association" do
- before { subject.class.decorates_association :poro_previous_version }
-
- context "when the association is present" do
- it "decorates the association" do
- subject.poro_previous_version.should be_decorated_with ProductDecorator
- end
- end
-
- context "when the association is absent" do
- it "doesn't decorate the association" do
- source.stub(:poro_previous_version).and_return(nil)
- subject.poro_previous_version.should be_nil
- end
- end
- end
-
- context "when a decorator is specified" do
- before { subject.class.decorates_association :previous_version, with: SpecificProductDecorator }
-
- it "decorates with the specified decorator" do
- subject.previous_version.should be_decorated_with SpecificProductDecorator
- end
- end
-
- context "with a scope" do
- before { subject.class.decorates_association :thing, scope: :foo }
-
- it "applies the scope before decoration" do
- SomeThing.any_instance.should_receive(:foo).and_return(:bar)
- subject.thing.model.should == :bar
- end
- end
-
- context "for a polymorphic association" do
- before { subject.class.decorates_association :thing, polymorphic: true }
-
- it "makes the association return the right decorator" do
- subject.thing.should be_decorated_with SomeThingDecorator
+ it "calls the DecoratedAssociation" do
+ Draper::DecoratedAssociation.stub(:new).and_return(decorated_association)
+ decorated_association.should_receive(:call).and_return(:decorated)
+ subject.similar_products.should be :decorated
end
end
end
View
0  performance/active_record.rb → spec/performance/active_record.rb
File renamed without changes
View
0  performance/bechmark.rb → spec/performance/benchmark.rb
File renamed without changes
View
0  performance/decorators.rb → spec/performance/decorators.rb
File renamed without changes
View
0  performance/models.rb → spec/performance/models.rb
File renamed without changes
Something went wrong with that request. Please try again.