gregnavis / active_record_doctor

Identify database issues before they hit production.
MIT License
1.76k stars 55 forks source link

Fix `missing_non_null_constraint` when there are no STI children for STI-like parent #139

Closed fatkodima closed 2 months ago

fatkodima commented 1 year ago

When the STI-like parent model has no children, all the next branches in the detect method pass (specifically next if !concrete_models.all? { |model| non_null_needed?(model, column) }) and MissingNonNullConstraint erroneously adds offenses for the column. See a test case for the example.

gregnavis commented 2 months ago

@fatkodima, I don't believe this PR is correct. I think it's perfectly valid to instantiate the base class of an STI hierarchy: in this case type will be nil. A subclass of the STI base class will have its name stored in type.

A proper fix for the problem would be to skip the check on the type column.

fatkodima commented 2 months ago

Agreed, fixed.

gregnavis commented 2 months ago

Perfect! Thank you, @fatkodima.