Polymorphic association reflection

rubyonrails-talk
1

Hi, while chasing N+1 queries i’m stuck with this association/reflection problem:

I have two models:

# model/proposal.rb
class Proposal < ApplicationRecord
    # ...
    has_many :quote_items, -> { order(:position) }, as: :salable, dependent: :destroy
    has_many :billable_quote_items, lambda {
                                      where(item_type: "billable").order(:position)
                                    }, class_name: "QuoteItem", inverse_of: :salable
   # ...
end

#model/quote_item
class QuoteItem < ApplicationRecord
    # ...
    belongs_to :salable, polymorphic: true
    # ...
end

The problem is that reflection is not working properly, when i do:

proposal = Proposal.find_by(title: "old_title")
proposal.billable_quote_items # fetch a first time the billable_quote_items association
proposal.quote_items.update(title: "new_title")

proposal.billable_quote_items.first.title
=> "old_title # Result
=> "new_title # Expected

Adding this relationship billable_quote_items is important to include it in my query and avoid N+1, a lot of specs break because of the problem above.

For now I prefered to stay away from .reload.

Open to any advice a lot! Thanks

2

I’m not really knowledgeable with polymorphic associations, but reading through the rails guides, it seems it was intended to have a model belong to more than one model, not have 2 relationships on the same model. But I really don’t know if this is common practice. And my point is just: maybe because you’re trying to have 2 associations like this on the same model is what’s causing things to not be updated as you expect.

That being said… I don’t know… wouldn’t it be easier to use a scope or something? And update the quote_item from that query, right?

Again, not knowledgeable on these things, just saw that you got no answers and was trying to through some ideas out there hahaha

EDIT: I did find this SO question that might help. Maybe?

3

Thanks for you answer Mateus,

I cannot use the scope because I need to include it in my query to avoid the N+1.

Here is quick example to illustrate

# controllers/proposals_controllers.rb
@proposals.include(:quote_items).each do |proposal|
    proposal.net_total
end

# models/proposal.rb
has_many :quote_items

def net_total
  quote_items.billable.sum(:amount) # N+1
end

#models/quote_item.rb
scope :billable { where(type: "billable") }

To avoid this N+1, I am not aware of another solution to this association: billable_quote_items

4

Your problem isn’t really with polymorphic associations: it’s with having two associations pointing to the same record(s).

Rails doesn’t recognize that it’s loading the same record twice, so it’s loading two separate copies of the same record which is why the copy you’re not updating isn’t reflecting the update.

To my knowledge Rails has no solution other than reload

5

@jjf21 Maybe you can have net_total do this?

def net_total
  quoted_items.select(&:billable).sum(&:amount)
end

Since you preloaded quoted_items, the select should just do things in memory, right?

And you avoid the polymorphic association

6

Really two things here –

  1. Just as @mateusdeap has indicated, polymorphism isn’t doing you any favours here. I suppose if other models are also saleable then it starts to make sense. But in this example it doesn’t have merit. I’ll scrap it in my demo code below.

  2. As @dcunning has said, your example is working with two different copies in RAM of the same thing.

To fix this, consider this simple setup:

class Proposal < ApplicationRecord
  has_many :quote_items

  def billable_quote_items
    quote_items.where('quote_items.item_type' => 'billable')
  end
end

class QuoteItem < Application Record
  # Note that the quote_items table has a +proposal_id+ column which is a bigint
  belongs_to :proposal
end

Because ActiveRecord automatically determines appropriate inverse_of values when tables are named according to conventions, there is no need to separately call out any inverse_of. It just happens. And then this code works just like you had wanted:

# Seed a few rows of data
new_proposal = Proposal.create(title: 'Taco Party')
new_proposal.quote_items.create(title: 'Dancing', item_type: 'non-billable')
new_proposal.quote_items.create(title: 'Lemons', item_type: 'billable')

# Your scenario from above
# Note that your 'old_title' was referring to the title of a proposal object,
# and 'new_title' was being applied to QuoteItem objects.  So that's a
# little weird, and I'll just use 'Taco Party' to retrieve the proposal instead.
proposal = Proposal.find_by(title: "Taco Party")
proposal.billable_quote_items # Fetch for the first time the related billable_quote_items
proposal.quote_items.update(title: "new_title") # Update _all_ related QuoteItem objects to have the same title

proposal.billable_quote_items.first.title
=> "new_title" # Result
7

Thanks for your answer.

@mateusdeap yes I had this idea in the first place but with hundreds of quote_items it was afraid of performance. Moreover I wanted to know what would be the Rails way of doing it :slight_smile:

@Lorin_Thwaits Yes the salable association is polymorphic, as quote_items is linked to multiple model (Proposal and Invoice)

I think my best solution will be to replace at some places the .quote_items to .billable_quote_items.

8

Cool. Given that, I do think your best option is to have a combination of polymorphism and what we said. And just test things out. Try to seed a db with about the same amount of quote items you have in production (or even replicate production, I suppose) and just run the code and see how much of a performance hit it would actually be. It might not be super quick, but probably better than N+1 queries?