From 8b4d344815655027d9f7584c0a59271dce8f1d5a Mon Sep 17 00:00:00 2001 From: Prathamesh Sonpatki Date: Tue, 16 Apr 2019 11:42:18 +0530 Subject: 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. --- .../test/generators/generated_attribute_test.rb | 17 +++++++- .../test/generators/migration_generator_test.rb | 29 ++++++++++++- railties/test/generators/model_generator_test.rb | 50 ++++++++++++++++++++-- 3 files changed, 89 insertions(+), 7 deletions(-) (limited to 'railties/test') 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"] -- cgit v1.2.3