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 aaba6e7
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 3 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
8 changes: 6 additions & 2 deletions 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 Expand Up @@ -215,7 +217,7 @@ def each_index(table_name, except: [], multicolumn_only: false)

def each_attribute(model, except: [], type: nil)
log("Iterating over attributes of #{model.name}") do
connection.columns(model.table_name).each do |column|
model.connection.columns(model.table_name).each do |column|
case
when ignored?("#{model.name}.#{column.name}", except)
log("#{model.name}.#{column.name} - ignored via the configuration; skipping")
Expand Down Expand Up @@ -323,6 +325,8 @@ def each_association(model, except: [], type: [:has_many, :has_one, :belongs_to]
end

def ignored?(name, patterns)
puts "name = #{name.inspect}"
puts "patterns = #{patterns.inspect}"
patterns.any? { |pattern| pattern === name } # rubocop:disable Style/CaseEquality
end
end
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,11 @@ 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 @@ -17,6 +17,20 @@ def test_presence_true_is_reported_on_boolean_only
OUTPUT
end

def test_mutli_database_support
skip if !multi_database_support?

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 SecondaryContext::User.active with `inclusion` - `presence` can't be used on booleans
OUTPUT
end

def test_inclusion_is_not_reported
Context.create_table(:users) do |t|
t.boolean :active, null: false
Expand All @@ -33,6 +47,25 @@ def test_models_with_non_existent_tables_are_skipped
refute_problems
end

def test_config_ignore_databases
skip if !multi_database_support?

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 +100,24 @@ def test_config_ignore_models_regexp
refute_problems
end

def test_global_ignore_databases
skip if !multi_database_support?

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
4 changes: 4 additions & 0 deletions test/setup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ def mysql?
ActiveRecord::Base.connection.adapter_name == "Mysql2"
end

def multi_database_support?
ActiveRecord::VERSION::MAJOR >= 6
end

def detector_name
self.class.name.sub(/Test$/, "").demodulize.underscore.to_sym
end
Expand Down

0 comments on commit aaba6e7

Please sign in to comment.