From 566f1fd068711dfe557bef63406f8dd6d41d473d Mon Sep 17 00:00:00 2001
From: Aaron Wortham <wortham.aaron@gmail.com>
Date: Wed, 9 Aug 2017 10:24:13 -0700
Subject: [PATCH] Ensure sum honors distinct on has_many through

When using a has_many through relation and then summing an attribute
the distinct was not being used. This will ensure that when summing
an attribute, the number is only used once when distinct has been used.
---
 activerecord/CHANGELOG.md                     |  6 +++++
 .../active_record/relation/calculations.rb    |  3 +++
 .../has_many_through_associations_test.rb     | 26 +++++++++++++++++++
 3 files changed, 35 insertions(+)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index c1c511a65c..03609a7a05 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Ensure `sum` honors `distinct` on `has_many :through` associations
+
+    Fixes #16791
+
+    *Aaron Wortham
+
 *   When using `Relation#or`, extract the common conditions and put them before the OR condition.
 
     *Maxime Handfield Lapointe*
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index d3b5be6bce..42d43224fa 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -259,6 +259,9 @@ def execute_simple_calculation(operation, column_name, distinct) #:nodoc:
           column = aggregate_column(column_name)
 
           select_value = operation_over_aggregate_column(column, operation, distinct)
+          if operation == "sum" && distinct
+            select_value.distinct = true
+          end
 
           column_alias = select_value.alias
           column_alias ||= @klass.connection.column_name_for_operation(operation, select_value)
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 4c2723addc..f4e907ba90 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -1139,6 +1139,32 @@ def test_has_many_through_obeys_order_on_through_association
     assert_equal ["parrot", "bulbul"], owner.toys.map { |r| r.pet.name }
   end
 
+  def test_has_many_through_associations_sum_on_columns
+    post1 = Post.create(title: "active", body: "sample")
+    post2 = Post.create(title: "inactive", body: "sample")
+
+    person1 = Person.create(first_name: "aaron", followers_count: 1)
+    person2 = Person.create(first_name: "schmit", followers_count: 2)
+    person3 = Person.create(first_name: "bill", followers_count: 3)
+    person4 = Person.create(first_name: "cal", followers_count: 4)
+
+    Reader.create(post_id: post1.id, person_id: person1.id)
+    Reader.create(post_id: post1.id, person_id: person2.id)
+    Reader.create(post_id: post1.id, person_id: person3.id)
+    Reader.create(post_id: post1.id, person_id: person4.id)
+
+    Reader.create(post_id: post2.id, person_id: person1.id)
+    Reader.create(post_id: post2.id, person_id: person2.id)
+    Reader.create(post_id: post2.id, person_id: person3.id)
+    Reader.create(post_id: post2.id, person_id: person4.id)
+
+    active_persons = Person.joins(:readers).joins(:posts).distinct(true).where("posts.title" => "active")
+
+    assert_equal active_persons.map(&:followers_count).reduce(:+), 10
+    assert_equal active_persons.sum(:followers_count), 10
+    assert_equal active_persons.sum(:followers_count), active_persons.map(&:followers_count).reduce(:+)
+  end
+
   def test_has_many_through_associations_on_new_records_use_null_relations
     person = Person.new
 
-- 
GitLab