From dd8196e7623a4ca927c471f64931f54637f5ab0a Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Sat, 26 Feb 2011 09:30:04 +0800 Subject: [PATCH] only bulk destroy authorized records --- app/controllers/rails_admin/main_controller.rb | 11 +++++++++-- lib/rails_admin/adapters/active_record.rb | 15 ++++++--------- .../authorization_adapters/cancan_adapter.rb | 2 +- spec/requests/authorization/cancan_spec.rb | 15 +++++++++++++++ 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/app/controllers/rails_admin/main_controller.rb b/app/controllers/rails_admin/main_controller.rb index 43cac7b0d5..1284895547 100644 --- a/app/controllers/rails_admin/main_controller.rb +++ b/app/controllers/rails_admin/main_controller.rb @@ -129,6 +129,8 @@ def destroy end def bulk_delete + @authorization_adapter.authorize(:bulk_delete, @abstract_model) if @authorization_adapter + @page_name = t("admin.actions.delete").capitalize + " " + @model_config.list.label.downcase @page_type = @abstract_model.pretty_name.downcase @@ -136,7 +138,10 @@ def bulk_delete end def bulk_destroy - @destroyed_objects = @abstract_model.destroy(params[:bulk_ids]) + @authorization_adapter.authorize(:bulk_destroy, @abstract_model) if @authorization_adapter + + scope = @authorization_adapter && @authorization_adapter.query(params[:action].to_sym, @abstract_model) + @destroyed_objects = @abstract_model.destroy(params[:bulk_ids], scope) @destroyed_objects.each do |object| message = "Destroyed #{@model_config.list.with(:object => object).object_label}" @@ -161,8 +166,10 @@ def handle_error(e) private def get_bulk_objects + scope = @authorization_adapter && @authorization_adapter.query(params[:action].to_sym, @abstract_model) @bulk_ids = params[:bulk_ids] - @bulk_objects = @abstract_model.get_bulk(@bulk_ids) + @bulk_objects = @abstract_model.get_bulk(@bulk_ids, scope) + not_found unless @bulk_objects end diff --git a/lib/rails_admin/adapters/active_record.rb b/lib/rails_admin/adapters/active_record.rb index ddd9f59dc0..2a01c4e82e 100644 --- a/lib/rails_admin/adapters/active_record.rb +++ b/lib/rails_admin/adapters/active_record.rb @@ -11,15 +11,11 @@ def get(id) else nil end - # TODO: ActiveRecord::Base.find_by_id will never raise RecordNotFound, will it? - rescue ActiveRecord::RecordNotFound - nil end - def get_bulk(ids) - model.find(ids) - rescue ActiveRecord::RecordNotFound - nil + def get_bulk(ids, scope = nil) + scope ||= model + scope.find_all_by_id(ids) end def count(options = {}, scope = nil) @@ -59,8 +55,9 @@ def new(params = {}) RailsAdmin::AbstractObject.new(model.new) end - def destroy(ids) - model.destroy(ids) + def destroy(ids, scope = nil) + scope ||= model + scope.destroy_all(:id => ids) end def destroy_all! diff --git a/lib/rails_admin/authorization_adapters/cancan_adapter.rb b/lib/rails_admin/authorization_adapters/cancan_adapter.rb index ccc6ebb2ab..0ede217d44 100644 --- a/lib/rails_admin/authorization_adapters/cancan_adapter.rb +++ b/lib/rails_admin/authorization_adapters/cancan_adapter.rb @@ -34,7 +34,7 @@ def translate_action(action) case action when :index then nil # we don't want to do extra action authorization for dashboard when :list then :index - when :delete then :destroy + when :delete, :bulk_delete, :bulk_destroy then :destroy else action end end diff --git a/spec/requests/authorization/cancan_spec.rb b/spec/requests/authorization/cancan_spec.rb index e8d5b13fd9..100573a5d2 100644 --- a/spec/requests/authorization/cancan_spec.rb +++ b/spec/requests/authorization/cancan_spec.rb @@ -158,6 +158,21 @@ def initialize(user) }.should raise_error(CanCan::AccessDenied) end + it "GET /admin/player/bulk_delete should render and destroy records which are authorized to" do + active_player = RailsAdmin::AbstractModel.new("Player").create(:team_id => rand(99999), :number => 32, :name => "Leonardo", :retired => false) + retired_player = RailsAdmin::AbstractModel.new("Player").create(:team_id => rand(99999), :number => 42, :name => "Splinter", :retired => true) + + @delete_ids = [active_player, retired_player].map(&:id) + get rails_admin_bulk_delete_path(:model_name => "player", :bulk_ids => @delete_ids) + + response.body.should contain("Leonardo") + response.body.should_not contain("Splinter") + click_button "Yes, I'm sure" + + Player.exists?(active_player.id).should be_false + Player.exists?(retired_player.id).should be_true + end + end # TODO: Authorize bulk_delete and bulk_destroy actions