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

Make "class_name" option for polymorphic has_many relation to STI required #48925

Closed
loqimean opened this issue Aug 10, 2023 · 8 comments · Fixed by #49101
Closed

Make "class_name" option for polymorphic has_many relation to STI required #48925

loqimean opened this issue Aug 10, 2023 · 8 comments · Fixed by #49101

Comments

@loqimean
Copy link

Steps to reproduce

Models:

class Product < ApplicationRecord
  has_many :requests, as: :requestable, dependent: :destroy
end

# STI tables
class Request < ApplicationRecord
  belongs_to :requestable, polymorphic: true

  validate :request_type, presence: true
end

class ProductRequest < Request
  belongs_to :user
end

Then running in your console:

Product.joins(requests: :user).where(requests: { user: { role: "admin" } })

Raises you an error:

/Users/some/.rvm/gems/ruby-3.1.1/gems/activerecord-7.0.4/lib/active_record/table_metadata.rb:22:in `has_column?': undefined method `key?' for nil:NilClass (NoMethodError)

Expected behavior

To raise a detailed error, like "provide a class_name option to use joins on STI table with polymorphic, or make required option class_name for polymorphic relation with STI", like:

class Product < ApplicationRecord
  has_many :requests, as: :requestable, class_name: "ProductRequest", dependent: :destroy
end

# STI tables
class Request < ApplicationRecord
  belongs_to :requestable, polymorphic: true

  validate :request_type, presence: true
end

class ProductRequest < Request
  belongs_to :user
end

Actual behavior

Unknown error 🙂

/Users/some/.rvm/gems/ruby-3.1.1/gems/activerecord-7.0.4/lib/active_record/table_metadata.rb:22:in `has_column?': undefined method `key?' for nil:NilClass (NoMethodError)

System configuration

Rails version: 7.0.4

Ruby version: 3.1.1

@rosanelli
Copy link

+1

@xfifix
Copy link
Contributor

xfifix commented Aug 31, 2023

+1

i interest because i have the same problem on my projet ! But i don't want tell the class_name, i want a belongs_to or has_many on the Request and finds the right object on its own.

@xfifix
Copy link
Contributor

xfifix commented Sep 1, 2023

@byroot do you have an idea ? :)

@matthewd
Copy link
Member

matthewd commented Sep 1, 2023

It's definitely a bug; this should be using &.:

klass&.columns_hash.key?(column_name)

I imagine I'll be happy with whatever behaviour that restores, though. "This might be pointing at an STI class and one of the STI children might have an association with the name you asked for, and you might know that all instances referencing this object are of that class" is clearly possible, but I don't foresee us encoding that level of detail into an error.

If anyone's up for an easy PR, ☝🏻 plus a test would be welcome.

@xfifix
Copy link
Contributor

xfifix commented Sep 1, 2023

@matthewd i seems to work :D

@xfifix
Copy link
Contributor

xfifix commented Sep 1, 2023

@matthewd i will to try do an PR ;)

byroot added a commit that referenced this issue Sep 1, 2023
[Fix #48925] fix "class_name" required for STI class
byroot added a commit that referenced this issue Sep 1, 2023
[Fix #48925] fix "class_name" required for STI class
@byroot
Copy link
Member

byroot commented Sep 1, 2023

Backported to 7-0-stable as c1150f4

@zzak
Copy link
Member

zzak commented Sep 27, 2023

plus a test would be welcome

I think we missed this but just incase anyone is looking for something to do 👀

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

Successfully merging a pull request may close this issue.

7 participants