diff options
-rw-r--r-- | actionpack/lib/action_dispatch/routing/polymorphic_routes.rb | 9 | ||||
-rw-r--r-- | actionpack/test/dispatch/prefix_generation_test.rb | 12 | ||||
-rw-r--r-- | actionpack/test/template/test_test.rb | 2 | ||||
-rw-r--r-- | activerecord/lib/active_record/base.rb | 3 | ||||
-rw-r--r-- | activerecord/lib/active_record/relation.rb | 14 | ||||
-rw-r--r-- | activerecord/test/cases/base_test.rb | 13 | ||||
-rw-r--r-- | activerecord/test/cases/persistence_test.rb | 20 | ||||
-rw-r--r-- | activerecord/test/models/bulb.rb | 5 | ||||
-rw-r--r-- | railties/test/application/assets_test.rb | 14 | ||||
-rw-r--r-- | railties/test/railties/mounted_engine_test.rb | 19 |
10 files changed, 89 insertions, 22 deletions
diff --git a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb index 49aef0bf72..e989a38d8b 100644 --- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb +++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb @@ -124,14 +124,7 @@ module ActionDispatch args.last.kind_of?(Hash) ? args.last.merge!(url_options) : args << url_options end - if proxy - proxy.send(named_route, *args) - else - # we need to use url_for, because polymorphic_url can be used in context of other than - # current routes (e.g. engine's routes). As named routes from engine are not included - # calling engine's named route directly would fail. - url_for _routes.url_helpers.__send__("hash_for_#{named_route}", *args) - end + (proxy || self).send(named_route, *args) end # Returns the path component of a URL for the given record. It uses diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb index b28a058250..93eccaecbd 100644 --- a/actionpack/test/dispatch/prefix_generation_test.rb +++ b/actionpack/test/dispatch/prefix_generation_test.rb @@ -50,7 +50,9 @@ module TestGenerationPrefix scope "/:omg", :omg => "awesome" do mount BlogEngine => "/blog", :as => "blog_engine" end + match "/posts/:id", :to => "outside_engine_generating#post", :as => :post match "/generate", :to => "outside_engine_generating#index" + match "/polymorphic_path_for_app", :to => "outside_engine_generating#polymorphic_path_for_app" match "/polymorphic_path_for_engine", :to => "outside_engine_generating#polymorphic_path_for_engine" match "/polymorphic_with_url_for", :to => "outside_engine_generating#polymorphic_with_url_for" match "/conflicting_url", :to => "outside_engine_generating#conflicting" @@ -101,6 +103,7 @@ module TestGenerationPrefix class ::OutsideEngineGeneratingController < ActionController::Base include BlogEngine.routes.mounted_helpers + include RailsApplication.routes.url_helpers def index render :text => blog_engine.post_path(:id => 1) @@ -110,6 +113,10 @@ module TestGenerationPrefix render :text => blog_engine.polymorphic_path(Post.new) end + def polymorphic_path_for_app + render :text => polymorphic_path(Post.new) + end + def polymorphic_with_url_for render :text => blog_engine.url_for(Post.new) end @@ -201,6 +208,11 @@ module TestGenerationPrefix assert_equal "/awesome/blog/posts/1", last_response.body end + test "polymorphic_path_for_app" do + get "/polymorphic_path_for_app" + assert_equal "/posts/1", last_response.body + end + test "[APP] generating engine's url with url_for(@post)" do get "/polymorphic_with_url_for" assert_equal "http://example.org/awesome/blog/posts/1", last_response.body diff --git a/actionpack/test/template/test_test.rb b/actionpack/test/template/test_test.rb index 3d0bbba435..bf789cd8b7 100644 --- a/actionpack/test/template/test_test.rb +++ b/actionpack/test/template/test_test.rb @@ -39,7 +39,7 @@ class PeopleHelperTest < ActionView::TestCase with_test_route_set do person = mock(:name => "David") person.class.extend ActiveModel::Naming - _routes.url_helpers.expects(:hash_for_mocha_mock_path).with(person).returns("/people/1") + expects(:mocha_mock_path).with(person).returns("/people/1") assert_equal '<a href="/people/1">David</a>', link_to_person(person) end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 2c162a6fc8..4136868b39 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -504,8 +504,7 @@ module ActiveRecord #:nodoc: if attributes.is_a?(Array) attributes.collect { |attr| create(attr, options, &block) } else - object = new(attributes, options) - yield(object) if block_given? + object = new(attributes, options, &block) object.save object end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 2d0861d5c9..fff0ad1b83 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -216,17 +216,13 @@ module ActiveRecord if conditions || options.present? where(conditions).apply_finder_options(options.slice(:limit, :order)).update_all(updates) else - limit = nil - order = [] - # Apply limit and order only if they're both present - if @limit_value.present? == @order_values.present? - limit = arel.limit - order = arel.orders + stmt = arel.compile_update(Arel.sql(@klass.send(:sanitize_sql_for_assignment, updates))) + + if limit = arel.limit + stmt.take limit end - stmt = arel.compile_update(Arel.sql(@klass.send(:sanitize_sql_for_assignment, updates))) - stmt.take limit if limit - stmt.order(*order) + stmt.order(*arel.orders) stmt.key = table[primary_key] @klass.connection.update stmt.to_sql, 'SQL', bind_values end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 8b4e7dd799..f2f5b73626 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -21,6 +21,7 @@ require 'models/parrot' require 'models/person' require 'models/edge' require 'models/joke' +require 'models/bulb' require 'rexml/document' require 'active_support/core_ext/exception' @@ -260,6 +261,18 @@ class BasicsTest < ActiveRecord::TestCase end end + def test_create_after_initialize_without_block + cb = CustomBulb.create(:name => 'Dude') + assert_equal('Dude', cb.name) + assert_equal(true, cb.frickinawesome) + end + + def test_create_after_initialize_with_block + cb = CustomBulb.create {|c| c.name = 'Dude' } + assert_equal('Dude', cb.name) + assert_equal(true, cb.frickinawesome) + end + def test_load topics = Topic.find(:all, :order => 'id') assert_equal(4, topics.size) diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 57d1441128..9cd07fa8a5 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -29,6 +29,26 @@ class PersistencesTest < ActiveRecord::TestCase end end + def test_update_all_doesnt_ignore_order + assert_equal authors(:david).id + 1, authors(:mary).id # make sure there is going to be a duplicate PK error + test_update_with_order_succeeds = lambda do |order| + begin + Author.order(order).update_all('id = id + 1') + rescue ActiveRecord::ActiveRecordError + false + end + end + + if test_update_with_order_succeeds.call('id DESC') + assert !test_update_with_order_succeeds.call('id ASC') # test that this wasn't a fluke and using an incorrect order results in an exception + else + # test that we're failing because the current Arel's engine doesn't support UPDATE ORDER BY queries is using subselects instead + assert_sql(/\AUPDATE .+ \(SELECT .* ORDER BY id DESC\)\Z/i) do + test_update_with_order_succeeds.call('id DESC') + end + end + end + def test_update_all_with_order_and_limit_updates_subset_only author = authors(:david) assert_nothing_raised do diff --git a/activerecord/test/models/bulb.rb b/activerecord/test/models/bulb.rb index efb98b66e7..888afc7604 100644 --- a/activerecord/test/models/bulb.rb +++ b/activerecord/test/models/bulb.rb @@ -33,4 +33,9 @@ class Bulb < ActiveRecord::Base end class CustomBulb < Bulb + after_initialize :set_awesomeness + + def set_awesomeness + self.frickinawesome = true if name == 'Dude' + end end diff --git a/railties/test/application/assets_test.rb b/railties/test/application/assets_test.rb index e1eee71a9e..7fb930bd99 100644 --- a/railties/test/application/assets_test.rb +++ b/railties/test/application/assets_test.rb @@ -8,7 +8,7 @@ module ApplicationTests include Rack::Test::Methods def setup - build_app + build_app(:initializers => true) boot_rails end @@ -46,7 +46,7 @@ module ApplicationTests assert defined?(Uglifier) end - test "assets are compiled properly" do + test "precompile creates the file and gives it the original asset's content" do app_file "app/assets/javascripts/application.js", "alert();" app_file "app/assets/javascripts/foo/application.js", "alert();" @@ -61,6 +61,16 @@ module ApplicationTests end end + test "precompile appends the md5 hash to files referenced with asset_path" do + app_file "app/assets/stylesheets/application.css.erb", "<%= asset_path('rails.png') %>" + + capture(:stdout) do + Dir.chdir(app_path){ `bundle exec rake assets:precompile` } + end + file = Dir["#{app_path}/public/assets/application-*.css"].first + assert_match /\/assets\/rails-([0-z]+)\.png/, File.read(file) + end + test "assets are cleaned up properly" do app_file "public/assets/application.js", "alert();" app_file "public/assets/application.css", "a { color: green; }" diff --git a/railties/test/railties/mounted_engine_test.rb b/railties/test/railties/mounted_engine_test.rb index b793a7401f..94dec405a7 100644 --- a/railties/test/railties/mounted_engine_test.rb +++ b/railties/test/railties/mounted_engine_test.rb @@ -15,10 +15,12 @@ module ApplicationTests app_file 'config/routes.rb', <<-RUBY AppTemplate::Application.routes.draw do + resources :posts match "/engine_route" => "application_generating#engine_route" match "/engine_route_in_view" => "application_generating#engine_route_in_view" match "/url_for_engine_route" => "application_generating#url_for_engine_route" match "/polymorphic_route" => "application_generating#polymorphic_route" + match "/application_polymorphic_path" => "application_generating#application_polymorphic_path" scope "/:user", :user => "anonymous" do mount Blog::Engine => "/blog" end @@ -59,6 +61,7 @@ module ApplicationTests resources :posts match '/generate_application_route', :to => 'posts#generate_application_route' match '/application_route_in_view', :to => 'posts#application_route_in_view' + match '/engine_polymorphic_path', :to => 'posts#engine_polymorphic_path' end RUBY @@ -79,6 +82,10 @@ module ApplicationTests def application_route_in_view render :inline => "<%= main_app.root_path %>" end + + def engine_polymorphic_path + render :text => polymorphic_path(Post.new) + end end end RUBY @@ -100,6 +107,10 @@ module ApplicationTests def polymorphic_route render :text => polymorphic_url([blog, Blog::Post.new]) end + + def application_polymorphic_path + render :text => polymorphic_path(Blog::Post.new) + end end RUBY @@ -172,6 +183,14 @@ module ApplicationTests # test polymorphic routes get "/polymorphic_route" assert_equal "http://example.org/anonymous/blog/posts/44", last_response.body + + # test that correct path is generated for the same polymorphic_path call in an engine + get "/somone/blog/engine_polymorphic_path" + assert_equal "/somone/blog/posts/44", last_response.body + + # and in an application + get "/application_polymorphic_path" + assert_equal "/posts/44", last_response.body end end end |