From b27fb2d8e76a31eb0d12b0bcea1fbd12dcf35320 Mon Sep 17 00:00:00 2001 From: yui-knk Date: Sun, 18 Oct 2015 10:51:03 +0900 Subject: [PATCH] Green version of moving the handling of supported arguments to `where` This commit follow up of 4d8f62d. The difference from 4d8f62d are below: * Change `WhereClauseFactory` to accept `Arel::Nodes::Node` * Change test cases of `relation_test.rb` --- .../lib/active_record/relation/query_methods.rb | 2 -- .../active_record/relation/where_clause_factory.rb | 4 +++- activerecord/test/cases/relation_test.rb | 11 ++++------- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 0dcecbd42d..55fd0e0b52 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -552,8 +552,6 @@ def where(opts = :chain, *rest) WhereChain.new(spawn) elsif opts.blank? self - elsif !opts.is_a?(String) && !opts.respond_to?(:to_h) - raise ArgumentError, "Unsupported argument type: #{opts} (#{opts.class})" else spawn.where!(opts, *rest) end diff --git a/activerecord/lib/active_record/relation/where_clause_factory.rb b/activerecord/lib/active_record/relation/where_clause_factory.rb index 83ac074f97..a81ff98e49 100644 --- a/activerecord/lib/active_record/relation/where_clause_factory.rb +++ b/activerecord/lib/active_record/relation/where_clause_factory.rb @@ -20,8 +20,10 @@ def build(opts, other) attributes, binds = predicate_builder.create_binds(attributes) parts = predicate_builder.build_from_hash(attributes) - else + when Arel::Nodes::Node parts = [opts] + else + raise ArgumentError, "Unsupported argument type: #{opts} (#{opts.class})" end WhereClause.new(parts, binds) diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index b0fb905760..675149556f 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -57,9 +57,6 @@ def test_extensions def test_empty_where_values_hash relation = Relation.new(FakeKlass, :b, nil) assert_equal({}, relation.where_values_hash) - - relation.where! :hello - assert_equal({}, relation.where_values_hash) end def test_has_values @@ -153,10 +150,10 @@ def test_references_values_dont_duplicate end test 'merging a hash into a relation' do - relation = Relation.new(FakeKlass, :b, nil) - relation = relation.merge where: :lol, readonly: true + relation = Relation.new(Post, Post.arel_table, Post.predicate_builder) + relation = relation.merge where: {name: :lol}, readonly: true - assert_equal Relation::WhereClause.new([:lol], []), relation.where_clause + assert_equal({"name"=>:lol}, relation.where_clause.to_h) assert_equal true, relation.readonly_value end @@ -185,7 +182,7 @@ def test_references_values_dont_duplicate end test '#values returns a dup of the values' do - relation = Relation.new(FakeKlass, :b, nil).where! :foo + relation = Relation.new(Post, Post.arel_table, Post.predicate_builder).where!(name: :foo) values = relation.values values[:where] = nil