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

undefined method `polymorphic?' for ActiveRecord::Reflection::PolymorphicReflection #1039

Closed
PhilCoggins opened this issue Jul 3, 2019 · 10 comments · Fixed by #1122
Closed

Comments

@PhilCoggins
Copy link
Contributor

PhilCoggins commented Jul 3, 2019

I am using Rails 5.2.3, and Ransack latest master(e17f1c6) and have found a potential bug.

It looks like this issue surfaces when trying to filter by a has_many through relationship where the source is a polymorphic belongs_to.

I am getting the below error:

undefined method `polymorphic?' for ActiveRecord::Reflection::PolymorphicReflection
# ./polyamorous/lib/polyamorous/activerecord_5.2.0_ruby_2/reflection.rb:4:in `build_join_constraint'
     # /Users/philcoggins/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rails-5dbae010a9d4/activerecord/lib/active_record/associations/join_dependency/join_association.rb:33:in `block in join_constraints'
     # /Users/philcoggins/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rails-5dbae010a9d4/activerecord/lib/active_record/associations/join_dependency/join_association.rb:29:in `reverse_each'
     # /Users/philcoggins/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rails-5dbae010a9d4/activerecord/lib/active_record/associations/join_dependency/join_association.rb:29:in `with_index'
     # /Users/philcoggins/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rails-5dbae010a9d4/activerecord/lib/active_record/associations/join_dependency/join_association.rb:29:in `join_constraints'
     # ./polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb:53:in `make_constraints'
     # /Users/philcoggins/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rails-5dbae010a9d4/activerecord/lib/active_record/associations/join_dependency.rb:183:in `block in walk'
     # /Users/philcoggins/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rails-5dbae010a9d4/activerecord/lib/active_record/associations/join_dependency.rb:183:in `each'
     # /Users/philcoggins/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rails-5dbae010a9d4/activerecord/lib/active_record/associations/join_dependency.rb:183:in `flat_map'
     # /Users/philcoggins/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rails-5dbae010a9d4/activerecord/lib/active_record/associations/join_dependency.rb:183:in `walk'
     # ./polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb:41:in `block in join_constraints'
     # ./polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb:38:in `each'
     # ./polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb:38:in `flat_map'
     # ./polyamorous/lib/polyamorous/activerecord_5.2.1_ruby_2/join_dependency.rb:38:in `join_constraints'
     # /Users/philcoggins/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rails-5dbae010a9d4/activerecord/lib/active_record/relation/query_methods.rb:1027:in `build_join_query'
     # /Users/philcoggins/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rails-5dbae010a9d4/activerecord/lib/active_record/relation/query_methods.rb:1009:in `build_joins'
     # /Users/philcoggins/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rails-5dbae010a9d4/activerecord/lib/active_record/relation/query_methods.rb:929:in `build_arel'
     # /Users/philcoggins/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rails-5dbae010a9d4/activerecord/lib/active_record/relation/query_methods.rb:903:in `arel'
     # /Users/philcoggins/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rails-5dbae010a9d4/activerecord/lib/active_record/relation.rb:454:in `block in to_sql'
     # /Users/philcoggins/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rails-5dbae010a9d4/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:206:in `unprepared_statement'
     # /Users/philcoggins/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rails-5dbae010a9d4/activerecord/lib/active_record/relation.rb:454:in `to_sql'
     # ./spec/ransack/search_spec.rb:295:in `block (3 levels) in <module:Ransack>'

I have forked your repository and produced a failing test case in a separate branch here. I had to nudge the models a bit to get my test case to work, but it does closely resemble a relationship that I have in my production application.

I have monkey-patched with the below, but I'm really not sure if this is what it's supposed to be (everything seems to be working great so far):

# overrides lib/polyamorous/activerecord_5.2.0_ruby_2/reflection.rb
module Polyamorous
  module ReflectionExtensions
    def build_join_constraint(table, foreign_table)
      if respond_to?(:polymorphic?) && polymorphic?
        super(table, foreign_table)
        .and(foreign_table[foreign_type].eq(klass.name))
      elsif @reflection.is_a?(ActiveRecord::Reflection::PolymorphicReflection)
        super(table, foreign_table)
        .and(foreign_table[@reflection.foreign_type].eq(klass.name))
      else
        super(table, foreign_table)
      end
    end
  end
end

Any assistance would be great, happy to work up a PR if someone can provide a nudge in the right direction.

Thanks for your time!

EDIT
I found an issue with the above monkey patch that was causing some queries with polymorphic joins to not add a filter on the polymorphic type. I have updated the original code in this issue.

leio10 added a commit to podemos-info/census that referenced this issue Aug 26, 2019
Don't update `ransack` until activerecord-hackery/ransack#1039 is fixed
leio10 added a commit to podemos-info/census that referenced this issue Aug 26, 2019
* Bump dependencies

Don't update `ransack` until activerecord-hackery/ransack#1039 is fixed

* Fix SMS tests when ESENDEX_USERNAME env variable is present.

* Remove dependent procedures feature.

* Rubocop issue

* Added missing test.
@StanBright
Copy link

StanBright commented Sep 19, 2019

Yup, I hit the same issue... and spent ~2h investigating and experimenting and reverting

The newly introduced polyamorous gem is hooking deep into ActiveRecord internals and messing things around.

@PhilCoggins
Copy link
Contributor Author

polyamorous has been around since the earliest release of Ransack AFAICT, but it was recently pulled into the library and the implementation for Rails 5.2 is just buggy.

@kuzmik
Copy link

kuzmik commented Sep 19, 2019

Same issue here, I see there is an issue opened in the polyamorous project but no response. I've just pinned to 2.1.1 until the library is updated :\

@deivid-rodriguez
Copy link
Contributor

@PhilCoggins or somebody else having this issue, can you try #1077 against your apps?

@scarroll32
Copy link
Member

Closed due to inactivity. Please reopen if still an issue.

@PhilCoggins
Copy link
Contributor Author

@seanfcarroll There's an open PR for this issue that you just commented in...

@scarroll32
Copy link
Member

@PhilCoggins which one?

@scarroll32
Copy link
Member

You mean this one: #1081

It can't be merged due to the 6.0 issue #1081

@scarroll32
Copy link
Member

I'll reopen it. If someone from the community can work on this it will be great.

@scarroll32 scarroll32 reopened this Jan 13, 2020
@PhilCoggins
Copy link
Contributor Author

@seanfcarroll I have started to look at this again as my organization is blocked on Rails upgrades until this issue is resolved.

The PR open for this issue is not #1081, it is #1077. This issue is unrelated to Rails 6 (though I have confirmed this is also broken in Rails 6), but rather how Ransack's mechanism for forming polymorphic joins is fundamentally broken since Rails 5.2.

If you have time, could you please see the linked PR and offer any guidance on how we can fix polymorphic joins?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants