Add `null: false` constraint by default for `belongs_to` associations

- Also deprecate passing {required} to the model generator.
- Also made sure the global config `belongs_to_required_by_default` is
  applied correctly to the model generator for `null: false` option.

3 files changed, 89 insertions, 7 deletions

diff --git a/railties/test/generators/generated_attribute_test.rb b/railties/test/generators/generated_attribute_test.rb
index bf60d6bc22..82d550a124 100644
--- a/railties/test/generators/generated_attribute_test.rb
+++ b/railties/test/generators/generated_attribute_test.rb
@@ -6,6 +6,15 @@ require "rails/generators/generated_attribute"
 class GeneratedAttributeTest < Rails::Generators::TestCase
   include GeneratorsTestHelper
 
+  def setup
+    @old_belongs_to_required_by_default = Rails.application.config.active_record.belongs_to_required_by_default
+    Rails.application.config.active_record.belongs_to_required_by_default = true
+  end
+
+  def teardown
+    Rails.application.config.active_record.belongs_to_required_by_default = @old_belongs_to_required_by_default
+  end
+
   def test_field_type_returns_number_field
     assert_field_type :integer, :number_field
   end
@@ -155,10 +164,16 @@ class GeneratedAttributeTest < Rails::Generators::TestCase
   end
 
   def test_parse_required_attribute_with_index
-    att = Rails::Generators::GeneratedAttribute.parse("supplier:references{required}:index")
+    att = Rails::Generators::GeneratedAttribute.parse("supplier:references:index")
     assert_equal "supplier", att.name
     assert_equal :references, att.type
     assert_predicate att, :has_index?
     assert_predicate att, :required?
   end
+
+  def test_parse_required_attribute_with_index_false_when_belongs_to_required_by_default_global_config_is_false
+    Rails.application.config.active_record.belongs_to_required_by_default = false
+    att = Rails::Generators::GeneratedAttribute.parse("supplier:references:index")
+    assert_not_predicate att, :required?
+  end
 end
diff --git a/railties/test/generators/migration_generator_test.rb b/railties/test/generators/migration_generator_test.rb
index 540bed551b..e6b614a935 100644
--- a/railties/test/generators/migration_generator_test.rb
+++ b/railties/test/generators/migration_generator_test.rb
@@ -6,6 +6,16 @@ require "rails/generators/rails/migration/migration_generator"
 class MigrationGeneratorTest < Rails::Generators::TestCase
   include GeneratorsTestHelper
 
+  def setup
+    @old_belongs_to_required_by_default = Rails.application.config.active_record.belongs_to_required_by_default
+
+    Rails.application.config.active_record.belongs_to_required_by_default = true
+  end
+
+  def teardown
+    Rails.application.config.active_record.belongs_to_required_by_default = @old_belongs_to_required_by_default
+  end
+
   def test_migration
     migration = "change_title_body_from_posts"
     run_generator [migration]
@@ -196,9 +206,9 @@ class MigrationGeneratorTest < Rails::Generators::TestCase
     end
   end
 
-  def test_add_migration_with_required_references
+  def test_add_migration_with_references_adds_null_false_by_default
     migration = "add_references_to_books"
-    run_generator [migration, "author:belongs_to{required}", "distributor:references{polymorphic,required}"]
+    run_generator [migration, "author:belongs_to", "distributor:references{polymorphic}"]
 
     assert_migration "db/migrate/#{migration}.rb" do |content|
       assert_method :change, content do |change|
@@ -208,6 +218,21 @@ class MigrationGeneratorTest < Rails::Generators::TestCase
     end
   end
 
+  def test_add_migration_with_references_does_not_add_belongs_to_when_required_by_default_global_config_is_false
+    Rails.application.config.active_record.belongs_to_required_by_default = false
+
+    migration = "add_references_to_books"
+
+    run_generator [migration, "author:belongs_to", "distributor:references{polymorphic}"]
+
+    assert_migration "db/migrate/#{migration}.rb" do |content|
+      assert_method :change, content do |change|
+        assert_match(/add_reference :books, :author/, change)
+        assert_match(/add_reference :books, :distributor, polymorphic: true/, change)
+      end
+    end
+  end
+
   def test_add_migration_with_references_adds_foreign_keys
     migration = "add_references_to_books"
     run_generator [migration, "author:belongs_to", "distributor:references{polymorphic}"]
diff --git a/railties/test/generators/model_generator_test.rb b/railties/test/generators/model_generator_test.rb
index f860e328f2..df765432bc 100644
--- a/railties/test/generators/model_generator_test.rb
+++ b/railties/test/generators/model_generator_test.rb
@@ -10,6 +10,14 @@ class ModelGeneratorTest < Rails::Generators::TestCase
   def setup
     super
     Rails::Generators::ModelHelpers.skip_warn = false
+    @old_belongs_to_required_by_default = Rails.application.config.active_record.belongs_to_required_by_default
+
+    Rails.application.config.active_record.belongs_to_required_by_default = true
+  end
+
+
+  def teardown
+    Rails.application.config.active_record.belongs_to_required_by_default = @old_belongs_to_required_by_default
   end
 
   def test_help_shows_invoked_generators_options
@@ -415,7 +423,7 @@ class ModelGeneratorTest < Rails::Generators::TestCase
   end
 
   def test_required_polymorphic_belongs_to_generates_correct_model
-    run_generator ["account", "supplier:references{required,polymorphic}"]
+    run_generator ["account", "supplier:references{polymorphic}"]
 
     expected_file = <<~FILE
       class Account < ApplicationRecord
@@ -426,7 +434,7 @@ class ModelGeneratorTest < Rails::Generators::TestCase
   end
 
   def test_required_and_polymorphic_are_order_independent
-    run_generator ["account", "supplier:references{polymorphic.required}"]
+    run_generator ["account", "supplier:references{polymorphic}"]
 
     expected_file = <<~FILE
       class Account < ApplicationRecord
@@ -436,8 +444,10 @@ class ModelGeneratorTest < Rails::Generators::TestCase
     assert_file "app/models/account.rb", expected_file
   end
 
-  def test_required_adds_null_false_to_column
-    run_generator ["account", "supplier:references{required}"]
+  def test_passing_required_to_model_generator_is_deprecated
+    assert_deprecated do
+      run_generator ["account", "supplier:references{required}"]
+    end
 
     assert_migration "db/migrate/create_accounts.rb" do |m|
       assert_method :change, m do |up|
@@ -446,6 +456,38 @@ class ModelGeneratorTest < Rails::Generators::TestCase
     end
   end
 
+  def test_null_false_is_added_for_references_by_default
+    run_generator ["account", "user:references"]
+
+    assert_migration "db/migrate/create_accounts.rb" do |m|
+      assert_method :change, m do |up|
+        assert_match(/t\.references :user,.*\snull: false/, up)
+      end
+    end
+  end
+
+  def test_null_false_is_added_for_belongs_to_by_default
+    run_generator ["account", "user:belongs_to"]
+
+    assert_migration "db/migrate/create_accounts.rb" do |m|
+      assert_method :change, m do |up|
+        assert_match(/t\.belongs_to :user,.*\snull: false/, up)
+      end
+    end
+  end
+
+  def test_null_false_is_not_added_when_belongs_to_required_by_default_global_config_is_false
+    Rails.application.config.active_record.belongs_to_required_by_default = false
+
+    run_generator ["account", "user:belongs_to"]
+
+    assert_migration "db/migrate/create_accounts.rb" do |m|
+      assert_method :change, m do |up|
+        assert_match(/t\.belongs_to :user/, up)
+      end
+    end
+  end
+
   def test_foreign_key_is_not_added_for_non_references
     run_generator ["account", "supplier:string"]