From 78ab821977d2fdc9adc9f8bc07bfa4b3d4833225 Mon Sep 17 00:00:00 2001 From: Chedli Bourguiba Date: Sat, 14 Oct 2023 23:43:41 +0200 Subject: [PATCH] Add support for rails 7.1 Fixes #230. rails 7.1 recently added this new default [configuration option](https://guides.rubyonrails.org/configuring.html#config-active-record-belongs-to-required-validates-foreign-key ) When set to false ( default in rails 7.1 ) an `if` option is added to the belongs_to option. which makes it not compatible with the current `WEAK_OPTIONS` array. To cover this new case, and as a pre-check, we make sure that we're on rails 7.1.0 minimum as well as the ActiveRecord option ( `ActiveRecord.belongs_to_required_validates_foreign_key` ) is set to false to mitigate issues with real `if` conditions. We also check that the if condition is coupled with a `message` option set to `required` This commit also adds rails 7.1 to the CI matrix. --- .github/workflows/tests.yml | 2 +- gemfiles/ar_7_1.gemfile | 7 ++++ .../column_presence_checker.rb | 18 ++++++++++- spec/checkers/column_presence_checker_spec.rb | 32 +++++++++++++++++++ 4 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 gemfiles/ar_7_1.gemfile diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index aeb15d1..f87b2f8 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -51,7 +51,7 @@ jobs: - ruby-version: '3.1' gemfile: 'gemfiles/ar_6_1.gemfile' - ruby-version: '3.2' - gemfile: 'gemfiles/ar_7_0.gemfile' + gemfile: ['gemfiles/ar_7_0.gemfile', 'gemfiles/ar_7_1.gemfile'] - ruby-version: 'head' gemfile: 'gemfiles/ar_main.gemfile' diff --git a/gemfiles/ar_7_1.gemfile b/gemfiles/ar_7_1.gemfile new file mode 100644 index 0000000..7ac21e5 --- /dev/null +++ b/gemfiles/ar_7_1.gemfile @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +source 'https://rubygems.org' + +gem 'activerecord', '~> 7.1.0' + +gemspec path: '../' diff --git a/lib/database_consistency/checkers/validators_fraction_checkers/column_presence_checker.rb b/lib/database_consistency/checkers/validators_fraction_checkers/column_presence_checker.rb index f54683b..dd57404 100644 --- a/lib/database_consistency/checkers/validators_fraction_checkers/column_presence_checker.rb +++ b/lib/database_consistency/checkers/validators_fraction_checkers/column_presence_checker.rb @@ -34,7 +34,23 @@ def report_template(status, column_name:, error_slug: nil) end def weak_option? - validators.all? { |validator| validator.options.slice(*WEAK_OPTIONS).any? } + validators.all? do |validator| + result = validator.options.slice(*WEAK_OPTIONS).any? + + result = false if required_with_if?(validator) + + result + end + end + + def belongs_to_required_validates_foreign_key_disabled? + ActiveRecord.version >= Gem::Version.new('7.1.0') && + !ActiveRecord.belongs_to_required_validates_foreign_key + end + + def required_with_if?(validator) + belongs_to_required_validates_foreign_key_disabled? && + validator.options[:message] == :required && validator.options[:if].present? end def check diff --git a/spec/checkers/column_presence_checker_spec.rb b/spec/checkers/column_presence_checker_spec.rb index 832f2e9..7cc814b 100644 --- a/spec/checkers/column_presence_checker_spec.rb +++ b/spec/checkers/column_presence_checker_spec.rb @@ -158,6 +158,25 @@ context 'when `belongs_to` is required' do let(:klass) { define_class { |klass| klass.belongs_to :user, **required } } + if ActiveRecord.version >= Gem::Version.new('7.1.0') + context 'when belongs_to_required_validates_foreign_key is set to false' do + specify do + old = ActiveRecord.belongs_to_required_validates_foreign_key + ActiveRecord.belongs_to_required_validates_foreign_key = false + + expect(checker.report.first).to have_attributes( + checker_name: 'ColumnPresenceChecker', + table_or_model_name: klass.name, + column_or_attribute_name: 'user', + status: :fail, + error_message: nil, + error_slug: :association_missing_null_constraint + ) + + ActiveRecord.belongs_to_required_validates_foreign_key = old + end + end + end specify do expect(checker.report.first).to have_attributes( checker_name: 'ColumnPresenceChecker', @@ -204,6 +223,19 @@ context 'when `belongs_to` is optional' do let(:klass) { define_class { |klass| klass.belongs_to :user, **optional } } + if ActiveRecord.version >= Gem::Version.new('7.1.0') + context 'when belongs_to_required_validates_foreign_key is set to false' do + specify do + old = ActiveRecord.belongs_to_required_validates_foreign_key + ActiveRecord.belongs_to_required_validates_foreign_key = false + + expect(checker.report).to be_nil + + ActiveRecord.belongs_to_required_validates_foreign_key = old + end + end + end + specify do expect(checker.report).to be_nil end