Back port fix so that OPTIONAL always comes before WHERE in method ch…

…ains

lib/neo4j/active_node/has_n.rb

@@ -106,6 +106,7 @@ def #{name}_query_proxy(options = {})
                                                        start_object: self,
                                                        node: options[:node],
                                                        rel: options[:rel],
+                                                       optional: options[:optional],
                                                        context: '#{self.name}##{name}',
                                                        caller: self
                                                      })
@@ -122,8 +123,8 @@ def #{name}_rels
           end}, __FILE__, __LINE__)
 
         instance_eval(%{
-          def #{name}(node = nil, rel = nil, proxy_obj = nil)
-            #{name}_query_proxy(node: node, rel: rel, proxy_obj: proxy_obj)
+          def #{name}(node = nil, rel = nil, proxy_obj = nil, options = {})
+            #{name}_query_proxy({node: node, rel: rel, proxy_obj: proxy_obj}.merge(options))
           end
 
           def #{name}_query_proxy(options = {})
@@ -139,7 +140,7 @@ def #{name}_query_proxy(options = {})
                                                        node: options[:node],
                                                        rel: options[:rel],
                                                        context: context,
-                                                       optional: query_proxy.optional?,
+                                                       optional: options[:optional] || query_proxy.optional?,
                                                        caller: query_proxy.caller
                                                      })
           end}, __FILE__, __LINE__)
@@ -180,9 +181,9 @@ def #{name}_query_proxy(options = {})
                                                      {session: self.neo4j_session}.merge(options))
           end
 
-          def #{name}(node = nil, rel = nil, query_proxy = nil)
+          def #{name}(node = nil, rel = nil, query_proxy = nil, options = {})
             context = (query_proxy && query_proxy.context ? query_proxy.context : '#{self.name}') + '##{name}'
-            #{name}_query_proxy(query_proxy: query_proxy, node: node, rel: rel, context: context)
+            #{name}_query_proxy({query_proxy: query_proxy, node: node, rel: rel, context: context}.merge(options))
           end}, __FILE__, __LINE__)
       end
       # rubocop:enable Style/PredicateName

lib/neo4j/active_node/query/query_proxy.rb

@@ -41,6 +41,7 @@ def initialize(model, association = nil, options = {})
           @rel_var = options[:rel] || _rel_chain_var
           @session = options[:session]
           @caller = options[:caller]
+          @chain_level = options[:chain_level]
           @chain = []
           @starting_query = options[:starting_query]
           @optional = options[:optional]
@@ -87,9 +88,11 @@ def query_as(var)
                          starting_query ? (starting_query & _query_model_as(var)) : _query_model_as(var)
                        end
           # Build a query chain via the chain, return the result
-          @chain.inject(base_query.params(@params)) do |query, (method, arg)|
+          result_query = @chain.inject(base_query.params(@params)) do |query, (method, arg)|
             query.send(method, arg.respond_to?(:call) ? arg.call(var) : arg)
           end
+
+          result_query.tap { |query| query.proxy_chain_level = _chain_level }
         end
 
         # Scope all queries to the current scope.
@@ -246,6 +249,8 @@ def _association_arrow(properties = {}, create = false)
         def _chain_level
           if query_proxy = @options[:query_proxy]
             query_proxy._chain_level + 1
+          elsif @chain_level
+            @chain_level + 1
           else
             1
           end

lib/neo4j/active_node/query/query_proxy_methods.rb

@@ -114,11 +114,8 @@ def destroy(node)
 
         # A shortcut for attaching a new, optional match to the end of a QueryProxy chain.
         # TODO: It's silly that we have to call constantize here. There should be a better way of finding the target class of the destination.
-        def optional(association, node_id = nil)
-          target_qp = self.send(association)
-          model = target_qp.name.constantize
-          var = node_id || target_qp.identity
-          self.query.proxy_as(model, var, true)
+        def optional(association, node_var = nil, rel_var = nil)
+          self.send(association, node_var, rel_var, nil, optional: true)
         end
 
         private

lib/neo4j/core/query.rb

@@ -8,12 +8,15 @@ class Query
     # @return [Neo4j::ActiveNode::Query::QueryProxy] A QueryProxy object.
     def proxy_as(model, var, optional = false)
       # TODO: Discuss whether it's necessary to call `break` on the query or if this should be left to the user.
-      Neo4j::ActiveNode::Query::QueryProxy.new(model, nil,  starting_query: self.break, node: var, optional: optional)
+      Neo4j::ActiveNode::Query::QueryProxy.new(model, nil,  starting_query: self, node: var, optional: optional, chain_level: @proxy_chain_level)
     end
 
     # Calls proxy_as with `optional` set true. This doesn't offer anything different from calling `proxy_as` directly but it may be more readable.
     def proxy_as_optional(model, var)
       proxy_as(model, var, true)
     end
+    #
+    # For instances where you turn a QueryProxy into a Query and then back to a QueryProxy with `#proxy_as`
+    attr_accessor :proxy_chain_level
   end
 end

spec/e2e/query_proxy_methods_spec.rb

@@ -245,7 +245,7 @@ def destroy_called; end
 
       context 'with a valid node' do
         it 'generates a match to the given node' do
-          expect(@john.lessons.match_to(@history).to_cypher).to include('AND ID(result_lessons) =')
+          expect(@john.lessons.match_to(@history).to_cypher).to include('AND (ID(result_lessons) =')
         end
 
         it 'matches the object' do
@@ -255,7 +255,7 @@ def destroy_called; end
 
       context 'with an id' do
         it 'generates cypher using the primary key' do
-          expect(@john.lessons.match_to(@history.id).to_cypher).to include('AND result_lessons.uuid =')
+          expect(@john.lessons.match_to(@history.id).to_cypher).to include('AND (result_lessons.uuid =')
         end
 
         it 'matches' do
@@ -268,7 +268,7 @@ def destroy_called; end
           after { @john.lessons.first_rel_to(@math).destroy }
 
           it 'generates cypher using IN with the IDs of contained nodes' do
-            expect(@john.lessons.match_to([@history, @math]).to_cypher).to include('AND result_lessons.uuid IN')
+            expect(@john.lessons.match_to([@history, @math]).to_cypher).to include('AND (result_lessons.uuid IN')
             expect(@john.lessons.match_to([@history, @math]).to_a).to eq [@history]
             @john.lessons << @math
             expect(@john.lessons.match_to([@history, @math]).to_a.count).to eq 2
@@ -285,7 +285,7 @@ def destroy_called; end
 
       context 'with a null object' do
         it 'generates cypher with 1 = 2' do
-          expect(@john.lessons.match_to(nil).to_cypher).to include('AND 1 = 2')
+          expect(@john.lessons.match_to(nil).to_cypher).to include('AND (1 = 2')
         end
 
         it 'matches nil' do
@@ -390,6 +390,8 @@ def destroy_called; end
 
   describe 'optional' do
     before(:all) do
+      delete_db
+
       @lauren = IncludeStudent.create(name: 'Lauren')
       @math = IncludeLesson.create(name: 'Math')
       @science = IncludeLesson.create(name: 'Science')
@@ -402,25 +404,11 @@ def destroy_called; end
     end
 
     it 'starts a new optional match' do
-      result = @lauren.lessons(:l).optional(:teachers, :t).where(age: 40).lessons(:l).pluck('distinct l, t')
-      expect(result.count).to eq 2
-
-      expect(result[0][0]).not_to be_nil
-      # The order of results changes between Neo4j 2.1.6 and 2.2.0.
-      # Until we stop supporting 2.1.6, this workaround is necessary.
-      if result[0][0] == @science
-        expect(result[0][0]).to eq @science
-        expect(result[0][1]).to be_nil
-
-        expect(result[1][0]).to eq @math
-        expect(result[1][1]).to eq @johnson
-      else
-        expect(result[0][0]).to eq @math
-        expect(result[0][1]).to eq @johnson
-
-        expect(result[1][0]).to eq @science
-        expect(result[1][1]).to be_nil
-      end
+      puts @lauren.lessons(:l).optional(:teachers, :t).where(age: 40).query.order(l: :name).to_cypher
+      result = @lauren.lessons(:l).optional(:teachers, :t).where(age: 40).query.order(l: :name).pluck('DISTINCT l, t')
+
+      expect(result).to eq [[@math, @johnson],
+                            [@science, nil]]
     end
   end
 end

spec/unit/query_spec.rb

@@ -34,17 +34,17 @@ def self.neo4j_session
     end
 
     it 'can be built upon' do
-      @class_a.query_as(:q).match('q--p').where(p: {name: 'Brian'}).to_cypher.should == 'MATCH (q:`Person`), q--p WHERE p.name = {p_name}'
+      @class_a.query_as(:q).match('q--p').where(p: {name: 'Brian'}).to_cypher.should == 'MATCH (q:`Person`), q--p WHERE (p.name = {p_name})'
     end
   end
 
   describe '#query_as' do
     it 'generates a basic query' do
-      @class_a.new.query_as(:q).to_cypher.should == 'MATCH (q:`Person`) WHERE ID(q) = {ID_q}'
+      @class_a.new.query_as(:q).to_cypher.should == 'MATCH (q:`Person`) WHERE (ID(q) = {ID_q})'
     end
 
     it 'can be built upon' do
-      @class_a.new.query_as(:q).match('q--p').return(p: :name).to_cypher.should == 'MATCH (q:`Person`), q--p WHERE ID(q) = {ID_q} RETURN p.name'
+      @class_a.new.query_as(:q).match('q--p').return(p: :name).to_cypher.should == 'MATCH (q:`Person`), q--p WHERE (ID(q) = {ID_q}) RETURN p.name'
     end
   end
 end