Merge pull request #12194 from thedarkone/readonly-merger-fix

Relation#merge should not lose readonly(false) flag.

3 files changed, 17 insertions, 1 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 9041f9d33b..d2f2c745a0 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Fix `AR::Relation#merge` sometimes failing to preserve `readonly(false)` flag.
+
+    *thedarkone*
+
 *   Re-use `order` argument pre-processing for `reorder`.
 
     *Paul Nikitochkin*
diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb
index 530c47d0d0..c05632e688 100644
--- a/activerecord/lib/active_record/relation/merger.rb
+++ b/activerecord/lib/active_record/relation/merger.rb
@@ -58,7 +58,11 @@ module ActiveRecord
       def merge
         normal_values.each do |name|
           value = values[name]
-          relation.send("#{name}!", *value) unless value.blank?
+          # The unless clause is here mostly for performance reasons (since the `send` call might be moderately
+          # expensive), most of the time the value is going to be `nil` or `.blank?`, the only catch is that
+          # `false.blank?` returns `true`, so there needs to be an extra check so that explicit `false` values
+          # don't fall through the cracks.
+          relation.send("#{name}!", *value) unless value.nil? || (value.blank? && false != value)
         end
 
         merge_multi_values
diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb
index a327b0d3e5..e77de19ac3 100644
--- a/activerecord/test/cases/relation_test.rb
+++ b/activerecord/test/cases/relation_test.rb
@@ -193,6 +193,14 @@ module ActiveRecord
       assert_equal ['foo = bar'], relation.where_values
     end
 
+    def test_merging_readonly_false
+      relation = Relation.new FakeKlass, :b
+      readonly_false_relation = relation.readonly(false)
+      # test merging in both directions
+      assert_equal false, relation.merge(readonly_false_relation).readonly_value
+      assert_equal false, readonly_false_relation.merge(relation).readonly_value
+    end
+
     def test_relation_merging_with_merged_joins_as_symbols
       special_comments_with_ratings = SpecialComment.joins(:ratings)
       posts_with_special_comments_with_ratings = Post.group("posts.id").joins(:special_comments).merge(special_comments_with_ratings)