From 5d68caefd080a90fdbf0fd82978e9216e54c812e Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Fri, 25 Mar 2011 16:29:04 -0700 Subject: [PATCH] removing skipping feature in ControllerResource for now --- lib/cancan/controller_additions.rb | 2 +- lib/cancan/controller_resource.rb | 41 ++++---- spec/cancan/controller_resource_spec.rb | 130 ++++++++++++------------ spec/cancan/inherited_resource_spec.rb | 2 +- 4 files changed, 84 insertions(+), 91 deletions(-) diff --git a/lib/cancan/controller_additions.rb b/lib/cancan/controller_additions.rb index cd1741e..f692641 100644 --- a/lib/cancan/controller_additions.rb +++ b/lib/cancan/controller_additions.rb @@ -261,7 +261,7 @@ module CanCan break if options[:if] && !controller.send(options[:if]) break if options[:unless] && controller.send(options[:unless]) unless controller.current_ability.fully_authorized? controller.params[:action], controller.params[:controller] - raise CanCan::InsufficientAuthorizationCheck, "Authorization check is not sufficient for this action. This is probably because you have a conditions or attributes defined in Ability and are not checking for them in the action." + raise CanCan::InsufficientAuthorizationCheck, "Authorization check is not sufficient for this action. This is probably because you have conditions or attributes defined in Ability and are not checking for them in the action. One way to solve this is adding load_and_authorize_resource to this controller." end end rescue_from(CanCan::Unauthorized, &block) if block diff --git a/lib/cancan/controller_resource.rb b/lib/cancan/controller_resource.rb index 13591a7..9361da0 100644 --- a/lib/cancan/controller_resource.rb +++ b/lib/cancan/controller_resource.rb @@ -16,9 +16,6 @@ module CanCan @params = controller.params @options = args.extract_options! @name = args.first - raise CanCan::ImplementationRemoved, "The :nested option is no longer supported, instead use :through with separate load/authorize call." if @options[:nested] - raise CanCan::ImplementationRemoved, "The :name option is no longer supported, instead pass the name as the first argument." if @options[:name] - raise CanCan::ImplementationRemoved, "The :resource option has been renamed back to :class, use false if no class." if @options[:resource] end def load_and_authorize_resource @@ -27,37 +24,33 @@ module CanCan end def load_resource - unless skip?(:load) - if load_instance? - self.resource_instance ||= load_resource_instance - elsif load_collection? - self.collection_instance ||= load_collection - end + if load_instance? + self.resource_instance ||= load_resource_instance + elsif load_collection? + self.collection_instance ||= load_collection end end def authorize_resource - unless skip?(:authorize) - @controller.authorize!(authorization_action, resource_instance || subject_name_with_parent) - end + @controller.authorize!(authorization_action, resource_instance || subject_name_with_parent) end def parent? @options.has_key?(:parent) ? @options[:parent] : @name && @name != name_from_controller.to_sym end - def skip?(behavior) # This could probably use some refactoring - options = @controller.class.cancan_skipper[behavior][@name] - if options.nil? - false - elsif options == {} - true - elsif options[:except] && ![options[:except]].flatten.include?(@params[:action].to_sym) - true - elsif [options[:only]].flatten.include?(@params[:action].to_sym) - true - end - end + # def skip?(behavior) # This could probably use some refactoring + # options = @controller.class.cancan_skipper[behavior][@name] + # if options.nil? + # false + # elsif options == {} + # true + # elsif options[:except] && ![options[:except]].flatten.include?(@params[:action].to_sym) + # true + # elsif [options[:only]].flatten.include?(@params[:action].to_sym) + # true + # end + # end protected diff --git a/spec/cancan/controller_resource_spec.rb b/spec/cancan/controller_resource_spec.rb index e7e4376..5c5311c 100644 --- a/spec/cancan/controller_resource_spec.rb +++ b/spec/cancan/controller_resource_spec.rb @@ -8,7 +8,7 @@ describe CanCan::ControllerResource do @ability = Ability.new(nil) stub(@controller).params { @params } stub(@controller).current_ability { @ability } - stub(@controller_class).cancan_skipper { {:authorize => {}, :load => {}} } + # stub(@controller_class).cancan_skipper { {:authorize => {}, :load => {}} } end it "should load the resource into an instance variable if params[:id] is specified" do @@ -333,69 +333,69 @@ describe CanCan::ControllerResource do @controller.instance_variable_get(:@project).should == project end - it "should raise ImplementationRemoved when adding :name option" do - lambda { - CanCan::ControllerResource.new(@controller, :name => :foo) - }.should raise_error(CanCan::ImplementationRemoved) - end + # it "should raise ImplementationRemoved when adding :name option" do + # lambda { + # CanCan::ControllerResource.new(@controller, :name => :foo) + # }.should raise_error(CanCan::ImplementationRemoved) + # end + # + # it "should raise ImplementationRemoved exception when specifying :resource option since it is no longer used" do + # lambda { + # CanCan::ControllerResource.new(@controller, :resource => Project) + # }.should raise_error(CanCan::ImplementationRemoved) + # end + # + # it "should raise ImplementationRemoved exception when passing :nested option" do + # lambda { + # CanCan::ControllerResource.new(@controller, :nested => :project) + # }.should raise_error(CanCan::ImplementationRemoved) + # end - it "should raise ImplementationRemoved exception when specifying :resource option since it is no longer used" do - lambda { - CanCan::ControllerResource.new(@controller, :resource => Project) - }.should raise_error(CanCan::ImplementationRemoved) - end - - it "should raise ImplementationRemoved exception when passing :nested option" do - lambda { - CanCan::ControllerResource.new(@controller, :nested => :project) - }.should raise_error(CanCan::ImplementationRemoved) - end - - it "should skip resource behavior for :only actions in array" do - stub(@controller_class).cancan_skipper { {:load => {nil => {:only => [:index, :show]}}} } - @params.merge!(:action => "index") - CanCan::ControllerResource.new(@controller).skip?(:load).should be_true - CanCan::ControllerResource.new(@controller, :some_resource).skip?(:load).should be_false - @params.merge!(:action => "show") - CanCan::ControllerResource.new(@controller).skip?(:load).should be_true - @params.merge!(:action => "other_action") - CanCan::ControllerResource.new(@controller).skip?(:load).should be_false - end - - it "should skip resource behavior for :only one action on resource" do - stub(@controller_class).cancan_skipper { {:authorize => {:project => {:only => :index}}} } - @params.merge!(:action => "index") - CanCan::ControllerResource.new(@controller).skip?(:authorize).should be_false - CanCan::ControllerResource.new(@controller, :project).skip?(:authorize).should be_true - @params.merge!(:action => "other_action") - CanCan::ControllerResource.new(@controller, :project).skip?(:authorize).should be_false - end - - it "should skip resource behavior :except actions in array" do - stub(@controller_class).cancan_skipper { {:load => {nil => {:except => [:index, :show]}}} } - @params.merge!(:action => "index") - CanCan::ControllerResource.new(@controller).skip?(:load).should be_false - @params.merge!(:action => "show") - CanCan::ControllerResource.new(@controller).skip?(:load).should be_false - @params.merge!(:action => "other_action") - CanCan::ControllerResource.new(@controller).skip?(:load).should be_true - CanCan::ControllerResource.new(@controller, :some_resource).skip?(:load).should be_false - end - - it "should skip resource behavior :except one action on resource" do - stub(@controller_class).cancan_skipper { {:authorize => {:project => {:except => :index}}} } - @params.merge!(:action => "index") - CanCan::ControllerResource.new(@controller, :project).skip?(:authorize).should be_false - @params.merge!(:action => "other_action") - CanCan::ControllerResource.new(@controller).skip?(:authorize).should be_false - CanCan::ControllerResource.new(@controller, :project).skip?(:authorize).should be_true - end - - it "should skip loading and authorization" do - stub(@controller_class).cancan_skipper { {:authorize => {nil => {}}, :load => {nil => {}}} } - @params.merge!(:action => "new") - resource = CanCan::ControllerResource.new(@controller) - lambda { resource.load_and_authorize_resource }.should_not raise_error - @controller.instance_variable_get(:@project).should be_nil - end + # it "should skip resource behavior for :only actions in array" do + # stub(@controller_class).cancan_skipper { {:load => {nil => {:only => [:index, :show]}}} } + # @params.merge!(:action => "index") + # CanCan::ControllerResource.new(@controller).skip?(:load).should be_true + # CanCan::ControllerResource.new(@controller, :some_resource).skip?(:load).should be_false + # @params.merge!(:action => "show") + # CanCan::ControllerResource.new(@controller).skip?(:load).should be_true + # @params.merge!(:action => "other_action") + # CanCan::ControllerResource.new(@controller).skip?(:load).should be_false + # end + # + # it "should skip resource behavior for :only one action on resource" do + # stub(@controller_class).cancan_skipper { {:authorize => {:project => {:only => :index}}} } + # @params.merge!(:action => "index") + # CanCan::ControllerResource.new(@controller).skip?(:authorize).should be_false + # CanCan::ControllerResource.new(@controller, :project).skip?(:authorize).should be_true + # @params.merge!(:action => "other_action") + # CanCan::ControllerResource.new(@controller, :project).skip?(:authorize).should be_false + # end + # + # it "should skip resource behavior :except actions in array" do + # stub(@controller_class).cancan_skipper { {:load => {nil => {:except => [:index, :show]}}} } + # @params.merge!(:action => "index") + # CanCan::ControllerResource.new(@controller).skip?(:load).should be_false + # @params.merge!(:action => "show") + # CanCan::ControllerResource.new(@controller).skip?(:load).should be_false + # @params.merge!(:action => "other_action") + # CanCan::ControllerResource.new(@controller).skip?(:load).should be_true + # CanCan::ControllerResource.new(@controller, :some_resource).skip?(:load).should be_false + # end + # + # it "should skip resource behavior :except one action on resource" do + # stub(@controller_class).cancan_skipper { {:authorize => {:project => {:except => :index}}} } + # @params.merge!(:action => "index") + # CanCan::ControllerResource.new(@controller, :project).skip?(:authorize).should be_false + # @params.merge!(:action => "other_action") + # CanCan::ControllerResource.new(@controller).skip?(:authorize).should be_false + # CanCan::ControllerResource.new(@controller, :project).skip?(:authorize).should be_true + # end + # + # it "should skip loading and authorization" do + # stub(@controller_class).cancan_skipper { {:authorize => {nil => {}}, :load => {nil => {}}} } + # @params.merge!(:action => "new") + # resource = CanCan::ControllerResource.new(@controller) + # lambda { resource.load_and_authorize_resource }.should_not raise_error + # @controller.instance_variable_get(:@project).should be_nil + # end end diff --git a/spec/cancan/inherited_resource_spec.rb b/spec/cancan/inherited_resource_spec.rb index dc4cc58..dc829c2 100644 --- a/spec/cancan/inherited_resource_spec.rb +++ b/spec/cancan/inherited_resource_spec.rb @@ -8,7 +8,7 @@ describe CanCan::InheritedResource do @ability = Ability.new(nil) stub(@controller).params { @params } stub(@controller).current_ability { @ability } - stub(@controller_class).cancan_skipper { {:authorize => {}, :load => {}} } + # stub(@controller_class).cancan_skipper { {:authorize => {}, :load => {}} } end it "show should load resource through @controller.resource" do