From 11614bddc015deff7095bdf8bbe9f11e5d81ae1c Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Sun, 1 Aug 2010 15:52:10 -0300 Subject: Fix label form helper to use I18n and html options, without the need of 'nil' text param: Before: f.label :title, nil, :class => 'title' After : f.label :title, :class => 'title' [#5267 state:committed] Signed-off-by: Santiago Pastorino --- actionpack/lib/action_view/helpers/form_helper.rb | 7 ++++--- actionpack/test/template/form_helper_test.rb | 11 +++++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index f8e60faa2a..ebe055bebd 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -215,7 +215,7 @@ module ActionView # ... # <% end %> # - # If your resource has associations defined, for example, you want to add comments + # If your resource has associations defined, for example, you want to add comments # to the post given that the routes are set correctly: # # <%= form_for([@document, @comment]) do |f| %> @@ -583,8 +583,9 @@ module ActionView # 'Accept Terms.' # end def label(object_name, method, content_or_options = nil, options = nil, &block) - if block_given? - options = content_or_options if content_or_options.is_a?(Hash) + content_is_options = content_or_options.is_a?(Hash) + if content_is_options || block_given? + options = content_or_options if content_is_options text = nil else text = content_or_options diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index f248a38ae9..9086a23345 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -120,6 +120,13 @@ class FormHelperTest < ActionView::TestCase I18n.locale = old_locale end + def test_label_with_locales_and_options + old_locale, I18n.locale = I18n.locale, :label + assert_dom_equal('', label(:post, :body, :class => 'post_body')) + ensure + I18n.locale = old_locale + end + def test_label_with_for_attribute_as_symbol assert_dom_equal('', label(:post, :title, nil, :for => "my_for")) end @@ -620,7 +627,7 @@ class FormHelperTest < ActionView::TestCase def test_form_for_with_symbol_object_name form_for(@post, :as => "other_name", :html => { :id => 'create-post' }) do |f| - concat f.label(:title) + concat f.label(:title, :class => 'post_title') concat f.text_field(:title) concat f.text_area(:body) concat f.check_box(:secret) @@ -628,7 +635,7 @@ class FormHelperTest < ActionView::TestCase end expected = whole_form("/posts/123", "create-post", "other_name_edit", :method => "put") do - "" + + "" + "" + "" + "" + -- cgit v1.2.3 From 111234e7c140398dba3bed4936e5d4d03681994c Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 1 Aug 2010 21:56:06 -0300 Subject: Use AS::OrderedHash when trusting in the order of the hash --- actionpack/test/controller/test_test.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/actionpack/test/controller/test_test.rb b/actionpack/test/controller/test_test.rb index 950ad9266f..13c9d9ee38 100644 --- a/actionpack/test/controller/test_test.rb +++ b/actionpack/test/controller/test_test.rb @@ -1,5 +1,6 @@ require 'abstract_unit' require 'controller/fake_controllers' +require 'active_support/ordered_hash' class TestTest < ActionController::TestCase class TestController < ActionController::Base @@ -137,14 +138,14 @@ XML end def test_raw_post_handling - params = {:page => {:name => 'page name'}, 'some key' => 123} + params = ActiveSupport::OrderedHash[:page, {:name => 'page name'}, 'some key', 123] post :render_raw_post, params.dup assert_equal params.to_query, @response.body end def test_body_stream - params = { :page => { :name => 'page name' }, 'some key' => 123 } + params = ActiveSupport::OrderedHash[:page, { :name => 'page name' }, 'some key', 123] post :render_body, params.dup -- cgit v1.2.3 From 3b44b52fb0f09e437645d12d2feb6b6d37a804ee Mon Sep 17 00:00:00 2001 From: "Prashant P. Shah" Date: Fri, 30 Jul 2010 15:27:59 +0530 Subject: Corrected the rake test:units and test:functionals description [#5251 state:committed] Signed-off-by: Santiago Pastorino --- railties/lib/rails/test_unit/testing.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/rails/test_unit/testing.rake b/railties/lib/rails/test_unit/testing.rake index ecd513c2c8..38c14fcd6b 100644 --- a/railties/lib/rails/test_unit/testing.rake +++ b/railties/lib/rails/test_unit/testing.rake @@ -70,7 +70,7 @@ module Kernel end end -desc 'Runs test:unit, test:functional, test:integration together (also available: test:benchmark, test:profile, test:plugins)' +desc 'Runs test:units, test:functionals, test:integration together (also available: test:benchmark, test:profile, test:plugins)' task :test do errors = %w(test:units test:functionals test:integration).collect do |task| begin -- cgit v1.2.3 From c544fcc8eb891e9066bae5eb1478c6823b1f6203 Mon Sep 17 00:00:00 2001 From: rohit Date: Mon, 2 Aug 2010 13:42:35 +0530 Subject: Failing test to check for route file corruption if legacy map parameter is used. [#5263 state:open] --- railties/test/generators/scaffold_generator_test.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/railties/test/generators/scaffold_generator_test.rb b/railties/test/generators/scaffold_generator_test.rb index ea469cb3c8..f12445ae35 100644 --- a/railties/test/generators/scaffold_generator_test.rb +++ b/railties/test/generators/scaffold_generator_test.rb @@ -216,4 +216,19 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase # Stylesheets (should not be removed) assert_file "public/stylesheets/scaffold.css" end + + def test_scaffold_generator_on_revoke_does_not_mutilate_legacy_map_parameter + run_generator + + # Add a |map| parameter to the routes block manually + route_path = File.expand_path("config/routes.rb", destination_root) + content = File.read(route_path).gsub(/\.routes\.draw do/) do |match| + "#{match} |map|" + end + File.open(route_path, "wb") { |file| file.write(content) } + + run_generator ["product_line"], :behavior => :revoke + + assert_file "config/routes.rb", /\.routes\.draw do\s*\|map\|\s*$/ + end end -- cgit v1.2.3 From e6331b1e97d608c46aaadd1814a1e370bdcdd40a Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sat, 31 Jul 2010 22:33:22 -0300 Subject: Makes rails destroy scaffold don't duplicate routes.draw do |map| |map| when using the deprecated syntax [#5263 state:committed] --- railties/lib/rails/generators/actions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/rails/generators/actions.rb b/railties/lib/rails/generators/actions.rb index 2280cc1507..668ef48892 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -275,7 +275,7 @@ module Rails # def route(routing_code) log :route, routing_code - sentinel = /\.routes\.draw do(\s*\|map\|)?\s*$/ + sentinel = /\.routes\.draw do(?:\s*\|map\|)?\s*$/ in_root do inject_into_file 'config/routes.rb', "\n #{routing_code}\n", { :after => sentinel, :verbose => false } -- cgit v1.2.3 From 311ea94f73c95be87f9a474da122719eebee3f98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ku=C5=BAma?= Date: Mon, 2 Aug 2010 11:07:43 +0200 Subject: added failing touch propagation test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activerecord/test/cases/timestamp_test.rb | 17 +++++++++++++++++ activerecord/test/schema/schema.rb | 2 ++ 2 files changed, 19 insertions(+) diff --git a/activerecord/test/cases/timestamp_test.rb b/activerecord/test/cases/timestamp_test.rb index f765540808..e3d12f6214 100644 --- a/activerecord/test/cases/timestamp_test.rb +++ b/activerecord/test/cases/timestamp_test.rb @@ -82,4 +82,21 @@ class TimestampTest < ActiveRecord::TestCase ensure Pet.belongs_to :owner, :touch => true end + + def test_touching_a_record_touches_parent_record_and_grandparent_record + Toy.belongs_to :pet, :touch => true + Pet.belongs_to :owner, :touch => true + + toy = Toy.first + pet = toy.pet + owner = pet.owner + + previously_owner_updated_at = owner.updated_at + + toy.touch + + assert_not_equal previously_owner_updated_at, owner.updated_at + ensure + Toy.belongs_to :pet + end end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index f3fd37cd61..a0e620c2ef 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -398,6 +398,7 @@ ActiveRecord::Schema.define do create_table :pets, :primary_key => :pet_id ,:force => true do |t| t.string :name t.integer :owner_id, :integer + t.timestamps end create_table :pirates, :force => true do |t| @@ -530,6 +531,7 @@ ActiveRecord::Schema.define do create_table :toys, :primary_key => :toy_id ,:force => true do |t| t.string :name t.integer :pet_id, :integer + t.timestamps end create_table :traffic_lights, :force => true do |t| -- cgit v1.2.3 From b613c3cc7b08b00595e33d1b5302cd5d42687d4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 2 Aug 2010 16:16:02 +0200 Subject: Add an internal (private API) after_touch callback. [#5271 state:resolved] --- activerecord/lib/active_record/associations.rb | 1 + activerecord/lib/active_record/callbacks.rb | 8 ++++++-- activerecord/lib/active_record/persistence.rb | 13 +++++++++++++ activerecord/lib/active_record/timestamp.rb | 13 ------------- activerecord/test/cases/timestamp_test.rb | 8 ++++---- 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index f540aa7f25..9663a36edf 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1498,6 +1498,7 @@ module ActiveRecord end end after_save(method_name) + after_touch(method_name) after_destroy(method_name) end diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index 637dac450b..82c45a41b0 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -228,7 +228,7 @@ module ActiveRecord extend ActiveSupport::Concern CALLBACKS = [ - :after_initialize, :after_find, :before_validation, :after_validation, + :after_initialize, :after_find, :after_touch, :before_validation, :after_validation, :before_save, :around_save, :after_save, :before_create, :around_create, :after_create, :before_update, :around_update, :after_update, :before_destroy, :around_destroy, :after_destroy @@ -238,7 +238,7 @@ module ActiveRecord extend ActiveModel::Callbacks include ActiveModel::Validations::Callbacks - define_model_callbacks :initialize, :find, :only => :after + define_model_callbacks :initialize, :find, :touch, :only => :after define_model_callbacks :save, :create, :update, :destroy end @@ -256,6 +256,10 @@ module ActiveRecord _run_destroy_callbacks { super } end + def touch(*) #:nodoc: + _run_touch_callbacks { super } + end + def deprecated_callback_method(symbol) #:nodoc: if respond_to?(symbol, true) ActiveSupport::Deprecation.warn("Overwriting #{symbol} in your models has been deprecated, please use Base##{symbol} :method_name instead") diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 38b91652ee..cbc2220e96 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -218,6 +218,19 @@ module ActiveRecord self end + # Saves the record with the updated_at/on attributes set to the current time. + # Please note that no validation is performed and no callbacks are executed. + # If an attribute name is passed, that attribute is updated along with + # updated_at/on attributes. + # + # Examples: + # + # product.touch # updates updated_at/on + # product.touch(:designed_at) # updates the designed_at attribute and updated_at/on + def touch(attribute = nil) + update_attribute(attribute, current_time_from_proper_timezone) + end + private def create_or_update raise ReadOnlyRecord if readonly? diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb index 92f7a7753d..32b3f03f13 100644 --- a/activerecord/lib/active_record/timestamp.rb +++ b/activerecord/lib/active_record/timestamp.rb @@ -31,19 +31,6 @@ module ActiveRecord class_inheritable_accessor :record_timestamps, :instance_writer => false self.record_timestamps = true end - - # Saves the record with the updated_at/on attributes set to the current time. - # Please note that no validation is performed and no callbacks are executed. - # If an attribute name is passed, that attribute is updated along with - # updated_at/on attributes. - # - # Examples: - # - # product.touch # updates updated_at/on - # product.touch(:designed_at) # updates the designed_at attribute and updated_at/on - def touch(attribute = nil) - update_attribute(attribute, current_time_from_proper_timezone) - end private diff --git a/activerecord/test/cases/timestamp_test.rb b/activerecord/test/cases/timestamp_test.rb index e3d12f6214..06ab7aa9c7 100644 --- a/activerecord/test/cases/timestamp_test.rb +++ b/activerecord/test/cases/timestamp_test.rb @@ -2,9 +2,10 @@ require 'cases/helper' require 'models/developer' require 'models/owner' require 'models/pet' +require 'models/toy' class TimestampTest < ActiveRecord::TestCase - fixtures :developers, :owners, :pets + fixtures :developers, :owners, :pets, :toys def setup @developer = Developer.first @@ -91,11 +92,10 @@ class TimestampTest < ActiveRecord::TestCase pet = toy.pet owner = pet.owner - previously_owner_updated_at = owner.updated_at - + owner.update_attribute(:updated_at, (time = 3.days.ago)) toy.touch - assert_not_equal previously_owner_updated_at, owner.updated_at + assert_not_equal time, owner.updated_at ensure Toy.belongs_to :pet end -- cgit v1.2.3 From 59cf514a5b769257a1538736d91f48ee0900e236 Mon Sep 17 00:00:00 2001 From: Chris Hoffman Date: Fri, 23 Jul 2010 19:09:27 -0500 Subject: Add missing require in ActiveSupport::HashWithIndifferentAccess [#5189 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activesupport/lib/active_support/hash_with_indifferent_access.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index f64f0f44cc..eec5d4cf47 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -1,3 +1,4 @@ +require 'active_support/core_ext/hash/indifferent_access' require 'active_support/core_ext/hash/keys' # This class has dubious semantics and we only have it so that -- cgit v1.2.3 From aeaab06c79a9e3cfa9988270847fa8c6f863570a Mon Sep 17 00:00:00 2001 From: Alex Le Date: Fri, 30 Jul 2010 15:47:26 -0500 Subject: ActiveModel::Errors json serialization to work as Rails 3b4 [#5254 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activemodel/lib/active_model/errors.rb | 4 ++-- .../cases/serializeration/json_serialization_test.rb | 16 +++++++++++++++- activemodel/test/cases/validations_test.rb | 8 +++++--- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index bf93126d27..f39678db83 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -169,9 +169,9 @@ module ActiveModel to_a.to_xml options.reverse_merge(:root => "errors", :skip_types => true) end - # Returns an array as JSON representation for this object. + # Returns an ActiveSupport::OrderedHash that can be used as the JSON representation for this object. def as_json(options=nil) - to_a + self end # Adds +message+ to the error messages on +attribute+, which will be returned on a call to diff --git a/activemodel/test/cases/serializeration/json_serialization_test.rb b/activemodel/test/cases/serializeration/json_serialization_test.rb index 04b50e5bb8..1ac991a8f1 100644 --- a/activemodel/test/cases/serializeration/json_serialization_test.rb +++ b/activemodel/test/cases/serializeration/json_serialization_test.rb @@ -89,7 +89,7 @@ class JsonSerializationTest < ActiveModel::TestCase assert_match %r{"preferences":\{"shows":"anime"\}}, json end - test "methds are called on object" do + test "methods are called on object" do # Define methods on fixture. def @contact.label; "Has cheezburger"; end def @contact.favorite_quote; "Constraints are liberating"; end @@ -102,4 +102,18 @@ class JsonSerializationTest < ActiveModel::TestCase assert_match %r{"label":"Has cheezburger"}, methods_json assert_match %r{"favorite_quote":"Constraints are liberating"}, methods_json end + + test "should return OrderedHash for errors" do + car = Automobile.new + + # run the validation + car.valid? + + hash = ActiveSupport::OrderedHash.new + hash[:make] = "can't be blank" + hash[:model] = "is too short (minimum is 2 characters)" + assert_equal hash.to_json, car.errors.to_json + end + + end diff --git a/activemodel/test/cases/validations_test.rb b/activemodel/test/cases/validations_test.rb index e94d8ce88c..8d6bdeb6a5 100644 --- a/activemodel/test/cases/validations_test.rb +++ b/activemodel/test/cases/validations_test.rb @@ -170,9 +170,11 @@ class ValidationsTest < ActiveModel::TestCase assert_match %r{}, xml assert_match %r{Title can't be blank}, xml assert_match %r{Content can't be blank}, xml - - json = t.errors.to_json - assert_equal t.errors.to_a.to_json, json + + hash = ActiveSupport::OrderedHash.new + hash[:title] = "can't be blank" + hash[:content] = "can't be blank" + assert_equal t.errors.to_json, hash.to_json end def test_validation_order -- cgit v1.2.3 From cdad483dff4fef1b640dc3c750719c325b252f89 Mon Sep 17 00:00:00 2001 From: Fred Wu Date: Wed, 28 Jul 2010 22:55:57 +1000 Subject: Improved how AppGenerator generates the application name. It now detects the current app name whenever possible. This means that renaming the residing directory will not effect the app name generated by AppGenerator. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [#5225 state:resolved] Signed-off-by: José Valim --- .../rails/generators/rails/app/app_generator.rb | 6 +++++- railties/test/generators/app_generator_test.rb | 24 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index 96c49a81bb..dd18588b39 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -356,8 +356,12 @@ module Rails @app_name ||= File.basename(destination_root) end + def defined_app_const_base + Rails.application.class.name.sub(/::Application$/, "") if Rails.application.instance_of?(Rails::Application) + end + def app_const_base - @app_const_base ||= app_name.gsub(/\W/, '_').squeeze('_').camelize + @app_const_base ||= defined_app_const_base || app_name.gsub(/\W/, '_').squeeze('_').camelize end def app_const diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 1e0b3bf4c7..21725a380c 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -106,6 +106,30 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_file "things-43/config/application.rb", /^module Things43$/ end + def test_application_name_is_detected_if_it_exists_and_app_folder_renamed + app_root = File.join(destination_root, "myapp") + app_moved_root = File.join(destination_root, "myapp_moved") + + run_generator [app_root] + + Rails.application.config.root = app_moved_root + Rails.application.class.stubs(:name).returns("Myapp") + Rails.application.stubs(:instance_of?).returns(Rails::Application) + + FileUtils.mv(app_root, app_moved_root) + + # forces the shell to automatically overwrite all files + Thor::Base.shell.send(:attr_accessor, :always_force) + shell = Thor::Base.shell.new + shell.send(:always_force=, true) + + generator = Rails::Generators::AppGenerator.new ["rails"], { :with_dispatchers => true }, + :destination_root => app_moved_root, :shell => shell + generator.send(:app_const) + silence(:stdout){ generator.send(:create_config_files) } + assert_file "myapp_moved/config/environment.rb", /Myapp::Application\.initialize!/ + end + def test_application_names_are_not_singularized run_generator [File.join(destination_root, "hats")] assert_file "hats/config/environment.rb", /Hats::Application\.initialize!/ -- cgit v1.2.3 From 558ee6e95ccd6c2098595f2edfa59e8aa9108167 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 2 Aug 2010 16:40:02 +0200 Subject: Handle edge cases in the previous patch. --- railties/lib/rails/generators/rails/app/app_generator.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index dd18588b39..a90f109844 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -357,7 +357,8 @@ module Rails end def defined_app_const_base - Rails.application.class.name.sub(/::Application$/, "") if Rails.application.instance_of?(Rails::Application) + Rails.respond_to?(:application) && defined?(Rails::Application) && + Rails.application.is_a?(Rails::Application) && Rails.application.class.name.sub(/::Application$/, "") end def app_const_base -- cgit v1.2.3 From f8b53f35b9cbf2a134a7d9184a044ce95764acfa Mon Sep 17 00:00:00 2001 From: Robert Pankowecki Date: Tue, 27 Jul 2010 22:57:27 +0200 Subject: test and fix collection_singular_ids= with string primary keys [#5125 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activerecord/lib/active_record/associations.rb | 4 ++- .../has_many_through_associations_test.rb | 40 +++++++++++++++++++++- activerecord/test/fixtures/subscriptions.yml | 2 +- activerecord/test/models/book.rb | 3 ++ 4 files changed, 46 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 9663a36edf..bd90cfc5d5 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1439,7 +1439,9 @@ module ActiveRecord end redefine_method("#{reflection.name.to_s.singularize}_ids=") do |new_value| - ids = (new_value || []).reject { |nid| nid.blank? }.map(&:to_i) + pk_column = reflection.klass.columns.find{|c| c.name == reflection.klass.primary_key } + ids = (new_value || []).reject { |nid| nid.blank? } + ids.map!{|i| pk_column.type_cast(i)} send("#{reflection.name}=", reflection.klass.find(ids).index_by(&:id).values_at(*ids)) end end diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index e4dd810732..3940e75ad6 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -14,9 +14,14 @@ require 'models/toy' require 'models/contract' require 'models/company' require 'models/developer' +require 'models/subscriber' +require 'models/book' +require 'models/subscription' class HasManyThroughAssociationsTest < ActiveRecord::TestCase - fixtures :posts, :readers, :people, :comments, :authors, :owners, :pets, :toys, :jobs, :references, :companies + fixtures :posts, :readers, :people, :comments, :authors, + :owners, :pets, :toys, :jobs, :references, :companies, + :subscribers, :books, :subscriptions # Dummies to force column loads so query counts are clean. def setup @@ -383,4 +388,37 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase lambda { authors(:david).very_special_comments.delete(authors(:david).very_special_comments.first) }, ].each {|block| assert_raise(ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection, &block) } end + + def test_collection_singular_ids_getter_with_string_primary_keys + book = books(:awdr) + assert_equal 2, book.subscriber_ids.size + assert_equal [subscribers(:first).nick, subscribers(:second).nick].sort, book.subscriber_ids.sort + end + + def test_collection_singular_ids_setter + company = companies(:rails_core) + dev = Developer.find(:first) + + company.developer_ids = [dev.id] + assert_equal [dev], company.developers + end + + def test_collection_singular_ids_setter_with_string_primary_keys + assert_nothing_raised do + book = books(:awdr) + book.subscriber_ids = [subscribers(:second).nick] + assert_equal [subscribers(:second)], book.subscribers(true) + + book.subscriber_ids = [] + assert_equal [], book.subscribers(true) + end + + end + + def test_collection_singular_ids_setter_raises_exception_when_invalid_ids_set + company = companies(:rails_core) + ids = [Developer.find(:first).id, -9999] + assert_raises(ActiveRecord::RecordNotFound) {company.developer_ids= ids} + end + end diff --git a/activerecord/test/fixtures/subscriptions.yml b/activerecord/test/fixtures/subscriptions.yml index 371bfd3422..5a93c12193 100644 --- a/activerecord/test/fixtures/subscriptions.yml +++ b/activerecord/test/fixtures/subscriptions.yml @@ -9,4 +9,4 @@ webster_rfr: alterself_awdr: id: 3 subscriber_id: alterself - book_id: 3 \ No newline at end of file + book_id: 1 diff --git a/activerecord/test/models/book.rb b/activerecord/test/models/book.rb index cfd07abddc..1e030b4f59 100644 --- a/activerecord/test/models/book.rb +++ b/activerecord/test/models/book.rb @@ -1,4 +1,7 @@ class Book < ActiveRecord::Base has_many :citations, :foreign_key => 'book1_id' has_many :references, :through => :citations, :source => :reference_of, :uniq => true + + has_many :subscriptions + has_many :subscribers, :through => :subscriptions end -- cgit v1.2.3 From e1344bf5048934b0f231974c3597dfbc1c76154f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 2 Aug 2010 16:51:08 +0200 Subject: Tidy up previous commit. --- activerecord/lib/active_record/associations.rb | 6 ++--- activerecord/lib/active_record/reflection.rb | 26 ++++++++++------------ .../has_many_through_associations_test.rb | 2 +- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index bd90cfc5d5..1dc094b893 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1437,11 +1437,11 @@ module ActiveRecord association.replace(new_value) association end - + redefine_method("#{reflection.name.to_s.singularize}_ids=") do |new_value| - pk_column = reflection.klass.columns.find{|c| c.name == reflection.klass.primary_key } + pk_column = reflection.primary_key_column ids = (new_value || []).reject { |nid| nid.blank? } - ids.map!{|i| pk_column.type_cast(i)} + ids.map!{ |i| pk_column.type_cast(i) } send("#{reflection.name}=", reflection.klass.find(ids).index_by(&:id).values_at(*ids)) end end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 03a932f642..7f47a812eb 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -91,25 +91,19 @@ module ActiveRecord # # composed_of :balance, :class_name => 'Money' returns :balance # has_many :clients returns :clients - def name - @name - end + attr_reader :name # Returns the macro type. # # composed_of :balance, :class_name => 'Money' returns :composed_of # has_many :clients returns :has_many - def macro - @macro - end + attr_reader :macro # Returns the hash of options used for the macro. # # composed_of :balance, :class_name => 'Money' returns { :class_name => "Money" } # has_many :clients returns +{}+ - def options - @options - end + attr_reader :options # Returns the class for the macro. # @@ -137,11 +131,6 @@ module ActiveRecord @sanitized_conditions ||= klass.send(:sanitize_sql, options[:conditions]) if options[:conditions] end - # Returns +true+ if +self+ is a +belongs_to+ reflection. - def belongs_to? - macro == :belongs_to - end - private def derive_class_name name.to_s.camelize @@ -213,6 +202,10 @@ module ActiveRecord @primary_key_name ||= options[:foreign_key] || derive_primary_key_name end + def primary_key_column + @primary_key_column ||= klass.columns.find { |c| c.name == klass.primary_key } + end + def association_foreign_key @association_foreign_key ||= @options[:association_foreign_key] || class_name.foreign_key end @@ -307,6 +300,11 @@ module ActiveRecord dependent_conditions end + # Returns +true+ if +self+ is a +belongs_to+ reflection. + def belongs_to? + macro == :belongs_to + end + private def derive_class_name class_name = name.to_s.camelize diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 3940e75ad6..0eaadac5ae 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -21,7 +21,7 @@ require 'models/subscription' class HasManyThroughAssociationsTest < ActiveRecord::TestCase fixtures :posts, :readers, :people, :comments, :authors, :owners, :pets, :toys, :jobs, :references, :companies, - :subscribers, :books, :subscriptions + :subscribers, :books, :subscriptions, :developers # Dummies to force column loads so query counts are clean. def setup -- cgit v1.2.3 From 59693c4c49cce5e4cf53eb54e42e3da07a98467e Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 27 Jul 2010 09:55:33 +0200 Subject: fix loading of different elements in array then int and string [#5036 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activeresource/lib/active_resource/base.rb | 6 +++--- activeresource/test/cases/base/load_test.rb | 12 ++++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index 72ad2f5872..62420725ad 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -1222,10 +1222,10 @@ module ActiveResource when Array resource = find_or_create_resource_for_collection(key) value.map do |attrs| - if attrs.is_a?(String) || attrs.is_a?(Numeric) - attrs.duplicable? ? attrs.dup : attrs - else + if attrs.is_a?(Hash) resource.new(attrs) + else + attrs.duplicable? ? attrs.dup : attrs end end when Hash diff --git a/activeresource/test/cases/base/load_test.rb b/activeresource/test/cases/base/load_test.rb index 7745a9439b..228dc36d9b 100644 --- a/activeresource/test/cases/base/load_test.rb +++ b/activeresource/test/cases/base/load_test.rb @@ -47,6 +47,8 @@ class BaseLoadTest < Test::Unit::TestCase { :id => 1, :name => 'Willamette' }, { :id => 2, :name => 'Columbia', :rafted_by => @matz }], :postal_codes => [ 97018, 1234567890 ], + :dates => [ Time.now ], + :votes => [ true, false, true ], :places => [ "Columbia City", "Unknown" ]}}} @person = Person.new @@ -149,6 +151,16 @@ class BaseLoadTest < Test::Unit::TestCase assert_kind_of Array, places assert_kind_of String, places.first assert_equal @deep[:street][:state][:places].first, places.first + + dates = state.dates + assert_kind_of Array, dates + assert_kind_of Time, dates.first + assert_equal @deep[:street][:state][:dates].first, dates.first + + votes = state.votes + assert_kind_of Array, votes + assert_kind_of TrueClass, votes.first + assert_equal @deep[:street][:state][:votes].first, votes.first end def test_nested_collections_within_the_same_namespace -- cgit v1.2.3 From 009aa8825b6932b006f005ac351b82ad8100d7f1 Mon Sep 17 00:00:00 2001 From: Neeraj Singh Date: Mon, 26 Jul 2010 15:12:36 -0400 Subject: Eager loading an association should not change the count of children MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [#4971 state:resolved] Signed-off-by: José Valim --- activerecord/lib/active_record/associations.rb | 4 ++++ activerecord/test/cases/associations_test.rb | 13 +++++++++++++ activerecord/test/models/electron.rb | 3 +++ activerecord/test/models/liquid.rb | 5 +++++ activerecord/test/models/molecule.rb | 4 ++++ activerecord/test/schema/schema.rb | 13 +++++++++++++ 6 files changed, 42 insertions(+) create mode 100644 activerecord/test/models/electron.rb create mode 100644 activerecord/test/models/liquid.rb create mode 100644 activerecord/test/models/molecule.rb diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 1dc094b893..9cd3b9662f 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1815,6 +1815,10 @@ module ActiveRecord when Hash associations.keys.each do |name| reflection = base.reflections[name] + + if records.any? && reflection.options && reflection.options[:uniq] + records.each { |record| record.send(reflection.name).target.uniq! } + end parent_records = [] records.each do |record| diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index a1c794c084..d328ca630b 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -14,11 +14,24 @@ require 'models/reader' require 'models/parrot' require 'models/ship_part' require 'models/ship' +require 'models/liquid' +require 'models/molecule' +require 'models/electron' class AssociationsTest < ActiveRecord::TestCase fixtures :accounts, :companies, :developers, :projects, :developers_projects, :computers, :people, :readers + def test_eager_loading_should_not_change_count_of_children + liquid = Liquid.create(:name => 'salty') + molecule = liquid.molecules.create(:name => 'molecule_1') + molecule.electrons.create(:name => 'electron_1') + molecule.electrons.create(:name => 'electron_2') + + liquids = Liquid.includes(:molecules => :electrons).where('molecules.id is not null') + assert_equal 1, liquids[0].molecules.length + end + def test_loading_the_association_target_should_keep_child_records_marked_for_destruction ship = Ship.create!(:name => "The good ship Dollypop") part = ship.parts.create!(:name => "Mast") diff --git a/activerecord/test/models/electron.rb b/activerecord/test/models/electron.rb new file mode 100644 index 0000000000..35af9f679b --- /dev/null +++ b/activerecord/test/models/electron.rb @@ -0,0 +1,3 @@ +class Electron < ActiveRecord::Base + belongs_to :molecule +end diff --git a/activerecord/test/models/liquid.rb b/activerecord/test/models/liquid.rb new file mode 100644 index 0000000000..b96c054f6c --- /dev/null +++ b/activerecord/test/models/liquid.rb @@ -0,0 +1,5 @@ +class Liquid < ActiveRecord::Base + set_table_name :liquid + has_many :molecules, :uniq => true +end + diff --git a/activerecord/test/models/molecule.rb b/activerecord/test/models/molecule.rb new file mode 100644 index 0000000000..69325b8d29 --- /dev/null +++ b/activerecord/test/models/molecule.rb @@ -0,0 +1,4 @@ +class Molecule < ActiveRecord::Base + belongs_to :liquid + has_many :electrons +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index a0e620c2ef..fc3810f82b 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -618,6 +618,19 @@ ActiveRecord::Schema.define do t.datetime :updated_at end + create_table :liquid, :force => true do |t| + t.string :name + end + create_table :molecules, :force => true do |t| + t.integer :liquid_id + t.string :name + end + create_table :electrons, :force => true do |t| + t.integer :molecule_id + t.string :name + end + + except 'SQLite' do # fk_test_has_fk should be before fk_test_has_pk create_table :fk_test_has_fk, :force => true do |t| -- cgit v1.2.3 From 9effe3cc18182abbcf0a8b01635a34ca77b67e02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 2 Aug 2010 17:20:17 +0200 Subject: Remove duplicated logic. --- activerecord/lib/active_record/associations.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 9cd3b9662f..fdc203e298 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1805,9 +1805,7 @@ module ActiveRecord case associations when Symbol, String reflection = base.reflections[associations] - if reflection && reflection.collection? - records.each { |record| record.send(reflection.name).target.uniq! } - end + remove_uniq_by_reflection(reflection, records) when Array associations.each do |association| remove_duplicate_results!(base, records, association) @@ -1815,10 +1813,7 @@ module ActiveRecord when Hash associations.keys.each do |name| reflection = base.reflections[name] - - if records.any? && reflection.options && reflection.options[:uniq] - records.each { |record| record.send(reflection.name).target.uniq! } - end + remove_uniq_by_reflection(reflection, records) parent_records = [] records.each do |record| @@ -1837,6 +1832,7 @@ module ActiveRecord end protected + def build(associations, parent = nil, join_class = Arel::InnerJoin) parent ||= @joins.last case associations @@ -1859,6 +1855,12 @@ module ActiveRecord end end + def remove_uniq_by_reflection(reflection, records) + if reflection && reflection.collection? + records.each { |record| record.send(reflection.name).target.uniq! } + end + end + def build_join_association(reflection, parent) JoinAssociation.new(reflection, self, parent) end -- cgit v1.2.3 From 88b5f938cf7d3eb26ad204451a4dbb9c2cf4f571 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 2 Aug 2010 18:40:20 +0200 Subject: Bring returning back to ease migration. --- .../lib/active_support/core_ext/object.rb | 1 + .../active_support/core_ext/object/returning.rb | 43 ++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 activesupport/lib/active_support/core_ext/object/returning.rb diff --git a/activesupport/lib/active_support/core_ext/object.rb b/activesupport/lib/active_support/core_ext/object.rb index 790a26f5c1..d671da6711 100644 --- a/activesupport/lib/active_support/core_ext/object.rb +++ b/activesupport/lib/active_support/core_ext/object.rb @@ -2,6 +2,7 @@ require 'active_support/core_ext/object/acts_like' require 'active_support/core_ext/object/blank' require 'active_support/core_ext/object/duplicable' require 'active_support/core_ext/object/try' +require 'active_support/core_ext/object/returning' require 'active_support/core_ext/object/conversions' require 'active_support/core_ext/object/instance_variables' diff --git a/activesupport/lib/active_support/core_ext/object/returning.rb b/activesupport/lib/active_support/core_ext/object/returning.rb new file mode 100644 index 0000000000..07250b2a27 --- /dev/null +++ b/activesupport/lib/active_support/core_ext/object/returning.rb @@ -0,0 +1,43 @@ +class Object + # Returns +value+ after yielding +value+ to the block. This simplifies the + # process of constructing an object, performing work on the object, and then + # returning the object from a method. It is a Ruby-ized realization of the K + # combinator, courtesy of Mikael Brockman. + # + # ==== Examples + # + # # Without returning + # def foo + # values = [] + # values << "bar" + # values << "baz" + # return values + # end + # + # foo # => ['bar', 'baz'] + # + # # returning with a local variable + # def foo + # returning values = [] do + # values << 'bar' + # values << 'baz' + # end + # end + # + # foo # => ['bar', 'baz'] + # + # # returning with a block argument + # def foo + # returning [] do |values| + # values << 'bar' + # values << 'baz' + # end + # end + # + # foo # => ['bar', 'baz'] + def returning(value) + ActiveSupport::Deprecation.warn('Object#returning has been deprecated in favor of Object#tap.', caller) + yield(value) + value + end +end \ No newline at end of file -- cgit v1.2.3