From 572dbf1ffafcdc30d41aa2146b4a49ffb8b1c32d Mon Sep 17 00:00:00 2001 From: Greg Bell Date: Sat, 24 Dec 2011 06:39:54 -0800 Subject: [PATCH] ActiveAdmin::Resource looks up classes at runtime using their name To deal with the reloading issues in #870, we now store reference to the resource class as a string and constantize it each time we need it. Also added a new cucumber profile called "class-reloading" which does not cache classes. Since Active Admin should always work as expected in development with Rails reloading, we should continue to grow scenarios that include the '@requires-reloading' tag. --- cucumber.yml | 5 ++- features/development_reloading.feature | 19 +++++++++ .../step_definitions/additional_web_steps.rb | 3 ++ .../step_definitions/configuration_steps.rb | 40 +++++++++++++++++++ features/step_definitions/flash_steps.rb | 10 ++++- lib/active_admin/resource.rb | 13 ++++-- lib/active_admin/resource_controller.rb | 11 +++++ .../resource_class_methods.rb | 24 +++++++++++ spec/spec_helper.rb | 4 ++ spec/support/rails_template.rb | 3 ++ .../templates/cucumber_with_reloading.rb | 5 +++ spec/unit/namespace/register_resource_spec.rb | 1 + spec/unit/resource_spec.rb | 8 ++-- tasks/test.rake | 7 +++- 14 files changed, 140 insertions(+), 13 deletions(-) create mode 100644 features/development_reloading.feature create mode 100644 lib/active_admin/resource_controller/resource_class_methods.rb create mode 100644 spec/support/templates/cucumber_with_reloading.rb diff --git a/cucumber.yml b/cucumber.yml index 30f61d9c989..0e47b81ab1b 100644 --- a/cucumber.yml +++ b/cucumber.yml @@ -1,2 +1,3 @@ -default: --format 'progress' --require features/support/env.rb --require features/step_definitions features -wip: --format 'progress' --require features/support/env.rb --require features/step_definitions features --tags @wip:3 --wip features \ No newline at end of file +default: --format 'progress' --require features/support/env.rb --require features/step_definitions features --tags ~@requires-reloading +wip: --format 'progress' --require features/support/env.rb --require features/step_definitions features --tags @wip:3 --wip features +class-reloading: RAILS_ENV=cucumber_with_reloading --format 'progress' --require features/support/env.rb --require features/step_definitions features --tags @requires-reloading diff --git a/features/development_reloading.feature b/features/development_reloading.feature new file mode 100644 index 00000000000..966179e679f --- /dev/null +++ b/features/development_reloading.feature @@ -0,0 +1,19 @@ +Feature: Development Reloading + + In order to quickly develop applications + As a developer + I want the application to reload itself in development + + @requires-reloading + Scenario: Reloading an updated model that a resource points to + Given a configuration of: + """ + ActiveAdmin.register Post + """ + And I am logged in + And I create a new post with the title "" + Then I should see a successful create flash + Given I add "validates_presence_of :title" to the "post" model + And I create a new post with the title "" + Then I should not see a successful create flash + And I should see a validation error "can't be blank" diff --git a/features/step_definitions/additional_web_steps.rb b/features/step_definitions/additional_web_steps.rb index 017c5bf2d17..09b5ccfbb7f 100644 --- a/features/step_definitions/additional_web_steps.rb +++ b/features/step_definitions/additional_web_steps.rb @@ -80,3 +80,6 @@ page.should have_css("#active_admin_content", :text => content) end +Then /^I should see a validation error "([^"]*)"$/ do |error_message| + page.should have_css(".inline-errors", :text => error_message) +end diff --git a/features/step_definitions/configuration_steps.rb b/features/step_definitions/configuration_steps.rb index dcd7634eead..1cc8e205a2a 100644 --- a/features/step_definitions/configuration_steps.rb +++ b/features/step_definitions/configuration_steps.rb @@ -9,8 +9,35 @@ def load_active_admin_configuration(configuration_content) end +module ActiveAdminContentsRollback + + def self.recorded_files + @files ||= {} + end + + # Records the contents of a file the first time we are + # about to change it + def self.record(filename) + recorded_files[filename] ||= File.read(filename) + end + + # Rolls the recorded files back to their original states + def self.rollback! + recorded_files.each do |filename, contents| + File.open(filename, "w+") do |f| + f << contents + end + end + end + +end + World(ActiveAdminReloading) +After do + ActiveAdminContentsRollback.rollback! +end + Given /^a configuration of:$/ do |configuration_content| load_active_admin_configuration(configuration_content) ActiveAdmin.application.namespaces.values.each{|n| n.load_menu! } @@ -37,3 +64,16 @@ def load_active_admin_configuration(configuration_content) FileUtils.mkdir_p File.dirname(filepath) File.open(filepath, 'w+'){|f| f << contents } end + +Given /^I add "([^"]*)" to the "([^"]*)" model$/ do |code, model_name| + filename = File.join(Rails.root, "app", "models", "#{model_name}.rb") + ActiveAdminContentsRollback.record(filename) + + # Update the file + contents = File.read(filename) + File.open(filename, "w+") do |f| + f << contents.gsub(/^(class .+)$/, "\\1\n #{code}\n") + end + + ActiveSupport::Dependencies.clear +end diff --git a/features/step_definitions/flash_steps.rb b/features/step_definitions/flash_steps.rb index 9572dfedc30..cdb9c30e428 100644 --- a/features/step_definitions/flash_steps.rb +++ b/features/step_definitions/flash_steps.rb @@ -1,3 +1,11 @@ Then /^I should see a flash with "([^"]*)"$/ do |text| - Then %{I should see "#{text}"} + page.should have_content(text) +end + +Then /^I should see a successful create flash$/ do + page.should have_css('div.flash_notice', :text => /was successfully created/) +end + +Then /^I should not see a successful create flash$/ do + page.should_not have_css('div.flash_notice', :text => /was successfully created/) end diff --git a/lib/active_admin/resource.rb b/lib/active_admin/resource.rb index 8c19fe1edb6..c73a9064c85 100644 --- a/lib/active_admin/resource.rb +++ b/lib/active_admin/resource.rb @@ -25,9 +25,8 @@ class Resource # The namespace this config belongs to attr_reader :namespace - # The class this resource wraps. If you register the Post model, Resource#resource - # will point to the Post class - attr_reader :resource_class + # The name of the resource class + attr_reader :resource_class_name # An array of member actions defined for this resource attr_reader :member_actions @@ -50,7 +49,7 @@ class Resource module Base def initialize(namespace, resource_class, options = {}) @namespace = namespace - @resource_class = resource_class + @resource_class_name = "::#{resource_class.name}" @options = default_options.merge(options) @sort_order = @options[:sort_order] @member_actions, @collection_actions = [], [] @@ -66,6 +65,12 @@ def initialize(namespace, resource_class, options = {}) include Sidebars include Menu + # The class this resource wraps. If you register the Post model, Resource#resource_class + # will point to the Post class + def resource_class + ActiveSupport::Dependencies.constantize(resource_class_name) + end + def resource_table_name resource_class.quoted_table_name end diff --git a/lib/active_admin/resource_controller.rb b/lib/active_admin/resource_controller.rb index 1708eed8eb3..ef85ad9fb56 100644 --- a/lib/active_admin/resource_controller.rb +++ b/lib/active_admin/resource_controller.rb @@ -5,6 +5,7 @@ require 'active_admin/resource_controller/collection' require 'active_admin/resource_controller/filters' require 'active_admin/resource_controller/scoping' +require 'active_admin/resource_controller/resource_class_methods' module ActiveAdmin # All Resources Controller inherits from this controller. @@ -23,15 +24,25 @@ class ResourceController < BaseController include Collection include Filters include Scoping + extend ResourceClassMethods class << self def active_admin_config=(config) @active_admin_config = config + defaults :resource_class => config.resource_class, :route_prefix => config.route_prefix, :instance_name => config.underscored_resource_name end + # Inherited Resources uses the inherited(base) hook method to + # add in the Base.resource_class class method. To override it, we + # need to install our resource_class method each time we're inherited from. + def inherited(base) + super(base) + base.override_resource_class_methods! + end + public :belongs_to end diff --git a/lib/active_admin/resource_controller/resource_class_methods.rb b/lib/active_admin/resource_controller/resource_class_methods.rb new file mode 100644 index 00000000000..d65e7ec8081 --- /dev/null +++ b/lib/active_admin/resource_controller/resource_class_methods.rb @@ -0,0 +1,24 @@ +module ActiveAdmin + class ResourceController < BaseController + module ResourceClassMethods + + # Override the default resource_class class and instance + # methods to only return the class defined in the instance + # of ActiveAdmin::Resource + def override_resource_class_methods! + self.class_eval do + def self.resource_class=(klass); end + + def self.resource_class + @active_admin_config ? @active_admin_config.resource_class : nil + end + + def resource_class + self.class.resource_class + end + end + end + + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index a29764c3292..00b0bc2af12 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -90,6 +90,10 @@ def mock_action_view(assigns = {}) end alias_method :action_view, :mock_action_view + # A mock resource to register + class MockResource + end + end ENV['RAILS_ENV'] = 'test' diff --git a/spec/support/rails_template.rb b/spec/support/rails_template.rb index 902298611e1..888ede0f143 100644 --- a/spec/support/rails_template.rb +++ b/spec/support/rails_template.rb @@ -2,8 +2,11 @@ # Create a cucumber database and environment copy_file File.expand_path('../templates/cucumber.rb', __FILE__), "config/environments/cucumber.rb" +copy_file File.expand_path('../templates/cucumber_with_reloading.rb', __FILE__), "config/environments/cucumber_with_reloading.rb" + gsub_file 'config/database.yml', /^test:.*\n/, "test: &test\n" gsub_file 'config/database.yml', /\z/, "\ncucumber:\n <<: *test\n database: db/cucumber.sqlite3" +gsub_file 'config/database.yml', /\z/, "\ncucumber_with_reloading:\n <<: *test\n database: db/cucumber.sqlite3" # Generate some test models generate :model, "post title:string body:text published_at:datetime author_id:integer category_id:integer" diff --git a/spec/support/templates/cucumber_with_reloading.rb b/spec/support/templates/cucumber_with_reloading.rb new file mode 100644 index 00000000000..e9b2592901a --- /dev/null +++ b/spec/support/templates/cucumber_with_reloading.rb @@ -0,0 +1,5 @@ +require File.expand_path('config/environments/cucumber', Rails.root) + +Rails.application.class.configure do + config.cache_classes = false +end diff --git a/spec/unit/namespace/register_resource_spec.rb b/spec/unit/namespace/register_resource_spec.rb index 33fa62b47f3..a57bcec74dc 100644 --- a/spec/unit/namespace/register_resource_spec.rb +++ b/spec/unit/namespace/register_resource_spec.rb @@ -52,6 +52,7 @@ module ::Mock; class Resource; def self.has_many(arg1, arg2); end; end; end namespace.load_menu! namespace.menu["Mock Resources"].should be_an_instance_of(ActiveAdmin::MenuItem) end + it "should use the resource as the model in the controller" do Admin::MockResourcesController.resource_class.should == Mock::Resource end diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb index 1326c0dc011..e04dc236323 100644 --- a/spec/unit/resource_spec.rb +++ b/spec/unit/resource_spec.rb @@ -200,17 +200,15 @@ class ::News; def self.has_many(*); end end context "when resource class responds to primary_key" do it "should sort by primary key desc by default" do - mock_resource = mock - mock_resource.should_receive(:primary_key).and_return("pk") - config = Resource.new(namespace, mock_resource) + MockResource.should_receive(:primary_key).and_return("pk") + config = Resource.new(namespace, MockResource) config.sort_order.should == "pk_desc" end end context "when resource class does not respond to primary_key" do it "should default to id" do - mock_resource = mock - config = Resource.new(namespace, mock_resource) + config = Resource.new(namespace, MockResource) config.sort_order.should == "id_desc" end end diff --git a/tasks/test.rake b/tasks/test.rake index ec43831810c..6b34ee82dc3 100644 --- a/tasks/test.rake +++ b/tasks/test.rake @@ -7,7 +7,7 @@ end # Run specs and cukes desc "Run the full suite" -task :test => ['spec:unit', 'spec:integration', 'cucumber'] +task :test => ['spec:unit', 'spec:integration', 'cucumber', 'cucumber:class_reloading'] namespace :test do @@ -28,6 +28,7 @@ namespace :test do cmd "export RAILS=#{version} && bundle exec rspec spec/unit" cmd "export RAILS=#{version} && bundle exec rspec spec/integration" cmd "export RAILS=#{version} && bundle exec cucumber features" + cmd "export RAILS=#{version} && bundle exec cucumber -p class-reloading features" end cmd "./script/use_rails #{current_version}" if current_version end @@ -65,4 +66,8 @@ namespace :cucumber do t.profile = 'wip' end + Cucumber::Rake::Task.new(:class_reloading, "Run the cucumber scenarios that test reloading") do |t| + t.profile = 'class-reloading' + end + end