aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorSean Griffin <sean@seantheprogrammer.com>2017-07-17 10:07:02 -0400
committerSean Griffin <sean@seantheprogrammer.com>2017-07-17 10:17:18 -0400
commit5c71000d086cc42516934415b79380c2224e1614 (patch)
treeacad52770bc3f3c7f604ef45885b700d54f4301c /activerecord
parent3bda7ff763c55d33d18318bb2b3ea5c295f34000 (diff)
downloadrails-5c71000d086cc42516934415b79380c2224e1614.tar.gz
rails-5c71000d086cc42516934415b79380c2224e1614.tar.bz2
rails-5c71000d086cc42516934415b79380c2224e1614.zip
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.
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md7
-rw-r--r--activerecord/lib/active_record/reflection.rb6
-rw-r--r--activerecord/test/cases/scoping/default_scoping_test.rb10
3 files changed, 19 insertions, 4 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 5ffbed28f6..6c7e2b19e7 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*
+
* Change sqlite3 boolean serialization to use 1 and 0
SQLite natively recognizes 1 and 0 as true and false, but does not natively
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index a453ca55c7..4f53280b5c 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -219,10 +219,8 @@ module ActiveRecord
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
klass.default_scoped(build_scope(table, predicate_builder))
end
diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb
index bd05cb4787..a5061cfce7 100644
--- a/activerecord/test/cases/scoping/default_scoping_test.rb
+++ b/activerecord/test/cases/scoping/default_scoping_test.rb
@@ -377,6 +377,16 @@ class DefaultScopingTest < ActiveRecord::TestCase
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