khiav223577 / deep_pluck

Allow you to pluck attributes from nested associations without loading a bunch of records.
MIT License
460 stars 14 forks source link

Issue with STI and polymorphism #44

Closed TaylorMerritt closed 3 years ago

TaylorMerritt commented 3 years ago

There seems to be an issue with selecting the appropriate type for use in a query when STI is involved.

class Parent
  ...
end
class Child
  has_one :information
end
class Information
 belongs_to :informationable, polymorphic: true
end

Attempting to use deep_pluck while create a query like the following:

> Child.deep_pluck(deep_information: [:id, :name])
...
SELECT informations.id, informations.name FROM "informations" WHERE ("informations"."informationable_type" = 'Child')

Comparatively, rails uses the parent of the STI for this query:

> Child.all.first.information 
...
SELECT informations.* FROM "informations" WHERE ("informations"."informationable_type" = 'Parent') AND ("informations"."informationable_id" = ...)

It seems like maybe this line needs to be addressed, but I'm not sure: https://github.com/khiav223577/deep_pluck/blob/master/lib/deep_pluck/model.rb#L120

khiav223577 commented 3 years ago

Hi @taylor-au , thanks for your report. Could you create an executable test case? See: https://edgeguides.rubyonrails.org/contributing_to_ruby_on_rails.html#create-an-executable-test-case for more details.


I wrote some test cases and found it works fine. See the test cases in the following:

begin
  require 'bundler/inline'
rescue LoadError => e
  warn 'Bundler version 1.10 or later is required. Please update your Bundler'
  raise e
end

gemfile(true) do
  source 'https://rubygems.org'
  gem 'deep_pluck', '~> 1.1.6'
  gem 'sqlite3'
end

require 'active_record'
require 'minitest/autorun'
require 'logger'

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table 'parent', force: :cascade do |t|
    t.string :name
  end

  create_table 'children', force: :cascade do |t|
    t.string :name
  end

  create_table 'information', force: :cascade do |t|
    t.string :name
    t.string :informationable_type
    t.integer :informationable_id
  end
end

class ApplicationRecord < ActiveRecord::Base
  self.abstract_class = true
end

class Parent < ApplicationRecord
end

class Child < ApplicationRecord
  has_one :information, as: :informationable
end

class Information < ApplicationRecord
  belongs_to :informationable, polymorphic: true
end

class BugTest < Minitest::Test
  def setup
    Child.create!(name: 'Child A', information: Information.new(name: 'Info A'))
  end

  def test_deep_pluck
    assert_equal Child.deep_pluck(:name, information: [:name]), [
      { 'name' => 'Child A', :information => { 'name' => 'Info A' } }
    ]
  end

  def test_model
    assert_equal Child.all.first.information.name, 'Info A'
  end
end
TaylorMerritt commented 3 years ago

This issue turned out to be an issue with the way my code was setup and Rails' STI pattern. It revolves around having multiple associations with another model that has a STI pattern and also associating with the parent class of the STI directly.

Small example of what was happening is say you have the following class:

class Consumer
  has_one :contact, as: :contactable
  has_one :main_contact, as: :contactable, class_name: 'MainContact' # MainContact is a child of Contact
end

Consumer.deep_pluck(contact: :name) will pluck contacts with a type of MainContact also. Was just a misunderstanding on my part of that part of STI.

Thanks for taking a look into this though!