Merge pull request activeadmin#2912 from carnesmedia/issue-2905

Decorate on new for consistency. Closes activeadmin#2905

Author: seanlinsley

lib/active_admin/orm/active_record/comments/comment.rb

@@ -14,8 +14,11 @@ class Comment < ActiveRecord::Base
 
     # @returns [String] The name of the record to use for the polymorphic relationship
     def self.resource_type(resource)
-      record = resource.respond_to?(:decorated?) && resource.decorated? ? resource.model : resource
-      record.class.name.to_s
+      undecorate_resource(resource).class.name.to_s
+    end
+
+    def self.undecorate_resource(resource)
+      ActiveAdmin::ResourceController::Decorators.undecorate_resource(resource)
     end
 
     # Postgres adapters won't compare strings to numbers (issue 34)

lib/active_admin/resource_controller/data_access.rb

@@ -113,14 +113,18 @@ def find_resource
       #
       # @returns [ActiveRecord::Base] An un-saved active record base object
       def build_resource
-        return resource if resource = get_resource_ivar
+        _resource = get_resource_ivar
+
+        return _resource if _resource
+
+        _resource = build_new_resource
 
-        resource = build_new_resource
+        run_build_callbacks _resource
+        authorize_resource! _resource
 
-        run_build_callbacks resource
-        authorize_resource! resource
+        _resource = apply_decorator(_resource)
 
-        set_resource_ivar(resource)
+        set_resource_ivar(_resource)
       end
 
       # Builds a new resource. This method uses the method_for_build provided
@@ -205,7 +209,7 @@ def apply_authorization_scope(collection)
 
       def apply_sorting(chain)
         params[:order] ||= active_admin_config.sort_order
-        
+
         order_clause = OrderClause.new params[:order]
 
         if order_clause.valid?

lib/active_admin/resource_controller/decorators.rb

@@ -4,19 +4,38 @@ module Decorators
       protected
 
       def apply_decorator(resource)
-        decorator_class ? decorator_class.new(resource) : resource
+        decorate? ? decorator_class.new(resource) : resource
       end
 
       def apply_collection_decorator(collection)
-        if decorator = collection_decorator
-          decorator.decorate(collection, with: decorator_class)
+        if decorate?
+          collection_decorator.decorate(collection, with: decorator_class)
         else
           collection
         end
       end
 
+      # TODO: find a more suitable place for this
+      def self.undecorate_resource(resource)
+        if resource.respond_to?(:decorated?) && resource.decorated?
+          resource.model
+        else
+          resource
+        end
+      end
+
       private
 
+      def decorate?
+        case action_name
+        when 'new', 'edit'
+          form = active_admin_config.get_page_presenter :form
+          form && form.options[:decorate] && decorator_class.present?
+        else
+          decorator_class.present?
+        end
+      end
+
       def decorator_class
         active_admin_config.decorator_class
       end

lib/active_admin/view_helpers/form_helper.rb

@@ -6,10 +6,6 @@ def active_admin_form_for(resource, options = {}, &block)
         options = Marshal.load( Marshal.dump(options) )
         options[:builder] ||= ActiveAdmin::FormBuilder
 
-        if !options.fetch(:decorate, false)
-          resource = resource.model if resource.respond_to? :model
-        end
-
         semantic_form_for resource, options, &block
       end
 
@@ -19,6 +15,7 @@ def hidden_field_tags_for(params, options={})
           hidden_field_tag k, v, :id => sanitize_to_id("hidden_active_admin_#{k}")
         end.join("\n").html_safe
       end
+
     end
   end
 end

spec/unit/resource_controller/data_access_spec.rb

@@ -14,19 +14,18 @@
   end
 
   describe "searching" do
-    let(:params){ {:q => {} }}
+    let(:params) {{ :q => {} }}
     it "should call the search method" do
       chain = double "ChainObj"
       expect(chain).to receive(:ransack).with(params[:q]).once.and_return(Post.ransack)
       controller.send :apply_filtering, chain
     end
-
   end
 
   describe "sorting" do
 
     context "valid clause" do
-      let(:params){ {:order => "id_asc" }}
+      let(:params) {{ :order => "id_asc" }}
 
       it "reorders chain" do
         chain = double "ChainObj"
@@ -36,8 +35,8 @@
     end
 
     context "invalid clause" do
-      let(:params){ {:order => "_asc" }}
-      
+      let(:params) {{ :order => "_asc" }}
+
       it "returns chain untouched" do
         chain = double "ChainObj"
         expect(chain).not_to receive(:reorder)
@@ -69,7 +68,6 @@
         expect(controller.send(:collection_before_scope)).to eq chain
       end
     end
-
   end
 
 end

spec/unit/resource_controller/decorators_spec.rb

@@ -3,21 +3,28 @@
 describe ActiveAdmin::ResourceController::Decorators do
   let(:controller_class) do
     Class.new do
-      include ActiveAdmin::ResourceController::Decorators
-
       def self.name
         "Test Controller using Decorators"
       end
 
+      def self.helper_method(method)
+        # STUB
+      end
+
+      include ActiveAdmin::ResourceController::Decorators
+
       public :apply_decorator, :apply_collection_decorator
     end
   end
 
   let(:controller) { controller_class.new }
-  before { controller.stub(active_admin_config: double(decorator_class: decorator_class)) }
+  let(:active_admin_config) { double(decorator_class: decorator_class) }
+  before { controller.stub(active_admin_config: active_admin_config) }
+  before { controller.stub(action_name: action) }
 
 
   describe '#apply_decorator' do
+    let(:action) { 'show' }
     let(:resource) { Post.new }
     subject(:applied) { controller.apply_decorator(resource) }
 
@@ -34,6 +41,7 @@ def self.name
 
   describe '#apply_collection_decorator' do
     before { Post.create! }
+    let(:action) { 'index' }
     let(:collection) { Post.scoped }
     subject(:applied) { controller.apply_collection_decorator(collection) }
 
@@ -54,4 +62,25 @@ def self.name
       end
     end
   end
+
+  describe 'form actions' do
+    let(:action) { 'edit' }
+    let(:resource) { Post.new }
+    let(:form_presenter) { double options: { decorate: decorate_form } }
+    let(:decorator_class) { PostDecorator }
+    before { active_admin_config.stub(:get_page_presenter).with(:form).and_return form_presenter }
+
+    subject(:applied) { controller.apply_decorator(resource) }
+
+    context 'when the form is not configured to decorate' do
+      let(:decorate_form) { false }
+      it { should be_kind_of(Post) }
+    end
+
+    context 'when the form is configured to decorate' do
+      let(:decorate_form) { true }
+      it { should be_kind_of(PostDecorator) }
+    end
+
+  end
 end

spec/unit/view_helpers/form_helper_spec.rb

@@ -19,25 +19,6 @@
       expect(view).to receive(:semantic_form_for).with(resource, builder: custom_builder)
       view.active_admin_form_for(resource, builder: custom_builder)
     end
-
-    context 'with a decorated resource' do
-      let(:decorated) { double 'decorated_resource', model: resource }
-
-      it 'can disable automatic decoration' do
-        expect(view).to receive(:semantic_form_for).with(resource, default_options.merge(decorate: false))
-        view.active_admin_form_for(decorated, decorate: false)
-      end
-
-      it 'can enable automatic decoration' do
-        expect(view).to receive(:semantic_form_for).with(decorated, default_options.merge(decorate: true))
-        view.active_admin_form_for(decorated, decorate: true)
-      end
-
-      it 'defaults to not decorating' do
-        expect(view).to receive(:semantic_form_for).with(resource, default_options)
-        view.active_admin_form_for(decorated)
-      end
-    end
   end
 
   describe ".hidden_field_tags_for" do