ActiveRecord::Associations::CollectionAssociation#sum bug for new records #7928

Closed
cupakromer opened this Issue Oct 12, 2012 · 1 comment

Projects

None yet

3 participants

@cupakromer
class Cart < ActiveRecord::Base
  has_many :line_items

  def total
    line_items.sum(&:price)
  end
end

class LineItem < ActiveRecord::Base
  belongs_to :product
  belongs_to :cart

  attr_accessible :amount, :product

  def price
    product.price * amount
  end
end


product = Product.create!(name: 'Ruby shirt', price: 2.25)
cart = Cart.new
cart.line_items.build do |item|
  item.amount = 2
  item.products = product
end

cart.line_items.sum(&:price)
#=> 0

cart.line_items.to_a.sum(&:price)
#=> #<BigDecimal:7ff4d34ca7c8,'0.0',9(36)>

cart.line_items.map(&:price).sum
#=> #<BigDecimal:7ff4d3373b40,'0.0',9(36)>

It seems in (Rails 3.2.8, Ruby 1.9.3) that has_many is creating
collection.sum.
Though this isn't reflected in A Guide to Active Record Associations
or ActiveRecord::Associations::ClassMethods API docs;
though it is briefly listed in the "Collection associations (one-to-many / many-to-many)"
chart, but not anywhere else in the doc.

According to the code, it will always try to hit the database using a SQL sum.
However, in this particular instance, the model isn't saved. So there's nothing
in the database. I would think it should pull from the cached copy.

Also, there is the potential where the model is saved, but the collection
was modified but not saved yet (i.e. with a build); this could cause
un-expected sums.

Line 28 in the code above returns 0 ignoring the built association.

A sample app with the issue reproduced is available here: https://github.com/cupakromer/sum_bug

I've added unit sests are in test/unit/cart_test.rb

To reproduce this issue:

  • Clone this repo
  • Run bundle
  • Run rake
@vipulnsward
Member

This is associated to #5215, in the way how association methods are currently being handled.

@jonleighton jonleighton added a commit that closed this issue Nov 9, 2012
@jonleighton jonleighton Delegate all calculations to the scope.
So that the scope may be a NullRelation and return a result without
executing a query.

Fixes #7928
edd94ce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment