Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Comparing changes

Choose two branches to see what's changed or to start a new pull request. If you need to, you can also compare across forks.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also compare across forks.
base fork: ryanb/cancan
base: 1.6.0
...
head fork: ryanb/cancan
compare: 1.6.1
Checking mergeability… Don't worry, you can still create the pull request.
  • 4 commits
  • 8 files changed
  • 1 commit comment
  • 2 contributors
Commits on Mar 16, 2011
@amw amw Fixes inherited_resources collection authorization
This reverts e3eab13

I don't know what was the idea of that, but it turned out REAL bad.

`collection` sets the collection instance variable. `resource_base` is used all
over CanCan. It's also used inside `load_collection?` which is checked before
`load_collection` is called. That means we actually set the collection instance
variable through inherited_resources (without any authorization whatsoever) before trying to load it through CanCan using `accessible_by`.

    1. def load_resource
    2.  unless skip?(:load)
    3.    if load_instance?
    4.      self.resource_instance ||= load_resource_instance
    5.    elsif load_collection?
    6.      self.collection_instance ||= load_collection
    7.    end
    8.  end
    9. end

`collection_instance` is set on line 5 instead of line 6.
3639ca9
@ryanb making accessible_by action default to :index and parent action defau…
…lt to :show so we don't check :read action directly - closes #302
fdd5ad0
@ryanb use Item.new instead of build_item for singleton resource so it doesn…
…'t mess up database - closes #304
3f6cecb
@ryanb releasing 1.6.1 b0c1646
View
9 CHANGELOG.rdoc
@@ -1,3 +1,12 @@
+1.6.1 (March 15, 2011)
+
+* Use Item.new instead of build_item for singleton resource so it doesn't effect database - see issue #304
+
+* Made accessible_by action default to :index and parent action default to :show instead of :read - see issue #302
+
+* Reverted Inherited Resources "collection" override since it doesn't seem to be working - see issue #305
+
+
1.6.0 (March 11, 2011)
* Added MetaWhere support - see issue #194 and #261
View
2  cancan.gemspec
@@ -1,6 +1,6 @@
Gem::Specification.new do |s|
s.name = "cancan"
- s.version = "1.6.0"
+ s.version = "1.6.1"
s.author = "Ryan Bates"
s.email = "ryan@railscasts.com"
s.homepage = "http://github.com/ryanb/cancan"
View
22 lib/cancan/controller_resource.rb
@@ -82,10 +82,10 @@ def load_collection
end
def build_resource
- method_name = @options[:singleton] && resource_base.respond_to?("build_#{name}") ? "build_#{name}" : "new"
- resource = resource_base.send(method_name, @params[name] || {})
- initial_attributes.each do |name, value|
- resource.send("#{name}=", value)
+ resource = resource_base.new(@params[name] || {})
+ resource.send("#{parent_name}=", parent_resource) if @options[:singleton] && parent_resource
+ initial_attributes.each do |attr_name, value|
+ resource.send("#{attr_name}=", value)
end
resource
end
@@ -97,15 +97,15 @@ def initial_attributes
end
def find_resource
- if @options[:singleton] && resource_base.respond_to?(name)
- resource_base.send(name)
+ if @options[:singleton] && parent_resource.respond_to?(name)
+ parent_resource.send(name)
else
@options[:find_by] ? resource_base.send("find_by_#{@options[:find_by]}!", id_param) : resource_base.find(id_param)
end
end
def authorization_action
- parent? ? :read : @params[:action].to_sym
+ parent? ? :show : @params[:action].to_sym
end
def id_param
@@ -155,7 +155,7 @@ def collection_instance
def resource_base
if @options[:through]
if parent_resource
- @options[:singleton] ? parent_resource : parent_resource.send(@options[:through_association] || name.to_s.pluralize)
+ @options[:singleton] ? resource_class : parent_resource.send(@options[:through_association] || name.to_s.pluralize)
elsif @options[:shallow]
resource_class
else
@@ -166,9 +166,13 @@ def resource_base
end
end
+ def parent_name
+ @options[:through] && [@options[:through]].flatten.detect { |i| fetch_parent(i) }
+ end
+
# The object to load this resource through.
def parent_resource
- @options[:through] && [@options[:through]].flatten.map { |i| fetch_parent(i) }.compact.first
+ parent_name && fetch_parent(parent_name)
end
def fetch_parent(name)
View
2  lib/cancan/inherited_resource.rb
@@ -13,7 +13,7 @@ def load_resource_instance
end
def resource_base
- @controller.send :collection
+ @controller.send :end_of_association_chain
end
end
end
View
4 lib/cancan/model_additions.rb
@@ -4,7 +4,7 @@ module CanCan
module ModelAdditions
module ClassMethods
# Returns a scope which fetches only the records that the passed ability
- # can perform a given action on. The action defaults to :read. This
+ # can perform a given action on. The action defaults to :index. This
# is usually called from a controller and passed the +current_ability+.
#
# @articles = Article.accessible_by(current_ability)
@@ -19,7 +19,7 @@ module ClassMethods
# @articles = Article.accessible_by(current_ability, :update)
#
# Here only the articles which the user can update are returned.
- def accessible_by(ability, action = :read)
+ def accessible_by(ability, action = :index)
ability.model_adapter(self, action).database_records
end
end
View
12 spec/cancan/controller_resource_spec.rb
@@ -104,7 +104,7 @@
it "should authorize parent resource in collection action" do
@params[:action] = "index"
@controller.instance_variable_set(:@category, :some_category)
- stub(@controller).authorize!(:read, :some_category) { raise CanCan::AccessDenied }
+ stub(@controller).authorize!(:show, :some_category) { raise CanCan::AccessDenied }
resource = CanCan::ControllerResource.new(@controller, :category, :parent => true)
lambda { resource.authorize_resource }.should raise_error(CanCan::AccessDenied)
end
@@ -268,14 +268,14 @@
@controller.instance_variable_get(:@project).should == :some_project
end
- it "should build record through has_one association with :singleton option" do
+ it "should not build record through has_one association with :singleton option because it can cause it to delete it in the database" do
@params.merge!(:action => "create", :project => {:name => "foobar"})
- category = Object.new
+ category = Category.new
@controller.instance_variable_set(:@category, category)
- stub(category).build_project { |attributes| Project.new(attributes) }
resource = CanCan::ControllerResource.new(@controller, :through => :category, :singleton => true)
resource.load_resource
@controller.instance_variable_get(:@project).name.should == "foobar"
+ @controller.instance_variable_get(:@project).category.should == category
end
it "should find record through has_one association with :singleton and :shallow options" do
@@ -293,10 +293,10 @@
@controller.instance_variable_get(:@project).name.should == "foobar"
end
- it "should only authorize :read action on parent resource" do
+ it "should only authorize :show action on parent resource" do
project = Project.create!
@params.merge!(:action => "new", :project_id => project.id)
- stub(@controller).authorize!(:read, project) { raise CanCan::AccessDenied }
+ stub(@controller).authorize!(:show, project) { raise CanCan::AccessDenied }
resource = CanCan::ControllerResource.new(@controller, :project, :parent => true)
lambda { resource.load_and_authorize_resource }.should raise_error(CanCan::AccessDenied)
end
View
4 spec/cancan/inherited_resource_spec.rb
@@ -32,10 +32,10 @@
@controller.instance_variable_get(:@project).should == :project_resource
end
- it "index should load through @controller.collection" do
+ it "index should load through @controller.end_of_association_chain" do
@params[:action] = "index"
stub(Project).accessible_by(@ability, :index) { :projects }
- stub(@controller).collection { Project }
+ stub(@controller).end_of_association_chain { Project }
CanCan::InheritedResource.new(@controller).load_resource
@controller.instance_variable_get(:@projects).should == :projects
end
View
1  spec/spec_helper.rb
@@ -29,4 +29,5 @@ class Category < SuperModel::Base
class Project < SuperModel::Base
belongs_to :category
+ attr_accessor :category # why doesn't SuperModel do this automatically?
end

Showing you all comments on commits in this comparison.

@flop

Just got a little suprise with this change, before/after_add callbacks on has_many association are not called any more when initializing nested resources with 'new' instead of 'build'
[Edit : Sorry wrong commit, 'new' was already used before but callbacks aren't called anyway ;)]

Something went wrong with that request. Please try again.