Merge pull request activeadmin#1831 from goshakkk/refactor

Some refactoring

Author: pcreux

lib/active_admin/helpers/optional_display.rb

@@ -1,5 +1,5 @@
 module ActiveAdmin
-  
+
   # Shareable module to give a #display_on?(action) method
   # which returns true or false depending on an options hash.
   #
@@ -23,7 +23,7 @@ def display_on?(action, render_context = nil)
         when Symbol, String
           render_context ? render_context.send(symbol_or_proc) : self.send(symbol_or_proc)
         when Proc
-            render_context ? render_context.instance_exec(&symbol_or_proc) : instance_exec(&symbol_or_proc)
+          render_context ? render_context.instance_exec(&symbol_or_proc) : instance_exec(&symbol_or_proc)
         else symbol_or_proc
         end
       end
@@ -33,12 +33,8 @@ def display_on?(action, render_context = nil)
     private
 
     def normalize_display_options!
-      if @options[:only]
-        @options[:only] = @options[:only].is_a?(Array) ? @options[:only] : [@options[:only]]
-      end
-      if @options[:except]
-        @options[:except] = @options[:except].is_a?(Array) ? @options[:except] : [@options[:except]]
-      end
+      @options[:only] = Array(@options[:only]) if @options[:only]
+      @options[:except] = Array(@options[:except]) if @options[:except]
     end
   end
 end

lib/active_admin/resource_dsl.rb

@@ -18,7 +18,7 @@ def belongs_to(target, options = {})
     #   end
     #
     # Then every time we instantiate and object, it would call
-    #   
+    #
     #   current_user.posts.build
     #
     # By default Active Admin will use the resource name to build a
@@ -28,7 +28,7 @@ def belongs_to(target, options = {})
     #   scope_to :current_user, :association_method => :blog_posts
     #
     # will result in the following
-    # 
+    #
     #   current_user.blog_posts.build
     #
     def scope_to(*args, &block)
@@ -94,9 +94,9 @@ def csv(options={}, &block)
     #
     # You can treat everything within the block as a standard Rails controller
     # action.
-    # 
-    def member_action(name, options = {}, &block)
-      config.member_actions << ControllerAction.new(name, options)
+    #
+    def action(set, name, options = {}, &block)
+      set << ControllerAction.new(name, options)
       title = options.delete(:title)
 
       controller do
@@ -105,14 +105,12 @@ def member_action(name, options = {}, &block)
       end
     end
 
-    def collection_action(name, options = {}, &block)
-      config.collection_actions << ControllerAction.new(name, options)
-      title = options.delete(:title)
+    def member_action(name, options = {}, &block)
+      action config.member_actions, name, options, &block
+    end
 
-      controller do
-        before_filter(:only => [name]){ @page_title = title } if title
-        define_method(name, &block || Proc.new{})
-      end
+    def collection_action(name, options = {}, &block)
+      action config.collection_actions, name, options, &block
     end
 
     # Defined Callbacks
@@ -151,7 +149,7 @@ def collection_action(name, options = {}, &block)
     # Specify which actions to create in the controller
     #
     # Eg:
-    #   
+    #
     #   ActiveAdmin.register Post do
     #     actions :index, :show
     #   end

lib/active_admin/router.rb

@@ -1,6 +1,5 @@
 module ActiveAdmin
   class Router
-
     def initialize(application)
       @application = application
     end
@@ -14,63 +13,42 @@ def initialize(application)
     #   end
     #
     def apply(router)
-      # Define any necessary dashboard routes and root
-      router.instance_exec(@application.namespaces.values) do |namespaces|
-        namespaces.each do |namespace|
-          root_and_dashboard_routes = Proc.new do
-            root :to => (namespace.root_to || "dashboard#index")
-            if ActiveAdmin::Dashboards.built?
-              match '/dashboard' => 'dashboard#index', :as => 'dashboard'
-            end
-          end
+      define_basic_routes router
+      define_resource_routes router
+    end
 
+    # Define any necessary dashboard routes and root
+    def define_basic_routes(router)
+      router.instance_exec(@application.namespaces.values, self) do |namespaces, aa_router|
+        namespaces.each do |namespace|
           if namespace.root?
-            instance_eval &root_and_dashboard_routes
+            instance_eval &aa_router.root_and_dashboard_routes(namespace)
           else
             namespace(namespace.name) do
-              instance_eval &root_and_dashboard_routes
+              instance_eval &aa_router.root_and_dashboard_routes(namespace)
             end
           end
         end
       end
+    end
 
-      # Now define the routes for each resource
-      router.instance_exec(@application.namespaces) do |namespaces|
-        resources = namespaces.values.collect{|n| n.resources.resources }.flatten
-        resources.each do |config|
-
-          # Define the block the will get eval'd within the namespace
-          route_definition_block = Proc.new do
-            case config
-            when Resource
-              resources config.resource_name.route_key, :only => config.defined_actions do
-                # Define any member actions
-                member do
-                  config.member_actions.each do |action|
-                    # eg: get :comment
-                    send(action.http_verb, action.name)
-                  end
-                end
-
-                # Define any collection actions
-                collection do
-                  config.collection_actions.each do |action|
-                    send(action.http_verb, action.name)
-                  end
+    def root_and_dashboard_routes(namespace)
+      Proc.new do
+        root :to => (namespace.root_to || "dashboard#index")
+        if ActiveAdmin::Dashboards.built?
+          match '/dashboard' => 'dashboard#index', :as => 'dashboard'
+        end
+      end
+    end
 
-                  post :batch_action
-                end
-              end
-            when Page
+    # Define the routes for each resource
+    def define_resource_routes(router)
+      resource_routes = method(:resource_routes)
 
-              match "/#{config.underscored_resource_name}" => "#{config.underscored_resource_name}#index"
-              config.page_actions.each do |action|
-                match "/#{config.underscored_resource_name}/#{action.name}" => "#{config.underscored_resource_name}##{action.name}", :via => action.http_verb
-              end
-            else
-              raise "Unsupported config class: #{config.class}"
-            end
-          end
+      router.instance_exec(@application.namespaces, self) do |namespaces, aa_router|
+        resources = namespaces.values.collect{|n| n.resources.resources }.flatten
+        resources.each do |config|
+          route_definition_block = aa_router.resource_routes(config)
 
           # Add in the parent if it exists
           if config.belongs_to?
@@ -110,5 +88,39 @@ def apply(router)
         end
       end
     end
+
+    def resource_routes(config)
+      Proc.new do
+        case config
+        when Resource
+          resources config.resource_name.route_key, :only => config.defined_actions do
+            # Define any member actions
+            member do
+              config.member_actions.each do |action|
+                # eg: get :comment
+                send(action.http_verb, action.name)
+              end
+            end
+
+            # Define any collection actions
+            collection do
+              config.collection_actions.each do |action|
+                send(action.http_verb, action.name)
+              end
+
+              post :batch_action
+            end
+          end
+        when Page
+          match "/#{config.underscored_resource_name}" => "#{config.underscored_resource_name}#index"
+          config.page_actions.each do |action|
+            match "/#{config.underscored_resource_name}/#{action.name}" => "#{config.underscored_resource_name}##{action.name}", :via => action.http_verb
+          end
+        else
+          raise "Unsupported config class: #{config.class}"
+        end
+      end
+
+    end
   end
 end