diff options
Diffstat (limited to 'activerecord')
5 files changed, 55 insertions, 166 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index d40251b9ca..acfd470c95 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -2199,7 +2199,7 @@ during calendar reform. #7649, #7724 [fedot, Geoff Buesing] * Escape database name in MySQL adapter when creating and dropping databases. #3409 [anna@wota.jp] -* Disambiguate table names for columns in validates_uniquness_of's WHERE clause. #3423 [alex.borovsky@gmail.com] +* Disambiguate table names for columns in validates_uniqueness_of's WHERE clause. #3423 [alex.borovsky@gmail.com] * .with_scope imposed create parameters now bypass attr_protected [Tobias Lütke] @@ -3714,7 +3714,7 @@ in effect. Added :readonly finder constraint. Calling an association collectio * Escape database name in MySQL adapter when creating and dropping databases. #3409 [anna@wota.jp] -* Disambiguate table names for columns in validates_uniquness_of's WHERE clause. #3423 [alex.borovsky@gmail.com] +* Disambiguate table names for columns in validates_uniqueness_of's WHERE clause. #3423 [alex.borovsky@gmail.com] * .with_scope imposed create parameters now bypass attr_protected [Tobias Lütke] diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 292da58831..829f0ac0c5 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -8,25 +8,11 @@ module ActiveRecord alias_method :new, :build def create!(attrs = nil) - ensure_owner_is_not_new - - transaction do - self << (object = attrs ? @reflection.klass.send(:with_scope, :create => attrs) { @reflection.create_association! } : @reflection.create_association!) - object - end + create_record(attrs, true) end def create(attrs = nil) - ensure_owner_is_not_new - - transaction do - object = attrs ? @reflection.klass.send(:with_scope, :create => attrs) { @reflection.create_association } : @reflection.create_association - raise_on_type_mismatch(object) - add_record_to_target_with_callbacks(object) do |r| - insert_record(object, false) - end - object - end + create_record(attrs, false) end def destroy(*records) @@ -44,8 +30,18 @@ module ActiveRecord return @target.size if loaded? return count end - + protected + def create_record(attrs, force = true) + ensure_owner_is_not_new + + transaction do + object = @reflection.klass.new(attrs) + add_record_to_target_with_callbacks(object) {|r| insert_record(object, force) } + object + end + end + def target_reflection_has_associated_record? if @reflection.through_reflection.macro == :belongs_to && @owner[@reflection.through_reflection.primary_key_name].blank? false @@ -69,9 +65,10 @@ module ActiveRecord return false unless record.save(validate) end end - through_reflection = @reflection.through_reflection - klass = through_reflection.klass - @owner.send(@reflection.through_reflection.name).proxy_target << klass.send(:with_scope, :create => construct_join_attributes(record)) { through_reflection.create_association! } + + through_association = @owner.send(@reflection.through_reflection.name) + through_record = through_association.create!(construct_join_attributes(record)) + through_association.proxy_target << through_record end # TODO - add dependent option support diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 911c908c8b..b6c4df2a49 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -1,92 +1,21 @@ +require 'active_support/core_ext/object/tap' + module ActiveRecord module AttributeMethods - # Track unsaved attribute changes. - # - # A newly instantiated object is unchanged: - # person = Person.find_by_name('Uncle Bob') - # person.changed? # => false - # - # Change the name: - # person.name = 'Bob' - # person.changed? # => true - # person.name_changed? # => true - # person.name_was # => 'Uncle Bob' - # person.name_change # => ['Uncle Bob', 'Bob'] - # person.name = 'Bill' - # person.name_change # => ['Uncle Bob', 'Bill'] - # - # Save the changes: - # person.save - # person.changed? # => false - # person.name_changed? # => false - # - # Assigning the same value leaves the attribute unchanged: - # person.name = 'Bill' - # person.name_changed? # => false - # person.name_change # => nil - # - # Which attributes have changed? - # person.name = 'Bob' - # person.changed # => ['name'] - # person.changes # => { 'name' => ['Bill', 'Bob'] } - # - # Resetting an attribute returns it to its original state: - # person.reset_name! # => 'Bill' - # person.changed? # => false - # person.name_changed? # => false - # person.name # => 'Bill' - # - # Before modifying an attribute in-place: - # person.name_will_change! - # person.name << 'y' - # person.name_change # => ['Bill', 'Billy'] module Dirty extend ActiveSupport::Concern - - DIRTY_AFFIXES = [ - { :suffix => '_changed?' }, - { :suffix => '_change' }, - { :suffix => '_will_change!' }, - { :suffix => '_was' }, - { :prefix => 'reset_', :suffix => '!' } - ] + include ActiveModel::Dirty included do - attribute_method_affix *DIRTY_AFFIXES - - alias_method_chain :save, :dirty - alias_method_chain :save!, :dirty - alias_method_chain :update, :dirty - alias_method_chain :reload, :dirty + alias_method_chain :save, :dirty + alias_method_chain :save!, :dirty + alias_method_chain :update, :dirty + alias_method_chain :reload, :dirty superclass_delegating_accessor :partial_updates self.partial_updates = true end - # Do any attributes have unsaved changes? - # person.changed? # => false - # person.name = 'bob' - # person.changed? # => true - def changed? - !changed_attributes.empty? - end - - # List of attributes with unsaved changes. - # person.changed # => [] - # person.name = 'bob' - # person.changed # => ['name'] - def changed - changed_attributes.keys - end - - # Map of changed attrs => [original value, new value]. - # person.changes # => {} - # person.name = 'bob' - # person.changes # => { 'name' => ['bill', 'bob'] } - def changes - changed.inject({}) { |h, attr| h[attr] = attribute_change(attr); h } - end - # Attempts to +save+ the record and clears changed attributes if successful. def save_with_dirty(*args) #:nodoc: if status = save_without_dirty(*args) @@ -97,49 +26,15 @@ module ActiveRecord # Attempts to <tt>save!</tt> the record and clears changed attributes if successful. def save_with_dirty!(*args) #:nodoc: - status = save_without_dirty!(*args) - changed_attributes.clear - status + save_without_dirty!(*args).tap { changed_attributes.clear } end # <tt>reload</tt> the record and clears changed attributes. def reload_with_dirty(*args) #:nodoc: - record = reload_without_dirty(*args) - changed_attributes.clear - record + reload_without_dirty(*args).tap { changed_attributes.clear } end private - # Map of change <tt>attr => original value</tt>. - def changed_attributes - @changed_attributes ||= {} - end - - # Handle <tt>*_changed?</tt> for +method_missing+. - def attribute_changed?(attr) - changed_attributes.include?(attr) - end - - # Handle <tt>*_change</tt> for +method_missing+. - def attribute_change(attr) - [changed_attributes[attr], __send__(attr)] if attribute_changed?(attr) - end - - # Handle <tt>*_was</tt> for +method_missing+. - def attribute_was(attr) - attribute_changed?(attr) ? changed_attributes[attr] : __send__(attr) - end - - # Handle <tt>reset_*!</tt> for +method_missing+. - def reset_attribute!(attr) - self[attr] = changed_attributes[attr] if attribute_changed?(attr) - end - - # Handle <tt>*_will_change!</tt> for +method_missing+. - def attribute_will_change!(attr) - changed_attributes[attr] = clone_attribute_value(:read_attribute, attr) - end - # Wrap write_attribute to remember original attribute value. def write_attribute(attr, value) attr = attr.to_s @@ -182,23 +77,6 @@ module ActiveRecord old != value end - - module ClassMethods - def self.extended(base) - class << base - alias_method_chain :alias_attribute, :dirty - end - end - - def alias_attribute_with_dirty(new_name, old_name) - alias_attribute_without_dirty(new_name, old_name) - DIRTY_AFFIXES.each do |affixes| - module_eval <<-STR, __FILE__, __LINE__+1 - def #{affixes[:prefix]}#{new_name}#{affixes[:suffix]}; self.#{affixes[:prefix]}#{old_name}#{affixes[:suffix]}; end # def reset_subject!; self.reset_title!; end - STR - end - end - end end 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 59985374d3..5f13b66d11 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -180,27 +180,41 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_associate_with_create_and_invalid_options - peeps = companies(:first_firm).developers.count - assert_nothing_raised { companies(:first_firm).developers.create(:name => '0') } - assert_equal peeps, companies(:first_firm).developers.count + firm = companies(:first_firm) + assert_no_difference('firm.developers.count') { assert_nothing_raised { firm.developers.create(:name => '0') } } end def test_associate_with_create_and_valid_options - peeps = companies(:first_firm).developers.count - assert_nothing_raised { companies(:first_firm).developers.create(:name => 'developer') } - assert_equal peeps + 1, companies(:first_firm).developers.count + firm = companies(:first_firm) + assert_difference('firm.developers.count', 1) { firm.developers.create(:name => 'developer') } end def test_associate_with_create_bang_and_invalid_options - peeps = companies(:first_firm).developers.count - assert_raises(ActiveRecord::RecordInvalid) { companies(:first_firm).developers.create!(:name => '0') } - assert_equal peeps, companies(:first_firm).developers.count + firm = companies(:first_firm) + assert_no_difference('firm.developers.count') { assert_raises(ActiveRecord::RecordInvalid) { firm.developers.create!(:name => '0') } } end def test_associate_with_create_bang_and_valid_options - peeps = companies(:first_firm).developers.count - assert_nothing_raised { companies(:first_firm).developers.create!(:name => 'developer') } - assert_equal peeps + 1, companies(:first_firm).developers.count + firm = companies(:first_firm) + assert_difference('firm.developers.count', 1) { firm.developers.create!(:name => 'developer') } + end + + def test_push_with_invalid_record + firm = companies(:first_firm) + assert_raises(ActiveRecord::RecordInvalid) { firm.developers << Developer.new(:name => '0') } + end + + def test_push_with_invalid_join_record + repair_validations(Contract) do + Contract.validate {|r| r.errors[:base] << 'Invalid Contract' } + + firm = companies(:first_firm) + lifo = Developer.new(:name => 'lifo') + assert_raises(ActiveRecord::RecordInvalid) { firm.developers << lifo } + + lifo = Developer.create!(:name => 'lifo') + assert_raises(ActiveRecord::RecordInvalid) { firm.developers << lifo } + end end def test_clear_associations diff --git a/activerecord/test/cases/validations/uniqueness_validation_test.rb b/activerecord/test/cases/validations/uniqueness_validation_test.rb index cb123d3498..17ba4e2f8a 100644 --- a/activerecord/test/cases/validations/uniqueness_validation_test.rb +++ b/activerecord/test/cases/validations/uniqueness_validation_test.rb @@ -59,7 +59,7 @@ class UniquenessValidationTest < ActiveRecord::TestCase assert t2.save, "Should now save t2 as unique" end - def test_validates_uniquness_with_newline_chars + def test_validates_uniqueness_with_newline_chars Topic.validates_uniqueness_of(:title, :case_sensitive => false) t = Topic.new("title" => "new\nline") |