From be81b5066074fee8126144d072c6132b93d1fe39 Mon Sep 17 00:00:00 2001
From: Maxime Lapointe <hunter_spawn@hotmail.com>
Date: Fri, 28 Jul 2017 17:53:50 -0400
Subject: Edits following the reviews

---
 .../lib/active_record/relation/where_clause.rb     | 14 +++---
 .../test/cases/relation/where_clause_test.rb       | 56 ++++++++++++++--------
 2 files changed, 43 insertions(+), 27 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb
index 5b68c25bdd..752bb38481 100644
--- a/activerecord/lib/active_record/relation/where_clause.rb
+++ b/activerecord/lib/active_record/relation/where_clause.rb
@@ -36,12 +36,14 @@ module ActiveRecord
         common = self - left
         right = other - common
 
-        return common if left.empty? || right.empty?
-
-        or_clause = WhereClause.new(
-          [left.ast.or(right.ast)]
-        )
-        common + or_clause
+        if left.empty? || right.empty?
+          common
+        else
+          or_clause = WhereClause.new(
+            [left.ast.or(right.ast)],
+          )
+          common + or_clause
+        end
       end
 
       def to_h(table_name = nil)
diff --git a/activerecord/test/cases/relation/where_clause_test.rb b/activerecord/test/cases/relation/where_clause_test.rb
index 42cf35ac9c..e5eb159d36 100644
--- a/activerecord/test/cases/relation/where_clause_test.rb
+++ b/activerecord/test/cases/relation/where_clause_test.rb
@@ -182,36 +182,50 @@ class ActiveRecord::Relation
     end
 
     test "or places common conditions before the OR" do
-      wcs = (0..6).map do |i|
-        WhereClause.new([table["col_#{i}"].eq(bind_param(i))])
-      end
-
-      actual = wcs[0]
-      expected = wcs[0]
+      a = WhereClause.new(
+        [table["id"].eq(bind_param(1)), table["name"].eq(bind_param("Sean"))],
+      )
+      b = WhereClause.new(
+        [table["id"].eq(bind_param(1)), table["hair_color"].eq(bind_param("black"))],
+      )
 
-      actual = (actual + wcs[1]).or(actual + wcs[2])
-      expected = expected + wcs[1].or(wcs[2])
+      common = WhereClause.new(
+        [table["id"].eq(bind_param(1))],
+      )
 
-      actual = (actual + wcs[3] + wcs[4]).or(actual + wcs[5] + wcs[6])
-      expected = expected + (wcs[3] + wcs[4]).or(wcs[5] + wcs[6])
+      or_clause = WhereClause.new([table["name"].eq(bind_param("Sean"))])
+        .or(WhereClause.new([table["hair_color"].eq(bind_param("black"))]))
 
-      assert_equal expected, actual
+      assert_equal common + or_clause, a.or(b)
     end
 
-    test "or will use common conditions only if one side only has common conditions" do
-      common = WhereClause.new(
-        [table["id"].eq(bind_param(1)), "foo = bar"]
-      )
+    test "or can detect identical or as being a common condition" do
+      common_or = WhereClause.new([table["name"].eq(bind_param("Sean"))])
+        .or(WhereClause.new([table["hair_color"].eq(bind_param("black"))]))
 
-      extra = WhereClause.new([table["extra"].eq(bind_param("pluto"))])
+      a = common_or + WhereClause.new([table["id"].eq(bind_param(1))])
+      b = common_or + WhereClause.new([table["foo"].eq(bind_param("bar"))])
 
-      actual_extra_only_on_left = (common + extra).or(common)
-      actual_extra_only_on_right = (common).or(common + extra)
+      new_or = WhereClause.new([table["id"].eq(bind_param(1))])
+        .or(WhereClause.new([table["foo"].eq(bind_param("bar"))]))
 
-      expected = common
+      assert_equal common_or + new_or, a.or(b)
+    end
+
+    test "or will use only common conditions if one side only has common conditions" do
+      only_common = WhereClause.new([
+        table["id"].eq(bind_param(1)),
+        "foo = bar",
+      ])
+
+      common_with_extra = WhereClause.new([
+        table["id"].eq(bind_param(1)),
+        "foo = bar",
+        table["extra"].eq(bind_param("pluto")),
+      ])
 
-      assert_equal expected, actual_extra_only_on_left
-      assert_equal expected, actual_extra_only_on_right
+      assert_equal only_common, only_common.or(common_with_extra)
+      assert_equal only_common, common_with_extra.or(only_common)
     end
 
     private
-- 
cgit v1.2.3