Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fix redefine a has_and_belongs_to_many inside inherited class

After ad7b5ef, which changed how
has_an_belongs_to_many used to work, we start raising an error when
redefining the same has_an_belongs_to_many association. This commits fix
that regression.

[Fixes #14983]
  • Loading branch information...
commit 8570f9391d725afae1f3d60bfc5536afc46733b5 1 parent 0bdfb4d
@arthurnn arthurnn authored
View
8 activerecord/CHANGELOG.md
@@ -1,3 +1,11 @@
+* Fix redefine a has_and_belongs_to_many inside inherited class
+ Fixing regression case, where redefining the same has_an_belongs_to_many
+ definition into a subclass would raise.
+
+ Fixes #14983.
+
+ *arthurnn*
+
* Add a properties API to allow custom types and type casting behavior
to be specified. Will enable many edge cases to be deprecated, and
allow for additional interesting features in the future.
View
47 activerecord/lib/active_record/autosave_association.rb
@@ -147,6 +147,7 @@ module ClassMethods
private
def define_non_cyclic_method(name, &block)
+ return if method_defined?(name)
define_method(name) do |*args|
result = true; @_already_called ||= {}
# Loop prevention for validation of associations
@@ -179,30 +180,28 @@ def add_autosave_association_callbacks(reflection)
validation_method = :"validate_associated_records_for_#{reflection.name}"
collection = reflection.collection?
- unless method_defined?(save_method)
- if collection
- before_save :before_save_collection_association
-
- define_non_cyclic_method(save_method) { save_collection_association(reflection) }
- # Doesn't use after_save as that would save associations added in after_create/after_update twice
- after_create save_method
- after_update save_method
- elsif reflection.macro == :has_one
- define_method(save_method) { save_has_one_association(reflection) }
- # Configures two callbacks instead of a single after_save so that
- # the model may rely on their execution order relative to its
- # own callbacks.
- #
- # For example, given that after_creates run before after_saves, if
- # we configured instead an after_save there would be no way to fire
- # a custom after_create callback after the child association gets
- # created.
- after_create save_method
- after_update save_method
- else
- define_non_cyclic_method(save_method) { save_belongs_to_association(reflection) }
- before_save save_method
- end
+ if collection
+ before_save :before_save_collection_association
+
+ define_non_cyclic_method(save_method) { save_collection_association(reflection) }
+ # Doesn't use after_save as that would save associations added in after_create/after_update twice
+ after_create save_method
+ after_update save_method
+ elsif reflection.macro == :has_one
+ define_method(save_method) { save_has_one_association(reflection) } unless method_defined?(save_method)
+ # Configures two callbacks instead of a single after_save so that
+ # the model may rely on their execution order relative to its
+ # own callbacks.
+ #
+ # For example, given that after_creates run before after_saves, if
+ # we configured instead an after_save there would be no way to fire
+ # a custom after_create callback after the child association gets
+ # created.
+ after_create save_method
+ after_update save_method
+ else
+ define_non_cyclic_method(save_method) { save_belongs_to_association(reflection) }
+ before_save save_method
end
if reflection.validate? && !method_defined?(validation_method)
View
16 activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -70,6 +70,14 @@ class DeveloperWithSymbolsForKeys < ActiveRecord::Base
:foreign_key => "developer_id"
end
+class SubDeveloper < Developer
+ self.table_name = 'developers'
+ has_and_belongs_to_many :special_projects,
+ :join_table => 'developers_projects',
+ :foreign_key => "project_id",
+ :association_foreign_key => "developer_id"
+end
+
class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
fixtures :accounts, :companies, :categories, :posts, :categories_posts, :developers, :projects, :developers_projects,
:parrots, :pirates, :parrots_pirates, :treasures, :price_estimates, :tags, :taggings
@@ -814,7 +822,7 @@ def test_destruction_does_not_error_without_primary_key
assert_equal [], Pirate.where(id: redbeard.id)
end
- test "has and belongs to many associations on new records use null relations" do
+ def test_has_and_belongs_to_many_associations_on_new_records_use_null_relations
projects = Developer.new.projects
assert_no_queries do
assert_equal [], projects
@@ -860,4 +868,10 @@ def test_namespaced_habtm
assert_includes magazine.articles, article
end
+
+ def test_redefine_habtm
+ child = SubDeveloper.new("name" => "Aredridel")
+ child.special_projects << SpecialProject.new("name" => "Special Project")
+ assert_equal true, child.save
+ end
end
Please sign in to comment.
Something went wrong with that request. Please try again.