thoughtbot / shoulda-matchers

Simple one-liner tests for common Rails functionality
https://matchers.shoulda.io
MIT License
3.51k stars 912 forks source link

Conditions qualifier, when association owner is part of the condition #952

Closed AlexVPopov closed 4 years ago

AlexVPopov commented 8 years ago

How do you validate the conditions for a has_one/has_many association, if the association owner is part of the condition? Example:

class Foo
  has_many :bars, -> (foo) { where(baz_id: foo.default_baz_id }
  belongs_to :default_baz, class_name: 'Baz'
end

class Bar
  belongs_to :foo
  belongs_to :baz
end

I tried both conditions(where(baz_id: foo.default_baz_id) and conditions(-> (foo) { where(baz_id: foo.default_baz_id }), but I get:

NoMethodError:
  undefined method `default_baz' for #<Class:0x007fc2d6ca7dd0>
mcmire commented 8 years ago

Hmm... would something like this work?

it do
  foo = Foo.new(default_baz_id: 42)
  expect(foo).to have_many(:bars).conditions(bars: { baz_id: 42 })
end
AlexVPopov commented 8 years ago

Thank you for your prompt response! :+1: Unfortunately this doesn't work, because I get an error

NoMethodError:
  undefined method `default_baz_id' for #<Class:0x0056340c8edf28>
  Did you mean?  default_scoped

I think that without the conditions method call, the lambda is not evaluated at all. if I add the proposed conditions call, than the lambda is evaluated. However, inside of it:

 has_many :bars, -> (foo) { binding.pry; where(baz_id: foo.default_baz_id }

foo is not an instance of the class, but the actual class and hence the error.

mcmire commented 8 years ago

Can you re-run that test with --backtrace? I'm curious as to where that error is appearing.

AlexVPopov commented 8 years ago

Sure, there you go:

Failures:

  1) MiniCms::LandingPage associations should have one default_translation class_name => LandingPageTranslation dependent => delete
     Failure/Error: ->(landing_page) { where(locale_id: landing_page.default_locale_id) },

     NoMethodError:
       undefined method `default_locale_id' for #<Class:0x007fea79e06918>
       Did you mean?  default_scoped
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activerecord-4.2.6/lib/active_record/dynamic_matchers.rb:26:in `method_missing'
     # ./app/models/mini_cms/landing_page.rb:9:in `block in <class:LandingPage>'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/shoulda-matchers-3.1.1/lib/shoulda/matchers/active_record/association_matchers/model_reflection.rb:43:in `instance_exec'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/shoulda-matchers-3.1.1/lib/shoulda/matchers/active_record/association_matchers/model_reflection.rb:43:in `association_relation'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/shoulda-matchers-3.1.1/lib/shoulda/matchers/active_record/association_matchers/model_reflector.rb:7:in `association_relation'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/shoulda-matchers-3.1.1/lib/shoulda/matchers/active_record/association_matchers/option_verifier.rb:105:in `actual_value_for_relation_clause'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/shoulda-matchers-3.1.1/lib/shoulda/matchers/active_record/association_matchers/option_verifier.rb:51:in `actual_value_for'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/shoulda-matchers-3.1.1/lib/shoulda/matchers/active_record/association_matchers/option_verifier.rb:44:in `correct_for?'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/shoulda-matchers-3.1.1/lib/shoulda/matchers/active_record/association_matchers/option_verifier.rb:32:in `correct_for_relation_clause?'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/shoulda-matchers-3.1.1/lib/shoulda/matchers/active_record/association_matcher.rb:1135:in `conditions_correct?'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/shoulda-matchers-3.1.1/lib/shoulda/matchers/active_record/association_matcher.rb:979:in `matches?'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-expectations-3.5.0/lib/rspec/expectations/handler.rb:50:in `block in handle_matcher'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-expectations-3.5.0/lib/rspec/expectations/handler.rb:27:in `with_matcher'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-expectations-3.5.0/lib/rspec/expectations/handler.rb:48:in `handle_matcher'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-expectations-3.5.0/lib/rspec/expectations/expectation_target.rb:65:in `to'
     # ./spec/models/mini_cms/landing_page_spec.rb:19:in `block (3 levels) in <top (required)>'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example.rb:252:in `instance_exec'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example.rb:252:in `block in run'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example.rb:494:in `block in with_around_and_singleton_context_hooks'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example.rb:451:in `block in with_around_example_hooks'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/hooks.rb:471:in `block in run'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/hooks.rb:611:in `block in run_around_example_hooks_for'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example.rb:336:in `call'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-rails-3.5.0/lib/rspec/rails/adapters.rb:127:in `block (2 levels) in <module:MinitestLifecycleAdapter>'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example.rb:441:in `instance_exec'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example.rb:441:in `instance_exec'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/hooks.rb:382:in `execute_with'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/hooks.rb:613:in `block (2 levels) in run_around_example_hooks_for'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example.rb:336:in `call'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/hooks.rb:614:in `run_around_example_hooks_for'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/hooks.rb:471:in `run'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example.rb:451:in `with_around_example_hooks'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example.rb:494:in `with_around_and_singleton_context_hooks'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example.rb:249:in `run'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example_group.rb:613:in `block in run_examples'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example_group.rb:609:in `map'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example_group.rb:609:in `run_examples'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example_group.rb:575:in `run'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example_group.rb:576:in `block in run'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example_group.rb:576:in `map'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/example_group.rb:576:in `run'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/runner.rb:113:in `block (3 levels) in run_specs'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/runner.rb:113:in `map'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/runner.rb:113:in `block (2 levels) in run_specs'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/configuration.rb:1837:in `with_suite_hooks'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/runner.rb:112:in `block in run_specs'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/reporter.rb:77:in `report'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/runner.rb:111:in `run_specs'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/runner.rb:87:in `run'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/runner.rb:71:in `run'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/lib/rspec/core/runner.rb:45:in `invoke'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rspec-core-3.5.0/exe/rspec:4:in `<top (required)>'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-4.2.6/lib/active_support/dependencies.rb:268:in `load'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-4.2.6/lib/active_support/dependencies.rb:268:in `block in load'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-4.2.6/lib/active_support/dependencies.rb:240:in `load_dependency'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-4.2.6/lib/active_support/dependencies.rb:268:in `load'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/spring-commands-rspec-1.0.4/lib/spring/commands/rspec.rb:18:in `call'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
     # /home/alex/.rbenv/versions/2.3.1/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
     # -e:1:in `<main>'

Finished in 1.31 seconds (files took 0.39637 seconds to load)
1 example, 1 failure
mcmire commented 8 years ago

Ah, okay... I see what you're saying.

The gem ends up comparing what you pass to conditions with the scope (lambda) that the association is defined with by manually calling that lambda. If you look here, we try to emulate what ActiveRecord is doing and we call the lambda with the ActiveRecord that the association is associated with. In the code this is subject and we get that by calling active_record on the association reflection object. I just took a look at the Rails code (also here) where associations are resolved and it looks like we need to use association.owner and not reflection.active_record.

As far as what you can do to get around this... I'm not sure. I think you might just have to test this manually. Sorry about that.

AlexVPopov commented 8 years ago

As always - an outstanding response - fast, kind and insightful. Thank you very much for the help!

mcmire commented 8 years ago

Yeah, no problem! And I'm going to keep this open since this is definitely something that should be fixed.

AlexVPopov commented 8 years ago

:+1:

mcmire commented 4 years ago

Hey folks. In an effort to lighten our load as maintainers and be able to serve you better in the future, the shoulda-matchers team is working on cleaning out the cobwebs in this repo by pruning the backlog. As there are few of us, there are a lot of items that will simply never earn our attention in a reasonable time frame, and rather than giving you an empty promise, we think it makes more sense to focus on more recent issues. That means, unfortunately, that we must close this issue.

Don't take this the wrong way: our aim is not to diminish the effort people have made or dismiss problems that have been raised. If you feel that we should reopen this issue, then please let us know so that we can reprioritize it. Thanks!

mcmire commented 4 years ago

Reopening this issue since it depends on a PR.