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

Don't add association name as key to returned json #33

Closed byt3pool closed 4 years ago

byt3pool commented 4 years ago

Is there a way to flatten the response of deep_pluck to not add the name of an association as key? (Please see current and wanted outcome examples below)

I have something like 'flat_deep_pluck' in my mind but what I basically want to achieve:

Associations

# Vehicle.rb
has_many :logbooks
has_many :sessions, through: :logbooks

# Session.rb
belongs_to :logbook
has_one :vehicle, through: :logbook

# Logbook.rb
belongs_to :vehicle
has_many :sessions

Query

Session.deep_pluck(:name, 'vehicles' => :manufacturer)

Current outcome

{
  "name": "Session_01",
  "logbook": {
    "vehicle": {
      "manufacturer": "BMW"
    }
  }
}

What I would like to get as outcome

{
  "name": "Session_01",
  "vehicle": {
    "manufacturer": "BMW"
  }
}
khiav223577 commented 4 years ago

@byt3pool Thanks for you report. I tried to reproduce the environment as you described. Didn't get an outcome but raise exceptions. There must have some bugs here. 🐛

I tested it with deep_pluck 1.1.2, pluck_all 2.0.4 and rails 6.0.1. Could you check which version are you using?

Test scripts

#!/usr/bin/env ruby
# frozen_string_literal: true

gem 'bundler'

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

gemfile(true) do
  source 'https://rubygems.org'

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem 'deep_pluck'
  gem 'sqlite3'
end

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

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :vehicles, force: true do |t|
    t.string :manufacturer
  end

  create_table :sessions, force: true do |t|
    t.string :name
    t.integer :logbook_id
  end

  create_table :logbooks, force: true do |t|
    t.integer :vehicle_id
  end
end

class Vehicle < ActiveRecord::Base
  has_many :logbooks
  has_many :sessions, through: :logbooks
end

class Session < ActiveRecord::Base
  belongs_to :logbook
  has_one :vehicle, through: :logbook
end

class Logbook < ActiveRecord::Base
  belongs_to :vehicle
  has_many :sessions
end

vehicle = Vehicle.create(manufacturer: 'BMW')
logbook = Logbook.create(vehicle: vehicle)
Session.create(name: 'Session_01', logbook: logbook)

class BugTest < Minitest::Test
  def test_deep_pluck
    expected = {
      'name' => 'Session_01',
      'vehicle' => {
        'manufacturer' => 'BMW'
      }
    }

    assert_equal expected, Session.deep_pluck(:name, 'vehicles' => :manufacturer)
  end
end

Error messages

1) Error:
BugTest#test_deep_pluck:
ActiveRecord::ConfigurationError: ActiveRecord::ConfigurationError: Association named           'vehicles' was not found on Session; perhaps you misspelled it?
    /Users/khiav223577/.rvm/gems/ruby-2.6.4/gems/deep_pluck-1.1.2/lib/deep_pluck/model.rb:30:in `get_reflect'
    /Users/khiav223577/.rvm/gems/ruby-2.6.4/gems/deep_pluck-1.1.2/lib/deep_pluck/model.rb:76:in `block in add_association'
    /Users/khiav223577/.rvm/gems/ruby-2.6.4/gems/deep_pluck-1.1.2/lib/deep_pluck/model.rb:75:in `each'
    /Users/khiav223577/.rvm/gems/ruby-2.6.4/gems/deep_pluck-1.1.2/lib/deep_pluck/model.rb:75:in `add_association'
    /Users/khiav223577/.rvm/gems/ruby-2.6.4/gems/deep_pluck-1.1.2/lib/deep_pluck/model.rb:88:in `block in add'
    /Users/khiav223577/.rvm/gems/ruby-2.6.4/gems/deep_pluck-1.1.2/lib/deep_pluck/model.rb:86:in `each'
    /Users/khiav223577/.rvm/gems/ruby-2.6.4/gems/deep_pluck-1.1.2/lib/deep_pluck/model.rb:86:in `add'
    /Users/khiav223577/.rvm/gems/ruby-2.6.4/gems/deep_pluck-1.1.2/lib/deep_pluck.rb:8:in `deep_pluck'
    /Users/khiav223577/.rvm/gems/ruby-2.6.4/gems/deep_pluck-1.1.2/lib/deep_pluck.rb:14:in `deep_pluck'
    test.rb:74:in `test_deep_pluck'
byt3pool commented 4 years ago

Oh, I mistakenly added the wrong deep_pluck command to my original question (I ran in the same error during development too) The command has to be:

Session.deep_pluck(:name, :logbook => { 'vehicle' => :manufacturer } )

If I run your test with the updated deep_pluck command

# ...
vehicle = Vehicle.create(manufacturer: 'BMW')
logbook = Logbook.create(vehicle: vehicle)
Session.create(name: 'Session_01', logbook: logbook)

class BugTest < Minitest::Test
  def test_deep_pluck
    expected = {
      'name' => 'Session_01',
      'vehicle' => {
        'manufacturer' => 'BMW'
      }
    }

    assert_equal expected, Session.deep_pluck(:name, :logbook => { 'vehicle' => :manufacturer } )
  end
end

the output is as follows:

...

Finished in 0.026050s, 38.3876 runs/s, 38.3876 assertions/s.

  1) Failure:
BugTest#test_deep_pluck [test.rb:74]:
--- expected
+++ actual
@@ -1 +1 @@
-{"name"=>"Session_01", "vehicle"=>{"manufacturer"=>"BMW"}}
+[{"name"=>"Session_01", :logbook=>{"vehicle"=>{"manufacturer"=>"BMW"}}}]

1 runs, 1 assertions, 1 failures, 0 errors, 0 skips

As you can see, the name of the through association is added to the resulting hash.


deep_pluck: 1.1.2, pluck_all: 2.0.4, rails: 5.2.3

khiav223577 commented 4 years ago

Let's sum up:

Expected return value is

['name' => 'Session_01', 'vehicle' => { 'manufacturer' => 'BMW' }]

You tried the following query, but didn't get what you want.

Session.deep_pluck(:name, 'logbook' => { 'vehicle' => :manufacturer } )
# => ['name' => 'Session_01', 'logbook' =>{ 'vehicle' => { 'manufacturer' =>'BMW' }}]

I suggested writing the query as the following, but got an error. :bug:

Session.deep_pluck(:name, 'vehicles' => :manufacturer)
# => ActiveRecord::ConfigurationError
# expected to return ['name' => 'Session_01', 'vehicle' => { 'manufacturer' => 'BMW' }]

The solution I think is that I'm going the fix the bug, and you wait new version of deep_pluck to be released. Then you can write the query as Session.deep_pluck(:name, 'vehicles' => :manufacturer) to get what you want.

khiav223577 commented 4 years ago

Just released deep_pluck 1.1.3. Try it out :)

byt3pool commented 4 years ago

Works like a charm! Thank you.

byt3pool commented 4 years ago

@khiav223577 I just found out that your fix (deep_pluck 1.1.3) only works for one direction, in this case from Session to Vehicle but not from Vehicle to Session.

# Testsetup like above

class BugTest < Minitest::Test
  # This direction does work with deep_pluck 1.1.3
  def test_deep_pluck_session_to_vehicle
    expected = [{
      'name' => 'Session_01',
      'vehicle' => {
        'manufacturer' => 'BMW'
      }
    }]

    assert_equal expected, Session.deep_pluck(:name, 'vehicle' => :manufacturer)
  end

  # This direction fails with "ActiveRecord::ConfigurationError: Can't join 'Session' to association named 'logbooks'; perhaps you misspelled it?"
  def test_deep_pluck_vehicle_to_session
    expected = [{
      'manufacturer' => 'BMW',
      'sessions' => [
        { 'name' => 'Session_01' }
      ]
    }]

    assert_equal expected, Vehicle.deep_pluck(:manufacturer, 'sessions' => :name )

    # => Finished in 0.026883s, 74.3956 runs/s, 37.1978 assertions/s.
    # 1) Error:
    # BugTest#test_deep_pluck_vehicle_to_session:
    # ActiveRecord::ConfigurationError: Can't join 'Session' to association named 'logbooks'; perhaps you misspelled it?
    # ...
  end
end

If I test with the explicit hop :logbooks => { ... } the error is gone but the output is not the expected one (similar as it was with deep_pluck <= 1.1.2 for each direction):

# Testsetup like above

class BugTest < Minitest::Test
  # ...  

  def test_deep_pluck_vehicle_to_session
    expected = [{
      'manufacturer' => 'BMW',
      'sessions' => [
        { 'name' => 'Session_01' }
      ]
    }]

    assert_equal expected, Vehicle.deep_pluck(:manufacturer, :logbooks => { 'sessions' => :name } )

    # =>   Finished in 0.061718s, 32.4056 runs/s, 32.4056 assertions/s.
    # 1) Failure:
    # BugTest#test_deep_pluck_vehicle_to_session [test.rb:87]:
    # --- expected
    # +++ actual
    # @@ -1 +1 @@
    # -[{"manufacturer"=>"BMW", "sessions"=>[{"name"=>"Session_01"}]}]
    # +[{"manufacturer"=>"BMW", :logbooks=>[{"sessions"=>[{"name"=>"Session_01"}]}]}]
  end
end
khiav223577 commented 4 years ago

Thanks for your reporting.

I found out the bug is that deep_pluck doesn't know which association should be used to joins tables. In your cases:

when you try to Vehicle.deep_pluck(:manufacturer, 'sessions' => :name) deep_pluck will try to pluck all vehicles' data and load sessions by vehicle_ids, Ex:

vehicle_ids = Vehicle.pluck(:id)
Session.joins(:logbook).where(vehicle_id: vehicle_ids)

It (Session) had to joins logbook but sadly I have no idea how to get it 😢
See the image for more detail: image

I submitted a workaround for it, see: https://github.com/khiav223577/deep_pluck/pull/36


REF:

class Vehicle < ActiveRecord::Base
  has_many :logbooks
  has_many :sessions, through: :logbooks
end

class Session < ActiveRecord::Base
  belongs_to :logbook
  has_one :vehicle, through: :logbook
end

class Logbook < ActiveRecord::Base
  belongs_to :vehicle
  has_many :sessions
end
khiav223577 commented 4 years ago

@byt3pool I released 1.1.4. It may helps.

byt3pool commented 4 years ago

It does help indeed, thank you.