From 3630d6354cab31bb233a1f1d7b1a4d2c24aef54d Mon Sep 17 00:00:00 2001
From: Sean Griffin <sean@seantheprogrammer.com>
Date: Mon, 17 Jul 2017 10:07:02 -0400
Subject: [PATCH] Post.joins(:users) should not be affected by
 `User.current_scope`

This change was introduced by #18109. The intent of that change was to
specifically apply `unscoped`, not to allow all changes to
`current_scope` to affect the join. The idea of allowing `current_scope`
to affect joins is interesting and potentially more consistent, but has
sever problems associated with it. The fact that we're specifically
stripping out joins indicates one such problem (and potentially leads to
invalid queries).

Ultimately it's difficult to reason about what `Posts.joins(:users)`
actually means if it's affected by `User.current_scope`, and it's
difficult to specifically control what does or doesn't get added. If we
were starting from scratch, I don't think I'd have `joins` be affected
by `default_scope` either, but that's too big of a breaking change to
make at this point.

With this change, we no longer apply `current_scope` when bringing in
joins, with the singular exception of the motivating use case which
introduced this bug, which is providing a way to *opt-out* of having the
default scope apply to joins.

Fixes #29338.
---
 activerecord/CHANGELOG.md                              |  7 +++++++
 activerecord/lib/active_record/reflection.rb           |  6 ++----
 .../test/cases/scoping/default_scoping_test.rb         | 10 ++++++++++
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 2df4a16d62245..6999a95ac50a8 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   `Relation#joins` is no longer affected by the target model's
+    `current_scope`, with the exception of `unscoped`.
+
+    Fixes #29338.
+
+    *Sean Griffin*
+
 *   Previously, when building records using a `has_many :through` association,
     if the child records were deleted before the parent was saved, they would
     still be persisted. Now, if child records are deleted before the parent is saved
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 82845e2c64dd4..185e1ed59a948 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -197,10 +197,8 @@ def join_scopes(table, predicate_builder) # :nodoc:
       end
 
       def klass_join_scope(table, predicate_builder) # :nodoc:
-        if klass.current_scope
-          klass.current_scope.clone.tap { |scope|
-            scope.joins_values = scope.left_outer_joins_values = [].freeze
-          }
+        if klass.current_scope && klass.current_scope.values.empty?
+          klass.unscoped
         else
           relation = ActiveRecord::Relation.create(
             klass,
diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb
index 89fb434b27d84..7a599eeccc821 100644
--- a/activerecord/test/cases/scoping/default_scoping_test.rb
+++ b/activerecord/test/cases/scoping/default_scoping_test.rb
@@ -372,6 +372,16 @@ def test_default_scope_with_joins
                  Comment.joins(:post).count
   end
 
+  def test_joins_not_affected_by_scope_other_than_default_or_unscoped
+    without_scope_on_post = Comment.joins(:post).to_a
+    with_scope_on_post = nil
+    Post.where(id: [1, 5, 6]).scoping do
+      with_scope_on_post = Comment.joins(:post).to_a
+    end
+
+    assert_equal with_scope_on_post, without_scope_on_post
+  end
+
   def test_unscoped_with_joins_should_not_have_default_scope
     assert_equal SpecialPostWithDefaultScope.unscoped { Comment.joins(:special_post_with_default_scope).to_a },
                  Comment.joins(:post).to_a