Merge remote branch 'upstream/master'

Conflicts:
	lib/cancan/ability.rb
	lib/cancan/active_record_additions.rb
	lib/cancan/can_definition.rb
	spec/cancan/ability_spec.rb

CHANGELOG.rdoc

@@ -1,3 +1,11 @@
+* Load nested parent resources on collection actions such as "index" (thanks dohzya)
+
+* Adding :name option to load_and_authorize_resource if it does not match controller - see issue #65
+
+* Fixing issue when using accessible_by with nil can conditions (thanks jrallison) - see issue #66
+
+* Pluralize table name for belongs_to associations in can conditions hash (thanks logandk) - see issue #62
+
 * Support has_many association or arrays in can conditions hash
 
 * Adding joins clause to accessible_by when conditions are across associations

lib/cancan/ability.rb

@@ -16,7 +16,7 @@ module CanCan
   #   end
   #
   module Ability
-    # Use to check the user's permission for a given action and object.
+    # Use to check if the user has permission to perform a given action on an object.
     #
     #   can? :destroy, @project
     #
@@ -47,6 +47,7 @@ module CanCan
     #     assert ability.cannot?(:destroy, Project.new)
     #   end
     #
+    # Also see the RSpec Matchers to aid in testing.
     def can?(action, subject, *extra_args)
       raise Error, "Nom nom nom. I eated it." if action == :has && subject == :cheezburger
       matching_can_definition(action, subject) do |can_definition|
@@ -80,8 +81,8 @@ module CanCan
     #
     #   can :read, Project, :active => true, :user_id => user.id
     #
-    # Here the user can only see active projects which he owns. See ControllerAdditions#conditions for a way to
-    # use this in database queries.
+    # Here the user can only see active projects which he owns. See ActiveRecordAdditions#accessible_by
+    # for how to use this in database queries.
     #
     # If the conditions hash does not give you enough control over defining abilities, you can use a block to
     # write any Ruby code you want.
@@ -122,7 +123,7 @@ module CanCan
       can_definitions << CanDefinition.new(true, action, subject, conditions, block)
     end
 
-    # Define an ability which cannot be done. Accepts the same arguments as "can".
+    # Defines an ability which cannot be done. Accepts the same arguments as "can".
     #
     #   can :read, :all
     #   cannot :read, Comment
@@ -198,14 +199,17 @@ module CanCan
     # If the ability is not defined then false is returned so be sure to take that into consideration.
     # If the ability is defined using a block then this will raise an exception since a hash of conditions cannot be
     # determined from that.
-    def conditions(action, subject)
+    def conditions(action, subject, options = {})
       matched = matching_can_definition(action, subject)
       unless matched.empty?
-        if matched.any?{|can_definition| can_definition.conditions.nil? && can_definition.block }
+        if matched.any?{|can_definition| 
+		cond = can_definition.conditions
+		(cond.nil? || cond.empty?) && can_definition.block 
+		}
           raise Error, "Cannot determine ability conditions from block for #{action.inspect} #{subject.inspect}"
         end
         matched.map{|can_definition|
-            [can_definition.base_behavior, can_definition.conditions]
+            [can_definition.base_behavior, can_definition.conditions(options)]
         }
       else
         false
@@ -226,8 +230,8 @@ module CanCan
     # If there is just one :can ability, it conditions returned untouched.
     # If the ability is defined using a block then this will raise an exception since a hash of conditions cannot be
     # determined from that.   
-    def sql_conditions(action, subject)
-      conds = conditions(action, subject)
+    def sql_conditions(action, subject, options = {})
+      conds = conditions(action, subject, options)
       return false if conds == false
       return (conds[0][1] || {}) if conds.size==1 && conds[0][0] == true # to match previous spec
       
@@ -235,7 +239,7 @@ module CanCan
       false_cond = subject.send(:sanitize_sql, ['?=?', true, false])
       conds.reverse.inject(nil) do |sql, action|
         behavior, condition = action
-        if condition
+        if condition && !condition.empty?
           condition = "#{subject.send(:sanitize_sql, condition)}"
           condition = "not (#{condition})" if behavior == false
         else

lib/cancan/active_record_additions.rb

@@ -1,5 +1,5 @@
 module CanCan
-  # This module is automatically included into all Active Record.
+  # This module is automatically included into all Active Record models.
   module ActiveRecordAdditions
     module ClassMethods
       # Returns a scope which fetches only the records that the passed ability
@@ -20,7 +20,7 @@ module CanCan
       # Here only the articles which the user can update are returned. This
       # internally uses Ability#conditions method, see that for more information.
       def accessible_by(ability, action = :read)
-        conditions = ability.sql_conditions(action, self) || {:id => nil}
+        conditions = ability.sql_conditions(action, self, :tableize => true) || {:id => nil}
         joins = ability.association_joins(action, self)
         if respond_to? :where
           where(conditions).joins(joins)

lib/cancan/can_definition.rb

@@ -1,16 +1,26 @@
 module CanCan
   # This class is used internally and should only be called through Ability.
+  # it holds the information about a "can" call made on Ability and provides
+  # helpful methods to determine permission checking and conditions hash generation.
   class CanDefinition # :nodoc:
     attr_reader :conditions, :block, :base_behavior, :definitive
+    include ActiveSupport::Inflector
     
+    # The first argument when initializing is the base_behavior which is a true/false
+    # value. True for "can" and false for "cannot". The next two arguments are the action
+    # and subject respectively (such as :read, @project). The third argument is a hash
+    # of conditions and the last one is the block passed to the "can" call.
     def initialize(base_behavior, action, subject, conditions, block)
       @base_behavior = base_behavior
       @actions = [action].flatten
       @subjects = [subject].flatten
-      @conditions = conditions
+      @conditions = conditions || {}
       @block = block
     end
 
+    # Accepts a hash of aliased actions and returns an array of actions which match.
+    # This should be called before "matches?" and other checking methods since they
+    # rely on the actions to be expanded.
     def expand_actions(aliased_actions)
       @expanded_actions = @actions.map do |action|
         aliased_actions[action] ? [action, *aliased_actions[action]] : action
@@ -27,9 +37,23 @@ module CanCan
       @base_behavior ? result : !result
     end
 
+    # Returns a hash of conditions. If the ":tableize => true" option is passed
+    # it will pluralize the association conditions to match the table name.
+    def conditions(options = {})
+      if options[:tableize] && @conditions.kind_of?(Hash)
+        @conditions.inject({}) do |tableized_conditions, (name, value)|
+          name = tableize(name).to_sym if value.kind_of? Hash
+          tableized_conditions[name] = value
+          tableized_conditions
+        end
+      else
+        @conditions
+      end
+    end
+
     def association_joins(conditions = @conditions)
       joins = []
-      (conditions || []).each do |name, value|
+      conditions.each do |name, value|
         if value.kind_of? Hash
           nested = association_joins(value)
           if nested

lib/cancan/controller_additions.rb

@@ -59,6 +59,11 @@ module CanCan
       #
       #     load_resource :nested => [:publisher, :author]
       #
+      # [:+name+]
+      #   The name of the resource if it cannot be determined from controller (string or symbol).
+      #
+      #     load_resource :name => :article
+      #
       # [:+resource+]
       #   The class to use for the model (string or constant).
       #
@@ -102,6 +107,11 @@ module CanCan
       # [:+except+]
       #   Does not apply before filter to given actions.
       #
+      # [:+name+]
+      #   The name of the resource if it cannot be determined from controller (string or symbol).
+      #
+      #     load_resource :name => :article
+      #
       # [:+resource+]
       #   The class to use for the model (string or constant). Alternatively pass a symbol
       #   to represent a resource which does not have a class.

lib/cancan/controller_resource.rb

@@ -1,5 +1,7 @@
 module CanCan
   # Used internally to load and authorize a given controller resource.
+  # This manages finding or building an instance of the resource. If a
+  # parent is given it will go through the association.
   class ControllerResource # :nodoc:
     def initialize(controller, name, parent = nil, options = {})
       raise ImplementationRemoved, "The :class option has been renamed to :resource for specifying the class in CanCan." if options.has_key? :class
@@ -9,6 +11,9 @@ module CanCan
       @options = options
     end
     
+    # Returns the class used for this resource. This can be overriden by the :resource option.
+    # Sometimes one will use a symbol as the resource if a class does not exist for it. In that
+    # case "find" and "build" should not be called on it.
     def model_class
       resource_class = @options[:resource]
       if resource_class.nil?
@@ -16,7 +21,7 @@ module CanCan
       elsif resource_class.kind_of? String
         resource_class.constantize
       else
-        resource_class # likely a symbol
+        resource_class # could be a symbol
       end
     end
     
@@ -24,12 +29,10 @@ module CanCan
       self.model_instance ||= base.find(id)
     end
     
+    # Build a new instance of this resource. If it is a class we just call "new" otherwise
+    # it's an associaiton and "build" is used.
     def build(attributes)
-      if base.kind_of? Class
-        self.model_instance ||= base.new(attributes)
-      else
-        self.model_instance ||= base.build(attributes)
-      end
+      self.model_instance ||= (base.kind_of?(Class) ? base.new(attributes) : base.build(attributes))
     end
     
     def model_instance
@@ -42,6 +45,8 @@ module CanCan
     
     private
     
+    # The object that methods (such as "find", "new" or "build") are called on.
+    # If there is a parent it will be the association, otherwise it will be the model's class.
     def base
       @parent ? @parent.model_instance.send(@name.to_s.pluralize) : model_class
     end

lib/cancan/exceptions.rb

@@ -18,7 +18,7 @@ module CanCan
   #   exception.action # => :read
   #   exception.subject # => Article
   #
-  # If the message is not specified (or is nil) it will default to "You are anot authorized
+  # If the message is not specified (or is nil) it will default to "You are not authorized
   # to access this page." This default can be overridden by setting default_message.
   #
   #   exception.default_message = "Default error message"

lib/cancan/resource_authorization.rb

@@ -20,7 +20,9 @@ module CanCan
     end
 
     def load_resource
-      unless collection_actions.include? @params[:action].to_sym
+      if collection_actions.include? @params[:action].to_sym
+        parent_resource
+      else
         if new_actions.include? @params[:action].to_sym
           resource.build(@params[model_name.to_sym])
         elsif @params[:id]
@@ -54,7 +56,7 @@ module CanCan
     end
 
     def model_name
-      @params[:controller].sub("Controller", "").underscore.split('/').last.singularize
+      @options[:name] || @params[:controller].sub("Controller", "").underscore.split('/').last.singularize
     end
 
     def collection_actions

spec/cancan/ability_spec.rb

@@ -226,7 +226,7 @@ describe CanCan::Ability do
   
   it "should return an array with just behavior for conditions when there are no conditions" do
     @ability.can :read, Array
-    @ability.conditions(:show, Array).should == [ [true, nil] ]
+    @ability.conditions(:show, Array).should == [ [true, {}] ]
   end
 
   it "should return false when performed on an action which isn't defined" do

spec/cancan/active_record_additions_spec.rb

@@ -16,13 +16,13 @@ describe CanCan::ActiveRecordAdditions do
 
   it "should call where with matching ability conditions" do
     @ability.can :read, @model_class, :foo => {:bar => 1}
-    stub(@model_class).where(:foo => { :bar => 1 }).stub!.joins([:foo]) { :found_records }
+    stub(@model_class).where(:foos => { :bar => 1 }).stub!.joins([:foo]) { :found_records }
     @model_class.accessible_by(@ability, :read).should == :found_records
   end
 
   it "should default to :read ability and use scoped when where isn't available" do
     @ability.can :read, @model_class, :foo => {:bar => 1}
-    stub(@model_class).scoped(:conditions => {:foo => {:bar => 1}}, :joins => [:foo]) { :found_records }
+    stub(@model_class).scoped(:conditions => {:foos => {:bar => 1}}, :joins => [:foo]) { :found_records }
     @model_class.accessible_by(@ability).should == :found_records
   end
 end

spec/cancan/can_definition_spec.rb

@@ -30,4 +30,15 @@ describe CanCan::CanDefinition do
     @conditions[:foo] = {:bar => {1 => 2}}
     @can.association_joins.should == [{:foo => [{:bar=>[]}]}]
   end
+
+  it "should return table names in conditions for association joins" do
+    @conditions[:foo] = {:bar => 1}
+    @conditions[:test] = 1
+    @can.conditions(:tableize => true).should == { :foos => { :bar => 1}, :test => 1 }
+  end
+
+  it "should return no association joins if conditions is nil" do
+    can = CanCan::CanDefinition.new(true, :read, Integer, nil, nil)
+    can.association_joins.should be_nil
+  end
 end

spec/cancan/resource_authorization_spec.rb

@@ -86,12 +86,23 @@ describe CanCan::ResourceAuthorization do
   end
 
   it "should load nested resource and fetch other resource through the association" do
-    stub(Person).find(456).stub!.abilities.stub!.find(123) { :some_ability }
+    person = Object.new
+    stub(Person).find(456) { person }
+    stub(person).abilities.stub!.find(123) { :some_ability }
     authorization = CanCan::ResourceAuthorization.new(@controller, {:controller => "abilities", :action => "show", :id => 123, :person_id => 456}, {:nested => :person})
     authorization.load_resource
+    @controller.instance_variable_get(:@person).should == person
     @controller.instance_variable_get(:@ability).should == :some_ability
   end
 
+  it "should load nested resource for collection action" do
+    person = Object.new
+    stub(Person).find(456) { person }
+    authorization = CanCan::ResourceAuthorization.new(@controller, {:controller => "abilities", :action => "index", :person_id => 456}, {:nested => :person})
+    authorization.load_resource
+    @controller.instance_variable_get(:@person).should == person
+  end
+
   it "should load nested resource and build resource through a deep association" do
     stub(Person).find(456).stub!.behaviors.stub!.find(789).stub!.abilities.stub!.build(nil) { :some_ability }
     authorization = CanCan::ResourceAuthorization.new(@controller, {:controller => "abilities", :action => "new", :person_id => 456, :behavior_id => 789}, {:nested => [:person, :behavior]})
@@ -114,4 +125,11 @@ describe CanCan::ResourceAuthorization do
     authorization.load_resource
     @controller.instance_variable_get(:@ability).should == :some_resource
   end
+
+  it "should use :name option to determine resource name" do
+    stub(Ability).find(123) { :some_resource }
+    authorization = CanCan::ResourceAuthorization.new(@controller, {:controller => "foo", :action => "show", :id => 123}, {:name => :ability})
+    authorization.load_resource
+    @controller.instance_variable_get(:@ability).should == :some_resource
+  end
 end