Closes activeadmin#77. Fix performance issue with scope :all.

Scope :all does not call :all on the relation anymore but returns
the ActiveRecord class instead.

Moved the logic scoping the scope into a helper called ScopeChain.

I wanted to implement this method on the Scope class itself but that
runs the scoping block in the Scope instance context instead of the
ResourceController one.

This method is both used in the ResourceController and the
ViewHelper Scopes.

@gregbell: Could we share this method without creating a new
helper?

lib/active_admin.rb

@@ -32,6 +32,7 @@ module ActiveAdmin
   autoload :ResourceController,       'active_admin/resource_controller'
   autoload :Renderer,                 'active_admin/renderer'
   autoload :Scope,                    'active_admin/scope'
+  autoload :ScopeChain,               'active_admin/helpers/scope_chain'
   autoload :SidebarSection,           'active_admin/sidebar_section'
   autoload :TableBuilder,             'active_admin/table_builder'
   autoload :ViewFactory,              'active_admin/view_factory'

lib/active_admin/helpers/scope_chain.rb

@@ -0,0 +1,23 @@
+module ActiveAdmin
+  module ScopeChain
+    # Scope an ActiveRecord::Relation chain
+    #
+    # Example:
+    #   scope_chain(Scope.new(:published), Article)
+    #   # => Article.published
+    #
+    # @param scope The <ActiveAdmin::Scope> we want to scope on
+    # @param chain The ActiveRecord::Relation chain or ActiveRecord::Base class to scope
+    # @return <ActiveRecord::Relation or ActiveRecord::Base> The scoped relation chain
+    #
+    def scope_chain(scope, chain)
+      if scope.scope_method
+        chain.send(scope.scope_method)
+      elsif scope.scope_block
+        instance_exec chain, &scope.scope_block
+      else
+        chain
+      end
+    end
+  end
+end

lib/active_admin/resource_controller/collection.rb

@@ -83,26 +83,20 @@ module Scoping
         protected
 
         def active_admin_collection
-          scope_collection(super)
+          scope_current_collection(super)
         end
 
-        def scope_collection(chain)
+        def scope_current_collection(chain)
           if current_scope
             @before_scope_collection = chain
-
-            # ActiveRecord::Base isn't a relation, so let's help you out
-            return chain if current_scope.scope_method == :all
-
-            if current_scope.scope_method
-              chain.send(current_scope.scope_method)
-            else
-              instance_exec chain, &current_scope.scope_block
-            end
+            scope_chain(current_scope, chain)
           else
             chain
           end
         end
 
+        include ActiveAdmin::ScopeChain
+
         def current_scope
           @current_scope ||= if params[:scope]
             active_admin_config.get_scope_by_id(params[:scope]) if params[:scope]

lib/active_admin/scope.rb

@@ -3,15 +3,29 @@ class Scope
 
     attr_reader :name, :scope_method, :id, :scope_block
 
+    # Create a Scope
+    #
+    # Examples:
+    #
+    #   Scope.new(:published)
+    #   # => Scope with name 'Published' and scope method :published
+    #
+    #   Scope.new('Published', :public)
+    #   # => Scope with name 'Published' and scope method :public
+    #
+    #   Scope.new('Published') { |articles| articles.where(:published => true) }
+    #   # => Scope with name 'Published' using a block to scope
+    #
     def initialize(name, method = nil, &block)
       @name = name.to_s.titleize
-      @scope_method = method || name.to_sym
+      @scope_method = method
+      # Scope ':all' means no scoping
+      @scope_method ||= name.to_sym unless name.to_sym == :all
       @id = @name.gsub(' ', '').underscore
       if block_given?
         @scope_method = nil
         @scope_block = block
       end
     end
-
   end
 end

lib/active_admin/views/components/scopes.rb

@@ -53,12 +53,11 @@ def scope_count(scope)
         end
       end
 
+      include ActiveAdmin::ScopeChain
+
+      # Return the count for the scope passed in.
       def get_scope_count(scope)
-        if scope.scope_method
-          scoping_class.send(scope.scope_method).count
-        else
-          instance_exec(scoping_class, &scope.scope_block).count
-        end
+        scope_chain(scope, scoping_class).count
       end
 
       def scoping_class

spec/unit/helpers/scope_chain_spec.rb

@@ -0,0 +1,36 @@
+require 'spec_helper'
+
+describe ActiveAdmin::ScopeChain do
+
+  include ActiveAdmin::ScopeChain
+
+  describe "#scope_chain" do
+    let(:relation) { mock }
+
+    context "when Scope has a scope method" do
+      let(:scope) { ActiveAdmin::Scope.new :published }
+
+      it "should call the method on the relation and return it" do
+        relation.should_receive(:published).and_return(:scoped_relation)
+        scope_chain(scope, relation).should == :scoped_relation
+      end
+    end
+
+    context "when Scope has the scope method method ':all'" do
+      let(:scope) { ActiveAdmin::Scope.new :all }
+
+      it "should return the relation" do
+        scope_chain(scope, relation).should == relation
+      end
+    end
+
+    context "when Scope has a name and a scope block" do
+      let(:scope) { ActiveAdmin::Scope.new("My Scope"){|s| :scoped_relation } }
+
+      it "should instance_exec the block and return it" do
+        scope_chain(scope, relation).should == :scoped_relation
+      end
+    end
+  end
+end
+

spec/unit/scope_spec.rb

@@ -12,6 +12,16 @@
       its(:scope_method) { should == :published }
     end
 
+    context "when scope method is :all" do
+      let(:scope)        { ActiveAdmin::Scope.new :all }
+      its(:name)         { should == "All"}
+      its(:id)           { should == "all"}
+      # :all does not return a chain but an array of active record
+      # instances. We set the scope_method to nil then.
+      its(:scope_method) { should == nil }
+      its(:scope_block)  { should == nil }
+    end
+
     context "when a name and scope method" do
       let(:scope)        { ActiveAdmin::Scope.new "My Scope", :scope_method }
       its(:name)         { should == "My Scope"}
@@ -26,6 +36,6 @@
       its(:scope_method) { should == nil }
       its(:scope_block)  { should be_a(Proc)}
     end
-  end
+  end # describe "creating a scope"
 
 end