Add: ability for active node instances to use class scopes

[jorroll]

This is a helpful patch I added to my own app, which may or may not be appropriate to add to ActiveNode.

Currently, the following fails:

class Person
  include Neo4j::ActiveNode

  property :country

  has_many :out, :friends, type: :FRIEND, model_class: "Person"

  scope :american_friends, ->{ where(country: 'United States') }
end

Person.american_friends # works

Person.first.american_friends #=> error, unknown method 'american_friends'

Person.first.as(:a).american_friends # works

This can be annoying. For example:

Employers.where(fun: true).jobs.employees.first.american_friends.phone_number
# => error on `first.american_friends`

Employers.where(fun: true).jobs.employees.first.as(:e).american_friends.phone_number
# => works

This PR overloads method_missing to check if the missing method is a scope in the ancestor's chain. This allows:

Employers.where(fun: true).jobs.employees.first.american_friends.phone_number

# or

person = Person.create
person.american_friends

If you think this is appropriate to add, I'll make specs / update the docs for it.

[jorroll]

I suppose Employers.where(fun: true).jobs.employees.first.american_friends.phone_number isn't a great example, because it could be solved better with .limit(). But you get the idea. Mostly helpful when starting a chain from an ActiveNode instance.

I suppose a point against this patch is that it would make it easier for someone to do Employers.where(fun: true).jobs.employees.first.american_friends.phone_number, which isn't an optimal query.

It would also make it easier for someone to do: employer.supervisor.american_friends (where supervisor is a has_one relationship). Ordinarily, this would generate an error, possibly encouraging them to write the better query: employer.supervisor(chainable: true).american_friends.

Still, personally I'd let folks take the easy out if they want.

[cheerfulstoic]

One clarification, I think the scope would be better as american rather than american_friends, right? Because if you do Person.all.american_friends as you have it, it doesn't traverse the friends association, but just filtered for all people who are from the US. If you make the scope american, then you would do Person.all.friends.american or person_node.friends.american, right? I think that's the beauty of the composibility of scopes and associations (scopes filter, associations traverse).

So does that feedback make this PR not needed?

[jorroll]

Oh you're right, I didn't name / set that up well. All of these examples are contrived, and I didn't spend enough time on them apparently. This being said, while you could do person_node.friends.american, you could not do person_node.american.friends (pretend that .american is a scope that you'd want to use there, maybe it's short for .friends.where(name: 'bob').employer.jobs.employees). You can't do person_node.american.friends because you can't call scopes on Person.new. You can only call scopes on the Person class or a person association proxy. I imagine that the difference could be confusing for a newbie ("what's an 'association proxy'?") and I could imagine folks having trouble knowing when they need to create an association proxy with "as()" person_node.as(:p).american.friends. That's what this patch would "fix".

As to if it's needed or not. Personally, I don't need this (I already patched my own app) and, as I mentioned previously, I'm also not 100% sure if this is a "good" thing to add. It does make it easier to make suboptimal chains. I do think it would likely make things easier for new folks though. It's main usage being Person.new.scope. I think you're in a better position to judge if this is a good idea to add or not.

[cheerfulstoic]

I may be confused. What would happen if you just did person_node.american without putting .friends on the end? If you do some_people_scope.american it returns the original scope filtered for Americans. But for an individual node, is is supposed to return a scope which either has that one person (if they are American) or no people (if they aren't)?

It seems like maybe you're trying to apply the american scope to friends, but if that comes later then you can't do that. Scopes (at least as inspired by ActiveRecord) come after query-type objects in order to modify them.

[jorroll]

You'd get a "method not found" error if you tried to call person_node.american. Instances of a class do not have access to their parent's scopes (<-- this is the problem). some_people_scope.american would work, because some_people_scope is a proxy object, and proxy objects use method_missing to forward scope methods on to the ActiveNode class.

For example, this raises an error:

class Person
  include Neo4j::ActiveNode

  scope :a_scope, ->{ imagine_that_this_is_a_scope }
end

Person.new.a_scope #=> ERROR!

RE:

But for an individual node, is is supposed to return a scope which either has that one person (if they are American) or no people (if they aren't)?

For an individual node, (after this patch) a scope will do the same thing as it always does: let you call predefined arbitrary (presumably query) methods on the node.

[cheerfulstoic]

I think I'm missing something so I pinged you on Gitter

[cheerfulstoic]

Our conversation was pretty clarifying, so I made a gist of it for posterity.

In the end, I think that this addition doesn't hurt anything. It would only be used in certain ways of using scopes and can be ignored in other cases. So I think that this can be added. I would also like to see docs around it and this might even be a good opportunity to explain this as an alternate use of scopes in the docs.

I think that at the beginning of this PR a lot of my confusion came from the american_friends example. From ActiveRecord (and the way that I generally use ActiveNode), in my mind adjectives are one thing and nouns are another. Since ActiveRecord (and ActiveNode) emphasizes composability, I would expect there to be.americanand.friends` which you could use separately or together. I can see examples of making a deep method/scope which does a more complex query and which filters in the middle where you would need to encapsulate things. I would just want to make sure the documents reflect that line where simple things should be composable whenever possible.

[klobuczek]

Sorry coming late into the discussion. This is a feature we wanted for a long time but never got the time to contribute it properly to this gem. We use scopes extensively for any possible association and query chain fragments. We probably have few dozens of those. For every such a scope (abstracting from the suitability of the name scope) we define a companion instance method. @thefliik thanks for taking initiative on that. However, I would not implement it with method missing. The instance method should be dynamically defined when the scope is defined in scope.rb as you already commented yourself.

[jorroll]

@klobuczek

However, I would not implement it with method missing. The instance method should be dynamically defined when the scope is defined in scope.rb as you already commented yourself.

Actually that wasn't what I was suggesting, but I think you are correct. That's a better solution. I think I chose method_missing simply because it mimics how AssociationProxy forwards methods on to QueryProxy, but I like your suggestion better.

I'm pretty busy at the moment though. I'll get back to this when I can.

[jorroll]

Closed in favor of #1512