Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

should have_many().through gives undefined method class_name' for nil:NilClass` #646

Closed
marnen opened this issue Feb 3, 2015 · 16 comments
Closed

Comments

@marnen
Copy link

marnen commented Feb 3, 2015

I've just run into a problem with shoulda-matchers 2.7.0 and 2.8.0, Rails 4.2, and Ruby 2.2.0 on Mac OS X 10.10. The complete application is at https://github.com/marnen/duckbill/tree/shoulda-matchers-issue , if you need to see the code in context.

Here's the relevant code:

# user.rb
has_many :clients
has_many :projects, through: :clients
# user_spec.rb
it { should have_many :clients }
it { should have_many(:projects).through :clients }

I would expect both specs to pass, but only the first one does. The second one gives the following error:

Failures:

  1) User should have many projects through clients
     Failure/Error: it {  should have_many(:projects).through :clients }
     NoMethodError:
       undefined method `class_name' for nil:NilClass
     # /Users/marnen/.rvm/gems/ruby-2.2.0/gems/activerecord-4.2.0/lib/active_record/reflection.rb:871:in `derive_class_name'
     # /Users/marnen/.rvm/gems/ruby-2.2.0/gems/activerecord-4.2.0/lib/active_record/reflection.rb:147:in `class_name'
     # /Users/marnen/.rvm/gems/ruby-2.2.0/gems/shoulda-matchers-2.8.0/lib/shoulda/matchers/active_record/association_matcher.rb:1067:in `rescue in class_exists?'
     # /Users/marnen/.rvm/gems/ruby-2.2.0/gems/shoulda-matchers-2.8.0/lib/shoulda/matchers/active_record/association_matcher.rb:1064:in `class_exists?'
     # /Users/marnen/.rvm/gems/ruby-2.2.0/gems/shoulda-matchers-2.8.0/lib/shoulda/matchers/active_record/association_matcher.rb:928:in `matches?'
     # ./spec/models/user_spec.rb:5:in `block (2 levels) in <top (required)>'

I have no idea what's going on here. Is this a Rails 4.2 incompatibility? Or something else? Help?

@lucianosousa
Copy link
Contributor

Have you tried this?

it { should have_many(:projects).through(:clients) }

with parenthesis

@marnen
Copy link
Author

marnen commented Feb 3, 2015

I have no reason to believe that would make any difference at all, though I'll try it to be sure. Assuming I'm correct about that, what else would you advise?

@marnen
Copy link
Author

marnen commented Feb 3, 2015

I tried it with parentheses. Exact same issue.

@marnen
Copy link
Author

marnen commented Feb 3, 2015

Apparently, the problem was that while I had done User.has_many :projects, through: :clients, I had not declared Client.has_many :projects. Leaving this open because a more informative error message might be helpful.

@mcmire
Copy link
Collaborator

mcmire commented May 12, 2015

This will also occur if you don't define a proper belongs_to association in the model you're going through.

@maurogeorge
Copy link
Contributor

@mcmire I started working on a fix to this on maurogeorge@48895ad.

It seens that AR treat this conditions and we can check for errors using the reflection.check_validity!, I wrote more useful info on the commit message before.

@mcmire
Copy link
Collaborator

mcmire commented May 12, 2015

Okay, that's a good start. I'd probably want a more specific error message in that case, but nice. Submit a PR and I'll take a closer look. (And btw I'm aware of your other PR's, I haven't had time lately to look at them but I'll get to them soon.)

@maurogeorge
Copy link
Contributor

I have worked on this today @mcmire master...maurogeorge:have_many-through-error. But on the rails 4.1 I got a broken spec, I suspect that is a regression bug on Rails since it works on the 4.0.0, 4.0.1and 4.2.

I will take a look on it.

@maurogeorge
Copy link
Contributor

@mcmire It was a bug on Rails 4.1 I send a patch and this was merged.

I will now take a look at this issue.

@mcmire
Copy link
Collaborator

mcmire commented May 28, 2015

Oh, very cool. Good to hear. Thanks for the work.

@ddavisgraphics
Copy link

ddavisgraphics commented Aug 2, 2019

I had the same issue with a reverse polymorphic association, the only error message was undefined method 'class_name'. Maybe that is expected, but even in the stack trace it was ambiguous and looked like it could have been a number of different errors. Is there a way to add a more concise error message?

Here is the code in case it helps.

class Field < ApplicationRecord
  belongs_to :form # this was the offender once added it worked
  belongs_to :field_types, polymorphic: true
end

class Form < ApplicationRecord
  has_many :fields, dependent: :destroy
end

class TypeA < ApplicationRecord
  has_one :form, through: :field
  has_one :field, as: :field_types
end

class TypeB < ApplicationRecord
  has_one :form, through: :field
  has_one :field, as: :field_types
end

@jmuheim
Copy link

jmuheim commented Sep 5, 2019

I'm getting this error with the following models:

class Project < ApplicationRecord
  has_many :wcag_criterion_proxies, dependent: :destroy
  has_many :wcag_criteria, through: :wcag_criterion_proxies
end

class WcagCriterionProxy < ApplicationRecord
  belongs_to :project
  belongs_to :wcag_criterion
end

class WcagCriterion < WcagElement
  has_many :wcag_criterion_proxies, dependent: :destroy
  has_many :projects, through: :wcag_criterion_proxies
end

Can anybody see the problem? I don't see it.

@CharlieIGG
Copy link

Apparently, the problem was that while I had done User.has_many :projects, through: :clients, I had not declared Client.has_many :projects. Leaving this open because a more informative error message might be helpful.

This has also happened to me repeatedly, and each time I end up coming back to this thread (because my memory is bad). How can we add more significant error messages for this?

@mcmire
Copy link
Collaborator

mcmire commented Nov 14, 2019

#723 should fix this issue. It's just one of many, many PRs that we need to review. @guialbuk is this something you think you could take a look at?

@guialbuk
Copy link
Collaborator

For sure. I'm working on having that PR in a state that it can be merged, so we can fix that issue.

@mcmire
Copy link
Collaborator

mcmire commented Jul 12, 2020

A fix for this is now in master: c0a1578

@mcmire mcmire closed this as completed Jul 12, 2020
@mcmire mcmire mentioned this issue Jul 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants