Missing Presence Validation with Polymorphic Type Column

[Numie]

I have a polymorphic association like:

belongs_to :documentable, polymorphic: true

There are not_null db constraints on the related documentable_id and documentable_type columns. When I run active_record_doctor, I get the following error:

add a `presence` validator to Document.documentable_type - it's NOT NULL but lacks a validator

I don't think an error should be raised since ActiveRecord already requires both the type and id columns from the single belongs_to association.

[gregnavis]

@Numie, I think what's wrong here is that the error message should be different: it should tell you to add optional: false to the association. I'm working on that enhancement.

Please double-check this, but I think if you do the following:

model = YourModel.new(documentable: nil)
model.validate

Then you won't find an error on documentable.

[fatkodima]

belongs_to associations are required by default, so do not get why you should add optional: false to them? Since OP has not null constraints on both columns, there should not be any error messages from this gem.

[gregnavis]

@fatkodima, you're right. I was confused by the fact that belongs_to seem not to be required by default in active_record_doctor test suite. For example, when I took your PR and made the following change:

  def test_non_null_column_is_not_reported_if_polymorphic_association_validation_present
    Context.create_table(:images) do |t|
      t.references :imageable, null: false, polymorphic: true
    end.define_model do
      # I removed required from the association.
      belongs_to :imageable, polymorphic: true
    end

    refute_problems
  end

Then I got an error:

% DATABASE_ADAPTER=postgresql ruby -Ilib -Itest -rsetup test/active_record_doctor/detectors/missing_presence_validation_test.rb -n test_non_null_column_is_not_reported_if_polymorphic_association_validation_present
Using postgresql
Run options: -n test_non_null_column_is_not_reported_if_polymorphic_association_validation_present --seed 40829

# Running:

F

Failure:
ActiveRecordDoctor::Detectors::MissingPresenceValidationTest#test_non_null_column_is_not_reported_if_polymorphic_association_validation_present [test/active_record_doctor/detectors/missing_presence_validation_test.rb:52]:
--- expected
+++ actual
@@ -1 +1 @@
-[]
+["add a `presence` validator to Context::Image.imageable_id - it's NOT NULL but lacks a validator", "add a `presence` validator to Context::Image.imageable_type - it's NOT NULL but lacks a validator"]



bin/rails test test/active_record_doctor/detectors/missing_presence_validation_test.rb:45

Are you able to reproduce it on your end?

[fatkodima]

Yes, I get the same error with your changes.

belongs_to_required_by_default is set to nil in ActiveRecord by default and is set to true in a rails app via https://github.com/rails/rails/blob/f0d66d6c47b9def492a6b5f70a7a45907e38fadc/railties/lib/rails/application/configuration.rb#L120-L122

So we need to add to test/setup.rb

ActiveRecord::Base.belongs_to_required_by_default = true

or leave a required: true as in the original test case.

[gregnavis]

Thanks for the link. Sadly, it seems the default is not set within the active record gem.

I prefer not to add an explicit dependency on the rails. however, given required is deprecated I suggest we pass optional explicitly. I'll leave some more feedback on the PR.