Merge pull request #18937 from simi/optional-belongs-to

Require `belongs_to` by default.

activerecord/CHANGELOG.md

+*   `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.

activerecord/lib/active_record/associations.rb

@@ -1520,10 +1520,16 @@ def has_one(name, scope = nil, options = {})
       #   object that is the inverse of this <tt>belongs_to</tt> association. Does not work in
       #   combination with the <tt>:polymorphic</tt> 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: <tt>required</tt> is set to <tt>true</tt> by default and is deprecated. If
+      #   you don't want to have association presence validated, use <tt>optional: true</tt>.
+      #
+      #
       #
       # 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

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

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

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

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

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
 

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:

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.

railties/CHANGELOG.md

+*   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.

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

railties/lib/rails/generators/rails/app/templates/config/initializers/active_record_belongs_to_required_by_default.rb

+# 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

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`

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)