diff --git a/lib/active_record_doctor/detectors/missing_non_null_constraint.rb b/lib/active_record_doctor/detectors/missing_non_null_constraint.rb index 40d050b..9e7474f 100644 --- a/lib/active_record_doctor/detectors/missing_non_null_constraint.rb +++ b/lib/active_record_doctor/detectors/missing_non_null_constraint.rb @@ -18,8 +18,18 @@ class MissingNonNullConstraint < Base # :nodoc: private + TIMESTAMPS = ["created_at", "created_on", "updated_at", "updated_on"].freeze + def message(column:, table:) - "add `NOT NULL` to #{table}.#{column} - models validates its presence but it's not non-NULL in the database" + if TIMESTAMPS.include?(column) + <<~WARN.squish + add `NOT NULL` to #{table}.#{column} - timestamp columns are set + automatically by Active Record and allowing NULL may lead to + inconsistencies introduced by bulk operations + WARN + else + "add `NOT NULL` to #{table}.#{column} - models validates its presence but it's not non-NULL in the database" + end end def detect @@ -50,6 +60,8 @@ def sti_base_model?(model) end def non_null_needed?(model, column) + return true if TIMESTAMPS.include?(column.name) + belongs_to = model.reflect_on_all_associations(:belongs_to).find do |reflection| reflection.foreign_key == column.name || (reflection.polymorphic? && reflection.foreign_type == column.name) diff --git a/test/active_record_doctor/detectors/missing_non_null_constraint_test.rb b/test/active_record_doctor/detectors/missing_non_null_constraint_test.rb index 580a868..b035896 100644 --- a/test/active_record_doctor/detectors/missing_non_null_constraint_test.rb +++ b/test/active_record_doctor/detectors/missing_non_null_constraint_test.rb @@ -310,4 +310,27 @@ def test_config_ignore_columns refute_problems end + + def test_timestamps_with_null_are_disallowed + Context.create_table(:users) do |t| + t.timestamps null: true + end.define_model + + assert_problems(<<~OUTPUT) + add `NOT NULL` to users.created_at - timestamp columns are set automatically by Active Record and allowing NULL may lead to inconsistencies introduced by bulk operations + add `NOT NULL` to users.updated_at - timestamp columns are set automatically by Active Record and allowing NULL may lead to inconsistencies introduced by bulk operations + OUTPUT + end + + def test_created_on_updated_on_with_null_are_disallowed + Context.create_table(:users) do |t| + t.datetime :created_on + t.datetime :updated_on + end.define_model + + assert_problems(<<~OUTPUT) + add `NOT NULL` to users.created_on - timestamp columns are set automatically by Active Record and allowing NULL may lead to inconsistencies introduced by bulk operations + add `NOT NULL` to users.updated_on - timestamp columns are set automatically by Active Record and allowing NULL may lead to inconsistencies introduced by bulk operations + OUTPUT + end end