Skip to content

Commit

Permalink
Add ignore_databases to incorrect_boolean_presence_validation
Browse files Browse the repository at this point in the history
This commit adds a new option for ignoring entire databases and models
backed by them. incorrect_boolean_presence_validation is the first
detector that gained that new option.
  • Loading branch information
gregnavis committed Oct 20, 2023
1 parent d322a6e commit 6aa3093
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 2 deletions.
1 change: 1 addition & 0 deletions lib/active_record_doctor/config/default.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

detector :incorrect_boolean_presence_validation,
enabled: true,
ignore_databases: [],
ignore_models: [],
ignore_attributes: []

Expand Down
4 changes: 3 additions & 1 deletion lib/active_record_doctor/detectors/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,14 @@ def underscored_name
self.class.underscored_name
end

def each_model(except: [], abstract: nil, existing_tables_only: false)
def each_model(except: [], ignore_databases: [], abstract: nil, existing_tables_only: false)
log("Iterating over Active Record models") do
models.each do |model|
case
when model.name.start_with?("HABTM_")
log("#{model.name} - has-belongs-to-many model; skipping")
when model.respond_to?(:connection_db_config) && ignored?(model.connection_db_config.name, ignore_databases)
log("#{model.name} - connects to the #{model.connection_db_config.name} which is ignored; skipping")
when ignored?(model.name, except)
log("#{model.name} - ignored via the configuration; skipping")
when abstract && !model.abstract_class?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ module Detectors
class IncorrectBooleanPresenceValidation < Base # :nodoc:
@description = "detect persence (instead of inclusion) validators on boolean columns"
@config = {
ignore_databases: {
description: "databases whose models should not be checked",
global: true
},
ignore_models: {
description: "models whose validators should not be checked",
global: true
Expand All @@ -25,7 +29,8 @@ def message(model:, attribute:)
end

def detect
each_model(except: config(:ignore_models), existing_tables_only: true) do |model|
each_model(except: config(:ignore_models), ignore_databases: config(:ignore_databases),
existing_tables_only: true) do |model|
each_attribute(model, except: config(:ignore_attributes)) do |column|
next unless column.type == :boolean
next unless has_presence_validator?(model, column)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,17 @@ def test_presence_true_is_reported_on_boolean_only
validates :email, :active, presence: true
end

# Another model is defined in a different database to ensure that all
# databases are checked.
SecondaryContext.create_table(:users) do |t|
t.boolean :active, null: false
end.define_model do
validates :active, presence: true
end

assert_problems(<<~OUTPUT)
replace the `presence` validator on Context::User.active with `inclusion` - `presence` can't be used on booleans
replace the `presence` validator on SecondaryContext::User.active with `inclusion` - `presence` can't be used on booleans
OUTPUT
end

Expand All @@ -33,6 +42,23 @@ def test_models_with_non_existent_tables_are_skipped
refute_problems
end

def test_config_ignore_databases
SecondaryContext.create_table(:users) do |t|
t.boolean :active, null: false
end.define_model do
validates :active, presence: true
end

config_file(<<-CONFIG)
ActiveRecordDoctor.configure do |config|
config.detector :incorrect_boolean_presence_validation,
ignore_databases: ["secondary"]
end
CONFIG

refute_problems
end

def test_config_ignore_models
Context.create_table(:users) do |t|
t.boolean :active, null: false
Expand Down Expand Up @@ -67,6 +93,22 @@ def test_config_ignore_models_regexp
refute_problems
end

def test_global_ignore_databases
SecondaryContext.create_table(:users) do |t|
t.boolean :active, null: false
end.define_model do
validates :active, presence: true
end

config_file(<<-CONFIG)
ActiveRecordDoctor.configure do |config|
config.global :ignore_databases, ["secondary"]
end
CONFIG

refute_problems
end

def test_global_ignore_models
Context.create_table(:users) do |t|
t.boolean :active, null: false
Expand Down

0 comments on commit 6aa3093

Please sign in to comment.