diff --git a/NEWS.md b/NEWS.md index bedc45a6..da18ac2c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -27,6 +27,10 @@ also happened to affect `validate_confirmation_of` when an i18n translation key is passed to `with_message`. ([#593]) +* Fix `class_name` qualifier for association matchers so that if the model being + referenced is namespaced, the matcher will correctly resolve the class before + checking it against the association's `class_name`. ([#537]) + ### Features * Add ability to test `:primary_key` option on associations. ([#597]) @@ -42,6 +46,7 @@ [#584]: https://github.com/thoughtbot/shoulda-matchers/pull/584 [#593]: https://github.com/thoughtbot/shoulda-matchers/pull/593 [#597]: https://github.com/thoughtbot/shoulda-matchers/pull/597 +[#537]: https://github.com/thoughtbot/shoulda-matchers/pull/537 # 2.7.0 diff --git a/lib/shoulda/matchers/active_record/association_matcher.rb b/lib/shoulda/matchers/active_record/association_matcher.rb index 359b50c5..a336661b 100644 --- a/lib/shoulda/matchers/active_record/association_matcher.rb +++ b/lib/shoulda/matchers/active_record/association_matcher.rb @@ -1017,7 +1017,7 @@ module Shoulda def class_name_correct? if options.key?(:class_name) - if option_verifier.correct_for_string?(:class_name, options[:class_name]) + if option_verifier.correct_for_constant?(:class_name, options[:class_name]) true else @missing = "#{name} should resolve to #{options[:class_name]} for class_name" diff --git a/lib/shoulda/matchers/active_record/association_matchers/option_verifier.rb b/lib/shoulda/matchers/active_record/association_matchers/option_verifier.rb index 4eac35aa..84debf62 100644 --- a/lib/shoulda/matchers/active_record/association_matchers/option_verifier.rb +++ b/lib/shoulda/matchers/active_record/association_matchers/option_verifier.rb @@ -24,6 +24,10 @@ module Shoulda correct_for?(:hash, name, expected_value) end + def correct_for_constant?(name, expected_unresolved_value) + correct_for?(:constant, name, expected_unresolved_value) + end + def correct_for_relation_clause?(name, expected_value) correct_for?(:relation_clause, name, expected_value) end @@ -50,7 +54,7 @@ module Shoulda if expected_value.nil? true else - expected_value = type_cast(type, expected_value_for(name, expected_value)) + expected_value = type_cast(type, expected_value_for(type, name, expected_value)) actual_value = type_cast(type, actual_value_for(name)) expected_value == actual_value end @@ -65,9 +69,11 @@ module Shoulda end end - def expected_value_for(name, value) + def expected_value_for(type, name, value) if RELATION_OPTIONS.include?(name) expected_value_for_relation_clause(name, value) + elsif type == :constant + expected_value_for_constant(value) else value end @@ -78,6 +84,20 @@ module Shoulda reflector.extract_relation_clause_from(relation, name) end + def expected_value_for_constant(name) + namespace = Shoulda::Matchers::Util.deconstantize( + reflector.model_class.to_s + ) + + ["#{namespace}::#{name}", name].each do |path| + constant = Shoulda::Matchers::Util.safe_constantize(path) + + if constant + return constant + end + end + end + def actual_value_for_relation_clause(name) reflector.extract_relation_clause_from(reflector.association_relation, name) end diff --git a/lib/shoulda/matchers/util.rb b/lib/shoulda/matchers/util.rb index d14ef6cc..373d7091 100644 --- a/lib/shoulda/matchers/util.rb +++ b/lib/shoulda/matchers/util.rb @@ -10,6 +10,19 @@ module Shoulda path.to_s[0...(path.to_s.rindex('::') || 0)] end end + + def self.safe_constantize(camel_cased_word) + if defined?(ActiveSupport::Inflector) && + ActiveSupport::Inflector.respond_to?(:safe_constantize) + ActiveSupport::Inflector.safe_constantize(camel_cased_word) + else + begin + camel_cased_word.constantize + rescue NameError + nil + end + end + end end end end diff --git a/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb b/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb index 5c4ab36d..9110429c 100644 --- a/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb @@ -146,6 +146,31 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher do }.to fail_with_message(message) end + it 'accepts an association with a namespaced class name' do + define_module 'Models' + define_model 'Models::Organization' + user_model = define_model 'Models::User', organization_id: :integer do + belongs_to :organization, class_name: 'Organization' + end + + expect(user_model.new). + to belong_to(:organization). + class_name('Organization') + end + + it 'resolves class_name within the context of the namespace before the global namespace' do + define_module 'Models' + define_model 'Organization' + define_model 'Models::Organization' + user_model = define_model 'Models::User', organization_id: :integer do + belongs_to :organization, class_name: 'Organization' + end + + expect(user_model.new). + to belong_to(:organization). + class_name('Organization') + end + it 'accepts an association with a matching :autosave option' do define_model :parent, adopter: :boolean define_model :child, parent_id: :integer do @@ -456,6 +481,31 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher do }.to fail_with_message(message) end + it 'accepts an association with a namespaced class name' do + define_module 'Models' + define_model 'Models::Friend', user_id: :integer + friend_model = define_model 'Models::User' do + has_many :friends, class_name: 'Friend' + end + + expect(friend_model.new). + to have_many(:friends). + class_name('Friend') + end + + it 'resolves class_name within the context of the namespace before the global namespace' do + define_module 'Models' + define_model 'Friend' + define_model 'Models::Friend', user_id: :integer + friend_model = define_model 'Models::User' do + has_many :friends, class_name: 'Friend' + end + + expect(friend_model.new). + to have_many(:friends). + class_name('Friend') + end + it 'accepts an association with a matching :autosave option' do define_model :child, parent_id: :integer define_model :parent do @@ -642,26 +692,6 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher do expect(having_one_detail).to have_one(:detail).class_name('Detail') end - it 'accepts an association with a matching :autosave option' do - define_model :detail, person_id: :integer, disabled: :boolean - define_model :person do - has_one :detail, autosave: true - end - expect(Person.new).to have_one(:detail).autosave(true) - end - - it 'rejects an association with a non-matching :autosave option with the correct message' do - define_model :detail, person_id: :integer, disabled: :boolean - define_model :person do - has_one :detail, autosave: false - end - - message = 'Expected Person to have a has_one association called detail (detail should have autosave set to true)' - expect { - expect(Person.new).to have_one(:detail).autosave(true) - }.to fail_with_message(message) - end - it 'accepts an association with a valid :class_name option' do define_model :person_detail, person_id: :integer define_model :person do @@ -690,6 +720,52 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher do }.to fail_with_message(message) end + it 'accepts an association with a namespaced class name' do + define_module 'Models' + define_model 'Models::Account', user_id: :integer + user_model = define_model 'Models::User' do + has_one :account, class_name: 'Account' + end + + expect(user_model.new). + to have_one(:account). + class_name('Account') + end + + it 'resolves class_name within the context of the namespace before the global namespace' do + define_module 'Models' + define_model 'Account' + define_model 'Models::Account', user_id: :integer + user_model = define_model 'Models::User' do + has_one :account, class_name: 'Account' + end + + expect(user_model.new). + to have_one(:account). + class_name('Account') + end + + it 'accepts an association with a matching :autosave option' do + define_model :detail, person_id: :integer, disabled: :boolean + define_model :person do + has_one :detail, autosave: true + end + expect(Person.new).to have_one(:detail).autosave(true) + end + + it 'rejects an association with a non-matching :autosave option with the correct message' do + define_model :detail, person_id: :integer, disabled: :boolean + define_model :person do + has_one :detail, autosave: false + end + + message = 'Expected Person to have a has_one association called detail (detail should have autosave set to true)' + expect { + expect(Person.new).to have_one(:detail).autosave(true) + }.to fail_with_message(message) + end + + it 'accepts an association with a through' do define_model :detail @@ -917,6 +993,45 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher do }.to fail_with_message(message) end + it 'accepts an association with a namespaced class name' do + possible_join_table_names = [:groups_users, :models_groups_users, :groups_models_users] + possible_join_table_names.each do |join_table_name| + create_table join_table_name, id: false do |t| + t.integer :group_id + t.integer :user_id + end + end + define_module 'Models' + define_model 'Models::Group' + user_model = define_model 'Models::User' do + has_and_belongs_to_many :groups, class_name: 'Group' + end + + expect(user_model.new). + to have_and_belong_to_many(:groups). + class_name('Group') + end + + it 'resolves class_name within the context of the namespace before the global namespace' do + possible_join_table_names = [:groups_users, :models_groups_users, :groups_models_users] + possible_join_table_names.each do |join_table_name| + create_table join_table_name, id: false do |t| + t.integer :group_id + t.integer :user_id + end + end + define_module 'Models' + define_model 'Group' + define_model 'Models::Group' + user_model = define_model 'Models::User' do + has_and_belongs_to_many :groups, class_name: 'Group' + end + + expect(user_model.new). + to have_and_belong_to_many(:groups). + class_name('Group') + end + it 'accepts an association with a matching :autosave option' do define_model :relatives, adopted: :boolean define_model :person do