Skip to content
This repository
Browse code

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

Relation#merge should not lose readonly(false) flag.
Conflicts:
	activerecord/CHANGELOG.md
  • Loading branch information...
commit 843e9c4c4164fa62215529bc30060ec8ecb5a269 1 parent 3b64d5b
Rafael Mendonça França authored September 11, 2013
4  activerecord/CHANGELOG.md
Source Rendered
... ...
@@ -1,5 +1,9 @@
1 1
 ## unreleased ##
2 2
 
  3
+*   Fix `AR::Relation#merge` sometimes failing to preserve `readonly(false)` flag.
  4
+
  5
+    *thedarkone*
  6
+
3 7
 *   PostgreSQL adapter recognizes negative money values formatted with
4 8
     parentheses (eg. `($1.25) # => -1.25`)).
5 9
     Fixes #11899.
6  activerecord/lib/active_record/relation/merger.rb
@@ -62,7 +62,11 @@ def normal_values
62 62
       def merge
63 63
         normal_values.each do |name|
64 64
           value = values[name]
65  
-          relation.send("#{name}!", *value) unless value.blank?
  65
+          # The unless clause is here mostly for performance reasons (since the `send` call might be moderately
  66
+          # expensive), most of the time the value is going to be `nil` or `.blank?`, the only catch is that
  67
+          # `false.blank?` returns `true`, so there needs to be an extra check so that explicit `false` values
  68
+          # don't fall through the cracks.
  69
+          relation.send("#{name}!", *value) unless value.nil? || (value.blank? && false != value)
66 70
         end
67 71
 
68 72
         merge_multi_values
8  activerecord/test/cases/relation_test.rb
@@ -179,6 +179,14 @@ def test_references_values_dont_duplicate
179 179
       assert_equal ['foo = bar'], relation.where_values
180 180
     end
181 181
 
  182
+    def test_merging_readonly_false
  183
+      relation = Relation.new FakeKlass, :b
  184
+      readonly_false_relation = relation.readonly(false)
  185
+      # test merging in both directions
  186
+      assert_equal false, relation.merge(readonly_false_relation).readonly_value
  187
+      assert_equal false, readonly_false_relation.merge(relation).readonly_value
  188
+    end
  189
+
182 190
     def test_relation_merging_with_merged_joins_as_symbols
183 191
       special_comments_with_ratings = SpecialComment.joins(:ratings)
184 192
       posts_with_special_comments_with_ratings = Post.group("posts.id").joins(:special_comments).merge(special_comments_with_ratings)

0 notes on commit 843e9c4

Please sign in to comment.
Something went wrong with that request. Please try again.