diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 52aaa2371da364e416f38b6489564d38d6dbf62e..b636ffdc971bc262aefbd817311c324219421256 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,10 @@ +* `belongs_to` will now trigger a validation error by default if the association is not present. + You can turn this off on a per-association basis with `optional: true`. + (Note this new default only applies to new Rails apps that will be generated with + `config.active_record.belongs_to_required_by_default = true` in initializer.) + + *Josef Šimánek* + * Fixed ActiveRecord::Relation#becomes! and changed_attributes issues for type column Fixes #17139. diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 499b00a815768d83a86c55d11a79744ad38a7ffa..0b33ee881b4d45b8903e1dbfb9fbb1cffd0f76fe 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1520,10 +1520,16 @@ def has_one(name, scope = nil, options = {}) # object that is the inverse of this belongs_to association. Does not work in # combination with the :polymorphic options. # See ActiveRecord::Associations::ClassMethods's overview on Bi-directional associations for more detail. + # [:optional] + # When set to +true+, the association will not have its presence validated. # [:required] # When set to +true+, the association will also have its presence validated. # This will validate the association itself, not the id. You can use # +:inverse_of+ to avoid an extra query during validation. + # NOTE: required is set to true by default and is deprecated. If + # you don't want to have association presence validated, use optional: true. + # + # # # Option examples: # belongs_to :firm, foreign_key: "client_of" @@ -1536,7 +1542,7 @@ def has_one(name, scope = nil, options = {}) # belongs_to :post, counter_cache: true # belongs_to :comment, touch: true # belongs_to :company, touch: :employees_last_updated_at - # belongs_to :user, required: true + # belongs_to :user, optional: true def belongs_to(name, scope = nil, options = {}) reflection = Builder::BelongsTo.build(self, name, scope, options) Reflection.add_reflection self, name, reflection diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index d0ad57f9c68c764695c8e8eec8546ce787359d8a..ec135d49b71aae032accebd767be60a2d77c52fd 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -5,7 +5,7 @@ def self.macro end def self.valid_options(options) - super + [:foreign_type, :polymorphic, :touch, :counter_cache] + super + [:foreign_type, :polymorphic, :touch, :counter_cache, :optional] end def self.valid_dependent_options @@ -110,5 +110,23 @@ def self.add_destroy_callbacks(model, reflection) name = reflection.name model.after_destroy lambda { |o| o.association(name).handle_dependency } end + + def self.define_validations(model, reflection) + if reflection.options.key?(:required) + reflection.options[:optional] = !reflection.options.delete(:required) + end + + if reflection.options[:optional].nil? + required = model.belongs_to_required_by_default + else + required = !reflection.options[:optional] + end + + super + + if required + model.validates_presence_of reflection.name, message: :required + end + end end end diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb index 64e9e6b334143db87d5a0cca6b84a088151d66d8..a272d3c7815053603c8a03933b6dd75b9dbba144 100644 --- a/activerecord/lib/active_record/associations/builder/has_one.rb +++ b/activerecord/lib/active_record/associations/builder/has_one.rb @@ -17,5 +17,12 @@ def self.valid_dependent_options def self.add_destroy_callbacks(model, reflection) super unless reflection.options[:through] end + + def self.define_validations(model, reflection) + super + if reflection.options[:required] + model.validates_presence_of reflection.name, message: :required + end + end end end diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb index f6274c027ecbaafcc0fd92f2f1aaa9e8f55f1571..42542f188e899c1d19376c0029069f8e07ea720e 100644 --- a/activerecord/lib/active_record/associations/builder/singular_association.rb +++ b/activerecord/lib/active_record/associations/builder/singular_association.rb @@ -27,12 +27,5 @@ def create_#{name}!(*args, &block) end CODE end - - def self.define_validations(model, reflection) - super - if reflection.options[:required] - model.validates_presence_of reflection.name, message: :required - end - end end end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 1244bd6195f4acf572e7d6027327435e2404022e..de4868174673dfb7988e28328371fe7ea012bda4 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -87,6 +87,8 @@ def self.configurations mattr_accessor :maintain_test_schema, instance_accessor: false + mattr_accessor :belongs_to_required_by_default, instance_accessor: false + class_attribute :default_connection_handler, instance_writer: false def self.connection_handler diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index a425b3ed88d95a0e5e1efceed4f555247474dbdd..47fd7345c83f5302d91987ad15535a4618757740 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -58,6 +58,56 @@ def test_belongs_to_with_primary_key_joins_on_correct_column end end + def test_optional_relation + original_value = ActiveRecord::Base.belongs_to_required_by_default + ActiveRecord::Base.belongs_to_required_by_default = true + + model = Class.new(ActiveRecord::Base) do + self.table_name = "accounts" + def self.name; "Temp"; end + belongs_to :company, optional: true + end + + account = model.new + assert account.valid? + ensure + ActiveRecord::Base.belongs_to_required_by_default = original_value + end + + def test_not_optional_relation + original_value = ActiveRecord::Base.belongs_to_required_by_default + ActiveRecord::Base.belongs_to_required_by_default = true + + model = Class.new(ActiveRecord::Base) do + self.table_name = "accounts" + def self.name; "Temp"; end + belongs_to :company, optional: false + end + + account = model.new + refute account.valid? + assert_equal [{error: :blank}], account.errors.details[:company] + ensure + ActiveRecord::Base.belongs_to_required_by_default = original_value + end + + def test_required_belongs_to_config + original_value = ActiveRecord::Base.belongs_to_required_by_default + ActiveRecord::Base.belongs_to_required_by_default = true + + model = Class.new(ActiveRecord::Base) do + self.table_name = "accounts" + def self.name; "Temp"; end + belongs_to :company + end + + account = model.new + refute account.valid? + assert_equal [{error: :blank}], account.errors.details[:company] + ensure + ActiveRecord::Base.belongs_to_required_by_default = original_value + end + def test_default_scope_on_relations_is_not_cached counter = 0 diff --git a/guides/source/association_basics.md b/guides/source/association_basics.md index cd715aba1f67a683e2d3b01978eb0f4fd58dbed1..0079049d440545e8a9b58250587e817005eced23 100644 --- a/guides/source/association_basics.md +++ b/guides/source/association_basics.md @@ -833,6 +833,7 @@ The `belongs_to` association supports these options: * `:polymorphic` * `:touch` * `:validate` +* `:optional` ##### `:autosave` @@ -956,6 +957,10 @@ end If you set the `:validate` option to `true`, then associated objects will be validated whenever you save this object. By default, this is `false`: associated objects will not be validated when this object is saved. +##### `:optional` + +If you set the `:optional` option to `true`, then associated object will be validated for presence. By default, this is `false`: associated objects will be validated for presence. + #### Scopes for `belongs_to` There may be times when you wish to customize the query used by `belongs_to`. Such customizations can be achieved via a scope block. For example: diff --git a/guides/source/configuring.md b/guides/source/configuring.md index ab1cc9306ff69718891ee7b8afdd9c1d140893d4..4fc1301e060be5c8531ee9778c9217865b78b8db 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -300,6 +300,8 @@ All these configuration options are delegated to the `I18n` library. `config/environments/production.rb` which is generated by Rails. The default value is true if this configuration is not set. +* `config.active_record.belongs_to_required_by_default` is a boolean value and controls whether `belongs_to` association is required by default. + The MySQL adapter adds one additional configuration option: * `ActiveRecord::ConnectionAdapters::MysqlAdapter.emulate_booleans` controls whether Active Record will consider all `tinyint(1)` columns in a MySQL database to be booleans and is true by default. diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 74d1ca3bd8021af70ba980fda1bf1ec5d514df64..df9f8fe993912fbdc4267bb8a139d5bc89873c34 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,18 @@ +* Add `config/initializers/active_record_belongs_to_required_by_default.rb` + + Newly generated Rails apps have a new initializer called + `active_record_belongs_to_required_by_default.rb` which sets the value of + the configuration option `config.active_record.belongs_to_requred_by_default` + to `true` when ActiveRecord is not skipped. + + As a result, new Rails apps require `belongs_to` association on model + to be valid. + + This initializer is *not* added when running `rake rails:update`, so + old apps ported to Rails 5 will work without any change. + + *Josef Šimánek* + * `delete` operations in configurations are run last in order to eliminate 'No such middleware' errors when `insert_before` or `insert_after` are added after the `delete` operation for the middleware being deleted. diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index 977f5a1c03f8b5b74e7c88fdb9832b18ea1abb69..899b33e529c7693a1b08dc98f2f7292804010d1b 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -89,6 +89,7 @@ def config def config_when_updating cookie_serializer_config_exist = File.exist?('config/initializers/cookies_serializer.rb') callback_terminator_config_exist = File.exist?('config/initializers/callback_terminator.rb') + active_record_belongs_to_required_by_default_config_exist = File.exist?('config/initializers/active_record_belongs_to_required_by_default.rb') config @@ -99,6 +100,10 @@ def config_when_updating unless cookie_serializer_config_exist gsub_file 'config/initializers/cookies_serializer.rb', /json/, 'marshal' end + + unless active_record_belongs_to_required_by_default_config_exist + remove_file 'config/initializers/active_record_belongs_to_required_by_default.rb' + end end def database_yml @@ -258,6 +263,12 @@ def delete_assets_initializer_skipping_sprockets end end + def delete_active_record_initializers_skipping_active_record + if options[:skip_active_record] + remove_file 'config/initializers/active_record_belongs_to_required_by_default.rb' + end + end + def finish_template build(:leftovers) end diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/active_record_belongs_to_required_by_default.rb b/railties/lib/rails/generators/rails/app/templates/config/initializers/active_record_belongs_to_required_by_default.rb new file mode 100644 index 0000000000000000000000000000000000000000..30c4f89792567c6e8c1f7400cf1d9b956bc0e78b --- /dev/null +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/active_record_belongs_to_required_by_default.rb @@ -0,0 +1,4 @@ +# Be sure to restart your server when you modify this file. + +# Require `belongs_to` associations by default. +Rails.application.config.active_record.belongs_to_required_by_default = true diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb index 8d71b813e617bea6daa5b8dd48c9dfedbe421736..2bff21dae557461ba2ba6add12c9429513aaf58f 100644 --- a/railties/test/application/rake_test.rb +++ b/railties/test/application/rake_test.rb @@ -194,7 +194,10 @@ def test_scaffold_tests_pass_by_default assert_no_match(/Errors running/, output) end - def test_scaffold_with_references_columns_tests_pass_by_default + def test_scaffold_with_references_columns_tests_pass_when_belongs_to_is_optional + app_file "config/initializers/active_record_belongs_to_required_by_default.rb", + "Rails.application.config.active_record.belongs_to_required_by_default = false" + output = Dir.chdir(app_path) do `rails generate scaffold LineItems product:references cart:belongs_to; bundle exec rake db:migrate test` diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index ca26e0c8d7077a47ac8658c3613f378e0e0168b0..4c5dd70a88b8a89e162d424dea5adec7b1566165 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -209,6 +209,38 @@ def test_rails_update_set_the_cookie_serializer_to_marchal_if_it_is_not_already_ assert_file("#{app_root}/config/initializers/cookies_serializer.rb", /Rails\.application\.config\.action_dispatch\.cookies_serializer = :marshal/) end + def test_rails_update_does_not_create_active_record_belongs_to_required_by_default + app_root = File.join(destination_root, 'myapp') + run_generator [app_root] + + FileUtils.rm("#{app_root}/config/initializers/active_record_belongs_to_required_by_default.rb") + + Rails.application.config.root = app_root + Rails.application.class.stubs(:name).returns("Myapp") + Rails.application.stubs(:is_a?).returns(Rails::Application) + + generator = Rails::Generators::AppGenerator.new ["rails"], { with_dispatchers: true }, destination_root: app_root, shell: @shell + generator.send(:app_const) + quietly { generator.send(:update_config_files) } + assert_no_file "#{app_root}/config/initializers/active_record_belongs_to_required_by_default.rb" + end + + def test_rails_update_does_not_remove_active_record_belongs_to_required_by_default_if_already_present + app_root = File.join(destination_root, 'myapp') + run_generator [app_root] + + FileUtils.touch("#{app_root}/config/initializers/active_record_belongs_to_required_by_default.rb") + + Rails.application.config.root = app_root + Rails.application.class.stubs(:name).returns("Myapp") + Rails.application.stubs(:is_a?).returns(Rails::Application) + + generator = Rails::Generators::AppGenerator.new ["rails"], { with_dispatchers: true }, destination_root: app_root, shell: @shell + generator.send(:app_const) + quietly { generator.send(:update_config_files) } + assert_file "#{app_root}/config/initializers/active_record_belongs_to_required_by_default.rb" + end + def test_application_names_are_not_singularized run_generator [File.join(destination_root, "hats")] assert_file "hats/config/environment.rb", /Rails\.application\.initialize!/ @@ -309,6 +341,7 @@ def test_generator_without_skips def test_generator_if_skip_active_record_is_given run_generator [destination_root, "--skip-active-record"] assert_no_file "config/database.yml" + assert_no_file "config/initializers/active_record_belongs_to_required_by_default.rb" assert_file "config/application.rb", /#\s+require\s+["']active_record\/railtie["']/ assert_file "test/test_helper.rb" do |helper_content| assert_no_match(/fixtures :all/, helper_content)