aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--railties/CHANGELOG.md7
-rw-r--r--railties/lib/rails/generators/generated_attribute.rb10
-rw-r--r--railties/test/generators/generated_attribute_test.rb17
-rw-r--r--railties/test/generators/migration_generator_test.rb29
-rw-r--r--railties/test/generators/model_generator_test.rb50
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"]