From 19496f0c4f2fde1606a01763db93e6d5c7adfbf3 Mon Sep 17 00:00:00 2001 From: Alexandre de Oliveira Date: Mon, 7 May 2012 23:24:01 -0300 Subject: [PATCH] Adds support for ActiveModel::Errors The goal here is to improve ActiveModel support so that Draper can work seamlessly with Rails' FormHelpers, proxying the model's #errors method. I also added support for ActiveModel::Errors, adding a proxy to the model's #errors method only if it's a descendant of ActiveModel::Validations. Also some refactoring was done. Draper now proxies #to_param and #id methods only if the model is an ActiveModel descendant. Other things I did include: - created Draper::ActiveModelSupport::Proxies, which contains the methods for proxying default methods(to_param, id, etc) depending on the ancestors - wrote specs for class with ActiveModel as ancestor - wrote specs for class without ActiveModel as ancestor --- lib/draper.rb | 1 + lib/draper/active_model_support.rb | 24 ++++++++++ lib/draper/base.rb | 11 ++--- spec/draper/base_spec.rb | 44 ++++++++++++++----- spec/draper/model_support_spec.rb | 2 +- spec/spec_helper.rb | 36 ++++++++------- spec/support/samples/active_model.rb | 9 ++++ spec/support/samples/active_record.rb | 8 ++++ .../samples/non_active_model_product.rb | 2 + 9 files changed, 101 insertions(+), 36 deletions(-) create mode 100644 lib/draper/active_model_support.rb create mode 100644 spec/support/samples/active_model.rb create mode 100644 spec/support/samples/non_active_model_product.rb diff --git a/lib/draper.rb b/lib/draper.rb index 318c739..b0fe4cb 100644 --- a/lib/draper.rb +++ b/lib/draper.rb @@ -1,5 +1,6 @@ require "draper/version" require 'draper/system' +require 'draper/active_model_support' require 'draper/base' require 'draper/lazy_helpers' require 'draper/model_support' diff --git a/lib/draper/active_model_support.rb b/lib/draper/active_model_support.rb new file mode 100644 index 0000000..872c47d --- /dev/null +++ b/lib/draper/active_model_support.rb @@ -0,0 +1,24 @@ +module Draper::ActiveModelSupport + module Proxies + def create_proxies + # These methods (as keys) will be created only if the correspondent + # model descends from a specific class (as value) + proxies = {} + proxies[:to_param] = ActiveModel::Conversion if defined?(ActiveModel::Conversion) + proxies[:errors] = ActiveModel::Validations if defined?(ActiveModel::Validations) + proxies[:id] = ActiveRecord::Base if defined?(ActiveRecord::Base) + + proxies.each do |method_name, dependency| + if model.kind_of?(dependency) || dependency.nil? + class << self + self + end.class_eval do + self.send(:define_method, method_name) do |*args, &block| + model.send(method_name, *args, &block) + end + end + end + end + end + end +end diff --git a/lib/draper/base.rb b/lib/draper/base.rb index c779c03..b9d3f1c 100644 --- a/lib/draper/base.rb +++ b/lib/draper/base.rb @@ -8,15 +8,11 @@ module Draper DEFAULT_DENIED = Object.instance_methods << :method_missing DEFAULT_ALLOWED = [] - FORCED_PROXY = [:to_param, :id] - FORCED_PROXY.each do |method| - define_method method do |*args, &block| - model.send method, *args, &block - end - end self.denied = DEFAULT_DENIED self.allowed = DEFAULT_ALLOWED + include Draper::ActiveModelSupport::Proxies + # Initialize a new decorator instance by passing in # an instance of the source class. Pass in an optional # context inside the options hash is stored for later use. @@ -28,6 +24,7 @@ module Draper self.class.model_class = input.class if model_class.nil? @model = input.kind_of?(Draper::Base) ? input.model : input self.options = options + create_proxies end # Proxies to the class specified by `decorates` to automatically @@ -260,7 +257,7 @@ module Draper private def allow?(method) - (allowed.empty? || allowed.include?(method) || FORCED_PROXY.include?(method)) && !denied.include?(method) + (allowed.empty? || allowed.include?(method)) && !denied.include?(method) end def find_association_reflection(association) diff --git a/spec/draper/base_spec.rb b/spec/draper/base_spec.rb index 6e46603..8aa2250 100644 --- a/spec/draper/base_spec.rb +++ b/spec/draper/base_spec.rb @@ -4,6 +4,7 @@ describe Draper::Base do before(:each){ ApplicationController.new.set_current_view_context } subject{ Decorator.new(source) } let(:source){ Product.new } + let(:non_active_model_source){ NonActiveModelProduct.new } context("proxying class methods") do it "should pass missing class method calls on to the wrapped class" do @@ -195,7 +196,7 @@ describe Draper::Base do end end - context("selecting methods") do + describe "method selection" do it "echos the methods of the wrapped class except default exclusions" do source.methods.each do |method| unless Draper::Base::DEFAULT_DENIED.include?(method) @@ -208,27 +209,48 @@ describe Draper::Base do DecoratorWithApplicationHelper.new(source).length.should == "overridden" end - it "should always proxy to_param" do - source.send :class_eval, "def to_param; 1; end" - Draper::Base.new(source).to_param.should == 1 - end - - it "should always proxy id" do - source.send :class_eval, "def id; 123456789; end" - Draper::Base.new(source).id.should == 123456789 - end - it "should not copy the .class, .inspect, or other existing methods" do source.class.should_not == subject.class source.inspect.should_not == subject.inspect source.to_s.should_not == subject.to_s end + + context "when an ActiveModel descendant" do + it "should always proxy to_param" do + source.stub(:to_param).and_return(1) + Draper::Base.new(source).to_param.should == 1 + end + + it "should always proxy id" do + source.stub(:id).and_return(123456789) + Draper::Base.new(source).id.should == 123456789 + end + + it "should always proxy errors" do + Draper::Base.new(source).errors.should be_an_instance_of ActiveModel::Errors + end + end + + context "when not an ActiveModel descendant" do + it "does not proxy to_param" do + non_active_model_source.stub(:to_param).and_return(1) + Draper::Base.new(non_active_model_source).to_param.should_not == 1 + end + + it "does not proxy errors" do + Draper::Base.new(non_active_model_source).should_not respond_to :errors + end + end end context 'the decorated model' do it 'receives the mixin' do source.class.ancestors.include?(Draper::ModelSupport) end + + it 'includes ActiveModel support' do + source.class.ancestors.include?(Draper::ActiveModelSupport) + end end it "should wrap source methods so they still accept blocks" do diff --git a/spec/draper/model_support_spec.rb b/spec/draper/model_support_spec.rb index 1ed1111..67c2de9 100644 --- a/spec/draper/model_support_spec.rb +++ b/spec/draper/model_support_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Draper::ModelSupport do +describe Draper::ActiveModelSupport do subject { Product.new } describe '#decorator' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6b4e82a..bf8ab92 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,20 +2,22 @@ require 'rubygems' require 'bundler/setup' Bundler.require -require './spec/support/samples/active_record.rb' -require './spec/support/samples/application_controller.rb' -require './spec/support/samples/application_helper.rb' -require './spec/support/samples/decorator.rb' -require './spec/support/samples/decorator_with_allows.rb' -require './spec/support/samples/decorator_with_multiple_allows.rb' -require './spec/support/samples/decorator_with_application_helper.rb' -require './spec/support/samples/decorator_with_denies.rb' -require './spec/support/samples/namespaced_product.rb' -require './spec/support/samples/namespaced_product_decorator.rb' -require './spec/support/samples/product.rb' -require './spec/support/samples/product_decorator.rb' -require './spec/support/samples/specific_product_decorator.rb' -require './spec/support/samples/some_thing.rb' -require './spec/support/samples/some_thing_decorator.rb' -require './spec/support/samples/widget.rb' -require './spec/support/samples/widget_decorator.rb' +require './spec/support/samples/active_model' +require './spec/support/samples/active_record' +require './spec/support/samples/application_controller' +require './spec/support/samples/application_helper' +require './spec/support/samples/decorator' +require './spec/support/samples/decorator_with_allows' +require './spec/support/samples/decorator_with_multiple_allows' +require './spec/support/samples/decorator_with_application_helper' +require './spec/support/samples/decorator_with_denies' +require './spec/support/samples/namespaced_product' +require './spec/support/samples/namespaced_product_decorator' +require './spec/support/samples/non_active_model_product' +require './spec/support/samples/product' +require './spec/support/samples/product_decorator' +require './spec/support/samples/specific_product_decorator' +require './spec/support/samples/some_thing' +require './spec/support/samples/some_thing_decorator' +require './spec/support/samples/widget' +require './spec/support/samples/widget_decorator' diff --git a/spec/support/samples/active_model.rb b/spec/support/samples/active_model.rb new file mode 100644 index 0000000..6befb9e --- /dev/null +++ b/spec/support/samples/active_model.rb @@ -0,0 +1,9 @@ +module ActiveModel + module Conversion; end + module Validations; end + + class Errors + def initialize(params = nil) + end + end +end diff --git a/spec/support/samples/active_record.rb b/spec/support/samples/active_record.rb index 06a2ac6..928463b 100644 --- a/spec/support/samples/active_record.rb +++ b/spec/support/samples/active_record.rb @@ -1,5 +1,13 @@ module ActiveRecord class Base + include ActiveModel::Validations + include ActiveModel::Conversion + + attr_reader :errors, :to_model + + def initialize + @errors = ActiveModel::Errors.new(self) + end def self.limit self diff --git a/spec/support/samples/non_active_model_product.rb b/spec/support/samples/non_active_model_product.rb new file mode 100644 index 0000000..40e32c0 --- /dev/null +++ b/spec/support/samples/non_active_model_product.rb @@ -0,0 +1,2 @@ +class NonActiveModelProduct +end