From 951b70878cb007d54b4a7aeadd708d4c7668727b Mon Sep 17 00:00:00 2001 From: Mitsuhiro Shibuya Date: Sun, 14 Aug 2022 16:28:40 +0900 Subject: [PATCH] Add block-style DSL support for extension adapters --- lib/rails_admin/config.rb | 4 +- .../cancancan/authorization_adapter.rb | 23 ++++++-- .../paper_trail/auditing_adapter.rb | 52 ++++++++++++------- ...aper_trail_test_with_custom_association.rb | 4 -- spec/dummy_app/app/active_record/trail.rb | 5 ++ ...aper_trail_spec.rb => paper_trail_spec.rb} | 4 +- .../authorization/cancancan_spec.rb | 6 ++- .../cancancan/authorization_adapter_spec.rb | 29 +++++++++++ .../paper_trail/auditing_adapter_spec.rb | 27 +++++----- .../paper_trail/version_proxy_spec.rb | 23 ++++++++ 10 files changed, 133 insertions(+), 44 deletions(-) create mode 100644 spec/dummy_app/app/active_record/trail.rb rename spec/integration/auditing/{rails_admin_paper_trail_spec.rb => paper_trail_spec.rb} (98%) create mode 100644 spec/rails_admin/extentions/cancancan/authorization_adapter_spec.rb create mode 100644 spec/rails_admin/extentions/paper_trail/version_proxy_spec.rb diff --git a/lib/rails_admin/config.rb b/lib/rails_admin/config.rb index 119665435b..6056953ab9 100644 --- a/lib/rails_admin/config.rb +++ b/lib/rails_admin/config.rb @@ -119,7 +119,7 @@ def audit_with(*args, &block) klass = RailsAdmin::AUDITING_ADAPTERS[extension] klass.setup if klass.respond_to? :setup @audit = proc do - @auditing_adapter = klass.new(*([self] + args).compact) + @auditing_adapter = klass.new(*([self] + args).compact, &block) end elsif block @audit = block @@ -156,7 +156,7 @@ def authorize_with(*args, &block) klass = RailsAdmin::AUTHORIZATION_ADAPTERS[extension] klass.setup if klass.respond_to? :setup @authorize = proc do - @authorization_adapter = klass.new(*([self] + args).compact) + @authorization_adapter = klass.new(*([self] + args).compact, &block) end elsif block @authorize = block diff --git a/lib/rails_admin/extensions/cancancan/authorization_adapter.rb b/lib/rails_admin/extensions/cancancan/authorization_adapter.rb index bc395b72fd..86c1befb68 100644 --- a/lib/rails_admin/extensions/cancancan/authorization_adapter.rb +++ b/lib/rails_admin/extensions/cancancan/authorization_adapter.rb @@ -9,18 +9,33 @@ module ControllerExtension def current_ability # use _current_user instead of default current_user so it works with # whatever current user method is defined with RailsAdmin - @current_ability ||= @ability.new(_current_user) + @current_ability ||= ability_class.new(_current_user) end end + include RailsAdmin::Config::Configurable + + def self.setup + RailsAdmin::Extensions::ControllerExtension.include ControllerExtension + end + # See the +authorize_with+ config method for where the initialization happens. - def initialize(controller, ability = ::Ability) + def initialize(controller, ability = nil, &block) @controller = controller - @controller.instance_variable_set '@ability', ability - @controller.extend ControllerExtension + ability_class { ability } if ability + instance_eval(&block) if block + + adapter = self + ControllerExtension.define_method(:ability_class) do + adapter.ability_class + end @controller.current_ability.authorize! :access, :rails_admin end + register_instance_option :ability_class do + Ability + end + # This method is called in every controller action and should raise an exception # when the authorization fails. The first argument is the name of the controller # action as a symbol (:create, :bulk_delete, etc.). The second argument is the diff --git a/lib/rails_admin/extensions/paper_trail/auditing_adapter.rb b/lib/rails_admin/extensions/paper_trail/auditing_adapter.rb index b7baa1015b..030eb4d3b7 100644 --- a/lib/rails_admin/extensions/paper_trail/auditing_adapter.rb +++ b/lib/rails_admin/extensions/paper_trail/auditing_adapter.rb @@ -51,43 +51,59 @@ class AuditingAdapter created_at: :created_at, message: :event, }.freeze + E_USER_CLASS_NOT_SET = <<~ERROR + Please set up PaperTrail's user class explicitly. + + config.audit_with :paper_trail do + user_class { User } + end + ERROR E_VERSION_MODEL_NOT_SET = <<~ERROR Please set up PaperTrail's version model explicitly. - config.audit_with :paper_trail, 'User', 'PaperTrail::Version' + config.audit_with :paper_trail do + version_class { PaperTrail::Version } + end If you have configured a model to use a custom version class (https://github.com/paper-trail-gem/paper_trail#6a-custom-version-classes) - that configuration will take precedence over what you specify in - `audit_with`. + that configuration will take precedence over what you specify in `audit_with`. ERROR + include RailsAdmin::Config::Configurable + def self.setup raise 'PaperTrail not found' unless defined?(::PaperTrail) RailsAdmin::Extensions::ControllerExtension.include ControllerExtension end - def initialize(controller, user_class = 'User', version_class = '::Version') + def initialize(controller, user_class_name = nil, version_class_name = nil, &block) @controller = controller @controller&.send(:set_paper_trail_whodunnit) - begin - @user_class = user_class.to_s.constantize - rescue NameError - raise "Please set up Papertrail's user model explicitly. Ex: config.audit_with :paper_trail, 'User'" - end - begin - @version_class = version_class.to_s.constantize - rescue NameError - raise E_VERSION_MODEL_NOT_SET - end + user_class { user_class_name.to_s.constantize } if user_class_name + version_class { version_class_name.to_s.constantize } if version_class_name + + instance_eval(&block) if block + end + + register_instance_option :user_class do + User + rescue NameError + raise E_USER_CLASS_NOT_SET + end + + register_instance_option :version_class do + PaperTrail::Version + rescue NameError + raise E_VERSION_MODEL_NOT_SET end def latest(count = 100) - @version_class. + version_class. order(id: :desc).includes(:item).limit(count). - collect { |version| VersionProxy.new(version, @user_class) } + collect { |version| VersionProxy.new(version, user_class) } end def delete_object(_object, _model, _user) @@ -133,7 +149,7 @@ def listing_for_model_or_object(model, object, query, sort, sort_reverse, all, p current_page, ).per(per_page) versions.each do |version| - paginated_proxies << VersionProxy.new(version, @user_class) + paginated_proxies << VersionProxy.new(version, user_class) end paginated_proxies end @@ -159,7 +175,7 @@ def versions_for_model(model) # has_paper_trail versions: { class_name: 'MyVersion' } # ``` def version_class_for(model) - model.paper_trail_options.dig(:versions, :class_name).try(:constantize) || @version_class + model.paper_trail.version_class end end end diff --git a/spec/dummy_app/app/active_record/paper_trail_test_with_custom_association.rb b/spec/dummy_app/app/active_record/paper_trail_test_with_custom_association.rb index 94eb0ce8fd..f339135b11 100644 --- a/spec/dummy_app/app/active_record/paper_trail_test_with_custom_association.rb +++ b/spec/dummy_app/app/active_record/paper_trail_test_with_custom_association.rb @@ -1,9 +1,5 @@ # frozen_string_literal: true -class Trail < PaperTrail::Version - self.table_name = :custom_versions -end - class PaperTrailTestWithCustomAssociation < ActiveRecord::Base self.table_name = :paper_trail_tests has_paper_trail versions: {class_name: 'Trail'} diff --git a/spec/dummy_app/app/active_record/trail.rb b/spec/dummy_app/app/active_record/trail.rb new file mode 100644 index 0000000000..87ccdb7bb5 --- /dev/null +++ b/spec/dummy_app/app/active_record/trail.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class Trail < PaperTrail::Version + self.table_name = :custom_versions +end diff --git a/spec/integration/auditing/rails_admin_paper_trail_spec.rb b/spec/integration/auditing/paper_trail_spec.rb similarity index 98% rename from spec/integration/auditing/rails_admin_paper_trail_spec.rb rename to spec/integration/auditing/paper_trail_spec.rb index c263bda09b..7e2d4cb29e 100644 --- a/spec/integration/auditing/rails_admin_paper_trail_spec.rb +++ b/spec/integration/auditing/paper_trail_spec.rb @@ -5,7 +5,7 @@ RSpec.describe 'RailsAdmin PaperTrail auditing', active_record: true do before(:each) do RailsAdmin.config do |config| - config.audit_with :paper_trail, 'User', 'PaperTrail::Version' + config.audit_with :paper_trail end end @@ -59,7 +59,7 @@ describe 'model history fetch with auditing adapter' do before(:all) do - @adapter = RailsAdmin::Extensions::PaperTrail::AuditingAdapter.new(nil, 'User', 'PaperTrail::Version') + @adapter = RailsAdmin::Extensions::PaperTrail::AuditingAdapter.new(nil) end it 'fetches on page of history' do diff --git a/spec/integration/authorization/cancancan_spec.rb b/spec/integration/authorization/cancancan_spec.rb index d4980357af..c593976ab1 100644 --- a/spec/integration/authorization/cancancan_spec.rb +++ b/spec/integration/authorization/cancancan_spec.rb @@ -310,7 +310,11 @@ def initialize(user) describe 'with a custom admin ability' do before do - RailsAdmin.config { |c| c.authorize_with :cancancan, AdminAbility } + RailsAdmin.config do |c| + c.authorize_with :cancancan do + ability_class { AdminAbility } + end + end @user = FactoryBot.create :user login_as @user end diff --git a/spec/rails_admin/extentions/cancancan/authorization_adapter_spec.rb b/spec/rails_admin/extentions/cancancan/authorization_adapter_spec.rb new file mode 100644 index 0000000000..f4b11635f1 --- /dev/null +++ b/spec/rails_admin/extentions/cancancan/authorization_adapter_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RailsAdmin::Extensions::CanCanCan::AuthorizationAdapter do + let(:user) { double } + let(:controller) { double(_current_user: user, current_ability: MyAbility.new(user)) } + + class MyAbility + include CanCan::Ability + def initialize(_user) + can :access, :rails_admin + can :manage, :all + end + end + + describe '#initialize' do + it 'accepts the ability class as an argument' do + expect(described_class.new(controller, MyAbility).ability_class).to eq MyAbility + end + + it 'supports block DSL' do + adapter = described_class.new(controller) do + ability_class MyAbility + end + expect(adapter.ability_class).to eq MyAbility + end + end +end diff --git a/spec/rails_admin/extentions/paper_trail/auditing_adapter_spec.rb b/spec/rails_admin/extentions/paper_trail/auditing_adapter_spec.rb index 613c1da199..b0c9c54c57 100644 --- a/spec/rails_admin/extentions/paper_trail/auditing_adapter_spec.rb +++ b/spec/rails_admin/extentions/paper_trail/auditing_adapter_spec.rb @@ -2,22 +2,23 @@ require 'spec_helper' -RSpec.describe RailsAdmin::Extensions::PaperTrail::VersionProxy do - describe '#username' do - subject { described_class.new(version, user_class).username } +RSpec.describe RailsAdmin::Extensions::PaperTrail::AuditingAdapter, active_record: true do + let(:controller) { double(set_paper_trail_whodunnit: nil) } - let(:version) { double(whodunnit: :the_user) } - let(:user_class) { double(find: user) } - - context 'when found user has email' do - let(:user) { double(email: :mail) } - it { is_expected.to eq(:mail) } + describe '#initialize' do + it 'accepts the user and version classes as arguments' do + adapter = described_class.new(controller, User::Confirmed, Trail) + expect(adapter.user_class).to eq User::Confirmed + expect(adapter.version_class).to eq Trail end - context 'when found user does not have email' do - let(:user) { double } # no email method - - it { is_expected.to eq(:the_user) } + it 'supports block DSL' do + adapter = described_class.new(controller) do + user_class User::Confirmed + version_class Trail + end + expect(adapter.user_class).to eq User::Confirmed + expect(adapter.version_class).to eq Trail end end end diff --git a/spec/rails_admin/extentions/paper_trail/version_proxy_spec.rb b/spec/rails_admin/extentions/paper_trail/version_proxy_spec.rb new file mode 100644 index 0000000000..98e95b5704 --- /dev/null +++ b/spec/rails_admin/extentions/paper_trail/version_proxy_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RailsAdmin::Extensions::PaperTrail::VersionProxy, active_record: true do + describe '#username' do + subject { described_class.new(version, user_class).username } + + let(:version) { double(whodunnit: :the_user) } + let(:user_class) { double(find: user) } + + context 'when found user has email' do + let(:user) { double(email: :mail) } + it { is_expected.to eq(:mail) } + end + + context 'when found user does not have email' do + let(:user) { double } # no email method + + it { is_expected.to eq(:the_user) } + end + end +end