diff options
| -rw-r--r-- | railties/CHANGELOG.md | 7 | ||||
| -rw-r--r-- | railties/lib/rails/generators/generated_attribute.rb | 10 | ||||
| -rw-r--r-- | railties/test/generators/generated_attribute_test.rb | 17 | ||||
| -rw-r--r-- | railties/test/generators/migration_generator_test.rb | 29 | ||||
| -rw-r--r-- | railties/test/generators/model_generator_test.rb | 50 |
5 files changed, 104 insertions, 9 deletions
diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 109c4836d5..78a94f7c28 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,10 @@ +* `null: false` is set in the migrations by default for column pointed by + `belongs_to` / `references` association generated by model generator. + + Also deprecate passing {required} to the model generator. + + *Prathamesh Sonpatki* + * New applications get `config.cache_classes = false` in `config/environments/test.rb` unless `--skip-spring`. diff --git a/railties/lib/rails/generators/generated_attribute.rb b/railties/lib/rails/generators/generated_attribute.rb index 99c1bc4269..1a80e71eae 100644 --- a/railties/lib/rails/generators/generated_attribute.rb +++ b/railties/lib/rails/generators/generated_attribute.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "active_support/time" +require "active_support/deprecation" module Rails module Generators @@ -51,6 +52,12 @@ module Rails type = $1 provided_options = $2.split(/[,.-]/) options = Hash[provided_options.map { |opt| [opt.to_sym, true] }] + + if options[:required] + ActiveSupport::Deprecation.warn("Passing {required} option has no effect on the model generator. It will be removed in Rails 6.1.\n") + options.delete(:required) + end + return type, options else return type, {} @@ -137,7 +144,7 @@ module Rails end def required? - attr_options[:required] + reference? && Rails.application.config.active_record.belongs_to_required_by_default end def has_index? @@ -183,7 +190,6 @@ module Rails def options_for_migration @attr_options.dup.tap do |options| if required? - options.delete(:required) options[:null] = false end 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"] |