From e0566cc4c16af42e0040fa1a43f8be096d565cc3 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Sun, 8 Dec 2024 03:45:14 +0200 Subject: [PATCH] incorrect_dependent_options: Fix infinite recursion when association of the same class as model class --- .../detectors/incorrect_dependent_option.rb | 8 +++++--- .../detectors/incorrect_dependent_option_test.rb | 10 ++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/active_record_doctor/detectors/incorrect_dependent_option.rb b/lib/active_record_doctor/detectors/incorrect_dependent_option.rb index 4c05dfc..20a25f8 100644 --- a/lib/active_record_doctor/detectors/incorrect_dependent_option.rb +++ b/lib/active_record_doctor/detectors/incorrect_dependent_option.rb @@ -90,7 +90,7 @@ def detect [[association.klass], nil] end - deletable_models, destroyable_models = associated_models.partition { |klass| deletable?(klass) } + deletable_models, destroyable_models = associated_models.partition { |klass| deletable?(klass, []) } case association.options[:dependent] when :destroy_async @@ -147,14 +147,16 @@ def models_having_association_with_options(as:) end end - def deletable?(model) + def deletable?(model, processing_list) + return true if processing_list.include?(model) + !defines_destroy_callbacks?(model) && dependent_models(model).all? do |dependent_model| foreign_key = foreign_key(dependent_model.table_name, model.table_name) foreign_key.nil? || foreign_key.on_delete == :nullify || ( - foreign_key.on_delete == :cascade && deletable?(dependent_model) + foreign_key.on_delete == :cascade && deletable?(dependent_model, processing_list + [model]) ) end end diff --git a/test/active_record_doctor/detectors/incorrect_dependent_option_test.rb b/test/active_record_doctor/detectors/incorrect_dependent_option_test.rb index 70dbdd9..366bcc3 100644 --- a/test/active_record_doctor/detectors/incorrect_dependent_option_test.rb +++ b/test/active_record_doctor/detectors/incorrect_dependent_option_test.rb @@ -213,6 +213,16 @@ def test_cascade_foreign_key_and_no_callbacks_on_second_level_association refute_problems end + def test_cascade_and_same_class_association + Context.create_table(:users) do |t| + t.references :parent, foreign_key: { to_table: :users, on_delete: :cascade } + end.define_model do + has_many :children, class_name: "Context::User", foreign_key: :parent_id + end + + refute_problems + end + def test_no_dependent_suggests_nothing Context.create_table(:companies) do end.define_model do