From ea5a4fc9039fac5e85a483110ed021e0b9bc51c1 Mon Sep 17 00:00:00 2001 From: Alexey Kuznetsov Date: Sun, 10 Jul 2016 02:25:20 +0300 Subject: [PATCH 1/4] Add opportunity for chain of configuration from initializers to concerns and models. Based on https://github.com/sferik/rails_admin/pull/1781 --- lib/rails_admin/config.rb | 8 +++--- lib/rails_admin/config/lazy_model.rb | 39 ++++++++++++++++++++++++++-- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/lib/rails_admin/config.rb b/lib/rails_admin/config.rb index 03e72acb78..592aa12ff0 100644 --- a/lib/rails_admin/config.rb +++ b/lib/rails_admin/config.rb @@ -226,11 +226,9 @@ def model(entity, &block) end end - if block - @registry[key] = RailsAdmin::Config::LazyModel.new(entity, &block) - else - @registry[key] ||= RailsAdmin::Config::LazyModel.new(entity) - end + @registry[key] ||= RailsAdmin::Config::LazyModel.new(entity) + @registry[key].add_deferred_block(&block) if block + @registry[key] end def default_hidden_fields=(fields) diff --git a/lib/rails_admin/config/lazy_model.rb b/lib/rails_admin/config/lazy_model.rb index efc60fa427..59f57cbe66 100644 --- a/lib/rails_admin/config/lazy_model.rb +++ b/lib/rails_admin/config/lazy_model.rb @@ -5,13 +5,48 @@ module Config class LazyModel < BasicObject def initialize(entity, &block) @entity = entity - @deferred_block = block + @deferred_blocks = [*block] + end + + def add_deferred_block(&block) + @deferred_blocks << block end def target unless @model @model = ::RailsAdmin::Config::Model.new(@entity) - @model.instance_eval(&@deferred_block) if @deferred_block + # When evaluating multiple configuration blocks, the order of + # execution is important. As one would expect (in my opinion), + # options defined within a resource should take precedence over + # more general options defined in an initializer. This way, + # general settings for a number of resources could be specified + # in the initializer, while models could override these settings + # later, if required. + # + # CAVEAT: It cannot be guaranteed that blocks defined in an initializer + # will be loaded (and adde to @deferred_blocks) first. For instance, if + # the initializer references a model class before defining + # a RailsAdmin configuration block, the configuration from the + # resource will get added to @deferred_blocks first: + # + # # app/models/some_model.rb + # class SomeModel + # rails_admin do + # : + # end + # end + # + # # config/initializers/rails_admin.rb + # model = 'SomeModel'.constantize # blocks from SomeModel get loaded + # model.config model do # blocks from initializer gets loaded + # : + # end + # + # Thus, sort all blocks to excute for a resource by Proc.source_path, + # to guarantee that blocks from 'config/initializers' evaluate before + # blocks defined within a model class. + @deferred_blocks.partition { |block| block.source_location.first =~ /config\/initializers/ } + .flatten.each { |block| @model.instance_eval(&block) } end @model end From 2ce4ebc563f5cb1e8db339f7b18b23309c8a993d Mon Sep 17 00:00:00 2001 From: Alexey Kuznetsov Date: Tue, 12 Jul 2016 00:06:27 +0300 Subject: [PATCH 2/4] Correct behavior of LazyModel with multiple blocks. Add specs. --- lib/rails_admin/config/lazy_model.rb | 69 +++++++++++++++------------- spec/rails_admin/config_spec.rb | 42 +++++++++++++++++ 2 files changed, 78 insertions(+), 33 deletions(-) diff --git a/lib/rails_admin/config/lazy_model.rb b/lib/rails_admin/config/lazy_model.rb index 59f57cbe66..956fbd872d 100644 --- a/lib/rails_admin/config/lazy_model.rb +++ b/lib/rails_admin/config/lazy_model.rb @@ -6,6 +6,7 @@ class LazyModel < BasicObject def initialize(entity, &block) @entity = entity @deferred_blocks = [*block] + @existing_blocks = [] end def add_deferred_block(&block) @@ -13,40 +14,42 @@ def add_deferred_block(&block) end def target - unless @model - @model = ::RailsAdmin::Config::Model.new(@entity) - # When evaluating multiple configuration blocks, the order of - # execution is important. As one would expect (in my opinion), - # options defined within a resource should take precedence over - # more general options defined in an initializer. This way, - # general settings for a number of resources could be specified - # in the initializer, while models could override these settings - # later, if required. - # - # CAVEAT: It cannot be guaranteed that blocks defined in an initializer - # will be loaded (and adde to @deferred_blocks) first. For instance, if - # the initializer references a model class before defining - # a RailsAdmin configuration block, the configuration from the - # resource will get added to @deferred_blocks first: - # - # # app/models/some_model.rb - # class SomeModel - # rails_admin do - # : - # end - # end - # - # # config/initializers/rails_admin.rb - # model = 'SomeModel'.constantize # blocks from SomeModel get loaded - # model.config model do # blocks from initializer gets loaded - # : - # end - # - # Thus, sort all blocks to excute for a resource by Proc.source_path, - # to guarantee that blocks from 'config/initializers' evaluate before - # blocks defined within a model class. - @deferred_blocks.partition { |block| block.source_location.first =~ /config\/initializers/ } + @model ||= ::RailsAdmin::Config::Model.new(@entity) + # When evaluating multiple configuration blocks, the order of + # execution is important. As one would expect (in my opinion), + # options defined within a resource should take precedence over + # more general options defined in an initializer. This way, + # general settings for a number of resources could be specified + # in the initializer, while models could override these settings + # later, if required. + # + # CAVEAT: It cannot be guaranteed that blocks defined in an initializer + # will be loaded (and adde to @deferred_blocks) first. For instance, if + # the initializer references a model class before defining + # a RailsAdmin configuration block, the configuration from the + # resource will get added to @deferred_blocks first: + # + # # app/models/some_model.rb + # class SomeModel + # rails_admin do + # : + # end + # end + # + # # config/initializers/rails_admin.rb + # model = 'SomeModel'.constantize # blocks from SomeModel get loaded + # model.config model do # blocks from initializer gets loaded + # : + # end + # + # Thus, sort all blocks to excute for a resource by Proc.source_path, + # to guarantee that blocks from 'config/initializers' evaluate before + # blocks defined within a model class. + unless @deferred_blocks.empty? + @existing_blocks += @deferred_blocks + @existing_blocks.partition { |block| block.source_location.first =~ /config\/initializers/ } .flatten.each { |block| @model.instance_eval(&block) } + @deferred_blocks = [] end @model end diff --git a/spec/rails_admin/config_spec.rb b/spec/rails_admin/config_spec.rb index eaa77a66aa..0acfe3c0a0 100644 --- a/spec/rails_admin/config_spec.rb +++ b/spec/rails_admin/config_spec.rb @@ -270,6 +270,48 @@ class RecursivelyEmbedsMany expect(RailsAdmin.config.parent_controller).to eq 'TestController' end end + + describe '.model' do + let(:fields) { described_class.model(Team).fields } + before do + described_class.model Team do + field :players do + visible false + end + end + end + context 'when model expanded' do + before do + described_class.model(Team) do + field :fans + end + end + it 'execute all passed blocks' do + expect(fields.map(&:name)).to eq %i(players fans) + end + end + context 'when expand redefine behavior' do + before do + described_class.model Team do + field :players + end + end + it 'execute all passed blocks' do + expect(fields.find { |f| f.name == :players }.visible).to be true + end + end + context 'when model expanded in config' do + let(:block) { proc { field :players } } + before do + allow(block).to receive(:source_location) + .and_return(['config/initializers/rails_admin.rb']) + described_class.model(Team, &block) + end + it 'executes first' do + expect(fields.find { |f| f.name == :players }.visible).to be false + end + end + end end module ExampleModule From 9e7715a4778722ccc31026b61dceb58665216c84 Mon Sep 17 00:00:00 2001 From: Alexey Kuznetsov Date: Tue, 12 Jul 2016 22:45:47 +0300 Subject: [PATCH 3/4] Fix spec for CI. --- spec/rails_admin/config_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/rails_admin/config_spec.rb b/spec/rails_admin/config_spec.rb index 0acfe3c0a0..aba1dc8239 100644 --- a/spec/rails_admin/config_spec.rb +++ b/spec/rails_admin/config_spec.rb @@ -287,7 +287,7 @@ class RecursivelyEmbedsMany end end it 'execute all passed blocks' do - expect(fields.map(&:name)).to eq %i(players fans) + expect(fields.map(&:name)).to match_array %i(players fans) end end context 'when expand redefine behavior' do From d324f82cce8edf1107e52836790d7d3d5535e3af Mon Sep 17 00:00:00 2001 From: Alexey Kuznetsov Date: Tue, 12 Jul 2016 22:56:30 +0300 Subject: [PATCH 4/4] Fix rubocop offenses. --- lib/rails_admin/config/lazy_model.rb | 6 ++++-- spec/rails_admin/config_spec.rb | 3 +-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/rails_admin/config/lazy_model.rb b/lib/rails_admin/config/lazy_model.rb index 956fbd872d..dc0d9bfd1f 100644 --- a/lib/rails_admin/config/lazy_model.rb +++ b/lib/rails_admin/config/lazy_model.rb @@ -47,8 +47,10 @@ def target # blocks defined within a model class. unless @deferred_blocks.empty? @existing_blocks += @deferred_blocks - @existing_blocks.partition { |block| block.source_location.first =~ /config\/initializers/ } - .flatten.each { |block| @model.instance_eval(&block) } + @existing_blocks. + partition { |block| block.source_location.first =~ %r{config\/initializers} }. + flatten. + each { |block| @model.instance_eval(&block) } @deferred_blocks = [] end @model diff --git a/spec/rails_admin/config_spec.rb b/spec/rails_admin/config_spec.rb index aba1dc8239..352b012e17 100644 --- a/spec/rails_admin/config_spec.rb +++ b/spec/rails_admin/config_spec.rb @@ -303,8 +303,7 @@ class RecursivelyEmbedsMany context 'when model expanded in config' do let(:block) { proc { field :players } } before do - allow(block).to receive(:source_location) - .and_return(['config/initializers/rails_admin.rb']) + allow(block).to receive(:source_location).and_return(['config/initializers/rails_admin.rb']) described_class.model(Team, &block) end it 'executes first' do